fix: detect atomic passkey counter anomalies (Closes #641)
All checks were successful
CI / lint (pull_request) Successful in 1m34s
CI / build-release (pull_request) Successful in 6s
CI / release-candidate (pull_request) Has been skipped

This commit is contained in:
timmy 2026-08-12 10:26:51 +00:00
parent 5806d47b61
commit c91194c283
7 changed files with 225 additions and 7 deletions

View File

@ -271,7 +271,12 @@ session linked to the credential is revoked atomically; unrelated credentials an
sessions remain valid. Removing the current device's passkey keeps its current sessions remain valid. Removing the current device's passkey keeps its current
session active, so the sheet warns that the recovery token will be required after session active, so the sheet warns that the recovery token will be required after
sign-out. Remotely revoking an enrolled device also deletes its passkey; signing out sign-out. Remotely revoking an enrolled device also deletes its passkey; signing out
normally keeps the passkey available for the next sign-in. normally keeps the passkey available for the next sign-in. Passkey assertion counters
advance with an atomic compare-and-swap across workers: stale or equal nonzero counters
fail before a session or authorization grant is issued, while counterless authenticators
remain compatible. The bounded **Passkey counter anomaly** Security activity entry names
only the device label and attempted action; repeated alerts should prompt removal and
re-enrollment of that passkey.
High-impact actions—merging a pull request, closing an assigned issue, revoking a High-impact actions—merging a pull request, closing an assigned issue, revoking a
remote device, or signing out every device—require a passkey assertion or the remote device, or signing out every device—require a passkey assertion or the

View File

@ -128,6 +128,7 @@
} }
const labels = { const labels = {
sign_in: 'Signed in', sign_out: 'Signed out', passkey_enrolled: 'Passkey enrolled', sign_in: 'Signed in', sign_out: 'Signed out', passkey_enrolled: 'Passkey enrolled',
passkey_counter_anomaly: 'Passkey counter anomaly',
device_revoked: 'Device access revoked', all_sessions_revoked: 'All device access revoked', device_revoked: 'Device access revoked', all_sessions_revoked: 'All device access revoked',
issue_closed: 'Issue closed', pull_merged: 'Pull request merged', issue_closed: 'Issue closed', pull_merged: 'Pull request merged',
pull_review_approved: 'Pull request approved', pull_review_approved: 'Pull request approved',
@ -141,6 +142,9 @@
const details = root.document.createElement('span'); const details = root.document.createElement('span');
details.className = 'small muted'; details.className = 'small muted';
const context = [event.device_label, event.method, event.target, const context = [event.device_label, event.method, event.target,
event.kind === 'passkey_counter_anomaly'
? 'Remove and re-enroll this passkey if the alert repeats'
: null,
event.status === 'pending' ? 'Outcome confirmation pending' : null] event.status === 'pending' ? 'Outcome confirmation pending' : null]
.filter(value => typeof value === 'string' && value).join(' · '); .filter(value => typeof value === 'string' && value).join(' · ');
details.textContent = `${new Date(event.created_at * 1000).toLocaleString()}${context ? ' · ' + context : ''}`; details.textContent = `${new Date(event.created_at * 1000).toLocaleString()}${context ? ' · ' + context : ''}`;

View File

@ -1595,9 +1595,19 @@ async def verify_passkey_authorization(
stored=stored, stored=stored,
) )
updated = await asyncio.to_thread( updated = await asyncio.to_thread(
store.update_counter, stored.credential_id, verified.new_sign_count store.advance_counter,
stored.credential_id,
expected=stored.sign_count,
new=verified.new_sign_count,
) )
if not updated: if not updated:
await asyncio.to_thread(
_security_event_store().record,
"passkey_counter_anomaly",
method="passkey",
device_label=stored.device_label,
target=f"{payload.action}:{payload.target}",
)
raise ValueError("stale passkey counter") raise ValueError("stale passkey counter")
grant = await dashboard_auth.issue_step_up( grant = await dashboard_auth.issue_step_up(
request.state.dashboard_session, request.state.dashboard_session,
@ -1686,9 +1696,19 @@ async def verify_passkey_authentication(
stored=stored, stored=stored,
) )
updated = await asyncio.to_thread( updated = await asyncio.to_thread(
store.update_counter, stored.credential_id, verified.new_sign_count store.advance_counter,
stored.credential_id,
expected=stored.sign_count,
new=verified.new_sign_count,
) )
if not updated: if not updated:
await asyncio.to_thread(
_security_event_store().record,
"passkey_counter_anomaly",
method="passkey",
device_label=stored.device_label,
target="sign_in:dashboard",
)
raise ValueError("stale passkey counter") raise ValueError("stale passkey counter")
await dashboard_auth.revoke_managed_session(stored.management_id) await dashboard_auth.revoke_managed_session(stored.management_id)
signed, session = await asyncio.to_thread( signed, session = await asyncio.to_thread(

View File

@ -225,13 +225,26 @@ class PasskeyStore:
raise SessionStoreError("Passkey registry is temporarily unavailable") from exc raise SessionStoreError("Passkey registry is temporarily unavailable") from exc
return StoredPasskey(*row) if row else None return StoredPasskey(*row) if row else None
def update_counter(self, credential_id: bytes, new_sign_count: int) -> bool: def advance_counter(
self, credential_id: bytes, *, expected: int, new: int
) -> bool:
if expected == 0 and new == 0:
comparison = "sign_count = 0"
elif new > expected:
comparison = "sign_count = ?"
else:
return False
try: try:
with self._connect() as connection: with self._connect() as connection:
parameters = (
(new, credential_id)
if expected == 0 and new == 0
else (new, credential_id, expected)
)
cursor = connection.execute( cursor = connection.execute(
"UPDATE passkey_credentials SET sign_count = ? " f"UPDATE passkey_credentials SET sign_count = ? "
"WHERE credential_id = ? AND sign_count <= ?", f"WHERE credential_id = ? AND {comparison}",
(new_sign_count, credential_id, new_sign_count), parameters,
) )
return cursor.rowcount == 1 return cursor.rowcount == 1
except (OSError, sqlite3.Error) as exc: except (OSError, sqlite3.Error) as exc:

View File

@ -170,6 +170,102 @@ async def test_enrolled_passkey_can_sign_in_without_the_operator_token(
assert "correct horse battery staple" not in signed_in.text assert "correct horse battery staple" not in signed_in.text
@pytest.mark.anyio
async def test_stale_nonzero_passkey_counter_denies_sign_in_and_records_anomaly(
access_control, monkeypatch
):
class StaleAuthentication:
new_sign_count = 4
monkeypatch.setattr(
main.passkeys,
"verify_authentication",
lambda **_kwargs: StaleAuthentication(),
)
await asyncio.to_thread(
main._passkey_store().register,
credential_id=b"phone-credential",
public_key=b"phone-public-key",
sign_count=4,
device_label="Phone",
management_id="phone-management-id",
)
transport = httpx.ASGITransport(app=main.app)
async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
options = await client.post("/api/v1/passkeys/authentication/options")
denied = await client.post(
"/api/v1/passkeys/authentication/verify",
json={
"challenge": options.json()["challenge"],
"credential": {"id": "cGhvbmUtY3JlZGVudGlhbA"},
"device_label": "Phone",
"action": "sign_in",
"target": "dashboard",
},
)
events = main._security_event_store().list(limit=10).events
assert denied.status_code == 401
assert "stackchain_session=" not in denied.headers.get("set-cookie", "")
assert [event.kind for event in events] == ["passkey_counter_anomaly"]
assert events[0].device_label == "Phone"
assert events[0].target == "sign_in:dashboard"
@pytest.mark.anyio
async def test_stale_nonzero_passkey_counter_denies_fresh_authorization(
access_control, monkeypatch
):
class StaleAuthentication:
new_sign_count = 4
monkeypatch.setattr(
main.passkeys,
"verify_authentication",
lambda **_kwargs: StaleAuthentication(),
)
await asyncio.to_thread(
main._passkey_store().register,
credential_id=b"phone-credential",
public_key=b"phone-public-key",
sign_count=4,
device_label="Phone",
management_id="phone-management-id",
)
transport = httpx.ASGITransport(app=main.app)
async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
await client.post(
"/api/v1/session",
json={"access_token": "correct horse battery staple", "device_label": "Phone"},
)
headers = {
"Origin": "https://test",
"X-CSRF-Token": client.cookies["stackchain_csrf"],
}
options = await client.post(
"/api/v1/passkeys/authorization/options",
json={"action": "merge_pull", "target": "stackchain/dashboard#42"},
headers=headers,
)
denied = await client.post(
"/api/v1/passkeys/authorization/verify",
json={
"challenge": options.json()["challenge"],
"credential": {"id": "cGhvbmUtY3JlZGVudGlhbA"},
"action": "merge_pull",
"target": "stackchain/dashboard#42",
},
headers=headers,
)
events = main._security_event_store().list(limit=10).events
assert options.status_code == 200
assert denied.status_code == 401
assert "grant" not in denied.json()
assert events[0].kind == "passkey_counter_anomaly"
assert events[0].target == "merge_pull:stackchain/dashboard#42"
@pytest.mark.anyio @pytest.mark.anyio
async def test_passkey_options_are_source_limited_before_challenge_generation( async def test_passkey_options_are_source_limited_before_challenge_generation(
access_control, monkeypatch access_control, monkeypatch

View File

@ -104,3 +104,44 @@ def test_existing_challenge_registry_is_migrated_without_losing_live_challenges(
).fetchall() ).fetchall()
assert "source_hash" in columns assert "source_hash" in columns
assert sorted(rows) == sorted([("existing",), (store._digest(b"new"),)]) assert sorted(rows) == sorted([("existing",), (store._digest(b"new"),)])
def test_passkey_counter_advancement_is_atomic_across_store_instances(tmp_path):
database = tmp_path / "passkeys.sqlite3"
first = PasskeyStore(database, clock=lambda: 1_000.0)
second = PasskeyStore(database, clock=lambda: 1_000.0)
first.register(
credential_id=b"phone-credential",
public_key=b"phone-public-key",
sign_count=4,
device_label="Phone",
management_id="phone-management-id",
)
assert first.advance_counter(b"phone-credential", expected=4, new=5) is True
assert second.advance_counter(b"phone-credential", expected=4, new=5) is False
assert first.get(b"phone-credential").sign_count == 5
def test_passkey_counter_rejects_non_advancing_values_but_supports_counterless_devices(
tmp_path,
):
store = PasskeyStore(tmp_path / "passkeys.sqlite3", clock=lambda: 1_000.0)
store.register(
credential_id=b"phone-credential",
public_key=b"phone-public-key",
sign_count=4,
device_label="Phone",
management_id="phone-management-id",
)
store.register(
credential_id=b"counterless-credential",
public_key=b"counterless-public-key",
sign_count=0,
device_label="Security key",
management_id="counterless-management-id",
)
assert store.advance_counter(b"phone-credential", expected=4, new=4) is False
assert store.advance_counter(b"phone-credential", expected=4, new=3) is False
assert store.advance_counter(b"counterless-credential", expected=0, new=0) is True

View File

@ -47,3 +47,42 @@ const boundary={{
"hidden": False, "hidden": False,
"focused": True, "focused": True,
} }
def test_counter_anomaly_activity_gives_passkey_remediation_guidance():
harness = f"""
const attachSecurityCenter=require({json.dumps(str(SECURITY_CENTER))});
const element=()=>({{
hidden:true, disabled:false, textContent:'', children:[], className:'',
addEventListener(){{}}, focus(){{}}, replaceChildren(){{this.children=[];}},
append(...values){{this.children.push(...values);}},
}});
const ids={{
'active-devices':element(), 'active-devices-sheet':element(),
'active-devices-list':element(), 'active-devices-status':element(),
'enrolled-passkeys-list':element(), 'enrolled-passkeys-status':element(),
'enroll-passkey':element(), 'security-activity-list':element(),
'security-activity-status':element(), 'load-more-security-activity':element(),
'close-active-devices':element(),
}};
const root={{document:{{getElementById:id=>ids[id]||null,createElement:()=>element()}}}};
const boundary={{
listActiveDevices:async()=>[], listPasskeys:async()=>[],
listSecurityEvents:async()=>({{events:[{{
kind:'passkey_counter_anomaly', device_label:'Phone', method:'passkey',
target:'sign_in:dashboard', status:'completed', created_at:1,
}}],authentication_alerts:[],next_cursor:null}}),
}};
(async()=>{{
await attachSecurityCenter({{root,boundary}}).open();
const row=ids['security-activity-list'].children[0];
console.log(JSON.stringify(row.children.map(child=>child.textContent)));
}})().catch(error=>{{console.error(error);process.exit(1);}});
"""
completed = subprocess.run(
["node", "-e", harness], check=True, capture_output=True, text=True
)
title, detail = json.loads(completed.stdout)
assert title == "Passkey counter anomaly"
assert "Remove and re-enroll this passkey" in detail