From 5fecdb9cee0fa6d3c3bbe4a7a9f81a5fbeefdfcb Mon Sep 17 00:00:00 2001
From: "Dylan M. Taylor" <dylan@dylanmtaylor.com>
Date: Tue, 27 Oct 2026 23:25:40 +0000
Subject: [PATCH] Group signing and timestamp validation inputs

---
 crates/sign/src/cms.rs          | 27 +++++++++-------
 crates/sign/src/pdf/validate.rs | 57 +++++++++++++++++----------------
 crates/sign/src/timestamp.rs    | 30 ++++++++++-------
 crates/sign/tests/it/pdf.rs     | 37 ++++++++++++++-------
 4 files changed, 89 insertions(+), 62 deletions(-)

diff --git a/crates/sign/src/cms.rs b/crates/sign/src/cms.rs
index b148ea4d1..4e232af61 100644
--- a/crates/sign/src/cms.rs
+++ b/crates/sign/src/cms.rs
@@ -376,7 +376,7 @@ pub fn sign_detached(
     alg: DigestAlg,
     content_digest: &[u8],
 ) -> Result<Vec<u8>, SignError> {
-    build(key, cert, chain, alg, DATA, None, content_digest)
+    build(SignedDataInput { key, cert, chain, alg, content_type: DATA, content: None, content_digest })
 }
 
 /// A CMS SignedData that encapsulates `content` under `content_type` (an RFC 3161 token wraps
@@ -389,20 +389,23 @@ pub fn sign_encapsulated(
     content_type: &str,
     content: &[u8],
 ) -> Result<Vec<u8>, SignError> {
-    build(key, cert, chain, alg, content_type, Some(content), &alg.digest(&[content]))
+    let content_digest = alg.digest(&[content]);
+    build(SignedDataInput { key, cert, chain, alg, content_type, content: Some(content), content_digest: &content_digest })
 }
 
-// The parameters mirror the SignedData fields being assembled.
-#[allow(clippy::too_many_arguments)]
-fn build(
-    key: &PrivateKey,
-    cert: &Certificate,
-    chain: &[Certificate],
+/// The values needed to assemble a CMS SignedData object.
+struct SignedDataInput<'a> {
+    key: &'a PrivateKey,
+    cert: &'a Certificate,
+    chain: &'a [Certificate],
     alg: DigestAlg,
-    content_type: &str,
-    content: Option<&[u8]>,
-    content_digest: &[u8],
-) -> Result<Vec<u8>, SignError> {
+    content_type: &'a str,
+    content: Option<&'a [u8]>,
+    content_digest: &'a [u8],
+}
+
+fn build(input: SignedDataInput<'_>) -> Result<Vec<u8>, SignError> {
+    let SignedDataInput { key, cert, chain, alg, content_type, content, content_digest } = input;
     let cert_hash = DigestAlg::Sha256.digest(&[&cert.raw]);
     let general_names = der::seq(&[&der::explicit(4, &cert.issuer.raw)]);
     let ess = der::seq(&[&der::seq(&[&der::seq(&[&der::octets(&cert_hash), &der::seq(&[&general_names, &der::uint(&cert.serial)])])])]);
diff --git a/crates/sign/src/pdf/validate.rs b/crates/sign/src/pdf/validate.rs
index 668798c20..08b482c4b 100644
--- a/crates/sign/src/pdf/validate.rs
+++ b/crates/sign/src/pdf/validate.rs
@@ -5,6 +5,22 @@ use super::{
     Certificate, Dict, DigestAlg, Document, HashSet, ObjRef, Object, SignError, SignatureInfo, SignedData, Time, build_chain, build_chain_noted,
 };
 
+struct SignatureValidation<'a> {
+    doc: &'a Document,
+    bytes: &'a [u8],
+    trust: &'a TrustStore,
+    info: &'a mut SignatureInfo,
+    cache: &'a DigestCache,
+}
+
+struct VerifiedSignature<'a> {
+    covered: usize,
+    certificate: Certificate,
+    embedded: &'a [Certificate],
+    cms_time: Option<Time>,
+    unverified_time: Option<Time>,
+}
+
 /// Validate one signature dictionary.
 pub fn validate(doc: &Document, bytes: &[u8], trust: &TrustStore, field: &str) -> Option<SignatureInfo> {
     list(doc, bytes, trust).into_iter().find(|s| s.field == field)
@@ -82,7 +98,7 @@ pub(super) fn validate_into(doc: &Document, bytes: &[u8], trust: &TrustStore, v:
         info.details.push(Detail::of("PdfCraft can't check this signature yet: {}.", Fill::Reason(why.to_owned())));
     };
     if info.sub_filter.as_deref() == Some("adbe.x509.rsa_sha1") {
-        return validate_x509_rsa_sha1(doc, bytes, trust, v, info, cache, (l0, o1, covered), &contents);
+        return validate_x509_rsa_sha1(SignatureValidation { doc, bytes, trust, info, cache }, v, (l0, o1, covered), &contents);
     }
     let sd = match SignedData::parse(&contents) {
         Ok(sd) => sd,
@@ -192,24 +208,17 @@ pub(super) fn validate_into(doc: &Document, bytes: &[u8], trust: &TrustStore, v:
             return invalid(info, Detail::of("The signature could not be checked ({}).", Fill::Reason(e.to_string())));
         }
     }
-    finish_validation(doc, bytes, trust, info, cache, covered, cert, &sd.certificates, s.signing_time, unverified_time);
+    finish_validation(
+        SignatureValidation { doc, bytes, trust, info, cache },
+        VerifiedSignature { covered, certificate: cert, embedded: &sd.certificates, cms_time: s.signing_time, unverified_time },
+    );
 }
 
 /// The verdict once the signature value has been checked: chain and trust, the signing time
 /// against the certificate's validity, and what later revisions changed.
