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

[Valerie Peng]

> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Config.java:
>
>     Maybe we should not support $ISA at all?

I suppose it should be ok not supporting $ISA. Existing PKCS11 provider 
documentation does not mention it anyway.

> test/jdk/sun/security/pkcs11/Provider/ConfigShortPath.java:
>
>     Will this test ever run? There is no out-of-box SunPKCS11 provider. Even if one is configured manually, the name is likely to be SunPKCS11-XYZ,
This test will be run as there is an out-of-box PKCS11 provider named "SunPKCS11" (which is sort of a dummy for actual crypto operations but it can still be used to test the generic configuration parsing functionality in this test).


As for removing the tests, I am more on the conservative side. We should only remove tests which are no longer necessary due to the removal of Solaris support. There are already a lot of changes involved here.
Valerie

On 5/4/2020 6:58 AM, Weijun Wang wrote:
> I've gone through all files. Many of them are PKCS11-related, it will be nice if Valerie can confirm my findings.
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Config.java:
>
>     Maybe we should not support $ISA at all?
>
> test/jdk/com/sun/security/auth/login/ConfigFile/InconsistentError.java:
>
>     Please also remove the whole else block from line 61.
>
> test/jdk/java/security/KeyPairGenerator/SolarisShortDSA.java:
>
>     It's not worth keeping this test. Error has never occurred on other platforms.
>
> test/jdk/java/security/KeyStore/TestKeyStoreBasic.java:
>
>     128-135: There is no need to use a try-catch block.
>
> test/jdk/java/security/KeyStore/TestKeyStoreEntry.java:
>
>     54-60: No need
>     81-97: No need. Just a single run() is OK.
>     101: Remove the ", p" argument
>
> test/jdk/java/security/MessageDigest/TestDigestIOStream.java:
> test/jdk/java/security/MessageDigest/TestSameLength.java:
> test/jdk/java/security/MessageDigest/TestSameValue.java:
>
>     No need for isSHA3Supported(). The SUN provider should always be there.
>     Inside the test where NoSuchAlgorithmException is caught, the catch block can be removed and no need to try
>
> test/jdk/java/security/MessageDigest/UnsupportedProvider.java:
>
>     Not worth keeping this test.
>
> test/jdk/sun/security/jca/PreferredProviderTest.java:
>
>     53: remove "for other platform"
>
> test/jdk/sun/security/pkcs/pkcs8/PKCS8Test.java
>
>     Remove the comments on lines 37-40
>
> test/jdk/sun/security/pkcs11/Config/ReadConfInUTF16Env.java:
>
>     Not worth keeping the tests in this directory.
>
> test/jdk/sun/security/pkcs11/Mac/MacSameTest.java:
>
>     No need for try
>
> test/jdk/sun/security/pkcs11/Provider/ConfigShortPath.java:
>
>     Will this test ever run? There is no out-of-box SunPKCS11 provider. Even if one is configured manually, the name is likely to be SunPKCS11-XYZ,
>
> test/jdk/sun/security/pkcs11/tls/TestKeyMaterial.java:
>
>     No need for this catch block
>
> test/jdk/sun/security/tools/keytool/fakegen/PSS.java:
>
>     Please also remove the comment on lines 35-37.
>
> test/jdk/sun/security/tools/keytool/fakegen/DefaultSignatureAlgorithm.java:
>
>     Please also remove the comment on lines 36-38.
>
> Thanks,
> Max
>
>
>
>> On May 4, 2020, at 1: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/