Merge PR #3716: fix(sign): embed the issuers of macOS Keychain identities

crates/sign/src/keychain.rs

@@ -5,9 +5,12 @@
 use std::path::Path;
 use std::sync::Arc;
 
+use security_framework::certificate::SecCertificate;
 use security_framework::item::{ItemClass, ItemSearchOptions, Limit, Reference, SearchResult};
 use security_framework::key::{Algorithm, SecKey};
 use security_framework::os::macos::keychain::SecKeychain;
+use security_framework::policy::SecPolicy;
+use security_framework::trust::SecTrust;
 
 use crate::keys::{DigestAlg, ExternalKey, PrivateKey, PublicKey};
 use crate::{Certificate, DigitalId, SignError};
@@ -37,15 +40,36 @@ pub fn reference(c: &Certificate) -> String {
     format!("keychain:{}", d.iter().map(|b| format!("{b:02x}")).collect::<String>())
 }
 
-/// The identity a reference (or a certificate common name) names.
+/// The identity a reference (or a certificate common name) names. Its `chain` holds the
+/// issuers macOS finds for the certificate, so a signature embeds them as it does for a file's.
 pub fn find(reference_or_name: &str) -> Result<DigitalId, SignError> {
-    identities(None)?
+    let mut id = identities(None)?
         .into_iter()
         .find(|id| {
             reference(&id.certificate) == reference_or_name
                 || id.certificate.subject.common_name() == Some(reference_or_name.trim_start_matches("keychain:"))
         })
-        .ok_or_else(|| SignError::Crypto(format!("no Keychain identity {reference_or_name}")))
+        .ok_or_else(|| SignError::Crypto(format!("no Keychain identity {reference_or_name}")))?;
+    id.chain = issuers(&id.certificate);
+    Ok(id)
+}
+
+/// The certificates macOS chains `certificate` to through the user's keychains (and the
+/// system roots), without `certificate` itself. Best effort: an identity signs without them
+/// too. Nothing is fetched from the network, so signing stays offline.
+fn issuers(certificate: &Certificate) -> Vec<Certificate> {
+    let Ok(leaf) = SecCertificate::from_der(&certificate.raw) else { return Vec::new() };
+    let Ok(mut trust) = SecTrust::create_with_certificates(&[leaf], &[SecPolicy::create_x509()]) else { return Vec::new() };
+    if trust.set_network_fetch_allowed(false).is_err() {
+        return Vec::new();
+    }
+    // The chain is built even when it doesn't end at a trusted root (a private CA, an expired
+    // certificate); whether the signer is trusted is decided when the signature is validated.
+    let _ = trust.evaluate_with_error();
+    // `SecTrust::chain` needs macOS 12; PdfCraft runs on macOS 11 (packaging/macos/package.sh).
+    #[allow(deprecated)]
+    let chain: Vec<SecCertificate> = (0..trust.certificate_count()).filter_map(|i| trust.certificate_at_index(i)).collect();
+    chain.iter().map(SecCertificate::to_der).filter(|der| *der != certificate.raw).filter_map(|der| Certificate::parse(&der).ok()).collect()
 }
 
 /// The signing identities in the user's keychains (or only in the keychain file `keychain`).

crates/sign/tests/data/README.md

@@ -11,6 +11,7 @@ The password of every `.p12` is `test`.
 | `ec-legacy.p12` | `ec-p256.p12` re-exported with `-legacy` (3DES, SHA-1 MAC): the format macOS `security import` accepts |
 | `ec-p384.p12` | P-384 self-signed "Test Signer P384"; AES-128-CBC, SHA-1 MAC |
 | `chain.p12` | RSA 2048 "Ada Lovelace" issued by a P-256 test root, chain included |
+| `chain-legacy.p12` | `chain.p12` re-exported with `-legacy`, for the Keychain tests |
 | `rsa.crt.pem`, `ca.crt.pem` | the RSA certificate and the test root alone |
 
 Regenerate with:
