On Fri, 4 Oct 2024 20:59:45 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 src/java.base/share/classes/sun/security/provider/ML_DSA.java line 545: > 543: int[][] s1 = > Arrays.stream(sk.s1()).map(int[]::clone).toArray(int[][]::new); > 544: int[][] s2 = > Arrays.stream(sk.s2()).map(int[]::clone).toArray(int[][]::new); > 545: int[][] t0 = > Arrays.stream(sk.t0()).map(int[]::clone).toArray(int[][]::new); Instead of calling `mlDsa.skDecode(skBytes)` in `ML_DSA_Provider`, can we move the call here? Then `sk` becomes a local variable and you probably don't need to make the deep clones above. src/java.base/share/classes/sun/security/provider/ML_DSA.java line 563: > 561: hash.update(rnd); > 562: hash.update(mu); > 563: byte[] rhoPrime = hash.squeeze(mlDsaMaskSeedLength); The name is `rhoDoublePrime`. src/java.base/share/classes/sun/security/provider/ML_DSA.java line 660: > 658: //Check verify conditions > 659: boolean hashEq = Arrays.equals(sig.commitmentHash(), > cTildePrime); > 660: boolean weight = hammingWeight(sig.hint()) <= omega; This is no longer required in the final FIPS 204. On the other hand, `hintBitUnpack` is modified to add more checks. src/java.base/share/classes/sun/security/provider/ML_DSA.java line 665: > 663: > 664: /* > 665: Data conversion functions in Section 8.1 of specification It's Section 8 now. src/java.base/share/classes/sun/security/provider/ML_DSA_Provider.java line 109: > 107: } > 108: > 109: // TODO: check key in initSign and initVerify? Maybe you can at least check the size of the keys? src/java.base/share/classes/sun/security/provider/ML_DSA_Provider.java line 135: > 133: var mlDsa = new ML_DSA(size); > 134: var pk = mlDsa.pkDecode(pkBytes); > 135: var sig = mlDsa.sigDecode(sigBytes); Check the size of `sigBytes` and throw a `SignatureException` if invalid. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1791986347 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1791999822 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1792033938 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1792048448 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1791975633 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1791978136
