On Tue, 8 Apr 2025 20:02:56 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.-
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11HKDF.java line 41:
> 39: import sun.security.pkcs11.wrapper.*;
> 40: import static sun.security.pkcs11.wrapper.PKCS11Constants.*;
> 41: import static sun.security.pkcs11.wrapper.PKCS11Exception.RV.*;
Nitpick: Does this import need to be a `*`? Wouldn't it be better to just have
a `import static
sun.security.pkcs11.wrapper.PKCS11Exception.RV.CKR_KEY_SIZE_RANGE;
`?
test/jdk/sun/security/pkcs11/KDF/TestHKDF.java line 619:
> 617: k.deriveKey("AES", HKDFParameterSpec.ofExtract()
> 618: .thenExpand(null, 31));
> 619: throw new Exception("No exception thrown.");
I think this throw is unnecessary, you can just call this code straight on line
619 and remove the catch block below
reportTestFailure(new Exception("Derivation of an AES key of " +
"invalid size (31 bytes) expected to throw " +
"InvalidAlgorithmParameterException.", e));
What do you think?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24526#discussion_r2035131173
PR Review Comment: https://git.openjdk.org/jdk/pull/24526#discussion_r2035122389
