From d48c35e2c7e00fad02e20022d22c311b230732d3 Mon Sep 17 00:00:00 2001 From: "shipmill-romamo[bot]" <338557544+shipmill-romamo[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 14:56:32 +0200 Subject: [PATCH] Stage an owner role's SCRAM-SHA-256 verifier, never its password The cloudfall_postgresql role staged the plaintext owner password on the host and sent ALTER ROLE ... PASSWORD with it; psql substitutes the value into the statement text, so the server received the plaintext and could log it (log_statement ddl or all, a failing statement) or show it in pg_stat_activity. The controller now computes the SCRAM-SHA-256 verifier with the standard library (fresh 16-byte salt, 4096 iterations, SASLprep as pg_saslprep does) in PostgreSQL's stored format and stages it in the same 0600 file; PostgreSQL stores a verifier as it is. Every path that runs services.yml goes through staged_password_files, so all switch. Answers and receipts still name the reference, the key and the password's SHA-256 (D-9). Fixes #140 Co-Authored-By: Claude Opus 5.5 --- changelog.d/140-scram-verifier.md | 3 + docs/secrets-guide.md | 13 +- docs/specs/006-standalone-postgresql.md | 5 +- .../tasks/owner_password.yml | 13 +- engine/tests/test_postgresql_owner_role.py | 16 ++- sdk/src/cloudfall/secrets.py | 95 ++++++++++++++- sdk/tests/test_owned_postgresql_converge.py | 20 +++- sdk/tests/test_secrets.py | 112 ++++++++++++++++++ 8 files changed, 249 insertions(+), 28 deletions(-) create mode 100644 changelog.d/140-scram-verifier.md diff --git a/changelog.d/140-scram-verifier.md b/changelog.d/140-scram-verifier.md new file mode 100644 index 0000000..cc71049 --- /dev/null +++ b/changelog.d/140-scram-verifier.md @@ -0,0 +1,3 @@ +### Security + +- An owner role's password no longer reaches the PostgreSQL host or server: `services.yml` and every path that runs it compute the password's SCRAM-SHA-256 verifier on the controller (a fresh 16-byte salt, 4096 iterations, the password prepared with SASLprep as PostgreSQL does) and stage that verifier in the `0600` file instead of the password, so `ALTER ROLE ... PASSWORD` sends a verifier PostgreSQL stores as it is and the plaintext can't show in the server log under `log_statement = 'ddl'` or `'all'`, in a failing statement, or in `pg_stat_activity`; answers and receipts still name the reference, the key, and the password's SHA-256 (#140) diff --git a/docs/secrets-guide.md b/docs/secrets-guide.md index e6bf0cc..415e65a 100644 --- a/docs/secrets-guide.md +++ b/docs/secrets-guide.md @@ -130,11 +130,14 @@ Every path that runs `services.yml` (the `converge_services` MCP tool, the `secrets`) before any playbook starts. A missing file or key stops it with `PRECONDITION`, naming the expected path and key. -The value is written to a `0600` file in a private temp directory outside the -project, and only that file's path reaches Ansible. The role copies the file -to the host as `0600`, `psql` reads it there, PostgreSQL stores the password as -SCRAM-SHA-256, and both files are removed when the run ends. Every task that -sees it runs with `no_log`. The answer's `owned_roles` names the reference, +The controller computes the password's SCRAM-SHA-256 verifier (a fresh salt, +4096 iterations, PostgreSQL's stored format) and writes the verifier, never the +password, to a `0600` file in a private temp directory outside the project; +only that file's path reaches Ansible. The role copies the file to the host as +`0600`, `psql` reads it there and sends the verifier, which PostgreSQL stores as +it is, and both files are removed when the run ends. The password itself never +reaches the host, so it can't show in the PostgreSQL log or `pg_stat_activity`. +Every task that sees the verifier runs with `no_log`. The answer's `owned_roles` names the reference, the key, and the value's SHA-256 (an operator proposal keeps them as `ownedRoles` in its receipt's `outcome`); the password is set on every converge, so a rotation is a commit in the private repository and a services converge. diff --git a/docs/specs/006-standalone-postgresql.md b/docs/specs/006-standalone-postgresql.md index 56d8453..68184d4 100644 --- a/docs/specs/006-standalone-postgresql.md +++ b/docs/specs/006-standalone-postgresql.md @@ -42,8 +42,9 @@ decrypted on the controller by the path that runs `services.yml` (the `converge_ MCP tool, the `services` step of `cloudfall migrate`, the operator's `converge-services` operation, and `cloudfall-engine playbook run services.yml`), each reading the secrets directory from a `--secrets-dir` flag (default `secrets`, as `secrets render` and -`mcp serve` use). It reaches the host in a `0600` file removed after the run, the role -sends it to `psql` on stdin with `no_log`, and PostgreSQL stores it as SCRAM. The value +`mcp serve` use). The controller computes its SCRAM-SHA-256 verifier, and only the +verifier reaches the host (D-9), in a `0600` file removed after the run; `psql` reads it +there with `no_log` and sends it, and PostgreSQL stores it as it is. The value never appears in the rendered inventory, a command line, Ansible output, a command's answer, or a receipt; the answer names the reference, the key, and the value's SHA-256, as `EnvironmentReceipt` does for environment files. A missing file or key stops the run diff --git a/engine/ansible/roles/cloudfall_postgresql/tasks/owner_password.yml b/engine/ansible/roles/cloudfall_postgresql/tasks/owner_password.yml index 4bec582..83b19c4 100644 --- a/engine/ansible/roles/cloudfall_postgresql/tasks/owner_password.yml +++ b/engine/ansible/roles/cloudfall_postgresql/tasks/owner_password.yml @@ -1,11 +1,14 @@ --- # Sets one owner role's password on every converge, so a rotation is a commit -# in the secrets repository and a services converge. The value reaches the -# host in a 0600 file removed after the run; psql reads the file itself, so -# the value is in no command line, no task argument, and no Ansible output. +# in the secrets repository and a services converge. The controller stages +# the password's SCRAM-SHA-256 verifier, never the password (D-9): it reaches +# the host in a 0600 file removed after the run, psql reads the file itself, +# and PostgreSQL stores a verifier as it is. The plaintext is in no host file, +# no statement the server logs or shows in pg_stat_activity, no command line, +# no task argument, and no Ansible output. - name: Set the password of owner role {{ cloudfall_postgresql_owner }} block: - - name: Stage the owner role password on the host + - name: Stage the owner role password verifier on the host ansible.builtin.copy: src: "{{ cloudfall_postgresql_password_files[cloudfall_postgresql_owner] }}" dest: "{{ cloudfall_postgresql_password_path }}" @@ -33,7 +36,7 @@ changed_when: true no_log: true always: - - name: Remove the staged owner role password + - name: Remove the staged owner role password verifier ansible.builtin.file: path: "{{ cloudfall_postgresql_password_path }}" state: absent diff --git a/engine/tests/test_postgresql_owner_role.py b/engine/tests/test_postgresql_owner_role.py index 2f2a807..f1046b8 100644 --- a/engine/tests/test_postgresql_owner_role.py +++ b/engine/tests/test_postgresql_owner_role.py @@ -17,6 +17,7 @@ import yaml from ansible.parsing.dataloader import DataLoader from ansible.template import Templar, trust_as_template +from cloudfall.secrets import scram_sha256_verifier from jinja2 import Environment, FileSystemLoader, StrictUndefined ROOT = Path(__file__).parents[2] @@ -24,6 +25,8 @@ POSTGRESQL = ROLES / "cloudfall_postgresql" DATA_MIGRATION = ROLES / "cloudfall_data_migration" PASSWORD = "s3cr3t-billing-value" # noqa: S105 - the value every output is grepped for +# What the controller stages in place of the password (D-9). +VERIFIER = scram_sha256_verifier(PASSWORD, salt=bytes(range(16))) BILLING: dict[str, object] = { "name": "billing", "owner": "billing", @@ -209,7 +212,7 @@ def _password_play(tmp_path: Path) -> tuple[Path, Path, Path]: psql.chmod(0o700) staged = tmp_path / "controller" / "postgresql-billing-billing" staged.parent.mkdir() - staged.write_text(PASSWORD, encoding="utf-8") + staged.write_text(VERIFIER, encoding="utf-8") host_path = tmp_path / "host" / ".cloudfall-owner-password" host_path.parent.mkdir() # Plain YAML, not Ansible's loader: the tasks are written out again. @@ -241,7 +244,7 @@ def _password_play(tmp_path: Path) -> tuple[Path, Path, Path]: # proves: S-006-6 -def test_s006_6_the_password_reaches_psql_through_a_removed_0600_file( +def test_s006_6_the_verifier_reaches_psql_through_a_removed_0600_file( tmp_path: Path, ) -> None: playbook, log, host_path = _password_play(tmp_path) @@ -257,11 +260,12 @@ def test_s006_6_the_password_reaches_psql_through_a_removed_0600_file( ) assert done.returncode == 0, done.stdout + done.stderr - assert PASSWORD not in done.stdout + done.stderr - assert (log / "value").read_text(encoding="utf-8") == PASSWORD + for secret in (PASSWORD, VERIFIER): + assert secret not in done.stdout + done.stderr + assert secret not in (log / "stdin").read_text(encoding="utf-8") + assert secret not in (log / "argv").read_text(encoding="utf-8") + assert (log / "value").read_text(encoding="utf-8") == VERIFIER assert (log / "mode").read_text(encoding="utf-8") == "0o600" - assert PASSWORD not in (log / "stdin").read_text(encoding="utf-8") - assert PASSWORD not in (log / "argv").read_text(encoding="utf-8") assert not host_path.exists() diff --git a/sdk/src/cloudfall/secrets.py b/sdk/src/cloudfall/secrets.py index 5f93d0e..4a2621d 100644 --- a/sdk/src/cloudfall/secrets.py +++ b/sdk/src/cloudfall/secrets.py @@ -9,17 +9,22 @@ from __future__ import annotations +import base64 import hashlib +import hmac import json import os import re import shutil +import stringprep import subprocess import tempfile +import unicodedata from contextlib import contextmanager from dataclasses import dataclass, field from datetime import UTC, datetime from pathlib import Path +from secrets import token_bytes from typing import TYPE_CHECKING, cast from cloudfall.domain import ( @@ -267,6 +272,85 @@ def owned_role_passwords( return tuple(passwords.values()) +_SCRAM_ITERATIONS = 4096 +_SCRAM_SALT_BYTES = 16 + + +def scram_sha256_verifier(password: str, *, salt: bytes | None = None) -> str: + """Return the SCRAM-SHA-256 verifier PostgreSQL would store for a password. + + The format is PostgreSQL's own, + ``SCRAM-SHA-256$:$:`` in base64, + with 4096 iterations and a fresh 16-byte salt, as PostgreSQL's defaults + are. PostgreSQL stores a value of this shape as it is, so the password + itself never reaches the server (D-9). The password is prepared with + SASLprep as PostgreSQL prepares it, so a client logs in with it. + """ + salt = token_bytes(_SCRAM_SALT_BYTES) if salt is None else salt + if not salt: + message = "a SCRAM-SHA-256 salt must not be empty" + raise ValueError(message) + salted = hashlib.pbkdf2_hmac( + "sha256", _saslprep(password).encode("utf-8"), salt, _SCRAM_ITERATIONS + ) + stored_key = hashlib.sha256(hmac.digest(salted, b"Client Key", "sha256")).digest() + server_key = hmac.digest(salted, b"Server Key", "sha256") + return ( + f"SCRAM-SHA-256${_SCRAM_ITERATIONS}:{_b64(salt)}" + f"${_b64(stored_key)}:{_b64(server_key)}" + ) + + +def _b64(raw: bytes) -> str: + return base64.b64encode(raw).decode("ascii") + + +_PROHIBITED = ( + stringprep.in_table_c12, + stringprep.in_table_c21, + stringprep.in_table_c22, + stringprep.in_table_c3, + stringprep.in_table_c4, + stringprep.in_table_c5, + stringprep.in_table_c6, + stringprep.in_table_c7, + stringprep.in_table_c8, + stringprep.in_table_c9, + stringprep.in_table_a1, +) + + +def _saslprep(password: str) -> str: + """Prepare a password as PostgreSQL's ``pg_saslprep`` does (RFC 4013). + + An ASCII password stays as it is. Otherwise non-ASCII spaces map to a + space, characters mapped to nothing drop, and the result is NFKC + normalized; a result that is empty, holds a prohibited or unassigned + character, or breaks the bidirectional rules is not used, and PostgreSQL + falls back to the password as given, so this does too. + """ + if password.isascii(): + return password + # A space first, as PostgreSQL maps it: U+200B is in both tables. + mapped = "".join( + " " if stringprep.in_table_c12(char) else char + for char in password + if stringprep.in_table_c12(char) or not stringprep.in_table_b1(char) + ) + prepared = unicodedata.normalize("NFKC", mapped) + if not prepared or any( + prohibited(char) for char in prepared for prohibited in _PROHIBITED + ): + return password + if any(stringprep.in_table_d1(char) for char in prepared) and ( + any(stringprep.in_table_d2(char) for char in prepared) + or not stringprep.in_table_d1(prepared[0]) + or not stringprep.in_table_d1(prepared[-1]) + ): + return password + return prepared + + PASSWORD_FILES_VARIABLE = "cloudfall_services_password_files" # noqa: S105 - a name """The ``services.yml`` variable mapping service, then owner, to a staged file.""" @@ -275,11 +359,12 @@ def owned_role_passwords( def staged_password_files( passwords: tuple[OwnedRolePassword, ...], ) -> Iterator[dict[str, object]]: - """Write each password to a 0600 file in a private temp dir, then remove it. + """Write each password's SCRAM verifier to a 0600 file, then remove it. - Yields the extra vars ``services.yml`` reads: the controller paths only, - so the values never cross a command line, the inventory, or the project. - No passwords yields no variable, leaving the run as it was. + The files sit in a private temp dir and hold the verifier only, never the + password (D-9). Yields the extra vars ``services.yml`` reads: the + controller paths only, so neither crosses a command line, the inventory, + or the project. No passwords yields no variable, leaving the run as it was. """ if not passwords: yield {} @@ -294,7 +379,7 @@ def staged_password_files( path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, _FILE_MODE ) with os.fdopen(descriptor, "w", encoding="utf-8") as handle: - handle.write(password.value) + handle.write(scram_sha256_verifier(password.value)) files.setdefault(password.service.value, {})[password.owner.value] = str( path ) diff --git a/sdk/tests/test_owned_postgresql_converge.py b/sdk/tests/test_owned_postgresql_converge.py index 51e09b0..3c5e6b7 100644 --- a/sdk/tests/test_owned_postgresql_converge.py +++ b/sdk/tests/test_owned_postgresql_converge.py @@ -2,7 +2,7 @@ ``services.yml`` needs real hosts, so these tests run every path that starts it (the lifecycle call the MCP tool, the operator, and ``cloudfall migrate`` -share) with a recording step runner: it reads the staged password file the +share) with a recording step runner: it reads the staged verifier file the way the role's copy task does, and the tests grep every output for the value. """ @@ -55,6 +55,7 @@ from cloudfall_engine.ansible_inventory import render_ansible_inventory from mcp import ClientSession, StdioServerParameters, stdio_client from mcp.types import TextContent +from test_secrets import verifies ROOT = Path(__file__).parents[2] EXAMPLES = ROOT / "config" / "examples" @@ -178,6 +179,15 @@ def _provider(secrets: Path) -> SopsSecretProvider: return SopsSecretProvider(secrets_directory=secrets, decrypt=_plaintext) +def _staged_verifier(recorder: Recorder) -> str: + """Return the one staged file: a SCRAM-SHA-256 verifier, never the password.""" + ((content, _, _),) = recorder.staged.values() + assert content.startswith("SCRAM-SHA-256$4096:") + assert verifies(content, PASSWORD) + assert PASSWORD not in content + return content + + def _everything_written(project: Path) -> str: return "\n".join( path.read_text(encoding="utf-8", errors="replace") @@ -187,7 +197,7 @@ def _everything_written(project: Path) -> str: # proves: S-006-6 -def test_s006_6_converge_stages_the_password_and_answers_its_hash( +def test_s006_6_converge_stages_a_verifier_and_answers_the_password_hash( tmp_path: Path, ) -> None: project = _project(tmp_path) @@ -199,7 +209,7 @@ def test_s006_6_converge_stages_the_password_and_answers_its_hash( render, playbook = recorder.steps assert playbook.argv[-1].endswith("services.yml") assert recorder.staged == { - "postgresql-billing/billing": (PASSWORD, 0o600, 0o700) + "postgresql-billing/billing": (_staged_verifier(recorder), 0o600, 0o700) } (staged,) = recorder.paths assert not staged.exists() @@ -348,7 +358,7 @@ def test_s006_6_operator_approve_answers_and_receipts_the_hash_only( via=ApprovalChannel.TERMINAL, ) - assert recorder.staged["postgresql-billing/billing"][0] == PASSWORD + _staged_verifier(recorder) answer = approved.as_document() outcome = cast("dict[str, dict[str, object]]", answer["spec"])["outcome"] assert outcome["ownedRoles"] == [OWNED_ROLE] @@ -452,7 +462,7 @@ def test_s006_6_migrate_records_the_hash_in_its_services_step(tmp_path: Path) -> } ], } - assert recorder.staged["postgresql-billing/billing"][0] == PASSWORD + _staged_verifier(recorder) assert PASSWORD not in plan assert all(PASSWORD not in " ".join(step.argv) for step in recorder.steps) diff --git a/sdk/tests/test_secrets.py b/sdk/tests/test_secrets.py index d624a87..c8c680b 100644 --- a/sdk/tests/test_secrets.py +++ b/sdk/tests/test_secrets.py @@ -2,6 +2,9 @@ from __future__ import annotations +import base64 +import hashlib +import hmac import stat from pathlib import Path @@ -12,6 +15,7 @@ SecretsError, SopsSecretProvider, render_environment, + scram_sha256_verifier, ) ROOT = Path(__file__).parents[2] @@ -153,3 +157,111 @@ def test_declared_and_secret_key_conflicts_fail_fast(tmp_path: Path) -> None: assert caught.value.code == "secrets_key_conflict" assert "FEATURE_SIGNUPS" in caught.value.message assert "exactly one place" in caught.value.message + + +def _verifier_parts(verifier: str) -> tuple[int, bytes, bytes, bytes]: + """Split PostgreSQL's stored SCRAM-SHA-256 format, refusing any other shape.""" + scheme, _, rest = verifier.partition("$") + assert scheme == "SCRAM-SHA-256" + iterations_and_salt, _, keys = rest.partition("$") + iterations, _, salt = iterations_and_salt.partition(":") + stored_key, _, server_key = keys.partition(":") + decoded = [ + base64.b64decode(part, validate=True) + for part in (salt, stored_key, server_key) + ] + assert all( + base64.b64encode(raw).decode() == part + for raw, part in zip(decoded, (salt, stored_key, server_key), strict=True) + ) + return int(iterations), decoded[0], decoded[1], decoded[2] + + +def verifies(verifier: str, password: str) -> bool: + """Recompute StoredKey and ServerKey from the verifier's salt and iterations.""" + iterations, salt, stored_key, server_key = _verifier_parts(verifier) + salted = hashlib.pbkdf2_hmac("sha256", password.encode(), salt, iterations) + client_key = hmac.digest(salted, b"Client Key", "sha256") + return ( + hashlib.sha256(client_key).digest() == stored_key + and hmac.digest(salted, b"Server Key", "sha256") == server_key + ) + + +# RFC 7677 section 3: the SCRAM-SHA-256 exchange for user "user", password "pencil". +_RFC7677_SALT = base64.b64decode("W22ZaJ0SNY7soEsUEjb6gQ==") +_RFC7677_AUTH_MESSAGE = ( + b"n=user,r=rOprNGfwEbeRWgbNEkqO," + b"r=rOprNGfwEbeRWgbNEkqO%hvYDpWUa2RaTCAfuxFIlj)hNlF$k0," + b"s=W22ZaJ0SNY7soEsUEjb6gQ==,i=4096," + b"c=biws,r=rOprNGfwEbeRWgbNEkqO%hvYDpWUa2RaTCAfuxFIlj)hNlF$k0" +) +_RFC7677_CLIENT_PROOF = base64.b64decode( + "dHzbZapWIk4jUhN+Ute9ytag9zjfMHgsqmmiz7AndVQ=" +) +_RFC7677_SERVER_SIGNATURE = base64.b64decode( + "6rriTRBi23WpRR/wtup+mMhUZUn/dB5nLTJRsjl95G4=" +) + + +def test_the_verifier_authenticates_the_rfc_7677_exchange() -> None: + verifier = scram_sha256_verifier("pencil", salt=_RFC7677_SALT) + + iterations, salt, stored_key, server_key = _verifier_parts(verifier) + assert verifier.startswith("SCRAM-SHA-256$4096:W22ZaJ0SNY7soEsUEjb6gQ==$") + assert (iterations, salt) == (4096, _RFC7677_SALT) + # What the server checks: the proof recovers a ClientKey hashing to StoredKey, + # and ServerKey signs the same exchange the RFC's server signed. + signature = hmac.digest(stored_key, _RFC7677_AUTH_MESSAGE, "sha256") + client_key = bytes( + a ^ b for a, b in zip(_RFC7677_CLIENT_PROOF, signature, strict=True) + ) + assert hashlib.sha256(client_key).digest() == stored_key + assert ( + hmac.digest(server_key, _RFC7677_AUTH_MESSAGE, "sha256") + == _RFC7677_SERVER_SIGNATURE + ) + + +def test_the_verifier_has_a_fresh_salt_and_never_holds_the_password() -> None: + password = "s3cr3t-billing-value" # noqa: S105 - a test value + + first = scram_sha256_verifier(password) + second = scram_sha256_verifier(password) + + assert first != second + for verifier in (first, second): + assert len(_verifier_parts(verifier)[1]) == 16 + assert verifies(verifier, password) + assert not verifies(verifier, password + "x") + assert password not in verifier + + +@pytest.mark.parametrize( + ("password", "prepared"), + [ + ("pass\u00a0word", "pass word"), + ("pass\u200bword", "pass word"), + ("pass\u00adword", "password"), + ("\uff21bc", "Abc"), + ("ab\ue000", "ab\ue000"), + ("ctl\x07", "ctl\x07"), + ], + ids=[ + "space-mapped", + "zero-width-space-mapped", + "soft-hyphen-dropped", + "nfkc", + "prohibited-kept", + "ascii-kept", + ], +) +def test_the_verifier_hashes_the_password_as_postgresql_saslprep_does( + password: str, prepared: str +) -> None: + assert verifies(scram_sha256_verifier(password), prepared) + + +def test_an_empty_salt_is_refused() -> None: + with pytest.raises(ValueError, match="salt"): + scram_sha256_verifier("pencil", salt=b"")