RFR: 8340327: A common framework to support public key algorithms with standard parameter sets [v6]

[Kevin Driver]

On Thu, 3 Oct 2024 17:40:22 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> To prepare for new PQC algorithms like ML-KEM and ML-DSA where there are only named standardized parameter sets, a common framework is introduced.
>> 
>> A example of EdDSA implementation using this framework is included as a test.
>
> 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 six additional commits since the last revision:
> 
>  - Merge branch 'master' into 8340327
>  - more test, more RAW support, fix a bug on cleaning up getRawBytes output
>  - add support for private class RawKeySpec
>  - ensure key is intact after being used
>  - renames
>  - the fix

src/java.base/share/classes/sun/security/provider/NamedSignature.java line 169:

> 167:     /// @throws ProviderException if there is an internal error
> 168:     /// @throws SignatureException if there is another error
> 169:     public abstract byte[] implSign(String name, byte[] sk, Object sk2,

I'm sure you have considered the visibility choices here, but I thought I'd ask why this should be `public`.

src/java.base/share/classes/sun/security/provider/NamedSignature.java line 182:

> 180:     /// @throws ProviderException if there is an internal error
> 181:     /// @throws SignatureException if there is another error
> 182:     public abstract boolean implVerify(String name, byte[] pk, Object pk2,

I'm sure you have considered the visibility choices here, but I thought I'd ask why this should be `public`.

src/java.base/share/classes/sun/security/provider/NamedSignature.java line 198:

> 196:     /// @return a parsed key, `null` if none.
> 197:     /// @throws InvalidKeyException if the key is invalid
> 198:     public Object implCheckPublicKey(String name, byte[] pk) throws InvalidKeyException {

I'm sure you have considered the visibility choices here, but I thought I'd ask why this should be `public`.

src/java.base/share/classes/sun/security/provider/NamedSignature.java line 215:

> 213:     /// @return a parsed key, `null` if none.
> 214:     /// @throws InvalidKeyException if the key is invalid
> 215:     public Object implCheckPrivateKey(String name, byte[] sk) throws InvalidKeyException {

I'm sure you have considered the visibility choices here, but I thought I'd ask why this should be `public`.

src/java.base/share/classes/sun/security/x509/NamedX509Key.java line 46:

> 44:     private final String fname;
> 45:     private final transient NamedParameterSpec paramSpec;
> 46:     private final byte[] h;

Can this name be more descriptive?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1787983385
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1787983495
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1787983610
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1787983721
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1787984861