fix(webauthn): F4 LAX tightening — credentialHash floor + both-must-match

Red round 4 flagged (F4, MEDIUM-HIGH) that the previous LAX binding mode in VerifyLiveness permitted a cross-ceremony replay: an attacker who observed a fresh WebAuthn challenge could trick SecureGate into signing a liveness envelope binding to the challenge alone (no credentialHash), then replay that envelope against any enrollment for the same userId that happened to reuse the challengeId-only expectation. Two LAX-only tightenings in pkg/webauthn/liveness.go (STRICT already enforced the missing-both rejection; this sharpens LAX): 1. credentialHash is a hard floor. If the server expects credentialHash and the envelope omits it, reject with "credentialHash required in LAX mode — envelope without credentialHash is replay-vulnerable". challengeId alone cannot satisfy the binding requirement in LAX — challengeId can be observed and replayed; credentialHash commits to the specific pubKey being enrolled. 2. When the envelope supplies BOTH credentialHash AND challengeId AND the server expects both, require BOTH to match. No more "either-or" loophole. The existing mismatch-rejects guards already caught individual mismatches; this closes the pathological both-supplied-one-blank-expected corner. handleBiometricEnroll always commits ExpectedCredentialHash (derived from the enrollment pubKey via sha256), so the LAX floor always engages in practice for WebAuthn enrollment — this hardens what is already the exercised path. Test coverage (pkg/webauthn/liveness_test.go): - TestLiveness_R38_LaxRejectsMissingCredentialHash — primary F4 gate: envelope with only challengeId, missing credentialHash → reject - TestLiveness_R38_LaxAcceptsBothMatch — happy path (extended envelope after SecureGate rollout) - TestLiveness_R38_LaxRejectsMismatchedCredentialHashBothPresent - TestLiveness_R38_LaxRejectsMismatchedChallengeIDBothPresent - TestLiveness_R38_LaxAcceptsCredentialHashOnly — current SecureGate envelope shape still accepted - TestLiveness_R38_LaxRejectsMissingBindingWhenCredentialHashExpected — replaces the old lax-warns-missing path - TestLiveness_R38_LaxWarnsOnMissing_ChallengeIDOnly — warn path still available when credentialHash is not expected Full pkg/webauthn suite green (18/18). pkg/api regression tests pass. pkg/custody pre-existing failures unrelated to this change (SQLite json_extract missing, blame predates F4). LLM.md entry R35 documents the full set. Cut v1.7.1 on merge.
2026-09-24 00:26:24 +00:00 · 2026-04-18 18:05:11 -07:00
parent d6e0d7c2a8
commit cd9814bd27
3 changed files with 232 additions and 9 deletions
@@ -475,7 +475,22 @@ make e2e-test

 33. **R3-2 No `*` wildcard in API-key permissions**: `hasPermission` no longer honors `*`. Every API key must enumerate explicit permissions. Tests `TestRequireRoleOrAPIPermission_WildcardRejectedOnSensitive` + `TestRequirePermission_WildcardRejected` assert `permissions=["*"]` is REJECTED for every sensitive name (`mpc:sign`, `mpc:settlement:sign`, `mpc:operations:approve`, `mpc:wallet:sweep`, `mpc:policy:write`, plus `trade:submit`). Breaking change: re-issue keys with explicit permissions; bump to v1.7.0.