@@ -29,6 +30,7 @@ openssl req -newkey rsa:2048 -nodes -keyout leaf.key -out leaf.csr -subj "/CN=Ad
 printf "basicConstraints=CA:FALSE\nkeyUsage=critical,digitalSignature,nonRepudiation\n" > leaf.ext
 openssl x509 -req -in leaf.csr -CA ca.crt -CAkey ca.key -set_serial 77 -days 3650 -extfile leaf.ext -out leaf.crt
 openssl pkcs12 -export -inkey leaf.key -in leaf.crt -certfile ca.crt -out chain.p12 -passout pass:test -name "Ada Lovelace"
+openssl pkcs12 -export -legacy -inkey leaf.key -in leaf.crt -certfile ca.crt -out chain-legacy.p12 -passout pass:test -name "Ada Lovelace"
 ```
 
 `openssl-signed.pdf` is a hand-written one-page PDF with a `/Contents` placeholder, signed with

crates/sign/tests/pdf.rs

@@ -667,6 +667,49 @@ fn signing_with_keychain_identities() {
     result.unwrap();
 }
 
+/// A Keychain identity carries the certificates macOS chains it to, as a `.p12`'s does, so the
+/// signature embeds its issuers. Ignored by default: the throwaway keychain joins the user's
+/// keychain search list while the test runs (restored afterwards).
+#[cfg(target_os = "macos")]
+#[test]
+#[ignore = "creates a temporary macOS keychain; run with --ignored"]
+fn keychain_identities_carry_their_issuers() {
+    use std::process::Command;
+    let dir = std::env::temp_dir().join(format!("pdfcraft-keychain-chain-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let kc = dir.join("test.keychain-db");
+    let sec = |args: &[&str]| Command::new("security").args(args).output().unwrap();
+    let list = String::from_utf8(sec(&["list-keychains", "-d", "user"]).stdout).unwrap();
+    let saved: Vec<String> = list.lines().map(|l| l.trim().trim_matches('"').to_string()).filter(|l| !l.is_empty()).collect();
+    let k = kc.to_str().unwrap();
+    assert!(sec(&["create-keychain", "-p", "pc-test", k]).status.success());
+    let set_list = |extra: Option<&str>| {
+        let mut args = vec!["list-keychains", "-d", "user", "-s"];
+        args.extend(saved.iter().map(String::as_str));
+        args.extend(extra);
+        sec(&args);
+    };
+    // macOS finds an identity's issuers in the keychains on the search list.
+    set_list(Some(k));
+    let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        sec(&["unlock-keychain", "-p", "pc-test", k]);
+        let p = format!("{}/tests/data/chain-legacy.p12", env!("CARGO_MANIFEST_DIR"));
+        let out = sec(&["import", &p, "-k", k, "-P", "test", "-A"]);
+        assert!(out.status.success(), "chain-legacy.p12: {}", String::from_utf8_lossy(&out.stderr));
+        let listed = pdfcraft_sign::keychain::identities(Some(&kc)).unwrap();
+        assert_eq!(listed.len(), 1, "{listed:?}");
+        let id = pdfcraft_sign::keychain::find(&pdfcraft_sign::keychain::reference(&listed[0].certificate)).unwrap();
+        let issuers: Vec<_> = id.chain.iter().map(|c| c.subject.common_name()).collect();
+        assert_eq!(issuers, [Some("PrintCraft Test Root CA")]);
+        let file = pkcs12::open(&data("chain.p12"), "test").unwrap();
+        assert_eq!(id.chain.iter().map(|c| &c.raw).collect::<Vec<_>>(), file.chain.iter().map(|c| &c.raw).collect::<Vec<_>>());
+    }));
+    sec(&["delete-keychain", k]);
+    set_list(None);
+    let _ = std::fs::remove_dir_all(&dir);
+    result.unwrap();
+}
+
 /// `fixture()` plus a Form XObject (object 7) that the page draws.
 fn fixture_with_xobject() -> Vec<u8> {
     let objs: Vec<&[u8]> = vec![