RFR 7191662: JCE providers should be located via ServiceLoader

Tue Jun 16 20:34:17 UTC 2015

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)