Fix two PKCS#7/CMS Go-interop gaps: SignedData DigestInfo verify + EnvelopedData definite [0] decrypt

[Frauschi]

This PR contains four related PKCS#7/CMS fixes. The first two are Go-interop gaps uncovered by the same SCEP enrollment flow against Go-based servers (e.g. micromdm/scep); the second was exposed only once the first let enrollment advance past the signature check to the enveloped-content decrypt step. The last two are pre-existing robustness bugs in the EnvelopedData decode path that the second fix's new negative tests surfaced: a memory leak on abandoned streaming teardown, and a NO_PKCS7_STREAM infinite loop on malformed constructed-definite content. All four touch independent code paths and can be reviewed separately.

• Fix 1 — RSA verify of SignedData with mismatched DigestInfo params (commit 4a6e7e91d)
• Fix 2 — EnvelopedData decrypt of a definite-length constructed [0] (commit dbd86d450)
• Fix 3 — decrypt-context leak on an abandoned streaming EnvelopedData decode (commit 76cfaf3e5)
• Fix 4 — NO_PKCS7_STREAM infinite loop on a malformed constructed-definite [0] (commit 453b146fa)

---

Fix 1 — RSA verify of SignedData with mismatched DigestInfo params

Summary

wc_PKCS7_VerifySignedData() returned SIG_VERIFY_E (-229) for a cryptographically valid RSA SignedData when the SignerInfo digestAlgorithm parameter encoding (NULL present vs. absent) differs from the parameter encoding of the AlgorithmIdentifier inside the PKCS#1 v1.5 DigestInfo that the RSA signature actually covers.

These two encodings are independent:

• The SignerInfo digestAlgorithm is a CMS field (RFC 5652 / RFC 5754).
• The DigestInfo AlgorithmIdentifier lives inside the signed value (RFC 8017).

wolfSSL rebuilt its comparison DigestInfo mirroring only the SignerInfo digestAlgorithm (pkcs7->hashParamsAbsent), so it never byte-matched the DigestInfo recovered from the signature, and every candidate signer was rejected.

Go's crypto/rsa.SignPKCS1v15 always signs a NULL-present DigestInfo while Go CMS/SCEP stacks (e.g. micromdm/scep) omit the NULL in the SignerInfo digestAlgorithm. That combination is legal DER and OpenSSL verifies it, but wolfSSL rejected it. It is a generic PKCS#7/CMS interop gap, not SCEP-specific.

Root cause

The RSA verify path compares the encoded signed value byte-for-byte. The comparison DigestInfo is built by wc_PKCS7_BuildSignedDataDigest(), which sets the AlgorithmIdentifier parameters from pkcs7->hashParamsAbsent — a value captured from the SignerInfo digestAlgorithm during parsing. Because that is independent of the DigestInfo encoding covered by the signature, the sizes differ (e.g. 33 vs. 35 bytes for SHA-1), XMEMCMP never runs, and no signer matches. The plain-digest fallback compares a bare hash against a full DigestInfo, so it does not rescue the case either.

The fix

• wc_PKCS7_BuildSignedDataDigest() now takes an explicit hashParamsAbsent argument instead of always reading pkcs7->hashParamsAbsent.
• The RSA verify path tries both DigestInfo encodings before failing: as-parsed → plain digest → flipped parameter encoding. The flipped rebuild runs last because it overwrites the digest buffer, keeping the plain-digest attempt valid.

This keeps wolfSSL's existing encode-and-compare approach (the same posture as the certificate verify path in asn.c) — it never parses the attacker-controlled recovered bytes, so it does not reintroduce the DigestInfo-parsing signature-forgery surface that a lenient decode would. NULL-present and NULL-absent are the only two legal encodings, so trying both fully covers the space, in both mismatch directions.

Signers that omit the NULL in the DigestInfo itself (RFC 5754-strict) continue to verify — the change only adds an attempt, it does not relax any comparison.

Tests

Adds test_wc_PKCS7_VerifySignedData_NoDigestParams (tests/api/test_pkcs7.c). The mismatch is produced entirely from wolfSSL's own signer — no externally captured message is embedded. The same content is signed twice (once NULL-absent, once NULL-present); because RSA signatures are fixed length, one signature is spliced over the other message to create the parameter mismatch without any ASN.1 re-encoding.

The test exercises the full 2×2 matrix plus a negative control:

Case	SignerInfo digestAlgorithm	Signed DigestInfo	Path
A	NULL-absent	NULL-present	no signed attributes
B	NULL-present	NULL-absent	no signed attributes
C	NULL-absent	NULL-present	signed attributes
D	NULL-present	NULL-absent	signed attributes
Neg	tampered signature	—	must still fail (!= 0)

Each mismatch case was confirmed to return SIG_VERIFY_E (-229) before the fix and 0 after, so no case is degenerate. The signed-attribute cases use deterministic attributes (contentType + messageDigest, no signingTime) so the two encodes stay byte-identical and the splice remains valid.

Testing done

Built with ./configure --enable-all --enable-debug (warnings-as-errors):

• ./wolfcrypt/test/testwolfcrypt — pass (exit 0), all PKCS7 tests pass
• ./tests/unit.test — 0 failed, 1670 passed
• New test passes with the fix and fails with the fix reverted (negative control)
• Clean -Werror build

Compatibility / risk

• Public API unchanged. Only the static helper wc_PKCS7_BuildSignedDataDigest() gained an argument; its single caller is updated.
• No behavior change for messages whose encodings already matched — the extra attempt only runs on the rare double-failure fallback.
• The external-verify path (PKCS7_SIGNEEDS_CHECK, no embedded signer cert) is unchanged and still hands both digest forms to the caller; extending dual-encoding there is out of scope for this fix.

References

• RFC 8017 (PKCS#1 v2.2) §9.2 — EMSA-PKCS1-v1_5 DigestInfo
• RFC 5754 §2 — AlgorithmIdentifier parameter conventions (absent vs. NULL)
• RFC 5652 §5.3 — CMS SignerInfo

---

Fix 2 — EnvelopedData decrypt of a definite-length constructed [0]

Summary

wc_PKCS7_DecodeEnvelopedData() failed to decrypt a valid CMS/SCEP EnvelopedData whose encryptedContent is a definite-length constructed context [0] wrapping a single definite-length OCTET STRING:

A0 82 xx xx        -- [0] (constructed, definite length)
   04 82 yy yy     -- OCTET STRING (definite length)
      <ciphertext>

This is the encoding emitted by Go's crypto/pkcs7 stacks (e.g. micromdm/scep). Streaming builds returned WC_PKCS7_WANT_READ_E; NO_PKCS7_STREAM builds spun. SCEP enrollment against those servers failed at the CertRep decrypt step.

The decoder already handled the other three encryptedContent encodings; only the constructed-definite form was unhandled:

encryptedContent encoding	path
primitive 80 … (definite)	single-shot decrypt ✓
constructed indefinite A0 80 … 00 00	fragmented OCTET-STRING loop ✓
constructed definite A0 82 … 04 82 …	unhandled ✗

Root cause

The handler for the constructed [0] tag (explicitOctet == 1) assumed the BER indefinite-length fragmented form — a run of OCTET STRINGs terminated by an EOC (00 00). The definite-length single-OCTET-STRING form has no EOC, so after decrypting the one OCTET STRING the loop kept scanning for a terminator that never comes, ran past the buffer, and AddDataToStream asked for more input (WANT_READ). With NO_PKCS7_STREAM the same mismatch spun.

The fix

Right after reading the [0] length, detect the case where the [0] definite length is filled exactly by a single inner OCTET STRING, skip that inner header, and fall into the existing primitive single-shot decrypt path (explicitOctet = 0).

• Placed before the NO_PKCS7_STREAM bounds check, so it corrects the sizes for both streaming and non-streaming builds (fixes the spin too).
• The inner length is read with NO_USER_CHECK because the full ciphertext may not be buffered yet in streaming mode; the size-equality test (inner header + inner length == [0] length) validates the structure instead.
• That equality is computed in word32 to avoid signed-overflow undefined behavior when a crafted message declares an oversized inner length. It fails safe either way (the value can never match), but the UB itself would trip UBSan / -ftrapv / fuzz builds — and this PKCS7 path is fuzz-hardened.
• Only fires for the constructed-definite single-OCTET-STRING shape. Primitive (explicitOctet already 0) and indefinite ([0] length 0) encodings are untouched → no behavior change for existing cases.

Tests

Adds test_wc_PKCS7_DecodeEnvelopedData_constructedDefiniteOctet (tests/api/test_pkcs7.c). wolfSSL's own encoder never emits this form (SetImplicit(ASN_OCTET_STRING, 0, …) produces the primitive or the indefinite form), so no encode→decode round-trip can reach the fixed branch. The test therefore transcodes a normal wolfSSL encode into the constructed-definite form, then decodes it.

Case	encryptedContent [0] shape	Asserts
Positive	single OCTET STRING (the Go form)	decrypts + round-trips
Negative	two OCTET STRINGs (fragmented)	decode fails
Negative	[0] longer than the inner OCTET STRING	decode fails
Negative	non-OCTET STRING inner	decode fails

The three negatives pin the unwrap guard's decline conditions. Verified by mutation:

• removing the whole unwrap → the positive case fails with WC_PKCS7_WANT_READ_E (-270) (the reported symptom);
• removing the innerTag == OCTET STRING check → the non-OS case wrongly decrypts;
• removing the size-equality check → the "[0] longer than inner" case wrongly decrypts (the two-OCTET-STRING case does not catch this, which is why the dedicated case exists).

Testing done

• ./wolfcrypt/test/testwolfcrypt — PKCS7 signed/enveloped/encrypted/authenveloped all pass
• ./tests/unit.test pkcs7, pkcs7_sd, pkcs7_ed, pkcs7_sed, pkcs7_cd groups — 0 failed
• New test passes with the fix and fails with the fix reverted
• Clean -Werror build

Compatibility / risk

• Public API unchanged. The change is confined to wc_PKCS7_DecodeEnvelopedData().
• No behavior change for the primitive or constructed-indefinite encodings — the unwrap only fires for the previously-unhandled constructed-definite single-OCTET-STRING shape.

---

Fix 3 — decrypt-context leak on an abandoned streaming EnvelopedData decode

Summary

wc_PKCS7_Free() released cachedEncryptedContent but never released pkcs7->decryptKey — the content-cipher context (Aes/Des/Des3) allocated by wc_PKCS7_DecryptContentInit() on the streaming EnvelopedData decode path. On WC_PKCS7_WANT_READ_E the decode deliberately keeps that context to resume on the next input chunk, so a decode abandoned mid-message (a truncated or malformed streaming message the caller gives up on) and then freed leaked the cipher context.

This is a pre-existing leak, not introduced by Fix 2 — but Fix 2's new negative tests are the first to Free a wc_PKCS7 that is mid-WANT_READ, so LeakSanitizer in CI surfaced it.

Root cause

wc_PKCS7_DecodeEnvelopedData()'s own error cleanup only runs for hard errors (ret < 0 && ret != WC_PKCS7_WANT_READ_E), so it deliberately does not free the decrypt context on WANT_READ (it expects to resume). Nothing then frees it if the object is destroyed instead of resumed, because wc_PKCS7_Free() did not release decryptKey.

The fix

In wc_PKCS7_Free(), before tearing down the stream, retrieve the content cipher OID from the stream's saved var and call the existing wc_PKCS7_DecryptContentFree():

• decryptKey is only ever left allocated by the streaming EnvelopedData flow, where that saved var holds the content cipher OID.
• It is NULL — making the call a no-op — whenever no decrypt context is pending, so completed decodes and non-enveloped flows are unaffected (decryptKey is zeroed at wc_PKCS7_New() and NULLed by DecryptContentFree() after a completed decode, so there is no double-free).

Tests

No new test — the leak is exercised by Fix 2's test_wc_PKCS7_DecodeEnvelopedData_constructedDefiniteOctet negative cases (which decode a malformed message that returns WC_PKCS7_WANT_READ_E, then free the object). Those cases fail under LeakSanitizer without this fix and pass with it.

Compatibility / risk

• Public API unchanged. The change is confined to wc_PKCS7_Free() and runs only in streaming builds (!NO_PKCS7_STREAM).
• No behavior change for any successful decode — the release is a no-op unless a decrypt context was left pending by an unfinished streaming decode.

---

Fix 4 — NO_PKCS7_STREAM infinite loop on a malformed constructed-definite [0]

Summary

In a NO_PKCS7_STREAM build, wc_PKCS7_DecodeEnvelopedData() could spin forever on a malformed EnvelopedData whose encryptedContent is a definite-length constructed [0] that the single-OCTET-STRING unwrap (Fix 2) declines — e.g. a definite [0] wrapping two OCTET STRINGs, or one OCTET STRING followed by trailing bytes. This is a denial-of-service on the non-streaming decoder for attacker-supplied input.

This is a pre-existing bug, not introduced by Fix 2 — but Fix 2's new negative tests are the first to feed exactly these shapes, so it hung the NO_PKCS7_STREAM CI configs (the make check shard timed out and was cancelled).

Root cause

The fragmented-OCTET-STRING loop's error exit — if (ret != 0) break; — is compiled only under #ifndef NO_PKCS7_STREAM. The non-streaming build has no error exit; it relies solely on finding an indefinite EOC (00 00). A definite-length [0] has no EOC, so once its OCTET STRING(s) are consumed the scan runs off the end of the buffer: GetASNTag keeps failing, ret is set but the loop never breaks, idx stops advancing, and it loops indefinitely (observed: ~16 million iterations before the CI kill).

The fix

Add the equivalent non-streaming error exit — after the EOC check (so a valid indefinite message still terminates on its EOC first), break when ret is non-zero. The break is guarded by #ifdef NO_PKCS7_STREAM, so streaming behavior is unchanged.

Tests

No new test — Fix 2's test_wc_PKCS7_DecodeEnvelopedData_constructedDefiniteOctet negative cases (two OCTET STRINGs, trailing bytes, non-OCTET-STRING inner) hang a NO_PKCS7_STREAM build without this fix and return a clean error (< 0) with it. Verified locally: NO_PKCS7_STREAM build goes from a 16 M-iteration hang to all PKCS7 groups passing; --enable-all streaming build unaffected.

Compatibility / risk

• Public API unchanged; change confined to the non-streaming path of wc_PKCS7_DecodeEnvelopedData().
• The single definite-length OCTET STRING form the decoder now unwraps to the primitive path (Fix 2) never enters this loop, so it is unaffected.

[github-actions]

MemBrowse Memory Report

gcc-arm-cortex-m4-pkcs7

• FLASH: .text +256 B (+0.1%, 213,242 B / 262,144 B, total: 81% used)

linuxkm-pie

• Data: __patchable_function_entries -8 B (-0.0%, 25,552 B)

linuxkm-standard

• Data: __patchable_function_entries -8 B (-0.0%, 48,240 B)
  No memory changes detected for:
• gcc-arm-cortex-m0plus
• gcc-arm-cortex-m3
• gcc-arm-cortex-m4
• gcc-arm-cortex-m4-baremetal
• gcc-arm-cortex-m4-crypto-only
• gcc-arm-cortex-m4-dtls13
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-openssl-compat
• gcc-arm-cortex-m4-pq
• gcc-arm-cortex-m4-rsa-only
• gcc-arm-cortex-m4-sp-math
• gcc-arm-cortex-m4-tls12
• gcc-arm-cortex-m4-tls13
• gcc-arm-cortex-m7
• gcc-arm-cortex-m7-pq
• gcc-arm-cortex-m7-tls13
• stm32-sim-stm32h753

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #10928

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-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.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #10928

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #10928

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

[Frauschi]

Jenkins retest this please

[Copilot]

Pull request overview

This PR addresses four PKCS#7/CMS interoperability and robustness issues: (1) RSA SignedData verification when SignerInfo digestAlgorithm parameters (NULL present/absent) don’t match the DigestInfo’s AlgorithmIdentifier parameters, (2) EnvelopedData decryption for a definite-length constructed [0] wrapping a single definite-length OCTET STRING (Go-style encoding), plus (3) a streaming-decode decrypt-context leak on abandoned WANT_READ, and (4) a NO_PKCS7_STREAM infinite-loop fix on malformed constructed-definite [0].

Changes:

• Extend RSA SignedData verification to try both DigestInfo parameter encodings (NULL-present and NULL-absent) before failing, without parsing attacker-controlled recovered DigestInfo bytes.
• Add EnvelopedData decode support for definite-length constructed [0] containing a single definite-length OCTET STRING by unwrapping to the existing primitive path.
• Harden EnvelopedData decode teardown paths (streaming decrypt-context free on abandoned decode; non-streaming loop breaks on parse errors to avoid infinite loops) and add targeted regression tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
wolfcrypt/src/pkcs7.c	Implements dual-encoding DigestInfo verification for RSA SignedData; adds constructed-definite [0] unwrapping for EnvelopedData; fixes a streaming decrypt-context leak on free; prevents NO_PKCS7_STREAM infinite loop on malformed input.
tests/api/test_pkcs7.h	Registers new PKCS#7 regression tests for the SignedData DigestInfo-params mismatch and constructed-definite EnvelopedData [0] handling.
tests/api/test_pkcs7.c	Adds regression tests (and small DER transcode helpers) to exercise Go-style constructed-definite [0] EnvelopedData and the SignedData DigestInfo parameter mismatch matrix plus a negative control.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Frauschi]

@philljj the Copilot finding is a false-positive