-34. **R3-8 Liveness envelope bound to enrollment**: `LivenessAttestation` now carries `credentialHash = sha256(pubKey)` and/or `challengeId`. `handleBiometricEnroll` passes expected values to `VerifyLiveness` so a stolen envelope cannot be replayed against a different WebAuthn public key. Mode toggle via `MPC_LIVENESS_BINDING=strict|lax` (default `strict`). **Blocker for mainnet biometric enroll**: SecureGate must ship the extended envelope; while in LAX the server emits a `mpc.biometric.binding_warn` log on every permissive accept. Tests in `pkg/webauthn/liveness_test.go` — matching-hash accept, mismatch-rejects, strict-rejects-missing, lax-warns-missing, challengeId accept/reject.
+34. **R3-8 Liveness envelope bound to enrollment**: `LivenessAttestation` now carries `credentialHash = sha256(pubKey)` and/or `challengeId`. `handleBiometricEnroll` passes expected values to `VerifyLiveness` so a stolen envelope cannot be replayed against a different WebAuthn public key. Mode toggle via `MPC_LIVENESS_BINDING=strict|lax` (default `strict`). **Blocker for mainnet biometric enroll**: SecureGate must ship the extended envelope; while in LAX the server emits a `mpc.biometric.binding_warn` log on every permissive accept.
+
+35. **F4 (2026-04-18) LAX tightening — credentialHash floor + both-must-match**: Red round 4 flagged that the previous LAX mode accepted envelopes with ONLY `challengeId` (no `credentialHash`), enabling a cross-ceremony replay: an attacker observing a fresh challenge could trick SecureGate into signing an envelope binding to the challenge alone and replay it against any enrollment for the same `userId`. Two tightenings in `VerifyLiveness`:
+    - LAX + server expects `credentialHash` + envelope omits it → **reject** (`credentialHash required in LAX mode`). `credentialHash` is now a hard floor — `challengeId` alone cannot satisfy the binding requirement in LAX.
+    - Envelope supplies BOTH `credentialHash` AND `challengeId` AND server expects both → require BOTH to match (no more "either-or" loophole).
+
+    LAX warn-on-missing still works when the call site does NOT commit a `credentialHash` expectation. Tests in `pkg/webauthn/liveness_test.go`:
+    - `TestLiveness_R38_LaxRejectsMissingCredentialHash` — challengeId-only envelope rejected
+    - `TestLiveness_R38_LaxAcceptsBothMatch` — happy path (extended envelope after SecureGate rollout)
+    - `TestLiveness_R38_LaxRejectsMismatchedCredentialHashBothPresent`
+    - `TestLiveness_R38_LaxRejectsMismatchedChallengeIDBothPresent`
+    - `TestLiveness_R38_LaxAcceptsCredentialHashOnly` — current SecureGate envelope shape
+    - `TestLiveness_R38_LaxRejectsMissingBindingWhenCredentialHashExpected` — replaces the old lax-warns-missing path
+    - `TestLiveness_R38_LaxWarnsOnMissing_ChallengeIDOnly` — warn path still available when `credentialHash` not expected
+
+    `handleBiometricEnroll` always commits `ExpectedCredentialHash` (derived from the enrollment pubKey via `sha256`), so the LAX floor always engages for WebAuthn enrollment. `v1.7.1` follows `v1.7.0`.

 ### Consensus-Embedded Transport (Jan 2026)

@@ -154,7 +154,7 @@ func VerifyLiveness(envelopeB64 string, opts *LivenessOpts) (*LivenessAttestatio
 		return nil, fmt.Errorf("liveness: attestation too old: age=%s max=%s", age, opts.MaxAge)
 	}

-	// R3-8: envelope→enrollment binding. Only applies when the caller
+	// R3-8 / F4: envelope→enrollment binding. Only applies when the caller
 	// supplies a binding expectation (ExpectedCredentialHash and/or
 	// ExpectedChallengeID). Callers that don't care about binding (e.g.
 	// generic liveness checks unrelated to WebAuthn enrollment) are
