On Thu, 3 Oct 2024 17:40:22 GMT, Weijun Wang <wei...@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