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
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.
49f1cc2 to
6673edd
Compare
|
jenkins retest this please |
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).
There was a problem hiding this comment.
I think we can remove this file. I only added it to the patch I originally sent you as a reference and easy starting point.
| 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
| * small for the digest. | ||
| * @return NOT_COMPILED_IN when the param set's hash family is disabled. | ||
| */ | ||
| int wc_LmsKey_HashMsg(const LmsKey* key, const byte* msg, word32 msgSz, |
There was a problem hiding this comment.
Should we move this method further down in the file to where the other public API methods are located? At the top of the file, only static internal functions are placed otherwise.
| if (*hashSz < needSz) | ||
| return BAD_FUNC_ARG; | ||
|
|
||
| switch (key->params->lmsType & 0xF000) { |
There was a problem hiding this comment.
Use LMS_HASH_MASK instead of the magic 0xF000 literal.
| WOLFSSL_MSG("error: XmssKey context is not set"); | ||
| ret = BAD_FUNC_ARG; | ||
| } | ||
| /* Callback context is opaque; NULL is allowed. */ |
| WC_PQC_STATEFUL_SIG_TYPE_XMSS, key, &sigsLeft); | ||
| if (cbRet == 0) { | ||
| /* Clamp to int range; callers treat 0 as "exhausted". */ | ||
| return (sigsLeft > (word32)0x7FFFFFFF) |
There was a problem hiding this comment.
Similar to LMS, we only return 1 or 0 here, not the actual amount of signatures left.
| if ((key == NULL) || (sig == NULL) || (m == NULL)) { | ||
| ret = BAD_FUNC_ARG; | ||
| } | ||
| if ((ret == 0) && (mLen <= 0)) { |
There was a problem hiding this comment.
Similar to LMS, should we allow mLen == 0 for verify? Not prohibited by the spec.
| WOLFSSL_MSG("error: LmsKey context is not set"); | ||
| ret = BAD_FUNC_ARG; | ||
| } | ||
| /* Callback context is opaque to wolfCrypt and may legitimately be NULL |
There was a problem hiding this comment.
Comment is unnecessary I think.
| 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 |
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