RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

Mikael Vidstedt mikael.vidstedt at oracle.com
Thu May 7 21:51:42 UTC 2020


Will generate a new webrev shortly, meanwhile some comments inline..

> On May 7, 2020, at 1:02 AM, Weijun Wang <weijun.wang at oracle.com> wrote:
> 
> 
> 
>> On May 7, 2020, at 1:05 PM, Mikael Vidstedt <mikael.vidstedt at oracle.com> wrote:
>> 
>> 
>> New webrev here:
>> 
>> webrev: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/security/open/webrev/
>> incremental: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/security.incr/open/webrev/
>> 
>> Items yet to be resolved:
>> 
>> * File follow-up to drop $ISA support in src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Config.java
> 
> Maybe keep it now. I remember on Ubuntu the libraries are also in different directories for x64 and x86.

I’ll leave it the way it is in webrev.01. Just to make sure we’re on the same page: the only remaining effect it will have is that the "$ISA/“ substring will be removed if it’s found. A possible removal can perhaps be handled as a follow-up instead?

> 
>> 
>> * Get confirmation on the “solaris” property in test/jdk/sun/security/tools/keytool/KeyToolTest.java
> 
> 1. remove all lines except for the 1st in the test's @summary
> 2. remove the whole "if (System.getProperty("solaris") != null)" block in main()
> 3. remove the static final String fields SUN_P11_ARG and SUN_SRC_P11_ARG

Thanks for the off-thread education on why this code is indeed Solaris specific: all other platforms need the -addprovider option to enable PKCS11, Solaris comes with enabled by default.

> 
>> 
>> * Get confirmation if https://bugs.openjdk.java.net/browse/JDK-8039280 is enough to cover test/jdk/sun/security/provider/PolicyParser/PrincipalExpansionError.java
> 
> The test has been neglected for a long time, but I think in theory the SolarisPrincipal can be substituted into UnixPrincipal. Let's keep it.

I changed SolarisPrincipal to UnixPrincipal in the .java and .policy files, but did *not* verify that the test works as expected with that change.

> 
>> 
>> * Get confirmation on what to do about the "(secret == null)” block in test/jdk/sun/security/pkcs11/tls/TestPRF.java
> 
> I don't quite understand this test, but I think we can simply remove the block. The comment seems to suggest that this will not fail on Solaris. Let's see what would happen on other platforms.

Consider it gone!

Cheers,
Mikael

> 
>> 
>>> On May 3, 2020, at 10:12 PM, Mikael Vidstedt <mikael.vidstedt at oracle.com> wrote:
>>> 
>>> 
>>> Please review this change which implements part of JEP 381:
>>> 
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
>>> webrev: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/security/open/webrev/
>>> JEP: https://bugs.openjdk.java.net/browse/JDK-8241787
>>> 
>>> 
>>> Note: When reviewing this, please be aware that this exercise was *extremely* mind-numbing, so I appreciate your help reviewing all the individual changes carefully. You may want to get that coffee cup filled up (or whatever keeps you awake)!
>>> 
>>> 
>>> Background:
>>> 
>>> Because of the size of the total patch and wide range of areas touched, this patch is one out of in total six partial patches which together make up the necessary changes to remove the Solaris and SPARC ports. The other patches are being sent out for review to mailing lists appropriate for the respective areas the touch. An email will be sent to jdk-dev summarizing all the patches/reviews. To be clear: this patch is *not* in itself complete and stand-alone - all of the (six) patches are needed to form a complete patch. Some changes in this patch may look wrong or incomplete unless also looking at the corresponding changes in other areas.
>>> 
>>> For convenience, I’m including a link below[1] to the full webrev, but in case you have comments on changes in other areas, outside of the files included in this thread, please provide those comments directly in the thread on the appropriate mailing list for that area if possible.
>>> 
>>> In case it helps, the changes were effectively produced by searching for and updating any code mentioning “solaris", “sparc”, “solstudio”, “sunos”, etc. More information about the areas impacted can be found in the JEP itself.
>>> 
>>> 
>>> Testing:
>>> 
>>> A slightly earlier version of this change successfully passed tier1-8, as well as client tier1-2. Additional testing will be done after the first round of reviews has been completed.
>>> 
>>> Cheers,
>>> Mikael
>>> 
>>> [1] http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/all/open/webrev/
>> 
> 




More information about the security-dev mailing list