Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jetty-dev] Comments on new SslConnector

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


Back to the top