feat: correctly handle invalid fingerprint#29
Merged
Conversation
When fprintd reports verify-no-match (the user's finger doesn't match
any enrolled template), the daemon used to return CTAP2_ERR_OPERATION_DENIED
(0x27) — the same code as "user pressed Cancel". Browsers treat that as
a hard failure and surface it to the page; the user has to click "sign in"
again from scratch.
Per CTAP 2.1 §8.2, the spec value for "built-in user verification
unsuccessful" is CTAP2_ERR_UV_INVALID (0x3F), with the description:
"The platform SHOULD retry." Browsers receiving 0x3F automatically
re-prompt for verification, which is the right UX for "wrong finger,
try again".
Changes:
- ctap2/ctap2.go: add StatusUVInvalid = 0x3F. Cross-checked against
CTAP 2.1 §8.2 status code table (FIDO Alliance spec PDF row
0x3F = CTAP2_ERR_UV_INVALID).
- fprintd/fprintd.go: add ErrNoMatch sentinel. processVerifyStatus
returns it (instead of fmt.Errorf) when fprintd's signal body says
result == "verify-no-match". Other terminal failures still get a
generic wrapped error.
- main.go:
- VerifyResult gains a Reason field (VerifyFailureReason enum;
ReasonUnspecified zero-value, ReasonNoMatch).
- fprintdVerifier.VerifyUser inspects fprintd's returned error with
errors.Is(fprintd.ErrNoMatch) and sets Reason: ReasonNoMatch
when it matches. pinentryVerifier doesn't set Reason — pinentry
is just a yes/no dialog with no equivalent of "wrong finger".
- New helper statusForFailure(VerifyResult) byte maps Reason to
the appropriate CTAP2 status code:
ReasonNoMatch → ctap2.StatusUVInvalid (0x3F)
anything else → ctap2.StatusOperationDenied (0x27)
- handleGetAssertion and handleMakeCredential both call
statusForFailure when the verifier returns !result.OK, instead
of always returning OperationDenied.
Tests:
- TestStatusCodeValuesMatchSpec extended with the StatusUVInvalid row.
- TestProcessVerifyStatus's verify-no-match case now checks
errors.Is(err, ErrNoMatch) instead of an error message string
literal — the sentinel is the contract.
- TestGetAssertion_VerifierRejection split into two sub-cases:
"user canceled (no Reason)" → expects StatusOperationDenied
"fingerprint no-match" → expects StatusUVInvalid
- New TestMakeCredential_VerifierRejection mirrors the same two cases
on the MakeCredential path.
Verified the new fix is load-bearing: making statusForFailure always
return OperationDenied makes both fingerprint_no-match sub-cases fail
with "expected 0x3f, got 0x27".
CI matrix verified locally:
go vet ./...
go test -race -count=1 -timeout 60s ./...
GOOS=linux GOARCH=386 go test -count=1 -timeout 60s ./...
GOOS=linux GOARCH=arm GOARM=5 go build -v ./...
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
A single wrong-finger scan used to bubble straight up to the browser as
CTAP2_ERR_UV_INVALID. Per CTAP 2.1 §8.2 the browser SHOULD retry on 0x3F
but Chrome (and others) surface NotAllowedError to the page on the first
attempt instead — same UX as if the user had pressed Cancel.
Real FIDO biometric authenticators (YubiKey Bio, Touch ID, Windows
Hello) all retry internally up to N attempts before reporting failure
to the platform. This commit makes linux-id behave the same way.
Changes in fprintd/fprintd.go:
- New constants: maxAttempts=3, attemptGap=200ms, totalDeadline=30s.
- verifyFingerprint() now drives a retry loop bounded by a single
context.WithTimeout. Between attempts it calls VerifyStop, sleeps the
gap (interruptible by the deadline), and calls VerifyStart again.
- ErrNoMatch from a single attempt is non-fatal as long as the budget
isn't exhausted; non-no-match terminal errors (verify-disconnected,
etc) still abort immediately because retrying can't help.
- waitForVerifyResult(ctx, sigCh) extracted as a pure helper that
drains signals until a terminal result or the context fires. This
replaces the old time.After-in-loop pattern (which leaked timers
and didn't bound total wall-clock time across iterations).
Tests in fprintd/fprintd_test.go:
- TestWaitForVerifyResult covers six paths with synthetic dbus.Signal
values pushed through a buffered channel:
success on first signal
no-match returns ErrNoMatch
non-terminal scan(s) then success
non-terminal scan then no-match
disconnected returns wrapped error (not reported as no-match)
context deadline returns timeout error
No D-Bus connection or sleep dependency.
verifyFingerprint() itself is still untested directly because it needs
a real D-Bus connection, but its retry-loop behaviour is now expressed
in terms of waitForVerifyResult, which IS tested.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two issues uncovered by user testing of the retry loop:
1. Stale signals between attempts. After fprintd reports verify-no-match
on attempt N and we call VerifyStop+VerifyStart for attempt N+1, the
buffered sigCh may still hold late signals from the previous
verification cycle (sometimes a duplicate verify-no-match, sometimes
a verify-stopped emitted by fprintd as it tears down). The next
waitForVerifyResult would consume one of those instead of waiting
for a new scan, so the user only ever scans once even though the
loop iterates 3 times. Fix: drain sigCh between attempts, after the
gap and before VerifyStart.
2. No visibility into retry behavior. The original log only showed the
final failure. Add log lines per attempt:
"fprintd: attempt 1/3, awaiting fingerprint"
"fprintd: attempt 2/3 (previous: no-match), restarting verify"
"fprintd: attempt 3/3 (previous: no-match), restarting verify"
"fprintd: all 3 attempts exhausted with no match"
so a journal tail conclusively shows whether retry iterated.
Also bumps attemptGap from 200ms to 500ms because some sensors don't
recognize the finger lift in 200ms and would re-process the same scan.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WebAuthn flows in Chrome (and likely Firefox) commonly send two
GetAssertion requests for one user "sign in" click: a preflight that
the browser uses to discover credentials and probe UV state, then the
real request with the actual challenge. Hardware authenticators with
biometric UV (YubiKey Bio, Touch ID, Windows Hello) all cache the UV
result for ~5 seconds so the second request doesn't re-prompt the
user. linux-id had no cache, so the user had to scan their finger
twice per login.
Adds a uvCacheTTL = 5 * time.Second window to fprintd.Fprintd. After
verifyFingerprint() returns nil, the wall-clock timestamp is recorded
on lastSuccessAt under the existing mutex. The next VerifyPresence
call within uvCacheTTL short-circuits with Result{OK: true} without
spawning a goroutine and without calling fprintd at all. Failed
verifications are NOT cached — only successes — so a wrong-finger
attempt always re-prompts.
Concurrency: lastSuccessAt is read at the top of VerifyPresence under
f.mu and written at the end of the verification goroutine under the
same lock, so the cache is race-clean. The cache check is sequenced
AFTER the in-flight check (f.active != nil) so multiple concurrent
calls during a verification still attach to the same channel.
Threading-related security note: the cache only short-circuits within
the same linux-id process, and only for ~5 seconds after a real
verification by the same user. This matches the spec-allowed pattern
described in CTAP 2.1 §6.5 and is exactly how hardware authenticators
behave.
Tests in fprintd/fprintd_test.go:
TestVerifyPresence_CachesRecentSuccess
Mocks verifyFingerprintFunc to count invocations. First
VerifyPresence triggers it; second VerifyPresence within the TTL
must return OK without invoking it.
TestVerifyPresence_DoesNotCacheFailure
Mocks verifyFingerprintFunc to return ErrNoMatch. The failure
must NOT populate lastSuccessAt; the next call must invoke
verifyFingerprintFunc again.
TestVerifyPresence_CacheExpires
Manipulates lastSuccessAt to simulate cache expiry, asserts the
next call invokes verifyFingerprintFunc again.
Mocking is via a package-level var verifyFingerprintFunc =
verifyFingerprint, swapped under defer in each test. Avoids needing
a struct field on Fprintd or a real D-Bus connection.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…fier
The previous version added the cache inside fprintd.Fprintd. That meant
pinentry mode (the default --auth) still got two prompts per WebAuthn
sign-in click, because Chrome sends a preflight + real GetAssertion pair
and pinentry has no equivalent dedup. User confirmed the symptom:
"prompts twice even without using fprintd I think the page wants it twice."
The fix is to put the cache at the verifier-abstraction level so any
backend benefits.
main.go:
- New cachingVerifier wraps any UserVerifier. The first call delegates
to inner.VerifyUser, and on success records lastOK = now(). The next
call within uvCacheTTL (5s) returns Result{OK: true} immediately
without ever invoking inner. Failures are not cached.
- newServer wraps the chosen pinentry/fprintd verifier in
cachingVerifier so both modes benefit from the cache.
- When the cache hits, log "verifier: UV cache hit, skipping prompt"
so users can confirm in journalctl/stdout that the second prompt
was correctly suppressed.
- cachingVerifier.PerformsUV() forwards to the inner verifier so the
AuthFlagUV flag in CTAP2 responses stays honest.
- cachingVerifier has a swappable now() field used in tests to fast-
forward the clock, so cache-expiry tests don't need real sleeps.
fprintd/fprintd.go:
- Removed the per-Fprintd cache, the uvCacheTTL constant, the
verifyFingerprintFunc package var, and lastSuccessAt field. Single
source of truth lives in the verifier wrapper now.
main_test.go: 4 new tests for the wrapper, all using fakeVerifier:
TestCachingVerifier_CachesRecentSuccess — second call hits cache
TestCachingVerifier_DoesNotCacheFailure — failed result is NOT cached
TestCachingVerifier_CacheExpires — fast-forward clock past TTL
TestCachingVerifier_PerformsUVForwardsToInner — UV semantics preserved
fprintd/fprintd_test.go:
- Removed the 3 cache tests; the equivalent assertions now live on
the wrapper in main_test.go.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
User reproduced "have to scan the right finger twice" with this log: 19:42:02 got Ctap2Cmd MakeCredential 19:42:02 fprintd: attempt 1/3, awaiting fingerprint 19:42:02 fprintd: attempt 2/3 (previous: no-match), restarting verify 19:42:03 fprintd: attempt 2/3 matched Attempt 1 → 2 in the same wall-clock second, no time for a real human scan in between. A phantom verify-no-match signal is arriving on attempt 1 immediately after VerifyStart and consuming the slot. Then the user's actual scan lands on attempt 2. The previous drainSignals call only ran inside the `if attempt > 1` branch, so attempt 1 had no protection. Likely cause: when a fresh verifyFingerprint() runs back-to-back after a previous one (e.g. GetAssertion fails, browser immediately sends MakeCredential), some combination of fprintd session-start state and godbus's signal delivery pipeline produces a near-instant phantom signal at the start of the new session. Changes: - New constant settleDelay = 100ms. - Drain logic moved out of the `if attempt > 1` branch and runs on every attempt: after VerifyStart, sleep settleDelay (interruptible by ctx) so any phantoms have a chance to arrive in sigCh, then drainSignals consumes them. Real fingerprint scans take much longer than 100ms so this can't accidentally drop a legitimate scan. - drainSignals now returns the count of dropped signals; if > 0 we log how many phantoms were eaten so the next reproduction makes the issue visible: "drained N phantom signal(s) before attempt M". - waitForVerifyResult logs every signal it receives via "fprintd: signal %q done=%v" so a journal tail shows the actual fprintd VerifyStatus stream and can pinpoint any future weirdness. If this fix works, the user's MakeCredential block should look like: got Ctap2Cmd MakeCredential fprintd: attempt 1/3, awaiting fingerprint fprintd: drained 1 phantom signal(s) before attempt 1 ← NEW fprintd: signal "verify-match" done=true ← NEW fprintd: attempt 1/3 matched i.e. the phantom is dropped, the real scan matches on attempt 1, no "have to touch it again" UX problem. Co-Authored-By: Claude Opus 4.6 (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
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.
Stacked on #27. When fprintd reports
verify-no-match(wrong finger), the daemon now returnsCTAP2_ERR_UV_INVALID(0x3F) instead ofCTAP2_ERR_OPERATION_DENIED(0x27). The browser SHOULD retry on 0x3F per CTAP 2.1 §8.2 — so the user gets re-prompted automatically instead of having to click "sign in" from scratch.What changes
fprintd/fprintd.goprocessVerifyStatusreturned a genericfmt.Errorf(...)for no-matchErrNoMatchmain.go::fprintdVerifierVerifyResult{OK: false}on failureerrors.Is(err, fprintd.ErrNoMatch)→ setsReason: ReasonNoMatchmain.go::handleGetAssertion/handleMakeCredentialStatusOperationDeniedon!result.OKstatusForFailure(r)→ mapsReasonNoMatch→StatusUVInvalid, elseOperationDeniedctap2/ctap2.goStatusUVInvalidconstantStatusUVInvalid = 0x3FpinentryVerifierSpec citation
CTAP 2.1 §8.2 status code table row
0x3F:Tests
TestStatusCodeValuesMatchSpec+ new row assertingStatusUVInvalid == 0x3FTestProcessVerifyStatus/verify-no-matchnow checkserrors.Is(err, ErrNoMatch)(sentinel is the contract)TestGetAssertion_VerifierRejectionsplit into:user canceled (no Reason)→ expectsStatusOperationDeniedfingerprint no-match→ expectsStatusUVInvalidTestMakeCredential_VerifierRejectionmirrors the same two sub-cases for the MakeCredential pathLoad-bearing: making
statusForFailurealways returnOperationDeniedmakes bothfingerprint_no-matchsub-cases fail withexpected 0x3f, got 0x27.CI matrix verified locally
Merge order
This PR is based on
tests-and-spec-fixes(#27), notmain— it depends onprocessVerifyStatus, theVerifyResultstruct, andTestStatusCodeValuesMatchSpecall of which land in #27. Once #27 merges into main, GitHub will rebase this onto main automatically.