RFR: 8312428: PKCS11 tests fail with NSS 3.91 [v2]

[Valerie Peng]

On Thu, 10 Aug 2023 18:12:59 GMT, Rajan Halade <rhalade at openjdk.org> wrote:

>> Valerie Peng has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   address review feedbacks
>
> test/jdk/sun/security/pkcs11/PSSUtil.java line 24:
> 
>> 22:  */
>> 23: import java.security.*;
>> 24: import java.security.interfaces.*;
> 
> Please remove unused imports

Sure

> test/jdk/sun/security/pkcs11/PSSUtil.java line 34:
> 
>> 32:     private static final String KEYALG = "RSA";
>> 33:     private static final String SIGALG = "RSASSA-PSS";
>> 34:     private static final String[] DIGESTS = {
> 
> is this constant used anywhere or supposed to be used?

Removed.

> test/jdk/sun/security/pkcs11/PSSUtil.java line 59:
> 
>> 57:         for (String h : hashAlgs) {
>> 58:             String sigAlg = (h.startsWith("SHA3-")?
>> 59:                     h : h.replace("-","")) + "withRSASSA-PSS";
> 
> Suggestion:
> 
>             String sigAlg = (h.startsWith("SHA3-") ?
>                     h : h.replace("-", "")) + "withRSASSA-PSS";

Ok.

> test/jdk/sun/security/pkcs11/Signature/KeyAndParamCheckForPSS.java line 129:
> 
>> 127:         sig.setParameter(paramsGood);
>> 128:         sig.initVerify(pub);
>> 129:         System.out.println("test#2: good params pass");
> 
> Either add a `test#1` message after `initSign` test or update this to remove `#2`

test#1 message matches with test#2 ones. Anyway, I removed the test output messages with your comments.

> test/jdk/sun/security/pkcs11/Signature/SignatureTestPSS.java line 66:
> 
>> 64:     public void main(Provider p) throws Exception {
>> 65:         if (!PSSUtil.isSignatureSupported(p)) {
>> 66:             System.out.println("Skip testing RSASSA-PSS" +
> 
> throw SkippedException here.

Ok. I initially thought that the SkippedException only applies to library not found and/or un-configured OS. With this extension of the meaning of SkippedException, many of the existing tests have to be updated to match.

> test/jdk/sun/security/pkcs11/Signature/SignatureTestPSS.java line 83:
> 
>> 81:                             PSSUtil.isHashSupported(p, hash, mgfHash);
>> 82:                     if (s == PSSUtil.AlgoSupport.NO) {
>> 83:                         System.out.println("    => Skip; no support");
> 
> Similar request here to mark test as skipped if all algorithms are not supported.

This is just one digest algorithm. To mark the test skipped if none of the algorithms are supported will require different handling.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290608992
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290607168
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290614205
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290598689
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290601153
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290602675