M9: name the Windows store certificates that can't sign (#179) (#761)

A store certificate PdfCraft can't use (no private key, a P-521 /
Brainpool / Ed25519 key, a key CNG can't open) was dropped from the
Sign dialog silently, so a user whose CA-issued identity shows in
Acrobat but not here had nothing to go on. `windows::list` now returns
those under `unusable` with the subject, fingerprint and a reason that
names the Windows error code; the Sign dialog shows them greyed and
unselectable with the reason, and `sign_windows_ids` returns them.
A key that needs a prompt to open (NTE_SILENT_CONTEXT, NTE_UI_REQUIRED
while listing) is listed and prompts at signing instead of being
skipped. Certificates without a private key stay out of the dialog but
are reported by the tool.

Tests: the explained-error table, the store listing's invariants, a
key-less certificate in the ignored store test, the automation output,
and a kittest dialog test for a greyed entry. Verified by hand that a
.p12 imported into the CNG and legacy Microsoft providers lists, and
that a P-521 certificate is explained.

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
Epsilon-LLC
2026-10-10 16:25:24 -04:00
committed by GitHub
co-authored by Claude Fable 5.1 Brandon Thomas
parent 4dafbfc325
commit 6116b0034c
11 changed files with 351 additions and 74 deletions
+1
View File
@@ -216,6 +216,7 @@ M0 → M1 → M2 → M3 → M4 must happen in order. After M4, M5–M12 can run
## Log
Newest first: the date, what moved, and the new overall estimate.
- **2026-10-10 (M9, #179 follow-up):** A Windows store certificate that can't sign is no longer left out of the Sign dialog silently: `windows::list` names it with the reason and Windows error code (no private key, a P-521/Brainpool/Ed25519 key, a key CNG can't open), the picker shows it greyed and unselectable, and `sign_windows_ids` returns it under `unusable`. A key that needs a permission prompt to open is listed instead of skipped. Reported by a user whose CA-issued identity showed in Acrobat but not here; a plain .p12 import into the CNG and legacy Microsoft providers was verified to list, so the next report will carry the reason. Store test adds a key-less certificate; overall estimate unchanged (about 30–35%).
- **2026-10-10 (M10.2, JPEG scanning follow-up to #729):** Skip marker fill bytes and keep reading JFIF, APP14 and ICC metadata after the frame header, stopping before scan data or at EOI. Invalid segment lengths return errors; damaged or mismatched ICC profiles retain #729's device-space fallback. Synthetic regression cases added but not run at the contributor's request. Estimates unchanged.
- **2026-10-10 (M13, desktop action outputs):** Desktop Action Wizard keeps every result when input basenames repeat, or when the destination already has a result from an earlier run. It preserves the original filename and extension for a free name and adds numbered suffixes for occupied names; the bounded search returns an error rather than reusing an occupied path. A real-shell regression drives Start twice and reopens the three-page and one-page results, checking that the original inputs and earlier output remain intact. This covers the desktop writer separately from the CLI/MCP action_run repair in #877. Milestone and effort estimates unchanged.
+16 -7
View File
@@ -600,14 +600,23 @@ impl Automation {
}
"sign_windows_ids" => {
#[cfg(target_os = "windows")]
let ids: Vec<Value> = pdfcraft_engine::sign::windows::identities()
.map_err(failed)?
.iter()
.map(|id| json!({ "id": pdfcraft_engine::sign::windows::reference(&id.certificate), "certificate": signing::cert_json(&id.certificate) }))
.collect();
let (ids, unusable): (Vec<Value>, Vec<Value>) = {
let listing = pdfcraft_engine::sign::windows::list().map_err(failed)?;
let ids = listing
.ids
.iter()
.map(|id| json!({ "id": pdfcraft_engine::sign::windows::reference(&id.certificate), "certificate": signing::cert_json(&id.certificate) }))
.collect();
let unusable = listing
.unusable
.iter()
.map(|u| json!({ "subject": u.subject, "sha256": u.fingerprint, "reason": u.reason, "no_private_key": u.no_private_key }))
.collect();
(ids, unusable)
};
#[cfg(not(target_os = "windows"))]
let ids: Vec<Value> = Vec::new();
json!({ "count": ids.len(), "ids": ids })
let (ids, unusable): (Vec<Value>, Vec<Value>) = (Vec::new(), Vec::new());
json!({ "count": ids.len(), "ids": ids, "unusable": unusable })
}
"sign_trust" => self.sign_trust(&a)?,
"comment_mark" => self.comment_mark(&a)?,
+1 -1
View File
@@ -609,7 +609,7 @@ pub fn tools() -> Vec<ToolDef> {
}),
&["name", "password", "path"],
)),
t("sign_windows_ids", "List Windows store digital IDs", "Windows: signing identities in the Current User Personal certificate store (certificate details and the windows: reference sign_document takes). Private keys remain in CNG; Windows may ask permission to use them.")
t("sign_windows_ids", "List Windows store digital IDs", "Windows: signing identities in the Current User Personal certificate store (certificate details and the windows: reference sign_document takes). Private keys remain in CNG; Windows may ask permission to use them. `unusable` lists the store's other certificates with why they can't sign (no private key, an unsupported key type, a key CNG can't open).")
.ro()
.cmd("sign.digital")
.with(schema(json!({}), &[])),
+4 -1
View File
@@ -2755,8 +2755,11 @@ fn digital_ids_signing_and_validation_through_tools() {
let ids = store["ids"].as_array().unwrap();
assert_eq!(store["count"].as_u64().unwrap(), ids.len() as u64);
assert!(ids.iter().all(|id| id["id"].as_str().unwrap().starts_with("windows:")));
// Store certificates that can't sign are explained, not dropped (issue #179).
let unusable = store["unusable"].as_array().unwrap();
assert!(unusable.iter().all(|u| u["subject"].is_string() && !u["reason"].as_str().unwrap().is_empty() && u["no_private_key"].is_boolean()));
#[cfg(not(windows))]
assert!(ids.is_empty());
assert!(ids.is_empty() && unusable.is_empty());
assert!(matches!(
a.call("sign_document", &json!({ "doc": doc, "id": "ada.p12", "password": "wrong!", "out": "signed.pdf" })),
Err(ToolError::InvalidArgs(_))
+5 -1
View File
@@ -54,7 +54,11 @@ ECDSA P-256/P-384; the store integration is tested with software-backed keys. A
or token whose driver registers a CNG key storage provider is such an identity too: the key
is opened for signing without the silent flag, so the provider shows its own PIN dialog
(checked by hand with a SafeNet token; CI has no hardware). A store identity signs with the
issuers Windows chains its certificate to, without the root.
issuers Windows chains its certificate to, without the root. Store certificates that can't
sign (no private key, another key type, a key CNG can't open) are returned by
`windows::list` as `unusable` with the reason and the Windows error code, shown greyed in
the Sign dialog and by `sign_windows_ids`, instead of being left out silently; a key that
needs a prompt to open is listed and asks at signing (issue #179).
- **Validation:** `/ByteRange` and the CMS are read from the file's own bytes; the digest,
the signature value and the signer's chain (against a `TrustStore`) are checked. Later
revisions are diffed against the signed one, and the changes are classified (signing, form
+175 -29
View File
@@ -1,14 +1,19 @@
//! Signing identities in the Windows Current User Personal ("My") certificate store.
//! 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
//! silently, so a missing identity can be explained (issue #179).
use std::sync::Arc;
use rustls_cng::cert::CertContext;
use rustls_cng::key::{AlgorithmGroup, SignaturePadding};
use rustls_cng::error::CngError;
use rustls_cng::key::{AlgorithmGroup, NCryptKey, SignaturePadding};
use rustls_cng::store::{CertStore, CertStoreType};
use crate::keys::{DigestAlg, ExternalKey, PrivateKey, PublicKey, StoreKey};
use crate::{Certificate, DigitalId, SignError};
use crate::{Certificate, DigitalId, SignError, x509};
struct WindowsKey {
context: CertContext,
@@ -25,16 +30,39 @@ impl ExternalKey for WindowsKey {
// signing handle without the silent flag so the provider can show its own dialog.
let key = self.context.acquire_key(false).map_err(|e| {
SignError::Crypto(format!(
"the Windows certificate store key couldn't be opened for signing (is the smart card or token connected?): {e}"
"the Windows certificate store key couldn't be opened for signing (is the smart card or token connected?): {}",
describe(&e)
))
})?;
let raw = key
.sign(&digest, padding)
.map_err(|e| SignError::Crypto(format!("the Windows certificate store didn't sign (key use may have been cancelled): {e}")))?;
let raw = key.sign(&digest, padding).map_err(|e| {
SignError::Crypto(format!("the Windows certificate store didn't sign (key use may have been cancelled): {}", describe(&e)))
})?;
self.public.store_signature_der(&raw)
}
}
/// A Personal store certificate PdfCraft can't sign with, and why.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Unusable {
/// The subject's common name (else organization, else the DN), or a placeholder when
/// the certificate couldn't be read at all.
pub subject: String,
/// SHA-256 of the certificate as upper-case hex pairs (`Certificate::fingerprint`).
pub fingerprint: String,
/// Why it can't sign, as a clause that follows the name ("its private key is …").
pub reason: String,
/// The store holds no private key for it: a certificate someone else issued, filed under
/// Personal. Not a problem with an identity, so the Sign dialog doesn't show these.
pub no_private_key: bool,
}
/// The Personal store's signing identities and the certificates that can't sign.
#[derive(Default)]
pub struct Listing {
pub ids: Vec<DigitalId>,
pub unusable: Vec<Unusable>,
}
/// The `windows:<SHA-256 of the certificate DER>` reference kept for an identity.
pub fn reference(certificate: &Certificate) -> String {
let digest = DigestAlg::Sha256.digest(&[&certificate.raw]);
@@ -45,6 +73,7 @@ pub fn reference(certificate: &Certificate) -> String {
/// Windows finds for the certificate, so a signature embeds them as it does for a file's.
pub fn find(reference_or_name: &str) -> Result<DigitalId, SignError> {
let (context, mut id) = entries()?
.ids
.into_iter()
.find(|(_, id)| {
reference(&id.certificate) == reference_or_name
@@ -62,39 +91,156 @@ fn issuers(context: &CertContext, certificate: &Certificate) -> Vec<Certificate>
chain.iter().filter(|der| **der != certificate.raw).filter_map(|der| Certificate::parse(der).ok()).collect()
}
/// List usable RSA, P-256 and P-384 CNG identities in Current User > Personal.
/// Certificates with unsupported keys or without an accessible private key are skipped.
/// List usable RSA, P-256 and P-384 CNG identities in Current User > Personal
/// (the `ids` of [`list`]).
pub fn identities() -> Result<Vec<DigitalId>, SignError> {
Ok(entries()?.into_iter().map(|(_, id)| id).collect())
Ok(entries()?.ids.into_iter().map(|(_, id)| id).collect())
}
/// Each usable identity with the store's certificate context it came from.
fn entries() -> Result<Vec<(CertContext, DigitalId)>, SignError> {
/// List Current User > Personal: the RSA, P-256 and P-384 identities whose private key CNG can
/// open, and every other certificate with the reason it can't sign. A key that needs a prompt
/// to open counts as usable; the prompt appears at signing.
pub fn list() -> Result<Listing, SignError> {
let Entries { ids, unusable } = entries()?;
Ok(Listing { ids: ids.into_iter().map(|(_, id)| id).collect(), unusable })
}
/// Each usable identity with the store's certificate context it came from, and the rest.
struct Entries {
ids: Vec<(CertContext, DigitalId)>,
unusable: Vec<Unusable>,
}
fn entries() -> Result<Entries, SignError> {
let store = CertStore::open(CertStoreType::CurrentUser, "My")
.map_err(|e| SignError::Crypto(format!("the Windows Current User Personal store couldn't be opened: {e}")))?;
let contexts = store.find_all().map_err(|e| SignError::Crypto(format!("the Windows certificate store couldn't be searched: {e}")))?;
let mut out = Vec::new();
.map_err(|e| SignError::Crypto(format!("the Windows Current User Personal store couldn't be opened: {}", describe(&e))))?;
let contexts =
store.find_all().map_err(|e| SignError::Crypto(format!("the Windows certificate store couldn't be searched: {}", describe(&e))))?;
let mut ids = Vec::new();
let mut unusable = Vec::new();
for context in contexts {
let Ok(certificate) = Certificate::parse(context.as_der()) else { continue };
// Enumeration must not open permission or PIN dialogs. Signing may prompt later.
let Ok(key) = context.acquire_key(true) else { continue };
// P-521, brainpool and Ed25519 certificates parse (to verify with) but aren't signed with.
let supported = match certificate.public_key.store_signing_key() {
Some(StoreKey::Rsa) => key.algorithm_group().is_ok_and(|a| a == AlgorithmGroup::Rsa),
Some(StoreKey::Ecdsa(want)) => {
key.algorithm_group().is_ok_and(|a| a == AlgorithmGroup::Ecdsa) && key.bits().is_ok_and(|bits| bits == want)
}
None => false,
let der = context.as_der();
let fingerprint = DigestAlg::Sha256.digest(&[der]).iter().map(|b| format!("{b:02X}")).collect::<Vec<_>>().join(" ");
let mut skip = |subject: String, reason: String, no_private_key: bool| {
unusable.push(Unusable { subject, fingerprint: fingerprint.clone(), reason, no_private_key });
};
if !supported {
let certificate = match Certificate::parse(der) {
Ok(c) => c,
Err(e) => {
let subject = x509::subject_of(der).map(|n| n.display_name()).unwrap_or_else(|| "(unreadable certificate)".to_string());
skip(subject, format!("the certificate can't be read: {e}"), false);
continue;
}
};
// P-521, brainpool and Ed25519 certificates parse (to verify with) but aren't signed with.
let Some(want) = certificate.public_key.store_signing_key() else {
let what = certificate.public_key.describe();
skip(certificate.display_name(), format!("its {what} key can't sign through PdfCraft yet (RSA, P-256 and P-384 only)"), false);
continue;
};
// Enumeration must not open permission or PIN dialogs. Signing may prompt later.
match context.acquire_key(true) {
Ok(key) => {
if let Err(reason) = matches_key(&key, want) {
skip(certificate.display_name(), reason, false);
continue;
}
// The silent handle was only for the checks above; `sign` acquires its own.
drop(key);
}
// The key opens only with a prompt (strong private key protection, a PIN): usable.
Err(CngError::WindowsError(NTE_SILENT_CONTEXT | NTE_UI_REQUIRED)) => {}
Err(e) => {
let no_private_key = e == CngError::WindowsError(CRYPT_E_NO_KEY_PROPERTY);
let reason = if no_private_key {
"it has no private key in the store".to_string()
} else {
format!("its private key can't be opened: {}", describe(&e))
};
skip(certificate.display_name(), reason, no_private_key);
continue;
}
}
// The silent handle was only for the checks above; `sign` acquires its own.
drop(key);
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());
out.push((context, DigitalId { key, certificate, chain: Vec::new(), friendly_name }));
ids.push((context, DigitalId { key, certificate, chain: Vec::new(), friendly_name }));
}
Ok(Entries { ids, unusable })
}
/// Whether the store's key is what the certificate needs (RSA; ECDSA on its curve).
fn matches_key(key: &NCryptKey, want: StoreKey) -> Result<(), String> {
let group = key.algorithm_group().map_err(|e| format!("its private key's algorithm couldn't be read: {}", describe(&e)))?;
let name = |g: &AlgorithmGroup| match g {
AlgorithmGroup::Rsa => "an RSA".to_string(),
AlgorithmGroup::Ecdsa => "an ECDSA".to_string(),
other => format!("a {other:?}"),
};
let expected = match want {
StoreKey::Rsa => AlgorithmGroup::Rsa,
StoreKey::Ecdsa(_) => AlgorithmGroup::Ecdsa,
};
if group != expected {
return Err(format!("its certificate holds {} key but the store's private key is {} key", name(&expected), name(&group)));
}
if let StoreKey::Ecdsa(bits) = want {
let actual = key.bits().map_err(|e| format!("its private key's size couldn't be read: {}", describe(&e)))?;
if actual != bits {
return Err(format!("its certificate holds a P-{bits} key but the store's private key has {actual} bits"));
}
}
Ok(())
}
// Error codes (winerror.h) that explain why a key can't be opened.
const NTE_NO_KEY: u32 = 0x8009_000D;
const NTE_PERM: u32 = 0x8009_0010;
const NTE_NOT_FOUND: u32 = 0x8009_0011;
const NTE_BAD_PROVIDER: u32 = 0x8009_0013;
const NTE_BAD_PROV_TYPE: u32 = 0x8009_0014;
const NTE_BAD_KEYSET: u32 = 0x8009_0016;
const NTE_PROV_DLL_NOT_FOUND: u32 = 0x8009_001E;
const NTE_SILENT_CONTEXT: u32 = 0x8009_0022;
const NTE_NOT_SUPPORTED: u32 = 0x8009_0029;
const NTE_UI_REQUIRED: u32 = 0x8009_002E;
const NTE_DEVICE_NOT_READY: u32 = 0x8009_0030;
const NTE_DEVICE_NOT_FOUND: u32 = 0x8009_0035;
const NTE_USER_CANCELLED: u32 = 0x8009_0036;
const CRYPT_E_NO_KEY_PROPERTY: u32 = 0x8009_200B;
/// A CNG error with the Windows code and, for the usual ones, what it means.
fn describe(e: &CngError) -> String {
let CngError::WindowsError(code) = e else { return e.to_string() };
let meaning = match *code {
NTE_NO_KEY | NTE_BAD_KEYSET | NTE_NOT_FOUND => "the key is missing from its key store (deleted, or imported under another Windows account)",
NTE_PERM => "access to the key was denied",
NTE_BAD_PROVIDER | NTE_BAD_PROV_TYPE | NTE_PROV_DLL_NOT_FOUND => {
"its key provider isn't available to CNG (a legacy CryptoAPI-only provider, or its driver isn't installed)"
}
NTE_NOT_SUPPORTED => "its key provider doesn't support the operation",
NTE_DEVICE_NOT_READY | NTE_DEVICE_NOT_FOUND => "the smart card or token isn't available",
NTE_USER_CANCELLED => "the request was cancelled",
NTE_SILENT_CONTEXT | NTE_UI_REQUIRED => "the key needs a prompt to open",
CRYPT_E_NO_KEY_PROPERTY => "the certificate has no private key",
0x8010_0000..=0x8010_FFFF => "the smart card isn't available",
_ => return format!("Windows error 0x{code:08X}"),
};
format!("{meaning} (Windows error 0x{code:08X})")
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn errors_are_explained_with_their_code() {
assert_eq!(
describe(&CngError::WindowsError(NTE_BAD_KEYSET)),
"the key is missing from its key store (deleted, or imported under another Windows account) (Windows error 0x80090016)"
);
assert_eq!(describe(&CngError::WindowsError(0x8010_000C)), "the smart card isn't available (Windows error 0x8010000C)");
assert_eq!(describe(&CngError::WindowsError(0x57)), "Windows error 0x00000057");
assert_eq!(describe(&CngError::InvalidHashLength), "Invalid hash length");
}
Ok(out)
}
+19 -1
View File
@@ -57,6 +57,11 @@ impl Name {
self.get(C)
}
/// The name Acrobat shows: the common name, else the organization, else the DN.
pub fn display_name(&self) -> String {
self.common_name().or(self.organization()).map(str::to_string).unwrap_or_else(|| self.display())
}
/// "CN=Ada Lovelace, O=Example, E=ada@example.com"
pub fn display(&self) -> String {
self.attrs
@@ -400,7 +405,7 @@ impl Certificate {
/// The display name Acrobat uses: the common name, else the organization, else the DN.
pub fn display_name(&self) -> String {
self.subject.common_name().or(self.subject.organization()).map(str::to_string).unwrap_or_else(|| self.subject.display())
self.subject.display_name()
}
/// SHA-256 fingerprint as upper-case hex pairs.
@@ -452,6 +457,19 @@ impl Certificate {
}
}
/// The subject alone, to name a certificate `Certificate::parse` rejects (one with a key type
/// PdfCraft doesn't support, say). `None` when even that can't be read.
pub fn subject_of(raw: &[u8]) -> Option<Name> {
let cert = Tlv::parse_all(raw).ok()?.expect(tag::SEQUENCE, "Certificate").ok()?;
let tbs = cert.children().ok()?.into_iter().next()?;
let mut f = tbs.children().ok()?.into_iter().peekable();
if f.peek().is_some_and(|t| t.tag == tag::ctx(0)) {
f.next();
}
// serial, signature algorithm, issuer, validity, then the subject.
Name::parse(&f.nth(4)?).ok()
}
/// 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
+23 -3
View File
@@ -1086,7 +1086,12 @@ fn a_timestamp_token_may_sign_with_a_different_digest_than_its_imprint() {
#[cfg(windows)]
#[test]
fn windows_store_enumeration_and_missing_identity() {
assert!(pdfcraft_sign::windows::identities().is_ok());
let listing = pdfcraft_sign::windows::list().unwrap();
assert_eq!(listing.ids.len(), pdfcraft_sign::windows::identities().unwrap().len());
// Whatever this machine's store holds, every certificate left out is named and explained.
for u in &listing.unusable {
assert!(!u.subject.is_empty() && !u.reason.is_empty() && u.fingerprint.len() == 95, "{u:?}");
}
assert!(pdfcraft_sign::windows::find("windows:no such signer").is_err());
}
@@ -1108,9 +1113,10 @@ impl Drop for Certificates {
fn drop(&mut self) {
for thumbprint in &self.0 {
// New-SelfSignedCertificate also leaves a public copy of a self-signed or CA
// certificate in Intermediate Certification Authorities (CA).
// certificate in Intermediate Certification Authorities (CA). -DeleteKey fails on
// a certificate without a key; the plain removal then runs.
let out = powershell(&format!(
"Remove-Item -LiteralPath 'Cert:\\CurrentUser\\My\\{thumbprint}' -DeleteKey; $copy='Cert:\\CurrentUser\\CA\\{thumbprint}'; if (Test-Path -LiteralPath $copy) {{ Remove-Item -LiteralPath $copy }}"
"$p='Cert:\\CurrentUser\\My\\{thumbprint}'; try {{ Remove-Item -LiteralPath $p -DeleteKey }} catch {{ Remove-Item -LiteralPath $p }}; $copy='Cert:\\CurrentUser\\CA\\{thumbprint}'; if (Test-Path -LiteralPath $copy) {{ Remove-Item -LiteralPath $copy }}"
));
if !out.status.success() {
eprintln!("test certificate cleanup failed: {}", String::from_utf8_lossy(&out.stderr));
@@ -1156,6 +1162,20 @@ fn signing_with_windows_store_identities() {
assert_eq!(signature.modification, Modification::None);
assert_eq!(signature.signer.as_deref(), Some(name.as_str()));
}
// A certificate without a private key (issued to someone else, filed under Personal) is
// reported as such rather than listed or dropped silently (issue #179).
let ada = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap().certificate;
let cer = std::env::temp_dir().join(format!("pdfcraft-windows-store-{unique}.cer"));
std::fs::write(&cer, &ada.raw).unwrap();
let out = powershell(&format!("(Import-Certificate -FilePath '{}' -CertStoreLocation 'Cert:\\CurrentUser\\My').Thumbprint", cer.display()));
let _ = std::fs::remove_file(&cer);
assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr));
created.0.push(String::from_utf8(out.stdout).unwrap().trim().to_string());
let listing = pdfcraft_sign::windows::list().unwrap();
let reported = listing.unusable.iter().find(|u| u.fingerprint == ada.fingerprint()).expect("key-less certificate reported");
assert!(reported.no_private_key, "{reported:?}");
assert_eq!(reported.subject, ada.display_name());
assert!(listing.ids.iter().all(|id| id.certificate.raw != ada.raw));
let thumbprints = created.0.clone();
drop(created);
for thumbprint in thumbprints {
+63 -29
View File
@@ -21,6 +21,10 @@ pub struct DigitalIdEntry {
pub issuer: String,
pub email: String,
pub expires: String,
/// Why the ID can't sign (a Windows store certificate whose key PdfCraft can't use): shown
/// greyed with the reason, and never selectable.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub unusable: Option<String>,
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -93,14 +97,16 @@ pub struct SignDraft {
}
impl SignDraft {
fn new(page: usize, rect: Option<[f64; 4]>, field: Option<String>, certify: Option<u8>, ids: usize) -> Self {
/// Starts at Choose when there are IDs to show (even only unusable ones, so their reasons
/// are seen), with the first usable one selected.
fn new(page: usize, rect: Option<[f64; 4]>, field: Option<String>, certify: Option<u8>, ids: &[DigitalIdEntry]) -> Self {
Self {
page,
rect,
field,
certify,
step: if ids == 0 { SignStep::Configure } else { SignStep::Choose },
selected: (ids > 0).then_some(0),
step: if ids.is_empty() { SignStep::Configure } else { SignStep::Choose },
selected: ids.iter().position(|e| e.unusable.is_none()),
password: String::new(),
reason: String::new(),
location: String::new(),
@@ -220,6 +226,20 @@ pub fn entry_for(path: &str, c: &Certificate) -> DigitalIdEntry {
issuer: c.issuer.common_name().map(str::to_string).unwrap_or_else(|| c.issuer.display()),
email: c.subject.email().unwrap_or("").to_string(),
expires: format!("{:04}.{:02}.{:02}", c.not_after.year, c.not_after.month, c.not_after.day),
unusable: None,
}
}
/// A greyed entry for a Windows store certificate that can't sign, with the reason.
#[cfg(target_os = "windows")]
fn entry_unusable(u: &sign::windows::Unusable) -> DigitalIdEntry {
DigitalIdEntry {
path: format!("windows:{}", u.fingerprint.split(' ').collect::<String>().to_ascii_lowercase()),
name: u.subject.clone(),
issuer: String::new(),
email: String::new(),
expires: String::new(),
unusable: Some(u.reason.clone()),
}
}
@@ -227,7 +247,7 @@ impl PdfCraftApp {
/// Start signing: the rectangle (or field) is known; show Sign with a Digital ID.
pub fn start_signing(&mut self, page: usize, rect: Option<[f64; 4]>, field: Option<String>, certify: Option<u8>) {
self.refresh_os_key_store_ids();
self.sign_draft = Some(SignDraft::new(page, rect, field, certify, self.digital_ids.len()));
self.sign_draft = Some(SignDraft::new(page, rect, field, certify, &self.digital_ids));
self.dialog = Some(crate::Dialog::Sign);
}
@@ -247,11 +267,16 @@ impl PdfCraftApp {
}
#[cfg(target_os = "windows")]
if self.os_key_store_ids {
match sign::windows::identities() {
Ok(ids) => {
for id in ids {
match sign::windows::list() {
Ok(listing) => {
for id in &listing.ids {
self.digital_ids.push(entry_for(&sign::windows::reference(&id.certificate), &id.certificate));
}
// Certificates without a private key aren't identities; the rest are shown
// greyed with why they can't sign (issue #179).
for u in listing.unusable.iter().filter(|u| !u.no_private_key) {
self.digital_ids.push(entry_unusable(u));
}
}
Err(e) => self.notify_fmt("The Windows store's digital IDs couldn't be listed: {e}", &[("e", &e.to_string())]),
}
@@ -325,7 +350,7 @@ impl PdfCraftApp {
}
let Some((_, doc_id)) = self.active_ids() else { return Err("no document".into()) };
let d = self.sign_draft.clone().ok_or("nothing to sign")?;
let entry = d.selected.and_then(|i| self.digital_ids.get(i)).cloned().ok_or("Choose a digital ID.")?;
let entry = d.selected.and_then(|i| self.digital_ids.get(i)).filter(|e| e.unusable.is_none()).cloned().ok_or("Choose a digital ID.")?;
let id = if entry.path.starts_with("keychain:") {
keychain_id(&entry.path)?
} else if entry.path.starts_with("windows:") {
@@ -467,17 +492,20 @@ fn choose(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> bool {
egui::ScrollArea::vertical().max_height(220.0).show(ui, |ui| {
for (i, e) in ids.iter().enumerate() {
let selected = d.selected == Some(i);
let usable = e.unusable.is_none();
let (rect, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 52.0), egui::Sense::click());
resp.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::RadioButton, true, selected, &e.name));
resp.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::RadioButton, usable, selected, &e.name));
let fill = if selected {
t.accent_soft
} else if resp.hovered() {
} else if resp.hovered() && usable {
t.hover
} else {
Color32::TRANSPARENT
};
ui.painter().rect_filled(rect, CornerRadius::same(6), fill);
icons::paint(ui, Rect::from_min_size(rect.min + vec2(10.0, 15.0), vec2(20.0, 20.0)), "badge-check", 18.0, t.accent);
// An unusable ID is greyed, with the reason where the issuer would be.
let (icon, name_color) = if usable { (t.accent, t.text) } else { (t.text_muted, t.text_muted) };
icons::paint(ui, Rect::from_min_size(rect.min + vec2(10.0, 15.0), vec2(20.0, 20.0)), "badge-check", 18.0, icon);
// Certificate names can be any length: cut both lines with "…" at the row's edge
// and show them whole on hover.
let line = |text: &str, font: egui::FontId, color: Color32| {
@@ -485,27 +513,33 @@ fn choose(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> bool {
job.wrap = egui::text::TextWrapping::truncate_at_width((rect.width() - 50.0).max(0.0));
ui.painter().layout_job(job)
};
let name = line(&e.name, theme::semibold(13.0), t.text);
let sub = format!(
"{keychain}{email}{issued}{issuer}{expires}{date}",
keychain = if e.path.starts_with("keychain:") {
tl!("Keychain · ").to_string()
} else if e.path.starts_with("windows:") {
tl!("Windows store · ").to_string()
} else {
String::new()
},
email = if e.email.is_empty() { String::new() } else { format!("{} · ", e.email) },
issued = tl!("Issued by: "),
issuer = e.issuer,
expires = tl!(", Expires: "),
date = e.expires,
);
let name = line(&e.name, theme::semibold(13.0), name_color);
let store = if e.path.starts_with("keychain:") {
tl!("Keychain · ").to_string()
} else if e.path.starts_with("windows:") {
tl!("Windows store · ").to_string()
} else {
String::new()
};
let sub = match &e.unusable {
Some(reason) => format!("{store}{reason}"),
None => format!(
"{store}{email}{issued}{issuer}{expires}{date}",
email = if e.email.is_empty() { String::new() } else { format!("{} · ", e.email) },
issued = tl!("Issued by: "),
issuer = e.issuer,
expires = tl!(", Expires: "),
date = e.expires,
),
};
let details = line(&sub, theme::regular(11.5), t.text_muted);
let elided = name.elided || details.elided;
ui.painter().galley(rect.min + vec2(40.0, 9.0), name, t.text);
ui.painter().galley(rect.min + vec2(40.0, 9.0), name, name_color);
ui.painter().galley(rect.min + vec2(40.0, 28.0), details, t.text_muted);
let resp = if elided { resp.on_hover_text(format!("{}\n{sub}", e.name)) } else { resp };
if !usable {
continue;
}
if resp.clicked() {
d.selected = Some(i);
}
@@ -1141,7 +1175,7 @@ mod tests {
fn create_in(dir: &std::path::Path) -> Result<PathBuf, String> {
let mut app = PdfCraftApp::new();
app.export_dir_override = Some(dir.to_string_lossy().into_owned());
let mut draft = SignDraft::new(0, None, None, None, 0);
let mut draft = SignDraft::new(0, None, None, None, &[]);
// P-256 keeps the test fast.
let key = KEY_ALGORITHMS.iter().position(|k| k.1 == "p256").unwrap();
draft.new_id =
+42
View File
@@ -258,6 +258,7 @@ fn os_store_identities_are_not_persisted() {
issuer: "Test issuer".into(),
email: String::new(),
expires: "2030.01.01".into(),
unusable: None,
});
}
let saved = app.persist();
@@ -282,6 +283,7 @@ fn windows_store_identity_does_not_ask_for_a_file_password() {
issuer: "Test issuer".into(),
email: String::new(),
expires: "2030.01.01".into(),
unusable: None,
});
let draft = h.state_mut().sign_draft.as_mut().unwrap();
draft.step = SignStep::Choose;
@@ -311,6 +313,7 @@ fn long_digital_id_names_stay_inside_the_picker() {
issuer: long,
email: String::new(),
expires: "2030.01.01".into(),
unusable: None,
});
h.state_mut().sign_draft.as_mut().unwrap().step = SignStep::Choose;
h.run_steps(3);
@@ -334,3 +337,42 @@ fn long_digital_id_names_stay_inside_the_picker() {
);
}
}
/// A store certificate that can't sign is shown with its reason (so the user learns why an
/// identity Acrobat lists is missing, issue #179) but is never selected or chosen.
#[test]
fn an_unusable_id_is_shown_with_its_reason_and_cannot_be_chosen() {
let mut h = harness(dir());
// Not a `windows:` path: those are replaced by the real store listing when signing starts.
h.state_mut().digital_ids.push(pdfcraft_ui_egui::DigitalIdEntry {
path: "store-test:broken".into(),
name: "Broken signer".into(),
issuer: String::new(),
email: String::new(),
expires: String::new(),
unusable: Some("its private key can't be opened: the key is missing from its key store (Windows error 0x80090016)".into()),
});
h.state_mut().start_signing(0, None, None, None);
h.run_steps(3);
let draft = h.state().sign_draft.as_ref().unwrap();
assert_eq!(draft.step, SignStep::Choose, "shown, with its reason, rather than skipped to Configure");
assert_eq!(draft.selected, None);
h.get_by_label("Broken signer").click();
h.run_steps(3);
assert_eq!(h.state().sign_draft.as_ref().unwrap().selected, None);
h.get_by_label("Continue").click();
h.run_steps(3);
assert_eq!(h.state().sign_draft.as_ref().unwrap().step, SignStep::Choose);
// A usable ID after it is the one selected to begin with.
h.state_mut().digital_ids.push(pdfcraft_ui_egui::DigitalIdEntry {
path: "store-test:fine".into(),
name: "Working signer".into(),
issuer: "Test issuer".into(),
email: String::new(),
expires: "2030.01.01".into(),
unusable: None,
});
h.state_mut().start_signing(0, None, None, None);
h.run_steps(3);
assert_eq!(h.state().sign_draft.as_ref().unwrap().selected, Some(1));
}
+2 -2
View File
@@ -5822,8 +5822,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"]
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:<fingerprint|common name> for agents and Windows store labels in the desktop ID picker. The signature embeds the issuers Windows chains the certificate to. Tested with throwaway software-backed RSA-2048, P-256 and P-384 certificates (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); direct PKCS #11 still planned."
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:<fingerprint|common name> 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."
[[feature]]
id = "sign.encrypted-documents"