RFR: 8298390: Implementing ML-KEM key encapsulation mechanism [v20]

Ben Perez bperez at openjdk.org
Wed Nov 13 21:20:48 UTC 2024


On Tue, 12 Nov 2024 22:45:33 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Ben Perez has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   no classpath exception in test copyright header
>
> src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 471:
> 
>> 469:             }
>> 470:         }
>> 471:         return null;
> 
> Why return null? Why not just use `void` as return type? Same for the `checkPrivateKey(...)` method.

This is because the `NamedKEM` methods for checking keys can optionally return the key. I can pass that key along if that makes more sense

> src/java.base/share/classes/com/sun/crypto/provider/SHA3Parallel.java line 37:
> 
>> 35: import static sun.security.provider.SHA3.keccak;
>> 36: 
>> 37: public class SHA3Parallel {
> 
> Why not merge this with `sun.security.provider.SHA3` class? A separate class in a different package seems harder to track...

@ferakocz might have some more insight here

> src/java.base/share/classes/com/sun/crypto/provider/SHA3Parallel.java line 122:
> 
>> 120:     }
>> 121: 
>> 122:     public static final class Shake256Parallel extends SHA3Parallel {
> 
> I didn't find usage of this class in this PR? Is this for future usage?

This is used in ML-DSA, but I can remove it from this PR

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1841166694
PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1841169108
PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1841168499


More information about the security-dev mailing list