Detect and surface atomic passkey counter anomalies #642
|
|
@ -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
|
||||
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
|
||||
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
|
||||
remote device, or signing out every device—require a passkey assertion or the
|
||||
|
|
|
|||
|
|
@ -128,6 +128,7 @@
|
|||
}
|
||||
const labels = {
|
||||
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',
|
||||
issue_closed: 'Issue closed', pull_merged: 'Pull request merged',
|
||||
pull_review_approved: 'Pull request approved',
|
||||
|
|
@ -141,6 +142,9 @@
|
|||
const details = root.document.createElement('span');
|
||||
details.className = 'small muted';
|
||||
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]
|
||||
.filter(value => typeof value === 'string' && value).join(' · ');
|
||||
details.textContent = `${new Date(event.created_at * 1000).toLocaleString()}${context ? ' · ' + context : ''}`;
|
||||
|
|
|
|||
24
src/main.py
24
src/main.py
|
|
@ -1595,9 +1595,19 @@ async def verify_passkey_authorization(
|
|||
stored=stored,
|
||||
)
|
||||
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:
|
||||
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")
|
||||
grant = await dashboard_auth.issue_step_up(
|
||||
request.state.dashboard_session,
|
||||
|
|
@ -1686,9 +1696,19 @@ async def verify_passkey_authentication(
|
|||
stored=stored,
|
||||
)
|
||||
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:
|
||||
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")
|
||||
await dashboard_auth.revoke_managed_session(stored.management_id)
|
||||
signed, session = await asyncio.to_thread(
|
||||
|
|
|
|||
|
|
@ -225,13 +225,26 @@ class PasskeyStore:
|
|||
raise SessionStoreError("Passkey registry is temporarily unavailable") from exc
|
||||
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:
|
||||
with self._connect() as connection:
|
||||
parameters = (
|
||||
(new, credential_id)
|
||||
if expected == 0 and new == 0
|
||||
else (new, credential_id, expected)
|
||||
)
|
||||
cursor = connection.execute(
|
||||
"UPDATE passkey_credentials SET sign_count = ? "
|
||||
"WHERE credential_id = ? AND sign_count <= ?",
|
||||
(new_sign_count, credential_id, new_sign_count),
|
||||
f"UPDATE passkey_credentials SET sign_count = ? "
|
||||
f"WHERE credential_id = ? AND {comparison}",
|
||||
parameters,
|
||||
)
|
||||
return cursor.rowcount == 1
|
||||
except (OSError, sqlite3.Error) as exc:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
@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
|
||||
async def test_passkey_options_are_source_limited_before_challenge_generation(
|
||||
access_control, monkeypatch
|
||||
|
|
|
|||
|
|
@ -104,3 +104,44 @@ def test_existing_challenge_registry_is_migrated_without_losing_live_challenges(
|
|||
).fetchall()
|
||||
assert "source_hash" in columns
|
||||
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
|
||||
|
|
|
|||
|
|
@ -47,3 +47,42 @@ const boundary={{
|
|||
"hidden": False,
|
||||
"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
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user