@@ -169,6 +169,55 @@ func VerifyLiveness(envelopeB64 string, opts *LivenessOpts) (*LivenessAttestatio
 		if att.ChallengeID != "" && att.ChallengeID != opts.ExpectedChallengeID && opts.ExpectedChallengeID != "" {
 			return nil, errors.New("liveness: challengeId mismatch — envelope not bound to this ceremony")
 		}
+
+		// F4 (2026-04-18): tighten LAX. Previously LAX accepted an
+		// envelope where EITHER field matched. That permitted a
+		// cross-ceremony replay: an envelope whose challengeId matches
+		// the current ceremony's challenge but whose credentialHash is
+		// absent would be accepted — an attacker who observes a fresh
+		// challenge can trick SecureGate into signing an envelope that
+		// binds to the challenge alone, then replay it against any
+		// ceremony for the same userId that reuses an uncaptured
+		// credentialHash expectation.
+		//
+		// Two tightenings (both layered on top of the existing
+		// "mismatch always rejects" guards above):
+		//
+		//   1. When BOTH expected fields are set AND the envelope
+		//      supplies BOTH, require BOTH to match. This closes the
+		//      "either-or" loophole for the happy path where SecureGate
+		//      has already rolled out the extended envelope.
+		//
+		//   2. credentialHash is mandatory at minimum in LAX. The
+		//      envelope MUST carry CredentialHash whenever the server
+		//      expects one. challengeId alone is insufficient — it can
+		//      be observed and replayed faster than credentialHash can
+		//      be forged against an attacker-controlled pubkey.
+		//
+		// STRICT already enforced the missing-both rejection; these
+		// rules add the LAX-only refinements.
+		if att.CredentialHash != "" && att.ChallengeID != "" &&
+			opts.ExpectedCredentialHash != "" && opts.ExpectedChallengeID != "" {
+			// Both sides have both fields — require both to match.
+			// The prior "mismatch rejects" guards have already caught
+			// any individual mismatch; this is a belt-and-braces
+			// assertion that closes the pathological both-supplied-
+			// one-blank-expected corner.
+			if att.CredentialHash != opts.ExpectedCredentialHash ||
+				att.ChallengeID != opts.ExpectedChallengeID {
+				return nil, errors.New("liveness: binding mismatch — both credentialHash and challengeId must match when both are present")
+			}
+		}
+
+		if opts.BindingMode == BindingLax &&
+			opts.ExpectedCredentialHash != "" &&
+			att.CredentialHash == "" {
+			// F4: LAX + expected credentialHash + envelope omits it =
+			// reject. This is the "challengeId-only replay" gate. The
+			// envelope must commit to the enrollment pubkey hash.
+			return nil, errors.New("liveness: credentialHash required in LAX mode — envelope without credentialHash is replay-vulnerable")
+		}
+
 		// A "matching" envelope has at least one field that matches its
 		// corresponding expected value. In STRICT mode, a missing field
 		// is rejected; in LAX mode we warn and accept.
@@ -175,9 +175,34 @@ func TestLiveness_R38_RejectsMissingBindingStrict(t *testing.T) {
 	}
 }

