Add crypto callbacks for LMS and XMSS#10380
Add crypto callbacks for LMS and XMSS#10380padelsbach wants to merge 2 commits intowolfSSL:masterfrom
Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10380
Scan targets checked: wolfcrypt-bugs, wolfcrypt-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| switch (key->params->hash) { | ||
| #ifdef WC_XMSS_SHA256 | ||
| case WC_HASH_TYPE_SHA256: | ||
| ret = wc_Hash(WC_HASH_TYPE_SHA256, msg, msgSz, hash, needSz); |
There was a problem hiding this comment.
🟠 [Medium] wc_XmssKey_HashMsg breaks for XMSS-SHA2_*_192 (SHA-256 truncated) · Logic errors
For XMSS-SHA2_*_192 variants params->n is 24 while params->hash is WC_HASH_TYPE_SHA256. wc_Hash rejects hash_len < 32 with BUFFER_E, so the helper always fails for the 192-bit profiles. The LMS counterpart hashes into a 32-byte stack buffer and copies needSz bytes; the XMSS path skips that step.
Fix: Hash into a WC_SHA256_DIGEST_SIZE stack buffer and XMEMCPY(hash, full, needSz), mirroring the LMS_SHA256_192 path.
padelsbach
force-pushed
the
lms-xmss
branch
3 times, most recently
from
49f1cc2 to
6673edd
Compare
|
|
jenkins retest this please |
Frauschi
left a comment
Frauschi
left a comment
There was a problem hiding this comment.
A lot of minor things, mainly as the WiP patch I sent you was in a very rough state tbh (sorry for that).
| struct { | ||
| /* Raw message. Backends following the PKCS#11 v3.2 | ||
| * CKM_HSS / CKM_XMSS convention of operating on a | ||
| * pre-computed digest can call wc_LmsKey_HashMsg / |
There was a problem hiding this comment.
Nit, but I'm unsure if we want to have a specific PKCS#11 reference here.
| WOLFSSL_LOCAL int wc_CryptoCb_PqcStatefulSigKeyGen(int type, void* key, | ||
| WC_RNG* rng); | ||
| /* The raw message is forwarded to the callback. Backends that follow the | ||
| * PKCS#11 v3.2 CKM_HSS / CKM_XMSS convention (digest input) can call |
There was a problem hiding this comment.
Same as above regarding the mention of PKCS#11 V3.2
| const byte* msg, int msgSz); | ||
| /* Compute the digest of a message with the hash function dictated by the | ||
| * XMSS parameter set. Useful for crypto-callback / HSM backends that follow | ||
| * the PKCS#11 v3.2 CKM_XMSS / CKM_XMSSMT convention of taking a |
There was a problem hiding this comment.
Again, I would remove/weaken the PKCS#11 reference here and be more general.
|
jenkins retest this please |
|
Ok, I have updates regarding the PKCS#11 "pre-hash" thing. It turns out that this is actually an error in their specification, which is already corrected in the current working drafts for the next version. The data passed to the sign and verify operations is indeed the whole message, not any pre-hashed digest! After taking a more in-depth look into the algorithms, a simple pre-hash as proposed in the two helper methods of this PR also does not work and breaks spec compliance. Within both algorithms, the message is hashed together with the current state of the private key, which is not available in a CryptoCb / PKCS#11 setup on the host. Hence, this would have never worked properly anyway. In total, this greatly simplifies our efforts, as the “normal” callbacks can directly be used for PKCS#11 as well. Hence, the two new helper methods for pre-hashing ( |
4 participants
Description
Add struct fields and callbacks (
MakeKey,Sign,VerifyandSigsLeft) into existing LMS and XMSS code. Addwc_LmsKey_InitIdand_InitLabelfor PKCS11 compat. Added unit tests.Testing
New unit tests
Checklist