fix(sign): offer only certificates that may sign documents from the Windows store (#3365)

A CAC puts its encryption, logon and signing certificates in the store
under the same name, so the digital ID list showed several identical
entries and only one of them makes a signature that validates. Grey out
the ones whose key usage or extended key usage doesn't allow document
signing, with the reason, the same way an ID without a usable key is
already treated.

Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
TomCatTandClaude Opus 5.5 authored and GitHub committed 2026-10-19 02:49:57 -04:00
1 parent 2f21f920a0
commit 4d42b48d20
4 files changed
+34 -3

No files matched your search

+8 -1
View File
@@ -2,7 +2,7 @@
//! Private keys remain in CNG; Windows may ask permission to use a key.
//!
//! Certificates that can't sign through PdfCraft (no usable private key, a key type it doesn't
//! sign with) are reported in [`Listing::unusable`] with the reason rather than left out
//! sign with, a key usage or extended key usage that doesn't allow signing documents) are reported in [`Listing::unusable`] with the reason rather than left out
//! silently, so a missing identity can be explained (issue #179).
use std::sync::Arc;
@@ -161,6 +161,13 @@ fn entries() -> Result<Entries, SignError> {
continue;
}
}
// A smart card holds encryption and logon certificates beside the signing one, all under
// the same name; a signature made with them doesn't validate, so they aren't offered.
// Checked after the key, so a certificate without one stays out of the list entirely.
if let Some(reason) = certificate.signing_problem_clause() {
skip(certificate.display_name(), reason, false);
continue;
}
let public = certificate.public_key.clone();
let key = PrivateKey::external(public.clone(), Arc::new(WindowsKey { context: context.clone(), public }));
let friendly_name = Some(certificate.display_name());
+9
View File
@@ -384,6 +384,15 @@ impl Certificate {
}
}
/// [`signing_problem`](Self::signing_problem) as a clause that follows the certificate's
/// name ("it is not issued for signing documents (…)"), for lists of digital IDs.
pub fn signing_problem_clause(&self) -> Option<String> {
self.signing_problem().map(|d| match d.message.strip_prefix("The signer's certificate ") {
Some(rest) => format!("it {}", rest.strip_suffix('.').unwrap_or(rest)),
None => d.message,
})
}
/// Issued by itself (subject = issuer and its own key verifies it).
pub fn is_self_signed(&self) -> bool {
self.issuer.raw == self.subject.raw && self.signed_by(&self.public_key)
+15
View File
@@ -281,6 +281,21 @@ fn a_trusted_signer_whose_certificate_may_not_sign_is_not_valid() {
assert_eq!(status, Status::Valid, "{details:?}");
}
#[test]
fn a_smart_cards_certificates_that_may_not_sign_say_why_in_id_lists() {
// A DoD CAC's four certificates, all under the same name; only the signing one may sign.
let r = root();
let clause = |exts: &[Vec<u8>]| signer_with("DOE.JANE.1234567890", &r, exts).cert.signing_problem_clause();
let encryption = clause(&[key_usage(KEY_ENCIPHERMENT), extended_key_usage(&["1.3.6.1.5.5.7.3.4"])]);
assert_eq!(encryption.as_deref(), Some("it does not allow digital signatures (its key usage has neither digital signature nor non-repudiation)"));
let logon = clause(&[key_usage(DIGITAL_SIGNATURE), extended_key_usage(&["1.3.6.1.4.1.311.20.2.2", "1.3.6.1.5.5.7.3.2"])]);
assert_eq!(logon.as_deref(), Some("it is not issued for signing documents (its extended key usage has no document-signing purpose)"));
let card_auth = clause(&[key_usage(DIGITAL_SIGNATURE), extended_key_usage(&["2.16.840.1.101.3.6.8"])]);
assert_eq!(card_auth, logon);
let signing = clause(&[key_usage(DIGITAL_SIGNATURE | 0x40), extended_key_usage(&["1.3.6.1.5.5.7.3.4", "1.3.6.1.4.1.311.10.3.12"])]);
assert_eq!(signing, None);
}
#[test]
fn unreadable_constraint_extensions_fail_closed() {
// keyUsage present but not a BIT STRING: no usage at all, so no keyCertSign.