[LinuxJedi]

With CKF_HKDF_SALT_KEY, CKM_HKDF_DERIVE and CKM_HKDF_DATA take the salt from a key, and that key is the HMAC key of HKDF-Extract, so the derived key depends on its value as much as on the base key's. The salt key only had to be a secret key with a value:

• It needed no CKA_DERIVE.
• The derived key's protection came from the base key alone. A readable base key with a sensitive or non-extractable salt key made an extractable, non-sensitive key whose value depends on the protected salt.

This was found while working on #291, which keeps tied TLS master secrets away from HKDF (as base or salt). This gap applies to any protected secret key used as a salt, so it gets its own PR.

Change. The salt key is now treated as the other key of CKM_CONCATENATE_BASE_AND_KEY already is (F-4533):

• It needs CKA_DERIVE, or the derive fails with CKR_KEY_FUNCTION_NOT_PERMITTED. As for the base key, this is skipped where WP11_NSS_PERMISSIVE_KEY_DEFAULTS is set (NSS builds).
• The derived key is sensitive if either key is and non-extractable if either key is. A template asking for less gets CKR_TEMPLATE_INCONSISTENT.
• CKA_ALWAYS_SENSITIVE and CKA_NEVER_EXTRACTABLE need both keys to have them, and CKA_WRAP_WITH_TRUSTED carries over from the salt key.
• The 16 byte derive length guard from Refuse protected derived keys shorter than 16 bytes #292 reads the folded protection, so it now also applies to a protected salt key under a readable base key.
• Builds with WOLFPKCS11_LEGACY_DERIVE_NO_INHERIT, which NSS builds use, leave the salt key out of the protection, as they do the base key.
• The salt is ignored, and not checked, when bExtract is false, as the specification says.

PKCS#11 3.0 (section 2.62) says nothing about the salt key beyond "salt is supplied as a key in hSaltKey", and its sensitivity rules only mention the base key, so this goes past the specification in the same way the base key inheritance does.

test_wrap_key_wrap_with_trusted used a trusted key without CKA_DERIVE as the salt key in its check that an undecoded token key fails with CKR_FUNCTION_FAILED. The key now sets CKA_DERIVE so the test still reaches that check.

The README also named CKR_KEY_TYPE_INCONSISTENT for a base key without CKA_DERIVE, which has returned CKR_KEY_FUNCTION_NOT_PERMITTED since F-6052. That is corrected next to the new salt key notes.

Not changed. The base key's CKA_WRAP_WITH_TRUSTED is still only carried over by the concatenation mechanisms. HKDF, ECDH, DH, the TLS PRF and AES encrypt-data derives drop it, as before.

Testing:

• kdf_tls_derive_test checks that a salt key without CKA_DERIVE is refused for both HKDF mechanisms (and accepted on NSS), and that an expand-only derive ignores it. It checks that an unprotected salt key works and gives the same key as the same salt passed as data. With a readable base key, a sensitive, non-extractable or wrap-with-trusted salt key passes its protection on and a weaker template is refused. A generated salt key keeps CKA_ALWAYS_SENSITIVE and CKA_NEVER_EXTRACTABLE, and an imported one clears them.
• derive_length_guard_test checks that an 8 byte key from a protected salt key and a readable base key is refused, while 16 bytes, or 8 bytes from a readable salt key, are allowed.
• With only the src/crypto.c change reverted, the new checks fail on "HKDF salt key without CKA_DERIVE rejected", "non-sensitive key from sensitive salt key refused", "imported salt clears CKA_ALWAYS_SENSITIVE" and "8 byte HKDF key from a protected salt key".
• make check passes on the default configuration (85 pass, 10 skipped) and with --enable-nss (85 pass, 8 skipped), and the pre-commit hook make test passes.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[LinuxJedi]

@claude review

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟡 src/crypto.c — A sensitive or non-extractable HKDF salt key can still yield keys under 16 bytes when the base is a CKO_DATA object. The new length guard on salt keys never runs in that case. The guard at src/crypto.c:11780 skips whenever WP11_Object_GetClass(obj) == CKO_DATA, even though baseSensitive/baseExtractable now come from the salt key via otherKey. Fix: skip the guard only when no protected key fed the derive, e.g. (WP11_Object_GetClass(obj) != CKO_DATA || otherKey != NULL). Readable data-object derives keep their current short-length behaviour. [also at: src/crypto.c:11782 - With a data-object HKDF base, a protected salt key still yields keys shorter than 16 bytes, so the length guard misses this case.]

  Why this was flagged

  The trigger is C_DeriveKey with CKM_HKDF_DATA, a CKO_DATA base object with CKA_DERIVE set, bExtract=CK_TRUE, ulSaltType=CKF_HKDF_SALT_KEY, a sensitive or non-extractable salt key and CKA_VALUE_LEN=8. This needs a build without WOLFPKCS11_LEGACY_DERIVE_NO_INHERIT. CheckKeyObject (src/crypto.c:653) accepts a CKO_DATA base for CKA_DERIVE, and src/crypto.c:11259 lets it through the HKDF type check. The new code sets otherKey = saltObj (src/crypto.c:11298), so lines 11737-11745 fold the salt key's protection into baseSensitive/baseExtractable. The DerivedKeyTooShort check at src/crypto.c:11780 has a WP11_Object_GetClass(obj) != CKO_DATA exemption, so it is skipped and the 8-byte protected key is created. The README and PR say the 16-byte guard now covers a protected salt key. That holds only for a key base, which is the only case test_hkdf_salt_key in tests/derive_length_guard_test.c exercises. The exemption was written when only the base's protection counted, and a data object has none.

  Verification: CheckKeyObject (src/crypto.c:651-653) accepts CKO_DATA for CKA_DERIVE. Lines 11737-11745 then fold the salt key's CKA_SENSITIVE/CKA_EXTRACTABLE into baseSensitive/baseExtractable. The length guard at 11780 is still gated on the base object's class, so with a data base DerivedKeyTooShort is never called. On the base branch the salt key's protection was not inherited at all.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #296

Scan targets checked: wolfpkcs11-src, wolfpkcs11-bugs
Not reviewed (no scan target covers them): README.md
Coverage: 4 of 4 in-scope changed file(s) opened by the reviewer

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Review tier: Max

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #296

Scan targets checked: wolfpkcs11-src, wolfpkcs11-bugs
Not reviewed (no scan target covers them): README.md
Coverage: 2 of 2 in-scope changed file(s) opened by the reviewer

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Lite

Fenrir's latest completed scan found no issues; clearing the prior automated change request.