Review Request: loading JCE providers using ServiceLoader

[Sean Mullan]

On 7/22/12 10:09 AM, Alan Bateman wrote:
> On 20/07/2012 17:58, Sean Mullan wrote:
>> Please review my code changes to enable JCE providers to be registered and
>> loaded as services using the ServiceLoader API:
>>
>> http://cr.openjdk.java.net/~mullan/jigsaw/webrevs/jce-service/webrev.00/
>>
>> There is one outstanding issue that needs to be resolved, but I think we can try
>> to resolve this later:
>>
>> You cannot load more than one PKCS11 provider (ex: with different
>> configurations), as currently supported and documented in the PKCS11 guide [1].
>>
>> Thanks,
>> Sean
>>
>> [1] http://docs.oracle.com/javase/7/docs/technotes/guides/security/p11guide.html
> It's good to see this.
> 
> One thing that I can't quite grok is the how this fits with the list of 
> providers in the java.security file. Would I be correct to say that if 
> you have a new provider then you install the provider module and in 
> addition, and add to the java.security file as above? (the reason being 
> to give it preference order). 

Yes that's the most common case. You can, however, instantiate the provider
directly which doesn't require any configuration to the java.security file.

> If so then it's a minor that we need to 
> re-visit the issue of configured preferred order. If we did that then 
> maybe it could be used in class path mode, assuming we could work out a 
> migration from the existing java.security list.

Actually, the preference order in the java.security file is preserved in either
classpath or module mode, since this order is enforced in the provider code
(sun.security.jca.ProviderList). It uses the ServiceLoader to find the Provider,
but ignores the order.

> Is there is another way to configure provider arguments?

Not that I know of.

> Also do you 
> know if is common to configure multiple PKCS11 providers? 

I don't think so, but that's mostly a guess.

> Just looking
> at the PKCS11 guide then it includes an example that does this:
> 
> Provider p = new sun.security.pkcs11.SunPKCS11(configName);
> 
> I assume we don't want anyone becomes dependent on this class name as it 
> will be not be exported when running with modules.

Good catch.
> 
> Minor comment in passing but you might have a merge error in 
> cli/Packager.java or maybe you just need the stack trace when there is a 
> CommandException? 

Not a merge error. cli/Librarian.java already has this so my change here is just
consistent, and it helps when debugging.

> Also I think the "tries" field in ProviderConfig needs 
> to be checked as it looks to have synchronization issues. I'll try to do 
> a more detailed few in the next few days.

I'll take a closer look at that, though that's existing code, I did not do
anything to change that.

--Sean