Add support for LMS and XMSS#352
Conversation
e01c4e8 to
322c2ba
Compare
d524cee to
c9dad02
Compare
|
wolfSSL/wolfssl#10488 is required for CI to pass |
d104594 to
2c5db59
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #352
Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #352
Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #352
Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
|
|
||
| /* Restore an XmssKey from a byte sequence. The caller must pass a key that | ||
| * has been wc_XmssKey_Init'd. The function calls wc_XmssKey_SetParamStr | ||
| * (which allocates key->sk) and copies pub and sk from the blob. */ |
There was a problem hiding this comment.
🔵 [Low] XmssDeserializeKey header doc contradicts implementation · Incorrect error handling
The header comment claims wc_XmssKey_SetParamStr allocates key->sk and that the function copies pub and sk from the blob, but the implementation in src/wh_crypto.c:651 only copies pk and notes that sk is loaded later via Reload/bridge ReadCb. Callers following the documented contract will get an unpopulated sk.
Fix: Update the header doc to state that only pk is restored and that sk must be populated by wc_XmssKey_Reload against the bridge ReadCb.
| if (ret == 0) { | ||
| ret = wc_LmsKey_GetPubLen(key, &pubLen32); | ||
| } | ||
| if (ret == 0 && req.pub.sz < pubLen32) { |
There was a problem hiding this comment.
🟠 [Medium] LMS/XMSS keygen DMA commits key to NVM before validating client pub buffer size, enabling orphan-key NVM exhaustion · NV storage vulnerabilities
_HandleLmsKeyGenDma (and _HandleXmssKeyGenDma at line 6560) calls wh_Server_KeystoreCommitKey and consumes a fresh unique keyId before checking req.pub.sz < pubLen32. On undersized pub buffer the response is suppressed (only the rc header is sent), so the client never learns the keyId of the now-persisted, unreachable key, letting an authenticated client exhaust NVM via repeated failed-but-committed keygens.
Fix: Validate req.pub.sz >= pubLen32 (and resolve the DMA pub address) before calling wh_Server_LmsKeyCacheImport/KeystoreCommitKey in both _HandleLmsKeyGenDma and _HandleXmssKeyGenDma.
Requires wolfSSL/wolfssl#10380 to be merged first (done).
Adds support for "stateful" PQC using crypto callbacks added to wolfssl.