On Thu, 8 May 2025 21:19:10 GMT, Valerie Peng <valer...@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 feedbacks from Brad. src/java.base/share/classes/sun/security/ssl/SSLBasicKeyDerivation.java line 57: > 55: KDF hkdf = KDF.getInstance(hkdfAlg); > 56: return hkdf.deriveKey(type, > 57: HKDFParameterSpec.expandOnly(secret, hkdfInfo, > keyLen)); I noticed we have done away with the `AlgorithmParameterSpec` parameter to this method. While the new implementation makes sense in light of the new parameter set, I wonder whether there was a deliberate use-case lost here... In the previous implementation, presumably, if the `keySpec`'s length was shorter than the expanded value, only a portion of the value would be used. With the new implementation, the hash algorithm's length of bytes is always used. I'm not sure how relevant this is, but it could be worth noting. If we had kept the old parameter, I suppose we could have used `deriveData` and truncated the result to `((SecretSizeSpec)keySpec).length` before returning a `SecretKey`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24393#discussion_r2085338105
