On Mon, 21 Oct 2024 15:28:47 GMT, Ben Perez <bpe...@openjdk.org> wrote:
>> Java implementation of ML-DSA, the FIPS 204 post-quantum signature scheme >> https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.204.pdf. Depends on >> https://github.com/openjdk/jdk/pull/21167 > > Ben Perez has updated the pull request incrementally with one additional > commit since the last revision: > > Revert "ML-DSA for jarsigner" > > This reverts commit cc231109513d0f3a939f0bff92a890ff921d94e0. src/java.base/share/classes/sun/security/provider/ML_DSA.java line 1: > 1: /* Does this class need to be public? Many methods are also public - do they need to be? src/java.base/share/classes/sun/security/provider/ML_DSA_Provider.java line 33: > 31: import java.util.Arrays; > 32: > 33: public class ML_DSA_Provider { This class isn't a `Provider`. Can we name it something else, like`ML_DSA_Impl`? src/java.base/share/classes/sun/security/provider/ML_DSA_Provider.java line 82: > 80: } > 81: > 82: public static class KPG5 extends KPG { What is the numbering scheme used here, why is this named KPG5? src/java.base/share/classes/sun/security/provider/ML_DSA_Provider.java line 90: > 88: public static class KF extends NamedKeyFactory { > 89: public KF() { > 90: super("ML-DSA", "ML-DSA-44", "ML-DSA-65", "ML-DSA-87"); Add a comment with the default as you do for `KPG`. src/java.base/share/classes/sun/security/provider/ML_DSA_Provider.java line 118: > 116: public static class SIG extends NamedSignature { > 117: public SIG() { > 118: super("ML-DSA", "ML-DSA-44", "ML-DSA-65", "ML-DSA-87"); Add a comment with the default as you do for KPG. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1809451837 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1809457804 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1809463327 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1809464756 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1809465058
