From 6578a1fe8f1fc7cf9a1a7f32e577084cf60e84e4 Mon Sep 17 00:00:00 2001 From: leora Date: Tue, 6 Oct 2026 06:23:17 +0800 Subject: [PATCH] Fix N4/N5/N6: enrol wording, suspended checkpoint, test count Drop misleading "or re-enrol" after permanent revoke; enrol refuses an existing agent before spending the seed. checkpoint() refuses a currently revoked/suspended signer. README (and DESIGN) test count updated to 156. --- DESIGN.md | 10 ++++++---- README.md | 2 +- sigil/identity.py | 4 ++++ sigil/store.py | 10 +++++++--- tests/test_production.py | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 8 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 7d7e1fe..f8c758d 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -123,9 +123,11 @@ means a future route cannot pass a string where a proof belongs. `revoke ` with no end time is permanent: the enrolled public key is cleared and the identity stays dead until someone bootstraps a fresh secret and -rekeys (or re-enrols). `reinstate` alone lifts the ban flags but does **not** -restore the compromised key — that was the footgun. `revoke --until -` is a pause that keeps the key, and it lapses on its own. +calls `enrol_rekey` (then `reinstate` if still banned). `enrol` will not work — +the agent row still exists — and would only burn the seed. `reinstate` alone +lifts the ban flags but does **not** restore the compromised key — that was the +footgun. `revoke --until ` is a pause that keeps the key, and it +lapses on its own. The distinction matters because the two failure modes are different. A key compromise wants death — a lapsing revocation would hand the attacker back in @@ -228,7 +230,7 @@ because the review looked for it. ## Status -106 passing tests, and two hardening passes that closed real defects. The worst were +156 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 d682469..b002fa1 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 ``` -130 tests. They cover the refusals, not just the happy paths: replayed +156 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/identity.py b/sigil/identity.py index bc2ee1a..45af896 100644 --- a/sigil/identity.py +++ b/sigil/identity.py @@ -78,6 +78,10 @@ def enrol( _validate_agent_id(agent) if not store.allow(f"enrol:{agent}"): raise errors.RateLimited(f"too many enrolment attempts for {agent}") + # Already enrolled (e.g. after permanent revoke): refuse *before* spending + # the seed so ops following a wrong "re-enrol" path do not burn it. + if store.get_agent(agent) is not None: + raise errors.EnrolmentFailed(f"{agent} is already enrolled; use enrol_rekey") # Spend the secret first so a burned secret cannot be retried. The agent # row insert below is INSERT OR IGNORE: if two secrets race for the same # unenrolled agent, exactly one put_agent wins and the loser fails closed diff --git a/sigil/store.py b/sigil/store.py index 04b50f6..333d447 100644 --- a/sigil/store.py +++ b/sigil/store.py @@ -352,9 +352,9 @@ class Store: """Suspend an agent. `until` of None is permanent. A permanent revoke clears the enrolled public key so `reinstate` alone - cannot restore a compromised identity — ops must bootstrap + rekey - (or enrol) before the agent can authenticate again. Timed suspension - keeps the key: when the pause lapses, the same key works. + cannot restore a compromised identity — ops must bootstrap + enrol_rekey + before the agent can authenticate again. Timed suspension keeps the + key: when the pause lapses, the same key works. Returns how many live sessions were killed. """ @@ -788,9 +788,13 @@ class Store: public = _keys.public_from_private(private) fingerprint = _keys.fingerprint(public) + from . import identity as _identity + agent_row = self.get_agent(signer) if agent_row is None: raise ValueError(f"{signer} is not enrolled, so cannot sign a checkpoint") + if _identity.is_revoked(agent_row): + raise ValueError(f"{signer} is revoked, so cannot sign a checkpoint") enrolled_fp = agent_row["fingerprint"] or "" if not enrolled_fp or fingerprint != enrolled_fp: raise ValueError( diff --git a/tests/test_production.py b/tests/test_production.py index 650e55d..e28bb22 100644 --- a/tests/test_production.py +++ b/tests/test_production.py @@ -531,3 +531,36 @@ class TestKeyHistoryKeepsCheckpointsVerifiable(unittest.TestCase): self.assertNotIn("privkey", cols) row = store.conn.execute("SELECT * FROM agent_keys WHERE agent = ?", ("carol",)).fetchone() self.assertEqual(len(row["pubkey"]), 32) + + +class TestEnrolAfterPermanentRevoke(unittest.TestCase): + """N4: enrol must not burn the seed when the agent row still exists.""" + + def test_enrol_after_permanent_revoke_does_not_burn_seed(self): + store, private = helpers.fresh() + identity.revoke(store, "bigpickle", reason="compromise") + identity.reinstate(store, "bigpickle") + secret = identity.seed_bootstrap(store, "bigpickle") + with self.assertRaises(errors.EnrolmentFailed) as ctx: + identity.enrol( + store, "bigpickle", keys.export_public_b64(keys.generate()[1]), secret + ) + self.assertIn("enrol_rekey", str(ctx.exception)) + # Seed still usable for the correct path. + identity.enrol_rekey( + store, "bigpickle", keys.export_public_b64(keys.generate()[1]), secret + ) + + +class TestSuspendedSignerCannotCheckpoint(unittest.TestCase): + """N5: timed-suspended signer must not checkpoint.""" + + def test_timed_suspend_refuses_checkpoint(self): + import time + store, private = helpers.fresh() + identity.revoke( + store, "bigpickle", until=int(time.time()) + 3600, reason="pause" + ) + with self.assertRaises(ValueError) as ctx: + store.checkpoint(private, "bigpickle") + self.assertIn("revoked", str(ctx.exception).lower()) -- 2.47.3