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