From 63b4af8ba558ef5cd5b5016b289b6c7153b6d7b2 Mon Sep 17 00:00:00 2001 From: ffishman <20211677+ffishman@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:27:52 -0300 Subject: [PATCH] fix(sign): embed the issuers of macOS Keychain identities A Keychain identity came back with an empty `chain`, so a signature made with it carried only the signer's certificate. A validator that doesn't already hold the intermediates (Acrobat, or PdfCraft on another machine) then can't build the path to the root. The Windows certificate store identities already carry the chain Windows builds. `keychain::find` now asks the Security framework for the chain (SecTrust with the basic X.509 policy, no network fetch) and keeps every certificate but the signer's. It is best effort: a certificate macOS can't chain still signs. `chain-legacy.p12` is `chain.p12` re-exported in the legacy format that `security import` accepts, for a new ignored Keychain test. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/sign/src/keychain.rs | 30 +++++++++++++++-- crates/sign/tests/data/README.md | 2 ++ crates/sign/tests/data/chain-legacy.p12 | Bin 0 -> 2878 bytes crates/sign/tests/pdf.rs | 43 ++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 crates/sign/tests/data/chain-legacy.p12 diff --git a/crates/sign/src/keychain.rs b/crates/sign/src/keychain.rs index 601f1b97b..90bb7408f 100644 --- a/crates/sign/src/keychain.rs +++ b/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::()) } -/// 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 { - 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 { + 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 = (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`). diff --git a/crates/sign/tests/data/README.md b/crates/sign/tests/data/README.md index b33f327e1..5685d0e35 100644 --- a/crates/sign/tests/data/README.md +++ b/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 diff --git a/crates/sign/tests/data/chain-legacy.p12 b/crates/sign/tests/data/chain-legacy.p12 new file mode 100644 index 0000000000000000000000000000000000000000..6a11065ca02d01319c5780448477e72b90039974 GIT binary patch literal 2878 zcmY+^cRU-476CQ7ulh0FCTc32+6=7mS8O02i6V@US>9VGBTd8{a?aZUy z+R6koe!hB}*Z_{psBg&|>pdb8*FY^baqS3fjob0Mf9&`hE`@jNblN?=9-^`kFK$Y= zoG_Bj%+Yrt3PM2frw_E5tAz$3jA#M!eS1lm@MtlXbXd;tAy7=@SX1@o5g*%H%i)Fx z#J2Zjq&)gV;kTN}4L4p%URjOr0cVx9vqg4LpKNzp21pl&-#cG#(|wNsY6F2ZNk%E; zn%PEEAl0<4>9~86v*PH(`twbJrVdf62w>}=3w*%`^70FI_npQ2H)D47{`X-%>tvP7 z@Zi`_puQIeWOUl_Skx;YRDFJsn(bw9BQQxqvWWNs<44P*N%T>QiRZF4^@~ z;fYWXUKC)Wq97>y&P+HOcvBAL97h>RMm&z}Timl9`@1 z1BXoI#;4#2%i)NGk_VzCq)CXd2et5y=DZuJbO#z$!A(SIsM`?Kv|=UnXf!%$p_-QX zTQXSuXOQ%vT5h!X!g3Zp35Z$Kse)f$G=!$x6o2G7C#Z)*1FqdD@r+Fq|Dfp*QtPBKnbYh79%&KRHicya>3eR6HK%bSgl{U zs+nuuB6hy({e!=0>gXfD&&@^pCK?7@JfTZbb#sl%^n4?sdFa^~dcN+QuF?9A(NpVr zd=UPHYZLsBjF5Ig_Ipki^6`7|vj>$NOYhrRN*9~LZxRDDw5J@|O^NF_;603w@k(P9 z(GMYfhoDPdKf!5dc=#v9dNO+;jM%cp$sr8ROxi{>mUl9l zk7b=3^(5%@>kh`q^5v{j7h`s;7E!L%x+zR!V6q~i?tlaPCIN*`e1GA z4Hwq%2-S#dZ(3??X+~si$)v1zDw}I|GED`a1H~@jF)J}q8+S`lADOZtQ%2_l_8pXjhk!e;FiZg8wDqL|90#%9f{LpU%WiO2gYFro)aMFEWkZ z4C|_bn5#AUEP7pBfOH*GKzP>;yL<=YVyBApBG2);@bCbNi|Q%V|H#DdVh-Q-=1e9* zbfzlz`u4dTr6M*roL0z-ZfuK-CiyF>%rfZk44%hcAK7!!&s!oe?h%FU2A~kX8!tc# zxPIAdyR^O!RTqFz!WHg=?qogJ-DubE-YD3tn%OM-y-N^nQP@BYUS<=_JYsPtTU%S0 zGo*=fj+v2u(i0>9u)y(R&-_nt4jF!Qm?Bh`lI-=<=!D|7G+h&@l?sb&S?EejEs&xN1KO3sfVTfn zU&_DrjgM{e&@I$q|65-W1~g*vP*~hJUD>bl%SGt#hOdlJapPqv<>6nIY;2iZWpRO= zost(!5cJ$l&{}4N0h+3*MrFC8b3YHj`7@Wa7Tlj#-r|{#?Hwx6D|P0mony5z)3FNG zRCevjy?v^r=-#Grtzlg=f@{#UKWtP!c3E`~*G$w)X-f~e;l9tG;wk7rt;2cSL=tcK zz6tnJKxZ}hb9oZ(XE`^CChj1sbJHHV2avbEuSjU-EkVRZ=DKI(G5Bj6(>>Ip(==!( zNu57DD2Y;?&xx6IC%%sKYQU8Hn&?7Tdq?eO)pFw1S`(85?hZ~nol}!kehCVnTFv$- zThYV=kCWN~0Tz@`5h9G%6^<|Th4C_P9gn$g#^t5|wr#B4_X=oZ%E{UYL9W=r_<-u+ z-V8O{utwV3duVQ_)@}=I(E$u{dYCZGqDhfhGEt7w6XIWQ7h&Z5%pB+b2+WLKVtk-Z z@qBE?P{|^Ll!hiNM3UZy6Q#9X=ZX*P{2e*&UZyyw#bhn9Xb?``-Zk6Qnyf(wC>zEP zm9#FE>-r_+eqB+QI(3@QUMO?DRGgbE2#syN*Oax#q&HtA_mStP%t+)!YTps(fZEOI zlo6W@!rsoA6qIeMcwsVKu6{%EV;oXCF|k!ivT_vGuF5f=bfdsLbn91M#r6l}xPf)v zdz-l-CJvi%;7Ci?lb(y+Cu?%v;_X%+7O%%2Pc*YzYd^!BTwm`rv~=j5oGJvh zxkRAdbQWsgO~4;l@RTy_kkwx)ijqlWSP1r(PBTVThTwn3Fj#3IS(>+B#Fy(&8hV8E zqPjedouf3v|1cP=r`o1%^NWz|KMXTwuYd%Y(V?Pm92cXXy2U;i=4*BsUGJNAK;wdK z4^N89czf`DwF3_ny`=Pcfac~gPp{8ClMncdr(0k6PAP@;xIf&pN^_N^stir@M0P0O zb716EoCT>unBisq!rL`Sgw+gmHGf*|oOVfvqiVAda>wX{NYC)G)ueKjIIQ|b8^^Fa zX*gyBO4`_p?NG^`gl8`AWHS?|Y&}TRMNYdRgaM%{? z_k?_7Mf$24?v(U@f=5kW#0(Wm?*{goG~mMYzsTT_w|4RGDv~eDG2=Ft_B2p3vXs-T zIA0@j_jyr?{RSpMFU0$tZKn7;<$U!G>QS?J(@NyT42$&2y0!9@Eqjn#SC~;#C^3a6 z_7;V!W7|zOZ_W#GR9nx?f(_Jd%U|+Lkt>FUEyB?^J53u~B>sH zV!qhfXqiO88y#6kN~07<7;shW*NgGQE{Z~gC zHfeSctJNd5FB|DeBBGM4M=<8h;!;D?0V&acj##C+csT&-09Sw$fFEE4cm#M1a07T> zO)da8sk>4FQZ!Ux2?k0EE-(PZ3aW%>**UlL`zCm;1)$yPIHO*%f|#mCmMLFW?I`=v No$K?29+CfM^JNW = 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::>(), file.chain.iter().map(|c| &c.raw).collect::>()); + })); + 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 { let objs: Vec<&[u8]> = vec![