RFR: 8342442: Static ACVP sample tests [v12]

Mark Powers mpowers at openjdk.org
Fri Nov 8 20:36:21 UTC 2024


On Fri, 8 Nov 2024 18:00:42 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Here we have a launcher and several algorithm-specific tests. Users can populate "internalProjection.json" files generated by NIST's ACVP Server into the `data` directory and test them with the launcher.
>> 
>> Currently, only SHA2, SHA3, ML-KEM, and ML-DSA are supported.
>
> Weijun Wang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision:
> 
>  - Merge branch 'master' into 8342442
>  - more precise ML-DSA sigVer negative test
>  - more clear exception message
>  - check provider availability; SourceRandom in lib; use Utils.toByteArray
>  - rename property names, add an example
>  - acvp.test.alg system property
>  - remove plugin design
>  - rename
>  - grammar issues
>  - license
>  - ... and 3 more: https://git.openjdk.org/jdk/compare/71ce3c80...a7e22873

test/jdk/sun/security/provider/acvp/Launcher.java line 42:

> 40: /// directory specified by the `test.acvp.data` test property.
> 41: /// The test walks through the directory recursively and looks for
> 42: /// file names equals to or ending with `internalProjection.json` and

I think it should be "equal to".

test/jdk/sun/security/provider/acvp/Launcher.java line 77:

> 75:             var p = Security.getProvider(provProp);
> 76:             if (p == null) {
> 77:                 System.err.println(provProp + " is not a registered provider name");

Seems odd to print the error message and throw an exception with the same message.

test/jdk/sun/security/provider/acvp/ML_DSA_Test.java line 85:

> 83:                     public byte[] getEncoded() { return toByteArray(c.get("sk").asString()); }
> 84:                 };
> 85:                 var sr = new FixedSecureRandom(det ? new byte[32] : toByteArray(c.get("rnd").asString()));

This line and others are greater than 80 characters.

test/jdk/sun/security/provider/acvp/ML_DSA_Test.java line 89:

> 87:                 s.update(toByteArray(c.get("message").asString()));
> 88:                 var sig = s.sign();
> 89:                 Asserts.assertEqualsByteArray(sig, toByteArray(c.get("signature").asString()));

If more than one test fails, it might be useful to print out all the failures and then, at the end, throw an assert saying one or more tests failed.

test/jdk/sun/security/provider/acvp/ML_KEM_Test.java line 60:

> 58:                 System.out.print(c.get("tcId").asString() + " ");
> 59:                 g.initialize(np, new FixedSecureRandom(
> 60:                         toByteArray(c.get("d").asString()), toByteArray(c.get("z").asString())));

This line and others below are way over 80 characters.

test/lib/jdk/test/lib/security/FixedSecureRandom.java line 28:

> 26: import java.security.SecureRandom;
> 27: 
> 28: /// A custom implementation of `SecureRandom` that outputs a

Anything special about `///`?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1834912418
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1834922801
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1834941865
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1834967912
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1834975363
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835028741


More information about the security-dev mailing list