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

[Valerie Peng]

One more thing:

> test/jdk/sun/security/pkcs11/Config/ReadConfInUTF16Env.java:
>
>     Not worth keeping the tests in this directory.
We do need the tests under test/jdk/sun/security/pkcs11/Config/, please 
add them back if you have already removed them.

As stated earlier, the config parsing function can be tested against the 
dummy SunPKCS11 provider.

Thanks,
Valerie
On 5/7/2020 2:48 PM, Valerie Peng wrote:
>> 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/