RFR: 8342442: Static ACVP sample tests [v12]

Sean Mullan mullan at openjdk.org
Fri Nov 8 21:25:47 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/d3963867...a7e22873

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

> 1: /*

By default, does this test run and do nothing because there are no `internalProjection.json` files? Are those going to be added as test data files later? Should the test be reported as skipped if there is nothing to test?

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

> 34:  */
> 35: 
> 36: /// This test runs on `internalProjection.json`-style files generated

Is there a reason you didn't put all these comments (or a copy of them) in the README file instead? It seems like a better place for it. The README doesn't help me understand how to run the tests.

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

> 41: /// The test walks through the directory recursively and looks for
> 42: /// file names equals to or ending with `internalProjection.json` and
> 43: /// runs test on them.

s/test/tests/

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

> 43: /// runs test on them.
> 44: ///
> 45: /// Set the `test.acvp.alg` test property to only test this algorithm.

What is "this algorithm"? Should it maybe be "the specified algorithm"?

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

> 49: ///
> 50: /// By default, the test uses system-preferred implementations.
> 51: /// If you want to test on a specific provider, set the

s/on a/a/

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835067732
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835055952
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835056533
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835057439
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835058786


More information about the security-dev mailing list