RFR 7191662: JCE providers should be located via ServiceLoader

[Valerie Peng]

It's to make the runtime configuration of 3rd party PKCS11 provider 
easier on non-Solaris systems.

With this entry, callers can simply do Security.getProvider("SunPKCS11") 
and then call Provider.configure with their own configuration file.
Otherwise, they will have to use reflection, e.g. 
Class.forName("sun.security.pkcs11.SunPKCS11") and then call its 
newInstance() method and then call Provider.configure method. Or, 
iteration through the ServiceLoader until the SunPKCS11 provider is 
found. Both doesn't seem very user-friendly in my view.
Valerie

On 6/16/2015 1:34 PM, Sean Mullan wrote:
> On 06/15/2015 07:58 PM, Valerie Peng wrote:
>>
>> It seems that the jimage refresh change may still take some time, so we
>> will end up removing the makefile related changes and then deferring the
>> ServiceLoader related changes to Jake workspace.
>>
>> Here is the latest webrev (Security source/test changes only, no more
>> makefile changes)
>> http://cr.openjdk.java.net/~valeriep/7191662/webrev.04/
>>
>> Summary of changes from webrev.03:
>> 1) switch back to provider class names for java.security file
>
> Why is there an entry for 
> security.provider.tbd=sun.security.pkcs11.SunPKCS11 on line 90?
>
>> 2) separated out the java.policy change into its own as SQE has filed a
>> bug for it
>> 3) updated sun.security.jca.Providers class due to 1)
>> 4) fixed sun.security.tools.jarsigner.Main to use the new
>> Provider.configure() api
>> 5) fixed a bug in sun.security.pkcs11.PKCS11Test regarding searching and
>> configuring PKCS11 provider
>
> Looks fine otherwise.
>
> --Sean
>
>>
>> Thanks,
>> Valerie
>>
>> On 6/8/2015 4:44 PM, Valerie Peng wrote:
>>>
>>> What is the bug id for the image refresh change? Just so I can keep a
>>> watch and hold my changes for the time being.
>>>
>>> My webrev has a new regression test which compares the list of
>>> providers found by ServiceLoader and Security.getProviders() call. So,
>>> if the META-INF/services/java.security.Provider file content is not
>>> correct, the new test would fail and it's a clear indication that
>>> ServiceLoader can't find one or more of the providers.
>>>
>>> Thanks,
>>> Valerie
>>>
>>> On 6/5/2015 10:43 PM, Mandy Chung wrote:
>>>>> On Jun 5, 2015, at 4:32 PM, Valerie Peng<valerie.peng at oracle.com>
>>>>> wrote:
>>>>>
>>>>>
>>>>> I don't know image builder well enough to answer your question.
>>>> The current implementation of the image builder sorts the modules by
>>>> their name that are mapped to the same class loader.  That explains
>>>> why java.naming is the first one containing
>>>> META-INF/services/java.security.Provider in your current list.
>>>>
>>>> There is no guarantee in what particular order a module is processed
>>>> first.   I don’t know if the jimage refresh work will change the
>>>> ordering either.  Since this is temporary, I’m okay if you want to
>>>> depend on the image build implementation as long as you have a test
>>>> to catch it and verify
>>>> java.naming/META-INF/services/java.security.Provider file content.
>>>> The existing security tests may also catch it but I think it’s not
>>>> obvious to indicate that the security tests fail because of the issue
>>>> of the merged service configuration file.
>>>>
>>>> Please also hold off until the image refresh change goes into
>>>> jdk9/dev so that you can verify if your build change still works.
>>>>
>>>> If you want to push earlier, another alternative we discussed earlier
>>>> is to separate the META-INF/services file and make change and
>>>> java.security to keep using classname.  That can be pushed that in a
>>>> second phase with a proper image builder support.
>>>>
>>>> Mandy
>>>>
>>>>> Based on my own experiment, it seems to pick up the one from
>>>>> java.naming when duplication occurs, so that's why I saved the
>>>>> merged result to there and named the Gensrc makefile with
>>>>> java.naming. The result build work fine.
>>>>>
>>>>> Does this explain what I am trying to do here? If you have better
>>>>> ways to get this done, I am certainly open to that idea.
>>>>> Thanks,
>>>>> Valerie
>>>>>
>>>>> On 6/5/2015 12:21 AM, Erik Joelsson wrote:
>>>>>> Hello Valerie,
>>>>>>
>>>>>> The merging seems ok, but I thought there was non determinism in
>>>>>> the image builder regarding which provider would get picked up. Is
>>>>>> that resolved or do you really need to override all of those
>>>>>> providers with your generated file in gensrc? I can assist in
>>>>>> writing that makefile logic if needed.
>>>>>>
>>>>>> /Erik
>>>>>>
>>>>>> On 2015-06-04 23:58, Valerie Peng wrote:
>>>>>>> Build experts,
>>>>>>>
>>>>>>> Can you please review the make file related change, i.e. the new
>>>>>>> file make/gensrc/Gensrc-java.naming.gmk, in the following webrev:
>>>>>>> http://cr.openjdk.java.net/~valeriep/7191662/webrev.03
>>>>>>>
>>>>>>> This is for merging the java.security.Provider file from various
>>>>>>> providers and use the (merged) result for the final image build.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Valerie
>>>>>>>
>>>>>>> On 6/3/2015 10:27 AM, Valerie (Yu-Ching) Peng wrote:
>>>>>>>> Correct, if the makefile related changes are removed then no need
>>>>>>>> for build team to review 7191662 webrev anymore.
>>>>>>>> There are other discussions ongoing and we should be able to
>>>>>>>> reach a decision in a day or two.
>>>>>>>> Will update the list again.
>>>>>>>> Thanks,
>>>>>>>> Valerie
>>>>>>>>
>>>>>>>> On 06/01/15 16:39, Magnus Ihse Bursie wrote:
>>>>>>>>> On 2015-05-29 00:10, Valerie Peng wrote:
>>>>>>>>>> Please find comments in line...
>>>>>>>>>>
>>>>>>>>>> On 5/27/2015 3:42 PM, Mandy Chung wrote:
>>>>>>>>>>> Valerie,
>>>>>>>>>>>
>>>>>>>>>>> Did you see my comment yesterday?
>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/security-dev/2015-May/012254.html 
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> Yes, we exchanged emails after this above one. I will follow up
>>>>>>>>>> your latest one later today.
>>>>>>>>>>
>>>>>>>>>>> Since you have reverted the java.security to keep the
>>>>>>>>>>> classname, to avoid causing merge conflict to jimage refresh,
>>>>>>>>>>> let’s remove the META-INF files in the first push and the
>>>>>>>>>>> build change.
>>>>>>>>>>>
>>>>>>>>>>> The security providers will be loaded via the fallback
>>>>>>>>>>> mechanism (i.e. ProviderLoader.legacyLoad method).  You should
>>>>>>>>>>> keep the ProviderLoader.load method to take the provider name
>>>>>>>>>>> instead of classname.
>>>>>>>>>> Sure, I can remove the META-INF files so the providers are
>>>>>>>>>> loaded through the legacyLoad().
>>>>>>>>>> Hmm, the ProviderLoader.load() method is used by java.security
>>>>>>>>>> file provider loading. Since the current list still uses class
>>>>>>>>>> name, it should take class name when checking for matches while
>>>>>>>>>> iterating through the list returned by ServiceLoader.
>>>>>>>>>> This way, when changes are sync'ed into Jake, no extra change
>>>>>>>>>> required and the providers will be loaded through
>>>>>>>>>> ProviderLoader.load() automatically with the current list.
>>>>>>>>>>
>>>>>>>>>>> I’ll file a bug to follow up to change java.security to list
>>>>>>>>>>> the provider name.  This will wait after the jimage refresh
>>>>>>>>>>> goes into jdk9/dev
>>>>>>>>>> Ok.
>>>>>>>>>> Thanks,
>>>>>>>>>> Valerie
>>>>>>>>> I'm not sure I followed completely here were this landed. Does
>>>>>>>>> this mean that there's currently no need for a build system code
>>>>>>>>> review on
>>>>>>>>> http://cr.openjdk.java.net/~valeriep/7191662/webrev.01/, and
>>>>>>>>> that a new webrev will be posted instead?
>>>>>>>>>
>>>>>>>>> /Magnus
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>> .
>>>>>>>>>>>
>>>>>>>>>>> Mandy
>>>>>>>>>>>
>>>>>>>>>>>> On May 27, 2015, at 3:29 PM, Valerie
>>>>>>>>>>>> Peng<valerie.peng at oracle.com>   wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi, build experts,
>>>>>>>>>>>>
>>>>>>>>>>>> Can you please review the make file related change, i.e. the
>>>>>>>>>>>> new file make/gensrc/Gensrc-java.naming.gmk, in the following
>>>>>>>>>>>> webrev:
>>>>>>>>>>>> http://cr.openjdk.java.net/~valeriep/7191662/webrev.01/
>>>>>>>>>>>>
>>>>>>>>>>>> This is for merging the java.security.Provider file from
>>>>>>>>>>>> various providers and use the (merged) result for the final
>>>>>>>>>>>> image build.
>>>>>>>>>>>>
>>>>>>>>>>>> The rest of source code changes are reviewed by my team 
>>>>>>>>>>>> already.
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Valerie
>>>>>>>>>>>> (Java Security Team)