Repository navigation
Add CKM_CONCATENATE_* derive mechanisms - #279
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The tests lack native end-to-end agreement and 96-byte capacity regression coverage, while an unconditional logout causes empty-PIN configurations to fail before derivation checks run.
2 open issues
What changed in this PR
Adds secret-key concatenation to support protected hybrid key-agreement inputs for HKDF, addressing #264 without exporting the component secrets.
Changes:
- Registers three concatenation mechanisms with length validation and protection-attribute inheritance.
- Expands ML-KEM key capacity in no-DH builds and improves AES derivation errors.
- Adds concatenation, storage-access, and deterministic concatenation-to-HKDF tests.
| File | Description |
|---|---|
| wolfpkcs11/pkcs11.h | Defines concatenation mechanisms and their data parameter type. |
| wolfpkcs11/internal.h | Declares concatenation helpers and expands ML-KEM key capacity. |
| tests/pkcs11test.c | Tests concatenation, protection inheritance, errors, and HKDF use. |
| tests/pkcs11str.c | Tests stored-key concatenation before login. |
| src/slot.c | Advertises concatenation mechanisms and capabilities. |
| src/internal.c | Implements key-state validation and concatenation helpers. |
| src/crypto.c | Adds derivation dispatch, attribute inheritance, and error handling. |
Review effort: Balanced
1a2f20a to
ea18833
Compare
|
@wolfSSL-Fenrir-bot review balanced |
|
@claude review |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #279
Scan targets checked: wolfpkcs11-src, wolfpkcs11-bugs
Coverage: 2 of 8 in-scope changed file(s) opened by the reviewer; not opened: src/slot.c, tests/pkcs11str.c, tests/pkcs11test.c, tests/pkcs11v3test.c, wolfpkcs11/internal.h, wolfpkcs11/pkcs11.h
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: Balanced
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Implement the PKCS#11 miscellaneous concatenation derive mechanisms (v3.1/v3.2 section 6.43.3 to 6.43.5). They let a hybrid key agreement such as ETSI TS 103 744 build PSK || Z_ECDH || Z_MLKEM as a sensitive, non-extractable secret and feed it to CKM_HKDF_DERIVE without exporting the parts. A CKA_VALUE_LEN shorter than the concatenation keeps the leading bytes, matching NSS and SoftHSM, and the data may then be longer than the largest key. The shorter length may not end inside the bytes of a sensitive or non-extractable input key: a caller could otherwise derive keys one byte longer at a time, use each to sign or encrypt, and find every key byte in 256 tries. Dropping trailing data, or all of a key, stays allowed. Without CKA_VALUE_LEN the key takes the summed length, as in NSS softoken; NSS asks for a CKK_HKDF key this way when it combines the secrets of its hybrid TLS groups. An AES key has no single well-defined length, so it needs CKA_VALUE_LEN and otherwise gets CKR_TEMPLATE_INCOMPLETE. CKA_KEY_TYPE stays required, as for the other derive mechanisms. For CKM_CONCATENATE_BASE_AND_KEY the second key must be a secret key with CKA_DERIVE set, the derived key is sensitive if either input is and non-extractable if either input is, and CKA_ALWAYS_SENSITIVE / CKA_NEVER_EXTRACTABLE need both inputs to have them. A CKM_CONCATENATE_DATA_AND_BASE result truncated to the caller's data holds no key bytes, so it gets neither flag. CKA_WRAP_WITH_TRUSTED is set when either input has it, so a copy cannot be wrapped with an untrusted key. A token key whose value is not decoded, before login or after logout, gives CKR_USER_NOT_LOGGED_IN instead of being concatenated as empty or zeroed bytes. An AES key wrapped by the STM32U5 DHUK gives CKR_KEY_FUNCTION_NOT_PERMITTED, as its stored value is not the key. The concatenation output contains the input key bytes, so these three mechanisms inherit CKA_SENSITIVE and CKA_EXTRACTABLE even in builds with WOLFPKCS11_LEGACY_DERIVE_NO_INHERIT (enabled for NSS); otherwise concatenating a sensitive key with empty data would give a readable copy. Under WP11_NSS_PERMISSIVE_KEY_DEFAULTS the second key skips the CKA_DERIVE check, as the base key does. Related derive changes: - Drop WP11_DERIVE_SECRET_KEY. Concatenation needs no crypto, so the secret-key derive path is always built. - For the other mechanisms, an AES key derived without CKA_VALUE_LEN takes the whole derived length. When that is not 16, 24 or 32 bytes, return CKR_TEMPLATE_INCOMPLETE before creating the object instead of failing in the key setter with CKR_FUNCTION_FAILED. A template that has CKA_VALUE_LEN keeps the error object creation returns for it. - Use a WP11_MAX_SYM_KEY_SZ of 256 bytes in ML-KEM builds without DH. The ECC-only value of 66 bytes could not hold a 96-byte PSK || ECDH || ML-KEM input. Fixes wolfSSL#264.
ea18833 to
f5370f6
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #279
Scan targets checked: wolfpkcs11-src, wolfpkcs11-bugs
Coverage: 1 of 2 in-scope changed file(s) opened by the reviewer; not opened: tests/pkcs11test.c
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.
|
Thanks for this, the concatenation support and the protection handling around it look good. My concern is how it lands on embedded and size-constrained builds. Concatenation has no build option Every other derive mechanism can be compiled out: Size is only part of it. Concatenation mechanisms have a history of key extraction attacks on PKCS#11 tokens (combine a sensitive key with known data, truncate, recover it a byte at a time). This PR guards against that with Secret key objects get larger in ML-KEM builds The new Suggestions
|
Implement CKM_CONCATENATE_BASE_AND_KEY, CKM_CONCATENATE_BASE_AND_DATA and CKM_CONCATENATE_DATA_AND_BASE, so a hybrid key agreement such as ETSI TS 103 744 can build PSK || Z_ECDH || Z_MLKEM as a sensitive key and feed it to HKDF without exporting the parts. Fixes #264.
Changes
Testing