diff --git a/.bumpversion.toml b/.bumpversion.toml index c65cef51..4d9a73b3 100644 --- a/.bumpversion.toml +++ b/.bumpversion.toml @@ -1,5 +1,5 @@ [tool.bumpversion] -current_version = "1.19.2+260912" +current_version = "1.19.3+260916" parse = "(?P\\d+)\\.(?P\\d+)\\.(?P\\d+)\\+(?P\\d+)" serialize = ["{major}.{minor}.{patch}+{build}"] search = "{current_version}" diff --git a/pyproject.toml b/pyproject.toml index 4e73826e..17c656dc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "rasenmaeher_api" -version = "1.19.2+260912" +version = "1.19.3+260916" description = "python-rasenmaeher-api" authors = [ { name = "Aciid", email = "703382+Aciid@users.noreply.github.com" }, diff --git a/src/rasenmaeher_api/__init__.py b/src/rasenmaeher_api/__init__.py index 86358cd7..9fcb79a6 100644 --- a/src/rasenmaeher_api/__init__.py +++ b/src/rasenmaeher_api/__init__.py @@ -1,3 +1,3 @@ """python-rasenmaeher-api""" -__version__ = "1.19.2+260912" # NOTE Use `bump-my-version` to bump versions correctly +__version__ = "1.19.3+260916" # NOTE Use `bump-my-version` to bump versions correctly diff --git a/src/rasenmaeher_api/cert/cert_manager/private.py b/src/rasenmaeher_api/cert/cert_manager/private.py index 618f45b1..eecf73cb 100644 --- a/src/rasenmaeher_api/cert/cert_manager/private.py +++ b/src/rasenmaeher_api/cert/cert_manager/private.py @@ -2,9 +2,8 @@ The signing flow creates a cert-manager ``CertificateRequest`` CR carrying the caller-provided CSR, waits until cert-manager issues the certificate, and returns -the issued certificate as PEM. Revocation is DB-driven (consumed by the -Traefik callsign-validity plugin), so the revoke functions only best-effort -clean up the CR. +the issued certificate as PEM. Revocation is DB-driven (served to clients by +the OCSP responder), so the revoke functions only best-effort clean up the CR. """ import base64 @@ -253,8 +252,8 @@ def validate_reason(reason: ReasonTypes) -> cryptography.x509.ReasonFlags: async def revoke_pem(pem: str | Path, reason: ReasonTypes) -> None: """Best-effort revoke. Under cert-manager, revocation is authoritative in - the rmapi DB (``Person.deleted``) and is consumed by the Traefik plugin via - websocket. This function only cleans up the matching CertificateRequest CR. + the rmapi DB (``Person.deleted``) and is served to the edge by the OCSP + responder. This function only cleans up the matching CertificateRequest CR. """ validate_reason(reason) LOGGER.debug( diff --git a/src/rasenmaeher_api/rmsettings.py b/src/rasenmaeher_api/rmsettings.py index 4a907141..00b4cdeb 100644 --- a/src/rasenmaeher_api/rmsettings.py +++ b/src/rasenmaeher_api/rmsettings.py @@ -113,11 +113,6 @@ def log_level_int(self) -> int: ocsp_ca_secret_namespace: str = "cert-manager" ocsp_response_validity: int = 3600 # seconds from now until nextUpdate - # Shared secret used by the Traefik callsign-validity plugin to auth to the - # internal websocket. Optional — if unset, the websocket accepts any caller - # (suitable for in-cluster-only Service exposure during local dev). - callsign_validity_secret: str | None = None - persistent_data_dir: str = "/data/persistent" # mtls diff --git a/src/rasenmaeher_api/web/api/internal/__init__.py b/src/rasenmaeher_api/web/api/internal/__init__.py index c19c76e2..f63cf668 100644 --- a/src/rasenmaeher_api/web/api/internal/__init__.py +++ b/src/rasenmaeher_api/web/api/internal/__init__.py @@ -1,10 +1,4 @@ -"""Internal (in-cluster only) API surface. - -Endpoints under here are not intended to be exposed via Traefik — they are -reachable only on the in-cluster Service. They power infrastructure-side -features such as the callsign-validity websocket consumed by the Traefik -plugin. -""" +"""Internal (in-cluster only) API surface, not exposed through Traefik's routers.""" from fastapi import APIRouter diff --git a/src/rasenmaeher_api/web/api/internal/callsign_validity.py b/src/rasenmaeher_api/web/api/internal/callsign_validity.py index f6b065f9..25a11039 100644 --- a/src/rasenmaeher_api/web/api/internal/callsign_validity.py +++ b/src/rasenmaeher_api/web/api/internal/callsign_validity.py @@ -1,86 +1,124 @@ -"""HTTP endpoint that answers callsign validity queries. +"""Per-request mTLS authorization, answered for Traefik's ``forwardAuth`` middleware. -Consumed by the Traefik ``callsign-validity`` plugin for per-request mTLS -authorization checks. A callsign is valid if EITHER: +Recovers the client certificate from ``X-Forwarded-Tls-Client-Cert`` and resolves +its serial through :func:`lookup_status`, the same function the OCSP responder +answers from. Always replies ``200``; the verdict is in the headers, which +``callsign-redirect`` acts on: -* a ``Person`` exists with the given callsign and ``deleted IS NULL``, OR -* the callsign matches a product service CN listed in the kraftwerk manifest - (``RMSettings.valid_product_cns``) — e.g. ``rasenmaeher`` / ``tak`` for - inter-service mTLS calls. Matches the same trust model as the existing - ``ValidUser`` dependency in ``middleware/user.py``. + Callsign the certificate CN + Callsign-Valid "true" or "false" + Callsign-Valid-Reason ok | no_cert | invalid | error -Wire protocol: - - POST /api/v1/internal/callsign-validity/check - {"callsign": ""} - -> 200 {"valid": } - -> 400 if body is malformed - -Auth: if ``RM_CALLSIGN_VALIDITY_SECRET`` is set, the client must send a -matching ``Validity-Secret`` header. Otherwise the endpoint is open -(suitable for in-cluster-only Service exposure). - -Note: this used to be a websocket endpoint, but Traefik's Yaegi interpreter -cannot load ``golang.org/x/net/websocket``, so the plugin was switched to -stdlib ``net/http``. The per-request semantics are identical. +SECURITY: the certificate header is authentication input. It is trustworthy only +because ``strip-identity-headers`` blanks it on the ``websecure`` entrypoint -- +entrypoint middlewares run before router middlewares -- and ``mtls-pass-client-cert`` +then sets it from the verified connection. """ import logging +import urllib.parse -from fastapi import APIRouter, Header, HTTPException, status -from pydantic import BaseModel +from cryptography import x509 +from cryptography.x509 import ocsp +from cryptography.x509.oid import NameOID +from fastapi import APIRouter, Header, Response -from ....db.errors import Deleted, NotFound -from ....db.people import Person -from ....rmsettings import RMSettings +from ....cert.cert_manager.ocsp.status import lookup_status LOGGER = logging.getLogger(__name__) router = APIRouter() +CERT_HEADER = "X-Forwarded-Tls-Client-Cert" +CALLSIGN_HEADER = "Callsign" +VALIDITY_HEADER = "Callsign-Valid" +REASON_HEADER = "Callsign-Valid-Reason" + +REASON_OK = "ok" +REASON_NO_CERT = "no_cert" +REASON_INVALID = "invalid" +REASON_ERROR = "error" + +PEM_LINE_WIDTH = 64 + + +def _rewrap(body: str) -> str: + """Restore the PEM armour and line breaks Traefik strips from the header""" + if "BEGIN CERTIFICATE" in body: + return body + body = "".join(body.split()) + lines = [body[offset : offset + PEM_LINE_WIDTH] for offset in range(0, len(body), PEM_LINE_WIDTH)] + return "-----BEGIN CERTIFICATE-----\n" + "\n".join(lines) + "\n-----END CERTIFICATE-----\n" + + +def _candidates(header: str) -> list[str]: + """The header url-encoded or literal; the literal first, since unquoting + would turn a legal base64 ``+`` into a space.""" + out = [header] + if "%" not in header: + return out + for decoded in (urllib.parse.unquote(header), urllib.parse.unquote_plus(header)): + if decoded not in out: + out.append(decoded) + return out + + +def parse_leaf(header: str) -> x509.Certificate | None: + """Recover the leaf from a passTLSClientCert header; the chain is + comma-separated and only the first entry is the leaf.""" + header = header.strip() + if not header: + return None + for candidate in _candidates(header): + first = candidate.split(",", 1)[0].strip() + if not first: + continue + try: + return x509.load_pem_x509_certificate(_rewrap(first).encode("utf-8")) + except ValueError: + continue + return None + + +def common_name(cert: x509.Certificate) -> str: + """Return the trimmed CN, which is the callsign""" + attributes = cert.subject.get_attributes_for_oid(NameOID.COMMON_NAME) + if not attributes: + return "" + return str(attributes[0].value).strip() + + +def _verdict(callsign: str, valid: bool, reason: str) -> Response: + headers = { + VALIDITY_HEADER: "true" if valid else "false", + REASON_HEADER: reason, + } + if callsign: + headers[CALLSIGN_HEADER] = callsign + return Response(status_code=200, headers=headers) + + +@router.get("/check") +async def callsign_validity_check( + client_cert: str | None = Header(default=None, alias=CERT_HEADER), +) -> Response: + """Answer Traefik's forwardAuth subrequest with the client certificate's verdict""" + leaf = parse_leaf(client_cert or "") + if leaf is None: + return _verdict("", False, REASON_NO_CERT) + + callsign = common_name(leaf) + if not callsign: + return _verdict("", False, REASON_NO_CERT) -class CheckRequest(BaseModel): - callsign: str - - -class CheckResponse(BaseModel): - valid: bool - - -def _check_secret(provided: str | None) -> None: - expected = RMSettings.singleton().callsign_validity_secret - if not expected: - return - if provided != expected: - raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="invalid validity secret") - - -async def _is_valid(callsign: str) -> bool: - settings = RMSettings.singleton() - # Service identities from the kraftwerk manifest (product backends like TAK, - # battlelog) and the rmapi self-CN are trusted by virtue of the CA chain; - # they don't appear in the Person table. - if callsign == settings.mtls_client_cert_cn: - return True try: - if callsign in settings.valid_product_cns: - return True + result = await lookup_status(leaf.serial_number) except Exception: # pylint: disable=broad-except - LOGGER.debug("valid_product_cns lookup failed", exc_info=True) - try: - await Person.by_callsign(callsign) - return True - except (NotFound, Deleted): - return False + LOGGER.exception("callsign validity lookup failed for %s", callsign) + return _verdict(callsign, False, REASON_ERROR) + if result.status == ocsp.OCSPCertStatus.GOOD: + return _verdict(callsign, True, REASON_OK) -@router.post("/check", response_model=CheckResponse) -async def callsign_validity_check( - req: CheckRequest, - validity_secret: str | None = Header(default=None, alias="Validity-Secret"), -) -> CheckResponse: - """Return whether the given callsign is currently valid.""" - _check_secret(validity_secret) - callsign = req.callsign.strip() - if not callsign: - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="callsign must be non-empty") - return CheckResponse(valid=await _is_valid(callsign)) + # REVOKED and UNKNOWN both deny. + LOGGER.info("callsign %s denied, status=%s", callsign, result.status) + return _verdict(callsign, False, REASON_INVALID) diff --git a/tests/test_callsign_validity.py b/tests/test_callsign_validity.py new file mode 100644 index 00000000..04a13c58 --- /dev/null +++ b/tests/test_callsign_validity.py @@ -0,0 +1,158 @@ +"""Test the forwardAuth endpoint Traefik asks on every mTLS request.""" + +import datetime +import logging +import urllib.parse +from typing import Any + +import pytest +from async_asgi_testclient import TestClient # type: ignore[import-untyped] +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.x509 import ocsp +from cryptography.x509.oid import NameOID + +from rasenmaeher_api.cert.cert_manager.ocsp.status import CertStatusResult +from rasenmaeher_api.web.api.internal import callsign_validity + +LOGGER = logging.getLogger(__name__) + +ENDPOINT = "/api/v1/internal/callsign-validity/check" +CERT_HEADER = callsign_validity.CERT_HEADER +GOOD = CertStatusResult(status=ocsp.OCSPCertStatus.GOOD) + + +def make_cert(common_name: str = "ALPHA01", serial: int = 0x4242) -> str: + """A self-signed leaf; only the CN and serial are read""" + key = ec.generate_private_key(ec.SECP256R1()) + name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, common_name)]) + now = datetime.datetime.now(datetime.UTC) + cert = ( + x509.CertificateBuilder() + .subject_name(name) + .issuer_name(name) + .public_key(key.public_key()) + .serial_number(serial) + .not_valid_before(now - datetime.timedelta(hours=1)) + .not_valid_after(now + datetime.timedelta(hours=1)) + .sign(key, hashes.SHA256()) + ) + return cert.public_bytes(serialization.Encoding.PEM).decode("utf-8") + + +def stripped(pem: str) -> str: + """The PEM as Traefik sends it: armour removed, single line""" + return "".join(line for line in pem.splitlines() if "-----" not in line) + + +def patch_status(monkeypatch: pytest.MonkeyPatch, result: Any, seen: list[int] | None = None) -> None: + """Replace the DB lookup; lookup_status has its own tests under tests/cert""" + + async def fake_lookup(serial: int) -> Any: + if seen is not None: + seen.append(serial) + if isinstance(result, Exception): + raise result + return result + + monkeypatch.setattr(callsign_validity, "lookup_status", fake_lookup) + + +@pytest.mark.asyncio(loop_scope="session") +@pytest.mark.parametrize("header", ["", " ", "not-a-certificate", "%%%"]) +async def test_no_usable_certificate(unauth_client_session: TestClient, header: str) -> None: + """Without a parseable cert the verdict is no_cert and no callsign leaks""" + resp = await unauth_client_session.get(ENDPOINT, headers={CERT_HEADER: header}) + assert resp.status_code == 200 + assert resp.headers["Callsign-Valid"] == "false" + assert resp.headers["Callsign-Valid-Reason"] == "no_cert" + assert "Callsign" not in resp.headers + + +@pytest.mark.asyncio(loop_scope="session") +async def test_missing_header(unauth_client_session: TestClient) -> None: + """A request with no certificate header at all is still answered 200""" + resp = await unauth_client_session.get(ENDPOINT) + assert resp.status_code == 200 + assert resp.headers["Callsign-Valid-Reason"] == "no_cert" + + +@pytest.mark.asyncio(loop_scope="session") +@pytest.mark.parametrize( + "status,want_valid,want_reason", + [ + (ocsp.OCSPCertStatus.GOOD, "true", "ok"), + (ocsp.OCSPCertStatus.REVOKED, "false", "invalid"), + (ocsp.OCSPCertStatus.UNKNOWN, "false", "invalid"), + ], +) +async def test_verdict_mapping( + unauth_client_session: TestClient, + monkeypatch: pytest.MonkeyPatch, + status: ocsp.OCSPCertStatus, + want_valid: str, + want_reason: str, +) -> None: + """Revoked and unknown both deny, and every verdict still rides a 200""" + patch_status(monkeypatch, CertStatusResult(status=status)) + resp = await unauth_client_session.get(ENDPOINT, headers={CERT_HEADER: make_cert("ALPHA01", 0x4242)}) + assert resp.status_code == 200 + assert resp.headers["Callsign"] == "ALPHA01" + assert resp.headers["Callsign-Valid"] == want_valid + assert resp.headers["Callsign-Valid-Reason"] == want_reason + + +@pytest.mark.asyncio(loop_scope="session") +async def test_lookup_failure_is_an_error_verdict( + unauth_client_session: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """A failing lookup denies with reason=error rather than 500ing the request""" + patch_status(monkeypatch, RuntimeError("database is down")) + resp = await unauth_client_session.get(ENDPOINT, headers={CERT_HEADER: make_cert("ALPHA01", 0x4242)}) + assert resp.status_code == 200 + assert resp.headers["Callsign-Valid-Reason"] == "error" + + +@pytest.mark.asyncio(loop_scope="session") +async def test_lookup_keys_on_the_certificate_serial( + unauth_client_session: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """Regression test. The endpoint this replaced looked the callsign up in the + Person table, which can disagree with whether the certificate itself was + revoked. Keying on the serial is what keeps this and the OCSP responder in + agreement.""" + seen: list[int] = [] + patch_status(monkeypatch, GOOD, seen) + await unauth_client_session.get(ENDPOINT, headers={CERT_HEADER: make_cert("BRAVO02", 0xDEADBEEF)}) + assert seen == [0xDEADBEEF] + + +@pytest.mark.asyncio(loop_scope="session") +async def test_accepts_every_header_encoding( + unauth_client_session: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """Traefik sends the cert armoured, stripped, url-encoded or chained""" + pem = make_cert("CHARLIE03", 0x99) + forms = { + "armoured": pem, + "stripped": stripped(pem), + "url encoded": urllib.parse.quote(stripped(pem)), + "chain": stripped(pem) + "," + stripped(make_cert("ISSUER", 0x1)), + } + for name, header in forms.items(): + seen: list[int] = [] + patch_status(monkeypatch, GOOD, seen) + resp = await unauth_client_session.get(ENDPOINT, headers={CERT_HEADER: header}) + assert resp.headers["Callsign"] == "CHARLIE03", name + assert seen == [0x99], f"{name} picked the wrong certificate from the chain" + + +def test_parse_leaf_takes_the_first_of_a_chain() -> None: + """Only the leaf is the client; the rest of the chain is issuers""" + leaf = make_cert("LEAF", 0x11) + issuer = make_cert("ISSUER", 0x22) + got = callsign_validity.parse_leaf(stripped(leaf) + "," + stripped(issuer)) + assert got is not None + assert got.serial_number == 0x11 + assert callsign_validity.common_name(got) == "LEAF" diff --git a/tests/test_rasenmaeher_api.py b/tests/test_rasenmaeher_api.py index e674b9ad..3a886f10 100644 --- a/tests/test_rasenmaeher_api.py +++ b/tests/test_rasenmaeher_api.py @@ -15,7 +15,7 @@ def test_version() -> None: """Make sure version matches expected""" - assert __version__ == "1.19.2+260912" + assert __version__ == "1.19.3+260916" @pytest.mark.asyncio(loop_scope="session") diff --git a/uv.lock b/uv.lock index 1822903c..c8c418ab 100644 --- a/uv.lock +++ b/uv.lock @@ -2110,7 +2110,7 @@ wheels = [ [[package]] name = "rasenmaeher-api" -version = "1.19.2+260912" +version = "1.19.3+260916" source = { editable = "." } dependencies = [ { name = "aiodns" },