RFR: 8342442: Static ACVP sample tests [v8]

Roger Riggs rriggs at openjdk.org
Fri Nov 1 20:30:23 UTC 2024


On Fri, 1 Nov 2024 18:32:53 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 incrementally with one additional commit since the last revision:
> 
>   rename property names, add an example

The duplication in utilities (RandomSource and xeh(String) should be avoided.
Perhaps in the test library or a common utility class.

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

> 32:  * @bug 8342442
> 33:  * @library /test/lib
> 34:  */

This seems more like an '@driver' type of test than the implied `@run` test.

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

> 43:         var provProp = System.getProperty("test.acvp.provider");
> 44:         PROVIDER = provProp != null
> 45:                 ? Security.getProvider(provProp)

How are errors in the provider prop reported? In a static block, will an uncaught exception provide enough/correct information to correct the supplied properties.

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

> 76:         //       -Dtest.acvp.alg=ML-KEM \
> 77:         //       -Dtest.acvp.data=/path/to/json-files/ \
> 78:         //       -jdk:/path/to/jdk Launcher.java

This seems more like either the class javadoc or the javadoc for main().  (Not an inline comment).

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

> 118:                 case "SHA2-256", "SHA2-224", "SHA3-256", "SHA3-224"
> 119:                     -> SHA_Test.run(kat, PROVIDER);
> 120:                 default -> System.out.println("Skipped unsupported algorithm: " + alg);

This seems more like a test configuration error and should not silently be ignored (you have to read the output to see the error/warning).
Perhaps it could be a "Skipped" kind of test, so it gets reported.

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

PR Review: https://git.openjdk.org/jdk/pull/21548#pullrequestreview-2410805715
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1826250496
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1826249022
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1826248157
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1826251779


More information about the security-dev mailing list