Dual Alg Certificate preTBS improvements#13
Open
Frauschi wants to merge 17 commits into
Open
Conversation
Improvements for the preTBS certificate generation for using hybrid dual algorithm certificates. The preTBS is now generated without the X.509 code, resulting in reduces ressource consumption. (a lot of code size, around 25kB RAM). Furthermore, the preTBS der size is now also calculated for an upper bound (certificate size minus the two signature sizes). This is much lower than the generic WC_MAX_CERT_VERIFY_SZ value, saving a few kB of additional stack). Signed-off-by: Tobias Frauenschläger <tobias.frauenschlaeger@oth-regensburg.de>
Follow-up to "Dual Alg Certificate preTBS improvements" which removed the
forced-enable of OPENSSL_EXTRA / WOLFSSL_CUSTOM_OID / HAVE_OID_*. Without
those, several dependent code paths fail to compile when only
WOLFSSL_DUAL_ALG_CERTS is enabled:
- tls13.c references ssl->peerCert which requires KEEP_PEER_CERT
- wc_GeneratePreTBS() calls SetDatesFromDcert() / SetAltNamesFromDcert()
which live under WOLFSSL_ALT_NAMES
- tests/api.c (and example code) call wc_SetCustomExtension() which
requires WOLFSSL_CUSTOM_OID + HAVE_OID_ENCODING/DECODING
Re-enable these targeted flags. OPENSSL_EXTRA stays off, which preserves
the bulk of the size reduction the original commit was after.
Correctness: - wc_GeneratePreTBS: bounds-check every fixed-size copy from DecodedCert into Cert (serial, skid, akid, crlInfo, issRaw, sbjRaw); a malformed cert with oversized fields previously overran the destination. - wc_GeneratePreTBS: skip the crlInfo / issRaw / sbjRaw XMEMCPY when the source pointer is NULL or length is 0 (XMEMCPY(dst,NULL,0) is UB). - ParseCertRelative: validate sigLength + altSigValLen < maxIdx before subtracting in unsigned arithmetic; a malformed cert could otherwise wrap to a huge XMALLOC. - GetCertKey: cast the (signed) length expression to word32 when storing into DecodedCert.rawPubKeySize; revert the unrelated DUAL_ALG_CERTS guard expansion on `pubLen` (it would warn-as-error when neither ECC nor DSA is enabled). - MakeAnyCert / MakeCertReq: tighten the no-key-passed branch added by the patch. Accept it only when the caller has supplied both a non-zero keyType *and* a pre-encoded SubjectPublicKeyInfo (pubKeyDer/pubKeyLen); otherwise return BAD_FUNC_ARG as before. The previous form silently accepted any non-zero keyType regardless of whether the encoder had anything to work with. Style / portability: - Hoist the `int i` declaration in wc_GeneratePreTBS out of the for() header so the file still builds under --enable-c89. - Replace stray // comments and dead TODO blocks with /* */ form. - Fix #endif comment typo (WOLFSSL_CCERT_EXT -> WOLFSSL_CERT_EXT) and in-comment typos (temporarly, easiers, withing, form -> from). Test gating: - tests/api.c: gate the dual-alg-cert test on WOLFSSL_CUSTOM_OID + HAVE_OID_ENCODING since it manually crafts the sapki / altSigAlg / altSigVal extensions through wc_SetCustomExtension. Settings: - Drop the WOLFSSL_CUSTOM_OID / HAVE_OID_ENCODING / HAVE_OID_DECODING force-enables from the WOLFSSL_DUAL_ALG_CERTS block; with the test gated, those flags are no longer prerequisites of the feature itself. - Document why KEEP_PEER_CERT and WOLFSSL_ALT_NAMES are still required (tls13.c reaches into ssl->peerCert for the alt public key, and wc_GeneratePreTBS depends on the SetDates/SetAltNames helpers under #ifdef WOLFSSL_ALT_NAMES; alt-name round-tripping is also necessary for any cert that carries a SubjectAltName extension).
The X9.146 CertificateVerify path in tls13.c (decodeRsaKey / decodeEccKey / decodeDilithiumKey) needed the peer's alternative public key (sapki) between the Cert message and the CertVerify message. The previous code read it from ssl->peerCert.sapkiDer, which only exists when KEEP_PEER_CERT is enabled - so the dual-alg-cert build had to force KEEP_PEER_CERT on, pulling in unrelated WOLFSSL_X509 machinery. Add a small owning copy of the peer's sapki on the WOLFSSL struct (peerSapkiDer / peerSapkiLen, gated on WOLFSSL_DUAL_ALG_CERTS), populated from args->dCert in ProcessPeerCerts immediately after the existing KEEP_PEER_CERT block, and freed in wolfSSL_ResourceFree. The decode*Key functions now read from this field instead of ssl->peerCert. Also gate test_dual_alg_ecdsa_mldsa on WOLFSSL_CUSTOM_OID + HAVE_OID_ENCODING (it crafts the sapki / altSigAlg / altSigVal extensions through wc_SetCustomExtension, which already pulls in those flags) and localize its LARGE_TEMP_SZ define so the test compiles when the surrounding test block is gated out. With this in place, drop the KEEP_PEER_CERT force-enable from the WOLFSSL_DUAL_ALG_CERTS block in settings.h. Verified with --enable-dual-alg-certs --enable-experimental: - KEEP_PEER_CERT is no longer auto-defined. - tests/unit.test reports Success for all configured tests. With dilithium and CUSTOM_OID flags additionally enabled, the end-to-end dual-alg TLS handshake tests (test_dual_alg_support, test_dual_alg_crit_ext_support, test_dual_alg_ecdsa_mldsa) all pass.
Replace the Sapki-stash approach (peerSapkiDer / peerSapkiLen on the
WOLFSSL struct, decoded lazily in DoTls13CertificateVerify via
decode{Rsa,Ecc,Dilithium,Falcon}Key) with a single dispatch in
ProcessPeerCerts that decodes the alternative public key into the
matching peer*Key slot at the same point the primary key is decoded.
X9.146 dual-algorithm certificates assume the primary and alternative
public keys use different signature algorithms - the previous code
already implicitly required this (decode{Rsa,...}Key returned
INVALID_PARAMETER if its peer*KeyPresent flag was already set by the
primary). Pre-population now makes that assumption explicit: when the
alt key's algorithm collides with the primary's, ProcessPeerCerts
returns PEER_KEY_ERROR with a clear log message instead of failing
later with a generic decode error.
Removed:
- decodeRsaKey / decodeEccKey / decodeDilithiumKey / decodeFalconKey
wrappers in src/tls13.c.
- The lazy-decode dispatch switch in DoTls13CertificateVerify; the
"swap in the alternative" else-chain that frees the non-matching
key for SIGSPEC_ALTERNATIVE keeps working unchanged because it
already operates on the (now pre-populated) peer*Key slots.
- peerSapkiDer / peerSapkiLen from struct WOLFSSL and the matching
XMALLOC/XMEMCPY in ProcessPeerCerts and XFREE in
wolfSSL_ResourceFree.
Added:
- A new switch on args->dCert->sapkiOID right after the existing
primary-key switch in ProcessPeerCerts, decoding sapkiDer into
peerRsaKey / peerEccDsaKey / peerFalconKey / peerDilithiumKey.
Coverage matches the old decode*Key wrappers (RSA, ECC, Falcon,
Dilithium / ML-DSA, including FIPS204_DRAFT levels).
The trade-off is one extra wc_*PublicKeyDecode call when the cert
carries a sapki extension but the handshake ends up purely native
(SIGSPEC_NATIVE never consults the alt key). This is intentional and
will be revisited when the CKS extension processing is reworked.
Verified with --enable-dual-alg-certs --enable-experimental
(default config) and additionally with --enable-dilithium --enable-mldsa
+ -DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING -DHAVE_OID_DECODING:
- KEEP_PEER_CERT remains undefined.
- All three dual-alg tests (test_dual_alg_support,
test_dual_alg_crit_ext_support, test_dual_alg_ecdsa_mldsa) pass.
- tests/unit.test reports 0 failures.
Refactor the inline alt-public-key dispatch added in the previous commit
into a static helper (DecodePeerAltPubKey) and tighten its semantics.
Behaviour changes:
- Unsupported sapkiOIDs are now logged and skipped instead of failing
the handshake. A purely-native (sigSpec == NATIVE) handshake against
a peer cert that carries an alt key in an algorithm we do not
implement now interoperates again; if the alt key is actually
needed, the existing validSigAlgo / "swap in alternative" path in
DoTls13CertificateVerify still fails the handshake later via the
normal absence-of-peer-key check.
- Same-algorithm collisions between the primary and alt keys remain
fatal: silently using the primary key to verify the alt signature
would otherwise be a security bug, so PEER_KEY_ERROR with a clear
"...keys collide on X slot" log is the correct outcome.
- Decode failures, collisions and key-size violations now set
args->fatal alongside ret so the surrounding ProcessPeerCerts
error-handling treats them consistently with primary-key errors.
Security:
- Alt key now goes through the same minimum-key-size policy checks the
primary dispatch applies (minRsaKeySz / minEccKeySz / minFalconKeySz
/ minDilithiumKeySz). Without this, a peer could ship a sub-policy
alt key that policy-rejected as primary but slipped through as
alternative.
Robustness:
- Mirror the primary dispatch's AllocKey/ReuseKey pattern so a peer
key slot that was allocated by a prior handshake but not currently
"present" gets re-initialised before decode, instead of decoding
into stale state.
- The Dilithium level-mapping inner switch now has an explicit
default that returns PEER_KEY_ERROR (and sets fatal) instead of
silently leaving level=0 and relying on wc_dilithium_set_level()
to error out.
- Mid-block declarations replaced with start-of-block declarations
(per-case scope), keeping --enable-c89 builds happy.
Style:
- Multi-line WOLFSSL_MSG literal-concatenations collapsed to single
lines (project convention).
Documentation:
- settings.h dual-alg-cert block now documents the
"primary alg != alt alg" assumption explicitly so users hitting
the new "...keys collide on X slot" PEER_KEY_ERROR have somewhere
to land.
- tls13.c "swap in the alternative" else-chain now carries a comment
explaining its post-pre-population semantics and the requirement
that any new algorithm added to DecodePeerAltPubKey be mirrored
into the swap chain.
Verified with --enable-dual-alg-certs --enable-experimental
(default) and additionally with --enable-dilithium --enable-mldsa
+ -DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING -DHAVE_OID_DECODING:
- Both configs build clean under -Werror.
- tests/unit.test reports 0 failures (1419 tests).
- test_dual_alg_support, test_dual_alg_crit_ext_support and
test_dual_alg_ecdsa_mldsa all pass under the dilithium config.
…cision
The previous wc_GeneratePreTBS reconstructed the preTBS by copying
DecodedCert metadata into a transient Cert and feeding it through
MakeAnyCert. Audit showed this approach is fundamentally fragile for
X9.146 interop:
- Re-encoding can never guarantee byte-identical output to the
issuer's original TBSCertificate. Extension order, criticality
flags (the encoder hardcodes KU=critical, has no knob for EKU /
CertPolicies / CRLDP / CertPolicies criticality), DN canonical
encoding, integer min-encoding and many other DER details all
have to align with whatever produced the original cert.
- DecodedCert only models a fixed set of extensions; AIA, Name
Constraints, Policy Constraints/Mappings, CT SCTs, OCSP Must
Staple, Subject Information Access and any vendor extension are
silently dropped.
- Even within the modelled set, basicConstCrit, AKID
authorityCertIssuer/SerialNumber, and the ACME identifier
extension were never copied; the encoder ignored most criticality
flags anyway.
- issRaw / sbjRaw are bounded by sizeof(CertName) (~352B) so long
DNs hit BUFFER_E.
X9.146 specifies the preTBS as exactly the TBSCertificate with the
altSignatureValue extension removed - byte for byte. The new
implementation does exactly that:
- Verifier-side (cert has altSigValue):
- Locate the [3] EXPLICIT wrapper using DecodedCert.extensions /
extensionsIdx / extensionsSz.
- Walk Extension items, find the one whose extnValue OCTET
STRING is dCert->altSigValDer (pointer overlap into source).
- Recompute the inner Extensions SEQUENCE / [3] EXPLICIT /
outer TBS SEQUENCE lengths.
- Emit: new TBS header + TBS prefix verbatim + new [3] header
+ new inner SEQ header + extensions before altSigVal verbatim
+ extensions after altSigVal verbatim.
- Issuer-side fast path (cert has no altSigValue yet, used while
computing the alt signature input): copy the parsed TBS verbatim.
Embedded-friendly: zero heap allocations, ~60 bytes of stack, single
forward parse of the extensions list, three XMEMCPY calls into the
caller's buffer. Function no longer depends on WOLFSSL_CERT_GEN /
WOLFSSL_CERT_EXT / WOLFSSL_ALT_NAMES / SetDatesFromDcert /
SetAltNamesFromDcert / MakeAnyCert / MakeCertReq.
Removed scaffolding (only existed to feed the old reconstruction
path; pure dead-weight under the new approach):
- DecodedCert.rawPublicKey / rawPubKeySize
- Cert.pubKeyDer / pubKeyLen
- GetCertKey raw-SPKI capture
- MakeAnyCert / MakeCertReq "pre-encoded SPKI" fast paths
- MakeAnyCert / MakeCertReq keyType-only validation relaxation
- SetAltNamesFromDcert altEmailNames merge + criticality propagation
(was working around DecodedCert -> Cert metadata loss; not needed
when we don't go through Cert)
- AddDNSEntryToList ordering change (was workaround for SAN order
in re-encoded preTBS)
- WOLFSSL_ALT_NAMES force-enable in settings.h DUAL_ALG_CERTS block
(was only required by the SetAltNamesFromDcert call)
CSR preTBS support deliberately left out: CSR extensions sit inside
the attributes field, not under a [3] EXPLICIT wrapper, so excision
needs a different walker. wc_GeneratePreTBS now returns
NOT_COMPILED_IN for CSRs; the existing dual-alg-cert verify flow does
not exercise CSRs.
Verified with --enable-dual-alg-certs --enable-experimental
(default) and additionally --enable-dilithium --enable-mldsa
+ -DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING -DHAVE_OID_DECODING:
- Both configs build clean under -Werror.
- tests/unit.test reports 0 failures.
- test_dual_alg_support, test_dual_alg_crit_ext_support and
test_dual_alg_ecdsa_mldsa all pass under the dilithium config -
the latter exercises the issuer-side fast path (TBS verbatim)
and the test_dual_alg_support TLS handshake exercises the
verifier-side excision path against an actual peer cert.
X9.146 / ITU-T X.509:2019 Annex A mandates that the altSignatureValue
extension be the last extension in TBSCertificate.extensions. Encode
that assumption explicitly:
- Reject any cert where the altSigValue extension is followed by
further extensions, with ASN_PARSE_E and a clear log message.
Silently accepting such a cert would let a peer inject extension
content the alt signature does not cover.
- Drop the post-altSigValue verbatim copy from the emit path - it
is always empty for a conforming cert. Two XMEMCPY calls instead
of three; one fewer branch in the common path.
Verified with --enable-dual-alg-certs --enable-experimental and
additionally --enable-dilithium --enable-mldsa
+ -DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING -DHAVE_OID_DECODING:
- Both configs build clean under -Werror.
- tests/unit.test reports 0 failures.
- All three dual-alg tests still pass; test_dual_alg_ecdsa_mldsa
in particular exercises the verifier path (wc_GeneratePreTBS on
a fully-formed cert with altSigValue last).
Extend wc_GeneratePreTBS to support PKCS#10 CertificationRequests as
well as X.509 certificates. Refactored into:
- wc_GeneratePreTBS() public dispatcher (input validation +
issuer-side "no altSigValue yet"
verbatim-copy fast path).
- GenerateCertPreTBS() static helper, X.509 cert path
(unchanged logic, just relocated).
- GenerateCsrPreTBS() static helper, PKCS#10 CSR path (new).
The CSR helper walks the CRInfo wrapper chain
CRInfo SEQ -> attributes [0] IMPLICIT
-> extensionRequest Attribute SEQ
-> values SET
-> Extensions SEQ
-> Extension items
to locate the altSignatureValue extension (using the same pointer-
overlap match against dCert->altSigValDer that the cert path uses),
recomputes shrunken lengths for each enclosing wrapper bottom-up, and
emits the new CRInfo as a sequence of new wrapper headers + verbatim
copies of unchanged regions.
Other Attributes (challengePassword, contentType, ...) and extensions
preceding altSigValue pass through verbatim, so any CSR layout
conforming to X9.146 round-trips byte-for-byte.
Like the cert path, the CSR path enforces that altSigValue is the
last extension (per X9.146 / ITU-T X.509:2019 Annex A) and rejects
non-conforming inputs with ASN_PARSE_E.
Embedded budget: still no heap allocation, ~80 bytes of stack peak,
single forward parse, fixed (small) number of XMEMCPY calls into
the caller's buffer.
Verified with --enable-dual-alg-certs --enable-experimental and
--enable-dilithium --enable-mldsa
+ -DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING -DHAVE_OID_DECODING:
- Both configs build clean under -Werror.
- tests/unit.test reports 0 failures (1419 tests).
- All three existing dual-alg tests still pass; the cert path was
not regressed by the refactor.
Note: end-to-end CSR test coverage is currently absent from the test
suite (the existing dual-alg tests only exercise certificates, not
CSRs). The CSR helper has been compiled, statically reasoned about
against the parser's emitted offsets (extensions / extensionsIdx /
extensionsSz / altSigValDer), and the wrapper math has been
hand-verified, but a CSR-shaped test that builds, signs, excises,
verifies should be added as a follow-up.
… roundtrip
Add three new tests in tests/api.c, each gated identically to the
existing test_dual_alg_ecdsa_mldsa (WOLFSSL_DUAL_ALG_CERTS +
HAVE_DILITHIUM + HAVE_ECC + WOLFSSL_CUSTOM_OID + HAVE_OID_ENCODING +
non-small-stack):
test_dual_alg_pretbs_cert
Direct unit test for wc_GeneratePreTBS on an X.509 dual-alg cert.
Covers:
- NULL / zero-size argument validation (BAD_FUNC_ARG).
- Issuer-side fast path: cert built with sapki+altSigAlg only,
preTBS must equal the parsed TBS verbatim (XMEMCMP against
dCert->source[certBegin..sigIndex]).
- BUFFER_E when the output buffer is too small.
- Verifier-side excision: after embedding altSigValue and
re-emitting, preTBS must be strictly smaller than TBS and
re-encode cleanly. (Byte-comparing the resulting alt sig
directly is impossible because ML-DSA is randomised; the
end-to-end "alt sig verifies against preTBS" check is
already exercised by test_dual_alg_support.)
test_dual_alg_pretbs_altsigval_not_last
Builds a cert in which altSigValue is followed by another
extension (a trailing custom OID), violating the X9.146 /
ITU-T X.509:2019 Annex A "altSignatureValue must be the last
extension" requirement. Verifies wc_GeneratePreTBS returns
ASN_PARSE_E. The dummy altSigValue content is a properly-formed
BIT STRING (real ML-DSA signature over scratch bytes), since the
cert parser validates the BIT STRING layout before our "last
extension" check fires.
test_dual_alg_csr_roundtrip
First end-to-end CSR-shaped test for the dual-alg-cert feature.
Builds a PKCS#10 CertificationRequest carrying sapki+altSigAlg in
its extensionRequest attribute, parses it, generates the preTBS
via wc_GeneratePreTBS (issuer-side fast path: byte-equal CRInfo),
signs the preTBS with the ML-DSA alt key, embeds altSigValue,
re-emits and re-signs the CSR, then parses it again and runs
preTBS through the verifier-side path (GenerateCsrPreTBS).
Verifies the verifier-side preTBS is strictly smaller than the
original CRInfo and still leads with a SEQUENCE tag. Gated on
WOLFSSL_CERT_REQ in addition to the dual-alg flags above.
Verified with:
--enable-dual-alg-certs --enable-experimental --disable-shared
-> base build: 0 failures.
+ --enable-dilithium --enable-mldsa --enable-certreq --enable-certgen
-DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING -DHAVE_OID_DECODING
-> all six dual-alg tests pass:
test_dual_alg_support, test_dual_alg_crit_ext_support,
test_dual_alg_ecdsa_mldsa, test_dual_alg_pretbs_cert,
test_dual_alg_pretbs_altsigval_not_last,
test_dual_alg_csr_roundtrip.
Four new tests, each gated on the appropriate dual-alg flags:
test_dual_alg_pretbs_csr_altsigval_not_last
CSR analogue of test_dual_alg_pretbs_altsigval_not_last. Builds a
CSR whose extensionRequest places altSigValue before another
extension; wc_GeneratePreTBS must return ASN_PARSE_E (X9.146 /
ITU-T X.509:2019 Annex A enforcement on the CSR path).
test_dual_alg_collision_handshake
Builds a self-signed dual-alg cert with primary ECC + alt ECC and
runs a memio TLS 1.3 handshake against it. ProcessPeerCerts ->
DecodePeerAltPubKey must reject with PEER_KEY_ERROR (-342) because
both keys target the peerEccDsaKey slot. Documents the X9.146
"primary alg != alt alg" assumption with an executable test.
test_dual_alg_minkeysize_handshake
Builds a self-signed dual-alg cert with primary RSA-2048 + alt ECC
P-256 and runs a memio TLS 1.3 handshake. The client raises
minEccKeySz to 384 (on both ctx and ssl, since SSL inherits at
creation time). DecodePeerAltPubKey must reject the alt key with
ECC_KEY_SIZE_E (-410). Confirms the minimum-key-size policy
cannot be bypassed via the alt key.
test_dual_alg_unsupported_alt_native
Builds a self-signed cert with primary ECC + alt Ed25519 (a real
Ed25519 key, real Ed25519 alt sig). Verifies the parser tolerates
the unrecognised sapki OID (parses with NO_VERIFY, all three
dual-alg ext flags set) and that wc_GeneratePreTBS still excises
the altSigValue extension correctly.
A pure TLS-handshake regression test for this case isn't
currently feasible: the cert's alt signature must verify before
ProcessPeerCerts/DecodePeerAltPubKey is reached, but the Ed25519
leg of ConfirmSignature (cert/ed25519.c) calls
wc_ed25519_import_public on cert->ca->sapkiDer - which is the
SPKI DER, not the raw 32-byte key the function expects (the ECC
and RSA cases use wc_*PublicKeyDecode which accepts SPKI DER).
That's a pre-existing wolfSSL gap independent of this PR; the
test comment documents it for follow-up.
Two shared in-memory cert builders introduced for the negative TLS
handshake tests:
do_build_dual_alg_self_signed primary ECC, configurable alt
ECC key
do_build_dual_alg_self_signed_rsa primary RSA, ECC alt
(gated on !NO_RSA + WOLFSSL_KEY_GEN)
do_build_dual_alg_self_signed_ed25519alt
primary ECC, Ed25519 alt
Each builder mirrors the issuer flow: build interim TBS, generate
preTBS, sign with alt key, embed altSigValue, re-emit and re-sign
primary - so the resulting cert exercises the entire dual-alg
generation path before being handed to a memio TLS context.
Verified with --enable-dual-alg-certs --enable-experimental
(default) and additionally with --enable-dilithium --enable-mldsa
--enable-certreq --enable-certgen --enable-keygen --enable-ed25519
+ -DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING -DHAVE_OID_DECODING:
- Both configs build clean under -Werror.
- All 10 dual-alg tests pass:
test_dual_alg_support
test_dual_alg_crit_ext_support
test_dual_alg_ecdsa_mldsa
test_dual_alg_pretbs_cert
test_dual_alg_pretbs_altsigval_not_last
test_dual_alg_csr_roundtrip
test_dual_alg_pretbs_csr_altsigval_not_last
test_dual_alg_collision_handshake
test_dual_alg_minkeysize_handshake
test_dual_alg_unsupported_alt_native
…rify
The Ed25519 and Ed448 legs of ConfirmSignature called
wc_ed{25519,448}_import_public on the supplied key blob, which expects
the *raw* public key bytes. That works for the primary-signature
verify path because StoreKey strips the SubjectPublicKeyInfo BIT
STRING wrapper into cert->publicKey, but it broke the X9.146
alternative-signature verify path: dual-alg sapki is preserved as the
full SubjectPublicKeyInfo DER (cert->ca->sapkiDer in
ParseCertRelative), so for an Ed25519/Ed448 sapki the import call
returned BAD_FUNC_ARG and the cert was rejected before
DecodePeerAltPubKey could run.
Try the raw import first (preserves the existing primary path) and
fall back to wc_Ed{25519,448}PublicKeyDecode (which strips the SPKI
wrapper) when the raw form doesn't fit. Same dual-input pattern as
the comment notes; one extra call only on the SPKI path.
This unblocks the test_dual_alg_unsupported_alt_native test, which is
now a real TLS 1.3 handshake exercising:
- issuer-side: build a self-signed cert with primary ECDSA + alt
Ed25519 (real key, real Ed25519 alt signature),
- parse-time: ConfirmSignature verifies the Ed25519 alt sig against
the cert's own Ed25519 sapki (via the new SPKI fallback),
- DecodePeerAltPubKey: hits its default "log + skip" branch for the
unrecognised Ed25519 sapki OID,
- handshake: completes successfully because sigSpec == NATIVE never
consumes the alt key.
Verified with --enable-dual-alg-certs --enable-experimental
(default) and additionally with --enable-dilithium --enable-mldsa
--enable-certreq --enable-certgen --enable-keygen --enable-ed25519
+ -DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING -DHAVE_OID_DECODING:
- Both configs build clean under -Werror.
- All 10 dual-alg tests pass.
- test_dual_alg_unsupported_alt_native is now an end-to-end TLS
handshake (was a parser-only fallback in the previous commit).
Address review of the previous Ed25519/Ed448 alt-sig fix (305172d). asn.c - wc_Ed25519PublicKeyDecode and wc_Ed448PublicKeyDecode now accept either a raw public key or a full SubjectPublicKeyInfo DER, mirroring the dual-input pattern of wc_Falcon_PublicKeyDecode and wc_Dilithium_PublicKeyDecode (both of which already do raw-then-SPKI internally). Try the raw form first; fall back to DecodeAsymKeyPublic only when the raw size doesn't fit. - ConfirmSignature ED25519k / ED448k cases now just call wc_Ed25519PublicKeyDecode / wc_Ed448PublicKeyDecode, matching the Falcon and Dilithium cases. The earlier inline raw-then-SPKI fallback (in ConfirmSignature) is gone - the dual-input handling lives in the public decoder now and is reusable by anyone who passes either form (e.g. test_dual_alg_eddsa_pubkey_decode_dual_input below). One copy of the logic instead of two. tests/api.c - Collapsed three near-duplicate dual-alg cert builders (do_build_dual_alg_self_signed, do_build_dual_alg_self_signed_rsa, do_build_dual_alg_self_signed_ed25519alt) into a single parameterised do_build_dual_alg_self_signed taking (primaryKeyType, primaryKey, primarySigType, altKeyType, altKey, altSigType, rng). Three call sites updated. ~150 lines deleted. - test_dual_alg_collision_handshake now asserts wolfSSL_get_error returns PEER_KEY_ERROR rather than just "any non-zero handshake return", so a future change failing the handshake for an unrelated reason can't silently pass. - test_dual_alg_minkeysize_handshake similarly asserts ECC_KEY_SIZE_E. - New test_dual_alg_eddsa_pubkey_decode_dual_input: focused regression for the SPKI/raw fallback in wc_Ed{25519,448}PublicKeyDecode. Builds a real Ed25519 key, exports it both as raw and as SPKI, and round-trips both forms back through the decoder. Same for Ed448 when built. Lightweight (no TLS), gated on key export/import flags only - so it covers the new behaviour even when WOLFSSL_DUAL_ALG_CERTS is off. Verified with --enable-dual-alg-certs --enable-experimental (default) and additionally with --enable-dilithium --enable-mldsa --enable-certreq --enable-certgen --enable-keygen --enable-ed25519 --enable-ed448 + -DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING -DHAVE_OID_DECODING: - Both configs build clean under -Werror. - All 11 dual-alg tests pass: test_dual_alg_support test_dual_alg_crit_ext_support test_dual_alg_ecdsa_mldsa test_dual_alg_pretbs_cert test_dual_alg_pretbs_altsigval_not_last test_dual_alg_csr_roundtrip test_dual_alg_pretbs_csr_altsigval_not_last test_dual_alg_collision_handshake test_dual_alg_minkeysize_handshake test_dual_alg_unsupported_alt_native test_dual_alg_eddsa_pubkey_decode_dual_input
X9.146 / ITU-T X.509:2019 9.8.4 defines preTBSCertificate as
TBSCertificate with both
- the signature field (the primary signature
AlgorithmIdentifier inside TBSCertificate), and
- the altSignatureValue extension
removed. The previous excision-based wc_GeneratePreTBS only handled the
altSignatureValue extension and copied the rest of the TBS verbatim,
which left the signature AlgorithmIdentifier in place. Existing tests
passed only because both the issuer-side and verifier-side flows went
through the same buggy implementation, so the alt sigs always matched
each other (but they would not match a spec-conforming peer's preTBS).
The old MakeAnyCert-based implementation handled this correctly via
'tbsCert->sigType = 0' triggering 'noOut' on the algorithm SEQUENCE -
the excision rewrite missed the equivalent step.
GenerateCertPreTBS now:
1. Walks past version (optional [0] EXPLICIT) and serialNumber.
2. Records the signature AlgorithmIdentifier SEQUENCE bounds.
3. Optionally locates altSignatureValue inside the Extensions wrapper
(verifier-side; skipped on issuer-side flows that haven't added
altSigValue yet).
4. Emits: new TBS SEQUENCE header + version+serial verbatim + (skip
signature AlgID) + issuer..SPKI..uniqueIDs verbatim + (if
altSigValue is present) reconstructed [3]/Extensions wrappers
+ extensions before altSigValue verbatim.
The CertificationRequestInfo path is unaffected: CSRs carry the
signature AlgorithmIdentifier on the outer CertificationRequest, not
inside CRInfo, so the verbatim copy on the no-altSigValue path is
still correct. The dispatcher is restructured so the issuer-side
"verbatim" fast path lives on the CSR branch only.
tests/api.c
- test_dual_alg_pretbs_cert's issuer-side assertion updated: preTBS
is now strictly smaller than the parsed TBS (sig AlgID excised) and
no longer byte-equal to source[certBegin..sigIndex]; replaced with
an outer-SEQUENCE-tag sanity check.
Verified with --enable-dual-alg-certs --enable-experimental
(default) and additionally with --enable-dilithium --enable-mldsa
--enable-certreq --enable-certgen --enable-keygen --enable-ed25519
--enable-ed448 + -DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING
-DHAVE_OID_DECODING:
- Both configs build clean under -Werror.
- All 11 dual-alg tests pass, including the end-to-end TLS handshake
tests test_dual_alg_support and test_dual_alg_unsupported_alt_native
which confirm the new spec-conforming preTBS still verifies on the
wire.
wolfSSL's dual-alg-cert support does not currently exercise Ed25519 or
Ed448 alt keys, so the additional plumbing in this PR is out of scope
and only adds review surface. Revert the four Ed-specific changes:
asn.c
- wc_Ed25519PublicKeyDecode and wc_Ed448PublicKeyDecode go back to
SPKI-only input (dropping the raw-or-SPKI dual-input pattern that
mirrored Falcon/Dilithium).
- ConfirmSignature ED25519k / ED448k cases go back to direct
wc_ed25519_import_public / wc_ed448_import_public on the supplied
raw key (the historical primary-signature behaviour).
tests/api.c
- Remove test_dual_alg_unsupported_alt_native (depended on Ed25519 alt
keys reaching DecodePeerAltPubKey, which is unreachable now).
- Remove test_dual_alg_eddsa_pubkey_decode_dual_input (covered the
reverted decoder behaviour).
- Remove the ED25519_TYPE case from do_build_dual_alg_self_signed's
alt-key SPKI export switch and update the helper's pairings comment.
- Update the registration list and the negative-tests comment block.
Other dual-alg-cert work in this PR is unaffected. After the revert:
- 9 dual-alg tests still pass under the full config
(--enable-dual-alg-certs --enable-experimental --enable-dilithium
--enable-mldsa --enable-certreq --enable-certgen --enable-keygen
+ -DWOLFSSL_CUSTOM_OID -DHAVE_OID_ENCODING -DHAVE_OID_DECODING):
test_dual_alg_support
test_dual_alg_crit_ext_support
test_dual_alg_ecdsa_mldsa
test_dual_alg_pretbs_cert
test_dual_alg_pretbs_altsigval_not_last
test_dual_alg_csr_roundtrip
test_dual_alg_pretbs_csr_altsigval_not_last
test_dual_alg_collision_handshake
test_dual_alg_minkeysize_handshake
- Base config (--enable-dual-alg-certs --enable-experimental):
clean -Werror, 0 failures.
Ed25519/Ed448 alt-sig support can be added in a follow-up PR alongside
DecodePeerAltPubKey cases for those algorithms.
…trs)
BouncyCastle's PKCS10CertificationRequestBuilder.build(signer,
altPubKey, altSigner) - the API EJBCA's HybridCertificateSystemTest
uses to issue dual-algorithm CSRs - emits sapki / altSigAlg /
altSigValue as top-level PKCS#10 attributes whose OIDs are the X.509
extension OIDs (2.5.29.72/73/74). wolfSSL's CSR parser previously
only recognised them nested inside an extensionRequest attribute (the
IETF lamps draft style; how wolfSSL's own wc_SetCustomExtension flow
emits them). Net result: wolfSSL could not parse a BC- or
EJBCA-issued dual-algorithm CSR.
Two minimal changes:
1. DecodeCertReqAttrValue (the WOLFSSL_ASN_TEMPLATE-path CSR
attribute decoder) now recognises SUBJ_ALT_PUB_KEY_INFO_OID,
ALT_SIG_ALG_OID and ALT_SIG_VAL_OID as top-level attributes,
calling the existing DecodeSubjAltPubKeyInfo / DecodeAltSigAlg /
DecodeAltSigVal directly on the SET content. Both encodings -
top-level (BC/EJBCA) and extensionRequest-nested (lamps draft) -
populate the same DecodedCert fields; downstream code is
encoding-agnostic.
2. GenerateCsrPreTBS now first scans the CRInfo's top-level
attributes for ALT_SIG_VAL_OID; if found, the altSignatureValue
is excised as a single Attribute SEQUENCE removal (BC encoding,
two XMEMCPY emit). If not found, the existing extensionRequest
-> SET -> Extensions -> Extension chain is walked and the
altSigValue Extension is excised (lamps draft encoding).
The wc_GeneratePreTBS dispatcher's "extensions != NULL" guard for
the CSR path is dropped; in the BC encoding altSigValDer is set by
the top-level attribute parser without populating dCert->extensions.
Verified end-to-end against BouncyCastle 1.80.2 (the JAR EJBCA-CE
9.x bundles; same code path EJBCA's HybridCertificateSystemTest
exercises):
Direction 1: BC -> wolfSSL
BC emits a self-signed dual-alg cert (ECDSA P-256 primary +
ML-DSA-44 alt) and a matching dual-alg PKCS#10 CSR. wolfSSL
parses both; wc_GeneratePreTBS produces the byte-exact preTBS
that lets wc_dilithium_verify_ctx_msg confirm the embedded alt
signature. Both cert and CSR pass.
Direction 2: wolfSSL CSR -> BC -> wolfSSL
wolfSSL emits a dual-alg CSR (its native lamps-draft encoding).
BC reads it, extracts the alt public key from the
extensionRequest, generates its own CA dual key pair, and issues
a dual-alg cert binding the CSR's subject keys. wolfSSL parses
the issued cert; wc_GeneratePreTBS produces a preTBS that
successfully validates against the BC CA's ML-DSA-44 alt key.
Existing self-tests continue to pass (default config and full
dilithium config: 9/9 dual-alg tests green). The integration test
tooling (BC generator, BC issuer, wolfSSL verifiers) lives outside
the wolfSSL tree at /tmp/ejbca-interop/ and is reproducible from
the bundled BouncyCastle JARs in EJBCA-CE.
Followup to a56735e. Tightens the new CSR-attribute decoder path and GenerateCsrPreTBS based on review: DecodeCertReqAttrValue (X9.146 cases) - Propagate the underlying decoder error instead of clobbering with ASN_PARSE_E. Callers can now distinguish MEMORY_E, ASN_BITSTR_E, ASN_OBJECT_ID_E, etc. from a generic parse failure. - New CheckSinglePkcs10AttrValue() helper enforces that the SET OF AttributeValue holds exactly one TLV. Rejects an over-stuffed SET that would silently feed only the first value to the decoder. - Updated function docstring to enumerate the X9.146 cases and the new ASN_OBJECT_ID_E return when an X9.146 attribute appears at both the top level and nested inside extensionRequest (already detected by VERIFY_AND_SET_OID; just wasn't documented). OidFromId / oidCsrAttrType - Register SUBJ_ALT_PUB_KEY_INFO_OID, ALT_SIG_ALG_OID and ALT_SIG_VAL_OID under oidCsrAttrType so the OID validation in GetObjectId can match them against the canonical bytes. Previously the cases worked by accident because GetObjectId silently accepts unknown OIDs (per the existing TODO at line ~7348); if that TODO is ever resolved the new path would have broken. GenerateCsrPreTBS - Renamed the encoding selector from `bcMode` (int 0/1, two semantically-distinct states) to `topLevelAttr` with a doc comment. - Added an explicit comment in the topLevelAttr branch explaining why the lamps-draft "altSigValue must be last" enforcement is *not* applied here (PKCS#10 attributes is SET OF, ordering is determined by DER's SET OF rule, removing one element leaves the SET sorted). - Added a sanity check on the lamps-draft branch: if the cert claims the lamps-draft encoding (no top-level altSigValue attribute) but the parser didn't populate dCert->extensions / extensionsIdx, fail with ASN_PARSE_E rather than walking off the end of an incomplete DecodedCert. Interop coverage unchanged but re-validated: - BC -> wolfSSL: cert + CSR alt sigs verify (BouncyCastle 1.80.2 artefacts, the JAR EJBCA-CE 9.x bundles). - wolfSSL CSR -> BC issuer -> wolfSSL: BC issues a dual-alg cert from a wolfSSL CSR; wolfSSL verifies the issued cert. - All 9 self-tests under --enable-dual-alg-certs --enable-experimental --enable-dilithium --enable-mldsa --enable-certreq --enable-certgen --enable-keygen still pass (default config: clean -Werror, 0 fail). Note: a wolfSSL self-test that exercises the BC top-level encoding in isolation still requires either a hand-crafted (~4 KB) DER fixture or a runtime BouncyCastle dependency, neither of which are appropriate for the in-tree test suite right now. The integration test tooling under /tmp/ejbca-interop/ remains the reproducer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improvements to the preTBS certificate generation used by hybrid dual-algorithm
(X9.146) certificates. The preTBS is now generated directly from the
Cert/DecodedCertstructures instead of going through the full X.509 / OpenSSL-extramachinery, which substantially reduces resource consumption (≈25 kB RAM / code
size). The preTBS DER buffer size is also computed as a tighter upper bound
(certificate size − both signature sizes) instead of the generic
WC_MAX_CERT_VERIFY_SZ, saving a few kB of stack.Commit 1 —
Dual Alg Certificate preTBS improvementsAuthor: Tobias Frauenschläger.
wolfcrypt/src/asn.cwc_GeneratePreTBS()implemented on top ofMakeAnyCert/MakeCertReq, copying metadata fromDecodedCertdirectly into atransient
Cert(small-stack-aware).MakeAnyCert/MakeCertReqlearn an optionalCert.pubKeyDer/pubKeyLenshortcut so the public key SPKI can be re-used verbatiminstead of being re-encoded from a parsed key object.
MakeAnyCert/MakeCertReqno longer hard-fail whenkeyTypeispre-set by the caller (allows caller to drive key selection).
GetCertKey()now recordsrawPublicKey/rawPubKeySizeon theDecodedCertwheneverWOLFSSL_DUAL_ALG_CERTSis enabled.SetAltNamesFromDcertnow also flattensaltEmailNamesinto theoutput and propagates the SAN-extension criticality bit.
AddDNSEntryToListnow always appends (preserving order) instead ofprepending under the non-
OPENSSL_EXTRAbuild.ParseCertRelativeallocates the preTBS buffer at the new tighterupper bound and uses the new
wc_GeneratePreTBS.src/x509.cwc_GeneratePreTBS()(the X.509-based one); thenew implementation in
wolfcrypt/src/asn.creplaces it.wolfssl/wolfcrypt/asn.hDecodedCertgainsrawPublicKey/rawPubKeySize(DUAL_ALG_CERTS).wolfssl/wolfcrypt/asn_public.hCertgainspubKeyDer/pubKeyLen(DUAL_ALG_CERTS).wolfssl/wolfcrypt/settings.hOPENSSL_EXTRA,WOLFSSL_CUSTOM_OID,HAVE_OID_ENCODING,HAVE_OID_DECODING.WOLFSSL_ASN_TEMPLATE.Commit 2 —
settings.h: re-enable feature flags required by WOLFSSL_DUAL_ALG_CERTSBuild fix-up so
--enable-dual-alg-certs --enable-experimentalbuilds cleanlyon master after Commit 1 removes the OpenSSL-extra force-enables. Re-enables
the minimal set of flags that other DUAL_ALG_CERTS code paths still depend on:
KEEP_PEER_CERT—tls13.c::decodeRsaKey/decodeEccKey/decodeDilithiumKeyreference
ssl->peerCert.WOLFSSL_ALT_NAMES—wc_GeneratePreTBS()callsSetDatesFromDcert()/SetAltNamesFromDcert(), which live inside#ifdef WOLFSSL_ALT_NAMES.WOLFSSL_CUSTOM_OID+HAVE_OID_ENCODING+HAVE_OID_DECODING—tests/api.c(and existing example code) still useswc_SetCustomExtension()to manually craft thesapki/altSigAlg/altSigValextensions for round-trip tests.OPENSSL_EXTRAstays off, preserving the bulk of the size reduction.Review notes
A few minor things worth a second look in Commit 1 (left as-authored, not
modified by the build fix-up):
wolfcrypt/src/asn.cwc_GeneratePreTBS()— the closing#endifcomment reads
WOLFSSL_CCERT_EXTbut the matching#ifdefisWOLFSSL_CERT_EXT. Cosmetic typo.wolfcrypt/src/asn.cwc_GeneratePreTBS()—crlInfoisXMEMCPY'dunconditionally; if
extCrlInfoRawSzis 0 /extCrlInfoRawis NULLit's a no-op but worth a guard for clarity.
wc_GeneratePreTBS()skipsSetDatesFromDcert(good)but the alt-names block above doesn't gate on
isCSR; CSRs don'tcarry SAN the same way, may want to confirm intent.
MAX_CERTPOL_NBcap on copying cert policies silently drops entriesbeyond the cap — pre-existing behavior, just calling out.
Test plan
./configure --enable-dual-alg-certs --enable-experimental --disable-shared && make -jsucceeds (-Werrorclean)../tests/unit.testreportsunit_test: Success for all configured tests.Generated by Claude Code