Bug 345651

Comment 1 Michael Gorovoy 2011-05-12 17:15:13 EDT

Chad,

These were (protected) helper methods that were used while creating an SSL Context. As the SSL Context creation was moved to SslContextFactory, these methods had to be removed.

We would appreciate if you could describe your use case so that we can better understand it and hopefully come up with a way to resolve this issue.

Also, we had a report of the same issue on the developer mailing list from Gunnar Wagenknecht (see http://goo.gl/3UEx8) who stated that he was able to override the following method that enabled him to achieve desired functionality.

protected KeyStore getKeyStore(InputStream storeStream, String storePath, String storeType, String storeProvider, String storePassword);

Cheers,
Michael

Comment 2 Chad La Joie 2011-05-12 17:35:23 EDT

Yes, those methods were protected, which means they are part of the accessible API for the connector.  And while I personally favor the use of the SslContextFactory (with the noted items I raised in other tickets), the change was still made in a patch release and resulted in code that was not backwards compatible.  As I mentioned in my email, I don't know if Jetty has an API versioning policy, so I'm not saying this change is wrong in any strict sense.  But, in the future, I personally think that making API changes with patch releases is best avoided.

Here's the initial ticket that resulted in those methods being add:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=283375

And the mailing list discussion that preceded the ticket:
http://dev.eclipse.org/mhonarc/lists/jetty-dev/msg00163.html

Comment 3 Michael Gorovoy 2011-05-12 19:48:24 EDT

Chad,

Thank you very much for providing context for the issue. You are absolutely right that these methods were part of the API that subclasses were able to access. In hindsight we should have probably saved this change for the 7.4.0 release. 

The issue that you brought up a while ago has a valid point. We did not account for the possibility that there need to be a way to supply a keystore and/or truststore that would not be file or stream based. And the way it was fixed earlier did not eliminate the problem completely either. And then here I come and break everything. ;)

I'm going to actually provide a way to either set an initialized keystore/truststore, or to override a method to do the same.

Cheers,
Michael

Comment 4 Chad La Joie 2011-05-12 19:56:12 EDT

The missing methods and the ability to read in data from different types of key information stores are actually different (though related in some ways).  The Java crypto APIs tend to conflate them.

This issue really related to the mechanism used to inject different trust evaluation algorithms.  In our use case, our application does the evaluation and therefore we need the certificates to simply pass-through the container.  To do that requires the ability to set a trust manager that is, essentially, a no-op.

But yes, the APIs as they were refactored in the patch I submitted do not address the reading of keying information from different sources.  Hence the reason for ticket #345658  :)

Comment 5 Michael Gorovoy 2011-05-12 20:09:33 EDT

Chad,

In order to replace the default trust manager, you should be able to override the following method in a subclass of SslContextFactory and when provide an instance of the subclass to the connector's constructor in to achieve the same effect as before.

protected TrustManager[] getTrustManagers(KeyStore trustStore, Collection<? extends CRL> crls);

It is not going to be possible to restore the methods of SslSelectChannelConnector in question because the code that creates SSL Context has been moved to SslContextFactory and the latter cannot access the methods of the former. Once again, we apologize for introducing an API incompatibility in a point release.

Cheers,
Michael

Comment 6 Chad La Joie 2011-05-12 20:12:38 EDT

Yeah, we created, and are currently testing, an extended SslContextFactory that we're pretty sure does what we need.

Comment 7 Michael Gorovoy 2011-05-12 20:13:58 EDT

Perfect! Thank you very much for bringing up this issue though.

-Michael

Comment 8 Michael Gorovoy 2011-05-13 00:01:26 EDT

See discussion above.