On Tue, 8 Apr 2025 20:02:56 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.-
> On a related note, I am working on #24393 and noticed that JSSE calls
> HKDF.deriveKey(...) with various names such as "TlsFinishedSecret",
> "TlsResumptionMasterSecret" as the key algorithm. This causes errors when
> using PKCS11 HKDF since the `P11SecretKeyFactory.getKeyInfo()` look up
> returns `null` and leads to `InvalidAlgorithmParameterException`. I am
> debating whether to do the special handling as below:
>
> ```
> P11SecretKeyFactory.KeyInfo ki = P11SecretKeyFactory.getKeyInfo(alg);
> if (ki == null) {
> - throw new InvalidAlgorithmParameterException("A PKCS #11 key " +
> - "type (CKK_*) was not found for a key of the algorithm
> '" +
> - alg + "'.");
> + // special handling for TLS
> + if (alg.startsWith("Tls")) {
> + ki = P11SecretKeyFactory.getKeyInfo("Generic");
> + } else {
> + throw new InvalidAlgorithmParameterException("A PKCS #11 key
> " +
> + "type (CKK_*) was not found for a key of the
> algorithm '" +
> + alg + "'.");
> + }
> ```
>
> Comments or suggestions? @martinuy
Thanks for the heads up. I was just looking at the possibility of passing key
algorithms for derived keys that were not considered before but are part of the
map (e.g. hmac, pbe, etc.), and wondered how that should be handled. Tls could
be the opposite case: not part of the map but needs to be handled. Will give it
some thinking.
I was also thinking of extending the `P11SecretKeyFactory::keyInfo` map to
include key generation mechanisms that would facilitate querying
`CK_MECHANISM_INFO` and obtaining valid key size ranges for each key type. This
would simplify the call to `P11KeyGenerator::checkKeySize` from
`P11SecretKeyFactory::createKey` and allow a call from `P11HKDF`. While the
idea suggested by @djelinski sounds reasonable, I want to notice an implicit
assumption that may not hold true for all PKCS 11 libraries:
`CK_MECHANISM_INFO` data for mechanisms such as `CKM_AES_KEY_GEN` will be used
for mechanisms such as `CKM_HKDF_DATA`/`CKM_HKDF_DERIVE`.
`P11SecretKeyFactory::createKey` has a similar type of assumption.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/24526#issuecomment-2791482999
