From b941402558bce89c172bc7eb593a49c2ee9a73a2 Mon Sep 17 00:00:00 2001
From: ffishman <20211677+ffishman@users.noreply.github.com>
Date: Tue, 20 Oct 2026 16:54:43 +0000
Subject: [PATCH] M9: require eContentType and at most one certificates and
 CRLs field
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The bundle's encapContentInfo must start with its eContentType, and the
optional [0] certificates and [1] CRLs appear at most once each, in that
order (RFC 5652 §5.1; CodeRabbit on #3561).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 crates/sign/src/cms.rs      | 8 +++++++-
 crates/sign/tests/crypto.rs | 6 ++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/crates/sign/src/cms.rs b/crates/sign/src/cms.rs
index c4cf5e027..d593bce8c 100644
--- a/crates/sign/src/cms.rs
+++ b/crates/sign/src/cms.rs
@@ -89,7 +89,13 @@ pub fn certificate_bundle(bytes: &[u8]) -> Result<Vec<Certificate>, SignError> {
     // then signerInfos (RFC 5652 §5.1).
     let [version, digests, encap, middle @ .., signers] = sd.as_slice() else { return Err(bad("SignedData")) };
     let shape = [(version, tag::INTEGER), (digests, tag::SET), (encap, tag::SEQUENCE), (signers, tag::SET)];
-    if shape.iter().any(|(t, want)| t.tag != *want) || middle.iter().any(|t| t.tag != tag::ctx(0) && t.tag != tag::ctx(1)) {
+    // At most one [0] certificates and one [1] CRLs, in that order.
+    const CERTS: u8 = tag::ctx(0);
+    const CRLS: u8 = tag::ctx(1);
+    let middle_ok = matches!(middle.iter().map(|t| t.tag).collect::<Vec<_>>().as_slice(), [] | [CERTS] | [CRLS] | [CERTS, CRLS]);
+    // encapContentInfo starts with its eContentType OID.
+    let encap_ok = encap.children()?.first().is_some_and(|t| t.tag == tag::OID);
+    if shape.iter().any(|(t, want)| t.tag != *want) || !middle_ok || !encap_ok {
         return Err(bad("SignedData"));
     }
     let mut certificates = Vec::new();
diff --git a/crates/sign/tests/crypto.rs b/crates/sign/tests/crypto.rs
index 05eacfde4..c23543581 100644
--- a/crates/sign/tests/crypto.rs
+++ b/crates/sign/tests/crypto.rs
@@ -303,6 +303,12 @@ fn certificates_load_from_pkcs7_bundles() {
         assert_eq!(load_certificates(&wrap(&[&int(1), &set_of(&[]), &encap, &certs, &set_of(&[])])).unwrap().len(), 2);
         assert!(load_certificates(&wrap(&[&int(1), &set_of(&[]), &encap, &certs])).is_err());
         assert!(load_certificates(&wrap(&[&set_of(&[]), &set_of(&[]), &encap, &certs, &set_of(&[])])).is_err());
+        // An empty encapContentInfo, a repeated [0], and [1] CRLs before [0] certificates.
+        let crls = tlv(0xA1, &[]);
+        assert!(load_certificates(&wrap(&[&int(1), &set_of(&[]), &seq(&[]), &certs, &set_of(&[])])).is_err());
+        assert!(load_certificates(&wrap(&[&int(1), &set_of(&[]), &encap, &certs, &certs, &set_of(&[])])).is_err());
+        assert!(load_certificates(&wrap(&[&int(1), &set_of(&[]), &encap, &crls, &certs, &set_of(&[])])).is_err());
+        assert_eq!(load_certificates(&wrap(&[&int(1), &set_of(&[]), &encap, &certs, &crls, &set_of(&[])])).unwrap().len(), 2);
     }
     let mut trailing = bundle.clone();
     trailing.extend_from_slice(&[0x05, 0x00]);