PIV applet fingerprinting, device specific feature gates - #128
Merged
Merged
Conversation
New `keyroost-piv::fingerprint` module: `AppletFingerprint` classifies which PIV applet implementation a card is actually running, since "answers PIV APDUs correctly" says nothing about vendor quirks worth knowing during bring-up (firmware bugs, proprietary extensions, which management commands exist). Classification is best-effort and layered: - ATR historical-byte text and the SELECT response's own text are the primary signals (YubiKey, Token2, uTrust, HID Crescendo C2300/C4000, Authentrend ATKey, arekinath/PivApplet and its Swissbit iShield variant, Trussed's piv-authenticator and its Nitrokey variant, OpenFIPS201). - A few identities have no distinguishing text at all and are recognised solely by whether a specific AID/RID can be selected: Feitian's registered RID, IdPrime's second PIV applet instance, and a Swissbit iShield running OpenFIPS201 (Swissbit's own registered RID). These probes only run when cheaper checks leave the fingerprint undecided, in a fixed priority order, and always re-SELECT PIV afterward. - `classify()` and every byte-level parser are pure and unit-tested without hardware; the transport layer supplies only the live-probe bits. `PivStatus` gains `applet_fingerprint`, `applet_name` (populated only when a token's own probe reports a specific name — currently Nitrokey's admin application), and `version_firmware` (ditto, not necessarily equal to the PIV applet's own `version`). `serial` widens to `u128`: a Nitrokey answers the Yubico GET SERIAL extension with an unrelated, made-up number, so its admin application supplies the real serial instead, and GET SERIAL is skipped whenever it does. `keyroostctl piv status` prints the fingerprint and name, appends the firmware version to the Version line when it differs from the applet's own, and extends the `--json` output with all of the above. SELECT APDUs in the `--debug` trace now name the AID/RID they target (the standard PIV application and each fingerprinting probe's own AID/RID), and `PivSession::open_with_debug` fixes that trace missing its own first SELECT (the previous `open()` + `set_debug()` sequencing always traced one APDU too late). Co-Authored-By: Claude <noreply@anthropic.com>
The status line under the PIV panel read "Applet <version> · Serial <serial> · PIN retries <n>" with no indication of which applet family was actually detected — that information already lived in PivStatus (applet_fingerprint / applet_name) and was surfaced in keyroostctl's `piv status` output, but not in the GUI. Prepend the applet's name to the line, using the same fallback already established for the CLI: the token's own reported name when it has one (e.g. a Nitrokey's admin application), otherwise the generic name for its fingerprinted applet family (AppletFingerprint::applet_name()). Co-Authored-By: Claude <noreply@anthropic.com>
`serial` was widened to `u128` for Nitrokey's 128-bit admin serial, but both `keyroostctl` and the GUI kept formatting every serial as if it were still Yubico's 4-byte `u32`: decimal with a padded hex form in parentheses (CLI) or bare decimal (GUI). Past 64 bits that's unwieldy and no vendor prints a serial that large in decimal. Add `keyroost_piv::format_serial_long`/`format_serial_short` that switch to hex-only display (no parenthetical) once the value no longer fits a `u64`, and use them at both call sites. JSON output is untouched — it stays the raw integer. Co-Authored-By: Claude <noreply@anthropic.com>
A Token2 PIV applet's raw serial reply turns out to be BCD-coded: each nibble of the raw value is a decimal digit of the number printed on the device, so the reply's hex text — not its integer value — reproduces the real serial. Add `keyroost_piv::decode_bcd_serial`, which recovers the real serial by walking the raw value's nibbles most-significant first and accumulating `acc * 10 + nibble`. This drops the earlier hex-format / parse-as-decimal round-trip in favour of plain bit-shift arithmetic. It fails safe the same way the string version did: a nibble outside `0..=9` is not a decimal digit, so the raw serial is returned unchanged rather than forcing a bad conversion through. A full 32 decimal digits still fit in `u128`, so the accumulation cannot overflow. The function itself carries no vendor knowledge. `keyroost-transport`'s `decode_serial_if_bcd` decides when to apply it, gating on the applet fingerprint; Token2 is the only device known to report a BCD serial so far, and others can be added there without touching the decoder. BCD decode only recovers the truncated 4-byte GET SERIAL value; framefilter#125 tracks getting the device's full serial instead. Refs framefilter#125 Co-Authored-By: Claude <noreply@anthropic.com>
Token2's OpenPGP applet reports its serial in the same BCD coding its PIV applet uses: each nibble of the raw value is a decimal digit of the number printed on the device. `OpenPgpStatus::serial` was handing back the raw integer. Move `decode_bcd_serial` out of `keyroost-piv`'s public API and into `keyroost-transport` as a crate-private helper next to the other serial/status-word utilities — both call sites live in this crate and the decode has no PIV specifics of its own. The PIV path is unchanged behaviourally; it just calls the local helper now. `OpenPgpStatus::serial` gains the same treatment, gated on the AID manufacturer ID: when it is Token2 the raw serial is BCD-decoded, otherwise it passes through as a plain integer. The Token2 manufacturer ID is now a named constant, `keyroost_openpgp::MANUFACTURER_ID_TOKEN2`, so the gate and `manufacturer_name` share one definition. Token2 remains the only device known to report a BCD serial on either applet; others can be added at the two gates without touching the decoder. BCD decode only recovers the truncated 4-byte AID-embedded serial; framefilter#125 tracks getting the device's full serial instead. Refs framefilter#125 Co-Authored-By: Claude <noreply@anthropic.com>
The PIV slot panel hid the Move key and Delete key controls whenever
the loaded status reported a YubiKey firmware older than 5.7 — or no
version at all. That made a real capability invisible: a user on a 5.7+
key with a status that hadn't populated the version field saw no button,
and a user on older firmware had no way to learn the feature exists or
why it's out of reach.
Keep both controls on screen in that case, rendered through a new
theme::button_disabled: same footprint and rounded shape as a live
button, painted muted, no hover-lift or press or click sense, but it
still senses hover so the call site can hang an .on_hover_text saying
why it's unavailable ("needs YubiKey 5.7+"). The Move key row no longer
disappears with the firmware gate — it only needs a key in the active
slot to make sense — and the Delete key button dims in place rather than
collapsing its row.
Co-Authored-By: Claude <noreply@anthropic.com>
Improve cross-vendor support for the non-standard PIV commands keyroost drives. Most PIV management extensions come from Yubico — MOVE KEY and DELETE KEY among them — and a working PIV applet does not indicate support for them: some work across implementations, some don't, and on one product the answer can change between firmware versions. For MOVE KEY / DELETE KEY specifically, keyroost hard-coded a single "YubiKey firmware 5.7+" check and refused the operation on anything else, which both hid the capability from other compatible devices and mislabelled unknown ones as broken. New keyroost-piv::compat module: a per-fingerprint white/blacklist (FingerprintVerdicts holding ascending VersionVerdicts, each Verdict::Whitelisted or ::Blacklisted) keyed by AppletFingerprint, resolved against the applet's reported version into a three-way FeatureGate — Supported, Unverified, Unsupported. resolve() is deliberately conservative about *disabling*: a control is only Unsupported when a blacklist verdict actually brackets the reported version; a lone stale blacklist from an older version, no row for the fingerprint, no reported version, or an applet older than every verdict all resolve to Unverified, which keeps the command usable. PivExtension (MoveKey / DeleteKey) carries a requirement() sentence and FeatureGate two state suffixes, so the GUI and CLI phrase things identically. Seeded only for MOVE KEY / DELETE KEY, from what keyroost has actually verified: on YubiKey they are blacklisted below firmware 5.7 and whitelisted from 5.7 on. Every other applet — and every other extension — has no verdict yet and degrades to a warning, never a block. transport: drop the `>= [5, 7]` version gates inside PivSession::move_key / delete_key, the move_key_supported helper, and the now-unused TransportError::PivFirmwareTooOld. A card that can't run the command refuses the APDU on its own. New PivSession::feature_gate resolves the compat table from a live fingerprint+version probe; it has to run before management-key auth because the probe re-SELECTs PIV. gui: the PIV slot panel resolves the gate from the loaded PivStatus and renders three ways — a live button (Supported), a live button plus a warning marker beside the row's help dot (Unverified), or a dimmed theme::button_disabled (Unsupported) — with a matching foot-of-card summary line per operation. New theme::warn_marker widget. The warning and blocked text is built from the shared compat strings. cli: `piv move-key` / `delete-key` gain `--force`. guard_piv_feature runs the gate before auth (via authenticate_piv, split out of open_piv_authed): Unverified prints a warning and proceeds; Unsupported fails with an error unless `--force` downgrades it to the same warning. Command help and runtime messages reuse the shared compat wording. Co-Authored-By: Claude <noreply@anthropic.com>
The PIV slot pane hid actions whose preconditions weren't met (Move key only appeared with a key in the slot) and let others fail deep on the card — self-sign / CSR / Export / Delete certificate all opened their flow and only then returned a low-level "slot has no key" / "slot holds no certificate" error. Make the whole pane consistent: every per-slot action stays visible, and when it can't run right now the button is dimmed with hover text that says why. - Self-signed -> slot / Sign & save CSR: dimmed when the slot has no key (both are signed by it). - Export certificate / Delete certificate: dimmed when the slot has no cert. Presence comes from status.slots[].cert_present; retired slots and the pre-first-read state have no signal, so they stay live and the on-card error still backstops. - Move key: row is always shown now, not hidden on an empty slot; dimmed with "no key to move" otherwise. - Delete key: left as-is — without GET METADATA key presence is unknowable, and a stale key is still worth an attempt to erase. The unverified-support warning marker shows regardless of slot state (device capability, not slot state). When a button is blocked by both a missing precondition and the fingerprint blacklist, the blacklist reason wins. The old always-visible foot-of-card firmware summary is removed as redundant with those hovers. Co-Authored-By: Claude <noreply@anthropic.com>
Import / Export / Sign & save CSR each had a path text field plus a separate Browse/Save button next to the action button. When the action button was dimmed (no cert to export, no key to sign with) the file picker button was left sitting there, which looked broken, and the split two-step flow was clumsy anyway. Collapse each to a single button. Pressing it opens the native file/save dialog first; the chosen path is routed through `drain_file_dialogs`, which then opens the secret modal (import needs the management key, CSR needs the PIN) or, for export, writes the certificate straight to the path. The path text fields and Browse/Save buttons are gone. The import and CSR modals now show the picked path. Because the save dialog runs its own "replace existing file?" prompt, the in-app empty/clobber checks in piv_request_csr / piv_export_cert are dropped. The "?" help for all three now says keyroost will ask for a file location. Co-Authored-By: Claude <noreply@anthropic.com>
The two single-button sections "Import cert" and "Export cert" become one "Import/Export cert" section with both buttons right-aligned on a single row (Import, then Export). The header carries both operations' help dots. Export still dims when the slot holds no certificate. Co-Authored-By: Claude <noreply@anthropic.com>
The "Valid for" row built its label with a bare ui.label(), so it sat in an auto-width column and the day counter started at a different x than the "Name" text box above (which text_field() puts in a fixed 96px label column). Give "Valid for" the same 96px add_sized label column so the two labels share an indent and both inputs start at the same position. The counter's own width is unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
…le, fuzz the new parsers, string JSON serial Maintainer-side fixes for framefilter#128: - find_tlv_recursive descended without a depth limit (the -1 "unbounded" mode). It runs on the full chained SELECT response, so a hostile or broken card could recurse once per byte and overflow a GUI job thread's stack. Capped at MAX_TLV_DEPTH (4 — one deeper than any real FCI nests), the unbounded mode removed, limit is now u8. Added a pathological-nesting regression test. - parse_serial(&[]) returned Ok(0), so a card answering GET SERIAL with an empty 9000 read as serial 0 rather than unavailable. Empty is an error again. - The new device-fed parsers (ATR historical bytes / identity, select identity, the Nitrokey admin replies, the recursive TLV walk) had no fuzz coverage; folded them into fuzz/piv_parse.rs. - piv status --json emitted serial as a bare u128 number, which loses precision past 2^53 in most JSON consumers. It is a string now (decimal within u64, 0x-hex beyond), matching the text output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpyGDFxYw5DSD97HXBAc2q
framefilter
added a commit
to episource/keyroost
that referenced
this pull request
Sep 11, 2026
…le, fuzz the new parsers, string JSON serial Maintainer-side fixes for framefilter#128: - find_tlv_recursive descended without a depth limit (the -1 "unbounded" mode). It runs on the full chained SELECT response, so a hostile or broken card could recurse once per byte and overflow a GUI job thread's stack. Capped at MAX_TLV_DEPTH (4 — one deeper than any real FCI nests), the unbounded mode removed, limit is now u8. Added a pathological-nesting regression test. - parse_serial(&[]) returned Ok(0), so a card answering GET SERIAL with an empty 9000 read as serial 0 rather than unavailable. Empty is an error again. - The new device-fed parsers (ATR historical bytes / identity, select identity, the Nitrokey admin replies, the recursive TLV walk) had no fuzz coverage; folded them into fuzz/piv_parse.rs. - piv status --json emitted serial as a bare u128 number, which loses precision past 2^53 in most JSON consumers. It is a string now (decimal within u64, 0x-hex beyond), matching the text output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpyGDFxYw5DSD97HXBAc2q
Contributor
Author
|
Thanks for your improvements! I think this PR will still take some time. I'm happy to receive your comments along the way. |
episource
force-pushed
the
feature/piv-fingerprint
branch
4 times, most recently
from
September 12, 2026 09:34
8b27e2b to
032ec90
Compare
The compat table used to carry one version axis, which conflated the PIV
applet's own version with the device firmware version reported over CCID.
Devices whose applet and firmware version independently -- Trussed-based
Nitrokeys most visibly -- cannot be described that way at all.
Split the table into an applet axis and a firmware axis, resolved
separately and combined into a single FeatureGate, and add a second
axis of the same shape for PivQuirk so behavioural quirks no longer have
to be smuggled in as support verdicts. Verdicts now extend backward to
the oldest version that is known to behave the same way, which lets the
redundant per-version sentinels go.
Seeded with the device data that motivated the split:
- Token2: BCD-encoded serial quirk, driving serial decode from the
quirk rather than a hard-coded vendor check; MOVE KEY / DELETE KEY
known-unsupported on applet 5.112.0.
- Swissbit iShield 2 Pro: stuck GET METADATA algorithm byte ignored;
MOVE KEY / DELETE KEY known-unsupported up to applet 1.4.1.0.
- Thetis PRO FIDO2 Security Key with PinPlex: new fingerprint.
Retired-slot occupancy is now decided by actual key material rather than
GET METADATA's status word, so devices that answer GET METADATA with a
blanket error no longer read as "every retired slot is full".
This is the quirk table and Thetis fingerprint framefilter#125's OTP-serial fix
(9bfbfe2) later builds on.
Refs framefilter#125
Co-Authored-By: Claude <noreply@anthropic.com>
Supporting these cards means working across two applets, because the PIV
instance alone does not expose enough to manage them.
On the PIV side, HID Crescendo answers neither GET METADATA nor ATTEST,
so every piece of slot information the rest of the tool takes from those
two commands has to come from somewhere else. The vendor GET PIV
PROPERTIES data object carries it, and both the C2300 and the C4000
families expose it, with a catalogued set of differences beyond the
obvious subtag 0x43:
- the slot's key algorithm, standing in for GET METADATA's algorithm
field;
- a key-init status that decides whether a slot holds a key at all, so
a PKI container that exists but was never generated/injected does
not read as occupied;
- whether 0x9B is exposed as a real slot object;
- the applet version block (a family byte plus a per-segment version).
That last one matters for compat resolution: this fingerprint never
answers Yubico's own GET VERSION extension, so the properties response
is the only applet-version source keyroost has for it. It is not a new
axis -- it feeds the same version axis every other fingerprint uses,
just sourced differently, and it is what the GetMetadata/Attest
known-unsupported rows are keyed on.
Device identification itself is unchanged and does not involve this
read: the ATR names the C2300 and C4000 models, and the SELECT response's
Application Label (tag 0x50, "HID Global ActivID Applet 3.0.3") matches
the generic variant when the ATR does not narrow it. That label also
supplies the applet name, with its embedded truncated version stripped
once GET PIV PROPERTIES has given us the fuller one.
The missing GET METADATA / ATTEST is modelled as a PivExtension rather
than a PivQuirk -- they are absent commands, not behavioural deviations
-- and the same reasoning gates ATTEST and GET METADATA on YubiKey's own
firmware version instead of a vendor-wide assumption.
The second applet is the ACA (Access Control Applet, AID A0000000791000,
partially standardized by NIST GSC-IS 2.1). Most Crescendo units -- every
one this crate has been tested against -- do not model the PIV management
key as a real object at all; a live C2300 answers SW 6D 00 to a standard
GENERAL AUTHENTICATE on 0x9B. A minority reportedly do, so it cannot be
assumed per-fingerprint and is checked per device, which is what the
properties read's 0x9B answer above is for. Where 0x9B is absent,
management-key authentication falls back to the ACA's EXTERNAL
AUTHENTICATE with XAUTH key 1. Per GSC-IS 2.1's access-condition model
that grant is card-wide rather than scoped to the ACA instance, so
re-selecting PIV afterward leaves the session authenticated for the same
admin operations 0x9B auth would have unlocked.
Rotation follows the same route. PivExtension::PinManagementAuth records
that a device unlocks PIV admin operations through PIN VERIFY rather than
the 0x9B round, with a PinManagementAuthProtected9BKey quirk for the
cards where the key itself is PIN-protected. The rotation SELECTs the ACA
AID, VERIFYs the PIN against ACA's own P2 reference rather than the PIV
one, then sends PUT XAUTH KEY. It gains a Delete option, because an ACA
XAUTH key can be removed outright and that is a meaningful state on these
cards; the delete form carries its own algorithm byte and an Lc of 04h,
and that short body is traced unredacted since it holds no key material.
The ACA AID is named in the APDU trace so the extra traffic is
self-explanatory.
GUI side: the new-key hint follows the selected algorithm, the
explanation text spells out what Delete means on both the PIN-protected
and the key-protected branch, and the Change-mgmt-key checkbox no longer
gets stuck on the PIN branch once it has been selected.
Two supporting changes came out of this work: SessionIdentity and
AppletFingerprintResult became named structs instead of tuples, and the
Verdict vocabulary was renamed from Whitelisted/Blacklisted to
KnownSupported/KnownUnsupported, with a new KnownUnsupportedSince for
the common "broken from this version onward" shape.
Co-Authored-By: Claude <noreply@anthropic.com>
Owner
|
Thanks @episource! Two questions:
I pushed 21862e7 with four fixes: find_tlv_recursive was unbounded (a hostile SELECT reply could overflow a GUI job thread) so it's capped at depth 4; parse_serial(&[]) returned Ok(0), now unavailable again; the new ATR/SELECT/Nitrokey parsers are in fuzz/piv_parse.rs; and Open question for you: the fingerprint re-runs on every status()/feature_gate() and each probe re-SELECTs PIV, which drops PIN/mgmt-key auth — safe today only by call ordering. Worth caching it per session? That's a bigger change to the session model with an invalidation-on-reset decision, so I left it to you. #127 lands first and conflicts in GUI main.rs, ui/help.rs and keyroostctl main.rs, so a rebase will be needed. |
Contributor
Author
|
I'm currently working on the Hid Crescendo C2300/C4000 implementation. This still takes some more time, as HID does not mimic Yubikey extensions, but comes up with its own mechanisms. Including ones that require some stretching to fit into the current Keyroost design: e.g. Reset clears more than PIV, but not necessarily everything (for C2300 FIDO is omitted, for C4000 it's included). I've not yet pushed the Hid Crescendo work here. I'll push&rebase once I've gone this step. |
Contributor
Author
I own samples of all of these and test fingerprinting against my sample devices. There might be other device revisions / firmware versions though, that behave differently. I have only few representatives per device, most having equal firmware.
Yes, this is real. I've observed this on my test token. Regarding Thales IdPrime: This PR will likely not go beyond fingerprinting of this device. Full IdPrime support would require some very extensive works on topics like Secure Messaging, that are not yet prepared inside Keyroost. |
Contributor
Author
I think this is the best way to go. |
RESET is the one PIV command we cannot probe safely, so it gets the same fingerprint gating Move/Delete Key already had: a PivExtension::Reset verdict decides whether we take the known-good path, and the quirks that describe how a device wants to be reset live alongside it -- ResetNeedsManagementAuth for cards that authenticate the reset, and the PIN/PUK-block workaround folded into its documentation rather than carried as a separate quirk. Reset is now fingerprinted before it runs, not after, and the PIN/PUK burn is gated on the verdict: force_reset tries a bare RESET first and only burns the retry counters when that is refused, accepting 6983 and 6985 as the blocked-precondition answers. force_reset then grew into factory_reset, a single entry point that plans the whole device wipe instead of leaving the caller to sequence steps. HID Crescendo's ACA RESET CARD is a genuine device-wide reset, so it is modelled as PivExtension::ResetGlobal rather than a PivQuirk on the PIV reset, marked explicitly unsupported everywhere else, and falls back to the PIV-only reset when an unverified global reset fails. After RESET CARD the session reauthenticates to ACA before restoring XAUTH key 1, and GET CHALLENGE sends P2 as 01h on both Crescendo families. GUI and CLI follow: the PIV pane links to the factory reset when Reset is not the known-good path, the confirm dialog names the whole device and drops the vendor-specific "extended global reset" framing, the FIDO finale is logged to the activity log, factory reset flushes its APDU trace and logs a summary, and keyroostctl's factory-reset and piv reset share one set of credential flags. The default management key is itself gated on a fingerprint quirk now. Co-Authored-By: Claude <noreply@anthropic.com>
Add PivExtension::GetSlotKeyStatus: not another read, but the gate that says whether a "this slot holds no key" reading can be trusted enough to block an operation on it. It is the one extension that is not primarily version-gated per fingerprint -- resolve() falls it through to GetMetadata's own verdict whenever a fingerprint carries no explicit row, because GET METADATA's algorithm field is how the capability is provided anywhere it is provided Yubico-compatibly. A fingerprint earns its own row only when some other, independently confirmed channel answers the question: today just HID Crescendo, via GET PIV PROPERTIES, which reports slot key status even though GET METADATA resolves Unsupported there. Two operations stop acting on a guess. Delete key and Import certificate are dimmed only when the slot is *confirmed* empty; an unconfirmed reading leaves them enabled, since a key may have been loaded out of band that this session cannot see. And import_certificate compares the certificate's public key against the slot's known key before sending any APDU, refusing with PivImportCertificateKeyMismatch -- with an algorithm-only secondary check for cards that cannot expose the key itself, and a fall-through to allowing the import whenever either side is unknowable, because absence of information is not evidence of a mismatch. The GUI now seeds that session from its pubkey cache, so a key it generated moments earlier is not treated as unknowable. Supporting this, parse_certificate_public_key extracts real key material from an X.509 SubjectPublicKeyInfo, sharing its TLV walk with the existing SPKI decode. The mismatch messages no longer suggest deleting the key first -- that is the wrong remedy and destroys material the user still needs. On HID Crescendo, MOVE KEY and DELETE KEY are no longer one verdict: MOVE KEY is known-unsupported-since across the Crescendo fingerprints, while DELETE KEY is implementable via INJECT PKI KEY and is now implemented and gated Supported. keyroostctl's import-cert loses --load-pubkey. It was documented as optional and turned out not to be load-bearing at all now that the session is seeded with the key it should check against. Co-Authored-By: Claude <noreply@anthropic.com>
Set PIN/PUK retry counts is a Yubico-shaped extension that not every card implements, so it gets its own PivExtension::SetPinPukRetries verdict. When it is unsupported the GUI blanks and clears the retry count fields instead of showing values it cannot act on. HID Crescendo exposes no PIV-side serial, but the GlobalPlatform card manager does: read the CPLC data and derive the serial from it, before PIV is ever selected. Both the ISD SELECT and the CPLC read are named in the debug APDU trace so the extra traffic is self-explanatory. The decimal-vs-hex serial rendering threshold moves from 64 to 80 bits so CPLC-derived serials print as hex rather than as an unreadable decimal run. The HID-specific fields that had accumulated on PivSession are replaced by a dict-backed applet cache, which also removes the double-caching of the CPLC serial, and the fingerprint cache now holds the full resolution rather than the narrow view the caller happened to ask for first. Co-Authored-By: Claude <noreply@anthropic.com>
The known-support and quirk tables had grown one const per extension and
per table-wide axis, so a single device's data was scattered across a
dozen tables and no one place answered "what do we know about this
card". Restructure into four consts per fingerprint --
<DEVICE>_APPLET_VERDICTS, <DEVICE>_FIRMWARE_VERDICTS,
<DEVICE>_APPLET_QUIRKS, <DEVICE>_FIRMWARE_QUIRKS -- grouped so a
device's complete data set reads as one block. The lookup functions
dispatch by fingerprint straight to the relevant const, and resolve_in /
resolve_quirks_in take the already-selected slice instead of a
table-plus-fingerprint pair.
Pure representation change, verified against the full compat test suite:
every (AppletFingerprint, PivExtension, AppletVersion, FirmwareVersion)
quadruple still resolves to the same FeatureGate as before.
Landing alongside it, in the new layout:
- Token2 5.112.0 known-supported for Reset, SetPinPukRetries and
GetMetadata; Thetis 5.112.0 likewise for GetMetadata.
- Arekinath PIV applet known-supported for MoveKey, DeleteKey,
GetMetadata, Reset and SetPinPukRetries.
- PivQuirk::ResetLongRunning, warning on Swissbit iShield 1's slow
reset so the GUI does not look hung.
Reset handling is tightened at the same time: a
force_reset_if_known_supported entry point threads management-key auth
through every reset path, SW_SECURITY_NOT_SATISFIED (6982) folds into
the precondition check, and the factory-reset dialog requires typing
"reset" like the other destructive confirmations. Generating a new key
now clears the slot's stale certificate.
Co-Authored-By: Claude <noreply@anthropic.com>
resolve_in() returned Unverified outright whenever a version was None, even for an extension pinned at the universal `version: &[]` sentinel whose verdict does not depend on the reported version at all -- every non-HID-Crescendo ResetGlobal row, and the several YubiKey / HID Crescendo rows that are KnownSupported or KnownUnsupportedSince from the start. A fingerprint with no supported mechanism to query a version -- answering neither GET VERSION nor GET PIV PROPERTIES -- therefore showed as unverified on controls whose verdict has nothing to do with versions. Both axes now substitute Some(&[]) when *both* applet and firmware version are None, in resolve() and in resolve_quirks() alike, so a verdict or quirk seeded at the sentinel still applies. When exactly one axis is None it is left alone and still resolves to Unverified on its own: this changes only the "we know nothing whatsoever" case, not "we know one axis". Sixteen existing compat tests plus one transport test had their None/None expectations flip accordingly, with the reasoning recorded at each site, and a new test keeps coverage of the genuinely no-data case (IdPrime, which has no sentinel row to find either). Add PivQuirk::ResetFailsIfManagementKeyIsAes. Arekinath's RESET handler casts the 0x9B key object to DESKey with no else branch for any other key type (arekinath/PivApplet#78), so a card whose management key was switched to AES throws on reset and surfaces a failure status word instead of a completed wipe -- true of every known version, upstream and the Swissbit fork alike. Seeded at the universal version on both Arekinath quirk tables, PivSession::reset now reads the card's current management-key algorithm first and refuses with PivResetManagementKeyMustBe3Des when it is not 3DES, rather than attempting a doomed RESET and, on the force_reset path, burning PIN and PUK retries for nothing. Every other fingerprint skips the extra round trip. Add PivExtension::SetManagementKey, the last of the Yubico-shaped extensions still ungated, and guard both surfaces that write a new management key with it. keyroostctl's change-management-key gains --force to match. Verdicts seeded in this pass: Swissbit iShield 2 for SetPinPukRetries, Reset and GetMetadata; Thetis 5.112.0 for Reset and SetPinPukRetries; Feitian at v0 known-unsupported for Set*, Move/Delete Key and GetMetadata; Nitrokey at firmware 1.8 and 1.8.3. Nitrokey is the case the applet/firmware split was built for, and its rows go on the firmware axis for a concrete reason now written down next to them: the Trussed piv-authenticator hard-codes its reply to Yubico's GET VERSION extension -- the YubicoPivExtension::GetVersion arm comments "make up a version" and answers the literal bytes 06 06 06 -- so every unit, on every firmware and every hardware revision, reports the same dummy "6.6.6" applet version, confirmed against a live unit running firmware 1.8.3. Keying a verdict to that value would apply identically to every Nitrokey ever made, which is what the universal [] row already does. The firmware version, read separately through Nitrokey's own admin application, is the only axis here capable of telling one behaviour from another. AppletFingerprint::UTrust gains Generic/Gov sub-fingerprints, following the established shape of OpenFips201Variant and HidCrescendoVariant. The Gov keys carry documented PIV differences from the general-purpose line, most notably a different default management key; nothing on the wire distinguishes them yet, so classify() still only ever produces Generic and Gov is reserved rather than reachable. Both are seeded known-unsupported for the Yubico-shaped extensions. The Move/Delete key dialogs' firmware note is corrected while in there. Co-Authored-By: Claude <noreply@anthropic.com>
Building on identity caching, add caches for GET METADATA, read certificates, CHUID, and PIN retries, all gated by open_cached()'s existing validity proof, with mutation calls (generate/delete/move key, PIN changes, cert import/clear) evicting or reseeding the affected slot's entries. PIN/PUK/management-key metadata reads and factory_reset's PUK-retries read stay live and uncached, since those counters mutate on nearly every VERIFY/CHANGE/RESET call. Rework the metadata cache into decoded, sharable PubkeyCache and PolicyCache maps in place of the old raw-reply MetadataCache, so every slot-keyed consumer works from already-decoded key material and policy instead of re-decoding a stored reply, with slot_key re-confirming live before signing a CSR/self-signed cert off a cached key. Fix resolve_serial() being called on every status read regardless of applet_fingerprint()'s own identity cache, by having the YubiKey and generic fingerprint arms resolve serial themselves like every other arm already does, removing resolve_serial entirely. Make SELECT PIV lazy: open_cached's own PC/SC identity check is now the sole reuse gate, and a session's first real PIV command issues SELECT itself via ensure_selected() instead of every open paying an upfront round trip. Expose touched_card() so the GUI can log a cache-validated status read distinctly from a live one instead of always claiming a real read happened. Evict pubkey_cache (not just policy_cache) before generate_key's post-generate policy resolve, so a cached pre-generate GET METADATA reply can't force the ATTEST fallback. Reorder PivSessionState's fields so the three open_cached validity fields sit together ahead of the operation-invalidated caches. Co-Authored-By: Claude <noreply@anthropic.com>
PivSession now owns a pcsc::Transaction instead of a bare Card, so every APDU it issues -- from the initial SELECT through whatever the caller does -- runs under one PC/SC transaction that closes reliably via Transaction's own Drop, even on error or panic. pcsc::Transaction borrows &mut Card rather than owning a handle, so a session can no longer be opened and returned for a caller to use across separate statements. open/open_with_debug/open_cached/ open_cached_with_debug are replaced with closure-based with_transaction/with_transaction_debug/with_cached_transaction/ with_cached_transaction_debug, generic over the closure's error type (E: From<TransportError>) so a caller's own error type can still mix freely with `?` on session methods. Updates every call site in keyroostctl's run_piv and the GUI's PIV jobs to the new shape. Co-Authored-By: Claude <noreply@anthropic.com>
The bool only ever gates trace::line() APDU hex dumps, nothing broader (no log level, no breakpoints) — "debug" overstated what it does. Renames field/methods to match what's actually happening: debug -> traced, set_debug -> set_traced, with_transaction_debug -> with_transaction_traced, with_cached_transaction_debug -> with_cached_transaction_traced. Updated all call sites and doc comments in keyroostctl/main.rs accordingly. Scoped to PivSession only, per request — the CLI's --debug flag is unchanged (it's shared UX across every subcommand), and the identical debug: bool/set_debug pattern in OathSession, OpenPgpSession, Token2ProgSession, Token2OtpTransport, and the Molto2 Session in keyroost-transport/src/lib.rs is left as-is for now. Co-Authored-By: Claude <noreply@anthropic.com>
CI on 132f639 flagged several cargo fmt --all --check diffs across main.rs, token2otp, and piv.rs; all but this one had already been resolved by later commits on this branch. Re-wrap the long assert_eq! line so the formatter is happy again. Co-Authored-By: Claude <noreply@anthropic.com>
framefilter
added a commit
that referenced
this pull request
Sep 23, 2026
…oc-link fixes (#150) * TODO: scope v0.11.0 as a small, fast release Fold the #147 compressed-certificate fix into 0.11.0 instead of a 0.10.1 patch, limit the release to three small cleanups plus the audit-rule amendment, and keep #128 from holding it: it lands only if ready in time, otherwise it anchors the next release. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * v0.11.0 cleanups: drop a dead PIV helper, gate changelog fragments in CI, rebuild before the docs check - Remove PivSession::management_key_algorithm. Nothing has called it since the management-key algorithm probe landed; callers use reported_management_key_algorithm or resolve_management_key_algorithm, and the doc links now point there. - Run assemble-changelog.py --check and its self-test in CI, so a malformed changelog.d fragment fails the PR that adds it rather than the release run. - check-docs-mechanical.sh now always builds keyroostctl (a no-op when current) instead of only when the binary is absent, so an old build can no longer validate the docs against a stale --help tree. - CLAUDE.md: findings about device or firmware behaviour need hardware or vendor confirmation before they are written as fact, and vendor features are described neutrally. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * TODO: drop the v0.11.0 cleanups now that this branch carries them Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs: fix broken and private intra-doc links across the workspace `cargo doc -D warnings` failed in seven crates: links to private items (now plain code spans), links rustdoc could not resolve from where the doc is rendered (now full paths), and a bare URL. keyroostctl allows rustdoc::invalid_html_tags because clap turns its arg doc comments into --help text, where placeholders like <group> are literal. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * ci: build the workspace docs with rustdoc warnings denied The library crates' docs are published on docs.rs, where a broken or private intra-doc link renders as a dead or missing link. rustdoc only warns about these, and no CI job built docs, so fourteen of them accumulated unnoticed across four months of changes. Deny them in CI so the next one fails its own PR. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
episource
force-pushed
the
feature/piv-fingerprint
branch
from
September 23, 2026 19:47
0d3a9f1 to
1f957e2
Compare
episource
pushed a commit
to episource/keyroost
that referenced
this pull request
Sep 23, 2026
…le, fuzz the new parsers, string JSON serial Maintainer-side fixes for framefilter#128: - find_tlv_recursive descended without a depth limit (the -1 "unbounded" mode). It runs on the full chained SELECT response, so a hostile or broken card could recurse once per byte and overflow a GUI job thread's stack. Capped at MAX_TLV_DEPTH (4 — one deeper than any real FCI nests), the unbounded mode removed, limit is now u8. Added a pathological-nesting regression test. - parse_serial(&[]) returned Ok(0), so a card answering GET SERIAL with an empty 9000 read as serial 0 rather than unavailable. Empty is an error again. - The new device-fed parsers (ATR historical bytes / identity, select identity, the Nitrokey admin replies, the recursive TLV walk) had no fuzz coverage; folded them into fuzz/piv_parse.rs. - piv status --json emitted serial as a bare u128 number, which loses precision past 2^53 in most JSON consumers. It is a string now (decimal within u64, 0x-hex beyond), matching the text output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpyGDFxYw5DSD97HXBAc2q
Add end-to-end support for Yubico's PIN-protected management-key scheme (PivExtension::PinManagementAuth) when rotating the 9B key: - keyroostctl: `piv change-management-key --allow-pin-unlock` stores the new key PIN-protected (OBJECT_PIN_PROTECTED_DATA / "PRINTED", tag 53:88:89), so `authenticate_management_via_pin` can retrieve it after a bare PIN VERIFY. Omitting the flag clears any existing PIN-protected storage instead. Gated the same way every other extension already is: silent on Supported, a warning on Unverified, refused on Unsupported unless --force -- but only when the flag was actually given. A plain key rotation on a device confirmed not to support this is never blocked or warned about, since it never asked to touch pin-unlock in the first place. - GUI: matching "Allow PIN unlock" checkbox in the management-key dialog, field-aligned with the rest of the form; disabled+forced-on for HID Crescendo (which unlocks management directly off the PIN, with no key material of its own to store); disabled with no warning when Unsupported, available with a warning triangle when Unverified. The "Use PIN" toggle itself now also shows (with the same warning) on Unverified devices instead of only Supported ones. - transport: new PivSession::set_management_key_pin_protected + PinProtectMaintenance return type, so a caller can tell a real failure (confirmed-supported device) from expected background noise (unverified device) and react accordingly instead of hard-failing either way. The Admin Data (OBJECT_ADMIN_DATA) bookkeeping bit ykman-style tooling reads is maintained best-effort -- it's a proprietary Yubico extension no spec obligates every device to carry, and a write failure there never blocks the substantive PRINTED-object write/clear. - Fix authenticate_management_via_pin to branch on the live fingerprint (HID Crescendo vs. everything else) instead of the now- removed PivQuirk::PinManagementAuthProtected9BKey quirk. That quirk only entered at specific *confirmed* applet versions, so any device whose version was unread, unrecognized, or otherwise Unverified fell through as if it were a confirmed direct-unlock device like HID Crescendo: a bare PIN VERIFY was accepted as success without ever reading back the real management key, silently leaving the card unauthenticated for the write that followed. - Add/merge PivExtension::PinManagementAuth (and a few related) verdicts across the Token2, Thetis, Nitrokey, ArekinathPivApplet, OpenFips201::SwissbitIShield2, UTrust, and YubiKey compat tables, from hardware confirmation. - keyroostctl: fix `--old-mgmt-key-*` args scattering across `piv change-management-key --help`, interleaved with the top-level `Cli` struct's own global args -- both structs' clap-derive display orders reset to 0 independently. Explicit display_order groups this command's own args together, globals first. Co-Authored-By: Claude <noreply@anthropic.com>
PIV PIN/PUK entry, the management-key fields (including the new
"New key" row and the factory-reset dialog's PIN/mgmt-key field),
and the sign/CSR/self-test/set-retries PIN fields now carry an eye
icon painted inside the field itself, at its right edge, instead of
a plain permanent mask.
Deliberately not a change to the shared `pin_field`/`secret_field`
helpers used elsewhere (FIDO, OpenPGP, OATH) — this only touches the
PIV pane and the factory-reset dialog, via new `piv_pin_field`/
`piv_secret_field`/`inline_secret_edit` helpers that keep each
field's original `desired_width` untouched (no extra column, so
existing row alignment is unaffected) and widen the field's own
right text-margin by roughly a character so typed text clears the
icon. Each field's reveal state persists independently in the
existing `secret_reveal` map.
Also fixes a pre-existing left-edge misalignment surfaced while
touching these dialogs: Change PIN/Change PUK's three fields, and
Self-sign/Set-retries' PIN + management-key fields, shared a fixed
96px label column that was too narrow for their longest label
("Confirm new PIN/PUK", "Management key") — that label overflowed
into the field next to it, shifting only that one field right of its
siblings. Each dialog's label column is now sized to its own widest
label via a new `label_text_width` helper (generalized from the
existing `chuid_label_width`), so every field's left edge lines up.
"Change management key" also gains a "Generate random key & Copy"
action below the "New key" field: it fills the field with a fresh
random key sized for the selected management-key algorithm
(`keyroost_transport::random_management_key`, host-side only, no
card I/O), copies it to the clipboard, reveals the field (the key is
already in plaintext on the clipboard, so hiding it in the field buys
nothing), and auto-clears the clipboard after 45s like the OTP-code
copy path. The link and the "Allow PIN unlock" checkbox below it are
centered rather than lined up under the field's label column, since
they read as standalone actions, not a labeled field.
That checkbox row needed its own fix once centered: `ui.horizontal`
always pre-claims the full available width up front, so
`vertical_centered` — which only centers a row by re-centering its
*claimed* size — had nothing left to center whenever the row held
more than one widget (the checkbox plus its conditional "unverified"
warning icon). The single-widget rows (the link above, and the
HID-Crescendo forced-checked variant of this same checkbox) never
hit this because they never call `ui.horizontal`. Fixed by measuring
the row's actual rendered width from the previous frame and padding
it into the middle by hand.
Co-Authored-By: Claude <noreply@anthropic.com>
Adds PivExtension::ManagementKeyAlgorithm(MgmtAlgChoice) and seeds known-support data for it, mirroring the existing SlotKeyAlgorithm gate: Yubikey/Token2/Swissbit iShield 2/Thetis/both ArekinathPivApplet variants/OpenFips201::Generic all support 3DES/AES-128/AES-192/ AES-256; HidCrescendo::* additionally supports removing the key outright (MgmtAlgChoice::Delete) but only 3DES/AES-128 among the real algorithms; every other fingerprint has Delete seeded KnownUnsupportedSince; Nitrokey's AES-128/AES-192 support is firmware-gated at 1.8.3. Each new verdict row shares an existing ExtensionVerdicts entry wherever a sibling extension in that table already resolves at the same version profile, rather than adding a redundant entry with an identical verdicts slice: the four real algorithms join whichever row already carries a universal `[]` KnownSupported verdict (Token2 and Thetis have none — their own Supported row sits at a specific applet version — so those two keep a standalone entry); Delete joins whichever row already carries a universal `[]` KnownUnsupportedSince verdict, including each generic-shaped fingerprint's PivExtension::ResetGlobal entry, keeping those tables' existing "single-entry" shape. HID Crescendo's three variants and Trussed NitroKey's firmware-axis table follow the same join-by-verdict rule. Wires this into the GUI's management-key algorithm drop-down the same way the slot key-algorithm drop-down already works: every candidate is always listed (Delete now shown, disabled, on every device instead of only ever appearing for HID Crescendo), each option individually disabled when Unsupported, and an aggregate warning triangle for Unverified options. Also fixes two related bugs surfaced along the way: - both drop-downs used to fall back to a fixed default (Aes192 / EccP256) when the current selection became Unsupported, even when that very default was the disabled entry, leaving a disabled option preselected; both now fall back to the first actually-selectable candidate. - the per-algorithm "unverified" warning could fire even while SetManagementKey itself was Unsupported (drop-down already fully disabled), which is now suppressed since the blocked-hint tooltip on the disabled controls already covers it. Also increases disabled-row dimming in these combos: egui's default disabled_alpha (0.5) left a gated row only faintly lighter than a pickable one at this popup's font size. PIV_COMBO_DISABLED_ALPHA (0.32) is now set on each combo popup's own Ui — key-algorithm, management-key-algorithm, and (pre-emptively, for when it gains gating too) the PIN/touch policy combo — leaving other disabled widgets in the app untouched. Also reorders PivMgmtAlgSel::ALL alphabetically by label (3DES, AES-128, AES-192, AES-256, Delete) instead of the ad-hoc order it had before — Delete still lands last, same as before, since it happens to sort after all four algorithm names anyway. ALL[0] is now TripleDes rather than Aes192, which decouples it from PivMgmtAlgSel::default() (still Aes192, unchanged — the two used to coincide, so this needed a regression test, piv_mgmtalg_default_stays_aes192_despite_..., to pin the default explicitly now that a reshuffle of ALL can't drag it along implicitly) and from piv_mgmtalg_unsupported_fallback's own options[0]-is-disabled fallback case, both updated accordingly. Co-Authored-By: Claude <noreply@anthropic.com>
Mark SetManagementKey, SetPinPukRetries, MoveKey, DeleteKey, and Reset as KnownUnsupportedSince for OpenFips201::Generic and IdPrime: neither vendor mimics these Yubico vendor-extension APDUs, each ships its own proprietary commands instead, none implemented in keyroost yet. Deliberately no SlotKeyAlgorithm verdicts for OpenFips201::Generic: upstream OpenFIPS201 documents a different algorithm set per release line (v1/v1.10/v2), but this fingerprint has no version-identification mechanism, so a version-gated verdict would assume a release line no evidence actually pins a live unit to. Co-Authored-By: Claude <noreply@anthropic.com>
Add known-support verdicts for applet v6, hardware-observed on a live v6.0.1 unit: applet v6 mimics the YubiKey PIV extension set close to completely. KnownSupported, floored at [6] (the whole v6 lineup is assumed to share this, not just the exact v6.0.1 build tested): SetPinPukRetries, SetManagementKey, DeleteKey, MoveKey, GetMetadata, PinManagementAuth, SlotPinPolicy, SlotTouchPolicy, Reset, SlotKeyAlgorithm(RSA-1024/2048, P-256/P-384), and ManagementKeyAlgorithm(3DES/AES-128/AES-192/AES-256). KnownUnsupported, kept at the exact tested [6, 0, 1] rather than widened the same way, since nothing suggests the rest of the v6 lineup shares an absence: SlotKeyAlgorithm(RSA-3072/4096, P-521, Ed25519, X25519). Co-Authored-By: Claude <noreply@anthropic.com>
Hardware-observed on a live IdPrime unit, no applet version read for any of the following: - The card's step-2 mgmt-key mutual-auth reply carries the right encrypted host challenge, but under tag 0x80 (the witness tag from step 1) rather than the spec's 0x82 -- a response-side quirk, not a request-side one. Add PivQuirk::HostChallengeResponsePermissiveTag, seeded for IdPrime at the universal version floor. keyroost_piv::parse_general_auth_permissive accepts whichever tag the reply's sole TLV element carries, refusing to guess only when more than one element is present. authenticate_management checks the resolved quirk set and switches to the permissive parser for fingerprints that carry it, falling back to the strict 0x82 check everywhere else. - PivExtension::SlotPinPolicy/SlotTouchPolicy/PinManagementAuth resolve Verdict::KnownUnsupported at the universal `[]` floor. Plain KnownUnsupported rather than KnownUnsupportedSince: unlike the Yubico vendor-extension row already on this fingerprint, pin/touch policy are standard SP 800-73-4 GENERATE ASYMMETRIC KEYPAIR tags (0xAA/0xAB), so there's no standing reason to expect IdPrime never adds them. PinManagementAuth joins the same row for the same reason: the indirect PIN-unlock mechanism it gates reads a standard SP 800-73-4 object (OBJECT_PIN_PROTECTED_DATA), not a Yubico vendor extension, and this unit's PIN VERIFY doesn't unlock it either. With no bracketing KnownSupported entry above the row, this floor only forces Unsupported for a query that resolves to version `[]` itself; an actually-reported non-empty version softens to Unverified instead of staying blocked. Narrow this once a version-tagged report comes in. - PivExtension::SlotKeyAlgorithm(KeyAlg::Rsa2048) resolves Verdict::KnownSupported at the same universal `[]` floor: the same live unit's GENERATE ASYMMETRIC KEYPAIR accepts RSA-2048. Gets its own ExtensionVerdicts row rather than joining either row above -- neither the KnownUnsupportedSince Yubico-extension row nor the KnownUnsupported pin/touch-policy row carries the right verdict for it. No other algorithm has been probed on this fingerprint yet. - PivQuirk::Default9bManagementKey(IDPRIME_AND_UTRUST_GOV_DEFAULT_MGMT_KEY): the unit's factory-default 0x9B key is 01 02 03 04 05 06 07 08 repeated twice (16 bytes, matching the AES-128 algorithm the earlier mgmt-key trace selected) -- the exact same pattern already seeded for UTrust::Gov, so the constant is renamed (UTRUST_GOV_DEFAULT_MGMT_KEY -> IDPRIME_AND_UTRUST_GOV_DEFAULT_MGMT_KEY) and reused rather than duplicated under IdPrime's own name. Updates the compat.rs tests that used IdPrime as their "carries no quirk data / no seeded default management key" examples (no longer true) to OpenFips201::Generic instead, plus the two matching GUI tests in keyroost/src/main.rs. Extends the SlotPinPolicy/SlotTouchPolicy bracketing-floor test to also cover PinManagementAuth, and adds a dedicated test for the new SlotKeyAlgorithm(Rsa2048) row. Co-Authored-By: Claude <noreply@anthropic.com>
Feitian's live v0 unit: RSA1024/RSA2048/EccP256/EccP384 accepted on GENERATE ASYMMETRIC KEYPAIR, RSA3072/RSA4096/EccP521/Ed25519/X25519 rejected, PinManagementAuth rejected too. Merged into the existing KnownUnsupported @ [0] row where the verdict/version already matched; new row only for the KnownSupported quartet. UTrust::Generic's live unit (can't report a version): RSA1024/RSA2048 accepted, every other algorithm plus PinManagementAuth rejected — unlike Feitian, ECC is rejected outright, not just the P-521 tail. SlotPinPolicy/SlotTouchPolicy join the same KnownUnsupported @ [] row too — same live unit's GENERATE ASYMMETRIC KEYPAIR rejects both the 0xAA and 0xAB tags. Same merge-where-possible treatment against the existing @ [] row throughout. Co-Authored-By: Claude <noreply@anthropic.com>
episource
marked this pull request as ready for review
September 26, 2026 00:41
Contributor
Author
|
@framefilter This PR is ready for review now. I've implemented fingerprinting, tested against my collection of test devices (YubiKey, Token2, Thetis, Nitrokey, Feitian, Utrust, Authentrend ATKey, IDPrime, Hid Crescendo C2300, Swissbit IShield 1+2, Cryptnox PIV Card / OpenFIPS 201 v2). For HID Crescendo I've gone beyond this by already implementing device specific management functionality to give an example for extended use of fingerprinting. See PR description for full abstract of changes and things left for later work (this PR has gone big enough already). |
The docs (rustdoc, deny warnings) CI job denies rustdoc warnings because a private or unresolved intra-doc link renders as dead on docs.rs. The recent PIV fingerprint/quirk-table work added many doc comments on public items that linked to private helpers, caches, and tables (e.g. PubkeyCache, PolicyCache, Self::identity, applet_verdicts, MAX_EXPIRATION_YEAR) — de-linked those to plain code spans since the reader loses nothing by not following a link to something docs.rs can't render anyway. A smaller set were genuinely broken paths: PivExtension/PivQuirk/ FeatureGate referenced unqualified where not in scope, Self::feature_gate used from the wrong type's doc, a stray "[link]: RESET" line accidentally parsed as a markdown reference-link definition, and a few GUI doc comments naming things that don't exist (PivState::use_pin, PivSession::open) — fixed by qualifying paths or pointing at the real name. Also ran cargo fmt to fix an unrelated import-wrapping drift in keyroost-transport/src/lib.rs flagged by the same pipeline. Co-Authored-By: Claude <noreply@anthropic.com>
Owner
|
Thanks for all the work on this, and for testing across so many devices. One concern I do have: I don't want keyroost to deny a feature a device might actually have, since at least from what I can tell the table could be wrong for edge cases like newer firmware or a misidentified card. I prototyped a per-key override on top of your branch: one line on the PIV card, "N features might not work on this key. Enable Anyway", which turns every "unsupported" into "unverified" for that key until the app closes, except RESET. It's on the branch proto/piv-compat-override in our repo if you'd like to look or pull it in. Would you be open to building that or a similar feature into #128? I'm not sure I exactly nailed it first try, and I do want to be respectful of the enormous amount of work you put into this, but I have concerns about upkeep on security key feature compatibility matrices. Let me know if the changes I made work for you, or if you think there's a better approach. |
Owner
Contributor
Author
|
Thanks for your feedback and holding back #154. A feature to override The CLI already got |
Owner
|
@episource thanks for being open to my suggestion. I saw that the CLI had an override feature, which is what I tried to mimic in the GUI. Again, not sure I was successful at nailing a good user experience on my first attempt, but it at least gives us a starting point to discuss and/or iterate on. And again, no rush on this. |
The PIV compatibility table greys out controls for features it lists as unsupported on a device. It can be wrong or incomplete for a given key, so the GUI now offers a per-key override: a line at the bottom of the PIV tab, below the Reset applet card, reads "N PIV features are marked unsupported for this key." with an "Unlock Anyway" link. Once unlocked it reads "Unsupported PIV features are unlocked for this key." (in the warning colour) with an "Undo" link. The choice lasts for the app run only and is never persisted. Unlocking downgrades the table's "unsupported" verdict to "unverified" for the pane's controls (key algorithms, move/delete key, PIN/touch policy, management-key algorithms, set retries, ...), so they work with the usual untested-feature warning. A device that really lacks a feature just refuses the command and nothing changes. Not overridable: the internal reads GET METADATA, ATTEST and GET SLOT KEY STATUS (not controls; they keep resolving from the real table), removing the management key (HID Crescendo's own command, not a feature another device might merely be untested for), and RESET with the device-wide reset. The count shown counts only what can actually be unlocked. RESET is left out on purpose: it is potentially destructive, and a UI unlock would attract people to just try it. Reaching RESET means exhausting the PIN and PUK first, so a failed attempt on a device that does not support it can leave the applet locked for good. Users who really need to unlock a known-unsupported RESET must use the CLI's --force, which is scoped per command and has no such exclusion. The extra effort is intended: only people who know what they are doing should go that way. We don't want anyone to brick their device. The override is UI-only: keyroost-piv and keyroost-transport are unchanged, and the CLI keeps its separately scoped --force. Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: framefilter <framefilter@proton.me>
Contributor
Author
|
@framefilter I've adjusted your proposal slightly (b2e7531):
Is this fine for you?
|
episource
force-pushed
the
feature/piv-fingerprint
branch
from
October 2, 2026 13:03
9845123 to
b2e7531
Compare
framefilter
added a commit
that referenced
this pull request
Oct 4, 2026
After the rebase onto #128, the Compression label sat outside the label column #128 gives the management-key and PIN rows. Size it to the same column (96px for Import certificate, the measured width for Self-signed). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU
framefilter
added a commit
that referenced
this pull request
Oct 4, 2026
) * piv: let encode_certificate write the gzip CertInfo as well Writing a compressed certificate needs the object's CertInfo byte to say so. encode_certificate now takes a CertInfo (Uncompressed = 0x00, Gzip = 0x01, the two values SP 800-73-4 Part 1 Appendix A defines) instead of hard-coding 0x00. The byte layer still has no compressor: the caller passes the gzip member as the payload. Known-answer tests pin both forms, short and long 0x70 lengths, and the round trip through cert_object_parts. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * piv: add a gzip writer for compressed certificate objects Storing a certificate compressed needs one RFC 1952 member to put in the object. gzip_member sits next to the reader and reuses its CRC-32: a fixed header (no optional fields, MTIME 0, so the same certificate always gives the same bytes), a level-9 DEFLATE body from the miniz_oxide dependency the reader already uses, and the CRC-32 / ISIZE trailer. Tests pin the header and trailer, round-trip through the CRC-checking reader, check determinism, and check a repetitive 6 KB input shrinks well below 3 KB. A member written this way was also confirmed readable by CPython's gzip module and by gzip -t. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * piv: choose whether an imported certificate is stored compressed Some certificates are larger than a card will take in one object. The PIV standard's answer is the gzip form (CertInfo 0x01), so import_certificate now takes a CertCompression: Auto (the default) stores the certificate uncompressed and, only when the card refuses it as too large, stores it compressed once instead; Always compresses; Never does not. There is no per-device size table: the card's own length refusal is the only signal. The call returns a CertImport saying what was stored, so the CLI and GUI can tell the user when Auto compressed and why. Every compressed write is read back and must return the exact DER that was written, else PivCertReadbackMismatch. A too-large refusal now says whether compression was tried: without it, that storing the certificate compressed may make it fit; with it, both sizes and that it does not fit even compressed. The PUT DATA / chaining body moves unchanged into put_cert_object so both encodings share it, and self_signed_certificate passes the choice on and returns the CertImport with the DER. Slot status gains cert_compressed for a certificate stored gzip-compressed that inflates cleanly (cert_len stays the DER length). The CLI and GUI pass Never for now, keeping today's behaviour until they expose the choice. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * keyroostctl: --compress / --no-compress for piv import-cert and self-sign The CLI now uses the transport's compression choice instead of pinning it off. With neither flag a certificate is stored uncompressed and, only if the card refuses it as too large, compressed; --compress always compresses and --no-compress never does (the two conflict). The help says what the flags do, that the compressed form is the PIV standard's gzip, and that some software may not read compressed certificates. A compressed import adds "(stored compressed: N bytes on the card)" to the success line. When the automatic choice compressed, a note says it did not fit uncompressed and that support in the Windows and macOS built-in PIV support has not been verified. A too-large refusal under --no-compress also names the flags that would let it fit. piv status marks a compressed certificate ("stored compressed"), and its --json slot gains cert_compressed, present only when true. import-cert and self-sign have no --json output today, so none was added for them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * keyroost: Compression choice in the certificate import and self-sign dialogs The GUI gets the same choice the CLI has. The Import certificate and Self-signed certificate dialogs gain a Compression dropdown: Automatic (only if too large), the default and reset per dialog; Always; Never. The help under it matches the CLI's: compressed is the PIV standard's gzip form, some software may not read it, and Automatic compresses only when the card refuses the certificate as too large. The notice and the dialog's success detail mirror the CLI output: the stored size when compressed, and the same note when Automatic had to compress (it did not fit uncompressed; support in the Windows and macOS built-in PIV support has not been verified). A too-large refusal with Never set points at the dropdown. The selected slot's state line reads "certificate present (ECC P-256, compressed)" for a compressed certificate; that line's wording moves into a small tested function. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * docs(piv): explain compressed certificates and the compression choice The Learn page now says what a compressed PIV certificate is (the standard's gzip form, flagged in the object's CertInfo byte), when keyroost writes one (automatically only when the card refuses the certificate as too large, or on request with --compress, never with --no-compress), that the card's refusal is the only size signal and every compressed write is read back, and where the same choice is in the desktop app. A note lists the readers confirmed to handle compressed certificates and states that support in the Windows and macOS built-in PIV support has not been verified. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * changelog: PIV certificate compression Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * piv: note the community-tested Windows built-in driver in the compression notes A tester on Windows 11 (#152) found that Windows' built-in PIV smart-card driver read a certificate keyroost stored compressed. The CLI/GUI note, the Learn page and the changelog now say so; macOS's built-in PIV support stays marked as not verified. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * TODO: AppImage-only republish input and a portability check in CI Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * TODO: make a holistic GUI design pass the v0.12.0 goal Folds the narrow-window/high-zoom overlap item into it, with the AppImageHub screenshot as the concrete case. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * metainfo: show a screenshot in software centers and AppImage catalogs The AppStream <screenshots> block was a commented-out placeholder pointing at an image that never existed. Enable it with the device-view screenshot the Learn site already hosts (WebP is allowed by AppStream). Software centers reading the Flatpak and AppImage metadata, and AppImage catalogs, can show it from the next release. Fresh screenshots are part of the v0.12.0 design pass (TODO.md). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * RELEASING: check the AppStream metainfo is current before each release The metainfo is the app's store page in software centers and AppImage catalogs, and travels inside the Flatpak and AppImage. Add it to the semantic audit inventory, plus a checklist item: appstreamcli validate, screenshot URLs load and show the current UI, and the summary, description and links match the release. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * RELEASING: check every published screenshot shows the current UI One item for all pictures of the app: the Learn site's screenshots and social card, the README (none today), and the metainfo store-page screenshots, retaken from the release build when the GUI changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * TODO: Windows/macOS duplicate keys with two identical keys (#51) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * keyroost: line the Compression row up with the dialog's label column After the rebase onto #128, the Compression label sat outside the label column #128 gives the management-key and PIN rows. Size it to the same column (96px for Import certificate, the measured width for Self-signed). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
framefilter
added a commit
that referenced
this pull request
Oct 4, 2026
…skips (#161) * piv: let "Enable Anyway" also send the reads the compatibility table skips #128's override relaxes the PIV pane's controls but left the transport's internal reads (GET METADATA, ATTEST) gated by the table. A wrong entry there, e.g. for newer firmware, silently hides information (algorithms, PIN/touch policy) with no way to get it back and no trace of why. - PivSessionState carries a send-unsupported-reads choice, kept across a card change; the GUI sets it from "Enable Anyway" for that key and re-reads status when it is toggled. RESET is never overridden. - Every internal read the table skips, or sends anyway under the override, leaves a --debug / activity-log trace line naming the applet fingerprint, so a wrong entry can be spotted. - "Unlock Anyway" is now "Enable Anyway" (and "…are enabled for this key"); the rest of the wording is #128's. - CLAUDE.md lists keyroost-pivtest and its RustCrypto/dalek dependencies, including the new p521. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU * keyroostctl: --force also sends the reads the compatibility table skips Parity with the GUI's "Enable Anyway": a PIV command run with --force (generate-key, change-management-key, reset, delete-key, move-key) now also sends GET METADATA and ATTEST for the rest of that command when the table lists them unsupported, each with a --debug trace line. Done once in guard_piv_feature, which every forced command calls first. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Keyroost now identifies which PIV applet a card is actually running — YubiKey, Token2, Nitrokey, uTrust, HID Crescendo, OpenFIPS201, and several others — mostly based on the card's ATR and PIV SELECT response, plus, if needed, a few cheap follow-up probes.
The feature-gating approach takes care to disable extension features only when they are explicitly known to be unsupported (
KnownUnsupported; e.g. Move Key prior to YubiKey 5.7). These features are still shown in the UI, but disabled. Otherwise, they are shown as unverified, with a warning. When explicitly whitelisted (KnownSupported), no unverified warning is shown. This also led to some cleanup of the PIV module UI. Both applet and firmware versions (if a device differentiates between them) are supported when defining feature gates; e.g. Nitrokey reports a fixed dummy applet version across fundamentally different firmware releases.Some widespread devices mimic YubiKey extensions for device management (e.g. Arekinath's PivApplet, Swissbit, Token2, Thetis, Nitrokey), while others do not. As an example of how fingerprinting can be used for more advanced device-specific handling, this PR implements HID Crescendo-specific management functionality, while postponing further device-specific implementations (e.g. OpenFIPS201 v2 / Cryptnox, Thales/Gemalto IDPrime) to follow-up PRs.
Pending work:
ECC P-384unconditionally #113 (Swissbit's GET METADATA algorithm and PIN/touch-policy info can be stale/unreliable)Postponed for follow-up PR: