diff --git a/ROADMAP.md b/ROADMAP.md index 1225b19f..707cbc4b 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -168,6 +168,8 @@ M0 → M1 → M2 → M3 → M4 must happen in order. After M4, M5–M12 can run Newest first. One line per session: the date, what moved, and the new overall percentage. +- **2026-10-09 (M9, review of #144):** Chain building checks what an issuer may do. `x509::build_chain` linked certificates by name and signature only, so an ordinary subscriber certificate embedded in a PDF, or one in the trusted list, could "issue" a certificate for any identity and chain to a trusted root. An issuer now needs basicConstraints CA:TRUE (an old v1 self-signed root without constraints still anchors), keyCertSign when it has a key usage, a pathLenConstraint that allows the CAs below it, and validity at the signing time (or the trusted timestamp/token time) when known; the signature's details say why an issuer was refused. Negative tests issue certificates in-process and cover each refusal, plus a signed PDF whose signer chains through an end-entity certificate (Unknown, not Valid). Still missing: key usage and extended key usage of the signer, name constraints and unknown critical extensions. Overall estimate unchanged (about 30–35%). + - **2026-10-09 (M14, German interface follow-up):** Follow the merged German catalog with shorter search/action labels, corrected callout/accessibility wording and a distinct certificate Subject context. Localized tool names retain their capitalization and comment composer titles go through the catalog. Search hints reserve shortcut space and Home cards bound text with full hover text; layout, context and German Preferences/search control regression tests cover these changes. Overall effort estimate remains about 30–35%. - **2026-10-09 (community, destinations):** Bookmarks and links go to their destination's position, not just its page (ISO 32000-2 §12.3.2.2): `/XYZ` puts (left, top) at the window's top-left and applies a non-zero zoom; `/Fit`, `/FitH` and `/FitV` switch to fit page, width and height with `top` or `left` at the window's edge; `/FitR` zooms to fit the rectangle, centred. Named destinations resolve the same way; crop-box origin, `/Rotate` and view rotation are accounted for; malformed or non-finite operands fall back to the top of the page. Not yet: the `/FitB*` content box (read as the page). Overall estimate unchanged (about 30–35%). diff --git a/crates/sign/README.md b/crates/sign/README.md index f77e5232..6e8e785b 100644 --- a/crates/sign/README.md +++ b/crates/sign/README.md @@ -53,6 +53,12 @@ ECDSA P-256/P-384; the store integration is tested with software-backed keys. revisions are diffed against the signed one, and the changes are classified (signing, form fill, comments, metadata, page content, document structure) under the DocMDP permissions. The verdict follows Acrobat: valid, unknown (intact but the identity isn't trusted) or invalid. +- **Chains are checked, not just linked.** `x509::build_chain` only takes an issuer whose key + verifies the certificate below it *and* that may issue: `basicConstraints` CA:TRUE (an old v1 + self-signed root without constraints counts), `keyCertSign` when it has a key usage, a + `pathLenConstraint` that allows the CAs below it, and, given the signing time, validity then. + Certificates embedded in a document are in the pool too, so an ordinary subscriber certificate + can never vouch for another one; when an issuer is refused, the signature's details say why. Oracles: poppler's `pdfsig` reports our signatures valid; OpenSSL reads our `.p12` files and verifies our CMS; `tests/data/openssl-signed.pdf` is a signature OpenSSL made, which we validate. diff --git a/crates/sign/src/pdf.rs b/crates/sign/src/pdf.rs index d10475ae..2881b7cf 100644 --- a/crates/sign/src/pdf.rs +++ b/crates/sign/src/pdf.rs @@ -21,7 +21,7 @@ use crate::cms::SignedData; use crate::der::Time; use crate::keys::DigestAlg; use crate::pkcs12::DigitalId; -use crate::x509::{Certificate, build_chain}; +use crate::x509::{Certificate, build_chain, build_chain_noted}; #[cfg(test)] #[path = "pdf_discovery_tests.rs"] @@ -536,8 +536,9 @@ fn validate_into(doc: &Document, bytes: &[u8], trust: &TrustStore, v: &Dict, inf // own claimed time, as without a timestamp. let mut pool = crate::timestamp::token_certs(raw); pool.extend(trust.certs.iter().cloned()); - let trusted_tsa = - t.signer_certificate().is_some_and(|c| c.valid_at(t.gen_time) && build_chain(&c, &pool).iter().any(|x| trust.trusts(x))); + let trusted_tsa = t + .signer_certificate() + .is_some_and(|c| c.valid_at(t.gen_time) && build_chain(&c, &pool, Some(t.gen_time)).iter().any(|x| trust.trusts(x))); if trusted_tsa { info.timestamp_time = Some(t.gen_time); } else { @@ -576,13 +577,20 @@ fn validate_into(doc: &Document, bytes: &[u8], trust: &TrustStore, v: &Dict, inf info.signing_time = s.signing_time.or_else(|| info.date.as_deref().and_then(Time::from_pdf)); let mut pool = sd.certificates.clone(); pool.extend(trust.certs.iter().cloned()); - let chain: Vec = build_chain(&cert, &pool).into_iter().cloned().collect(); + // The time the chain is judged at: trusted timestamp, else the signer's claimed time. + let at = info.timestamp_time.or(info.signing_time); + let (chain, refused) = build_chain_noted(&cert, &pool, at); + let chain: Vec = chain.into_iter().cloned().collect(); let trusted = chain.iter().any(|c| trust.trusts(c)); info.chain = chain; info.certificate = Some(cert.clone()); + if let Some(why) = refused + && !trusted + { + info.details.push(why); + } // Revocation evidence embedded in the document security store, if any. A verified // revocation is final: later blocks must not soften the verdict. - let at = info.timestamp_time.or(info.signing_time); let signer = info.chain.first().cloned(); let revoked = check_dss_revocation(doc, signer.as_ref(), &at, info); // Changes after signing. @@ -773,7 +781,8 @@ fn validate_doc_timestamp(doc: &Document, bytes: &[u8], v: &Dict, info: &mut Sig // ordinary signature from an unknown signer. let mut pool = crate::timestamp::token_certs(&contents); pool.extend(trust.certs.iter().cloned()); - let trusted_tsa = token.signer_certificate().map(|c| build_chain(&c, &pool).iter().any(|x| trust.trusts(x))).unwrap_or(false); + let trusted_tsa = + token.signer_certificate().map(|c| build_chain(&c, &pool, Some(token.gen_time)).iter().any(|x| trust.trusts(x))).unwrap_or(false); if trusted_tsa { info.status = Status::Valid; info.details.push("The timestamp token is valid and its authority is trusted.".into()); diff --git a/crates/sign/src/x509.rs b/crates/sign/src/x509.rs index ee5d00b8..1b9848a1 100644 --- a/crates/sign/src/x509.rs +++ b/crates/sign/src/x509.rs @@ -117,6 +117,11 @@ pub struct Certificate { pub public_key: PublicKey, /// Basic constraints: a CA certificate. pub is_ca: bool, + /// Whether the certificate has a basicConstraints extension at all (old v1 roots don't). + pub has_basic_constraints: bool, + /// Basic constraints `pathLenConstraint`: how many CA certificates may follow this one + /// before the end-entity certificate. + pub path_len: Option, /// Key usage bits (bit 0 = digitalSignature, 1 = nonRepudiation, 5 = keyCertSign), if present. pub key_usage: Option, pub subject_key_id: Option>, @@ -128,6 +133,8 @@ pub struct Certificate { pub ocsp_urls: Vec, /// CRL distribution point URLs (2.5.29.31). pub crl_urls: Vec, + /// The `version` field: 0 for v1, 1 for v2, 2 for v3. + pub version: u64, } /// The certificate extensions `Certificate::parse` reads, gathered tolerantly: an @@ -137,6 +144,8 @@ pub struct Certificate { #[derive(Default)] struct Extensions { is_ca: bool, + has_basic_constraints: bool, + path_len: Option, key_usage: Option, subject_key_id: Option>, authority_key_id: Option>, @@ -149,10 +158,21 @@ impl Extensions { fn read(&mut self, oid: &str, value: &Tlv<'_>) { match oid { "2.5.29.19" => { + // Present at all, even when it doesn't parse: a malformed basicConstraints must + // not make a certificate look like an old v1 root (which may issue). Unparsed, + // it stays "not a CA". + self.has_basic_constraints = true; let Ok(bc) = Tlv::parse_all(value.value) else { return }; - self.is_ca = bc.children().is_ok_and(|c| c.first().is_some_and(|b| b.tag == tag::BOOLEAN && b.value != [0])); + let fields = bc.children().unwrap_or_default(); + self.is_ca = fields.first().is_some_and(|b| b.tag == tag::BOOLEAN && b.value != [0]); + // pathLenConstraint is the INTEGER after the optional cA BOOLEAN. One that is + // present but unreadable (negative, too long) allows no CA below, not any number. + self.path_len = fields.iter().find(|t| t.tag == tag::INTEGER).map(|t| t.u64().map_or(0, |n| u32::try_from(n).unwrap_or(u32::MAX))); } "2.5.29.15" => { + // A keyUsage that is present restricts the key even when it doesn't parse: an + // unreadable one allows no usage at all, never every usage. + self.key_usage = Some(0); let Ok(bits) = Tlv::parse_all(value.value) else { return }; let Some((_, b)) = bits.value.split_first() else { return }; // Bit 0 is the most significant bit of the first byte. @@ -231,8 +251,11 @@ impl Certificate { let parts = cert.children()?; let [tbs, sig_alg, sig] = parts.as_slice() else { return Err(SignError::Malformed("Certificate".into())) }; let mut f = tbs.children()?.into_iter().peekable(); + // version [0] EXPLICIT INTEGER DEFAULT v1 (0); v3 is 2. Unreadable counts as v3, the + // stricter reading (no v1-root exception in `may_issue`). + let mut version = 0u64; if f.peek().is_some_and(|t| t.tag == tag::ctx(0)) { - f.next(); + version = f.next().and_then(|v| v.inner().ok()).and_then(|v| v.u64().ok()).unwrap_or(2); } let serial = f.next().ok_or_else(|| SignError::Malformed("serial".into()))?.expect(tag::INTEGER, "serial")?.value.to_vec(); let _inner_alg = f.next(); @@ -255,7 +278,17 @@ impl Certificate { ext.read(o.as_str(), value); } } - let Extensions { is_ca, key_usage, subject_key_id, authority_key_id, extended_key_usage, ocsp_urls, crl_urls } = ext; + let Extensions { + is_ca, + has_basic_constraints, + path_len, + key_usage, + subject_key_id, + authority_key_id, + extended_key_usage, + ocsp_urls, + crl_urls, + } = ext; Ok(Certificate { raw: raw.to_vec(), tbs: tbs.raw.to_vec(), @@ -268,12 +301,15 @@ impl Certificate { not_after: na.time()?, public_key, is_ca, + has_basic_constraints, + path_len, key_usage, subject_key_id, authority_key_id, extended_key_usage, ocsp_urls, crl_urls, + version, }) } @@ -289,6 +325,17 @@ impl Certificate { key.verify(scheme, digest, &digest.digest(&[&self.tbs]), &self.signature).unwrap_or(false) } + /// Whether this certificate may issue certificates (RFC 5280 §4.2.1.9, §4.2.1.3): it is a + /// CA by its basic constraints, and if it has a key usage, that includes `keyCertSign`. A + /// self-signed v1 or v2 certificate (an old root, which predates extensions) with no basic + /// constraints counts as a CA; a v3 certificate must say it is one. + pub fn may_issue(&self) -> bool { + /// keyUsage bit 5. + const KEY_CERT_SIGN: u16 = 1 << 5; + let old_root = self.version < 2 && !self.has_basic_constraints && self.is_self_signed(); + (self.is_ca || old_root) && self.key_usage.is_none_or(|u| u & KEY_CERT_SIGN != 0) + } + /// Valid at `t`. pub fn valid_at(&self, t: Time) -> bool { self.not_before <= t && t <= self.not_after @@ -348,21 +395,53 @@ impl Certificate { } } -/// The chain from `leaf` up through `pool`, as far as issuers can be found and their keys -/// verify the certificate below. Stops at a self-signed certificate. -pub fn build_chain<'a>(leaf: &'a Certificate, pool: &'a [Certificate]) -> Vec<&'a Certificate> { +/// The chain from `leaf` up through `pool`, as far as issuers can be found and may issue: each +/// issuer's key verifies the certificate below it, it is a CA allowed to sign certificates, its +/// `pathLenConstraint` allows the CA certificates below it, and (when `at` is given) it was valid +/// then. Stops at a self-signed certificate. Certificates embedded in a document are in `pool` +/// too, so an ordinary end-entity certificate must never be accepted as an issuer. +pub fn build_chain<'a>(leaf: &'a Certificate, pool: &'a [Certificate], at: Option