On Thu, 7 Nov 2024 16:51:38 GMT, Kevin Driver <kdri...@openjdk.org> wrote:
>> Ben Perez has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - default random for encaps, supported alg in SunJCE >> - copyright header > > src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 515: > >> 513: mlKemH.update(encapsKey); >> 514: mlKemH.digest(decapsKey, kPkePrivateKey.length + >> encapsKey.length, 32); >> 515: System.arraycopy(kem_z, 0, decapsKey, kPkePrivateKey.length + >> encapsKey.length + 32, 32); > > Should values be zeroed after this line/before return? Which value? `kem_z` is an input argument `decapsKey` is an output argument. They belong to the caller. > src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 568: > >> 566: var kAndCoins = mlKemG.digest(); >> 567: var realResult = Arrays.copyOfRange(kAndCoins, 0, 32); >> 568: var coins = Arrays.copyOfRange(kAndCoins, 32, 64); > > Several copies take place here. Should anything be zeroed? We should zero out `kPkePrivateKeyBytes` and `kAndCoins`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1834358403 PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1834360261