From 1530bfd010435701b2328fd3e40384c9c42acd36 Mon Sep 17 00:00:00 2001 From: leora Date: Tue, 6 Oct 2026 06:15:15 +0800 Subject: [PATCH] Fix R6/R7/R8: claim_code expiry CAS, rate-limit rekey, delete spent bootstrap claim_code now requires redeem_expires_at > now in the CAS UPDATE. enrol_rekey shares the enrol: rate-limit bucket. spend_bootstrap and sibling clear delete rows instead of retaining spent hashes, matching the schema comment. --- sigil/identity.py | 2 ++ sigil/store.py | 48 ++++++++++++++++++++----------------- tests/test_hardening.py | 52 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 80 insertions(+), 22 deletions(-) diff --git a/sigil/identity.py b/sigil/identity.py index 9257983..b93486b 100644 --- a/sigil/identity.py +++ b/sigil/identity.py @@ -285,6 +285,8 @@ def enrol_rekey(store: Store, agent: str, public_b64: str, secret: str) -> str: Rekey does not clear revocation. A revoked agent stays revoked until an explicit reinstate; rotating the key must not be a back door back in. """ + if not store.allow(f"enrol:{agent}"): + raise errors.RateLimited(f"too many enrolment attempts for {agent}") row = store.get_agent(agent) if row is None: raise errors.UnknownAgent(f"{agent} is not enrolled") diff --git a/sigil/store.py b/sigil/store.py index dce41b3..88182ce 100644 --- a/sigil/store.py +++ b/sigil/store.py @@ -335,8 +335,8 @@ class Store: with self._write_lock: now = int(time.time()) self.conn.execute( - "UPDATE bootstrap SET spent_at = ? WHERE agent = ? AND spent_at IS NULL", - (now, agent), + "DELETE FROM bootstrap WHERE agent = ? AND spent_at IS NULL", + (agent,), ) self.conn.execute( "INSERT INTO bootstrap (secret_hash, agent, created_at) VALUES (?, ?, ?)", @@ -345,7 +345,7 @@ class Store: self.conn.commit() def clear_bootstrap_for_agent(self, agent: str, *, keep_hash: Optional[str] = None) -> int: - """Spend (or delete) every unspent bootstrap secret for an agent. + """Delete every unspent bootstrap secret for an agent. After a successful enrol / enrol_rekey, leftover seeds must not remain usable. keep_hash, when set, is left alone (already consumed by CAS). @@ -353,14 +353,14 @@ class Store: with self._write_lock: if keep_hash is None: cur = self.conn.execute( - "UPDATE bootstrap SET spent_at = ? WHERE agent = ? AND spent_at IS NULL", - (int(time.time()), agent), + "DELETE FROM bootstrap WHERE agent = ? AND spent_at IS NULL", + (agent,), ) else: cur = self.conn.execute( - "UPDATE bootstrap SET spent_at = ? WHERE agent = ? AND spent_at IS NULL" + "DELETE FROM bootstrap WHERE agent = ? AND spent_at IS NULL" " AND secret_hash != ?", - (int(time.time()), agent, keep_hash), + (agent, keep_hash), ) self.conn.commit() return cur.rowcount @@ -368,23 +368,26 @@ class Store: def spend_bootstrap(self, secret_hash: str) -> Optional[str]: """Atomically consume an enrolment secret. Returns the agent or None. - Compare-and-set: the UPDATE only matches an unspent row, so exactly one - concurrent caller can win. Check-then-update would let a secret enrol - more than one key. + Compare-and-set then delete: exactly one concurrent caller can win, and + the spent row is removed so a table dump cannot offline-crack a + low-entropy custom secret. Matches the schema comment. """ with self._write_lock: + row = self.conn.execute( + "SELECT agent FROM bootstrap WHERE secret_hash = ? AND spent_at IS NULL", + (secret_hash,), + ).fetchone() + if row is None: + return None + agent = row["agent"] cur = self.conn.execute( - "UPDATE bootstrap SET spent_at = ? WHERE secret_hash = ?" - " AND spent_at IS NULL", - (int(time.time()), secret_hash), + "DELETE FROM bootstrap WHERE secret_hash = ? AND spent_at IS NULL", + (secret_hash,), ) self.conn.commit() if cur.rowcount != 1: return None - row = self.conn.execute( - "SELECT agent FROM bootstrap WHERE secret_hash = ?", (secret_hash,) - ).fetchone() - return row["agent"] if row else None + return agent # -- nonces ------------------------------------------------------------ def put_nonce(self, nonce: str, agent: str, expires_at: int) -> None: @@ -510,14 +513,17 @@ class Store: return self._one("SELECT * FROM codes WHERE code_hash = ?", (code_hash,)) def claim_code(self, code_hash: str) -> bool: - """Consume a one-time code exactly once. False if already used. + """Consume a one-time code exactly once. False if already used or expired. - Atomic, so a code cannot mint two sessions under a race. + Atomic CAS includes the expiry predicate so a race at the TTL boundary + cannot mint a session slightly after redeem_expires_at. """ with self._write_lock: + now = int(time.time()) cur = self.conn.execute( - "UPDATE codes SET used_at = ? WHERE code_hash = ? AND used_at IS NULL", - (int(time.time()), code_hash), + "UPDATE codes SET used_at = ? WHERE code_hash = ? AND used_at IS NULL" + " AND redeem_expires_at > ?", + (now, code_hash, now), ) self.conn.commit() return cur.rowcount == 1 diff --git a/tests/test_hardening.py b/tests/test_hardening.py index 34ba7cf..fb901d2 100644 --- a/tests/test_hardening.py +++ b/tests/test_hardening.py @@ -359,4 +359,54 @@ class TestAgentIdValidation(unittest.TestCase): if __name__ == "__main__": - unittest.main() \ No newline at end of file + unittest.main() + + +class TestClaimCodeExpiryCAS(unittest.TestCase): + """R6: claim_code CAS must include redeem_expires_at predicate.""" + + def test_claim_code_after_expiry_returns_false(self): + store, private = helpers.fresh() + issued = helpers.issue(store, private, code_ttl=1, view_ttl=60) + digest = viewcodes._hash_code(issued["code"]) + # Force expiry in the row. + store.conn.execute( + "UPDATE codes SET redeem_expires_at = ? WHERE code_hash = ?", + (int(time.time()) - 1, digest), + ) + store.conn.commit() + self.assertFalse(store.claim_code(digest)) + row = store.get_code(digest) + self.assertIsNone(row["used_at"]) + + +class TestEnrolRekeyRateLimit(unittest.TestCase): + """R7: enrol_rekey must share the enrol rate-limit bucket.""" + + def test_enrol_rekey_is_rate_limited(self): + store, _ = helpers.fresh("limited") + # Exhaust enrol bucket. + for i in range(20): + store.allow("enrol:limited") + secret = identity.seed_bootstrap(store, "limited") + with self.assertRaises(errors.RateLimited): + identity.enrol_rekey( + store, "limited", keys.export_public_b64(keys.generate()[1]), secret + ) + + +class TestBootstrapRowDeletedOnSpend(unittest.TestCase): + """R8: spent bootstrap secrets are deleted, not retained as hashes.""" + + def test_spend_deletes_the_row(self): + store = Store(":memory:") + secret = identity.seed_bootstrap(store, "dave") + digest = __import__("hashlib").sha256(secret.encode()).hexdigest() + self.assertIsNotNone(store.conn.execute( + "SELECT 1 FROM bootstrap WHERE secret_hash = ?", (digest,) + ).fetchone()) + owner = store.spend_bootstrap(digest) + self.assertEqual(owner, "dave") + self.assertIsNone(store.conn.execute( + "SELECT 1 FROM bootstrap WHERE secret_hash = ?", (digest,) + ).fetchone()) -- 2.47.3