RFR: 8298387: Implementing ML-DSA signature algorithm [v8]

[Weijun Wang]

On Mon, 21 Oct 2024 20:07:10 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> 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?

No need, but Mark's test might be using it.

> 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`?

I understand what you meant, but `ML_DSA` is the actual implementation and this class just expose the functions as `Spi`s. It's more like a glue between the implementation and JCA.

> 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?

The number maps to security levels of each parameter set. See `security_level` in `ML_DSA`.

> 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`.

There is no concept of default parameter set for `KeyFactory` or `Signature`. They are just supported parameter sets.

> 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.

See above.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1809502192
PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1809502333
PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1809505229
PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1809508685
PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1809509093