feat: add composite YubikeyDevice (v2)#491
Draft
DennisDyallo wants to merge 31 commits into
Draft
Conversation
Move DeviceInfo, DeviceCapabilities, DeviceFlags, FormFactor, and VersionQualifier/VersionQualifierType from Yubico.YubiKit.Management to Yubico.YubiKit.Core.YubiKey, along with the internal CapabilityMapper parser support. Management retains device configuration, reset, and session ownership and now returns the Core DeviceInfo type. Migrate mandatory compile consumers (Management, Tests.Shared, CLI shared/commands, PIV and Management examples) to the Core namespace, relocate CapabilityMapperTests into Core, and add Core DeviceInfoTests covering required TLV parsing, version qualifier behavior, and malformed input handling. Phase 34 of the composite-device program (ISC-8, ISC-9, ISC-9.1, ISC-9.2, ISC-10, ISC-11, ISC-12, ISC-25, ISC-25.1, ISC-31.1).
Bring the tree into compliance with the repository .editorconfig (import ordering, final-newline, and whitespace rules). dotnet format was not previously CI-enforced, so formatting had drifted across modules. No behavioral changes; solution builds clean. Run via: dotnet format
Add an internal Yubico.YubiKit.Core.YubiKey.DeviceInfoReader that reads read-only DeviceInfo over SmartCard (APDU 0x1D), FIDO HID (CTAP vendor 0xC2), and OTP HID (slot 0x13 + CRC), with multi-page sequencing, length validation, and OTP CRC handling. ManagementSession.GetDeviceInfoAsync now delegates to it via an InternalsVisibleTo grant; the Management backends become mutating-only (write config, set mode, reset). Core keeps no dependency on Management. Move device-info read tests into Core (fake-protocol coverage for all three transports, multi-page, malformed length, and CRC failure). Phase 35 of the composite-device program (ISC-3..ISC-13, ISC-14..ISC-18). Cross-vendor DevTeam review waived: the GPT-5.5 reviewer is rate-limited; recorded per the Phase 35 ISA waiver allowance. Review: cross-vendor DevTeam (GPT-5.5) unavailable due to rate limit; waiver recorded in phase-35 learnings, no same-family substitution.
… test GPT-5.5 (DevTeam/Cato opposite-family reviewer) is rate-limited. Add scripts/interim-cross-vendor-review.sh, a read-only GitHub Copilot CLI wrapper (GPT-5.4, high reasoning, --deny-tool=write) used as an interim opposite-family reviewer for an Anthropic-family primary, and document the workflow, parameters, and queue-GPT-5.5 policy in the composite device ISA. The interim review of Phase 35 returned PASS WITH NOTES; apply its one finding by replacing the validation-only ReadAsync_NullProtocol test with ReadAsync_DefaultVersionProvided_OverridesFirmwareVersionTlv, a behavior regression covering defaultVersion passthrough to DeviceInfo.CreateFromTlvs. A proper GPT-5.5 review of commit c36bec2 is queued for when quota returns.
Replace the scalar IYubiKey.ConnectionType (one interface per device) with AvailableConnections (a concrete-bit ConnectionType set) plus a default SupportsConnection predicate and an ambiguity-safe default ConnectAsync() that resolves only when exactly one connection is available and otherwise throws. Correct the filter-matching helper to compare a filter against a combined capability set (the old MatchesDevice HasFlag logic was wrong for multi-bit sets). Migrate every production scalar consumer (discovery, repository, ConnectionTypeMapper, CLI selectors, example tools, Tests.Shared). The FIDO2 extension gets a mechanical, preference-free transport selection (both FIDO-capable transports present -> throw, deferring explicit selection to Phase 38). Device-info access on IYubiKey and composite discovery merge remain deferred to Phase 37. Phase 36 of the composite-device program (broad API-boundary). Interim Cato (pre-edit, converged) and DevTeam (PASS WITH NOTES, fixed) reviews ran via GPT-5.4; GPT-5.5 reviews queued. BREAKING CHANGE: IYubiKey.ConnectionType is removed; use AvailableConnections / SupportsConnection / ConnectAsync<TConnection>(). Review: interim GPT-5.4 (Cato + DevTeam); GPT-5.5 reviews queued per ISA workaround.
…evices Phase 37 of the composite-device program. Discovery now returns one logical IYubiKey per physical USB YubiKey: the CCID, FIDO HID, and OTP HID interfaces of the same key are merged into a CompositeYubiKey, keyed by the application serial number read during discovery via the Core DeviceInfoReader. - CompositeYubiKey unions member connections and routes typed ConnectAsync<T> to the member exposing the requested connection; it owns no long-lived connection. - CompositeDeviceMerger is a pure, deterministic merge over per-interface descriptors: USB interfaces sharing a non-null serial merge; NFC, unknown PC/SC kind, and null/unreadable serial never collapse (conservative). - DiscoveryIdentityReader reads serial over a short-lived per-interface connection, gated to >1 USB interface, cached per interface with eviction and retry on transient PC/SC sharing violations. Core stays free of any Management dependency. - FindYubiKeys enumerates both transports and filters the merged set; discovery is serialized to avoid concurrent-scan sharing violations. - YubiKeyDeviceRepository keys by physical identity and emits Removed+Added when a present device's available connections change. - Merge-safety gate (master ISA line 247): the two parameterless ConnectAsync() consumers (Management, YubiOtp) resolve a concrete transport via the new IYubiKey.ResolvePreferredConnection (Management SmartCard->HidFido->HidOtp, YubiOtp SmartCard->HidOtp). Full smart-default/override policy stays Phase 38. - Tests.Shared filters with set-correct SupportsConnection and separates the requested transport from the device's available set. Verified: full build 0/0; Core unit suite 458 (0 failed) incl. new merge/composite/resolver/repository tests; hardware smoke on a composite key (serial 103, FW 5.8.0 beta) returns one merged device and opens all three typed transports. Known limitation: a process holding the CCID exclusively (e.g. GnuPG scdaemon) makes the CCID read fail and that interface degrades to a separate device (safe no-merge). Satisfies master ISC-16, ISC-17, ISC-18, ISC-19, ISC-20, ISC-27. Review: interim GPT-5.4 Cato (ISA, resolved) + DevTeam (impl, PASS); GPT-5.5 reviews queued.
Phase 37 follow-up. Document the two known limitations of the serial-based merge and the concrete Rust-reference remediations, and add explicit test coverage for serial-less keys. - Known Limitation 1 (exclusive CCID holder): a process holding the CCID exclusively (e.g. GnuPG scdaemon) makes the discovery serial read fail, so that interface degrades to a separate device. Records how the Rust reference avoids this (single preferred-transport read with CCID->OTP->FIDO fallback; exclusive->shared fallback; kill scdaemon/yubikey-agent and retry). - Known Limitation 2 (serial-less / SKY series): single-interface SKY keys are handled (one device); serial-less multi-interface keys do not merge (conservative no-collapse). Records how Rust merges on PID-from-reader-name instead of serial, which handles serial-less keys. - Recommended remediation (Cato-gated follow-up): adopt the Rust PID-from-reader-name merge model, which addresses both limitations and lowers discovery cost. - Tests: add Merge_SeriallessSingleInterface_SkyStyle_PassesThroughAsOneDevice and Merge_SeriallessMultiInterface_DoesNotMerge_ConservativeNoCollapse.
Phase 37.5 of the composite-device program. Replace the Phase 37 serial-only
merge (open every interface, group by serial) with the Rust reference's
PID-from-reader-name model, fixing both Phase 37 limitations recorded in
phase-37-composite-discovery-learnings.md.
- ReaderNamePidParser derives a YubiKey USB PID from the PC/SC reader-name
capability string (case-insensitive; OTP/FIDO/U2F/CCID; NEO variant) and
gates on a known-Yubico-PID set; HID interfaces use the descriptor ProductId
only when > 0 and known (no PID-0 over-merge).
- CompositeDeviceMerger correlates by PID: a key whose PID is present once
merges its CCID+FIDO+OTP with ZERO opens, so the CCID is included even when
another process (e.g. GnuPG scdaemon) holds it exclusively, and serial-less
keys (SKY series) merge correctly. Serial only disambiguates multiple
same-model keys (conservative no-collapse when serial-less). NFC/null-PID
stand alone. An unparsed USB CCID name degrades the scan to serial-based
merge so it rejoins its HID siblings rather than fragmenting.
- FindYubiKeys reads serial only for PID-count > 1; DeviceInfo is read
best-effort via a separate bounded, no-retry, concurrent metadata pass
(CCID -> OTP -> FIDO) that never blocks the merge. ProtocolDeviceInfo is the
shared connection -> DeviceInfo reader. Composite DeviceId is
ykphysical:pid:{pid} or ykphysical:{serial}.
The SDK never kills scdaemon/yubikey-agent (unlike the Rust CLI reference).
Verified: full build 0/0; Core unit suite 486 (0 failed), incl. a unit test
proving a single key merges even when every ConnectAsync throws. Hardware
(serial 103, FW 5.8.0 beta): FindAll merges all three interfaces by PID while
scdaemon holds the CCID (the exact Phase 37 failure, now fixed), and the full
connect smoke passes with the CCID free.
Supersedes the Phase 37 merge mechanism; Phase 37 public criteria remain
satisfied (master ISC-16/17/19/20/27).
Review: interim GPT-5.4 Cato (ISA, BLOCKED->PASS) + /DevTeam (impl, PASS WITH NOTES); GPT-5.5 reviews queued.
Persist the post-Phase-37.5 comparison of the .NET SDK against the Rust yubikey-manager reference across discovery, merging, connectivity, and robustness, with the remaining connectivity/lifecycle gaps (held-CCID session fallback, reinsert, device-info synthesis) and their dispositions.
Upstream experiment/rust HEAD d9a77abb is keys/PIV/OpenPGP/MSI/test work only; the discovery/merge/connectivity logic (merge_devices, pcsc::open, reinsert) is unchanged, so the comparison still holds. Record the verification stamp.
… 38.5 Record the directional shift in the master composite-device ISA: Phase 38 keeps extension smart defaults plus explicit overrides (ISC-21..24), and held-CCID transport fallback moves to a new Phase 38.5 (ISC-23.1), mirroring the Phase 37 -> 37.5 split. Adds a changelog entry, a dated decision, ISC-23.1, the Phase 38.5 features row and narrative, and repoints Phase 39 onto Phase 38.5.
Phase 38 ISA scoped to master ISC-21..24: optional ConnectionType? preferredConnection override on Management/YubiOtp/Fido2/WebAuthn, per-app default orders (Fido2/WebAuthn HidFido->SmartCard replacing the Phase 36 placeholder throw), single-transport applets unchanged, fake-probe transport tests, and docs. Held-transport fallback and SCP-implied transport selection are explicitly out of scope (38.5 / deferred). Interim cross-vendor Cato gate (GPT-5.4 high) run on the ISA before any source edits: converged to PASS over 10 rounds; GPT-5.5 Cato queued.
…overrides Formalize applet session-entry transport selection on the composite physical IYubiKey: an app-specific smart default order plus an optional explicit ConnectionType? override. - Core: add YubiKeyConnectionExtensions.ResolveSessionTransports — returns the ordered, non-empty candidate list to attempt (single element for an override; device-supported subset of the default order otherwise). Override taxonomy validated before connect: non-concrete/combined/Unknown or applet-invalid -> ArgumentException; applet-valid but device-unsupported -> NotSupportedException. The ordered list is the Phase 38.5 seam. - Management (SmartCard->HidFido->HidOtp), YubiOtp (SmartCard->HidOtp), Fido2/WebAuthn (HidFido->SmartCard) gain optional preferredConnection (before CancellationToken for CA1068) + a back-compat overload preserving the old positional shape. Fido2 replaces the Phase 36 dual-transport throw with the default+override policy; WebAuthn forwards it. - Single-transport applets (Piv/Oath/OpenPgp/SecurityDomain/YubiHsm) unchanged. - SCP unchanged and a doc bug fixed: SCP is SmartCard-only; scpKeyParams with a non-SmartCard selection throws (ApplicationSession contract) — the old 'ignored over HID' doc was wrong. - Tests: fake-probe IYubiKeyExtensionsTransportTests for Management/YubiOtp/WebAuthn + extended Fido2 (ISA cases a-i); Core ApplicationSessionScpTests for the SCP guard. - Docs: per-app default-order table (Management README) + Fido2 README transport section. Satisfies master ISC-21, ISC-21.1, ISC-22, ISC-23, ISC-24. Held-transport fallback is Phase 38.5. Verified: build 0/0; Core/Management/YubiOtp/Fido2/WebAuthn units pass; hardware (serial 103, CCID free) default + HidOtp/HidFido/SmartCard overrides all open a working session. Interim Cato (10 rounds, PASS) + interim /DevTeam (5 rounds) reviews; GPT-5.5 cross-vendor reviews queued.
Reconcile the master composite-device ISA checkboxes for the Extension Ergonomics criteria now that Phase 38 (extension smart defaults + explicit overrides) is implemented, verified, and committed (f1e1f4f).
Phase 38.5 adds held-transport fallback to the multi-transport applet session-entry extensions: when no explicit override is given and the SmartCard transport fails to connect because another process holds the CCID (SCARD_E_SHARING_VIOLATION / SCARD_E_SERVER_TOO_BUSY), fall back to the next supported transport in the applet's default order. An explicit override never falls back; no process is ever killed. Design: one public Core helper ConnectSessionTransportAsync over the Phase 38 ordered candidate list (centralized detection + fallback), with the four applet connect sites refactored onto it. Detection is exactly the two PC/SC held/busy codes via SCardException HResult; fallback is gated on candidate == SmartCard (HID-held deferred). Live held-CCID verified on serial 103 via an exclusive self-held holder (CoreCompatSwitches), scdaemon as best-effort fallback holder; gpg --card-status forbidden. Interim Cato gate (GPT-5.4, read-only) converged to PASS over 13 rounds; GPT-5.5 Cato review queued. Satisfies master ISC-23.1.
…applets When no explicit override is given and the SmartCard transport fails to connect because another process holds the CCID (SCARD_E_SHARING_VIOLATION / SCARD_E_SERVER_TOO_BUSY), the session now falls back to the next supported transport in the applet's default order. An explicit override never falls back; the SDK never kills another process. - Core: add public YubiKeyConnectionExtensions.ConnectSessionTransportAsync(candidates, sessionName, ct) — the connect half of the Phase 38 resolve->connect seam. It opens the first candidate that connects; falls back only when a SmartCard connect fails with a held code AND a further candidate remains; rethrows on the last candidate (so an override's single-element list never falls back). Detection (private IsHeldTransportError) matches only an SCardException whose (uint)HResult is SCARD_E_SHARING_VIOLATION or SCARD_E_SERVER_TOO_BUSY. Cancellation is checked before each attempt and between attempts; non-held errors and OperationCanceledException propagate immediately. Public input is validated (null/empty/non-concrete/duplicate). Successful transport logged at debug. - Management/YubiOtp/Fido2 connect sites refactored onto the helper (duplicated transport switches removed); WebAuthn rides the shared Fido2 path. The Fido2 'no FIDO-capable connection' remap stays scoped to ResolveSessionTransports only, never the helper. - HID-held fallback is out of scope (SmartCard PC/SC codes only), per master ISC-23.1. Tests: Core ConnectSessionTransportTests (ISA cases a-m), HeldExceptionPropagationTests (ISC-9: raw SCardException reaches the applet seam unwrapped through CompositeYubiKey and PcscYubiKey), and applet wiring (Management/YubiOtp held->HID fallback with disposal of the opened fallback connection on session-init failure, override-no-fallback; Fido2 remap scope). Satisfies master ISC-23.1. Verified: build 0/0; Core/Management/YubiOtp/Fido2/WebAuthn units pass; format/docs-qa/diff clean; no Core->Management dependency; no process-management code. Live held-CCID fallback proven on serial 103 (CCID held exclusively via the compat switch): default selected SmartCard with CCID free, fell back to HidFido when held, and read serial 103 over the fallback. Interim Cato (13 rounds, PASS) + interim /DevTeam (PASS); GPT-5.5 cross-vendor reviews queued.
Final phase of the composite-device program. Light ISA + one final-program Cato audit (no multi-round pre-edit gate; no API-surface change). Scope: consumer documentation of the physical-device model (master ISC-29) via a new docs/architecture/physical-device-model.md plus Core README/CLAUDE updates; safe composite-discovery + typed-connect hardware smoke on serial 103 (ISC-28); the final verification gate (ISC-30); evidence-backed reconciliation of every master criterion (ISC-1..27, 31..32) and the master Verification section; interim Cato program audit with GPT-5.5 final Cato + backlog DevTeam reviews queued.
…closeout Final phase of the composite-device program (Phases 33-39). Documentation, verification, and evidence-backed reconciliation of the master ISA; no product behavior change. - ISC-29 docs: new docs/architecture/physical-device-model.md (physical IYubiKey semantics, AvailableConnections/SupportsConnection incl. Hid/Unknown/All behavior, Core-owned read-only metadata vs Management mutating ops, typed ConnectAsync<T>, per-applet smart defaults + preferredConnection overrides + held-transport fallback, SCP note, v1->v2 migration table). Core README/CLAUDE updated to the physical-device model (removed a stale per-interface example) and linked to the new doc. - ISC-28 hardware smoke (serial 103, CCID free, no UP/UV): CompositeDiscoveryIntegrationTests 4/4 — one merged device, per-connection filters return the same device, typed SmartCard/FIDO/OTP connects succeed. - Final gate (ISC-30): build 0/0; 12/12 unit test projects; docs-qa 55; changed-file format + git diff --check clean; structural greps confirm no Core->Management ref, no scalar IYubiKey.ConnectionType routing, no applet->Management metadata coupling. - Reconciled master ISA: all 32 criteria (ISC-1..32) checked with per-criterion phase citations true at HEAD; Verification section populated. - Consumer migration the gate caught: migrated the CLI YkDeviceSelectorTests fake from the removed scalar ConnectionType to AvailableConnections (completes ISC-13.1). Also removed two stray trailing newlines (DeviceInfoReader.cs, PhysicalYubiKeyTests.cs) per .editorconfig insert_final_newline=false (whitespace only). Interim final program Cato audit PASS (3 rounds); GPT-5.5 final Cato + backlog GPT-5.5 /DevTeam reviews (Phases 35-38.5) queued. The composite-device program is complete with the GPT-5.5 cross-vendor reviews queued.
…ge claims Final Cato audit (round 3) caught two documentation overclaims: - The physical-device docs presented composite HID+CCID discovery as current general behavior, but Windows HID enumeration is not yet implemented (FindHidDevices.cs returns [] on Windows), so on Windows a YubiKey surfaces only its PC/SC (CCID) interface. Added an explicit Windows/ platform caveat to docs/architecture/physical-device-model.md and src/Core/README.md, and recorded Windows HID enumeration as a deferred platform follow-up. - The architecture doc stated 'one IYubiKey per physical key' as an absolute guarantee; qualified it as the common PID-merge case with the intentional conservative no-merge fallbacks (unparsed USB CCID PID, unreadable serial). Also tightened 'build 0/0' to '0 errors (1 pre-existing unrelated warning)' in the closeout evidence. Docs-only; no code change.
DennisDyallo
changed the title
Extend ConnectSessionTransportAsync with a session-factory overload so held SmartCard errors raised during session creation can fall back to the next supported transport. Dispose failed connections before fallback while preserving explicit override behavior and keeping fallback scoped to held SmartCard PC/SC errors.
Treat ambiguous partial full-PID observations conservatively unless serial evidence is available, and detect composite member changes for repository remove/add events. This avoids collapsing potentially separate same-model keys behind the same PID-only physical identity and prevents stale cache entries when a PID-based composite ID is reused for different member interfaces.
Create Management sessions inside the shared transport fallback helper so held SmartCard errors during session initialization can fall back to HID defaults. When SCP parameters are supplied without an explicit override, force SmartCard because SCP is SmartCard-only.
Create YubiOTP sessions inside the shared transport fallback helper so held SmartCard failures during session initialization can fall back to OTP HID. Force SmartCard when SCP parameters are supplied without an explicit transport, and format composite transport flags in the OTP example instead of displaying Unknown.
Create FIDO2 sessions inside the shared transport fallback helper and force SmartCard when SCP parameters are supplied without an explicit transport. This keeps normal FIDO2 defaults HID-first while avoiding SCP-over-HID failures; WebAuthn test expectations follow the forwarded FIDO2 behavior.
Format combined ConnectionType flags as joined transport names instead of treating them as Unknown. Composite physical devices expose multiple available connections, so CLI display must handle flags rather than scalar per-interface transports.
Carry the parsed global --transport option through YkDeviceContext and pass it to FIDO, OTP, and Management session factories. This keeps device selection and actual session creation aligned so an explicit transport no longer only affects which device is selected.
Select a concrete FIDO reset transport from the composite device and pass it through reset preflight and reset execution. Avoid treating combined AvailableConnections as a scalar current transport, and avoid mapping SmartCard to NFC when the SDK cannot distinguish USB CCID from NFC from ConnectionType alone.
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
1 participant
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.
No description provided.