-#[allow(clippy::too_many_arguments)]
-fn finish_validation(
-    doc: &Document,
-    bytes: &[u8],
-    trust: &TrustStore,
-    info: &mut SignatureInfo,
-    cache: &DigestCache,
-    covered: usize,
-    cert: Certificate,
-    embedded: &[Certificate],
-    cms_time: Option<Time>,
-    unverified_time: Option<Time>,
-) {
+fn finish_validation(context: SignatureValidation<'_>, verified: VerifiedSignature<'_>) {
+    let SignatureValidation { doc, bytes, trust, info, cache } = context;
+    let VerifiedSignature { covered, certificate: cert, embedded, cms_time, unverified_time } = verified;
     info.signing_time = cms_time.or_else(|| info.date.as_deref().and_then(Time::from_pdf));
     let mut pool = embedded.to_vec();
     pool.extend(trust.certs.iter().cloned());
@@ -590,17 +599,8 @@ pub(crate) fn signature_contents(doc: &Document) -> Vec<Vec<u8>> {
 /// `adbe.x509.rsa_sha1` (ISO 32000-2 §12.8.3.2, deprecated): `/Contents` is the PKCS #1
 /// signature as a DER OCTET STRING over the SHA-1 digest of the byte ranges, and `/Cert` holds the
 /// signer's certificate (then any others) as DER byte strings.
-#[allow(clippy::too_many_arguments)]
-fn validate_x509_rsa_sha1(
-    doc: &Document,
-    bytes: &[u8],
-    trust: &TrustStore,
-    v: &Dict,
-    info: &mut SignatureInfo,
-    cache: &DigestCache,
-    (l0, o1, covered): (usize, usize, usize),
-    contents: &[u8],
-) {
+fn validate_x509_rsa_sha1(context: SignatureValidation<'_>, v: &Dict, (l0, o1, covered): (usize, usize, usize), contents: &[u8]) {
+    let SignatureValidation { doc, bytes, trust, info, cache } = context;
     let fail = |info: &mut SignatureInfo, why: Detail| {
         info.status = Status::Invalid;
         info.details.push(why);
@@ -647,5 +647,8 @@ fn validate_x509_rsa_sha1(
     }
     info.details.push(Detail::plain("The signature uses the legacy adbe.x509.rsa_sha1 format."));
     info.details.extend(DigestAlg::Sha1.weakness().map(Detail::plain));
-    finish_validation(doc, bytes, trust, info, cache, covered, cert, &certs, None, None);
+    finish_validation(
+        SignatureValidation { doc, bytes, trust, info, cache },
+        VerifiedSignature { covered, certificate: cert, embedded: &certs, cms_time: None, unverified_time: None },
+    );
 }
diff --git a/crates/sign/src/timestamp.rs b/crates/sign/src/timestamp.rs
index 0eba6bb72..01c17221a 100644
--- a/crates/sign/src/timestamp.rs
+++ b/crates/sign/src/timestamp.rs
@@ -26,6 +26,22 @@ pub struct TimestampQuery {
     pub certificate_request: bool,
 }
 
+/// Identity and digest algorithm used by a timestamp authority to sign tokens.
+pub struct TimestampSigner<'a> {
+    pub key: &'a PrivateKey,
+    pub certificate: &'a Certificate,
+    pub chain: &'a [Certificate],
+    pub digest: DigestAlg,
+}
+
+/// Issuance details for one timestamp response.
+pub struct TimestampIssuance<'a> {
+    pub query: &'a TimestampQuery,
+    pub policy: &'a str,
+    pub generation_time: Time,
+    pub serial: u64,
+}
+
 impl TimestampQuery {
     pub fn new(digest: DigestAlg, imprint: Vec<u8>) -> Result<Self, SignError> {
         if imprint.len() != digest.digest(&[&[]]).len() {
@@ -186,17 +202,9 @@ pub fn parse_response(bytes: &[u8], query: &TimestampQuery) -> Result<TimestampT
 
 /// The TSA side: sign a `TimeStampResp` for `query` at `gen_time`. Used by deterministic test
 /// authorities and a locally configured stamping identity, never implied to be a remote TSA.
-#[allow(clippy::too_many_arguments)]
-pub fn respond(
-    key: &PrivateKey,
-    cert: &Certificate,
-    chain: &[Certificate],
-    alg: DigestAlg,
-    query: &TimestampQuery,
-    policy: &str,
-    gen_time: Time,
-    serial: u64,
-) -> Result<Vec<u8>, SignError> {
+pub fn respond(signer: TimestampSigner<'_>, issuance: TimestampIssuance<'_>) -> Result<Vec<u8>, SignError> {
+    let TimestampSigner { key, certificate: cert, chain, digest: alg } = signer;
+    let TimestampIssuance { query, policy, generation_time: gen_time, serial } = issuance;
     let mut info = vec![
         der::int(1),
         der::try_oid(policy)?,
diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs
index 2ebb27d2a..869a4bebd 100644
--- a/crates/sign/tests/it/pdf.rs
+++ b/crates/sign/tests/it/pdf.rs
@@ -607,14 +607,13 @@ impl TimestampAuthority for TestTsa {
     fn timestamp(&self, request: &[u8]) -> Result<Vec<u8>, SignError> {
         let q = pdfcraft_sign::timestamp::parse_request(request)?;
         pdfcraft_sign::timestamp::respond(
-            &self.id.key,
-            &self.id.certificate,
-            &self.id.chain,
-            pdfcraft_sign::DigestAlg::Sha256,
-            &q,
-            "1.2.3.4",
-            self.time,
-            7,
+            pdfcraft_sign::timestamp::TimestampSigner {
+                key: &self.id.key,
+                certificate: &self.id.certificate,
+                chain: &self.id.chain,
+                digest: pdfcraft_sign::DigestAlg::Sha256,
+            },
+            pdfcraft_sign::timestamp::TimestampIssuance { query: &q, policy: "1.2.3.4", generation_time: self.time, serial: 7 },
         )
     }
 }
@@ -1918,8 +1917,16 @@ fn a_timestamp_token_may_sign_with_a_different_digest_than_its_imprint() {
     let imprint = pdfcraft_sign::DigestAlg::Sha1.digest(&[b"document"]);
     let q = pdfcraft_sign::timestamp::TimestampQuery::new(pdfcraft_sign::DigestAlg::Sha1, imprint.clone()).unwrap();
     let time = Time { year: 2026, month: 10, day: 6, hour: 12, minute: 0, second: 0 };
-    let resp =
-        pdfcraft_sign::timestamp::respond(&tsa.key, &tsa.certificate, &tsa.chain, pdfcraft_sign::DigestAlg::Sha256, &q, "1.2.3.4", time, 9).unwrap();
+    let resp = pdfcraft_sign::timestamp::respond(
+        pdfcraft_sign::timestamp::TimestampSigner {
+            key: &tsa.key,
+            certificate: &tsa.certificate,
+            chain: &tsa.chain,
+            digest: pdfcraft_sign::DigestAlg::Sha256,
+        },
+        pdfcraft_sign::timestamp::TimestampIssuance { query: &q, policy: "1.2.3.4", generation_time: time, serial: 9 },
+    )
+    .unwrap();
     let token = pdfcraft_sign::timestamp::parse_response(&resp, &q).unwrap();
     assert_eq!((token.digest, token.imprint, token.gen_time), (pdfcraft_sign::DigestAlg::Sha1, imprint, time));
 }
@@ -1927,7 +1934,7 @@ fn a_timestamp_token_may_sign_with_a_different_digest_than_its_imprint() {
 /// A token answers the request that asked for it: a requested nonce or policy must come back.
 #[test]
 fn a_timestamp_response_must_carry_the_requested_nonce_and_policy() {
-    use pdfcraft_sign::timestamp::{TimestampQuery, parse_response, respond};
+    use pdfcraft_sign::timestamp::{TimestampIssuance, TimestampQuery, TimestampSigner, parse_response, respond};
     let tsa = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap();
     let alg = pdfcraft_sign::DigestAlg::Sha256;
     let time = Time { year: 2026, month: 10, day: 6, hour: 12, minute: 0, second: 0 };
@@ -1936,7 +1943,13 @@ fn a_timestamp_response_must_carry_the_requested_nonce_and_policy() {
         (q.nonce, q.policy) = (nonce, policy.map(str::to_string));
         q
     };
-    let answer = |q: &TimestampQuery| respond(&tsa.key, &tsa.certificate, &tsa.chain, alg, q, "1.2.3.4", time, 9).unwrap();
+    let answer = |q: &TimestampQuery| {
+        respond(
+            TimestampSigner { key: &tsa.key, certificate: &tsa.certificate, chain: &tsa.chain, digest: alg },
+            TimestampIssuance { query: q, policy: "1.2.3.4", generation_time: time, serial: 9 },
+        )
+        .unwrap()
+    };
     let with_nonce = answer(&query(Some(42), None));
     assert!(parse_response(&with_nonce, &query(Some(42), Some("1.2.3.4"))).is_ok());
     assert!(parse_response(&with_nonce, &query(None, None)).is_ok());