-// LAX: server expected binding, envelope missing — accept with warning.
-// Used during SecureGate extended-envelope rollout.
-func TestLiveness_R38_LaxWarnsOnMissing(t *testing.T) {
+// F4 (2026-04-18): LAX + server expects credentialHash + envelope carries
+// NEITHER binding field → now rejects. Pre-F4 this was accept-with-warn;
+// Red round 4 flagged that as a cross-ceremony replay window. The
+// tightening makes credentialHash a hard floor in LAX.
+func TestLiveness_R38_LaxRejectsMissingBindingWhenCredentialHashExpected(t *testing.T) {
+	pub, priv, _ := ed25519.GenerateKey(rand.Reader)
+	att := LivenessAttestation{
+		ProviderID: "securegate",
+		UserID:     "u-1",
+		Score:      0.9,
+		Timestamp:  time.Now().Unix(),
+		// intentionally empty — pre-R3-8 envelope shape
+	}
+	env := signEnvelope(t, priv, att)
+	_, err := VerifyLiveness(env, &LivenessOpts{
+		PubKey: pub, UserID: "u-1", MinScore: 0.8, MaxAge: time.Minute,
+		ExpectedCredentialHash: "VICTIM==",
+		BindingMode:            BindingLax,
+	})
+	if err == nil || !strings.Contains(err.Error(), "credentialHash required in LAX mode") {
+		t.Fatalf("F4: LAX + expected credentialHash + envelope empty must reject: got %v", err)
+	}
+}
+
+// F4: the LAX warn path survives ONLY when the server does NOT expect a
+// credentialHash (e.g. a pure challengeId-binding call site). Envelope
+// missing is then accepted with a warning, as before.
+func TestLiveness_R38_LaxWarnsOnMissing_ChallengeIDOnly(t *testing.T) {
 	pub, priv, _ := ed25519.GenerateKey(rand.Reader)
 	att := LivenessAttestation{
 		ProviderID: "securegate",
@@ -189,12 +214,14 @@ func TestLiveness_R38_LaxWarnsOnMissing(t *testing.T) {
 	var warned string
 	_, err := VerifyLiveness(env, &LivenessOpts{
 		PubKey: pub, UserID: "u-1", MinScore: 0.8, MaxAge: time.Minute,
-		ExpectedCredentialHash: "VICTIM==",
-		BindingMode:            BindingLax,
-		WarnFn:                 func(m string) { warned = m },
+		// NOTE: only ExpectedChallengeID is set — the LAX warn path is
+		// reserved for call sites that never commit credentialHash.
+		ExpectedChallengeID: "chal-xyz",
+		BindingMode:         BindingLax,
+		WarnFn:              func(m string) { warned = m },
 	})
 	if err != nil {
-		t.Fatalf("LAX should accept missing binding: %v", err)
+		t.Fatalf("LAX + challengeId-only expectation + missing binding must warn-accept: %v", err)
 	}
 	if warned == "" {
 		t.Fatalf("LAX must emit a warn message on missing binding")
@@ -242,3 +269,135 @@ func TestLiveness_R38_RejectsMismatchedChallengeID(t *testing.T) {
 		t.Fatalf("mismatched challengeId must reject: got %v", err)
 	}
 }
+
+// F4 (2026-04-18): LAX tightening — credentialHash is mandatory floor.
+//
+// Previous LAX behavior accepted an envelope that had ONLY challengeId and
+// no credentialHash, so long as the challengeId matched. An attacker
+// observing a fresh challenge could persuade SecureGate to sign an
+// envelope binding to the challenge alone and replay it against any
+// enrollment for the same userId. The challengeId binds the ceremony;
+// the credentialHash binds the specific pubkey being enrolled. Without
+// the credentialHash floor there is no binding to the pubkey — that is
+// the cross-ceremony replay Red flagged in round 4.
+
+// LAX + envelope omits credentialHash (only challengeId) → reject.
+// This is the primary F4 gate.
+func TestLiveness_R38_LaxRejectsMissingCredentialHash(t *testing.T) {
+	pub, priv, _ := ed25519.GenerateKey(rand.Reader)
+	att := LivenessAttestation{
+		ProviderID:  "securegate",
+		UserID:      "u-1",
+		Score:       0.9,
+		Timestamp:   time.Now().Unix(),
+		ChallengeID: "chal-xyz", // challengeId alone is NOT enough in LAX
+	}
+	env := signEnvelope(t, priv, att)
+	_, err := VerifyLiveness(env, &LivenessOpts{
+		PubKey: pub, UserID: "u-1", MinScore: 0.8, MaxAge: time.Minute,
+		ExpectedCredentialHash: "VICTIM-HASH",
+		ExpectedChallengeID:    "chal-xyz",
+		BindingMode:            BindingLax,
+	})
+	if err == nil || !strings.Contains(err.Error(), "credentialHash required in LAX mode") {
+		t.Fatalf("LAX + missing credentialHash must reject: got %v", err)
+	}
+}
+
+// LAX + both fields present + both match → accept (happy path for the
+// extended envelope post-SecureGate-rollout).
+func TestLiveness_R38_LaxAcceptsBothMatch(t *testing.T) {
+	pub, priv, _ := ed25519.GenerateKey(rand.Reader)
+	att := LivenessAttestation{
+		ProviderID:     "securegate",
+		UserID:         "u-1",
+		Score:          0.9,
+		Timestamp:      time.Now().Unix(),
+		CredentialHash: "HASH-OK",
+		ChallengeID:    "chal-ok",
+	}
+	env := signEnvelope(t, priv, att)
+	_, err := VerifyLiveness(env, &LivenessOpts{
+		PubKey: pub, UserID: "u-1", MinScore: 0.8, MaxAge: time.Minute,
+		ExpectedCredentialHash: "HASH-OK",
+		ExpectedChallengeID:    "chal-ok",
+		BindingMode:            BindingLax,
+	})
+	if err != nil {
+		t.Fatalf("LAX + both present + both match must accept: %v", err)
+	}
+}
+
+// LAX + both fields present + credentialHash mismatch → reject. This is
+// the existing "mismatch always rejects" rule; the test makes it explicit
+// for LAX to prove the tightening didn't regress.
+func TestLiveness_R38_LaxRejectsMismatchedCredentialHashBothPresent(t *testing.T) {
+	pub, priv, _ := ed25519.GenerateKey(rand.Reader)
+	att := LivenessAttestation{
+		ProviderID:     "securegate",
+		UserID:         "u-1",
+		Score:          0.9,
+		Timestamp:      time.Now().Unix(),
+		CredentialHash: "HASH-ATTACKER",
+		ChallengeID:    "chal-ok",
+	}
+	env := signEnvelope(t, priv, att)
+	_, err := VerifyLiveness(env, &LivenessOpts{
+		PubKey: pub, UserID: "u-1", MinScore: 0.8, MaxAge: time.Minute,
+		ExpectedCredentialHash: "HASH-VICTIM",
+		ExpectedChallengeID:    "chal-ok",
+		BindingMode:            BindingLax,
+	})
+	if err == nil || !strings.Contains(err.Error(), "credentialHash mismatch") {
+		t.Fatalf("LAX + mismatched credentialHash must reject: got %v", err)
+	}
+}
+
+// LAX + both fields present + challengeId mismatch → reject.
+func TestLiveness_R38_LaxRejectsMismatchedChallengeIDBothPresent(t *testing.T) {
+	pub, priv, _ := ed25519.GenerateKey(rand.Reader)
+	att := LivenessAttestation{
+		ProviderID:     "securegate",
+		UserID:         "u-1",
+		Score:          0.9,
+		Timestamp:      time.Now().Unix(),
+		CredentialHash: "HASH-OK",
+		ChallengeID:    "chal-ATTACKER",
+	}
+	env := signEnvelope(t, priv, att)
+	_, err := VerifyLiveness(env, &LivenessOpts{
+		PubKey: pub, UserID: "u-1", MinScore: 0.8, MaxAge: time.Minute,
+		ExpectedCredentialHash: "HASH-OK",
+		ExpectedChallengeID:    "chal-VICTIM",
+		BindingMode:            BindingLax,
+	})
+	if err == nil || !strings.Contains(err.Error(), "challengeId mismatch") {
+		t.Fatalf("LAX + mismatched challengeId must reject: got %v", err)
+	}
+}
+
+// LAX + credentialHash only (no challengeId) + match → accept.
+// This is the "SecureGate not yet rolled out challengeId" case. The
+// mandatory credentialHash floor is satisfied, so the envelope carries
+// enough binding to be safe.
+func TestLiveness_R38_LaxAcceptsCredentialHashOnly(t *testing.T) {
+	pub, priv, _ := ed25519.GenerateKey(rand.Reader)
+	att := LivenessAttestation{
+		ProviderID:     "securegate",
+		UserID:         "u-1",
+		Score:          0.9,
+		Timestamp:      time.Now().Unix(),
+		CredentialHash: "HASH-OK",
+		// no ChallengeID — simulates the current SecureGate envelope
+	}
+	env := signEnvelope(t, priv, att)
+	_, err := VerifyLiveness(env, &LivenessOpts{
+		PubKey: pub, UserID: "u-1", MinScore: 0.8, MaxAge: time.Minute,
+		ExpectedCredentialHash: "HASH-OK",
+		ExpectedChallengeID:    "chal-xyz",
+		BindingMode:            BindingLax,
+	})
+	if err != nil {
+		t.Fatalf("LAX + credentialHash-only + match must accept: %v", err)
+	}
+}