From ad51b2548a4bc74d2604a5626b645122f6d7eec1 Mon Sep 17 00:00:00 2001 From: leora Date: Tue, 6 Oct 2026 06:27:46 +0800 Subject: [PATCH] Fix N7/N8: human session fingerprint epoch; drop duplicate view_ttl resolve() now requires sess_fp == live_fp for every session that carries a fingerprint (including human_via_agent), so a redeem-vs-rekey race cannot leave a view:read bearer alive. Drop the duplicate view_ttl key in issue_code's return dict. Regression tests cover both. --- DESIGN.md | 13 ++++++------ README.md | 2 +- sigil/sessions.py | 13 +++++++----- sigil/viewcodes.py | 1 - tests/test_hardening.py | 44 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 60 insertions(+), 13 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index f8c758d..5a065eb 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -141,11 +141,12 @@ close every door the old key could open. A narrow check-then-insert race can still insert a session or code after the revoke/rekey statement has run. The choke point is `sessions.resolve`: every -use re-checks that the agent is not revoked, still has a live key, and (for -agent sessions) that the session fingerprint matches the current key epoch. -View codes store the issuing fingerprint and refuse redeem after rekey or key -wipe. That is what makes revocation overtake in-flight verify/issue/redeem, -not the first-pass invalidation alone. +use re-checks that the agent is not revoked, still has a live key, and that +the session fingerprint (when present — always for agent sessions, and for +human_via_agent sessions minted with a fingerprint) matches the current key +epoch. View codes store the issuing fingerprint and refuse redeem after rekey +or key wipe. That is what makes revocation overtake in-flight +verify/issue/redeem, not the first-pass invalidation alone. ### Existing controls stay @@ -230,7 +231,7 @@ because the review looked for it. ## Status -156 passing tests, and two hardening passes that closed real defects. The worst were +158 passing tests, and two hardening passes that closed real defects. The worst were concurrency: one-time codes were redeemable repeatedly (753 sessions from 200 codes under load), and a bootstrap secret could enrol more than one key. Those are now atomic compare-and-set operations, each covered by a test that races diff --git a/README.md b/README.md index 072c76a..fbde7dc 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ python3 sigil.py sweep # prune stale rows python3 -m unittest discover -s tests -v ``` -156 tests. They cover the refusals, not just the happy paths: replayed +158 tests. They cover the refusals, not just the happy paths: replayed signatures, wrong keys, expired nonces, single-use codes, scope escalation, and the invariant below. `test_hardening.py`, `test_production.py`, `test_anchor.py` and `test_sealed.py` have one test per defect found in review, so any of them diff --git a/sigil/sessions.py b/sigil/sessions.py index 8ee0c64..390e641 100644 --- a/sigil/sessions.py +++ b/sigil/sessions.py @@ -207,11 +207,14 @@ def resolve(store: Store, token: str) -> dict: raise errors.SessionExpired("agent revoked") if not _identity.has_live_key(agent_row): raise errors.SessionExpired("agent has no live key") - # Agent sessions are bound to the key epoch they were minted against. - # After rekey, old agent:act bearers must die even if revoke_sessions raced. - if row["kind"] == AGENT: - sess_fp = row["fingerprint"] or "" - live_fp = agent_row["fingerprint"] or "" + # Every session that carries a fingerprint is bound to that key epoch. + # After rekey, race-surviving agent:act *and* human_via_agent bearers die + # even if revoke_sessions missed them (N7). + sess_fp = row["fingerprint"] or "" + live_fp = agent_row["fingerprint"] or "" + if sess_fp or row["kind"] == AGENT: + # Agent sessions always require a matching fingerprint; human sessions + # require a match whenever a fingerprint was recorded at mint time. if not sess_fp or sess_fp != live_fp: raise errors.SessionExpired("session key epoch no longer current") diff --git a/sigil/viewcodes.py b/sigil/viewcodes.py index 0ba58d9..1d85b17 100644 --- a/sigil/viewcodes.py +++ b/sigil/viewcodes.py @@ -144,7 +144,6 @@ def issue_code( "code_ttl": code_ttl, "view_ttl": view_ttl, "redeem_expires_at": now + code_ttl, - "view_ttl": view_ttl, } diff --git a/tests/test_hardening.py b/tests/test_hardening.py index 96bd3ff..fca9f5e 100644 --- a/tests/test_hardening.py +++ b/tests/test_hardening.py @@ -528,3 +528,47 @@ class TestResolveRechecksAgentState(unittest.TestCase): identity.revoke(store, "bigpickle", reason="compromise") with self.assertRaises(errors.SessionExpired): sessions.resolve(store, sess["token"]) + + +class TestHumanSessionFingerprintEpoch(unittest.TestCase): + """N7: rekey must kill race-surviving human_via_agent sessions too.""" + + def test_resolve_refuses_stale_human_fingerprint_after_rekey(self): + store, private = helpers.fresh() + old_fp = store.get_agent("bigpickle")["fingerprint"] + # Plant a human session as if redeem's put_session won a race against rekey. + import time as _t + token = "human-race-survivor" + store.put_session( + sessions.hash_token(token), + sessions.HUMAN_VIA_AGENT, + "bigpickle's human", + "bigpickle", + list(sessions.HUMAN_VIEW_SCOPES), + int(_t.time()) + 3600, + granted_by="bigpickle", + fingerprint=old_fp, + ) + new_pub = keys.generate()[1] + secret = identity.seed_bootstrap(store, "bigpickle") + identity.enrol_rekey( + store, "bigpickle", keys.export_public_b64(new_pub), secret + ) + # Clear revoked_at that rekey set, to isolate the fingerprint check. + store.conn.execute( + "UPDATE sessions SET revoked_at = NULL, revoked_reason = NULL" + " WHERE token_hash = ?", + (sessions.hash_token(token),), + ) + store.conn.commit() + with self.assertRaises(errors.SessionExpired) as ctx: + sessions.resolve(store, token) + self.assertIn("key epoch", str(ctx.exception)) + + def test_issue_code_return_has_single_view_ttl(self): + """N8: issue_code return dict must not duplicate view_ttl.""" + import inspect + from sigil import viewcodes as vc + src = inspect.getsource(vc.issue_code) + # Count literal "view_ttl" keys in the return dict construction region. + self.assertEqual(src.count('"view_ttl":'), 1, src[src.rfind("return"):])