security: make require_ir load-bearing and passive anti-spoof honest (Plan 01)#82
Open
tyvsmith wants to merge 8 commits into
Open
security: make require_ir load-bearing and passive anti-spoof honest (Plan 01)#82tyvsmith wants to merge 8 commits into
tyvsmith wants to merge 8 commits into
Conversation
…onest (Plan 01)
Closes auth-review findings H1 (IR heuristic bypass), H3 (CLAHE inflated the
texture check), and the passive half of H2 (frame-variance too loose). Stays
fully passive: no landmark/blink default flip, require_ir=false still supported,
no fail-open path or lockout introduced.
IR classification (facelock-camera/device.rs):
- New IrSource::{Quirk,Format,Name,None} provenance. Quirks force_ir is
authoritative (both directions); an "ir"/"infrared" name *token* (tokenized,
not contains) classifies IR; a GREY/Y16 format counts only when corroborated
by a name token. Mere enumeration of GREY no longer implies IR (H1).
- auto_detect_device prefers a quirks-confirmed IR device, then heuristic IR,
then the first device; never auto-picks an unknown cam for self-reporting GREY.
- Display/auto-select surfaces (direct.rs, daemon handler, setup.rs) now consult
the quirks DB so the shown [IR] tag matches the auth decision.
IR texture (H3): check_ir_texture now runs on the RAW frame, never CLAHE (which
inflated flat-photo std_dev and masked replays). Cutoff is configurable via
security.ir_texture_min_stddev (default 10.0; raw bands: flat <5, real >15).
Frame variance (H2 passive): check_frame_variance takes a configurable
security.frame_variance_max_similarity (default 0.97, require >=0.03 drift).
Documented as passive anti-photo only — it does not stop video replay; IR is the
load-bearing defense.
Config: two new keys under [security] with validation, defaults, and unit tests.
Docs: security.md (§A/B/C rewritten for honesty + raw-frame calibration) and
contracts.md (new keys, defaults, IR/texture/variance semantics).
Tests: device corpus (RGB names + GREY-only + quirk force_ir), frame-variance
boundary, texture-cutoff config, config validation. Container: added an
anti-spoof refusal assertion to run-oneshot-tests.sh (moves the system quirks DB
aside so the gate fires on any host); container-config.toml posture documented.
Verified: cargo build/test/clippy/fmt clean; just test-arch-oneshot 12/12 and
just test-arch-integration 7/7 on real hardware.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rk devices Hardware-verified regression (Logitech BRIO 046d:085e): making the quirks DB authoritative classified EVERY V4L2 capture node of a quirk-matched USB device as IR. The BRIO exposes /dev/video0 (RGB, YUYV/MJPG) and /dev/video2 (IR, native GREY) under one VID:PID, so both were tagged [IR]: setup lost its auto-select, and auto-detect captured from the RGB sensor (white LED) instead of the IR sensor. force_ir now means "this USB device has an IR sensor", not "every capture node of it is IR": when multiple nodes share one quirk-matched USB identity AND at least one exposes an IR-like format (GREY/Y16, or the quirk's format_preference), only the format-bearing node(s) classify IR; siblings fall back to the quirk-free heuristic. If no node has an IR-like format, force_ir is trusted for all nodes (some quirks exist precisely because the camera advertises no IR-like format). IrSource semantics are preserved. - device.rs: classify_ir_sources (list, sibling-aware), ir_source_resolved / is_ir_camera_resolved (single device, enumerates siblings), auto-detect now prefers the format-corroborated IR node; sysfs USB-ID reads kept at the call boundary for testability (classify_ir_sources_with_ids) - quirks.rs: find_match_with_ids (injectable USB IDs) - callers gating require_ir / displaying [IR] use the sibling-aware forms (oneshot auth, daemon build_handler, direct, D-Bus ListDevices, setup wizard) - capture.rs: log the negotiated capture format at camera open - quirks defaults: BRIO entry gains format_preference = "GREY" - docs: security.md §A and contracts.md describe node-level disambiguation - test: BRIO-topology corpus tests; oneshot container script asserts exactly one [IR] node, GREY-native, auto-detect picks it, negotiated format GREY Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
facelock auth's default tracing filter names the package (facelock_cli) rather than the bin crate (facelock), so it has never emitted logs without RUST_LOG — the assertion needs the env var to observe the auto-detected device. Verified against the real BRIO: with RUST_LOG the log shows auto-detected camera device=/dev/video2 and the RGB sibling demotion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tracing's fmt layer emits ANSI styling between field names and values even when redirected to a file, so 'format=GREY' never matches; match the log message and the bare GREY value instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Hardware-verified field data (Logitech BRIO IR, require_ir=true) showed two bugs in the Plan 01 frame-variance gate: Bug 1 (poisoned history): matched-frame embeddings accumulated append-only for the whole session and EVERY consecutive pair had to drift, so one too-still pair made success permanently unreachable — a user who started still and then moved could never authenticate. The gate now evaluates a sliding window (FrameVarianceWindow, size = min_auth_frames) of the most recent matched frames; old frames are forgotten and evicted embeddings are zeroized at eviction (ring-buffer overwrite, zeroize-before-overwrite, plus zeroize on drop). A truly static input keeps every pair above the cutoff in every window, so it still can never pass — proven by unit tests. Bug 2 (wrong default + misleading UX): the 0.97 default assumed 0.02-0.10 live drift, which is empirically wrong — a frozen live human sits at 0.98-0.995 while truly static input sits >= ~0.999. Default raised to 0.995 (top of the frozen-human band): still users pass, static input never does. When the timeout expires with frames matching above the recognition threshold but the variance gate unsatisfied, the outcome now carries an internal AuthFailureReason so 'facelock test' says 'Face matched (best: X) but the liveness variance check was not satisfied' instead of a misleading 'No match'. D-Bus AuthResult contract and PAM fall-through semantics unchanged; the daemon path derives the same message client-side from matched=false + similarity >= threshold. Per-window min/max pair similarities are logged at debug level (values only) for field tuning. The inline oneshot copy in facelock-cli direct.rs is fixed identically. docs/security.md §B and docs/contracts.md updated with the sliding-window semantics and field-measured ranges (the wrong 0.02-0.10 claim removed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The oneshot success message now includes the similarity score; update the container assertion to the stable prefix. The live auth itself passed (Matched (similarity: 0.81) in 0.80s) — only the grep pattern was stale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stricter than the previous 0.995 (top of the field-measured frozen-human band) to add margin against static replays with sensor-noise drift. A fully frozen user may not pass at 0.985, but the sliding-window gate recovers as soon as they move slightly, so the worst case is a brief delay — never a lockout — and password fallback always remains. frame_variance_max_similarity stays the user-tunable knob (loosen toward 0.995, tighten toward 0.97). Boundary tests updated to exercise the new default: rejected just above (cos 0.15 ~ 0.9888) and accepted just below (cos 0.19 ~ 0.9820); recovery tests now drift at cos(0.20) ~ 0.9801 to stay under the cutoff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Strengthens Facelock’s passive anti-spoof posture by making security.require_ir = true genuinely IR-backed (fixing H1 bypass), ensuring IR texture checks operate on raw frames (fixing H3), and making frame-variance liveness both configurable and non-locking via a sliding window (fixing H2 + a real hardware regression).
Changes:
- Reworked IR camera classification with
IrSourceprovenance and sibling-aware multi-node USB disambiguation; updated auto-detect to prefer confirmed IR nodes. - Moved IR texture checking to the raw grayscale frame with a configurable cutoff (
security.ir_texture_min_stddev), leaving CLAHE for recognition only. - Replaced append-only variance history with
FrameVarianceWindow, addedsecurity.frame_variance_max_similarity(default 0.985), and improved failure reporting for variance-blocked matches.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/run-oneshot-tests.sh | Adds BRIO multi-node IR regression gate and a require_ir refusal assertion. |
| test/container-config.toml | Documents container E2E posture for require_ir / frame variance settings. |
| docs/security.md | Updates security documentation for IR classification, sliding-window variance, and raw-frame texture checks. |
| docs/contracts.md | Extends config contract keys and clarifies IR/variance/texture semantics. |
| crates/facelock-daemon/tests/daemon_integration.rs | Adds tests for variance-blocked failure reasons and window recovery. |
| crates/facelock-daemon/src/handler.rs | Makes ListDevices IR tagging quirks-aware and sibling-disambiguated. |
| crates/facelock-daemon/src/auth.rs | Implements raw-frame texture check and sliding-window frame variance with failure reason plumbing. |
| crates/facelock-core/src/types.rs | Adds AuthFailureReason, configurable variance threshold, and FrameVarianceWindow. |
| crates/facelock-core/src/config.rs | Adds ir_texture_min_stddev and frame_variance_max_similarity with defaults + validation. |
| crates/facelock-cli/src/ipc_client.rs | Adapts D-Bus AuthResult mapping to updated internal MatchResult shape. |
| crates/facelock-cli/src/direct.rs | Aligns direct path with sibling-aware IR classification and sliding-window variance behavior. |
| crates/facelock-cli/src/commands/test_cmd.rs | Improves oneshot output (similarity + clearer variance-blocked messaging) and daemon-mode inference. |
| crates/facelock-cli/src/commands/setup.rs | Uses quirks-aware IR classification for correct wizard auto-select/display. |
| crates/facelock-cli/src/commands/daemon.rs | Switches to sibling-aware IR classification for auto-detect and configured device. |
| crates/facelock-cli/src/commands/auth.rs | Uses sibling-aware IR classification for require_ir gating and logs variance-blocked flag. |
| crates/facelock-camera/src/quirks.rs | Adds find_match_with_ids and test helper to support sibling grouping without repeated sysfs reads. |
| crates/facelock-camera/src/preprocess.rs | Makes check_ir_texture threshold configurable and documents raw-frame requirement. |
| crates/facelock-camera/src/lib.rs | Re-exports new IR classification APIs and IrSource. |
| crates/facelock-camera/src/device.rs | Implements IrSource, token-based name heuristic, sibling-aware classification, and improved auto-detect selection. |
| crates/facelock-camera/src/capture.rs | Logs negotiated camera capture format for diagnostics/testing. |
| config/quirks.d/00-defaults.toml | Adds BRIO format_preference = "GREY" and explains multi-node behavior. |
| config/facelock.toml | Documents new anti-spoof tuning keys and sliding-window semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+151
to
+166
| QUIRKS_SYS="/usr/share/facelock/quirks.d" | ||
| QUIRKS_BAK="/tmp/facelock-quirks.bak" | ||
| rm -rf "$QUIRKS_BAK" | ||
| [ -d "$QUIRKS_SYS" ] && mv "$QUIRKS_SYS" "$QUIRKS_BAK" | ||
| cp /etc/facelock/config.toml /tmp/facelock-requireir.toml | ||
| if grep -q '^require_ir' /tmp/facelock-requireir.toml; then | ||
| sed -i 's|^require_ir.*|require_ir = true|' /tmp/facelock-requireir.toml | ||
| else | ||
| sed -i '/^\[security\]/a require_ir = true' /tmp/facelock-requireir.toml | ||
| fi | ||
| run_test "facelock auth refuses non-IR camera when require_ir=true (anti-spoof, H1)" \ | ||
| "facelock auth --user testuser --config /tmp/facelock-requireir.toml" \ | ||
| 2 | ||
| # Restore the system quirks DB. | ||
| [ -d "$QUIRKS_BAK" ] && rm -rf "$QUIRKS_SYS" && mv "$QUIRKS_BAK" "$QUIRKS_SYS" | ||
|
|
Comment on lines
+171
to
+189
| } else if config.security.require_frame_variance | ||
| && result.similarity >= config.recognition.threshold | ||
| { | ||
| // The D-Bus AuthResult contract carries no failure reason, but | ||
| // matched=false with similarity above the recognition threshold | ||
| // means a liveness gate (frame variance) blocked the attempt. | ||
| println!( | ||
| "Face matched (best: {:.2}) but the liveness variance check was not \ | ||
| satisfied after {:.1}s — try moving slightly, or tune \ | ||
| security.frame_variance_max_similarity", | ||
| result.similarity, | ||
| elapsed.as_secs_f64() | ||
| ); | ||
| notify_if_enabled( | ||
| notif_config, | ||
| &NotifyEvent::Failure { | ||
| reason: "face matched but liveness variance not satisfied".to_string(), | ||
| }, | ||
| ); |
…upt CI state Under `set -euo pipefail`, a failing require_ir anti-spoof test aborted the script before the trailing inline restore ran, leaving /usr/share/facelock/quirks.d moved aside and corrupting shared state for later tests in the same CI job (e.g. pamtester, which needs the quirks DB present). Wrap the move/restore in a `trap restore_quirks EXIT` so restoration always runs on any exit path. The inline restore is kept (and the trap dropped) so happy-path behavior is unchanged: subsequent tests still see the restored DB. Co-Authored-By: Claude Fable 5 <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
Makes
require_ir = trueactually mean "IR-backed" and makes the passive flatness/liveness checks do what the docs claim (Plan 01). Stays fully passive — no landmark/blink default flip,require_ir = falseremains a supported knowing choice, and no fail-open path or lockout is introduced.IR classification (H1) — replaced the
contains("ir")substring test and the "GREY/Y16 is available ⇒ IR" assumption. NewIrSource::{Quirk, Format, Name, None}provenance: the quirks DBforce_iris authoritative, anir/infraredtoken (tokenized, not substring) classifies IR, and a GREY/Y16 format counts only when corroborated by a name token. Mere enumeration of GREY no longer implies IR.auto_detect_deviceprefers a quirks-confirmed IR device and never auto-picks an unknown camera just because it self-reports GREY.IR texture (H3) —
check_ir_texturenow runs on the RAW frame, never the CLAHE-equalized frame (CLAHE inflated flat-photo std-dev and masked replays). Cutoff is configurable viasecurity.ir_texture_min_stddev(default 10.0; raw bands: flat <5, real >15). CLAHE stays in the recognition path only.Frame variance (H2 passive) —
check_frame_variancetakes a configurablesecurity.frame_variance_max_similarity. Documented honestly as passive anti-photo only — it does not stop video replay; IR is the load-bearing defense.Hardware-found regressions fixed in-branch
Two regressions surfaced on a real Logitech BRIO (046d:085e) IR host and were fixed within this branch:
Multi-node USB IR disambiguation. Making the quirks DB authoritative initially tagged every V4L2 node of a quirk-matched USB device as IR. The BRIO exposes
/dev/video0(RGB, YUYV/MJPG) and/dev/video2(IR, native GREY) under one VID:PID, so both got[IR]— setup lost auto-select and auto-detect captured from the RGB sensor (white LED).force_irnow means "this USB device has an IR sensor", not "every node is IR": when multiple nodes share one quirk-matched identity and at least one exposes an IR-like format, only the format-bearing node(s) classify IR; siblings fall back to the quirk-free heuristic. If no node has an IR-like format,force_iris trusted for all nodes. The BRIO quirk gainsformat_preference = "GREY".Frame-variance poisoned history + retune + honest message. Field data showed the original append-only history required every consecutive matched-frame pair to drift, so one too-still pair made success permanently unreachable (a user who started still then moved could never authenticate). Replaced with a sliding window (
FrameVarianceWindow, size =min_auth_frames) of the most recent matched frames; evicted embeddings are zeroized (ring-buffer overwrite + zeroize-on-drop). A truly static input still fails every window. The default was also empirically wrong (0.97 assumed 0.02–0.10 live drift; a frozen live human actually sits 0.98–0.995, static ≥ ~0.999): default retuned to 0.985 (from field measurement, stricter than the 0.995 top-of-band for replay margin) and left as the user knob. When the timeout expires with frames matching above the recognition threshold but the variance gate unsatisfied, the outcome now carries an internalAuthFailureReasonsofacelock testreports "Face matched (best: X) but the liveness variance check was not satisfied" instead of a misleading "No match". The inline oneshot copy infacelock-cli/direct.rsis fixed identically.Verification
cargo build/cargo test/cargo clippy -D warnings/cargo fmtclean.force_ir), BRIO multi-node topology, frame-variance boundary tests (reject just above / accept just below the 0.985 default), texture-cutoff and config validation.just test-arch-oneshotandjust test-arch-integrationpass with a legitimate live enroll→auth succeeding (frame-variance/raw-texture false-reject regression gate); real auth observed atMatched (similarity: 0.81) in 0.80s; auto-detect picks the GREY-native/dev/video2and demotes the RGB sibling. Container assertion added torun-oneshot-tests.sh: withrequire_ir=trueagainst an RGB device that merely enumerates GREY,facelock authrefuses.Contract docs
docs/security.md(§A/B/C rewritten for honesty + raw-frame calibration, sliding-window semantics, field-measured ranges) anddocs/contracts.md(newir_texture_min_stddev/frame_variance_max_similaritykeys, node-level IR disambiguation, IR/texture/variance semantics).Closes #61
Closes #62
Closes #63
🤖 Generated with Claude Code