On Thu, 10 Aug 2023 00:56:56 GMT, Valerie Peng <valer...@openjdk.org> wrote:
> Starting NSS v3.91, SHA-3 support is added for MessageDigest but not for PSS
> Signature. This breaks existing test assumptions made by PSS regression
> tests. In addition, the NSS SHA-3 message digests do not support cloning
> which causes the failure of TestCloning.java.
>
> This PR adds a PSSUtil.java class which provides utility method for
> detecting/guessing whether a digest algorithm is valid for PSS signature or
> not.
>
> The changes are verified with NSS v3.46, v3.57 and v3.91 (on local Linux
> machine).
>
> Thanks in advance for review~
test/jdk/sun/security/pkcs11/MessageDigest/TestCloning.java line 26:
> 24: /*
> 25: * @test
> 26: * @bug 6414899 8242332 8312428
No need to add bug id for test only updates.
test/jdk/sun/security/pkcs11/MessageDigest/TestCloning.java line 67:
> 65: } catch (CloneNotSupportedException cnse) {
> 66: // skip test if clone isn't supported
> 67: System.out.println("=> Clone not supported; skip!");
Can you please update the test to throw SkippedException if no digest
algorithms are found to not support clone? This would help us with coverage
analysis.
test/jdk/sun/security/pkcs11/PSSUtil.java line 24:
> 22: */
> 23: import java.security.*;
> 24: import java.security.interfaces.*;
Please remove unused imports
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?
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";
test/jdk/sun/security/pkcs11/Signature/KeyAndParamCheckForPSS.java line 51:
> 49: public void main(Provider p) throws Exception {
> 50: if (!PSSUtil.isSignatureSupported(p)) {
> 51: System.out.println("Skip testing RSASSA-PSS" +
Update to throw SkippedException.
test/jdk/sun/security/pkcs11/Signature/KeyAndParamCheckForPSS.java line 75:
> 73: runTest(p, 1040, "SHA3-512", "SHA3-256");
> 74: runTest(p, 1040, "SHA3-512", "SHA3-384");
> 75: runTest(p, 1040, "SHA3-512", "SHA3-512");
Similar check can be added here to throw SkippedException if all runTest call
skip testing.
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/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.
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290485564
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290488107
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290509833
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290510886
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290513331
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290490575
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290492472
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290494138
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290496752
PR Review Comment: https://git.openjdk.org/jdk/pull/15217#discussion_r1290497611
