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

[Sean Mullan]

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

Some initial comments.

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

> 37: import java.security.ProviderException;
> 38: import java.security.spec.NamedParameterSpec;
> 39: 

Class description?

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

> 46:     private final byte[] h;
> 47: 
> 48:     /// Ctor from family name, parameter set name, raw key bytes.

Any reason you use 3 slashes instead of 2 for comments?

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

> 54:             this.algid = AlgorithmId.get(pname);
> 55:         } catch (NoSuchAlgorithmException e) {
> 56:             throw new ProviderException(e);

Can this ever happen?

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

> 65:         this.fname = fname;
> 66:         decode(encoded);
> 67:         paramSpec = new NamedParameterSpec(algid.getName());

Use `this` to be consistent with other ctor.

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

> 69:             throw new InvalidKeyException("algorithm identifier has params");
> 70:         }
> 71:         h = getKey().toByteArray();

Use this to be consistent with other ctor.

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

PR Review: https://git.openjdk.org/jdk/pull/21167#pullrequestreview-2346498738
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1786723545
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1786724186
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1786746283
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1786728384
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1786728759