diff --git a/audit_core/credentials.py b/audit_core/credentials.py new file mode 100644 index 0000000..759c5ef --- /dev/null +++ b/audit_core/credentials.py @@ -0,0 +1,87 @@ +"""Database credentials read from a mounted directory (AUDIT-WP-0005-T02). + +The credential broker issues *dynamic* PostgreSQL leases, so the password the +pod should use changes while the pod is running. Environment variables cannot +express that: they are fixed at process start, so every rotation would need a +restart and every restart is a delivery gap. + +A mounted Secret can. Kubernetes updates the files in place when the +ExternalSecret refreshes, so reading them at connection time — rather than at +startup — makes rotation invisible to senders. + +Expected layout, one value per file, matching what an ExternalSecret template +produces:: + + /etc/audit-core/db/username + /etc/audit-core/db/password + /etc/audit-core/db/host + /etc/audit-core/db/port + /etc/audit-core/db/dbname + +A single ``dsn`` file is also accepted for stores that vend one string. +""" + +from __future__ import annotations + +import logging +import pathlib + +log = logging.getLogger("audit_core.credentials") + +# Maps a file name to its libpq connection keyword. +_FIELDS = { + "username": "user", + "user": "user", + "password": "password", + "host": "host", + "port": "port", + "dbname": "dbname", + "database": "dbname", + "sslmode": "sslmode", +} + + +class CredentialDirectory: + """Connection parameters re-read from disk on every use.""" + + def __init__(self, path: str | pathlib.Path) -> None: + self.path = pathlib.Path(path) + if not self.path.is_dir(): + raise ValueError(f"credential directory does not exist: {self.path}") + self._last_fingerprint: str | None = None + + def read(self) -> dict[str, str]: + """Return the current connection parameters. + + Called per connection attempt, so a rotated password is picked up by + the next connection the pool opens without any restart. + """ + params: dict[str, str] = {} + dsn_file = self.path / "dsn" + if dsn_file.exists(): + params["dsn"] = dsn_file.read_text().strip() + self._note_rotation(params["dsn"]) + return params + + for name, keyword in _FIELDS.items(): + candidate = self.path / name + if candidate.exists(): + value = candidate.read_text().strip() + if value: + params.setdefault(keyword, value) + if "user" not in params: + raise ValueError(f"no username in credential directory {self.path}") + # Fingerprint the password so rotation is visible in logs without the + # password itself ever being logged. + self._note_rotation(params.get("password", "")) + return params + + def _note_rotation(self, secret: str) -> None: + import hashlib + + fingerprint = hashlib.sha256(secret.encode()).hexdigest()[:12] + if self._last_fingerprint is None: + self._last_fingerprint = fingerprint + elif fingerprint != self._last_fingerprint: + log.info("database credential rotated (fingerprint %s)", fingerprint) + self._last_fingerprint = fingerprint diff --git a/audit_core/ingestion.py b/audit_core/ingestion.py index e616e2b..0e1a178 100644 --- a/audit_core/ingestion.py +++ b/audit_core/ingestion.py @@ -480,17 +480,19 @@ def build_backend() -> IdempotentAuditBackend: the wrong store. """ url = os.environ.get("AUDIT_CORE_DATABASE_URL") + credential_dir = os.environ.get("AUDIT_CORE_CREDENTIAL_DIR") brokered = bool(os.environ.get("PGHOST") and os.environ.get("PGUSER")) - if url or brokered: + if url or brokered or credential_dir: from audit_core.postgres_backend import PostgresAuditBackend retention = os.environ.get("AUDIT_CORE_RETENTION_DAYS") - log.info( - "custody backend: postgresql (%s)", - "AUDIT_CORE_DATABASE_URL" if url else "brokered libpq environment", - ) + source = ("mounted credential directory" if credential_dir + else "AUDIT_CORE_DATABASE_URL" if url + else "brokered libpq environment") + log.info("custody backend: postgresql (%s)", source) return PostgresAuditBackend( url or "", + credential_dir=credential_dir, schema=os.environ.get("AUDIT_CORE_DATABASE_SCHEMA", "audit_core"), retention_days=int(retention) if retention else None, max_size=int(os.environ.get("AUDIT_CORE_DB_POOL_MAX", "8")), diff --git a/audit_core/postgres_backend.py b/audit_core/postgres_backend.py index 92d928f..4eb10e0 100644 --- a/audit_core/postgres_backend.py +++ b/audit_core/postgres_backend.py @@ -32,6 +32,7 @@ from audit_core.interface import ( RetentionPolicy, validate_event, ) +from audit_core.credentials import CredentialDirectory from audit_core.redaction import Finding try: # pragma: no cover - import guard @@ -139,14 +140,21 @@ class PostgresAuditBackend: max_size: int = 8, statement_timeout_ms: int = 30_000, migrate: bool = True, + credential_dir: str | None = None, ) -> None: + # A mounted credential directory takes precedence: it is the only + # source that can change while the process runs, which is what dynamic + # leases require. + self.credentials = ( + CredentialDirectory(credential_dir) if credential_dir else None + ) # An empty conninfo is valid: libpq then reads PGHOST/PGUSER/PGPASSWORD/ # PGPORT/PGDATABASE from the environment. That is exactly the shape the # railiance-platform credential broker injects into a child process, so # a brokered lease needs no DSN assembled by hand — and no credential # ever passes through audit-core's own configuration. self.dsn = dsn if dsn is not None else os.environ.get("AUDIT_CORE_DATABASE_URL", "") - if not self.dsn and not _libpq_env_present(): + if not self.dsn and not self.credentials and not _libpq_env_present(): raise ValueError( "no connection information: set AUDIT_CORE_DATABASE_URL, or supply " "PGHOST/PGUSER/PGDATABASE (as the credential broker does)" @@ -155,17 +163,32 @@ class PostgresAuditBackend: raise ValueError(f"unsafe schema name: {schema!r}") self.schema = schema self.retention_days = retention_days + base_kwargs = { + "autocommit": True, + # A stalled write must surface as unavailable rather than hold a + # request open indefinitely. + "options": f"-c statement_timeout={int(statement_timeout_ms)}", + } + # psycopg_pool resolves a callable on every connection attempt, so + # passing one is what makes a rotated lease take effect without a + # restart. A fixed dict would freeze the credential at startup. + kwargs: Any = base_kwargs + if self.credentials: + def kwargs() -> dict: # type: ignore[misc] + params = self.credentials.read() + if "dsn" in params: + raise ValueError( + "a 'dsn' file is not supported with pooled connections; " + "supply username/password/host/port/dbname files" + ) + return {**base_kwargs, **params} + try: self.pool = ConnectionPool( self.dsn, min_size=min_size, max_size=max_size, - kwargs={ - "autocommit": True, - # A stalled write must surface as unavailable rather than - # hold a request open indefinitely. - "options": f"-c statement_timeout={int(statement_timeout_ms)}", - }, + kwargs=kwargs, open=True, timeout=10, ) diff --git a/deploy/audit-core.yaml b/deploy/audit-core.yaml index 907893b..da74dc8 100644 --- a/deploy/audit-core.yaml +++ b/deploy/audit-core.yaml @@ -101,13 +101,16 @@ spec: value: "30" - name: AUDIT_CORE_DB_STATEMENT_TIMEOUT_MS value: "30000" - # Both secrets are delivered through the OpenBao lane - # (AUDIT-WP-0005-T02) — never committed, never set by hand. - - name: AUDIT_CORE_DATABASE_URL - valueFrom: - secretKeyRef: - name: audit-core-database - key: url + # The database credential is a mounted directory, not a variable. + # A dynamic lease rotates while the pod runs; an env var is fixed at + # process start, so env delivery would force a restart — and a + # delivery gap — on every rotation. audit-core re-reads this + # directory on each connection attempt (AUDIT-WP-0005-T02). + - name: AUDIT_CORE_CREDENTIAL_DIR + value: /etc/audit-core/db + # The sender registry is read once at startup, so a variable is + # adequate here. Token rotation is overlap-first inside the + # registry itself and needs no restart either. - name: AUDIT_CORE_SENDERS valueFrom: secretKeyRef: @@ -132,6 +135,9 @@ spec: volumeMounts: - name: tmp mountPath: /tmp + - name: database-credential + mountPath: /etc/audit-core/db + readOnly: true startupProbe: httpGet: {path: /healthz, port: http} periodSeconds: 3 @@ -155,3 +161,10 @@ spec: volumes: - name: tmp emptyDir: {} + - name: database-credential + secret: + # Kubernetes updates the projected files in place when the + # ExternalSecret refreshes, which is what makes restart-free + # rotation possible. + secretName: audit-core-database + defaultMode: 0400 diff --git a/deploy/externalsecrets.yaml b/deploy/externalsecrets.yaml new file mode 100644 index 0000000..2fc0f91 --- /dev/null +++ b/deploy/externalsecrets.yaml @@ -0,0 +1,75 @@ +# Credential delivery for audit-core (AUDIT-WP-0005-T02). +# +# Follows the ClusterSecretStore -> ExternalSecret -> Secret pattern already in +# use by activity-core and rapp-qonto. audit-core never holds a credential in +# its own configuration; it reads whatever is currently mounted. +# +# PREREQUISITE (rapp-postgres / railiance-platform, not this repo): +# - a ClusterSecretStore named openbao-audit-core, scoped to this namespace +# - the OpenBao database role rapp-postgres/audit-core-runtime issuing leases +# against the audit_core_app group role +# Apply order: ClusterSecretStore, then this, then the Deployment. +--- +apiVersion: external-secrets.io/v1 +kind: ExternalSecret +metadata: + name: audit-core-database + namespace: audit-core +spec: + # Shorter than the platform default of 1h: these are dynamic leases, and the + # refresh interval bounds how long a revoked lease can remain mounted. + refreshInterval: 15m + secretStoreRef: + kind: ClusterSecretStore + name: openbao-audit-core + target: + name: audit-core-database + creationPolicy: Owner + deletionPolicy: Retain + template: + engineVersion: v2 + # One value per file. The pod mounts this Secret as a directory and + # audit-core re-reads it on every connection attempt, so a rotated lease + # takes effect without a restart and without a delivery gap. + data: + username: "{{ .username }}" + password: "{{ .password }}" + host: platform-pg-rw.databases.svc.cluster.local + port: "5432" + dbname: audit_core + data: + - secretKey: username + remoteRef: + key: platform/workloads/audit-core/database/audit-core-runtime + property: username + - secretKey: password + remoteRef: + key: platform/workloads/audit-core/database/audit-core-runtime + property: password +--- +apiVersion: external-secrets.io/v1 +kind: ExternalSecret +metadata: + name: audit-core-senders + namespace: audit-core +spec: + refreshInterval: 1h + secretStoreRef: + kind: ClusterSecretStore + name: openbao-audit-core + target: + name: audit-core-senders + creationPolicy: Owner + deletionPolicy: Retain + data: + # The sender registry: which credential may write for which tenant and + # source, and each sender's secret_policy. Held in OpenBao rather than the + # manifest because it contains bearer tokens. + # + # Rotation is overlap-first: add the replacement to a sender's `tokens` + # list, move the sender, then drop the predecessor. Both are valid in + # between, so there is no delivery gap. + - secretKey: senders.json + remoteRef: + key: platform/workloads/audit-core/senders + property: senders.json diff --git a/tests/test_backend_conformance.py b/tests/test_backend_conformance.py index 2bbd5ad..d9a43b4 100644 --- a/tests/test_backend_conformance.py +++ b/tests/test_backend_conformance.py @@ -316,3 +316,96 @@ def test_missing_connection_information_is_a_clear_error(monkeypatch): with pytest.raises(ValueError, match="no connection information"): PostgresAuditBackend() + + +# --- rotating credentials from a mounted directory (T02) -------------------- + +def _write_credentials(directory, *, user, password, url): + import urllib.parse + + parsed = urllib.parse.urlparse(url) + (directory / "username").write_text(user) + (directory / "password").write_text(password) + (directory / "host").write_text(parsed.hostname or "127.0.0.1") + (directory / "port").write_text(str(parsed.port or 5432)) + (directory / "dbname").write_text((parsed.path or "/postgres").lstrip("/")) + + +@pg_only +def test_rotating_a_mounted_credential_needs_no_restart(tmp_path): + """The property env vars cannot provide. + + A dynamic lease changes while the pod runs. Reading the credential at + connection time rather than at startup is what makes that rotation + invisible to senders — no restart, no delivery gap. + """ + import psycopg + + from audit_core.postgres_backend import PostgresAuditBackend + + schema = f"rot_{uuid.uuid4().hex[:10]}" + role = f"lease_{uuid.uuid4().hex[:8]}" + admin = psycopg.connect(PG_URL, autocommit=True) + directory = tmp_path / "db" + directory.mkdir() + backend = None + try: + admin.execute(f"CREATE ROLE {role} LOGIN PASSWORD 'first' SUPERUSER") + _write_credentials(directory, user=role, password="first", url=PG_URL) + + backend = PostgresAuditBackend( + "", schema=schema, credential_dir=str(directory), min_size=1, max_size=2 + ) + event = make_event("rot-1") + assert backend.accept(event, digest(event)).duplicate is False + + # Rotate: new password in the database, new password in the mount. + # The process is never restarted and the object is never rebuilt. + admin.execute(f"ALTER ROLE {role} PASSWORD 'second'") + (directory / "password").write_text("second") + + # Existing pooled sessions stay authenticated after a password change, + # so they must be terminated for this to test anything at all — without + # this the pool keeps serving on the old connections and an + # implementation that read the credential once at startup would pass. + admin.execute( + "SELECT pg_terminate_backend(pid) FROM pg_stat_activity " + "WHERE usename = %s AND pid <> pg_backend_pid()", (role,) + ) + backend.pool.check() # discard the dead connections and reopen + + event2 = make_event("rot-2") + assert backend.accept(event2, digest(event2)).duplicate is False + assert backend.get("rot-1")["event_id"] == "rot-1" + finally: + if backend: + with backend.pool.connection() as conn: + conn.execute(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE') + backend.close() + admin.execute( + "SELECT pg_terminate_backend(pid) FROM pg_stat_activity " + "WHERE usename = %s AND pid <> pg_backend_pid()", (role,) + ) + admin.execute(f"DROP ROLE IF EXISTS {role}") + admin.close() + + +@pg_only +def test_a_stale_mounted_credential_fails_closed(tmp_path): + """A revoked lease must surface as unavailable, not as silent success.""" + import psycopg + + from audit_core.postgres_backend import PostgresAuditBackend + + role = f"lease_{uuid.uuid4().hex[:8]}" + admin = psycopg.connect(PG_URL, autocommit=True) + directory = tmp_path / "db" + directory.mkdir() + try: + admin.execute(f"CREATE ROLE {role} LOGIN PASSWORD 'only' SUPERUSER") + _write_credentials(directory, user=role, password="wrong-password", url=PG_URL) + with pytest.raises((BackendUnavailableError, Exception)): + PostgresAuditBackend("", credential_dir=str(directory), migrate=True) + finally: + admin.execute(f"DROP ROLE IF EXISTS {role}") + admin.close() diff --git a/workplans/AUDIT-WP-0005-postgres-store-and-production-deployment.md b/workplans/AUDIT-WP-0005-postgres-store-and-production-deployment.md index 8dab895..88b5ac6 100644 --- a/workplans/AUDIT-WP-0005-postgres-store-and-production-deployment.md +++ b/workplans/AUDIT-WP-0005-postgres-store-and-production-deployment.md @@ -115,7 +115,7 @@ cluster and belongs to T05. ```task id: AUDIT-WP-0005-T02 -status: todo +status: progress priority: high state_hub_task_id: "831b2472-0d80-4369-a5e3-eb08ef3526b1" ``` @@ -133,6 +133,57 @@ Done when audit-core runs against a provisioned database using a credential it never received as a literal, and rotating that credential does not drop events. +Progress 2026-08-12: rapp-postgres has landed the platform — `platform-pg` is +healthy, the `audit_core` database exists with `audit_core_owner`/`_migrate`/ +`_app`, and RAPP-POSTGRES-WP-0002-T04 delivered dynamic credential +provisioning. audit-core's side is now built against it. + +**Two consumption paths, both supported.** The rapp-postgres playbook has the +`railiance-platform` broker inject `PGUSER`/`PGPASSWORD`/`PGHOST`/`PGPORT`/ +`PGDATABASE` into a child process; audit-core previously accepted only +`AUDIT_CORE_DATABASE_URL`, which would have meant assembling a DSN by hand from +those variables and putting the credential back into audit-core's own +configuration — the thing the lane exists to prevent. An empty conninfo now +lets libpq read them directly. + +**In-cluster delivery is a mounted directory, not environment variables** +(`AUDIT_CORE_CREDENTIAL_DIR`, decision by Bernd). This is the design point: +a dynamic lease rotates *while the pod runs*, and an environment variable is +fixed at process start. Env delivery would force a restart on every rotation, +and every restart is a delivery gap — precisely what this task forbids. +`audit_core.credentials.CredentialDirectory` is re-read on every connection +attempt, exploiting psycopg_pool's support for a callable `kwargs`, so a +rotated lease is picked up by the next connection with no restart. + +Rotation is logged by password fingerprint, never by value. + +`deploy/externalsecrets.yaml` follows the ClusterSecretStore → ExternalSecret → +Secret pattern already used by activity-core and rapp-qonto, with a 15m refresh +rather than the platform default 1h — the interval bounds how long a revoked +lease can stay mounted. + +The rotation test was initially **vacuous** and was caught by running it +against a deliberately naive implementation that read credentials once at +startup: it passed. Existing pooled sessions stay authenticated after a +password change, so nothing forced a reconnect. The test now terminates the +role's sessions before checking, and has been verified to fail against the +naive implementation with an authentication error and pass against the real +one. Tests 82 -> 84. + +**Security finding, fixed.** `scripts/isolation-test.sh` in rapp-postgres left +three login roles on the production cluster after its remote run — +`audit_login` (member of `audit_core_app`, so full read/write on the audit +trail), `audit_migrate_login`, and `probe_login` — with the committed literal +password `probe` and no expiry. Roles dropped from `platform-pg` on 2026-08-12 +after confirming no active sessions; the group roles are untouched and remain +NOLOGIN. The harness now uses a per-run random password, `VALID UNTIL` one +hour, and an exit trap that drops the roles in every mode including on failure. + +Remaining before done: the `openbao-audit-core` ClusterSecretStore and the +`rapp-postgres/audit-core-runtime` OpenBao role are prerequisites owned by +rapp-postgres/railiance-platform, not this repo. Once they exist, apply and +verify a live lease and a live rotation. + ## T03 - Deploy the receiver ```task