Community
Participate
Working Groups
Now, there are various keystore types introduced, e.g. JKS (a default), JCEKS, PKCS12,JCERACFKS, . . . So, there is a requirement to use the product's own keystore. The attached patch is one of the proposals to enable a product to use its own keystore.
Created attachment 121480 [details] To provide a way to use its own keystore
I think this patch should be okay since the modified class is internal.
I've committed the patch to cvs.
Bug 260256 is opened for the backport to 3.0.3.
Legal Message: I, {Noriaki Takatsu}, declare that I developed attached code from scratch, without referencing any 3rd party materials except material licensed under the EPL. I am authorized by my employer to make this contribution under the EPL.
There is a new method public static void DStoreSSLContext#setKeyManager(KeyManager[]) Some questions: 1.) Shouldn't the method be named setKeyManagers(KeyManager[]) 2.) If it's intended that this method be used by products, shouldn't it be exposed as API We're currently having a discussion at the Architecture Council on how to make it easier for products to pick up newer versions of Eclipse products. Being API clean plays a central role in this discussion. I understand that you cannot add API for the 3.0.3 TM service release. But for the 3.1 release, we should be adding API or we'll get into maintenance hell... who could ever understand what "internal" stuff can be changed and what not? Reopening to clarify.
(In reply to comment #6) > There is a new method > public static void DStoreSSLContext#setKeyManager(KeyManager[]) > > Some questions: > 1.) Shouldn't the method be named setKeyManagers(KeyManager[]) Yeah, it probably should be setKeyManagers(KeyManager[]). > 2.) If it's intended that this method be used by products, shouldn't it be > exposed as API > The class wasn't intended to be exposed as API but apparently this team requires the ability to set the key managers. Would there be any problems moving this class into org.eclipse.dstore.core.util.ssl in 3.1? >
setKeyManagers() is static, so instead of moving the entire class I'd rather create some new API class that just exposes what's necessary and delegates to the existing class.
Created attachment 123080 [details] patch providing public api abstract class for keymanagers Martin, is this patch the sort of thing you had in mind?
Well... kind of. 1. Why "AbstractSSLContext". There is nothing abstract in there. 2. Why does DStoreSSLContext need to extend the abstract one. I had initially thought that util.ssl.SSLContext#set...() would relay into DStoreSSLContext#set...(). Another option is that DStoreSSLContext just performs util.ssl.SSLContext#getKeyManagers() when it needs to access the list of key managers.
(In reply to comment #10) > Well... kind of. > > 1. Why "AbstractSSLContext". There is nothing abstract in there. > 2. Why does DStoreSSLContext need to extend the abstract one. > > I had initially thought that util.ssl.SSLContext#set...() would relay into > DStoreSSLContext#set...(). > > Another option is that DStoreSSLContext just performs > util.ssl.SSLContext#getKeyManagers() > when it needs to access the list of key managers. > I guess "Abstract" is not an appropriate name here. I didn't want to use util.ssl.SSLContext because it could be confused with javax.net.ssl.SSLContext which is the thing that gets returned by DStoreSSLContext. Perhaps "BaseSSLContext" or, if we relay into DStoreSSLContext.setKeyManagers(), maybe something like SSLContextUtil?
Both names seem OK. In terms of API, I'm more in favor of BaseSSLContext actually store the context that a user sets, and have DStoreSSLContext "pull" the information from there. This seems more logical than having an util push stuff into something that's internal. Be sure to mark the BaseSSLContext as @noextend or final. The little drawback of this approach is that you cannot make it backward compatible with the old "internal" access methods in 3.0.3 -- not sure if that's an issue for your product teams.
Created attachment 123292 [details] patch for internal class to pick up keymanagers from public class I think this patch addresses suggestions mentioned in the last comment.
I've committed the changes to cvs.
FYI, I had to rev up the version number of the org.eclipse.dstore.doc.isv plugin to v3.1 in order to account for the added API.
