diff --git a/server/packages/kca-app/src/kca/ocsp.py b/server/packages/kca-app/src/kca/ocsp.py index f808f1e..2adef97 100644 --- a/server/packages/kca-app/src/kca/ocsp.py +++ b/server/packages/kca-app/src/kca/ocsp.py @@ -130,10 +130,11 @@ async def mark_ocsp_dirty(ca_id: UUID) -> None: if row is None: await OcspSnapshot.create(ca_id=ca_id, dirty=True) return - if not row.dirty: - row.dirty = True - row.last_error = None # type: ignore[assignment] - await row.save(update_fields=["dirty", "last_error", "updated_at"]) + # Always save, even when already dirty: updated_at is the publisher's + # optimistic-concurrency token for changes made during an upload. + row.dirty = True + row.last_error = None # type: ignore[assignment] + await row.save(update_fields=["dirty", "last_error", "updated_at"]) async def mark_all_cas_dirty(vault: Vault) -> None: @@ -220,12 +221,23 @@ class OcspPublisherMixin(KayaMixin): try: snapshot = await snapshot_for_ca(ca) key = await publish_snapshot(ca, snapshot) - row.dirty = False - row.published_at = datetime.now(timezone.utc) - row.last_error = None # type: ignore[assignment] - await row.save( - update_fields=["dirty", "published_at", "last_error", "updated_at"] + published_at = datetime.now(timezone.utc) + cleared = await OcspSnapshot.filter( + ca_id=row.ca_id, + dirty=True, + updated_at=row.updated_at, + ).update( + dirty=False, + published_at=published_at, + last_error=None, + updated_at=published_at, ) + if not cleared: + log.info( + "OCSP snapshot changed during publication; leaving dirty ca=%s", + ca.id, + ) + continue log.info("published OCSP snapshot ca=%s key=%s", ca.id, key) except Exception as exc: # noqa: BLE001 - keep retrying the same row row.dirty = True diff --git a/server/packages/kca-app/src/kca/routes/certificates.py b/server/packages/kca-app/src/kca/routes/certificates.py index b02d2d7..f9b51d9 100644 --- a/server/packages/kca-app/src/kca/routes/certificates.py +++ b/server/packages/kca-app/src/kca/routes/certificates.py @@ -20,7 +20,7 @@ from ..auth import authenticated_user, require_auth from ..config import settings from ..http import JsonRequestError, read_json, send_error, send_json from ..models import Authority, Certificate, CertificateRevocation, Crl, Key, OcspSnapshot, Request, Template -from ..ocsp import snapshot_for_ca +from ..ocsp import mark_ocsp_dirty, snapshot_for_ca from ..openapi import error_response, json_response, schema_ref from ..service import ( ValidationError, @@ -455,6 +455,7 @@ def register(app: KayaApp, kca: "Kca") -> None: if not created: await send_error(ctx, 409, "this serial is already revoked") return + await mark_ocsp_dirty(cert.id) await send_json(ctx, 201, await serialize_revocation(row)) @app.PATCH("/api/vaults/${vault_id}/certificates/${cert_id}/revocations/${rev_id}") @@ -495,6 +496,7 @@ def register(app: KayaApp, kca: "Kca") -> None: return row.reason = str(body["reason"]) await row.save() + await mark_ocsp_dirty(cert.id) await send_json(ctx, 200, await serialize_revocation(row)) @app.DELETE("/api/vaults/${vault_id}/certificates/${cert_id}/revocations/${rev_id}") @@ -513,6 +515,7 @@ def register(app: KayaApp, kca: "Kca") -> None: await send_error(ctx, 404, "revocation not found") return await row.delete() + await mark_ocsp_dirty(cert.id) await ctx.send_empty(204) @app.POST("/api/vaults/${vault_id}/certificates/${cert_id}/crl") diff --git a/server/packages/kca-app/src/kca/service.py b/server/packages/kca-app/src/kca/service.py index 7218027..a637f94 100644 --- a/server/packages/kca-app/src/kca/service.py +++ b/server/packages/kca-app/src/kca/service.py @@ -972,6 +972,7 @@ async def revoke_certificate(vault: Vault, cert: Certificate, payload: Mapping[s raise ValidationError("the issuer has no private key") existing = await CertificateRevocation.get_or_none(ca_id=ca.id, serial_hex=cert.serial_hex) if existing is not None: + await mark_ocsp_dirty(ca.id) return existing revoked_at = parse_datetime(payload.get("revoked_at") or datetime.now(timezone.utc).isoformat(), "revoked_at") invalidity = payload.get("invalidity_date") diff --git a/server/tests/test_routes_extra.py b/server/tests/test_routes_extra.py index 401ea05..46931a8 100644 --- a/server/tests/test_routes_extra.py +++ b/server/tests/test_routes_extra.py @@ -456,6 +456,137 @@ class OcspSnapshotTests(unittest.TestCase): row = await OcspSnapshot.get(ca_id=ca["id"]) assert row.dirty is True + @async_test + async def test_existing_revocation_marks_snapshot_dirty(self) -> None: + from dataclasses import replace + from unittest.mock import patch + + from kca import ocsp + from kca.models import OcspSnapshot + + async with api_client() as client: + with oidc_user("alice"): + vault_id = await _vault(client) + ca_key = await _key(client, vault_id, "existing dirty ca") + ca = await _cert(client, vault_id, ca_key["id"], "Existing Dirty CA", ca=True) + leaf_key = await _key(client, vault_id, "existing dirty leaf") + leaf = await _cert( + client, vault_id, leaf_key["id"], "Existing Dirty Leaf", signer_id=ca["id"] + ) + configured = replace(ocsp.settings, ocsp_s3_bucket="test-bucket") + with patch.object(ocsp, "settings", configured): + response = await client.post( + f"/api/vaults/{vault_id}/certificates/{leaf['id']}/revoke", + json={"reason": "superseded"}, + ) + assert response.status_code == 200, response.text + await OcspSnapshot.filter(ca_id=ca["id"]).update(dirty=False) + response = await client.post( + f"/api/vaults/{vault_id}/certificates/{leaf['id']}/revoke", + json={"reason": "superseded"}, + ) + assert response.status_code == 200, response.text + row = await OcspSnapshot.get(ca_id=ca["id"]) + assert row.dirty is True + + @async_test + async def test_manual_revocation_changes_mark_snapshot_dirty(self) -> None: + from dataclasses import replace + from unittest.mock import patch + + from kca import ocsp + from kca.models import OcspSnapshot + + async with api_client() as client: + with oidc_user("alice"): + vault_id = await _vault(client) + ca_key = await _key(client, vault_id, "manual dirty ca") + ca = await _cert(client, vault_id, ca_key["id"], "Manual Dirty CA", ca=True) + configured = replace(ocsp.settings, ocsp_s3_bucket="test-bucket") + with patch.object(ocsp, "settings", configured): + await OcspSnapshot.create(ca_id=ca["id"], dirty=False) + response = await client.post( + f"/api/vaults/{vault_id}/certificates/{ca['id']}/revocations", + json={"serial_hex": "1234", "reason": "keyCompromise"}, + ) + assert response.status_code == 201, response.text + revocation = response.json() + row = await OcspSnapshot.get(ca_id=ca["id"]) + assert row.dirty is True + + await OcspSnapshot.filter(ca_id=ca["id"]).update(dirty=False) + response = await client.patch( + f"/api/vaults/{vault_id}/certificates/{ca['id']}/revocations/{revocation['id']}", + json={"reason": "superseded"}, + ) + assert response.status_code == 200, response.text + row = await OcspSnapshot.get(ca_id=ca["id"]) + assert row.dirty is True + + await OcspSnapshot.filter(ca_id=ca["id"]).update(dirty=False) + response = await client.delete( + f"/api/vaults/{vault_id}/certificates/{ca['id']}/revocations/{revocation['id']}" + ) + assert response.status_code == 204, response.text + row = await OcspSnapshot.get(ca_id=ca["id"]) + assert row.dirty is True + + @async_test + async def test_publisher_preserves_changes_during_publication(self) -> None: + from dataclasses import replace + from unittest.mock import patch + + from kca import ocsp + from kca.models import CertificateRevocation, OcspSnapshot + + async with api_client() as client: + with oidc_user("alice"): + vault_id = await _vault(client) + ca_key = await _key(client, vault_id, "concurrent publish ca") + ca = await _cert(client, vault_id, ca_key["id"], "Concurrent Publish CA", ca=True) + leaf_key = await _key(client, vault_id, "concurrent publish leaf") + leaf = await _cert( + client, vault_id, leaf_key["id"], "Concurrent Publish Leaf", signer_id=ca["id"] + ) + published = [] + + async def fake_publish(publish_ca, snapshot): + published.append(snapshot) + if len(published) == 1: + await CertificateRevocation.create( + ca_id=publish_ca.id, + serial_hex=leaf["serial_hex"], + revoked_at=datetime.now(timezone.utc), + reason="keyCompromise", + ) + await ocsp.mark_ocsp_dirty(publish_ca.id) + return ocsp.object_key(str(publish_ca.id)) + + configured = replace(ocsp.settings, ocsp_s3_bucket="test-bucket") + with patch.object(ocsp, "settings", configured), patch.object( + ocsp, "publish_snapshot", fake_publish + ): + await ocsp.mark_ocsp_dirty(ca["id"]) + + async def ensure_context(): + return None + + mixin = ocsp.OcspPublisherMixin(1, ensure_context) + await mixin.run_once() + row = await OcspSnapshot.get(ca_id=ca["id"]) + assert row.dirty is True + await mixin.run_once() + + assert len(published) == 2 + first_entries = {entry["serial_hex"]: entry for entry in published[0]["entries"]} + second_entries = {entry["serial_hex"]: entry for entry in published[1]["entries"]} + serial = leaf["serial_hex"].lower() + assert first_entries[serial]["status"] == "valid" + assert second_entries[serial]["status"] == "revoked" + assert second_entries[serial]["reason"] == "keyCompromise" + row = await OcspSnapshot.get(ca_id=ca["id"]) + assert row.dirty is False + @async_test async def test_publisher_marks_dirty_and_uploads(self) -> None: from dataclasses import replace