On Thu, 10 Apr 2025 23:54:03 GMT, Martin Balao <mba...@openjdk.org> 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 two additional
> commits since the last revision:
>
> - Algorithm and key size checking before derivation. Mechanism normalization
> for TLS.
> - Minor import adjustment.
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11HKDF.java line 251:
> 249: (int) PCKK_TLSMASTER -> {
> 250: return CKK_GENERIC_SECRET;
> 251: }
It's easier to troubleshoot to add a default case and not let it fall through
to the exception on line 253? It's possible that P11SecretKeyFactory is
enhanced with more KeyInfo, but the newly added keyType is not added here.
Lumping different causes into the same exception may be harder to debug.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24526#discussion_r2040321172
