2nd round code review request: 7055363: jdk_security3 cleanup

Weijun Wang weijun.wang at oracle.com
Fri Aug 5 10:14:34 UTC 2011 


On 08/05/2011 06:09 PM, Xuelei Fan wrote:
> Hi Max,
>
> I finished the update. It looks good in my testing. We may be able to
> join the effort in one CR. What do you think?

It's up to you. If you want to make it a single changeset, just send the 
patch to me.

Thanks
Max

>
> I addressed the following issues in the update:
>
> 1. reserve and restore security properties, java.security.Security
>     Security.setProperty
>     Security.getProperty
>
> 2. reserve and restore default HostnameVerifier and SSLSocketFactory of
> javax.net.ssl.HttpsURLConnection
>     HttpsURLConnection.setDefaultHostnameVerifier(HostnameVerifier v)
>     HttpsURLConnection.getDefaultHostnameVerifier()
>     HttpsURLConnection.setDefaultSSLSocketFactory(SSLSocketFactory sf)
>     HttpsURLConnection.getDefaultSSLSocketFactory()
>
> 3. reserve and restore default java.net.CookieHandler and
> java.net.ResponseCache
>     CookieHandler.setDefault
>     CookieHandler.getDefault
>     ResponseCache.setDefault
>     ResponseCache.getDefault
>
> 4. reserve and restore system scope of java.security.IdentityScope
>     IdentityScope.setSystemScope
>     IdentityScope.getSystemScope
>
> 5. reserve and restore identity policy of java.security.Policy
>     Policy.getPolicy
>     Policy.setPolicy
>
> 6. security provider manage, java.security.Security
>     Security.insertProviderAt
>     Security.addProvider
>     Security.removeProvider
>     Security.getProviders
>
> 7. handle special system properties,
>       "java.protocol.handler.pkgs"
>       "javax.net.ssl.keyStore"
>       "javax.net.ssl.keyStorePassword"
>       "javax.net.ssl.trustStore"
>       "javax.net.ssl.trustStorePassword"
>
>     JSSE does not support above dynamic system properties. For every test
> that needs to set any property of above, need to run in othervm mode.
>
> 8. reserve and restore default java.net.ssl.SSLContext
>     SSLContext.setDefault(SSLContext context)
>     SSLContext.getDefault()
>
>
> Thanks,
> Xuelei
>
> On 8/4/2011 6:03 PM, Weijun Wang wrote:
>> I'll wait then. Plus I can try running the final result.
>>
>> -Max
>>
>> On 08/04/2011 06:01 PM, Xuelei Fan wrote:
>>> On 8/4/2011 5:58 PM, Weijun Wang wrote:
>>>>
>>>>
>>>> On 08/04/2011 05:50 PM, Xuelei Fan wrote:
>>>>> Good point. I will update the test files. Even we have to make
>>>>> update to
>>>>> JSSE tests, I would evaluate every tests, and won't add "othervm"
>>>>> tag to
>>>>> those samevm safe tests. 170+ files, need some more time.
>>>>
>>>> So you will add the tag? How long will you use?
>>> Should be able to finish the update in the end of next week. I have
>>> updated 20+ files.
>>>
>>> Xuelei
>>>
>>>> If less than a week, I
>>>> can wait after you're finished and update the Makefile to call
>>>> RunasSamevmBatch. Otherwise, I'll push my changeset first and keep
>>>> RunasOthervmBatch, and in your changeset you change it to
>>>> RunasSamevmBatch.
>>>>
>>>> Thanks
>>>> Max
>>>>
>>>>>
>>>>>>> ... at least I know tests inside test/closed/
>>>>>>> sun/security/ssl/java/security/KeyStore can run
>>>>>>> in samevm. Can you give a whitelist?
>>>>>>>
>>>>> The tests won't try to establish a TLS/SSL connection, so they are
>>>>> safe.
>>>>>
>>>>> Thanks,
>>>>> Xuelei
>>>>>
>>>>> On 8/4/2011 5:30 PM, Kelly O'Hair wrote:
>>>>>>
>>>>>> On Aug 4, 2011, at 11:06 AM, Weijun Wang wrote:
>>>>>>
>>>>>>>> Max, if we can get an agree, would you please update the Makefile to
>>>>>>>> run JSSE in othervm mode  in your fix? So that we don't have to fill
>>>>>>>> a new CR.
>>>>>>>
>>>>>>> There are two ways to do this:
>>>>>>>
>>>>>>> 1. Add "@run main/othervm" to all JSSE tests. This means a lot of
>>>>>>> code changes, but I can automate it.
>>>>>>>
>>>>>>> 2. Create new test set for JSSE. This is what I had done before in
>>>>>>>
>>>>>>>      http://cr.openjdk.java.net/~weijun/7055363/webrev.00/
>>>>>>>
>>>>>>> many changes to Makefile, jprt.properties and jprt.properties in the
>>>>>>> parent repo.
>>>>>>>
>>>>>>> I'd choose #1. What's your opinion?
>>>>>>
>>>>>> Just a comment from the peanut gallery... ;^)
>>>>>>
>>>>>> I think #1 is the way to go.
>>>>>>
>>>>>> One thing we wanted people to be able to do is just cd to the test
>>>>>> directory and run 'jtreg -samevm' or 'jtreg -agentvm'
>>>>>> and by changing the test itself, this can work. If at some point
>>>>>> later someone fixes the test or is able to
>>>>>> make it work with samevm, they can remove the main/othervm.
>>>>>> If you do it with the Makefile, you only fixed it for when people use
>>>>>> the Makefile or with JPRT.
>>>>>>
>>>>>> -kto
>>>>>>
>>>>>>>
>>>>>>> BTW, you mentioned HTTPS and JNDI codes. If they are not inside
>>>>>>> sun/security, I won't touch them this time. Also, at least I know
>>>>>>> tests inside test/closed/sun/security/ssl/java/security/KeyStore can
>>>>>>> run in samevm. Can you give a whitelist?
>>>>>>>
>>>>>>> Thanks
>>>>>>> Max
>>>>>>>
>>>>>>>
>>>>>>> On 08/04/2011 12:05 PM, Xuelei Fan wrote:
>>>>>>>>> - JSSE tests, Xuelei is working on them
>>>>>>>> It may be a bad news for the purpose to quicken the testing run
>>>>>>>> time. Most of the JSSE test may not be able to under samevm/agentvm
>>>>>>>> mode.
>>>>>>>>
>>>>>>>> 1. JSSE does not support dynamic system properties For performance
>>>>>>>> tuning and synchronization, JSSE does not support dynamic system
>>>>>>>> properties of: "java.protocol.handler.pkgs" "javax.net.ssl.keyStore"
>>>>>>>> "javax.net.ssl.keyStorePassword" "javax.net.ssl.trustStore"
>>>>>>>> "javax.net.ssl.trustStorePassword"
>>>>>>>>
>>>>>>>> The JSSE default implementation will use these properties when
>>>>>>>> initializing the static objects, and then any changes to these
>>>>>>>> properties will be ignored in the same VM.
>>>>>>>>
>>>>>>>> There are around 90 of all 170+ JSSE regression tests depends on
>>>>>>>> system properties.
>>>>>>>>
>>>>>>>> 2. JSSE caches TLS sessions, as will result in unexpected reuse of
>>>>>>>> TLS sessions in samevm/agentvm mode. There are a lot of tests using
>>>>>>>> the default SSL/TLS context, which will the same TLS session cache.
>>>>>>>>
>>>>>>>> There are only a very very few tests could be updated to run in
>>>>>>>> samevm mode. Because samevm-safe in JSSE tests will result in
>>>>>>>> unnecessary complex, and the performance improvement is very little,
>>>>>>>> I would suggest tun all JSSE tests at othervm mode.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>> Max, if we can get an agree, would you please update the Makefile to
>>>>>>>> run JSSE in othervm mode  in your fix? So that we don't have to fill
>>>>>>>> a new CR.
>>>>>>>>
>>>>>>>> Thanks, Xuelei
>>>>>>>>
>>>>>>>> On 8/2/2011 3:02 PM, Weijun Wang wrote:
>>>>>>>>> Hi All
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~weijun/7055363/webrev.01/
>>>>>>>>>
>>>>>>>>> *Attention*: please note that the jtreg mode for jdk_security3 is
>>>>>>>>> still RunOthervmBatch. Xuelei is now working on the JSSE tests.
>>>>>>>>> Once he is ready, he will change the mode to RunSamevmBatch.
>>>>>>>>>
>>>>>>>>> The code changes:
>>>>>>>>>
>>>>>>>>> * Still included in test/ProblemList.txt
>>>>>>>>>
>>>>>>>>> - JSSE tests, Xuelei is working on them - ec tests on solaris-i586,
>>>>>>>>> they fail even if run standalone
>>>>>>>>>
>>>>>>>>> * PKCS11/EC tests:
>>>>>>>>>
>>>>>>>>> - New Providers.setAt, to ensure provider is added to desired
>>>>>>>>> position: test/sun/security/pkcs11/ec/*
>>>>>>>>>
>>>>>>>>> - PKCS11Test.safeReload, to ensure shared library can be loaded
>>>>>>>>> again: test/sun/security/pkcs11/PKCS11Test.java
>>>>>>>>> test/sun/security/pkcs11/SecmodTest.java
>>>>>>>>>
>>>>>>>>> - Test using config files need to run in othervm:
>>>>>>>>> test/sun/security/pkcs11/Secmod/* test/sun/security/pkcs11/fips/*
>>>>>>>>>
>>>>>>>>> - Restoring provider lists: test/sun/security/ec/TestEC.java
>>>>>>>>> test/sun/security/pkcs11/PKCS11Test.java
>>>>>>>>>
>>>>>>>>> * Try-with-resources updates:
>>>>>>>>>
>>>>>>>>> - test/com/sun/security/auth/login/ConfigFile/IllegalURL.java -
>>>>>>>>> test/sun/security/pkcs12/PKCS12SameKeyId.java -
>>>>>>>>> test/sun/security/tools/keytool/StartDateTest.java
>>>>>>>>>
>>>>>>>>> * Other tests still need to be in -othervm:
>>>>>>>>>
>>>>>>>>> Using special policy file:
>>>>>>>>> test/sun/security/provider/PolicyFile/Comparator.java AlgorithmId
>>>>>>>>> static field oidTable initialization:
>>>>>>>>> test/sun/security/x509/AlgorithmId/ExtensibleAlgorithmId.java
>>>>>>>>>
>>>>>>>>> * JAAS configuration restore:
>>>>>>>>>
>>>>>>>>> -
>>>>>>>>> test/javax/security/auth/login/LoginContext/ResetConfigModule.java
>>>>>>>>>
>>>>>>>>> * Use "localhost" as hostname, to make name resolution fast
>>>>>>>>>
>>>>>>>>> - test/sun/security/jgss/spnego/NoSpnegoAsDefMech.java
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> After these changes, JPRT test run (with RunSamevmBatch and no
>>>>>>>>> JSSE tests) shows most tests are OK. There are still intermittent
>>>>>>>>> failures which are PKCS11/EC provider related.
>>>>>>>>>
>>>>>>>>> Thanks Max
>>>>>>>>
>>>>>>
>>>>>
>>>
>

More information about the security-dev mailing list