diff --git a/crates/sign/src/windows.rs b/crates/sign/src/windows.rs index 3974aff8ca..275bdb52c8 100644 --- a/crates/sign/src/windows.rs +++ b/crates/sign/src/windows.rs @@ -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 { 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()); diff --git a/crates/sign/src/x509.rs b/crates/sign/src/x509.rs index 1a0ef58ade..6f4a731e7a 100644 --- a/crates/sign/src/x509.rs +++ b/crates/sign/src/x509.rs @@ -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 { + 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) diff --git a/crates/sign/tests/chain.rs b/crates/sign/tests/chain.rs index ece529765a..59834104ee 100644 --- a/crates/sign/tests/chain.rs +++ b/crates/sign/tests/chain.rs @@ -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]| 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. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 94b2093b98..e19efb0a09 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5912,8 +5912,8 @@ milestone = "M9" status = "shipped" commands = ["sign.digital", "sign.certify"] tools = ["sign_windows_ids", "sign_document"] -evidence = ["crates/sign/tests/pdf.rs::signing_with_windows_store_identities", "crates/sign/tests/pdf.rs::a_windows_store_identity_signs_with_its_issuers", "crates/sign/tests/pdf.rs::windows_store_enumeration_and_missing_identity", "crates/automation/tests/automation.rs::digital_ids_signing_and_validation_through_tools", "crates/ui-egui/tests/signing.rs::windows_store_identity_does_not_ask_for_a_file_password", "crates/ui-egui/tests/signing.rs::an_unusable_id_is_shown_with_its_reason_and_cannot_be_chosen"] -notes = "Current User Personal (My) CNG identities: RSA PKCS #1 v1.5 and DER-encoded ECDSA P-256/P-384. Keys never leave Windows; windows: for agents and Windows store labels in the desktop ID picker. The signature embeds the issuers Windows chains the certificate to. Store certificates that can't sign (no private key, another key type, a key CNG can't open) are reported with the reason and Windows error code (greyed in the picker, `unusable` in sign_windows_ids) rather than left out; keys that need a prompt to open are listed and prompt at signing. Tested with throwaway software-backed RSA-2048, P-256 and P-384 certificates and a key-less certificate (run with --ignored). Smart cards and tokens whose driver registers a CNG key storage provider sign with the provider's own PIN dialog (checked by hand with a SafeNet token only; no hardware in CI); still missing: the Local Machine store, legacy CryptoAPI-only providers, P-521/Brainpool/Ed25519 keys, direct PKCS #11." +evidence = ["crates/sign/tests/pdf.rs::signing_with_windows_store_identities", "crates/sign/tests/pdf.rs::a_windows_store_identity_signs_with_its_issuers", "crates/sign/tests/pdf.rs::windows_store_enumeration_and_missing_identity", "crates/automation/tests/automation.rs::digital_ids_signing_and_validation_through_tools", "crates/ui-egui/tests/signing.rs::windows_store_identity_does_not_ask_for_a_file_password", "crates/ui-egui/tests/signing.rs::an_unusable_id_is_shown_with_its_reason_and_cannot_be_chosen", "crates/sign/tests/chain.rs::a_smart_cards_certificates_that_may_not_sign_say_why_in_id_lists"] +notes = "Current User Personal (My) CNG identities: RSA PKCS #1 v1.5 and DER-encoded ECDSA P-256/P-384. Keys never leave Windows; windows: for agents and Windows store labels in the desktop ID picker. The signature embeds the issuers Windows chains the certificate to. Store certificates that can't sign (no private key, another key type, a key CNG can't open, or a key usage or extended key usage that doesn't allow signing documents, like a CAC's encryption and logon certificates) are reported with the reason and Windows error code (greyed in the picker, `unusable` in sign_windows_ids) rather than left out; keys that need a prompt to open are listed and prompt at signing. Tested with throwaway software-backed RSA-2048, P-256 and P-384 certificates and a key-less certificate (run with --ignored). Smart cards and tokens whose driver registers a CNG key storage provider sign with the provider's own PIN dialog (checked by hand with a SafeNet token only; no hardware in CI); still missing: the Local Machine store, legacy CryptoAPI-only providers, P-521/Brainpool/Ed25519 keys, direct PKCS #11." [[feature]] id = "sign.encrypted-documents"