From bc0b79602973496053a5daf0e53556b0fd2b8549 Mon Sep 17 00:00:00 2001 From: ybai08 <271969568+ybai08@users.noreply.github.com> Date: Tue, 29 Sep 2026 00:59:46 -0500 Subject: [PATCH] fix(local): read a stored license issued ahead of the clock as clock_untrusted GET /license/status re-ran verify(), whose clock check raised for a stored file whose meta.issued was more than MAX_CLOCK_DRIFT ahead of the clock, so the read failed with 500 instead of the clock_untrusted state Settings can show. A read now trusts the file's signature only and lets the watermark classify the clock; importing the same file still answers 422. Fixes #1939 --- docs/architecture/license/app.md | 9 +++---- .../backend/modules/license/service.py | 6 +++-- .../backend/modules/license/verify.py | 17 +++++++++---- .../tests/integration/license/test_license.py | 24 +++++++++++++++++++ 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/docs/architecture/license/app.md b/docs/architecture/license/app.md index 1596fa40b3..b68db38e3f 100644 --- a/docs/architecture/license/app.md +++ b/docs/architecture/license/app.md @@ -22,7 +22,9 @@ The [plugins proposal](../../proposals/plugins/README.md) specifies that a paid 3. The signature over `"license/" + enc` must verify with one of the compiled keys, or the file is `bad_signature`. `enc` is not decoded until this passes. 4. `meta.issued` more than `MAX_CLOCK_DRIFT`, five minutes, in the future is `clock_untrusted`; a non-null `meta.expiry` in the past is `file_expired`. The portal checks files out with `meta.expiry: null`. -The five minutes are there because a laptop clock running a few minutes fast is not tampering. A passing file yields its key, plan, email, license expiry, `maxUsers` and `meta.issued`. `PUT /license` answers a rejection with 422 and `{code, message}`; the messages are in `router.py`. +The five minutes are there because a laptop clock running a few minutes fast is not tampering. A passing file yields its key, plan, email, license expiry, `maxUsers`, `meta.issued` and `meta.expiry`. `PUT /license` answers a rejection with 422 and `{code, message}`; the messages are in `router.py`. + +`verify_signature()` is steps 1 to 3 plus the decode; `verify()` adds step 4 and is what import runs. A status read of a stored file runs only `verify_signature()`: the time checks were passed when the file was imported, and from then on the clock is the watermark's business (below). The file TTL is not re-checked on a read either, since the portal always issues `meta.expiry: null` (contract 1); import still refuses a file whose TTL has lapsed. The other three rejections stay errors on a read: a stored file passed the same three checks at import and the key list only grows, so they can only mean the row was edited by hand, which is corruption to notice rather than a state to show. `KEYGEN_PUBLIC_KEYS_HEX` is a tuple, newest first, and a file signed by any key in it is accepted. Rotating the Keygen account key is therefore a release that trusts both keys, not a recall of every license already issued. The keys are source code, never read from configuration, environment or disk; tests swap in the fixture key by patching the constant. @@ -35,9 +37,9 @@ The singleton row `license_state` holds the file as imported, when it was import | `none` | no file is stored | | `active` | the license expiry is in the future | | `license_expired` | the license expiry has passed; the file stays stored and shown | -| `clock_untrusted` | the clock is more than five minutes behind the watermark, whatever the expiry | +| `clock_untrusted` | the clock is more than five minutes behind the watermark, which includes the stored file's `meta.issued`, whatever the expiry | -Every import, and every status read of a stored file that verifies, moves the watermark to `max(watermark, now, meta.issued)`. A clock wound back therefore reads as `clock_untrusted` until time catches up, instead of reviving an expiring license. The watermark lives in SQLite the user can edit, so a `ponytail:` comment calls it honesty for the UI, not enforcement. +Every import, and every status read of a stored file whose signature verifies, moves the watermark to `max(watermark, now, meta.issued)`. A clock wound back, or behind the time the stored file was issued, therefore reads as `clock_untrusted` until time catches up, instead of reviving an expiring license or failing the read. The watermark lives in SQLite the user can edit, so a `ponytail:` comment calls it honesty for the UI, not enforcement. There is one slot: importing a valid file replaces whatever was stored. That is what contract 1's rule 7 needs, since a resend checks out a fresh file for the same license with a later `meta.issued`, and it takes the old one's place. @@ -58,4 +60,3 @@ The release workflow's step "Refuse to build with the test signing key" runs `te ## Known gaps - Settings has no "Start trial" or "Buy" link, and its expired-license notice says to renew "from your account", which the portal does not have. -- A stored file whose `meta.issued` is more than five minutes ahead of the clock makes `GET /license/status` fail with 500 instead of answering `clock_untrusted`: `status()` re-runs `verify()`, and nothing catches its rejection. diff --git a/surfsense_local/backend/modules/license/service.py b/surfsense_local/backend/modules/license/service.py index 294b1dbae4..2839a7a648 100644 --- a/surfsense_local/backend/modules/license/service.py +++ b/surfsense_local/backend/modules/license/service.py @@ -5,7 +5,7 @@ from sqlalchemy.orm import Session from modules.license.models import LicenseState -from modules.license.verify import MAX_CLOCK_DRIFT, Verified, verify +from modules.license.verify import MAX_CLOCK_DRIFT, Verified, verify, verify_signature State = Literal["none", "active", "license_expired", "clock_untrusted"] @@ -42,7 +42,9 @@ def status(session: Session) -> LicenseStatus: row = _row(session, instant) if row.certificate is None: return LicenseStatus(state="none") - return _status(row, verify(row.certificate, instant), instant) + # Only an import runs the clock checks. On a read, a clock behind + # `meta.issued` is the watermark's clock_untrusted below, not a rejection. + return _status(row, verify_signature(row.certificate), instant) def _status(row: LicenseState, verified: Verified, instant: datetime) -> LicenseStatus: diff --git a/surfsense_local/backend/modules/license/verify.py b/surfsense_local/backend/modules/license/verify.py index 419e96a697..d6e4d2e563 100644 --- a/surfsense_local/backend/modules/license/verify.py +++ b/surfsense_local/backend/modules/license/verify.py @@ -40,10 +40,21 @@ class Verified: expiry: datetime max_users: int | None issued: datetime + file_expiry: datetime | None def verify(certificate: str, now: datetime) -> Verified: """Contract 1 consumer steps, in order, stopping at the first failure.""" + verified = verify_signature(certificate) + if verified.issued > now + MAX_CLOCK_DRIFT: + raise LicenseRejectedError("clock_untrusted") + if verified.file_expiry is not None and verified.file_expiry < now: + raise LicenseRejectedError("file_expired") + return verified + + +def verify_signature(certificate: str) -> Verified: + """What the file says once its signature is trusted; the clock is not consulted.""" try: body = base64.b64decode(re.sub(r"\s+", "", _PEM_MARKERS.sub("", certificate))) outer = json.loads(body) @@ -68,11 +79,6 @@ def verify(certificate: str, now: datetime) -> Verified: payload = json.loads(base64.b64decode(enc)) meta, attributes = payload["meta"], payload["data"]["attributes"] - if _instant(meta["issued"]) > now + MAX_CLOCK_DRIFT: - raise LicenseRejectedError("clock_untrusted") - if meta["expiry"] is not None and _instant(meta["expiry"]) < now: - raise LicenseRejectedError("file_expired") - return Verified( key=attributes["key"], plan=attributes["metadata"]["plan"], @@ -80,6 +86,7 @@ def verify(certificate: str, now: datetime) -> Verified: expiry=_instant(attributes["expiry"]), max_users=attributes["maxUsers"], issued=_instant(meta["issued"]), + file_expiry=None if meta["expiry"] is None else _instant(meta["expiry"]), ) diff --git a/surfsense_local/backend/tests/integration/license/test_license.py b/surfsense_local/backend/tests/integration/license/test_license.py index 998a969f97..c11dd7c3d2 100644 --- a/surfsense_local/backend/tests/integration/license/test_license.py +++ b/surfsense_local/backend/tests/integration/license/test_license.py @@ -194,6 +194,30 @@ async def test_a_clock_set_back_is_untrusted_until_it_catches_up( assert (rolled_back, caught_up) == ("clock_untrusted", "active") +async def test_a_stored_file_issued_ahead_of_the_clock_is_a_state_not_an_error( + client: AsyncClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """A clock behind the issue time is explained in Settings; only a fresh import is refused.""" + clock = {"now": TODAY} + monkeypatch.setattr(service, "now", lambda: clock["now"]) + certificate = signed(payload("2026-09-11T12:00:00Z")) + await put_license(client, certificate) + + clock["now"] = TODAY - timedelta(minutes=10) + status = await client.get("/license/status") + reimport = await put_license(client, certificate) + + assert status.status_code == 200 + assert (status.json()["state"], status.json()["plan"]) == ( + "clock_untrusted", + "individual", + ) + assert (reimport.status_code, reimport.json()["detail"]["code"]) == ( + 422, + "clock_untrusted", + ) + + async def test_a_fresh_install_has_no_license(client: AsyncClient) -> None: """Nothing imported, nothing to show; the free app does not nag.""" assert (await client.get("/license/status")).json() == {