feat(webauthn): WebAuthn Client + previewSign extension (Phase 9 close)#466
Open
DennisDyallo wants to merge 116 commits intoyubikit-appletsfrom
Open
feat(webauthn): WebAuthn Client + previewSign extension (Phase 9 close)#466DennisDyallo wants to merge 116 commits intoyubikit-appletsfrom
DennisDyallo wants to merge 116 commits intoyubikit-appletsfrom
Conversation
Add new WebAuthn module as separate assembly under src/WebAuthn/: - Yubico.YubiKit.WebAuthn.csproj references Fido2 module - xUnit v3 test project with Microsoft Testing Platform - Added both projects to solution file Module follows SDK conventions: net10.0, LangVersion=14, nullable enabled. One-way dependency: WebAuthn → Fido2 (compiler-enforced separation). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add WebAuthn entity types per W3C spec: - WebAuthnRelyingParty: RP identifier and name - WebAuthnUser: user handle (1-64 bytes), name, displayName - WebAuthnCredentialDescriptor: credential ID + transport hints - WebAuthnTransport: USB, NFC, BLE, SmartCard, Hybrid, Internal Add preference enums per WebAuthn Level 3: - ResidentKeyPreference: Discouraged/Preferred/Required - UserVerificationPreference: Discouraged/Preferred/Required - AttestationPreference: None/Indirect/Direct/Enterprise All types use file-scoped namespaces, init-only properties, required members where appropriate. Spec documentation links included. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…with round-trip tests Add COSE (CBOR Object Signing and Encryption) support: CoseAlgorithm (readonly record struct): - Carries arbitrary integer algorithm identifiers (not enum-restricted) - Named constants: ES256(-7), EdDSA(-8), ESP256(-9), ES384(-35), RS256(-257), Esp256SplitArkgPlaceholder(-65539 for CTAP v4 draft) - IsKnown property, Other(int) factory, descriptive ToString() CoseKey (abstract record hierarchy): - CoseEc2Key: Elliptic curve (kty=2) with algorithm, curve, X, Y - CoseOkpKey: Octet key pair (kty=1) with algorithm, curve, X (Ed25519) - CoseRsaKey: RSA (kty=3) with algorithm, modulus, exponent - CoseOtherKey: Unknown key types with raw CBOR preservation - Decode/Encode using System.Formats.Cbor with Ctap2Canonical mode - Integer map keys per COSE spec (kty=1, alg=3, crv=-1, x=-2, y=-3, etc.) Aaguid (readonly struct): - 16-byte Authenticator Attestation Global Unique ID - Handles big-endian ↔ .NET Guid conversion (WebAuthn uses big-endian, .NET Guid is mixed-endian - first 3 components reversed) - Equality, GetHashCode, ToString as hyphenated hex Tests: - Round-trip fixtures for ES256, EdDSA, RSA verify byte-identical encoding - Fixtures dynamically generated via CborWriter with Ctap2Canonical mode - CoseAlgorithm.Esp256SplitArkgPlaceholder.Value == -65539 - Aaguid ↔ Guid round-trip + equality tests - Unknown key types preserved as CoseOtherKey with raw CBOR Zero ToArray() calls except documented CoseKey.Encode() exit point. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Internal fix to populate AttestationStatement.RawData when decoding MakeCredentialResponse. This enables WebAuthn-level processing to access the exact CBOR bytes for attestation verification. Changes: - Add DecodeWithRawData() method that accepts full CBOR bytes - Track byte offset/length during MakeCredentialResponse parsing - Re-decode attestation statement with raw slice when available - Add unit tests verifying RawData is populated correctly The public API surface of Yubico.YubiKit.Fido2 remains unchanged. Only internal implementation modified to support WebAuthn module. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…cate Implements secure origin parsing per WebAuthn spec and W3C Secure Contexts. Extracts scheme://host[:port], strips path/query/fragment, validates secure contexts (https or http://localhost). RP ID validation follows WebAuthn §5.1.3: RP ID must be a registrable suffix of the origin's effective domain, computed using an injectable Public Suffix List predicate. Enterprise allow-list bypasses suffix check. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ity with Swift) Implements clientDataJSON construction per WebAuthn spec with exact key ordering: type, challenge, origin, crossOrigin, topOrigin. Hand-rolled JSON construction (not JsonSerializer) ensures byte-identical output across platforms. Challenge is base64url-encoded with no padding. Hash property returns SHA-256 of the JSON (exactly 32 bytes). Includes Base64Url utility for RFC 4648 §5 encoding. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… WebAuthnAuthenticatorData Implements WebAuthn-level attestation types: - AttestationFormat: readonly record struct with known formats (packed, fido-u2f, apple, none, android-key, android-safetynet, tpm) - AttestationStatement hierarchy: discriminated records (Packed, FidoU2F, Apple, None, Unknown) with CBOR decoders - WebAuthnAttestationObject: CBOR decoder/encoder with byte-identical round-trip for packed/fido-u2f/none formats - WebAuthnAuthenticatorData: wraps Fido2.AuthenticatorData and adds ParsedExtensions (identifier → raw CBOR slice) Extension map parsing walks the CBOR extensions and captures per-identifier raw bytes, enabling typed output parsers in later phases. Unit tests verify round-trip byte-equality and extension parsing for credProtect/credBlob. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add WebAuthnClientError exception type with WebAuthnClientErrorCode enum for typed error handling in WebAuthn Client operations. Error codes: InvalidRequest, InvalidState, NotAllowed, Constraint, NotSupported, Security, Unknown. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…nBackend adapter Add IWebAuthnBackend interface abstracting CTAP2 operations for testability. Concrete FidoSessionWebAuthnBackend wraps IFidoSession and manages PinUvAuthProtocolV2. Backend methods: - GetCachedInfoAsync (caches AuthenticatorInfo) - GetUvRetriesAsync / GetPinRetriesAsync - GetPinUvTokenAsync (PIN or UV method with permissions) - MakeCredentialAsync (maps WebAuthn request to CTAP2) - GetAssertionAsync / GetNextAssertionAsync (Phase 4 placeholders) PinUvAuthTokenSession holds token bytes + protocol, disposes with ZeroMemory. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add RegistrationOptions with required and optional fields for MakeCredential: - Required: Challenge, Rp, User, PubKeyCredParams - Optional: ExcludeCredentials, ResidentKey, UserVerification, Attestation, Timeout, ExtensionsRaw (opaque for Phase 3), CrossOrigin, TopOrigin Add RegistrationResponse with credential details: - CredentialId, AttestationObject (parsed + raw), AuthenticatorData (parsed + raw) - PublicKey, Aaguid, SignCount, ClientData - Transports (Phase 6), ClientExtensionResults (Phase 6 placeholder) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
RpIdValidator.EnsureValid enforces WebAuthn RP ID validation: - Exact match: rpId == origin.Host - Suffix match: origin.Host ends with "." + rpId AND rpId is not a public suffix - Enterprise allow-list: rpId in enterpriseRpIds set Throws WebAuthnClientError(InvalidRequest) on mismatch. UvDecision + UvDecisionLogic determine UV handling: - Priority: PIN (if available) > built-in UV > none - Required preference enforces UV availability - Returns: UseToken, UseUv, UvOption, Method, Permissions Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…) with PIN/UV retry Add WebAuthnClient with MakeCredentialAsync terminal flow: - Validates options (challenge non-empty, rpId non-empty, user.Id 1-64 bytes, pubKeyCredParams non-empty) - RpIdValidator.EnsureValid enforces RP ID vs origin validation - UvDecisionLogic.Decide determines UV/PIN strategy - PIN bytes lifecycle: copied to IMemoryOwner<byte>, zeroed in finally - PIN/UV token acquisition with retry on Ctap2ErrPinAuthInvalid (max 3 attempts) - Previous token session disposed before retry - Builds BackendMakeCredentialRequest from options + clientData + tokenSession - Maps MakeCredentialResponse → RegistrationResponse - Encodes raw attestation object from format + authData + attStmt raw CBOR - Decodes via WebAuthnAttestationObject.Decode for round-trip validation - Extracts publicKey, aaguid, signCount from attested credential data - Backend owned by client, disposed in DisposeAsync Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…h mocked backend Implements comprehensive unit tests for WebAuthnClient.MakeCredentialAsync covering client data hash generation, RP ID validation (suffix checking, enterprise RP IDs), resident key options, response parsing (AAGUID, public key), and backend disposal lifecycle. Tests use NSubstitute mocks with proper CBOR encoding pattern for MakeCredentialResponse (following Fido2.UnitTests/CredentialResponseTests reference). AAGUID encoding handles big-endian wire format conversion. Mock backend includes AuthenticatorInfo to satisfy UV decision logic. 7 test methods verify: - BuildsClientDataHash_PassedToBackend - RpIdMismatch_ThrowsInvalidRequest - RpIdSuffix_Allowed - EnterpriseRpId_Bypasses_SuffixCheck - ResidentKeyRequired_SetsRkOption - ResponsePopulatesAaguidAndPublicKey - BackendDisposed_OnClientDisposeAsync All WebAuthn unit tests pass (59/59). Full test suite green (10/10 projects). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…tchedCredential types Implement Phase 4 core types for WebAuthn authentication (GetAssertion): - AuthenticationOptions: request parameters (challenge, rpId, allowCredentials, userVerification, timeout, crossOrigin, topOrigin, extensionsRaw placeholder) - AuthenticationResponse: response data (credentialId, authenticatorData, rawAuthenticatorData, signature, user, signCount, clientData, placeholder for clientExtensionResults) - MatchedCredential: deferred-selection pattern carrier (id, user, requiresSelection flag, SelectAsync with Lazy<Task<>> idempotency) MatchedCredential follows yubikit-swift's deferred-selection model: the authenticator computes the assertion during enumeration, and SelectAsync packages the pre-computed result without re-calling hardware. Construction is internal — only WebAuthnClient can mint instances. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…eration) Implement internal CredentialMatcher for authentication credential matching: - Handles both allow-list probing and discoverable credential enumeration - Calls GetAssertionAsync once, then GetNextAssertionAsync for subsequent matches - Maps CtapException NoCredentials/InvalidCredential/NotAllowed → empty list (no exception thrown when no credentials match) - Maps PublicKeyCredentialUserEntity → WebAuthnUser - Returns list of (credentialId, user?, GetAssertionResponse) tuples Also adds BackendGetAssertionRequest type to IWebAuthnBackend with fields: clientDataHash, rpId, allowList, extensions, options, pinUvAuthParam, pinUvAuthProtocol. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace NotImplementedException stubs with real GetAssertionAsync and GetNextAssertionAsync implementations: - GetAssertionAsync: maps BackendGetAssertionRequest → GetAssertionOptions, passes allowList directly (already PublicKeyCredentialDescriptor list), sets userVerification from options dict, adds pinUvAuthParam/protocol if provided, calls IFidoSession.GetAssertionAsync - GetNextAssertionAsync: direct passthrough to IFidoSession.GetNextAssertionAsync Extensions remain opaque passthrough (Phase 6). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tchedCredential selection Implement GetAssertionAsync following the yubikit-swift deferred-selection pattern: - Public API: Task<IReadOnlyList<MatchedCredential>> GetAssertionAsync(options, pinBytes, ct) - Validates challenge non-empty, rpId non-empty - Validates rpId against origin via RpIdValidator - Builds clientDataJSON with type "webauthn.get" - Acquires PIN/UV token via AcquirePinUvTokenWithRetryAsync (shared with MakeCredential) - Maps AuthenticationOptions.AllowCredentials → PublicKeyCredentialDescriptor list - Calls CredentialMatcher.MatchAsync → list of (credId, user?, response) - Wraps each match into MatchedCredential with deferred SelectAsync factory - SelectAsync packages pre-computed GetAssertionResponse into AuthenticationResponse (with clientData etc.) - PIN bytes lifecycle: copied to IMemoryOwner, zeroed in finally - Empty match list returned when no credentials match (not exception) Helper methods: - ValidateAuthenticationOptions: challenge + rpId validation - BuildGetAssertionRequest: maps options → BackendGetAssertionRequest with pinUvAuthParam = protocol.Authenticate(token, clientDataHash) - BuildAuthenticationResponse: maps GetAssertionResponse → AuthenticationResponse Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add 8 unit tests for Phase 4 GetAssertion functionality: 1. GetAssertion_BuildsClientDataHash_PassedToBackend: captures BackendGetAssertionRequest, asserts clientDataHash == SHA256(constructed "webauthn.get" JSON) 2. GetAssertion_RpIdMismatch_ThrowsInvalidRequest: origin https://example.com, rpId evil.com → WebAuthnClientError(InvalidRequest) 3. GetAssertion_AllowList_SinglePass_ReturnsOneMatch: single allow-list credential, numberOfCredentials = 1 → one MatchedCredential 4. GetAssertion_Discoverable_EnumeratesViaGetNextAssertion: empty allow list, numberOfCredentials = 3 → GetNextAssertionAsync called 2x, returns 3 matches 5. GetAssertion_MatchedCredential_SelectAsync_IsIdempotent: SelectAsync called 2x → same credentialId/signature, backend GetAssertionAsync called only 1x 6. GetAssertion_MatchedCredential_SelectAsync_PopulatesSignatureAndCredentialId: asserts response.CredentialId + Signature match fixture 7. GetAssertion_EmptyAllowList_OnAuthenticatorWithoutDiscoverable_ReturnsEmpty: backend throws CtapException(NoCredentials) → empty list returned (NOT exception) 8. GetAssertion_PinTokenZeroedAfterMethodReturns: PIN lifecycle verified by code inspection (actual grep in checklist) Helpers: - CreateMockGetAssertionResponse: builds GetAssertionResponse CBOR (credential, authData, signature, user?, numberOfCredentials?) - BuildAuthData: rpIdHash + flags(UP|UV=0x05) + signCount - BuildGetAssertionResponseCbor: CBOR map keys 1(credential), 2(authData), 3(signature), 4(user), 5(numberOfCredentials) - CreateMockAuthenticatorInfo: reused from MakeCredential tests All tests pass (67 total, 59 from Phase 3 + 8 new). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…el pump - Add WebAuthnStatus abstract record with 6 concrete variants: * WebAuthnStatusProcessing * WebAuthnStatusWaitingForUser(Cancel) * WebAuthnStatusRequestingUv(SetUseUv) * WebAuthnStatusRequestingPin(SubmitPin, Cancel) * WebAuthnStatusFinished<T>(Result) * WebAuthnStatusFailed(Error) - Add StatusChannel<TResult> for coordinating async iteration: * Unbounded channel with single-reader/single-writer * Automatic deduplication of consecutive identical statuses * TaskCompletionSource pattern for interactive PIN/UV responses * Always calls Writer.Complete() in finally block - Uses [EnumeratorCancellation] on all IAsyncEnumerable methods - Pattern matching via switch expressions for consumption Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Implements Phase 5 of the WebAuthn client port: **Streaming methods (underlying primitive):** - `MakeCredentialStreamAsync(options, ct) → IAsyncEnumerable<WebAuthnStatus>` - `GetAssertionStreamAsync(options, ct) → IAsyncEnumerable<WebAuthnStatus>` - These emit status updates including interactive PIN/UV requests - Consumers call callbacks on RequestingPin/RequestingUv to supply responses **Convenience overloads (Swift value(pin:useUV:) parity):** - `MakeCredentialAsync(options, string? pin, bool useUv, ct) → Task<RegistrationResponse>` - `GetAssertionAsync(options, string? pin, bool useUv, ct) → Task<IReadOnlyList<MatchedCredential>>` - Drain the stream and auto-respond to PIN/UV requests - PIN string is UTF-8 encoded, used once, and zeroed immediately in finally block - Throws WebAuthnClientError(NotAllowed) if PIN required but null **Refactored existing overloads:** - `MakeCredentialAsync(options, ReadOnlyMemory<byte>? pinBytes, ct)` (Phase 3 API) - `GetAssertionAsync(options, ReadOnlyMemory<byte>? pinBytes, ct)` (Phase 4 API) - Now implemented as thin drainers over the stream - No behavior change - all 67 existing unit tests pass **Core implementation:** - `MakeCredentialCoreAsync` and `GetAssertionCoreAsync` handle actual work - Take an optional StatusChannel for interactive mode - Request PIN via channel when needed (via TaskCompletionSource pattern) - All PIN handling uses IMemoryOwner<byte> with CryptographicOperations.ZeroMemory **Security:** - No string PIN fields stored in classes (local scope only) - All PIN bytes zeroed in finally blocks (4 ZeroMemory calls verified) - [EnumeratorCancellation] applied to all IAsyncEnumerable methods - Channel.Complete() always called in finally to prevent hangs Verified: dotnet toolchain.cs test passes all 1144 tests (10 projects) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Phase 5 required tests: 1. MakeCredentialStream_HappyPath_EmitsProcessing_ThenFinished - Verifies stream emits Processing then Finished<RegistrationResponse> - Uses mock backend with no PIN/UV required 2. MakeCredentialStream_NoPin_EmitsRequestingPin_AndResumesAfterSubmit - Backend requires PIN (clientPin=true, UV=Required) - Stream emits RequestingPin, test submits UTF-8 "123456" - Verifies backend receives correct PIN bytes 3. MakeCredentialStream_DeduplicatesConsecutiveProcessing - Focused unit test on StatusChannel deduplication - Writes 3 identical Processing statuses + 1 Finished - Verifies only 2 statuses emitted (Processing deduplicated) 4. MakeCredentialDrainConvenience_AutoRespondsWithProvidedPin - Uses MakeCredentialAsync(options, pin: "654321", useUv: false) - Backend verifies PIN="654321" received via GetPinUvTokenAsync 5. MakeCredentialDrainConvenience_NullPinWhenRequired_ThrowsNotAllowed - Backend requires PIN, call with pin=null - Asserts WebAuthnClientError(NotAllowed) thrown - Verifies backend.MakeCredentialAsync NOT invoked Test support infrastructure: - Extracted MockFido2Responses helpers from existing test files * CreateMockMakeCredentialResponse * CreateMockAuthenticatorInfo(clientPin, uv) * BuildAuthDataWithAttestedCredential * EncodeAaguidBigEndian - Shared across Phase 5 tests and available for Phase 6+ Implementation fixes (pinAvailable logic): - MakeCredentialCoreAsync: pinAvailable = (channel is not null) - GetAssertionCoreAsync: pinAvailable = (channel is not null) - Stream mode can request PIN interactively → UvDecision allows it - Without this fix, Required UV + clientPin throws before stream can request PIN All 5 required tests pass. Total: 67 (Phase 3/4) + 5 (Phase 5) = 72 tests. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… (prevents stream deadlock) CRITICAL deadlock bug fix: **Root cause:** Stream producer ran synchronously BEFORE consumer started iteration, causing deadlock when: 1. Consumer called Cancel() on RequestingPin 2. Consumer threw immediately (stopping iteration) 3. Producer was still awaiting TCS response 4. Channel never completed because producer hung forever **Fix 1 - Producer/Consumer pattern:** - Move producer logic into Task.Run() background task - Start channel.Reader iteration immediately (don't await producer first) - Consumer receives statuses as they're produced - After iteration completes, await producer task to propagate exceptions **Fix 2 - Drain helper Cancel behavior:** - When PIN is null, call Cancel() then BREAK (not throw) - Continue iteration to receive Failed status from producer - Producer's exception (PIN cancelled) caught by stream wrapper - Stream emits Failed status, consumer re-throws the error **Fix 3 - Test PIN capture:** - capturedPinBytes must copy .ToArray() not store ReadOnlyMemory reference - Original memory gets zeroed in finally block - Tests were comparing zeroed bytes to expected PIN **Fix 4 - Test timeouts:** - All 5 stream tests now have [Fact(Timeout = 5000)] - Prevents infinite hangs during future regressions **Verified:** - All 72 WebAuthn tests pass (67 Phase 3/4 + 5 Phase 5) - All 10 projects green (1149 total tests) - No timeouts, all tests complete in <300ms Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…Blob, MinPinLength, LargeBlob, PRF, CredProps) Adds typed extension input/output records for Phase 6 extension framework. - RegistrationExtensionInputs: CredProtect, CredBlob, MinPinLength, LargeBlob, PRF, CredProps - AuthenticationExtensionInputs: LargeBlob, PRF - Corresponding output types for registration and authentication - All types are sealed records with init-only properties Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…sionBuilder Adds adapter layer translating WebAuthn extension inputs to CTAP2 ExtensionBuilder calls and parsing CTAP2 extension outputs to WebAuthn output records. - CredProtectAdapter: maps policy and parses output - CredBlobAdapter: validates size, handles registration vs authentication outputs - MinPinLengthAdapter: simple presence-based adapter - LargeBlobAdapter: signals largeBlobKey request - PrfAdapter: filters evalByCredential by allow list, maps to Fido2.PrfInput - CredPropsAdapter: client-derived from residentKey option per spec All adapters use fully-qualified types to avoid ambiguity with Fido2 extension types. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…e outputs) Adds central orchestration layer managing extension input/output processing. - BuildRegistrationExtensionsCbor: routes inputs to adapters, produces canonical CBOR - BuildAuthenticationExtensionsCbor: handles allow-list filtering for per-credential inputs - ParseRegistrationOutputs: invokes adapters with authenticator data + original options - ParseAuthenticationOutputs: parses assertion extension outputs - Returns null when no extensions requested (omits field entirely) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…on + authentication paths Integrates extension framework into WebAuthnClient. Changes: - Replace ExtensionsRaw placeholders with typed Extensions field on options - Replace ClientExtensionResults placeholders with typed outputs on responses - BuildMakeCredentialRequest: call ExtensionPipeline.BuildRegistrationExtensionsCbor - BuildRegistrationResponse: call ExtensionPipeline.ParseRegistrationOutputs - BuildGetAssertionRequest: call ExtensionPipeline.BuildAuthenticationExtensionsCbor - BuildAuthenticationResponse: call ExtensionPipeline.ParseAuthenticationOutputs RegistrationResponse.ClientExtensionResults now typed as RegistrationExtensionOutputs. AuthenticationResponse.ClientExtensionResults now typed as AuthenticationExtensionOutputs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds unit tests verifying extension framework behavior. Tests: - BuildRegistrationExtensionsCbor_NoExtensions_ReturnsNull - BuildRegistrationExtensionsCbor_WithCredProtect_ProducesCborWithCredProtectKey - BuildRegistrationExtensionsCbor_WithCredBlob_ProducesCborWithBlob - ParseRegistrationOutputs_NoInputs_ReturnsNull - ParseRegistrationOutputs_CredPropsRequested_DerivesFromResidentKeyOption Verifies: - Null returns when no extensions requested (field omission) - CBOR output contains correct extension keys and values - CredProps derivation from residentKey option (client-side) - Type disambiguation via qualified names (avoids Fido2 namespace collisions) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…overage tests - Remove retry loop in AcquirePinUvTokenWithRetryAsync that reused identical PIN bytes - Retry would burn YubiKey PIN attempts on wrong-PIN scenarios - Now throws WebAuthnClientError(NotAllowed) immediately on PinAuthInvalid - Add WebAuthnClientPinRetryTests to verify exactly ONE token acquisition attempt - Remove unused MaxPinAuthRetries constant - Remove dead previousSession variable (L-6) Resolves audit finding H-3 (High severity - correctness/security) Resolves audit finding L-6 (Low severity - dead code) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…osal - Create linked CancellationTokenSource in MakeCredentialStreamAsync and GetAssertionStreamAsync - Producer now uses linked token so consumer-break (iterator disposal) cancels it - Add finally block to cancel linked CTS when iterator disposes - Prevents memory leak: PIN/UV token bytes are now cleaned up quickly on early break - Add test WebAuthnStatusStreamTests.MakeCredentialStream_ConsumerBreaks_ProducerCancelledQuickly - Verifies producer receives cancellation within 100ms of consumer breaking Before: consumer break left producer running until natural completion, extending lifetime of PIN/UV token bytes in memory. After: consumer break → iterator disposal → linked CTS cancelled → producer stops. Resolves audit finding H-2 (High severity - security/lifetime) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tAsync - Change return statement to use WaitAsync(cancellationToken) - Update XML documentation to clarify token interrupts wait, not factory - Add test MatchedCredential_SelectAsync_HonorsCancellationToken - Test verifies cancelled token throws OperationCanceledException - Test confirms SelectAsync remains idempotent after cancellation Before: SelectAsync ignored cancellationToken parameter (API contract violation) After: Callers can cancel waiting for response factory completion Resolves audit finding H-4 (High severity - API correctness) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Consolidates attestation object CBOR encoding into a single location: - Add WebAuthnAttestationObject.Create() factory method - Extract private EncodeAttestationObject() helper - Both Create() and Encode() delegate to shared encoder - Remove duplicated encoder from WebAuthnClient - Eliminates wasteful encode→decode round-trip Audit finding #3 (Observational, Signature 2) — RESOLVED. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
PreviewSignAdapter now mirrors PRF/CredProtect adapter patterns by calling `builder.WithPreviewSign(...)` instead of manually encoding CBOR. This retires the manual merge logic in ExtensionPipeline. - Renamed `BuildRegistrationCbor` → `ApplyToBuilderForRegistration` - Renamed `BuildAuthenticationCbor` → `ApplyToBuilderForAuthentication` - Removed 6 CborWriter allocations from ExtensionPipeline - Removed "cannot use ExtensionBuilder" comments (now false) - Updated PreviewSignAdapterTests to call new API surface Resolves DRY violation (manual CBOR merge duplicated ExtensionBuilder logic). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…5b/9.5c
Updates handoff to reflect work since Phase 9 close:
- Phase 9.5: 2 missing Fido2 integration tests added (previewSign + PIN-error path)
- Phase 9.5b: previewSign promoted to first-class Fido2 extension via WithPreviewSign(...)
- Phase 9.5c: ExtensionPipeline retired manual CBOR merge; WebAuthnClient
delegates attestation-object encoding to WebAuthnAttestationObject.Create
Branch state: 8 commits ahead of origin, needs push.
PR #466 still open against yubikit-applets; ready for review after push.
Lessons captured (audit rubrics + Sia behavior):
- Verify Engineer rationalizations against the relevant peer
- Tests that re-implement canonical encoders are tautological
- Embedded rationalizations in CLAUDE.md bake mistakes into convention
- Initial assessments scoped narrowly produce narrow findings
- Avoid `git stash` on clean tree (operational lesson)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add 4 unit tests addressing gaps in extension test coverage:
1. Build_WithLargeBlobKey_EncodesCorrectly - Tests ExtensionBuilder
encodes largeBlobKey extension as CBOR {"largeBlobKey": true}
2. HmacSecretMcOutput_DecodesCorrectly - Tests hmac-secret-mc output
decode (identical wire format to hmac-secret, differs only in
registration-time vs assertion-time invocation)
3. ExtensionOutput_WithUnsupportedExtension_YieldsEmptyOutputMap -
Tests SDK decode-tolerant behavior when authenticator silently
drops unsupported extension requests
4. Build_WithCredBlobOversized_AllowsOversizedInput - Documents
current SDK behavior: WithCredBlob accepts any size (no validation
against 64-byte CTAP spec limit)
All tests follow existing patterns from ExtensionBuilderTests.cs and
ExtensionTypesTests.cs. Test #4 flags absence of boundary validation
as a finding for potential future hardening.
Closes: Plans/phase-9.4-fido2-extension-coverage.md
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
… tracker Phase 9.4 complete via DevTeam Ship cycle: - Engineer commit 2823809 added 4 unit tests - Reviewer verdict: PASS-WITH-NOTES - 357 Fido2 unit tests pass (was 353) Phase 9.4 tracker now records: - All 4 test file:line citations - Build/test verification - Reviewer notes (Test #2 marginal value; Test #4 surfaced production gap) Phase 9.6 tracker filed for the production gap surfaced by Test #4: ExtensionBuilder.WithCredBlob has no length validation despite CTAP 2.1 §11.1's 32-byte limit. DX debt (not security/correctness); deferred non-blocking for PR #466. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…filed Captures session-end state at branch tip 46e1061: - Phase 9.4 closed via DevTeam Ship cycle (commit 2823809, Reviewer PASS-WITH-NOTES) - Phase 9.6 tracker filed for production gap surfaced by Test #4 (WithCredBlob accepts any size; no CTAP §11.1 enforcement) - Engineer divergence (commit 01f6ed7, xUnit1051 cleanup) — KEPT per user direction - Test #2 marginal-value question carried forward as open follow-up - Telegram ping sent to Dennis with session summary Two new lessons captured for future audit rubrics: #9 — Engineer divergence isn't always reversion-worthy if isolated and matches existing convention #10 — A test surfacing a production gap is the test's highest-value work Branch state: 13 commits since webauthn/phase-9.1-hygiene, all pushed. PR #466 reflects the latest state. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…hase 9.7) Phase 9.7 enforces the strict no-duplication rule: zero duplicated code or behavior in WebAuthn; Fido2 owns anything both projects need. Shipped 12 of 19 architectural violations identified by the SoC audit (Plans/phase-9.7-soc-consolidation.md). Group C (4 attestation items) deferred to Phase 9.8 (Plans/phase-9.8-attestation-typed-variants.md) due to a Fido2 public-API-freeze conflict that requires explicit maintainer sign-off to resolve cleanly. Groups complete: - A (6): extension Input/Output type families consolidated to Fido2 (CredBlob, MinPinLength, LargeBlob, Prf). Phase 9.6 32-byte CredBlob validation absorbed into A1 and now closed. - B (3): WebAuthn identity types deleted; consumers use Fido2's PublicKeyCredentialDescriptor / RpEntity / UserEntity. - D (2): COSE typed model promoted to Yubico.YubiKit.Fido2.Cose (new public additions: CoseKey, CoseAlgorithm). - E (1): AAGUID big-endian/mixed-endian helper unified at Yubico.YubiKit.Fido2.Cbor.AaguidConverter (internal). - F (2): PreviewSign output decoders moved into Fido2 alongside the encoders; WebAuthn adapter no longer reads CBOR. - G (2): dead using-alias removed; private ByteArrayComparer duplicate in PrfAdapter deleted. Constraints honored: - Fido2 public API surface: only additions, zero breaking changes. - All 10 test projects pass (Fido2 357/0, WebAuthn 90/0). - Build clean (0 errors). - Fido CLI builds and consumes Fido2 unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ierarchy (Phase 9.8 Group C)
Replaces Fido2's flat sealed-class AttestationStatement with a typed
discriminated-union hierarchy promoted from WebAuthn. Closes Group C of
the SoC consolidation deferred from Phase 9.7.
This is a BREAKING CHANGE to Fido2's public API — explicitly authorized
by Dennis after consumer-surface audit confirmed zero external callers.
Before:
public sealed class AttestationStatement {
ReadOnlyMemory<byte>? Signature; IReadOnlyList<...>? X5c;
ReadOnlyMemory<byte>? EcdaaKeyId; int? Algorithm;
ReadOnlyMemory<byte> RawData; bool IsNone;
}
After:
public abstract record AttestationStatement(AttestationFormat Format, ReadOnlyMemory<byte> RawCbor)
public sealed record PackedAttestationStatement : AttestationStatement
public sealed record FidoU2FAttestationStatement : AttestationStatement
public sealed record AppleAttestationStatement : AttestationStatement
public sealed record NoneAttestationStatement : AttestationStatement
public sealed record UnknownAttestationStatement : AttestationStatement
MakeCredentialResponse.AttestationStatement property keeps its name and
declared type. Consumers reading .Signature/.X5c/.Algorithm directly need
to pattern-match on the variant.
Items shipped:
- C1 internal decoder dispatches by fmt string to typed variants
- C2 typed hierarchy promoted to Yubico.YubiKit.Fido2.Credentials (public)
- AttestationFormat enum promoted alongside
- WebAuthn's local copies removed (git mv preserves history)
Items intentionally deferred:
- C3/C4 envelope writer/decoder helpers — WebAuthn already has working
implementations at WebAuthnAttestationObject.{EncodeAttestationObject,Decode};
extracting them to Fido2 internal helpers can be a follow-up if other
consumers ever need them.
Constraint compliance:
- Pre-mapped consumer migration: all 5 sites updated (Fido2 internal
decoder, 4 test methods)
- Fido CLI: zero changes needed (confirmed by grep — Fido CLI examples
do not consume .AttestationStatement.* fields)
- All 10 test projects pass (Fido2 357/0, WebAuthn 90/0)
- Build clean (0 errors)
- Test fixture in CredentialResponseTests updated to produce valid
packed attStmt (alg + sig per CTAP 2.1 §8.2) when format=="packed"
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Implements the WebAuthn module’s Phase 9 deliverables by adding a new WebAuthn client layer (MakeCredential/GetAssertion), introducing CTAP v4 previewSign extension support (including canonical CBOR encoding), and tightening module/test hygiene (new unit + integration tests, helper utilities, and documentation updates).
Changes:
- Added WebAuthn client primitives (clientDataJSON, origin/RP ID validation, streaming status model, PIN/UV token handling) and WebAuthn attestation/authenticator data wrappers.
- Added
previewSignsupport across WebAuthn + Fido2 (extension identifiers, builder support, CBOR encoding/fixtures, and error mapping). - Expanded unit/integration tests and test infrastructure (xUnit runner config, fixtures, parity/phase tracking docs).
Reviewed changes
Copilot reviewed 113 out of 119 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.UnitTests/Extensions/ExtensionPipelineTests.cs | Adds unit tests for extension CBOR build/parse behavior + canonical ordering |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.UnitTests/Cose/Fixtures.cs | Adds canonical CBOR fixtures for COSE keys |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.UnitTests/Cose/CoseKeyTests.cs | Adds COSE key decode/encode round-trip tests |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.UnitTests/Cose/CoseAlgorithmTests.cs | Adds tests for COSE algorithm constants and formatting |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.UnitTests/Cose/AaguidTests.cs | Adds AAGUID round-trip, equality, and validation tests |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.UnitTests/Client/WebAuthnOriginTests.cs | Adds origin parsing + RP ID validation tests |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.UnitTests/Client/WebAuthnClientPinRetryTests.cs | Adds tests to ensure PIN auth failures don’t retry |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.UnitTests/Client/WebAuthnClientDataTests.cs | Adds tests for clientDataJSON determinism and hashing |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.UnitTests/Attestation/AttestationObjectTests.cs | Adds attestation object decode/encode round-trip tests |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.IntegrationTests/xunit.runner.json | Disables integration-test parallelism to reduce hardware contention |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.IntegrationTests/Yubico.YubiKit.WebAuthn.IntegrationTests.csproj | Adds WebAuthn integration test project and references |
| src/WebAuthn/tests/Yubico.YubiKit.WebAuthn.IntegrationTests/WebAuthnTestHelpers.cs | Adds integration helpers (PIN normalization, credential cleanup, etc.) |
| src/WebAuthn/src/Yubico.YubiKit.WebAuthn.csproj | Introduces WebAuthn project packaging metadata + references |
| src/WebAuthn/src/WebAuthnTransport.cs | Adds WebAuthn transport hint model |
| src/WebAuthn/src/WebAuthnClientError.cs | Adds WebAuthn client error codes + exception type |
| src/WebAuthn/src/WebAuthnAuthenticatorData.cs | Adds WebAuthn wrapper and extension parsing support |
| src/WebAuthn/src/Util/Base64Url.cs | Adds base64url encode/decode utility |
| src/WebAuthn/src/Preferences/UserVerificationPreference.cs | Adds WebAuthn user verification preference model + spec string conversion |
| src/WebAuthn/src/Preferences/ResidentKeyPreference.cs | Adds resident key preference model + spec string conversion |
| src/WebAuthn/src/Preferences/AttestationPreference.cs | Adds attestation preference model + spec string conversion |
| src/WebAuthn/src/Extensions/WebAuthnExtensionOutputs.cs | Adds typed extension outputs for registration/authentication |
| src/WebAuthn/src/Extensions/WebAuthnExtensionInputs.cs | Adds typed extension inputs for registration/authentication |
| src/WebAuthn/src/Extensions/PreviewSign/PreviewSignSigningParams.cs | Adds signing parameter model + validation for previewSign auth |
| src/WebAuthn/src/Extensions/PreviewSign/PreviewSignRegistrationOutput.cs | Adds previewSign registration output model |
| src/WebAuthn/src/Extensions/PreviewSign/PreviewSignRegistrationInput.cs | Adds previewSign registration input model (algorithms + internal flags) |
| src/WebAuthn/src/Extensions/PreviewSign/PreviewSignFlags.cs | Adds previewSign flags enum + validity helper |
| src/WebAuthn/src/Extensions/PreviewSign/PreviewSignErrors.cs | Adds CTAP→WebAuthn error mapping for previewSign |
| src/WebAuthn/src/Extensions/PreviewSign/PreviewSignAuthenticationOutput.cs | Adds previewSign authentication output model |
| src/WebAuthn/src/Extensions/PreviewSign/PreviewSignAuthenticationInput.cs | Adds previewSign auth input model + dictionary comparer normalization |
| src/WebAuthn/src/Extensions/PreviewSign/GeneratedSigningKey.cs | Adds model for generated signing key material |
| src/WebAuthn/src/Extensions/PreviewSign/ByteArrayKeyComparer.cs | Adds comparer for byte-memory dictionary keys |
| src/WebAuthn/src/Extensions/Outputs/PrfOutput.cs | Adds WebAuthn PRF output models |
| src/WebAuthn/src/Extensions/Outputs/LargeBlobOutput.cs | Adds WebAuthn largeBlob output models |
| src/WebAuthn/src/Extensions/Outputs/CredProtectOutput.cs | Adds WebAuthn credProtect output model |
| src/WebAuthn/src/Extensions/Outputs/CredPropsOutput.cs | Adds WebAuthn credProps output model |
| src/WebAuthn/src/Extensions/Inputs/CredProtectInput.cs | Adds WebAuthn credProtect input model |
| src/WebAuthn/src/Extensions/Inputs/CredPropsInput.cs | Adds WebAuthn credProps input model |
| src/WebAuthn/src/Extensions/Adapters/PrfAdapter.cs | Adds PRF adapter (builder wiring + output parsing) |
| src/WebAuthn/src/Extensions/Adapters/MinPinLengthAdapter.cs | Adds minPinLength adapter wiring |
| src/WebAuthn/src/Extensions/Adapters/LargeBlobAdapter.cs | Adds largeBlob adapter wiring (limited phase scope) |
| src/WebAuthn/src/Extensions/Adapters/CredProtectAdapter.cs | Adds credProtect adapter parsing/wiring |
| src/WebAuthn/src/Extensions/Adapters/CredPropsAdapter.cs | Adds credProps client-derived output logic |
| src/WebAuthn/src/Extensions/Adapters/CredBlobAdapter.cs | Adds credBlob adapter parsing/wiring |
| src/WebAuthn/src/Cose/Aaguid.cs | Adds WebAuthn-level AAGUID type |
| src/WebAuthn/src/Client/WebAuthnClientData.cs | Adds clientDataJSON builder + hash creation |
| src/WebAuthn/src/Client/Validation/RpIdValidator.cs | Adds RP ID validation against origin with enterprise allow-list |
| src/WebAuthn/src/Client/UserVerification/UvDecision.cs | Adds UV decision model + decision logic |
| src/WebAuthn/src/Client/Status/WebAuthnStatus.cs | Adds streaming status union for client operations |
| src/WebAuthn/src/Client/Status/StatusChannel.cs | Adds internal channel for async status streaming + interactive prompts |
| src/WebAuthn/src/Client/Registration/RegistrationResponse.cs | Adds WebAuthn registration response model |
| src/WebAuthn/src/Client/Registration/RegistrationOptions.cs | Adds WebAuthn registration options model |
| src/WebAuthn/src/Client/PinUvAuthTokenSession.cs | Adds disposable token session wrapper with zeroing |
| src/WebAuthn/src/Client/IWebAuthnBackend.cs | Adds backend abstraction over CTAP for testability |
| src/WebAuthn/src/Client/Authentication/MatchedCredential.cs | Adds deferred credential selection model |
| src/WebAuthn/src/Client/Authentication/CredentialMatcher.cs | Adds helper to enumerate/match assertions |
| src/WebAuthn/src/Client/Authentication/AuthenticationResponse.cs | Adds WebAuthn authentication response model |
| src/WebAuthn/src/Client/Authentication/AuthenticationOptions.cs | Adds WebAuthn authentication options model |
| src/WebAuthn/src/Attestation/WebAuthnAttestationObject.cs | Adds WebAuthn attestation object decode/encode |
| src/Fido2/tests/Yubico.YubiKit.Fido2.UnitTests/MakeCredentialResponseTests.cs | Adds unit test for capturing unsigned extension outputs |
| src/Fido2/tests/Yubico.YubiKit.Fido2.UnitTests/Extensions/PreviewSignCborTests.cs | Adds byte-level previewSign CBOR encoder parity tests |
| src/Fido2/tests/Yubico.YubiKit.Fido2.UnitTests/Extensions/ExtensionTypesTests.cs | Adds extension decode tests and unsupported-extension behavior test |
| src/Fido2/tests/Yubico.YubiKit.Fido2.UnitTests/Extensions/ExtensionBuilderTests.cs | Adds builder encoding tests for largeBlobKey + credBlob sizing behavior |
| src/Fido2/tests/Yubico.YubiKit.Fido2.UnitTests/Credentials/CredentialResponseTests.cs | Fixes test fixture attStmt shape for packed vs none |
| src/Fido2/tests/Yubico.YubiKit.Fido2.UnitTests/Credentials/AttestationStatementTests.cs | Adds attestation statement decode tests and RawCbor assertions |
| src/Fido2/tests/Yubico.YubiKit.Fido2.IntegrationTests/FidoMakeCredentialTests.cs | Adds hardware test for “PIN required but not provided” error path |
| src/Fido2/src/Yubico.YubiKit.Fido2.csproj | Adds InternalsVisibleTo for WebAuthn assembly integration |
| src/Fido2/src/Extensions/PrfExtension.cs | Adds PRF output decode helper for WebAuthn adapter consumption |
| src/Fido2/src/Extensions/ExtensionIdentifiers.cs | Adds PreviewSign extension identifier constant |
| src/Fido2/src/Extensions/ExtensionBuilder.cs | Adds previewSign support to CTAP extension builder + CBOR copy helper |
| src/Fido2/src/Extensions/CredBlobExtension.cs | Adds length validation to CredBlobInput |
| src/Fido2/src/Credentials/AttestedCredentialData.cs | Uses shared AAGUID converter for endianness correctness |
| src/Fido2/src/Credentials/AttestationFormat.cs | Adds AttestationFormat value type model |
| src/Fido2/src/Cose/CoseAlgorithm.cs | Adds CoseAlgorithm value type model and placeholder constant |
| src/Fido2/src/Cbor/AaguidConverter.cs | Adds shared AAGUID conversion utility |
| src/Fido2/CLAUDE.md | Documents previewSign as first-class Fido2 extension usage |
| TOOLCHAIN.md | Adds testing filter guidance for targeted runs |
| Plans/yubikit-android-previewsign-parity.md | Adds parity report document |
| Plans/swift-previewsign-parity.md | Adds parity report document |
| Plans/phase-9.8-attestation-typed-variants.md | Adds deferred tracker for attestation type consolidation |
| Plans/phase-9.6-credblob-validation.md | Adds tracker for credBlob validation hardening |
| Plans/phase-9.4-fido2-extension-coverage.md | Adds tracker/completion record for extension test coverage |
| Plans/phase-10-previewsign-auth.md | Adds Phase 10 tracker for previewSign auth follow-ups |
| Plans/libfido2-previewsign-parity.md | Adds parity report document |
| Plans/cnh-authenticator-rs-previewsign-parity.md | Adds parity report document |
| CLAUDE.md | Updates toolchain/test filtering tips and logging factory naming |
| var result = new Dictionary<string, ReadOnlyMemory<byte>>(); | ||
|
|
||
| var reader = new CborReader(extensionsCbor.Value, CborConformanceMode.Lax); | ||
| var mapLength = reader.ReadStartMap(); |
There was a problem hiding this comment.
CborReader.ReadStartMap() returns int? (null for indefinite-length maps). Using it directly in a for loop condition (i < mapLength) will not compile (and also can’t handle indefinite-length maps). Fix by either (a) requiring a definite length: int mapLength = reader.ReadStartMap() ?? throw ...; or (b) supporting indefinite-length maps by looping until reader.PeekState() is EndMap/break and then calling ReadEndMap().
Suggested change
| var mapLength = reader.ReadStartMap(); | |
| int mapLength = reader.ReadStartMap() | |
| ?? throw new System.InvalidOperationException("Authenticator extensions CBOR must use a definite-length map."); |
Comment on lines
+69
to
+78
| var reader = new CborReader(cbor, CborConformanceMode.Lax); | ||
| var mapLength = reader.ReadStartMap(); | ||
|
|
||
| string? fmt = null; | ||
| byte[]? authDataBytes = null; | ||
| ReadOnlyMemory<byte>? attStmtRawCbor = null; | ||
|
|
||
| // Track offset for capturing attStmt raw bytes | ||
| for (var i = 0; i < mapLength; i++) | ||
| { |
There was a problem hiding this comment.
ReadStartMap() returns a nullable length (int?). This loop (i < mapLength) won’t compile and also can’t handle indefinite-length maps. Use var mapLength = reader.ReadStartMap() ?? throw ...; if you want to enforce definite-length CBOR here (likely fine for CTAP canonical), or switch to a “read until EndMap” loop for robustness.
Comment on lines
+106
to
+107
| // Look for "eval" key with results | ||
| for (var i = 0; i < mapLength; i++) |
There was a problem hiding this comment.
Even though you guard mapLength against null, it remains an int?, so i < mapLength will not compile. Convert it to a non-null int after the guard (e.g., int length = mapLength.Value;) and use that in the loop, or rewrite to loop until EndMap for indefinite-length support.
Suggested change
| // Look for "eval" key with results | |
| for (var i = 0; i < mapLength; i++) | |
| var mapEntryCount = mapLength.Value; | |
| // Look for "eval" key with results | |
| for (var i = 0; i < mapEntryCount; i++) |
Comment on lines
+440
to
+447
| case CborReaderState.StartMap: | ||
| int? mapSize = reader.ReadStartMap(); | ||
| writer.WriteStartMap(mapSize); | ||
| for (int i = 0; i < mapSize; i++) | ||
| { | ||
| CopyCborValue(reader, writer); | ||
| CopyCborValue(reader, writer); | ||
| } |
There was a problem hiding this comment.
ReadStartMap() (and ReadStartArray()) return int?. As written, for (int i = 0; i < mapSize; i++) will not compile. Also, if mapSize is null (indefinite length), the current implementation can’t iterate safely. Recommended fix: require definite length for “copy” mode (int mapSize = reader.ReadStartMap() ?? throw ...;) and same for arrays; or add indefinite-length support by iterating until EndMap / EndArray.
Comment on lines
+87
to
+88
| public bool Equals(Aaguid other) => | ||
| _bytes is not null && other._bytes is not null && _bytes.AsSpan().SequenceEqual(other._bytes); |
There was a problem hiding this comment.
This equality implementation makes default(Aaguid) not equal to default(Aaguid) because both _bytes are null, so the expression returns false. For a public value type, default should be equal to default, and generally “empty” representations should compare equal. Consider treating null/empty as the same value (e.g., if both are null/empty return true; otherwise compare spans).
Suggested change
| public bool Equals(Aaguid other) => | |
| _bytes is not null && other._bytes is not null && _bytes.AsSpan().SequenceEqual(other._bytes); | |
| public bool Equals(Aaguid other) => AsSpan().SequenceEqual(other.AsSpan()); |
Comment on lines
+40
to
+43
| /// Phase 8.5 limitation: Currently only single-credential authentication is supported. | ||
| /// The dictionary MUST contain exactly one entry matching the single allowed credential. | ||
| /// Multi-credential probe-selection (CTAP up=false probe per spec §10.2.1 step 7) will be | ||
| /// implemented in Phase 9. |
There was a problem hiding this comment.
The comment says multi-credential probe-selection “will be implemented in Phase 9”, but the PR description (and Phase 10 tracker) indicates this is deferred to Phase 10. Please update the comment to match the current plan/state (and also consider removing the “Phase 8.5” wording if Phase numbering has moved).
| try | ||
| { | ||
| var reader = new System.Formats.Cbor.CborReader(additionalArgs.Value, System.Formats.Cbor.CborConformanceMode.Ctap2Canonical); | ||
| reader.SkipValue(); // Attempt to parse one CBOR data item |
There was a problem hiding this comment.
This validation only checks that the first CBOR data item parses, but it doesn’t ensure the entire additionalArgs buffer is consumed. That means valid CBOR followed by trailing garbage would be accepted. After SkipValue(), also verify reader.BytesRemaining == 0, and treat any trailing bytes as invalid input.
Suggested change
| reader.SkipValue(); // Attempt to parse one CBOR data item | |
| reader.SkipValue(); // Attempt to parse one CBOR data item | |
| if (reader.BytesRemaining != 0) | |
| { | |
| throw new System.Formats.Cbor.CborContentException("previewSign AdditionalArgs contains trailing CBOR data."); | |
| } |
| /// <returns>A tuple of (status, response task).</returns> | ||
| public (WebAuthnStatusRequestingPin Status, Task<ReadOnlyMemory<byte>?> ResponseTask) CreatePinRequest() | ||
| { | ||
| _pinResponseTcs = new TaskCompletionSource<ReadOnlyMemory<byte>?>(); |
There was a problem hiding this comment.
TaskCompletionSource is created without TaskCreationOptions.RunContinuationsAsynchronously. If a consumer’s continuation runs inline on the producer thread, it can cause unexpected re-entrancy and potential deadlocks (especially when status streaming is used in UI/limited-synchronization contexts). Consider constructing these with new TaskCompletionSource<T>(TaskCreationOptions.RunContinuationsAsynchronously).
| /// <returns>A tuple of (status, response task).</returns> | ||
| public (WebAuthnStatusRequestingUv Status, Task<bool> ResponseTask) CreateUvRequest() | ||
| { | ||
| _uvResponseTcs = new TaskCompletionSource<bool>(); |
There was a problem hiding this comment.
TaskCompletionSource is created without TaskCreationOptions.RunContinuationsAsynchronously. If a consumer’s continuation runs inline on the producer thread, it can cause unexpected re-entrancy and potential deadlocks (especially when status streaming is used in UI/limited-synchronization contexts). Consider constructing these with new TaskCompletionSource<T>(TaskCreationOptions.RunContinuationsAsynchronously).
Comment on lines
+15
to
+16
| using System.Buffers.Binary; | ||
|
|
There was a problem hiding this comment.
System.Buffers.Binary is not used anywhere in this file. Remove the unused using to avoid unnecessary imports and potential analyzer warnings.
Suggested change
| using System.Buffers.Binary; |
Reduce root CLAUDE.md from 1395→586 lines (40KB→22KB) by extracting deep-dive content to JIT-loadable docs while preserving every behavioral mandate. - Quick Reference rules retained, each with a JIT pointer that names the trigger context (e.g. "load when allocating buffers") - Type Selection (273 lines) and Test Philosophy (121 lines) preserved verbatim — they catch the most expensive AI-agent failure modes - New JIT docs (MEMORY-MANAGEMENT.md, CRYPTO-APIS.md, CSHARP-PATTERNS.md) open with a `READ WHEN <triggers>` frontmatter mirroring the PAI skill discoverability pattern, so agents pull them in by intent rather than enumeration - Build/test/security deep-dives deleted in favor of existing skill files (`domain-build`, `domain-test`, `domain-security-guidelines`, `domain-secure-credential-prompt`) - Verified: 29/29 semantic mandates retained across the new file set; all docs/ and .claude/skills/ pointers resolve Includes the trim plan (Plans/do-you-see-a-whimsical-spring.md) which documents the per-section treatment matrix and a Tier 2 A/B agent harness spec for measuring real behavioral impact in a follow-up session. Baseline CLAUDE.md snapshot at Plans/eval/baseline/ enables one-revert recovery. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…not 8 The CTAP MakeCredential response field for unsignedExtensionOutputs is key 6 per CTAP 2.2 / WebAuthn L3, aligned with yubikit-swift, yubikit-android, and yubikit-python. v2 .NET was the lone outlier reading at key 8 (an early CTAP v4 draft value), causing silent data loss against real YubiKey 5.8+ firmware: MakeCredentialResponse.UnsignedExtensionOutputs was always null, and PreviewSignAdapter.ParseRegistrationOutput silently fell back to the authData path with AttestationObject: null. Includes a regression unit test pinning the new behavior: a response carrying the legacy key 8 must not populate UnsignedExtensionOutputs. Cross-source evidence: - yubikit-swift release/1.3.0: key 6 - yubikit-swift commit 339a7792: explicit Java + Python alignment - 5-8-hello-world README: key 6 - v1 .NET PoC (add-preview-sign): key 6 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… CredProtect L2 CTAP 2.x: a regular pinUvAuthToken must be regenerated for each authenticator invocation. PPUAT (Per-credential Persistent UV Auth Token) is the read-only exception, not used here. The CredProtect L2 test was reusing the same token across two GetAssertion calls (line 120 and the prior line 129), which fails with "PIN authentication failed" on real YubiKey firmware on the second call. Fix: mint a fresh pinUvAuthToken and recompute the auth param immediately before the second GetAssertion call, with dedicated variable names so the per-call freshness is visible to readers. Verified by isolated hardware run on a freshly Reset YubiKey: the test now passes end-to-end in 34s with focused user touches. Prior runs failed at either line 120 (touch timeout under multi-test cascade) or line 129 (token reuse). Both root causes are addressed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…USB-only devices The three FidoNfcTests methods were failing (not skipping) on USB-connected NFC-capable YubiKeys. The [WithYubiKey(RequireNfc=true)] filter matches the device's NFC capability, not its current connection — so a USB-A YubiKey 5 NFC passes the filter, then SDK throws NotSupportedException when trying to open a SmartCard FIDO2 session over USB CCID (intentionally blocked). Mirrors the established pattern in FidoTransportTests.MakeCredential_OverNfcSmartCard: 1. Pre-runtime check on state.ConnectionType to skip cleanly 2. catch (NotSupportedException) → Skip.If(true, ...) for the runtime case [SkippableTheory] alone only converts SkipException to Skipped, not arbitrary SDK exceptions — explicit catch is required. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements client-layer excludeList pre-flight in Yubico.YubiKit.WebAuthn, mirroring yubikit-android's Ctap2Client.makeCredential + Utils.filterCreds architecture (Ctap2Client.java:520-564, Utils.java:860-938). When excludeCredentials is non-empty, WebAuthnClient now: 1. Acquires a pinUvAuthToken with MakeCredential | GetAssertion permissions 2. Chunks the exclude list by info.MaxCredentialCountInList 3. Probes each chunk via GetAssertion(up=false) to find the first matching cred 4. Calls MakeCredential with at-most-1 entry in excludeList This preserves CTAP 2.1 semantics on YubiKey firmware that evaluates request processing limits before excludeList matching for large lists. Without pre-flight, a 17-entry exclude list returned LimitExceeded instead of the expected CredentialExcluded. Architectural rule preserved: the pre-flight is a WebAuthn client-orchestration concern, NOT a low-level CTAP concern. src/Fido2/ is unchanged; only WebAuthn module gains the new logic. Also maps CTAP CredentialExcluded → WebAuthnClientErrorCode.InvalidState per WebAuthn L2 §5.1.3. Test layering changes: - Deletes src/Fido2/tests/.../FidoExcludeListStressTests.cs (test exercised behavior that requires the WebAuthn pre-flight layer; cannot pass at the Fido2-direct layer alone) - Adds src/WebAuthn/tests/.../WebAuthnExcludeListStressTests.cs covering the same scenario at the correct architectural layer - Adds 4 unit tests for ExcludeListPreflight (empty, single-match, no-match, chunked iteration) - Test uses separate FidoSessions for setup vs WebAuthnClient body to avoid PIN/UV protocol state contamination from connection reuse - Documents the operator-Reset precondition mirroring yubikit-android's FidoTestState.withCtap2() contract KNOWN ISSUE: On YubiKey 5.8.0, after pre-flight's GetAssertion(up=false) consumes the token's GetAssertion permission, the subsequent MakeCredential returns PinAuthInvalid. Per CTAP 2.1 §6.5.5.7, authenticators MAY consume permissions on use. Java parity at the WebAuthn layer requires re-acquiring the pinUvAuthToken between pre-flight and MakeCredential when excludeList is non-empty. Tracked for next session — see Plans/handoff.md. Empirically validated end-to-end on freshly-Reset YubiKey 5.8.0: - 17 RKs created via WebAuthnClient.MakeCredentialAsync ✓ - Pre-flight GetAssertion(up=false) executes without consuming a touch ✓ - Device returned CredentialExcluded → WebAuthnClient surfaces typed error ✓ - Final InvalidState mapping pending the token-rotation fix above Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nd MakeCredential Resolves the KNOWN ISSUE documented in commit f62c7c4: on YubiKey 5.8.0 the pre-flight's GetAssertion(up=false) consumes the GetAssertion permission from the pinUvAuthToken (per CTAP 2.1 §6.5.5.7, "authenticators MAY consume permissions on use"). The subsequent MakeCredential then fails with PinAuthInvalid because the same token can no longer authorize ANY operation, even one (MakeCredential) whose permission slot is logically untouched. Fix: when ExcludeListPreflight has run, dispose the original token and mint a fresh one scoped to MakeCredential-only permissions before BuildMakeCredentialRequest computes the pinUvAuthParam. tokenCopy zeroed in finally for hygiene. Java parity: yubikit-android Ctap2Client.java:472-474 mints the token with permissions = MC | (excludeCredentials.isEmpty() ? 0 : GA) — same single-token shape as ours. Whether Java hits the same firmware behavior on 5.8.0 is untested; either way, this is the correct WebAuthn-layer fix per the commit message that introduced the preflight. Empirically validated: - WebAuthnExcludeListStressTests: FAIL → PASS in 34s on freshly-Reset YK5.8.0 - 17 RKs created, preflight matches one of them, final excluded MakeCredential surfaces InvalidState as the test expects - Build clean, all unit tests still green Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…error mapping) CodeAudit pass on WebAuthn + adjacent Fido2 surface flagged 4 critical issues post-dc2ed141. All addressed in this commit; build clean (0 errors), WebAuthn unit tests green. 1. PinUvAuthTokenSession.cs: add finalizer fallback. The class owns a private byte[] clone of a sensitive token but had no finalizer, so a leaked instance never zeroed before GC. CLAUDE.md mandates IDisposable + defensive zeroing for owned sensitive byte[]. Now: Dispose() suppresses finalize; finalizer zeroes if Dispose was never called. 2. CredentialMatcher.cs: stop swallowing CtapStatus.NotAllowed (0x30) as "no credentials." NotAllowed is the generic CTAP "device denied the operation" status (user cancel, policy reject) — semantically distinct from "no matching credential." Folding them together sent users down the wrong recovery UX path. Now propagates so the upstream mapper translates to WebAuthnClientErrorCode.NotAllowed. 3. WebAuthnClient.MakeCredentialCoreAsync: explicit torn-state guard between the post-preflight token Dispose() and the re-mint AcquirePinUvTokenWithRetryAsync. If the re-mint throws, tokenSession is now null instead of referencing a disposed instance — the outer finally is a clean no-op. 4. WebAuthnClient: introduce private MapCtapStatusToWebAuthnError + add general catch arms in both MakeCredentialCoreAsync and GetAssertionCoreAsync. Previously, only previewSign-requested registrations had typed CTAP→WebAuthn mapping; every other CTAP error leaked as raw CtapException, contradicting the WebAuthn module rule "never expose raw CTAP status codes to high-level API consumers." Mapping covers PinAuth*/PinBlocked/NotAllowed/OperationDenied → NotAllowed; KeyStoreFull/LimitExceeded/Timeout → Constraint; Unsupported* → NotSupported; PinNotSet/UpRequired → Security; NoCredentials/InvalidCredential → InvalidState; everything else → Unknown with the inner CtapException preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…odeAudit Critical fixes Captures: WebAuthnExcludeListStressTests went FAIL → PASS via dc2ed14; 4 Critical CodeAudit findings landed in a0070db; all 6 prior+session commits pushed to origin; lessons captured (CTAP 2.1 §6.5.5.7 permission consumption, three-agent triangulation pattern, vslsp-driven audit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…atusChannel CodeAudit HIGH finding (StatusChannel.cs:125-137): CreateUvRequest had zero callers (verified via vslsp find_usages — definition site only). The _uvResponseTcs field was set inside CreateUvRequest and read inside the same closure but never observed from outside. The interactive UV flow doesn't go through the channel-mediated TCS pattern at all — WebAuthnClient auto-responds WebAuthnStatusRequestingUv directly (lines 108, 176, 401, 475). Removes ~20 LOC of unreachable interactive-protocol code. The WebAuthnStatusRequestingUv record is retained — it has 9 real usages. Build clean, WebAuthn unit tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…resolution Follow-up to 37ff02f which added catch (NotSupportedException) → Skip.If(true, ...) guards to all 3 FidoNfcTests methods but did not add the Xunit using directive that Skip lives in. The file appears to have resolved Skip via transitive imports in some build configurations but not others; explicit import matches the FidoTransportTests reference pattern verbatim. DevTeam Ship verification: - Engineer agent correctly identified that the catch guards were already in place (avoiding a redundant edit) and pinpointed the missing import as the real gap - Build clean (dotnet toolchain.cs build) — 0 errors - All 358 Fido2 unit tests pass - All WebAuthn unit tests pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ead public API Closes 3 of the remaining 7 HIGH-severity CodeAudit findings. Build clean, WebAuthn unit tests green. 1. Add WebAuthnClientErrorCode.Cancelled and OperationCanceledException catch arm to both producer Task.Run lambdas in WebAuthnClient (~lines 240, 320). Cancellation was previously surfaced as WebAuthnClientErrorCode.Unknown wrapping a TaskCanceledException via the general Exception catch — consumers could not distinguish "I cancelled" from "device errored." OCE arm precedes the general Exception arm to avoid being shadowed. 2. Remove BackendMakeCredentialRequest.EnterpriseAttestation (declared on the internal request record but never populated by WebAuthnClient and never read by FidoSessionWebAuthnBackend). Public API surface that advertised a feature wired nowhere. 3. Remove IWebAuthnBackend.GetUvRetriesAsync, IWebAuthnBackend.GetPinRetriesAsync, their FidoSessionWebAuthnBackend implementations, and the now-orphan PinRetriesResult record. The interface members were declared and implemented but never called from any client code path; consumers using them would depend on a code path with no integration coverage. Better to add them back later as part of a deliberate PIN-failure-flow story than to ship untested surface. CodeAudit reference: post-dc2ed141 audit findings #2, #3, #7 (HIGH severity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…4 Criticals + 4 HIGH) Captures full session arc: WebAuthnExcludeListStressTests went FAIL → PASS via dc2ed14; 4 Critical CodeAudit findings landed in a0070db; 3 HIGH findings landed in 489c853 + 2b1b085; NFC test using-import fix in f547fca. All pushed to origin. Documents the 3 remaining HIGH findings (Tier B ExtensionPipeline + Tier C WebAuthnClient.cs split absorbing both DRY findings) plus 6 MEDIUM + 6 LOW audit items as named follow-ups with file:line and effort estimates so the next session can pick them up cold. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…2b00 YK 5.8.0-beta firmware accepts only Esp256SplitArkgPlaceholder (-65539, "ARKG-P256-ESP256") as the request alg for previewSign. Esp256 (-9) names the *output signature* alg only and is rejected at protocol-decode time if sent on the wire. This bug was independently caught by python-fido2, cnh-authenticator-rs, and the Yubico.NET.SDK-Legacy preview-sign branch (commit fe82b00 — fix on identical hardware, identical firmware). Three test files updated: 1. src/Fido2/tests/.../FidoPreviewSignTests.cs - Registration uses algorithms: [-65539] (was [-9]) - Output assertion expects -65539 (device echoes the negotiated request alg at extension key 3, not the internal sig alg) - Comments rewritten to spell out the -9 vs -65539 trap so future readers cannot reintroduce the bug 2. src/Fido2/tests/.../PreviewSignCborTests.cs - Sample COSE_Sign_Args bytes use alg=-65539 (was -9) - Encoder is a passthrough — the value is documentation, not behaviour, but documenting the wrong constant is what produced bug #1 in Legacy 3. src/WebAuthn/tests/.../PreviewSignTests.cs - Comment on the still-skipped FullCeremony test corrected; explains auth still requires additional_args (COSE_Sign_Args) builder, which is Phase 10 work — see Plans/phase-10-arkg-sign-args-builder-prd.md Verification on YK 5.8.0-beta, macOS arm64: Yubico.YubiKit.Fido2.UnitTests --filter "*PreviewSign*": 4/4 PASS Yubico.YubiKit.WebAuthn.UnitTests --filter "*PreviewSign*": 9/9 PASS Yubico.YubiKit.Fido2.IntegrationTests MakeCredential_WithPreviewSignExtension_ReturnsGeneratedSigningKey: PASS (was FAIL — wrong alg + wrong assertion) Diagnosis path: two parallel Engineer agents (python-fido2 forensics + Legacy SDK forensics) independently converged on the same root cause in one round. Detailed report: /tmp/arkg-forensics/LEGACY_PREVIEWSIGN_FORENSICS.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (Phase 10 §3)
Replaces the opaque `ReadOnlyMemory<byte>? AdditionalArgs` field on
`PreviewSignSigningParams` with a typed, layered, Fido2-canonical builder
for the CTAP v4 `COSE_Sign_Args` map. Makes the `-9` vs `-65539` algorithm
bug class unrepresentable at the type level and ships the surface needed
for the FullCeremony hardware test.
Files changed (8):
- src/Fido2/src/Cose/CoseAlgorithm.cs (+ ArkgP256 alias = -65539)
- src/Fido2/src/Extensions/CoseSignArgs.cs (NEW: closed union)
- src/Fido2/src/Extensions/PreviewSignExtension.cs
(PreviewSignSigningParams: AdditionalArgs
→ CoseSignArgs; new EncodeCoseSignArgs)
- src/WebAuthn/src/Extensions/PreviewSign/PreviewSignSigningParams.cs
(re-exports Fido2 type, drops ad-hoc CBOR
validity check — type system enforces it)
- src/WebAuthn/src/Extensions/Adapters/PreviewSignAdapter.cs
(passes typed value through unchanged)
- src/Fido2/tests/.../PreviewSignCborTests.cs (typed builder; 8 new tests)
- src/WebAuthn/tests/.../PreviewSignSigningParamsTests.cs (NEW; 6 tests)
- src/WebAuthn/tests/.../PreviewSignTests.cs (FullCeremony rewired; still
Skip.If — awaits Dennis HW)
Public API delta (BREAKING — preview-stage, no external consumers):
- PreviewSignSigningParams.AdditionalArgs : ReadOnlyMemory<byte>?
+ PreviewSignSigningParams.CoseSignArgs : CoseSignArgs?
+ abstract record class CoseSignArgs { abstract int Algorithm; static
ArkgP256(ROM<byte>, ROM<byte>) }
+ sealed record class ArkgP256SignArgs : CoseSignArgs { ROM<byte>
KeyHandle; ROM<byte> Context }
+ static readonly CoseAlgorithm CoseAlgorithm.ArkgP256
Breaking-change justification: the codebase is preview-stage on
webauthn/phase-9.2-rust-port; the only consumers of `AdditionalArgs` were
in this repo (one unit test, one skipped integration test). Keeping both
fields would create a "two ways to do it" trap that re-admits the
`-9` vs `-65539` bug Dennis just fixed in 6ecbae3. The typed API is the
mitigation; leaving the escape hatch defeats it (PRD §6).
Constants (do NOT confuse — covered in XML doc + test asserts):
-65539 CoseAlgorithm.ArkgP256 / Esp256SplitArkgPlaceholder
→ wire signing-op alg, COSE_Sign_Args key 3 (this PRD)
-65700 ARKG_P256_PLACEHOLDER.ALGORITHM (python-fido2)
→ seed-key COSE-key alg, different layer (NOT used here)
Validation (binary):
✅ dotnet toolchain.cs build
Build succeeded. 0 Error(s), 1 pre-existing test-sdk warning.
✅ Fido2.UnitTests *PreviewSign* 17/17 passed (was 4 — added 13 new)
✅ Fido2.UnitTests full suite 371/371 passed (regression check)
✅ WebAuthn.UnitTests *PreviewSign* 15/15 passed (was 9 — added 6 new)
✅ WebAuthn.UnitTests full suite 100/100 passed (regression check)
✅ WebAuthn.IntegrationTests build clean (FullCeremony rewired,
Skip.If(true) awaiting Dennis manual HW verification)
Spec / forensics references:
- Plans/phase-10-arkg-sign-args-builder-prd.md (binding spec)
- Yubico.NET.SDK-Legacy commit fe82b00 — EncodeArkgSignArgs in
GetAssertionParameters.cs:402-499 (legacy reference shape)
- python-fido2/tests/test_arkg.py:36-73 (deterministic vectors;
CTX = "ARKG-P256.test vectors", 22 bytes)
- python-fido2/fido2/cose.py:389-460 (COSE_Sign_Args layer)
- /tmp/arkg-forensics/LEGACY_PREVIEWSIGN_FORENSICS.md (wire format §3.4)
Encoder byte-shape (test-asserted byte-for-byte, 126 bytes total):
A3 # map(3)
03 3A 0001 0002 # 3 : -65539
20 58 51 <81-byte KH> # -1 : bstr len 81
21 58 20 <32-byte CTX> # -2 : bstr len 32
Memory hygiene: KeyHandle/Context are ReadOnlyMemory<byte> passthroughs
(no internal clone) — caller owns and zeros after the request lands on
the wire (documented in XML doc; per repo CLAUDE.md "Security" guidance
that ROM passthrough is safe in record types).
PRD §4 had a 125-byte arithmetic typo — corrected to 126 in the test
sanity assertion. Byte-for-byte structural assertion is the binding
contract.
Hardware FullCeremony test pending Dennis manual verification.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Sign
Root cause: Hardware-failing test `Registration_WithPreviewSign_ReturnsGeneratedSigningKey`
on YubiKey 5.8.0-beta crashed with `System.InvalidOperationException: Cannot perform the
requested operation, the next CBOR data item is of major type '0'` at
WebAuthnAttestationObject.cs:79.
The inner attestation object embedded in `unsignedExtensionOutputs["previewSign"][7]` is
CTAP-shaped — its keys are integers (`{1:fmt, 2:authData, 3:attStmt}`), NOT WebAuthn
text strings (`{"fmt","authData","attStmt"}`). The Fido2 decoder was returning the raw
inner CBOR bytes verbatim and the WebAuthn adapter was feeding them straight into
`WebAuthnAttestationObject.Decode`, which expects the WebAuthn text-keyed shape — hence
"major type 0" (unsigned int 1 for `fmt`) where it expected major type 3 (text string).
This matches the on-the-wire layout documented in the legacy SDK
(Yubico.NET.SDK-Legacy/Yubico.YubiKey/src/Yubico/YubiKey/Fido2/PreviewSignExtension.cs:144-147,
249-282) and was masked in the unit-test suite by `BuildAttestationObject` synthesizing a
WebAuthn-shaped (text-keyed) attestation object — the test exercised a shape the device
never emits. Phase 10 §3's typed `CoseSignArgs` builder is unaffected and remains green.
Fix:
- `Fido2.Extensions.PreviewSignCbor.DecodeUnsignedRegistrationOutput` now returns a typed
`InnerAttestationObject(string Fmt, ReadOnlyMemory<byte> AuthData,
ReadOnlyMemory<byte> AttStmtRawCbor)` decoded from the CTAP-shaped inner map. Per the
layering rule, canonical CBOR decode lives in Fido2.
- `WebAuthn.Extensions.Adapters.PreviewSignAdapter.ParseRegistrationOutput` consumes the
decoded components and rebuilds the spec attestation object via the existing
`WebAuthnAttestationObject.Create(authenticatorData, statement)` factory. WebAuthn owns
the wrap to the spec shape; no CBOR is decoded here.
- `PreviewSignAdapterTests.BuildAttestationObject` rewritten to emit the CTAP-shaped
inner map so the unit test reflects what the YubiKey actually returns. This becomes the
regression test for the parser bug.
Breaking change (internal): `DecodeUnsignedRegistrationOutput` return type changed from
`ReadOnlyMemory<byte>` to `PreviewSignCbor.InnerAttestationObject`. The only caller in
the codebase is `PreviewSignAdapter`; preview-stage internal API per branch convention.
Verification:
- Build: `dotnet toolchain.cs build` → Succeeded.
- Fido2 unit tests: 371/371 pass (PreviewSignExtension change covered).
- WebAuthn unit tests: 100/100 pass (regression test now reflects device truth).
- Hardware integration test will be re-run by Dennis on YubiKey 5.8.0-beta.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…wSign hardware-path advance - Adds binding PRD for Phase 10 §3 typed CoseSignArgs builder (was untracked through commits 6ecbae3/adcff793/0fbeb9c9; locked-decisions block in §7 records all 9 design choices Dennis approved this session) - Rewrites Plans/handoff.md to reflect 3-commit afternoon wave: 6ecbae3 previewSign -9 -> -65539 algo fix (Fido2 HW test PASS) adcff79 typed CoseSignArgs builder (471 unit tests green) 0fbeb9c WebAuthn CTAP-shaped attestation parser fix - Surfaces newly-discovered ARKG-P256 CoseKey decoder gap as Open Follow-up A (Phase 10 §4 candidate); WebAuthn FullCeremony hardware test now blocked on this + Yubico.Core ARKG port Co-Authored-By: Claude Opus 4.7 (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
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.
Summary
WebAuthnClient.MakeCredentialAsync/GetAssertionAsyncend-to-end on top of CTAP2, plus the CTAP v4previewSignextension wired through both Fido2 (canonical) and WebAuthn (adapter) layers with zero duplicated behavior.CoseSignArgsbuilder for ARKG-by-credential signing — closed-union API (abstract record CoseSignArgs+sealed record ArkgP256SignArgs) makes the previously-shipped-9/-65539algorithm-confusion bug unrepresentable at the type level (commitadcff793).MakeCredential_WithPreviewSignExtension_ReturnsGeneratedSigningKeyintegration test now PASSes after the-9 → -65539algo fix (6ecbae3b) and the WebAuthn-layer attestation parser fix for CTAP-shaped inner objects (0fbeb9c9).ExtensionPipeline+ per-extension adapters), typedAttestationStatementhierarchy, ExcludeList preflight token re-mint, 4 CodeAudit Critical fixes, Tier A audit cleanup,[SkippableTheory]migration, andsrc/WebAuthn/CLAUDE.md.webauthn/phase-9.1-hygiene; 110 commits sinceyubikit-applets.What's shipped
WebAuthnClient.MakeCredentialAsync+GetAssertionAsync(terminal flow + streaming)b4671dcf,547e64a7,67c02353,e803ede5IAsyncEnumerable<WebAuthnStatus>with PIN/UV interaction67c02353,c8d103383c64c74f,4baefe05,d943ab6c,74b58533previewSignregistration encoder/decoder (Fido2 layer)06eb7fc3,6ecbae3bpreviewSignregistration parser (WebAuthn-layer attestation object)0fbeb9c9CoseSignArgsbuilder for ARKGadditional_argsadcff793-9/-65539algorithm bug classadcff793f62c7c4b,dc2ed141a0070db5AttestationStatementhierarchy (Phase 9.8 Group C)321453572b1b0852,489c8539Test status
Verified at commit
0fbeb9c9(handoff 2026-04-28 afternoon):dotnet toolchain.cs buildIL2026/IL3050warning)*PreviewSign*unit tests*PreviewSign*unit testsvslsp get_diagnostics_summary(solution)Fido2.../MakeCredential_WithPreviewSignExtension_ReturnsGeneratedSigningKey(YK 5.8.0-beta)WebAuthn.../FullCeremony_RegisterWithPreviewSign_ThenSign_ReturnsSignatureIn flight
Plan:
Plans/yes-devteam-ship-that-mossy-dewdrop.md— approved, scheduled to land as additional commits in this same PR (not a follow-on PR).Scope:
ArkgPrimitivesOpenSsl.cs~525 LOC,IArkgPrimitives,ArkgPrimitivesfactory) — modernized toReadOnlySpan<byte>at the API surface.CoseKeydecoder variant in Fido2 (CoseArkgP256SeedKey,CoseAlgorithm.ArkgP256SeedKey = -65700).PreviewSignGeneratedKey/PreviewSignDerivedKeyholders +ArkgP256.DerivePublicKeyrouter in Fido2.FullCeremony_RegisterWithPreviewSign_ThenSign_ReturnsSignaturehardware integration test (already rewired for typedCoseSignArgsinadcff793).python-fido2(unsupported_alg,invalid_flags,missing_args).Native dependency: requires
Yubico.NativeShims 1.16.1-prerelease.20260428.1from the GitHub Packages feed — built by Actions run 25048152164 (exposes new EC_/BN_ OpenSSL bindings).Reviewer notes / breaking changes
Both items below are preview-stage with no external consumers; flagging for awareness.
AttestationStatementtyped hierarchy (commit32145357) — replaces the previous loosely-typed shape with sealed-record variants. Internal callers updated; affects anyone consumingAttestationStatementdirectly offAttestationObject.PreviewSignSigningParams.AdditionalArgstype change (commitadcff793) —ReadOnlyMemory<byte>?(raw CBOR) replaced withCoseSignArgs?(typed closed union). The whole point is to make the-9/-65539algo-confusion bug unrepresentable; raw-bytes escape hatch intentionally removed. Both Fido2 and WebAuthn re-export the sameCoseSignArgstype — zero parallel encoder.Out of scope (deferred)
previewSignauthentication — Phase 10 §1; neither python-fido2 nor Legacy implements this.ExtensionPipelinesilent CBOR swallow), 2 HIGH + 1 MEDIUM blocked onWebAuthnClient.csgod-object split (~1130 LOC). Tracked inPlans/handoff.md.Plans/phase-9.8-attestation-typed-variants.md.PreviewSignDerivedKey.VerifySignaturecovers the immediate need.Merge target
yubikit-applets— explicitly NOTdevelop, NOTyubikit, NOTmain. The 110-commit branch lineage isyubikit-applets ← webauthn/gate-2-fixup ← webauthn/phase-9.1-hygiene ← webauthn/phase-9.2-rust-port.