fix(sign): one content type and one message digest in the signed attributes

RFC 5652 allows one content-type and one message-digest attribute in the signed attributes, each with one value. The parser kept the last attribute and the first value, so a CMS with two of either could mean one thing here and another in a different reader. Such a CMS is now not read. The content-type attribute must also name the type of the encapsulated content. The signature covers the attribute but not eContentType, so a CMS where the two differ is not read either.
2026-10-27 19:48:14 -06:00 · 2026-10-19 23:38:58 +03:00
parent 737f2f725f
commit 4f56e18103
3 changed files with 134 additions and 2 deletions
@@ -83,6 +83,10 @@ needs a prompt to open is listed and asks at signing (issue #179).
  attributes are still verified over their own exact bytes (signed attributes written with an
  indefinite length are re-encoded as DER first, as RFC 5652 §5.4 has them signed). Nesting is
  bounded, and the details say that the signature was BER. High tag numbers are not read.
+- **One content type, one message digest.** A CMS whose signed attributes repeat the content type
+  or the message digest, or give either of them more or fewer than one value, is not read (RFC
+  5652 §11.1, §11.2). Neither is one whose content-type attribute differs from `eContentType`:
+  the signature covers the attribute but not `eContentType`.
 - **Later changes are classified narrowly.** Only the catalog's own `/Metadata` stream is the
  document's XMP, and only objects reached through the `/DSS` keys above are the security store. A
  `/Type /Metadata` or `/Type /DSS` *label* on any other dictionary or stream (a page's Form
@@ -80,6 +80,7 @@ impl SignedData {
        let _version = it.next();
        let _digest_algs = it.next();
        let encap = it.next().ok_or_else(|| bad("encapContentInfo"))?.children()?;
+        let encap_type = encap.first().and_then(|t| t.oid().ok());
        let content = match encap.get(1) {
            Some(c) if c.tag == tag::ctx(0) => {
                // BER signers may split the content into a constructed OCTET STRING.
@@ -111,7 +112,16 @@ impl SignedData {
        }
        let infos = signer_infos.ok_or_else(|| bad("no signerInfos"))?.children()?;
        let si = infos.first().ok_or_else(|| bad("no signer"))?;
-        Ok(SignedData { certificates, signer: parse_signer(si)?, content, quirks })
+        let signer = parse_signer(si)?;
+        // RFC 5652 §11.1: the content-type attribute names the type of the encapsulated content.
+        // The signature covers the attribute but not `eContentType`, so when they differ, one of
+        // them was written wrong or changed after signing.
+        if let (Some(signed), Some(encap)) = (&signer.content_type, &encap_type)
+            && signed != encap
+        {
+            return Err(bad("the signed content type is not the encapsulated content type"));
+        }
+        Ok(SignedData { certificates, signer, content, quirks })
    }

    /// The signer's certificate among those carried.
@@ -204,10 +214,24 @@ fn parse_signer(si: &Tlv<'_>) -> Result<SignerInfo, SignError> {
        if let Some(t) = set.first_mut() {
            *t = tag::SET;
        }
+        let (mut seen_type, mut seen_digest) = (false, false);
        for a in next.children()? {
            let p = a.children()?;
            let Some(o) = p.first().and_then(|o| o.oid().ok()) else { continue };
-            let value = p.get(1).and_then(|v| v.children().ok()).and_then(|v| v.into_iter().next());
+            let values = p.get(1).and_then(|v| v.children().ok()).unwrap_or_default();
+            // RFC 5652 §11.1 and §11.2: one content-type and one message-digest attribute, each
+            // with one value. With a second one, two readers could each take a different value.
+            let (seen, what) = match o.as_str() {
+                CONTENT_TYPE => (Some(&mut seen_type), "content type"),
+                MESSAGE_DIGEST => (Some(&mut seen_digest), "message digest"),
+                _ => (None, ""),
+            };
+            if let Some(seen) = seen
+                && (std::mem::replace(seen, true) || values.len() != 1)
+            {
+                return Err(bad(&format!("the signed attributes must hold one {what} with one value")));
+            }
+            let value = values.into_iter().next();
            match (o.as_str(), value) {
                (MESSAGE_DIGEST, Some(v)) => message_digest = Some(v.value.to_vec()),
                (CONTENT_TYPE, Some(v)) => content_type = v.oid().ok(),
@@ -0,0 +1,104 @@
+//! What a CMS's signed attributes may hold. RFC 5652 allows one content-type and one
+//! message-digest attribute, each with one value, and the content type must be the type of the
+//! encapsulated content.
+//!
+//! These go through `SignedData::parse`, the way a PDF's signature reaches it. Each CMS is built
+//! here as the signer builds it, so a case differs from a valid one only where it says.
+
+use pdfcraft_sign::cms::SignedData;
+use pdfcraft_sign::der::{self, tag};
+use pdfcraft_sign::keys::DigestAlg;
+use pdfcraft_sign::{SignError, pkcs12};
+
+const SIGNED_DATA: &str = "1.2.840.113549.1.7.2";
+const DATA: &str = "1.2.840.113549.1.7.1";
+const TST_INFO: &str = "1.2.840.113549.1.9.16.1.4";
+const CONTENT_TYPE: &str = "1.2.840.113549.1.9.3";
+const MESSAGE_DIGEST: &str = "1.2.840.113549.1.9.4";
+
+fn identity() -> pkcs12::DigitalId {
+    let p12 = std::fs::read(format!("{}/tests/data/rsa-aes.p12", env!("CARGO_MANIFEST_DIR"))).unwrap();
+    pkcs12::open(&p12, "test").unwrap()
+}
+
+/// A signed attribute: its type and the SET of its values.
+fn attribute(oid: &str, values: &[&[u8]]) -> Vec<u8> {
+    der::seq(&[&der::oid(oid), &der::set_of(values)])
+}
+
+/// A SignedData whose encapsulated content has the type `encap_type` and whose one signer
+/// signed the attributes `attrs`.
+fn signed_data(id: &pkcs12::DigitalId, encap_type: &str, attrs: &[Vec<u8>]) -> Vec<u8> {
+    let alg = DigestAlg::Sha256;
+    let refs: Vec<&[u8]> = attrs.iter().map(Vec::as_slice).collect();
+    let set = der::set_of(&refs);
+    let signature = id.key.sign(alg, &set).unwrap();
+    let mut signed_attrs = set;
+    signed_attrs[0] = tag::ctx(0);
+    let cert = &id.certificate;
+    let sid = der::seq(&[&cert.issuer.raw, &der::uint(&cert.serial)]);
+    let signer = der::seq(&[&der::int(1), &sid, &alg.algorithm(), &signed_attrs, &id.key.signature_algorithm(alg), &der::octets(&signature)]);
+    let encap = der::seq(&[&der::oid(encap_type)]);
+    let signed_data =
+        der::seq(&[&der::int(1), &der::set_of(&[&alg.algorithm()]), &encap, &der::tlv(tag::ctx(0), &cert.raw), &der::set_of(&[&signer])]);
+    der::seq(&[&der::oid(SIGNED_DATA), &der::explicit(0, &signed_data)])
+}
+
+/// Why `SignedData::parse` refuses `cms`.
+fn refused(cms: &[u8]) -> String {
+    match SignedData::parse(cms) {
+        Err(SignError::Malformed(why)) => why,
+        other => panic!("expected the CMS to be refused, got {other:?}"),
+    }
+}
+
+#[test]
+fn signed_attributes_hold_one_content_type_and_one_message_digest() {
+    let id = identity();
+    let digest = DigestAlg::Sha256.digest(&[b"document"]);
+    let (data, tst, md, other_md) = (der::oid(DATA), der::oid(TST_INFO), der::octets(&digest), der::octets(&[0; 32]));
+    let content_type = attribute(CONTENT_TYPE, &[&data]);
+    let message_digest = attribute(MESSAGE_DIGEST, &[&md]);
+
+    // As a signer writes them: read, and the signature over them verifies.
+    let sd = SignedData::parse(&signed_data(&id, DATA, &[content_type.clone(), message_digest.clone()])).unwrap();
+    assert_eq!(sd.signer.content_type.as_deref(), Some(DATA));
+    assert_eq!(sd.signer.message_digest.as_deref(), Some(digest.as_slice()));
+    assert!(sd.verify_signature(&id.certificate, &digest).unwrap());
+
+    // A second attribute of either kind, with the same value or another one, and an attribute
+    // with two values or none.
+    for (attrs, what) in [
+        (vec![content_type.clone(), content_type.clone(), message_digest.clone()], "content type"),
+        (vec![content_type.clone(), attribute(CONTENT_TYPE, &[&tst]), message_digest.clone()], "content type"),
+        (vec![attribute(CONTENT_TYPE, &[&data, &tst]), message_digest.clone()], "content type"),
+        (vec![attribute(CONTENT_TYPE, &[]), message_digest.clone()], "content type"),
+        (vec![content_type.clone(), message_digest.clone(), message_digest.clone()], "message digest"),
+        (vec![content_type.clone(), message_digest.clone(), attribute(MESSAGE_DIGEST, &[&other_md])], "message digest"),
+        (vec![content_type.clone(), attribute(MESSAGE_DIGEST, &[&md, &other_md])], "message digest"),
+        (vec![content_type.clone(), attribute(MESSAGE_DIGEST, &[])], "message digest"),
+    ] {
+        let why = refused(&signed_data(&id, DATA, &attrs));
+        assert!(why.contains(&format!("one {what} with one value")), "{why}");
+    }
+}
+
+#[test]
+fn the_signed_content_type_is_the_type_of_the_encapsulated_content() {
+    let id = identity();
+    let digest = DigestAlg::Sha256.digest(&[b"document"]);
+    let message_digest = attribute(MESSAGE_DIGEST, &[&der::octets(&digest)]);
+    let signed_as = |oid: &str| attribute(CONTENT_TYPE, &[&der::oid(oid)]);
+
+    // A PDF signature names data in both places, and an RFC 3161 token names TSTInfo in both.
+    SignedData::parse(&signed_data(&id, DATA, &[signed_as(DATA), message_digest.clone()])).unwrap();
+    SignedData::parse(&signed_data(&id, TST_INFO, &[signed_as(TST_INFO), message_digest.clone()])).unwrap();
+
+    // The signature does not cover eContentType: here it was changed from data to signedData
+    // (the last byte of the OID), and the signed attributes still say data.
+    let why = refused(&signed_data(&id, SIGNED_DATA, &[signed_as(DATA), message_digest.clone()]));
+    assert!(why.contains("not the encapsulated content type"), "{why}");
+
+    // Without a content-type attribute there is nothing to compare here.
+    SignedData::parse(&signed_data(&id, SIGNED_DATA, std::slice::from_ref(&message_digest))).unwrap();
+}