On Thu, 17 Apr 2025 03:14:14 GMT, Martin Balao <[email protected]> wrote:
>> Hi,
>>
>> I would like to request a review for the fix of JDK-8350661. In this fix, we
>> translate the native PKCS 11 error code into an
>> `InvalidAlgorithmParameterException`, as documented in the `KDF::deriveKey`
>> API. With that said, different PKCS 11 libraries may throw different errors
>> and may even (in theory) delay the error until the key is used, as _SunJCE_
>> does. I believe that this is an improvement but further adjustments may be
>> needed in the future.
>>
>> No regressions observed in `test/jdk/sun/security/pkcs11/KDF/TestHKDF.java`.
>>
>> Thanks,
>> Martin.-
>
> Martin Balao has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Inform key sizes in the exception when failing check.
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11HKDF.java line 252:
> 250: throw new InvalidAlgorithmParameterException("A key of
> algorithm " +
> 251: "'" + alg + "' is not valid for derivation.");
> 252: }
So, essentially, we only allow HKDF to derive keys when ki is either
`TLSKeyInfo` or `KeyInfo` w/ the key types in `canDeriveKeyInfoType(long)`
method? Are you checking each key algorithm just to rule out `RC2` and `IDEA`?
Not sure if it's worth the extra checking. Besides, if more `CKK_xxx` is added
to `KeyInfo`, it'd need to be added to `canDeriveKeyInfoType(long)` which can
be easily missed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24526#discussion_r2049765576
