Re: [jetty-dev] Comments on new SslConnector

[Chad La Joie <lajoie@xxxxxxxxx>]

Thanks, done.

Bugs: 345651, 345654, 345655, 345656, 345658


On 5/12/11 3:48 PM, Michael Gorovoy wrote:
> Chad,
> 
> Thank you very much for your comments. We would appreciate if you could open
> tickets in Bugzilla for each of the issues so that they can be investigated.
> The last issue about the missing method should be opened first. We are
> currently working on the 7.4.1 release and if there was an inadvertent API
> change this is the perfect time to correct that.
> 
> Thanks,
> Michael
> 
> 
> On Thu, May 12, 2011 at 3:05 PM, Chad La Joie <lajoie@xxxxxxxxx> wrote:
> 
>> Given that this has come up in our project as well as other people's,
>> judging from the user mailing list, I wanted to offer a few comments on
>> the changes made to the SslConnector implementations.
>>
>> First, I think splitting out the SslContext generation stuff in to the
>> SslContextFactory was a good idea.  However, there are a few things that
>> I think are a bit off in the current implementation.
>>  - Renegotiation support is on by default.  Is there code elsewhere in
>> the request processing that checks whether the user-agent supports the
>> features necessary to make this operation safe?  I didn't see any such
>> checks but I also don't know that I was looking in the right places.
>>  - In the createSSLContext method, the PKIX validation of the end-entity
>> certificate, prior to usage, is not something you'd usually do because
>> the answer is meaningless.  It's up to the peer to determine if your
>> certificate is valid for its purposes.  I recommend getting rid of this.
>>  - The "validateCerts" method controls the aforementioned validation of
>> the end-entity certificate by the user of that certificate and the
>> validation of received certificates.  If the end-entity certificate
>> validation is not done away with, this property should be split in to
>> two different ones.  As it stands right now, as best as I can tell, it
>> would be impossible to have a self-signed end-entity certificate and to
>> use client-cert auth.
>>  - The getKeyStore method assumes that the keystore data is going to be
>> coming from a file.  In most cases this is true, however if you are
>> using a PKCS11 keystore or wish to construct a keystore from plain old
>> PEM encoded files this isn't going to work.  A better approach would be
>> to either have getter/setter methods for the keystore and truststore
>> (and provide a few implementations that support different types of
>> sources) or getKeystore and getTruststore methods that could be
>> overridden within a subclass to do the appropriate thing.  I recommend
>> the former.
>>
>> Second, I think it's unfortunate that breaking API changes were made, in
>> a patch-level release, to the SslSelectChannelConnector by removing the
>> getTrustManagers method.  I don't recall seeing any versioning policy
>> for Jetty but such changes in a patch level can be problematic.  It has
>> prevented us from being able to upgrade to 7.3.1 and above.
>>
>> --
>> Chad La Joie
>> http://itumi.biz
>> trusted identities, delivered
>> _______________________________________________
>> jetty-dev mailing list
>> jetty-dev@xxxxxxxxxxx
>> https://dev.eclipse.org/mailman/listinfo/jetty-dev
>>
> 
> 
> 
> _______________________________________________
> jetty-dev mailing list
> jetty-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/jetty-dev

-- 
Chad La Joie
http://itumi.biz
trusted identities, delivered