Support RFC 9802 LMS and XMSS in X.509 certificate and CSR generation#10572
Support RFC 9802 LMS and XMSS in X.509 certificate and CSR generation#10572Frauschi wants to merge 2 commits into
Conversation
|
12aee55 to
13f9db6
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends wolfCrypt’s X.509 certificate and PKCS#10 CSR generation pipeline to produce RFC 9802 HSS/LMS and XMSS/XMSS^MT artifacts (matching the existing verification support), including new key-type selectors, SPKI encoders, and runtime signature sizing so large hash-based signatures are handled correctly. It also updates tests to generate certificates/CSRs in-process instead of relying on committed DER fixtures (keeping a single LMS interop anchor).
Changes:
- Add new certificate/CSR key selectors (
LMS_TYPE,XMSS_TYPE,XMSSMT_TYPE) and internal key-type routing for cert/CSR generation. - Add RFC 9802 SPKI encoders for LMS and XMSS, plus runtime signature-buffer sizing and
sigType/key-family consistency checks to prevent mismatched OIDs. - Update memory sizing constants to separate “classic” signature buffers from PQC-sized buffers; replace most committed LMS/XMSS fixtures with in-process generation + round-trip verification tests.
Reviewed changes
Copilot reviewed 16 out of 29 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| wolfssl/wolfcrypt/wc_xmss.h | Refactors XmssKey typedef to allow forward-declare and adds wc_XmssKey_PublicKeyToDer() API declaration. |
| wolfssl/wolfcrypt/wc_lms.h | Refactors LmsKey typedef to allow forward-declare and adds wc_LmsKey_PublicKeyToDer() API declaration. |
| wolfssl/wolfcrypt/types.h | Introduces MAX_ENCODED_CLASSIC_SIG_SZ vs MAX_ENCODED_SIG_SZ and adds WOLFSSL_MAX_SIG_SZ for fixed-buffer builds. |
| wolfssl/wolfcrypt/settings.h | Enables asym key import/export when LMS/XMSS signing support is compiled in (non-verify-only). |
| wolfssl/wolfcrypt/asn.h | Uses classic-tier signature-copy sizing for RSA/DSA verify paths and adds internal key enums for LMS/XMSS. |
| wolfssl/wolfcrypt/asn_public.h | Adds forward declarations for LmsKey/XmssKey and exposes new cert/CSR key selector values. |
| wolfcrypt/src/signature.c | Switches fixed RSA plain-text buffer sizing to classic-tier max when ASN isn’t available. |
| wolfcrypt/src/asn.c | Implements LMS/XMSS SPKI encoders, adds runtime signature sizing + sigType/key checks, and wires LMS/XMSS into cert/CSR generation (ASN template path). |
| wolfcrypt/src/asn_orig.c | Explicitly rejects LMS/XMSS cert/CSR generation on the non-template ASN path (template-only support). |
| tests/api/test_lms_xmss.h | Registers new RFC 9802 LMS/XMSS X.509 generation tests. |
| tests/api/test_lms_xmss.c | Reworks RFC 9802 tests to generate certs/CSRs/chains in-process and verify via existing parse/verify path; removes most fixture dependence. |
| src/pk_rsa.c | Uses classic-tier max buffer sizing for RSA DigestInfo encoding. |
| src/internal.c | Removes fixed ceiling when copying parsed signatures into X509 objects so large LMS/XMSS signatures are retained. |
| certs/xmss/include.am | Removes XMSS fixture distribution list (fixtures no longer used). |
| certs/lms/include.am | Keeps only the stock BC LMS interop anchor fixture in distribution list. |
| certs/include.am | Stops including the XMSS fixture include file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Jenkins retest this please |
dgarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: COMMENT
Findings: 5 total — 5 posted, 0 skipped
5 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] CheckSigTypeForKey now rejects the wc_InitCert default sigType for non-RSA keys —
wolfcrypt/src/asn.c:29956-29964 - [Low] NO_MALLOC sign paths pass MAX_ENCODED_CLASSIC_SIG_SZ capacity against a WOLFSSL_MAX_SIG_SZ buffer —
wolfcrypt/src/asn.c:30360-30370, 36899-36912 - [Low] XMSS test key-persistence callbacks use raw stdio instead of XFILE abstraction —
tests/api/test_lms_xmss.c:921-944 - [Low] Uninitialized derSz in rfc9802_gen_roundtrip —
tests/api/test_lms_xmss.c:766-790 - [Info] OID-occurrence count assertion assumes the HBS OID never appears inside the signature value —
tests/api/test_lms_xmss.c:660-676, 1027-1033
Review generated by Skoll
Extend wc_MakeCert_ex/wc_SignCert_ex/wc_MakeCertReq_ex to issue HSS/LMS and
XMSS/XMSS^MT certificates and PKCS#10 requests, building on the existing
RFC 9802 verification support. New LMS_TYPE/XMSS_TYPE/XMSSMT_TYPE selectors,
wc_{Lms,Xmss}Key_PublicKeyToDer SPKI encoders, runtime signature-buffer
sizing, and sigType/key consistency checks. Generation is ASN.1-template
only, matching where the verification path lives.
Tests generate self-signed roots, CSRs and a CA->ECC-leaf chain in-process
and verify them, replacing the patched Bouncy Castle fixtures (only the stock
RFC 9802-aligned LMS interop anchor is kept).
13f9db6 to
ccf5bf3
Compare
MAX_ENCODED_SIG_SZ grows to ~50KB once SLH-DSA is enabled, yet it was used to size PKCS#1/signature scratch and output buffers across the library, wasting stack and heap even for classic RSA/ECC operations. - Add MAX_ENCODED_CLASSIC_SIG_SZ for RSA/DSA/ECC DigestInfo buffers that can never hold a PQC signature. - Size the certificate/CSR signing output buffer from the signing key at runtime instead of the worst-case macro. - Add overridable WOLFSSL_MAX_SIG_SZ for the WOLFSSL_NO_MALLOC buffer. - Reject a signature type that does not match the signing key.
ccf5bf3 to
5ee3ca1
Compare
|
Jenkins retest this please |
|
The Arduino failure is unrelated and fixed in #10644. |
| #elif !defined(NO_RSA) | ||
| /* Max classic sig (RSA/DSA/ECC); separate from MAX_ENCODED_SIG_SZ so | ||
| * PKCS#1/verify buffers stay small when PQC is enabled for verify-only. */ | ||
| #if !defined(NO_RSA) |
There was a problem hiding this comment.
MEDIUM-1: MAX_ENCODED_CLASSIC_SIG_SZ ladder omits Ed25519/Ed448; SendCertificateVerify classic signature buffer undersized for Ed448
- File:
wolfssl/wolfcrypt/types.h:2380-2397; src/internal.c:35154-35159 - Function:
SendCertificateVerify / enum Max_ASN - Category: Regression
- Action: SUGGEST
- Tag: bug
- Confidence: High
Description: The diff splits the old MAX_ENCODED_SIG_SZ into a classic tier (MAX_ENCODED_CLASSIC_SIG_SZ) and a PQC-inclusive MAX_ENCODED_SIG_SZ, and changes SendCertificateVerify to allocate ssl->buffers.sig.buffer with MAX_ENCODED_CLASSIC_SIG_SZ. For ECC/EdDSA the actual signature is written directly into this buffer (EccSign/Ed25519Sign/Ed448Sign at internal.c:35296-35340), and the inline comment claims it 'comfortably holds an ECC/EdDSA signature'. But the ladder has branches only for RSA, ECC (140), and CURVE448 (114), falling through to 64 otherwise -- there is NO branch for HAVE_ED448 (114-byte signature) or HAVE_ED25519 (64). In a build with HAVE_ED448 but without RSA, ECC, and CURVE448, the value is 64, smaller than the 114-byte Ed448 signature. The change WORSENS PQC builds: previously MAX_ENCODED_SIG_SZ was 5120 (Falcon/ML-DSA) or 51200 (SLH-DSA) and safely held the 114-byte Ed448 signature; now it drops back to 64. Both reviewers agree the PQC+Ed448 combination is newly broken by this change. SEVERITY VIEWS DIFFER: the review mode rates this Medium and characterizes it as a potential heap write past the allocation (out-of-bounds write) for an Ed448-only config, since the Ed448 signature is copied into the under-sized buffer. The review-security mode rates it Low and argues it is NOT a memory-safety overflow because wc_ed448_sign_msg checks *outLen < ED448_SIG_SIZE (ed448.c:461) and returns BUFFER_E first, making it a functional/availability regression that breaks Ed448 certificate-verify signing rather than corrupting memory. The stricter (Medium) severity is retained pending confirmation of the write path. Either way the comment is inaccurate for Ed448 and the configuration regresses.
Code:
#elif defined(HAVE_ECC)
MAX_ENCODED_CLASSIC_SIG_SZ = 140,
#elif defined(HAVE_CURVE448)
MAX_ENCODED_CLASSIC_SIG_SZ = 114,
#else
MAX_ENCODED_CLASSIC_SIG_SZ = 64, /* too small for Ed448 (114) */
#endif
...
/* src/internal.c SendCertificateVerify */
ssl->buffers.sig.length = MAX_ENCODED_CLASSIC_SIG_SZ;
ssl->buffers.sig.buffer = (byte*)XMALLOC(MAX_ENCODED_CLASSIC_SIG_SZ, ssl->heap, DYNAMIC_TYPE_SIGNATURE);
...
ret = Ed448Sign(ssl, ssl->hsHashes->messages, ssl->hsHashes->length,
ssl->buffers.sig.buffer, (word32*)&ssl->buffers.sig.length, key, ...);
Recommendation: Add explicit HAVE_ED448 (114) and HAVE_ED25519 (64) tiers to the MAX_ENCODED_CLASSIC_SIG_SZ ladder so it is never below the largest enabled classic signature -- e.g. take the max of the RSA/ECC/CURVE448 base and a per-EdDSA floor (>=114 when HAVE_ED448, >=64 when HAVE_ED25519). This restores pre-diff safety for PQC+Ed448 builds, closes the latent Ed448-only undersize, and makes the buffer match the comment's stated intent. Also correct the SendCertificateVerify comment.
| return NOT_COMPILED_IN; | ||
| #endif | ||
|
|
||
| /* The callback produces the signature for keyType while the cert's |
There was a problem hiding this comment.
MEDIUM-2: New signature-type vs key checks on wc_SignCert_cb and wc_SignCRL_ex lack negative-path tests
- File:
wolfcrypt/src/asn.c:30349-30359; wolfcrypt/src/asn.c:36914-36921 - Function:
wc_SignCert_cb / wc_SignCRL_ex - Category: Test Coverage
- Action: SUGGEST
- Tag: test
- Confidence: Medium
Description: The diff adds new family/OID-consistency validation: wc_SignCert_cb now returns ALGO_ID_E when keyType==RSA_TYPE && !IsRsaSigType(sType) (and the ECC analogue), and wc_SignCRL_ex now calls CheckSigTypeForKey(...). The new tests (test_rfc9802_lms_x509_gen / test_rfc9802_xmss_x509_gen) exercise the LMS/XMSS branches of CheckSigTypeForKey via wc_SignCert_ex, but the RSA/ECC mismatch branches reached through wc_SignCert_cb and wc_SignCRL_ex are not directly covered. A regression that weakened or removed these specific checks would not be caught.
Code:
#ifndef NO_RSA
if (keyType == RSA_TYPE && !IsRsaSigType(sType))
return ALGO_ID_E;
#endif
#ifdef HAVE_ECC
if (keyType == ECC_TYPE && !IsEccSigType(sType))
return ALGO_ID_E;
#endif
Recommendation: Add a small negative test that calls wc_SignCert_cb with an RSA keyType but an ECDSA sType (and vice versa), and wc_SignCRL_ex with a mismatched sType, asserting ALGO_ID_E. This locks in the new consistency guarantee on those paths.
|
|
||
| if (dCert->signature != NULL && dCert->sigLength != 0 && | ||
| dCert->sigLength <= MAX_ENCODED_SIG_SZ) { | ||
| /* Store a copy of the signature for later retrieval. The buffer is sized |
There was a problem hiding this comment.
INFO-3: Removal of MAX_ENCODED_SIG_SZ ceiling on X.509 signature copy (reviewed; bounded and safe)
- File:
src/internal.c:14252-14263, 14599-14611 - Function:
CopyDecodedToX509 / CopyDecodedAcertToX509 - Category: Security
- Confidence: High
Description: The dCert->sigLength <= MAX_ENCODED_SIG_SZ (and acert equivalent) guard was removed so large LMS/XMSS signatures are retained. BEFORE, an oversized signature was silently dropped (buffer left NULL); AFTER, it is always copied. This is safe: the allocation XMALLOC(dCert->sigLength, ...) is bounded by the parsed cert DER already resident in memory (no amplification), the XMEMCPY length equals the allocation size (no overflow), and the only in-tree consumer, wolfSSL_X509_get_signature (src/x509.c:4399-4405), validates *bufSz < x509->sig.length before copying. No exploitable path was found. Recorded for reviewer awareness because it relaxes a long-standing input bound on attacker-supplied certificate signatures; downstream OpenSSL-compat consumers that assume a bounded x509->sig.length should be confirmed to length-check.
Code:
- if (dCert->signature != NULL && dCert->sigLength != 0 &&
- dCert->sigLength <= MAX_ENCODED_SIG_SZ) {
+ if (dCert->signature != NULL && dCert->sigLength != 0) {
x509->sig.buffer = (byte*)XMALLOC(
dCert->sigLength, x509->heap, DYNAMIC_TYPE_SIGNATURE);
Recommendation: No change required for memory safety. Optionally retain a generous sanity ceiling (e.g. derived from the maximum supported LMS/XMSS parameter set) to bound worst-case allocations from malformed certs, and audit any external consumers of x509->sig.length for fixed-size copies.
Extends wolfCrypt's certificate generation to issue HSS/LMS and XMSS/XMSS^MT certificates and PKCS#10 requests, building on the existing RFC 9802 verification support so the library can now both produce and consume stateful hash-based-signature certificates.
Changes
LMS_TYPE/XMSS_TYPE/XMSSMT_TYPEforwc_MakeCert_ex,wc_SignCert_ex, andwc_MakeCertReq_ex.wc_LmsKey_PublicKeyToDerandwc_XmssKey_PublicKeyToDerto emit RFC 9802 SubjectPublicKeyInfo.sigType/key consistency checks for the new algorithms.WOLFSSL_ASN_TEMPLATE), matching where the verification path already lives. Non-template builds reject these key types withALGO_ID_E.Tests
No change to default builds — the feature is gated behind the existing LMS/XMSS and certificate-generation options.