From 456a42a896619cf04d51497335d051763882df90 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:56:00 -0400 Subject: [PATCH 1/2] Never crash: sign has no expect/unreachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PKCS #12 HMAC errors now propagate as `SignError`. Key-bag lookup uses `find_map` and chain building uses `let … else`. `der::oid` (only ever given this crate's OID literals) keeps the standard's narrow, documented `expect` exception. The crate now denies the panic lints. Co-Authored-By: Claude Opus 5.5 --- crates/sign/src/der.rs | 3 +++ crates/sign/src/lib.rs | 2 ++ crates/sign/src/pkcs12.rs | 22 ++++++++++++++-------- crates/sign/src/x509.rs | 2 +- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/crates/sign/src/der.rs b/crates/sign/src/der.rs index 4274ff6001..a2403f8ec2 100644 --- a/crates/sign/src/der.rs +++ b/crates/sign/src/der.rs @@ -313,6 +313,9 @@ pub fn explicit(n: u8, inner: &[u8]) -> Vec { tlv(tag::ctx(n), inner) } +/// The encoding of an OID constant (callers pass only literals from this crate). +// The documented never-crash exception: every caller passes a literal that always parses. +#[allow(clippy::expect_used)] pub fn oid(dotted: &str) -> Vec { let arcs: Vec = dotted.split('.').map(|a| a.parse().expect("valid OID constant")).collect(); let mut body = Vec::new(); diff --git a/crates/sign/src/lib.rs b/crates/sign/src/lib.rs index da35ffeb25..9af82ba154 100644 --- a/crates/sign/src/lib.rs +++ b/crates/sign/src/lib.rs @@ -5,6 +5,8 @@ //! - [`pdf`]: signature fields in a document — listing and validating them, and signing //! (PAdES B-B, `ETSI.CAdES.detached`) with an incremental save. +#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] + pub mod cms; pub mod der; #[cfg(target_os = "macos")] diff --git a/crates/sign/src/pkcs12.rs b/crates/sign/src/pkcs12.rs index 3511462d21..419d25c112 100644 --- a/crates/sign/src/pkcs12.rs +++ b/crates/sign/src/pkcs12.rs @@ -135,14 +135,15 @@ impl Cipher { } } -fn hmac(alg: DigestAlg, key: &[u8], data: &[u8]) -> Vec { - fn run(key: &[u8], data: &[u8]) -> Vec +fn hmac(alg: DigestAlg, key: &[u8], data: &[u8]) -> Result, SignError> { + fn run(key: &[u8], data: &[u8]) -> Result, SignError> where hmac::Hmac: hmac::KeyInit + Mac, { - let mut m = as hmac::KeyInit>::new_from_slice(key).expect("HMAC takes any key length"); + // HMAC takes any key length, so this never fails in practice. + let mut m = as hmac::KeyInit>::new_from_slice(key).map_err(|_| bad("MAC key"))?; m.update(data); - m.finalize().into_bytes().to_vec() + Ok(m.finalize().into_bytes().to_vec()) } match alg { DigestAlg::Sha1 => run::(key, data), @@ -301,7 +302,7 @@ pub fn open(bytes: &[u8], password: &str) -> Result { let iterations = m.get(2).map(|i| i.u64()).transpose()?.unwrap_or(1) as u32; let key_len = alg.digest(&[]).len(); let key = pkcs12_kdf(alg, &bmp(password), salt, 3, iterations, key_len); - if hmac(alg, &key, content) != expected { + if hmac(alg, &key, content)? != expected { return Err(SignError::WrongPassword); } } @@ -328,8 +329,13 @@ pub fn open(bytes: &[u8], password: &str) -> Result { _ => {} } } - let key_bag = all.iter().find(|b| matches!(b.kind, BagKind::Key(_))).ok_or_else(|| bad("no private key"))?; - let BagKind::Key(pkcs8) = &key_bag.kind else { unreachable!() }; + let (key_bag, pkcs8) = all + .iter() + .find_map(|b| match &b.kind { + BagKind::Key(k) => Some((b, k)), + _ => None, + }) + .ok_or_else(|| bad("no private key"))?; let key = PrivateKey::from_pkcs8(pkcs8)?; let certs: Vec<(Certificate, &Bag)> = all .iter() @@ -401,7 +407,7 @@ pub fn write(id: &DigitalId, password: &str) -> Result, SignError> { let auth_safe = der::seq(&[&ci_certs, &ci_key]); let salt = random(16)?; let mac_key = pkcs12_kdf(DigestAlg::Sha256, &bmp(password), &salt, 3, ITER, 32); - let mac = hmac(DigestAlg::Sha256, &mac_key, &auth_safe); + let mac = hmac(DigestAlg::Sha256, &mac_key, &auth_safe)?; let mac_data = der::seq(&[&der::seq(&[&DigestAlg::Sha256.algorithm(), &der::octets(&mac)]), &der::octets(&salt), &der::int(ITER as u64)]); Ok(der::seq(&[&der::int(3), &der::seq(&[&der::oid(DATA), &der::explicit(0, &der::octets(&auth_safe))]), &mac_data])) } diff --git a/crates/sign/src/x509.rs b/crates/sign/src/x509.rs index ec789da9f3..213197faf9 100644 --- a/crates/sign/src/x509.rs +++ b/crates/sign/src/x509.rs @@ -267,7 +267,7 @@ impl Certificate { pub fn build_chain<'a>(leaf: &'a Certificate, pool: &'a [Certificate]) -> Vec<&'a Certificate> { let mut chain = vec![leaf]; while chain.len() < 10 { - let last = *chain.last().expect("non-empty"); + let Some(&last) = chain.last() else { break }; if last.issuer.raw == last.subject.raw { break; } From 926171569e461dc966612841e8630810143c362a Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 13:35:00 -0400 Subject: [PATCH 2/2] Root clippy.toml: tests may unwrap, expect, panic and index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same file as the steering PR (#11), so this PR stands on its own. The crate-level `#![deny(clippy::unwrap_used, …)]` applies to production code only. Co-Authored-By: Claude Opus 5.5 --- clippy.toml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 clippy.toml diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000000..4b6c79f555 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,4 @@ +allow-unwrap-in-tests = true +allow-expect-in-tests = true +allow-panic-in-tests = true +allow-indexing-slicing-in-tests = true