RFR: 8353578: Refactor existing usage of internal HKDF impl to use the KDF API [v7]

Bradford Wetmore wetmore at openjdk.org
Tue May 6 05:14:16 UTC 2025


On Thu, 1 May 2025 18:49:33 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> This PR removes the internal JSSE HKDF impl and changes to use the KDF API for the HKDF support from JCA/JCE providers.
>> 
>> This is just code refactoring. Known-answer regression test for the internal JSSE HKDF impl is removed as the test vectors are already covered by the HKDF impl in SunJCE provider.
>> 
>> Thanks in advance for the review~
>
> Valerie Peng has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review comments from Sean.

Just a few comments to think about, and one copyright nit.

src/java.base/share/classes/com/sun/crypto/provider/DHKEM.java line 409:

> 407:             HKDFParameterSpec spec =
> 408:                     HKDFParameterSpec.ofExtract().addIKM(s).extractOnly();
> 409:             return hkdf.deriveKey("Generic", spec);

I haven't done much with DHKEM yet, but should the returned key have algorithm name of "Generic," or something more descriptive like the previous "HKDF-PRK"?

src/java.base/share/classes/sun/security/ssl/CipherSuite.java line 1:

> 1: /*

Copyright update.

src/java.base/share/classes/sun/security/ssl/ServerHello.java line 1222:

> 1220:             CipherSuite.HashAlg hashAlg = hc.negotiatedCipherSuite.hashAlg;
> 1221:             KDF hkdf = KDF.getInstance(hashAlg.hkdfAlgorithm);
> 1222:             SecretKey earlySecret = hkdf.deriveKey("TlsEarlySecret",

I'm a little worried that the proper number of salt zeros are now expected to be known in the KDF deriveKey code instead of specified specifically here (and in other similar places).  Should we consider specifying them here and the other places instead to play it safe?

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

PR Review: https://git.openjdk.org/jdk/pull/24393#pullrequestreview-2816859138
PR Review Comment: https://git.openjdk.org/jdk/pull/24393#discussion_r2074723921
PR Review Comment: https://git.openjdk.org/jdk/pull/24393#discussion_r2074724316
PR Review Comment: https://git.openjdk.org/jdk/pull/24393#discussion_r2074735247


More information about the security-dev mailing list