You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@qpid.apache.org by "Day, Jem" <jd...@paypal.com.INVALID> on 2016/06/06 15:59:32 UTC
Re: ProtonJ - Bouncy Castle Dependency
Also, opened a JIRA for this:
https://issues.apache.org/jira/browse/PROTON-1224
It would be great if this could be merged/incorporated in 0.13 if possible.
Thanks
On 5/31/16, 10:15 AM, "Day, Jem" <jd...@paypal.com.INVALID> wrote:
>
>I’ve opened a PR for this change: https://github.com/apache/qpid-proton/pull/74
>
>Verified test-suite using BC version 1.48 (where the deprecated class was removed) and also version 1.54.
>
>@Rob – This is probably not the most ideal change based on your comments, I decided to concentrate on the BC version update rather than completely re-working it.
>
>Jem..
>
>On 5/5/16, 7:02 AM, "Robbie Gemmell" <ro...@gmail.com> wrote:
>
>>On 5 May 2016 at 12:37, Rob Godfrey <ro...@gmail.com> wrote:
>>> I think the idea was that Proton-J should support SSL/TLS even without
>>> bouncy castle, but it would only be able to use the JVM default
>>> keystore/truststore (as set by system properties)...
>>
>>Ah, well if that currently works, that could be reason to keep the
>>reflection then if keeping bouncycastle, yes.
>>
>>> Having to use PEM
>>> files is not a particularly java idiomatic way of configuring SSL/TLS, but
>>> was required in order to preserve API equivalence with proton-c.
>>> Personally I'd think we'd want to avoid a hard dependency on BouncyCastle
>>> to simply support SSL/TLS, and if we want to parse PEM files then just do
>>> the necessary decoding ourselves.
>>>
>>> -- Rob
>>>
>>> On 5 May 2016 at 10:19, Robbie Gemmell <ro...@gmail.com> wrote:
>>>
>>>> I can't speak to why BC is used, but if it is then using provided
>>>> scope instead of reflection seems reasonable. The file history shows
>>>> it used to be a hard dependency and was later changed to use
>>>> reflection to make it a runtime dependency. Its not clear if folks
>>>> were even aware of provided/optional dependencies at the time.
>>>>
>>>> Robbie
>>>>
>>>> On 4 May 2016 at 23:04, Day, Jem <jd...@paypal.com.invalid> wrote:
>>>> >
>>>> > Rob,
>>>> >
>>>> > I was also wondering that; my assumption was that BC was being leveraged
>>>> to reduce configuration concerns with JCE providers etc and to keep
>>>> everything “in house”.
>>>> >
>>>> > I do have a question on style/approach : The class in question jumps
>>>> through reflection hoops to, I assume, remove the need for a hard
>>>> dependency on the BC libraries. In reality though the code DOES have an
>>>> absolute dependency on those classes to function correctly so one might
>>>> argue it would be better to declare a maven <scope>provided</scope>
>>>> dependency on BC. This would simplify the code greatly without changing the
>>>> usage model for existing users of the proton-j library (it won’t result in
>>>> everybody downloading BC, and it won’t result in version conflicts) - you
>>>> should get the best of both worlds so long as the code is constructed in
>>>> manner where it fails gracefully if the desired dependencies aren’t present.
>>>> >
>>>> > I do, of course, have a subtle hidden agenda here as the changes
>>>> required to use the more recent versions of BC seems to require leveraging
>>>> more of their capabilities, and doing that by via reflection just seems
>>>> wrong ...
>>>> >
>>>> > Opinions/Comments ???
>>>> >
>>>> >
>>>> >
>>>> > On 5/4/16, 7:19 AM, "Rob Godfrey" <ro...@gmail.com> wrote:
>>>> >
>>>> >>As an aside, I think it should be possible to eliminate the bouncy castle
>>>> >>dependency entirely, it's pretty easy to convert PEM files into the
>>>> binary
>>>> >>encoding which can then be used directly by Java (and in the Certificates
>>>> >>case, the Java X509 CertificateFactory will work directly off PEM files).
>>>> >>
>>>> >>-- Rob
>>>> >>
>>>> >>On 4 May 2016 at 15:01, Robbie Gemmell <ro...@gmail.com> wrote:
>>>> >>
>>>> >>> On 3 May 2016 at 22:38, Day, Jem <jd...@paypal.com.invalid> wrote:
>>>> >>> > Hi Guys,
>>>> >>> >
>>>> >>> > ‘SslEngineFacadeFactory’ uses reflection on some bouncy-castle
>>>> classes
>>>> >>> to load certificates and keys from the configured PEM files.
>>>> >>> >
>>>> >>> > Currently the code expects to find
>>>> ‘org.bouncycastle.openssl.PEMReader’
>>>> >>> but this has been removed and as far as I can tell
>>>> >>> ‘org.bouncycastle.openssl.PEMParser’ should be used in its place - as a
>>>> >>> side note it looks like this functionality has been around for quite a
>>>> >>> while and the PEMReader had been flagged for deprecation.
>>>> >>> >
>>>> >>> > I’m happy to provide a patch but wanted to ensure nobody else was was
>>>> >>> addressing this.
>>>> >>> >
>>>> >>> > —
>>>> >>> > Jem
>>>> >>> > Principal Architect, Core Payments
>>>> >>> >
>>>> >>> >
>>>> >>> >
>>>> >>>
>>>> >>> Nobody is working on that (or at least hasn't said so if they are),
>>>> feel
>>>> >>> free.
>>>> >>>
>>>> >>> Robbie
>>>> >>>
>>>> >>> ---------------------------------------------------------------------
>>>> >>> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>>>> >>> For additional commands, e-mail: users-help@qpid.apache.org
>>>> >>>
>>>> >>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>>>> For additional commands, e-mail: users-help@qpid.apache.org
>>>>
>>>>
>>
>>---------------------------------------------------------------------
>>To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>>For additional commands, e-mail: users-help@qpid.apache.org
>>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>For additional commands, e-mail: users-help@qpid.apache.org
>
Re: ProtonJ - Bouncy Castle Dependency
Posted by "Day, Jem" <jd...@paypal.com.INVALID>.
I’ve re-worked the change as suggested.
- Went back to a reflection based approach
- Added additional UnitTests
- Added test certificate/key resources
Jem..
On 6/6/16, 8:59 AM, "Day, Jem" <jd...@paypal.com.INVALID> wrote:
>Also, opened a JIRA for this:
> https://issues.apache.org/jira/browse/PROTON-1224
>
>It would be great if this could be merged/incorporated in 0.13 if possible.
>
>Thanks
>
>On 5/31/16, 10:15 AM, "Day, Jem" <jd...@paypal.com.INVALID> wrote:
>
>>
>>I’ve opened a PR for this change: https://github.com/apache/qpid-proton/pull/74
>>
>>Verified test-suite using BC version 1.48 (where the deprecated class was removed) and also version 1.54.
>>
>>@Rob – This is probably not the most ideal change based on your comments, I decided to concentrate on the BC version update rather than completely re-working it.
>>
>>Jem..
>>
>>On 5/5/16, 7:02 AM, "Robbie Gemmell" <ro...@gmail.com> wrote:
>>
>>>On 5 May 2016 at 12:37, Rob Godfrey <ro...@gmail.com> wrote:
>>>> I think the idea was that Proton-J should support SSL/TLS even without
>>>> bouncy castle, but it would only be able to use the JVM default
>>>> keystore/truststore (as set by system properties)...
>>>
>>>Ah, well if that currently works, that could be reason to keep the
>>>reflection then if keeping bouncycastle, yes.
>>>
>>>> Having to use PEM
>>>> files is not a particularly java idiomatic way of configuring SSL/TLS, but
>>>> was required in order to preserve API equivalence with proton-c.
>>>> Personally I'd think we'd want to avoid a hard dependency on BouncyCastle
>>>> to simply support SSL/TLS, and if we want to parse PEM files then just do
>>>> the necessary decoding ourselves.
>>>>
>>>> -- Rob
>>>>
>>>> On 5 May 2016 at 10:19, Robbie Gemmell <ro...@gmail.com> wrote:
>>>>
>>>>> I can't speak to why BC is used, but if it is then using provided
>>>>> scope instead of reflection seems reasonable. The file history shows
>>>>> it used to be a hard dependency and was later changed to use
>>>>> reflection to make it a runtime dependency. Its not clear if folks
>>>>> were even aware of provided/optional dependencies at the time.
>>>>>
>>>>> Robbie
>>>>>
>>>>> On 4 May 2016 at 23:04, Day, Jem <jd...@paypal.com.invalid> wrote:
>>>>> >
>>>>> > Rob,
>>>>> >
>>>>> > I was also wondering that; my assumption was that BC was being leveraged
>>>>> to reduce configuration concerns with JCE providers etc and to keep
>>>>> everything “in house”.
>>>>> >
>>>>> > I do have a question on style/approach : The class in question jumps
>>>>> through reflection hoops to, I assume, remove the need for a hard
>>>>> dependency on the BC libraries. In reality though the code DOES have an
>>>>> absolute dependency on those classes to function correctly so one might
>>>>> argue it would be better to declare a maven <scope>provided</scope>
>>>>> dependency on BC. This would simplify the code greatly without changing the
>>>>> usage model for existing users of the proton-j library (it won’t result in
>>>>> everybody downloading BC, and it won’t result in version conflicts) - you
>>>>> should get the best of both worlds so long as the code is constructed in
>>>>> manner where it fails gracefully if the desired dependencies aren’t present.
>>>>> >
>>>>> > I do, of course, have a subtle hidden agenda here as the changes
>>>>> required to use the more recent versions of BC seems to require leveraging
>>>>> more of their capabilities, and doing that by via reflection just seems
>>>>> wrong ...
>>>>> >
>>>>> > Opinions/Comments ???
>>>>> >
>>>>> >
>>>>> >
>>>>> > On 5/4/16, 7:19 AM, "Rob Godfrey" <ro...@gmail.com> wrote:
>>>>> >
>>>>> >>As an aside, I think it should be possible to eliminate the bouncy castle
>>>>> >>dependency entirely, it's pretty easy to convert PEM files into the
>>>>> binary
>>>>> >>encoding which can then be used directly by Java (and in the Certificates
>>>>> >>case, the Java X509 CertificateFactory will work directly off PEM files).
>>>>> >>
>>>>> >>-- Rob
>>>>> >>
>>>>> >>On 4 May 2016 at 15:01, Robbie Gemmell <ro...@gmail.com> wrote:
>>>>> >>
>>>>> >>> On 3 May 2016 at 22:38, Day, Jem <jd...@paypal.com.invalid> wrote:
>>>>> >>> > Hi Guys,
>>>>> >>> >
>>>>> >>> > ‘SslEngineFacadeFactory’ uses reflection on some bouncy-castle
>>>>> classes
>>>>> >>> to load certificates and keys from the configured PEM files.
>>>>> >>> >
>>>>> >>> > Currently the code expects to find
>>>>> ‘org.bouncycastle.openssl.PEMReader’
>>>>> >>> but this has been removed and as far as I can tell
>>>>> >>> ‘org.bouncycastle.openssl.PEMParser’ should be used in its place - as a
>>>>> >>> side note it looks like this functionality has been around for quite a
>>>>> >>> while and the PEMReader had been flagged for deprecation.
>>>>> >>> >
>>>>> >>> > I’m happy to provide a patch but wanted to ensure nobody else was was
>>>>> >>> addressing this.
>>>>> >>> >
>>>>> >>> > —
>>>>> >>> > Jem
>>>>> >>> > Principal Architect, Core Payments
>>>>> >>> >
>>>>> >>> >
>>>>> >>> >
>>>>> >>>
>>>>> >>> Nobody is working on that (or at least hasn't said so if they are),
>>>>> feel
>>>>> >>> free.
>>>>> >>>
>>>>> >>> Robbie
>>>>> >>>
>>>>> >>> ---------------------------------------------------------------------
>>>>> >>> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>>>>> >>> For additional commands, e-mail: users-help@qpid.apache.org
>>>>> >>>
>>>>> >>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>>>>> For additional commands, e-mail: users-help@qpid.apache.org
>>>>>
>>>>>
>>>
>>>---------------------------------------------------------------------
>>>To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>>>For additional commands, e-mail: users-help@qpid.apache.org
>>>
>>
>>
>>---------------------------------------------------------------------
>>To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>>For additional commands, e-mail: users-help@qpid.apache.org
>>
>
>?B�KKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKCB�?�?[��X��ܚX�K??K[XZ[?�?\�\��][��X��ܚX�P?\?Y?�\?X�?K�ܙ�B��܈?Y??]?[ۘ[??��[X[�?�??K[XZ[?�?\�\��Z?[???\?Y?�\?X�?K�ܙ�B�B