diff --git a/docs/ASVS-L2-PHASE0-CHANGES.md b/docs/ASVS-L2-PHASE0-CHANGES.md index 71b318b4b..d5712f537 100644 --- a/docs/ASVS-L2-PHASE0-CHANGES.md +++ b/docs/ASVS-L2-PHASE0-CHANGES.md @@ -125,6 +125,7 @@ for, not how it is protected before it gets there. | AD transport | LDAPS (TLS) with `CERT_REQUIRED` by default; optional internal CA via `ad_tls_ca_cert_file` | OS / configured CA trust | Managed by the directory / OS trust store | | SQL Server transport | TLS via ODBC Driver 18 (`Encrypt=yes`, `TrustServerCertificate=no` by default) | Server certificate | Managed by SQL Server / OS trust store | | Console → engine TLS (a remote native client — the Qt-free `apiclient`, today the test harness; the PySide6 desktop console is retired) | Verifies the engine API server cert: **OS trust store** by default (`truststore.SSLContext`) or a pinned PEM via `cacert` (`ssl.create_default_context`); opt-in client cert (mTLS) via `load_cert_chain`; `ssl` in `apiclient/client.py` (CONSOLE-3; extracted from the since-deleted `console/client.py` per ADR 0088) | OS trust store / operator-supplied CA PEM (`cacert`) | Managed by the OS trust store; `cacert` for a self-signed / internal-CA engine | +| Load-test harness → spawned-engine TLS (BACKLOG #1276 part A) | The engine always serves TLS now and mints a self-signed placeholder when no operator cert is configured — which every harness driver hardcoded `http://` against, so no spawned node ever became healthy. The harness supplies its own certificate instead of chasing the one the engine mints: one EC P-256 pair minted per process via `pki.make_self_signed`, handed to each node as `[api].tls_cert_file`/`tls_key_file` (`ensure_api_tls_material` honours it and never mints over it), and pinned by every client that talks to that node (`ssl.create_default_context(cafile=...)` in `harness/load/tlsmat.py`) — the same posture `apiclient.EngineClient`'s `cacert` already supports, just resolved locally instead of by an operator. **Usage scope: non-prod only.** The pair lives in a per-process temp directory, covers loopback names alone (`127.0.0.1`/`localhost`/`::1`), and is inherited by a spawned child harness process (`connscale-remote`) via environment so a parent and its child never mint two different anchors for the same engines. A second host in a two-box shardcert rig is out of scope by design — this anchor cannot cover a certificate it never minted, so those URLs stay `http://` and say why inline. | Minted in-process (non-persistent, never written outside a per-run temp dir) | Regenerated every harness run; nothing to rotate | | Tray → engine TLS (local status probe, [ADR 0113](adr/0113-windows-tray-service-manager-stdlib-ctypes-tokenless.md) 2026-07-22 amendment) | Verifies the engine API server cert on the tray's **tokenless** `/health` + `/ui` probes when `[api].tls_cert_file` makes the loopback bind serve https: **OS trust store only** (`truststore.SSLContext`); `ssl` in `tray/probe.py`. **No pinned-PEM option and no `verify=False` escape** — an AST test in `tests/test_tray_probe.py` freezes that | Windows machine trust store (an internal-CA/AD-CS cert verifies as-is; a self-signed engine cert is installed under Trusted Root) | Managed by the OS trust store; a verification failure renders the engine `DOWN`, never an unverified connection | | Cert tooling — `.pfx` import / read-only inventory / self-signed dev cert (BACKLOG #71/#72) | `cryptography` in [`pki.py`](../messagefoundry/pki.py) — the single PKI call site for the `cert` CLI group: PKCS#12/.pfx import (`pkcs12.load_key_and_certificates`) writes the leaf cert + private key + CA chain to the PEM files the TLS loaders already read; a **read-only** inventory reads only **public** cert facts (subject/issuer/notAfter/SAN/days) via `x509`; `make_self_signed` mints an **EC P-256 / SHA-256** self-signed cert for **non-prod** bring-up. `pipeline/cert_expiry.py` shares this module's `read_cert_facts` (so it no longer imports `cryptography` itself). **Usage scope:** an operator CLI utility — it imports/serializes/inspects operator-supplied cert material and mints throwaway dev certs; it holds no long-lived engine key, signs no message, and encrypts nothing at rest. The imported/minted **private-key** PEM is written `O_EXCL` + `0o600` + the `_secure_file` DACL; the `.pfx` passphrase is env-only (`MEFOR_PFX_PASSWORD`), never a CLI arg, never logged/echoed/put in an exception | Operator-supplied `.pfx` bundle → cert/key/CA PEM files on disk (`--out-dir`) | Managed by the operator / PKI; self-signed dev certs are disposable (default 365-day validity) | | Engine/console seam identity (BACKLOG #1220) | SHA-256 over the **discovered** engine/console contract surface, truncated to 16 hex characters, published as `ENGINE_UI_SEAM` in [`api/_ui_seam.py`](../messagefoundry/api/_ui_seam.py) and derived by `hashlib` in [`scripts/webconsole_seam_snapshot.py`](../scripts/webconsole_seam_snapshot.py). **A change detector, not a security control** — no secret, no key, no message authentication, and nothing user- or PHI-derived is hashed; the input is a serialization of public type signatures, field names, enum members and `Literal` values. It replaced a hand-picked incrementing integer, which two unlanded branches had both claimed for two different contract changes while the golden snapshot auto-merged clean under one value. What it needs is accidental-collision avoidance across the contract surfaces this project will ever produce: at 64 bits the birthday bound is 2.7e-12 for 10,000 distinct surfaces, about 500x the ~20 seam moves to date. Preimage resistance buys nothing — anyone able to craft a colliding surface already has commit access to the file holding the constant. SHA-256 rather than BLAKE2 or a non-approved digest only because the engine renders a `fips_mode` attestation and a non-approved hash in the shipped surface invites a FIPS question for no gain | Not a secret: the digest is committed in source and mirrored in the console's `SUPPORTED_ENGINE_SEAMS` | Recomputed by `scripts/webconsole_seam_snapshot.py --write` whenever the contract changes; a stale value fails `tests/test_webconsole_seam_snapshot.py` | diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 83bef1c4d..e37d512eb 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -13310,6 +13310,7 @@ measurement from this row's subject and it is named here rather than performed.* > 🔢 **Re-scored 2026-08-20 -> P2.** Value **6/10** · Difficulty **6/10** · _big bet_. Nothing mints a certificate on first start: tls_enabled is literally bool(tls_cert_file) with both keys defaulting None, so an unconfigured engine opens a cleartext socket. Value 6 because the minting primitive and its CLI verb already ship, giving an operator a real if awkward workaround; difficulty 6 because the change spans the serve path, five written DEV ONLY prohibitions that must be rewritten in the same PR, and the scheme-inference seam at tray/config.py:207-219 that the harness, apiclient and IDE share, which is in-repo client work rather than deployment migration. _(previously unscored.)_ > > **Filed 2026-08-16 - not started. THE ENGINE SERVES PLAIN HTTP WHEN NOBODY HAS SUPPLIED A CERTIFICATE, AND IT ALREADY OWNS EVERY PIECE NEEDED TO MINT ONE.** `[api].tls_cert_file` and `[api].tls_key_file` both ship `None` ([`config/settings.py:758-759`](../messagefoundry/config/settings.py)), `tls_enabled` is literally `bool(self.tls_cert_file)` (`settings.py:828-831`), and [`__main__.py:2840`](../messagefoundry/__main__.py) builds an SSL context **only** when that property is true. With no certificate configured, `uvicorn.run` at `__main__.py:2868` opens a cleartext socket. **THE CHANGE: when no certificate is configured, mint a self-signed one on first start, persist it, and serve HTTPS.** +> **PART A BUILT 2026-08-25 -- PR 575, ADR 0172. PROGRESS NOTE, NOT A CLOSURE: the item stays OPEN.** `ensure_api_tls_material` mints a self-signed pair beside the store on first run, beneath any operator-supplied `[api].tls_cert_file`, and returns `None` under `tls_terminated_upstream` so it cannot break a proxy hop that already terminates in front. **So the "not started" above is superseded for part A only.** STILL OPEN: nothing re-mints an EXPIRED generated pair -- `build_api_ssl_context` performs no expiry check, so a site past day 365 would serve an expired certificate; the rotation shape is undecided, and `CertExpiryRunner` alarms on this path meanwhile. Written by the LANDER to satisfy the required backlog-hygiene gate on PR 575, whose author deliberately did not touch this banner. **Status glyph untouched; correct this text freely.** > **OPERATOR-SUPPLIED CERTIFICATES KEEP PRECEDENCE, and the mechanism is already that way round.** The generated certificate is a **first-run fallback beneath** `[api].tls_cert_file`/`tls_key_file`, never a replacement: the fallback is reached only when `tls_enabled` is false, which is exactly the state in which no operator certificate exists. A site that sets those keys sees no behaviour change at all. > **THE MINTING PRIMITIVE IS BUILT AND ALREADY DRIVEN END TO END BY A CLI VERB -- this is wiring, not cryptography:** > messagefoundry/pki.py:136 make_self_signed(cn, sans, days) -> (cert_pem, key_pem) diff --git a/docs/adr/0172-the-engine-always-serves-tls-minting-a-self-signed-certificate-on-first-run.md b/docs/adr/0172-the-engine-always-serves-tls-minting-a-self-signed-certificate-on-first-run.md new file mode 100644 index 000000000..5c943ce45 --- /dev/null +++ b/docs/adr/0172-the-engine-always-serves-tls-minting-a-self-signed-certificate-on-first-run.md @@ -0,0 +1,99 @@ + + + +# ADR 0172 — The engine always serves TLS, minting a self-signed certificate on first run + +- **Status:** Accepted (2026-08-22) +- **Date:** 2026-08-22 +- **Supersedes:** [ADR 0143](0143-web-console-on-by-default-disableable-with-loopback-secure-context-browser-hardening.md)'s *decision*, not its analysis — see "What of 0143 survives" below +- **Related:** [ADR 0002](0002-phase2-transport-security-and-strong-auth.md) · [ADR 0065](0065-web-ops-dashboard.md) · [ADR 0118](0118-secure-by-default-security-configuration-section.md) · BACKLOG #1276 + +## Context + +`[api].tls_cert_file` and `tls_key_file` both shipped `None`, and `tls_enabled` was literally +`bool(self.tls_cert_file)`. An engine nobody had configured therefore opened a **cleartext +socket** — `uvicorn.run` with no `ssl_context_factory`. + +The minting primitive already shipped and was already driven end to end by a CLI verb: +`pki.make_self_signed`, and `_write_private_key` with its `O_EXCL` + `0o600` + Windows-DACL +sequence. Nothing needed inventing; the gap was wiring. + +**ADR 0143 considered exactly this change and declined it.** Its own words: *"A full fix — +terminate TLS on the loopback bind so `effective_https` is true and everything (headers + +secure cookie + HSTS) engages — is an **XL**: it means moving the whole API to https by default +and migrating every client (harness, `apiclient`, tray, IDE) in lockstep. Out of scope here."* +It shipped an http-safe hardening subset over the loopback secure-context **without** auto-TLS. + +That decline was reasonable on the information it had. **The sizing claim it rests on is +measurably false**, which is why this ADR supersedes the decision rather than merely amending it. + +## The measurement that overturns the sizing + +0143 sized the client migration as four clients moving in lockstep. Measured on `origin/main`: + +| Client | How it decides the scheme | +|---|---| +| tray | **Infers** — `service_toml_uses_tls`, exactly ONE caller (`tray/config.py`) | +| `apiclient` | **Does not.** Zero references to `tls_cert_file`; it is *given* a base URL and only validates the scheme | +| IDE | **Does not.** Its `tls_cert_file` hits are MLLP *connector* schema — the same name for a different setting | +| harness | **Does not infer — it assumes.** Hardcoded `http://127.0.0.1:8765` | + +So it is **one inference site plus a set of hardcoded defaults**, not a four-way lockstep +migration. Each default is a one-line flip. The XL that justified declining the full fix does +not exist. + +## Decision + +**The engine always serves TLS.** An operator-supplied `[api].tls_cert_file` always wins; with +none configured the engine mints a self-signed pair on first run, persists it, and serves HTTPS. + +1. **Unconditional, deliberately.** A *conditional* scheme is what let the tray, the harness and + the DAST target each decide it their own way. Clients cannot disagree about a scheme that has + no conditional — the divergence is removed rather than managed. +2. **Beneath the operator, never instead of.** The fallback is reached only when no certificate + is configured, so a site with its own chain sees no behaviour change at all. +3. **NOT in every topology.** `tls_terminated_upstream` (+ `trusted_proxies`) declares a reverse + proxy terminating TLS *in front* of the engine and speaking plaintext to it. Minting there + would break the proxy's own hop rather than harden anything. **"Always serves TLS" means the + engine never leaves a hop unprotected — not that it terminates TLS in every deployment.** +4. **The generated pair is a placeholder to be replaced.** Self-signed, so no chain of trust: + strictly better than cleartext, strictly worse than an operator chain. A browser shows a trust + interstitial until it is imported. +5. **Mint-once, then reuse.** `_write_private_key` refuses to overwrite, so a second start loads + rather than rotating. +6. **Re-minting an expired pair is AUDITED, never silent** (owner ruling, 2026-08-22). Nothing + re-mints today and `build_api_ssl_context` performs no expiry check, so an unrefreshed pair + would serve an expired certificate every client rejects. *Silent* is the defect in replacing a + key on disk, not *replaces*: an audited re-mint keeps this decision true without a human and + leaves a trail. Timing (at startup versus inside the expiry warn window) is a build detail — + both mutate disk identically, so the security question is settled for both. + +**Storage:** beside the store database. That directory is already the engine's own writable +state, already operator-controlled via `--db` / `[store].path`, and is **not** operator-authored +configuration. *Rejected:* a new `[api].tls_generated_dir` setting — a knob for a question with +one sensible answer. *Rejected outright:* the engine writing `tls_cert_file` into the operator's +service TOML. An engine that edits operator configuration is a surprising side effect, and it was +not needed once the scheme stopped being conditional. + +**Lifetime:** 365 days, inheriting the `cert self-signed` CLI default rather than inventing a +second lifetime for the same primitive. + +## What of ADR 0143 survives + +**Its analysis stands; only its decision is superseded.** 0143's diagnosis — that +`effective_https` gated two coupled concerns on one signal, and that a secure cookie over +cleartext http is dropped by Chrome and Safari and *breaks login* — is correct and is precisely +why this change is the better end state. Its `app.state.loopback` mechanism becomes vestigial +where the engine terminates TLS, because `effective_https` is now true on that bind. + +It is not vestigial everywhere: the `tls_terminated_upstream` topology in decision 3 still +reaches the engine over plaintext, and 0143's http-safe subset is what covers it. + +## Consequences + +- An operator reaching the console for the first time gets a **trust interstitial** until the + generated certificate is imported. `docs/TRAY.md` already documents that import. +- Every first-party client default becomes `https`. `service_toml_uses_tls` becomes vestigial. +- **No deployment axis** ([§0](../../CLAUDE.md)) — zero instances, so nothing is served in the + clear today and no upgrade breaks anyone. The change is cheap now and gets dearer with every + client that learns the scheme its own way. diff --git a/docs/adr/README.md b/docs/adr/README.md index 55421f6ce..79f5f154a 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -194,4 +194,5 @@ what is withheld and what you can request. | [0169](0169-username-identity-is-case-sensitive-and-must-not-depend-on-store-collation.md) | **Username identity is case-sensitive, and no identity decision may depend on store collation** (BACKLOG #1268) -- `users.username` was the one identifier column in the SQL Server schema with no `COLLATE` clause, so it inherited the DATABASE default (case-INsensitive on a stock install) while every sibling identifier column in the same file pinned `Latin1_General_100_BIN2` and both other backends were case-SENSITIVE -- `Admin` and `admin` two accounts on two backends and one on the third, under a `UNIQUE` constraint that reads as if it had settled the question. That portability defect became a SECURITY defect because a second site answered the same question by a different rule: `_login_local` gated WP-3 bootstrap expiry/supersession enforcement on a PYTHON `username == BOOTSTRAP_USERNAME` against the caller's input, while the lookup one line below was resolved by the COLUMN'S collation. The two disagree in exactly one direction -- `Admin` FAILS the Python guard so retirement never runs, then SUCCEEDS at the lookup and returns the very row the skipped call would have disabled. MEASURED on a lapsed unclaimed bootstrap with 6.4.1 disarmed so it could not mask the result: `login("admin")` refused and retired, `login("Admin")` returned ok=True with a session issued and `disabled` unset -- SDS-3.7 exactly, a compensating control resting on the false premise that the username the gate compared is the username the store matched. Decision, two rules: usernames ARE case-sensitive (the column now pins the collation its own file's convention already required), and **no identity decision may be delegated to store collation** -- the gate compares the value THE STORE RETURNED, never the caller's input, then re-reads by id since retirement may have disabled the row. The second rule is load-bearing and does NOT depend on the first: it stays correct under a collation the engine does not control (operator-supplied database, restored dump, column altered downstream), where limb 1 alone leaves the gate one `ALTER COLUMN` from being wrong again with nothing reporting it. Cost is one extra lookup ON THE BOOTSTRAP PATH ONLY, so the original guard's stated intent (normal logins free of extra lookups) is preserved rather than traded. Rejected: case-INsensitive normalisation (requires a canonicalisation that is not locale-neutral -- the Turkish dotless `i` -- so a wrong fold silently MERGES two accounts under a UNIQUE constraint that enforces rather than catches it; and it would have to hold across three backends plus the audit trail, every one a fresh place for the two rules to diverge again); column-only (makes the gate accidentally correct, contingent on a schema the engine stops controlling); gate-only (closes the security defect, leaves identity store-dependent). Existing SQL Server databases keep their original collation -- the DDL is creation-guarded and no re-type is attempted; zero deployments (CLAUDE.md section 0) so there is nothing to migrate, recorded so the schema-hash bump is not misread as a column alteration. Flagged undecided: two accounts differing only in case are themselves a confusability risk, closeable additively by a registration-time refusal without reopening this decision. Verification carries its own retraction -- the first version of both gate tests PASSED against unfixed code because they used the supersession arm, which `create_local_user` retires eagerly at `service.py:2685`, so the account was already disabled before the login ran; only the EXPIRY arm reaches the login path with retirement still pending | Proposed (2026-08-20) -- written in the conditional; **zero deployments**, so this is what a first deployment against a SQL Server store would hit, not a live exposure | | [0170](0170-constant-work-recovery-code-verification-pad-to-the-configured-slot-count-rather-than-short-circuit.md) | **Constant-work recovery-code verification: pad to the configured slot count rather than short-circuit** (BACKLOG #1167, ASVS 11.2.4) -- `_verify_second_factor` walked the argon2id recovery-code hashes and `return`ed on the first match, so the NUMBER of ~64 MiB verifications was a function of which code was presented. **Two leaks and only one matters:** the matched INDEX is worthless (the attacker holds the code and the response answers them anyway), but on the FAILURE path the cost is one verify per REMAINING code -- so anyone holding the password can time a wrong-code refusal and learn how many recovery codes an account has left, without authenticating to the second factor. **The item rated this difficulty 7 on a premise that does not survive measurement:** the re-score says a constant loop 'converts a timing leak into a memory and CPU amplification target', which is the right objection to raise -- and the failure path ALREADY verifies every remaining hash, so making the walk unconditional introduces no new cost, it makes today's WORST CASE the only case. Decision: always run exactly `mfa_recovery_code_count` verifies, padding with the same fixed `_DUMMY_PASSWORD_HASH` the local login leg uses, and select the winner AFTER the loop. Ceiling unmoved (default 10, validator-capped 50); `_argon2`'s semaphore means the concurrent-argon2 footprint cannot widen either; and the path sits behind primary authentication, so it is not an unauthenticated flood surface. **Claims constant WORK, not constant TIME** -- the store round trip on a match is not equalized, the TOTP branch returns earlier, and argon2's own constant-timeness is INHERITED from `argon2-cffi` and has never been measured in this tree, a gap #1167 names and this does not close. No timing measurement was run by the item or by this change. Rejected: leaving the short-circuit as accepted (the fix cost nothing against the existing ceiling, so 'accepted' would have been a judgement made before the amplification premise was checked); and a non-secret lookup index so only ONE verify ever runs -- strictly better on both axes, rejected as OUT OF SCOPE rather than wrong, needing a schema change across three backends and a migration, and recorded so it is not re-derived if the constant walk's cost ever bites | **Accepted (2026-08-22)** -- built with the change. Three parametrized tests pin the count for a first-slot match, a last-slot match and a non-match; proven red-first, removing the padding reds ALL THREE and the file restores byte-identical by SHA-256. Severity conditional per CLAUDE.md section 0 -- **zero deployments**, so this is what a first deployment would have inherited | | [0171](0171-offline-administrator-unlock-a-host-gated-cli-recovery-path-for-a-sole-administrator-lockout.md) | **Offline administrator unlock: a host-gated CLI recovery path for a sole-administrator lockout** (BACKLOG #1236) -- a deployment with ONE administrator had no recovery from account lockout, and every exit is individually deliberate: the bootstrap account is literally `admin`, it is created with no email so the ACCOUNT_LOCKED notice never leaves the process, self-reset is refused, an admin reset needs ANOTHER admin, re-bootstrap fires only on an EMPTY users table, and none of 38 CLI subcommands managed users. **The defect is that they close SIMULTANEOUSLY for that deployment** and nothing notices the conjunction. **The filed acceptance criterion could not discriminate and was amended 2026-08-21:** "recover without hand-editing the database and without a second admin" PASSES ON THE SHIPPED SYSTEM BY WAITING, since the lock self-expires after `lockout_minutes`; a test both a fixed and a broken system pass is not a test. Decision: `messagefoundry admin-unlock --username `. **The gate is HOST ACCESS and it is a real gate rather than an absent one** -- reaching it needs the config, the store path and on an encrypted store the key material, so anyone holding all three already has the database and does not need an unlock to reach an account; that is why it ships unauthenticated, and it is the load-bearing claim. **Clears the lockout and does NOT reset the password** -- deliberately narrower, since a reset would hand the runner a working account. **Reuses `record_login_failure(failed_attempts=0, locked_until=None)` rather than adding a protocol method**, decided by a MEASURED cross-lane fact rather than taste: a named `clear_lockout` would touch base/store/postgres/sqlserver, and all four were uncommitted in a peer lane at the time, so reuse avoided a four-file collision. Exit codes follow the `--json` convention (`_emit_error`, 1) not the M-31 lineage (stderr, 2), verified against `audit-verify` which has no `--json` flag. Carries M-31 forward: a typo'd `--db` is refused rather than creating an empty SQLite store and reporting a false "no such account" | **Accepted (2026-08-22)** -- built with the change. Four tests; **exactly ONE is the control** and the other three are deliberately insensitive -- neutering the clearing call reds only the acceptance test, and the audit-row test still passes under that plant, so it evidences the flow RAN and never that it WORKED. Does NOT address #1236's repetition limb: lock cycles remain unbounded and an attacker can re-lock. Severity conditional per CLAUDE.md section 0 -- **zero deployments** | +| [0172](0172-the-engine-always-serves-tls-minting-a-self-signed-certificate-on-first-run.md) | **The engine always serves TLS, minting a self-signed certificate on first run** (BACKLOG #1276) -- `[api].tls_cert_file` and `tls_key_file` both shipped `None` and `tls_enabled` was literally `bool(self.tls_cert_file)`, so an unconfigured engine opened a **cleartext socket**. The minting primitive already shipped and was already driven by a CLI verb, so the gap was wiring rather than cryptography. **SUPERSEDES [ADR 0143](0143-web-console-on-by-default-disableable-with-loopback-secure-context-browser-hardening.md)'s DECISION, not its analysis:** 0143 explicitly CONSIDERED terminating TLS on the loopback bind and DECLINED it as an XL requiring 'migrating every client (harness, `apiclient`, tray, IDE) in lockstep'. **That sizing is measurably false and the measurement is why this supersedes rather than amends:** only the TRAY infers the scheme (`service_toml_uses_tls`, ONE caller); `apiclient` has ZERO references to `tls_cert_file` and is GIVEN a URL it only validates; the IDE's hits are MLLP CONNECTOR schema, the same name for a different setting; and the harness does not infer at all, it hardcodes `http://127.0.0.1:8765`. One inference site plus one-line default flips, not a four-way lockstep. Decision: an operator certificate always wins and this is a fallback BENEATH it; unconditional deliberately, because a CONDITIONAL scheme is what let three clients each decide it their own way and clients cannot disagree about a scheme with no conditional; **NOT in every topology** -- `tls_terminated_upstream` declares a proxy terminating TLS IN FRONT and speaking plaintext, so minting there breaks the proxy's own hop, and 'always serves TLS' means the engine never leaves a hop unprotected rather than that it terminates everywhere; the pair is a PLACEHOLDER to be replaced, self-signed and chainless; mint-once, since `_write_private_key` refuses to overwrite; and **re-minting an expired pair is AUDITED, never silent** (owner ruling) -- *silent* is the defect in replacing a key on disk, not *replaces*. Storage beside the store database: already the engine's own writable state, already operator-controlled via `--db`, and NOT operator-authored config. Rejected: an `[api].tls_generated_dir` knob for a question with one sensible answer; and **rejected outright**, the engine writing `tls_cert_file` into the operator's service TOML, which stopped being necessary once the scheme stopped being conditional. Lifetime 365 days, inheriting the CLI default rather than inventing a second one. **0143's analysis stands and is why this is better** -- its diagnosis that a secure cookie over cleartext http is dropped by Chrome and Safari and BREAKS LOGIN is correct; its `app.state.loopback` mechanism becomes vestigial where the engine terminates TLS, but NOT in the upstream-proxy topology, which still reaches the engine over plaintext | **Accepted (2026-08-22)** -- owner-ruled twice on this item, both forks found while SCOPING and handed back before code. Part A built; the existing suite caught the upstream-proxy break and the CODE was fixed rather than the test. Severity conditional per CLAUDE.md section 0 -- **zero deployments**, so nothing is served in the clear today | | [0173](0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md) | **TLS peer revocation checking and OCSP stapling across terminating and originating surfaces** (BACKLOG #1005, ASVS 12.1.4) -- the requirement reaches in two directions and the engine answers neither: where the product TERMINATES TLS it does not staple its own certificate's status, and where it ORIGINATES it does not check the peer's revocation. Direction 1 is RUNTIME-BLOCKED rather than unbuilt -- CPython 3.14.6 exposes no stapling surface at all, measured against live positive controls, so no amount of engineering here reaches it. The opt-in client-certificate CRL checking that DOES ship (`config/tls_policy.py:215-276`, three PROTOCOL_TLS_SERVER call sites) is a THIRD combination -- peer revocation on the terminating side -- and moves neither graded direction; that is the single easiest thing in this area to misread. DECISION: accept and document both directions, with one build rider the accept reasoning does not cover -- three originating hops that never reach the existing revocation guard, filed by subject and deliberately unallocated. | **Proposed (2026-08-23)** -- no code changed. Severity is conditional per CLAUDE.md section 0: on a first deployment a revoked partner certificate would keep verifying on the unguarded hops; there are zero deployments today. Five citation errors from the adversarial pass were repaired before filing. | diff --git a/harness/load/connscale/batchbox.py b/harness/load/connscale/batchbox.py index 5f27cc411..fc120daa1 100644 --- a/harness/load/connscale/batchbox.py +++ b/harness/load/connscale/batchbox.py @@ -197,7 +197,7 @@ def build_remote_argv( "harness", "connscale-remote", "--engine-url", - f"http://{engine_host}:{api_port}", + f"https://{engine_host}:{api_port}", "--engine-host", engine_host, "--inbound-base", diff --git a/harness/load/connscale/runner.py b/harness/load/connscale/runner.py index d4ca4fb78..94a230aad 100644 --- a/harness/load/connscale/runner.py +++ b/harness/load/connscale/runner.py @@ -272,7 +272,7 @@ async def run_connscale( ) return ConnScaleReport( profile=profile.name, - engine_url=f"http://{sink_host}:{api_port}", + engine_url=f"https://{sink_host}:{api_port}", db_backend=db_backend, shim_installed=shim_installed, records=records, @@ -667,8 +667,13 @@ def _is_rcsi_gate(detail: str) -> bool: async def _await_node_healthy(node: EngineNode, *, timeout: float) -> None: import httpx + from harness.load.tlsmat import harness_ssl_context + start = time.perf_counter() - async with httpx.AsyncClient(timeout=4.0) as client: + # Pin to the run's own certificate (harness.load.tlsmat): the harness minted it and handed it to + # the node as operator-supplied [api] material, so it is on disk before the process starts. That + # is what makes pinning cheaper than skipping verification here -- there is no file to wait for. + async with httpx.AsyncClient(timeout=4.0, verify=harness_ssl_context()) as client: while time.perf_counter() - start < timeout: if not node.alive: raise ConnScaleError(f"engine exited during startup:\n{node.log_tail()}") diff --git a/harness/load/enginepoll.py b/harness/load/enginepoll.py index d2d257a34..6e27e4fc0 100644 --- a/harness/load/enginepoll.py +++ b/harness/load/enginepoll.py @@ -26,10 +26,15 @@ from collections.abc import Iterable, Mapping, Sequence from dataclasses import dataclass from typing import Any, TypeVar +from urllib.parse import urlsplit from harness.load.metrics import Counters from messagefoundry.apiclient import ApiError, EngineClient +#: Hosts whose engine this harness could have spawned itself. Mirrors apiclient's own loopback set; +#: kept local rather than importing a private name from that package. +_LOOPBACK_HOSTS = frozenset({"127.0.0.1", "localhost", "::1", "[::1]"}) + _T = TypeVar("_T") @@ -583,10 +588,35 @@ async def await_drain(self, *, timeout: float, interval: float) -> float | None: # --- sync helpers (run in the executor) ---------------------------------- + @staticmethod + def _cacert_for(url: str) -> str | None: + """The PEM that verifies ``url``, or None to leave the client's default trust in place. + + Every engine this harness SPAWNS is served with the run's own certificate + (:mod:`harness.load.tlsmat`), which is handed to the node as operator-supplied ``[api]`` + material -- so a loopback ``https`` URL always pins to that one anchor. Resolving it here + rather than threading a parameter is what keeps the eleven EnginePoller call sites unchanged: + minting once per run means there is only ever ONE anchor to resolve. + + A NON-loopback ``https`` URL is an engine on another box (shardcert's two-box rig) whose + certificate this process has never seen. Returning None there is deliberate: it leaves the + remote posture exactly as it was rather than pinning it to a cert that cannot match. + """ + parts = urlsplit(url) + if parts.scheme != "https": + return None + if (parts.hostname or "").lower() not in _LOOPBACK_HOSTS: + return None + from harness.load.tlsmat import harness_tls_material + + return harness_tls_material()[0] + def _open_sync(self) -> None: clients: list[EngineClient] = [] for url in self._urls: - client = EngineClient(url, allow_insecure=self._allow_insecure) + client = EngineClient( + url, allow_insecure=self._allow_insecure, cacert=self._cacert_for(url) + ) if self._token: client.set_token(self._token) # does a /me request to validate clients.append(client) diff --git a/harness/load/estate/runner.py b/harness/load/estate/runner.py index d32f83dc9..683a38a48 100644 --- a/harness/load/estate/runner.py +++ b/harness/load/estate/runner.py @@ -107,7 +107,7 @@ async def run_estate( result_ok = all(c.ok for c in slos) return EstateReport( profile=profile.name, - engine_url=f"http://{sink_host}:{engine_api_port}", + engine_url=f"https://{sink_host}:{engine_api_port}", db_backend=profile.store_backend, records=records, slos=slos, @@ -292,8 +292,13 @@ def _startup_failure_detail(exc: BaseException, node: EngineNode) -> str: async def _await_node_healthy(node: EngineNode, *, timeout: float) -> None: import httpx + from harness.load.tlsmat import harness_ssl_context + start = time.perf_counter() - async with httpx.AsyncClient(timeout=4.0) as client: + # Pin to the run's own certificate (harness.load.tlsmat): the harness minted it and handed it to + # the node as operator-supplied [api] material, so it is on disk before the process starts. That + # is what makes pinning cheaper than skipping verification here -- there is no file to wait for. + async with httpx.AsyncClient(timeout=4.0, verify=harness_ssl_context()) as client: while time.perf_counter() - start < timeout: if not node.alive: raise EstateError(f"engine exited during startup:\n{node.log_tail()}") diff --git a/harness/load/failover.py b/harness/load/failover.py index 684ea594d..e456edc04 100644 --- a/harness/load/failover.py +++ b/harness/load/failover.py @@ -58,6 +58,7 @@ from harness.load.report import EXIT_OK, EXIT_SLO_VIOLATION, SloCheck from harness.load.sender import ConnectionPool, Dispatcher from harness.load.sink import CorrelationSink +from harness.load.tlsmat import harness_ssl_context, harness_tls_material _STOP_GRACE = 5.0 _SETTLE = 0.5 # let the final ACKs/arrivals settle before the truly-final engine sample @@ -118,8 +119,20 @@ def __init__( ) -> None: self.node_id = node_id self.api_port = api_port - self.url = f"http://127.0.0.1:{api_port}" + # The engine always serves TLS (BACKLOG #1276 part A). This harness SUPPLIES the certificate + # (below) rather than letting the node mint its own placeholder, so the anchor is known before + # the process starts -- see harness.load.tlsmat for why that ordering matters. + self.url = f"https://127.0.0.1:{api_port}" self._env = dict(env) + # Hand this node the run's certificate as operator-supplied [api] material. + # ensure_api_tls_material returns tls_cert_file on its FIRST branch and never mints over it, + # so the generated-placeholder path is unreachable for a harness node. setdefault, so a + # scenario that wants to exercise the minting path itself can still override. + _cert, _key = harness_tls_material() + self._env.setdefault("MEFOR_API_TLS_CERT_FILE", _cert) + self._env.setdefault("MEFOR_API_TLS_KEY_FILE", _key) + #: The PEM a client must pin to reach THIS node (EngineClient(cacert=...)). + self.cacert = self._env["MEFOR_API_TLS_CERT_FILE"] # GIVEN 1 (ADR 0148): the default env `dev` now derives PHI, so a bare `serve --env dev` runs the # secure PHI posture (keyless/egress/retention/notify refusals). This harness node serves the # SYNTHETIC load graph (no real PHI), so declare the loud opt-out — matching the `--env dev` @@ -480,7 +493,12 @@ async def run_failover_load( for tag, api in (("a", ports.api_a), ("b", ports.api_b)) ] - async with httpx.AsyncClient(timeout=4.0) as client: + # Pin to the run's own certificate. The harness minted it, so verifying costs nothing and keeps + # these ad-hoc probes on the same posture as EngineClient, which offers pinning and no way to + # switch verification off. (Chasing the cert the ENGINE mints was the alternative and was + # declined: it does not exist until the engine writes it, so every client would wait on a file + # whose timeout presents identically to the bug this fixes.) + async with httpx.AsyncClient(timeout=4.0, verify=harness_ssl_context()) as client: try: await sink.start() for node in nodes: diff --git a/harness/load/multishard.py b/harness/load/multishard.py index 71ae0fed1..98d9b1951 100644 --- a/harness/load/multishard.py +++ b/harness/load/multishard.py @@ -642,7 +642,7 @@ def _inbound_rows_per_node(nodes: list[EngineNode]) -> list[int]: counts: list[int] = [] for node in nodes: try: - client = EngineClient(node.url) + client = EngineClient(node.url, cacert=node.cacert) try: rows = client.connections() finally: @@ -683,7 +683,7 @@ def _attribute_engines_sync( foreign_rows = 0 reads = 0 try: - client = EngineClient(node.url) + client = EngineClient(node.url, cacert=node.cacert) try: rows = client.connections() finally: diff --git a/harness/load/shardcert.py b/harness/load/shardcert.py index 6f6c74809..4ae7889df 100644 --- a/harness/load/shardcert.py +++ b/harness/load/shardcert.py @@ -68,6 +68,7 @@ from harness.load.profile import TypeMix, load_profile_text from harness.load.sender import PersistentConnection from harness.load.sink import CorrelationSink +from harness.load.tlsmat import harness_ssl_context from messagefoundry.config.wiring import load_config from messagefoundry.pipeline.sharding import ( owned_destination_set, @@ -1115,7 +1116,9 @@ def _free_contiguous(n: int, start: int = 3600, tries: int = 60) -> int: async def _await_health(url: str, *, timeout: float) -> bool: deadline = time.monotonic() + timeout - async with httpx.AsyncClient(timeout=2.0) as client: + # url is always a locally-spawned ShardCertNode/EngineNode address (verified: every call site + # passes node.url or restart.url), so the run's own pinned certificate covers it. + async with httpx.AsyncClient(timeout=2.0, verify=harness_ssl_context()) as client: while time.monotonic() < deadline: with contextlib.suppress(Exception): r = await client.get(f"{url}/health") @@ -3380,6 +3383,16 @@ async def run_shardcert_driver( # Drain against the engines' REMOTE /stats — the authoritative drain signal, polled off-box. # allow_insecure: the remote engine API is plaintext http, so the poller needs it (loopback # never does) — else poller.open() fail-closes on the non-loopback http URL. + # STILL http, KNOWINGLY. This is the TWO-BOX path: the engine runs on another host that + # this process never spawned, so the run's own anchor (harness.load.tlsmat) cannot cover it + # and EnginePoller._cacert_for deliberately declines to pin a non-loopback URL. + # + # #1276 makes that remote engine serve TLS too, so this rig needs its own answer -- either + # the two boxes share one certificate (export MEFOR_HARNESS_TLS_CERT_FILE/KEY_FILE on both, + # which the tlsmat inheritance already honours) or the rig ships the engine box's cert to the + # load box. That is a rig-provisioning decision, not a code one, and it is NOT exercised by + # any CI workflow -- so it is left visible here rather than half-converted to https, which + # would fail verification against a cert this box has never seen. urls = [f"http://{engine_host}:{p}" for p in api_ports] poller = EnginePoller(urls, None, origin=time.perf_counter(), allow_insecure=allow_insecure) await poller.open() @@ -4450,6 +4463,16 @@ async def run_shardcert_drive( driver_dones = await _await_indexed( coord, DRIVER_DONE, driver_count, timeout=driver_done_wait ) + # STILL http, KNOWINGLY. This is the TWO-BOX path: the engine runs on another host that + # this process never spawned, so the run's own anchor (harness.load.tlsmat) cannot cover it + # and EnginePoller._cacert_for deliberately declines to pin a non-loopback URL. + # + # #1276 makes that remote engine serve TLS too, so this rig needs its own answer -- either + # the two boxes share one certificate (export MEFOR_HARNESS_TLS_CERT_FILE/KEY_FILE on both, + # which the tlsmat inheritance already honours) or the rig ships the engine box's cert to the + # load box. That is a rig-provisioning decision, not a code one, and it is NOT exercised by + # any CI workflow -- so it is left visible here rather than half-converted to https, which + # would fail verification against a cert this box has never seen. urls = [f"http://{engine_host}:{p}" for p in api_ports] # allow_insecure threads the plaintext-http-to-remote posture: the engine box's API is http and # off-box, so without it EngineClient fail-closes and poller.open() raises AFTER the children are diff --git a/harness/load/tlsmat.py b/harness/load/tlsmat.py new file mode 100644 index 000000000..0127c64b1 --- /dev/null +++ b/harness/load/tlsmat.py @@ -0,0 +1,96 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""One TLS trust anchor for a whole harness run. + +The engine always serves TLS (BACKLOG #1276 part A, owner ruling 2026-08-22) and mints a +self-signed placeholder when no operator certificate is configured. That left every harness driver +talking cleartext to a TLS socket. + +**The harness supplies the certificate rather than chasing the one the engine mints.** +:func:`ensure_api_tls_material` returns operator-supplied material on its FIRST branch and never +mints over it, so handing each node ``[api].tls_cert_file`` makes the generated path unreachable for +harness engines. Two properties follow, and both are the reason this module exists: + +* **No race.** The pair is on disk before any engine is spawned, so no client ever waits for a file + to appear. Waiting was the alternative, and its timeout would surface as "nodes did not become + healthy" -- indistinguishable from the very bug this fixes. +* **One anchor, not N.** Minting per node would mean threading a different CA through every + multi-node ``EnginePoller`` URL list. Minting once collapses that to a single constant. + +Non-prod only, and it never leaves this box: the pair lands in a per-process temp directory and +covers loopback names alone. +""" + +from __future__ import annotations + +import os +import ssl +import tempfile +import threading +from pathlib import Path + +__all__ = ["harness_ssl_context", "harness_tls_material"] + +# Loopback only, on purpose. Every engine this harness SPAWNS binds 127.0.0.1; an engine on another +# box (shardcert's two-box rig) mints its own cert that this process has never seen, so it is out of +# this module's scope rather than quietly covered by a SAN that would not match anyway. +_CN = "127.0.0.1" +_SANS = ["127.0.0.1", "localhost", "::1"] + +#: Published into the environment after minting so a CHILD harness process (connscale-remote, spawned +#: by batchbox) inherits the SAME anchor. Without this the child would mint its own pair and fail to +#: verify engines the PARENT started -- a cross-process bug that no single-process test would show. +_ENV_CERT = "MEFOR_HARNESS_TLS_CERT_FILE" +_ENV_KEY = "MEFOR_HARNESS_TLS_KEY_FILE" + +_LOCK = threading.Lock() +_MATERIAL: tuple[str, str] | None = None +_CONTEXT: ssl.SSLContext | None = None + + +def harness_tls_material() -> tuple[str, str]: + """``(cert_path, key_path)`` for this process, minted once on first call. + + Feed these to a node as ``MEFOR_API_TLS_CERT_FILE`` / ``MEFOR_API_TLS_KEY_FILE``. + """ + global _MATERIAL + with _LOCK: + if _MATERIAL is None: + inherited = os.environ.get(_ENV_CERT), os.environ.get(_ENV_KEY) + if all(inherited) and Path(inherited[0] or "").exists(): + # A parent harness process already minted for this run; reuse its anchor verbatim. + _MATERIAL = (inherited[0] or "", inherited[1] or "") + return _MATERIAL + # Local import: pki pulls cryptography, which the harness should not require merely to + # be imported (the load package is imported by report-only paths too). + from messagefoundry import pki + + state = Path(tempfile.mkdtemp(prefix="mefor-harness-tls-")) + cert_pem, key_pem = pki.make_self_signed(_CN, _SANS, 365) + cert_path = state / "harness-api-cert.pem" + key_path = state / "harness-api-key.pem" + cert_path.write_bytes(cert_pem) + key_path.write_bytes(key_pem) + # Best-effort on Windows, where mode bits are advisory; the engine's own + # _write_private_key applies the real DACL to material IT writes, and this pair is a + # throwaway in a temp dir either way. + key_path.chmod(0o600) + _MATERIAL = (str(cert_path), str(key_path)) + os.environ.setdefault(_ENV_CERT, _MATERIAL[0]) + os.environ.setdefault(_ENV_KEY, _MATERIAL[1]) + return _MATERIAL + + +def harness_ssl_context() -> ssl.SSLContext: + """A client context whose ONLY trust anchor is :func:`harness_tls_material`'s certificate. + + Pinning rather than disabling verification: the harness minted this certificate itself, so + verifying against it costs nothing and keeps the ad-hoc ``httpx`` probes and the shared + ``EngineClient`` (which offers pinning and no way to switch verification off) on one posture. + """ + global _CONTEXT + cert_path, _ = harness_tls_material() + with _LOCK: + if _CONTEXT is None: + _CONTEXT = ssl.create_default_context(cafile=cert_path) + return _CONTEXT diff --git a/messagefoundry/__main__.py b/messagefoundry/__main__.py index da5062d17..a4a88b4a7 100644 --- a/messagefoundry/__main__.py +++ b/messagefoundry/__main__.py @@ -2856,15 +2856,29 @@ def registry_filter(reg: Registry) -> Registry: # noqa: F811 (local shard-bound # server implementation/version to an unauthenticated caller. "server_header": False, } - if settings.api.tls_enabled: + # BACKLOG #1276: THE ENGINE ALWAYS SERVES TLS. Owner ruling 2026-08-22 (option 3), which + # SUPERSEDES ADR 0143's premise that the console is hardened "over a cleartext loopback + # secure-context WITHOUT auto-TLS". An operator certificate always wins; with none configured + # the engine mints a self-signed placeholder rather than opening a cleartext socket. + # + # Unconditional on purpose: a CONDITIONAL scheme is what let the tray, the harness and the + # DAST target each decide it their own way, which is the defect this item exists to remove. + from pathlib import Path as _Path + + from messagefoundry.api.tls import build_api_ssl_context, ensure_api_tls_material + + _material = ensure_api_tls_material( + settings.api, state_dir=_Path(settings.store.path).resolve().parent + ) + if _material is not None: + _cert, _key = _material + _api_tls = settings.api.model_copy(update={"tls_cert_file": _cert, "tls_key_file": _key}) # WP-13a: terminate TLS in-process. Build the context now so a bad cert/key/passphrase fails # fast (before uvicorn opens the socket); pass it via uvicorn's ssl_context_factory so the # tls_min_version floor is enforced exactly. - from messagefoundry.api.tls import build_api_ssl_context - # #285: build_api_ssl_context preflights [api].tls_client_ca_file (pin + owner-only DACL) at # construction; enforcing is the [security].enforcement refuse/warn dial. - ctx = build_api_ssl_context(settings.api, enforcing=enforcing) + ctx = build_api_ssl_context(_api_tls, enforcing=enforcing) run_kwargs["ssl_context_factory"] = lambda config, default_factory: ctx # ADR 0083 activation: only when in-process mTLS (client CA) AND a cert-identity map are BOTH # configured, swap in the scope-populating HTTP protocol so a verified peer cert reaches diff --git a/messagefoundry/api/tls.py b/messagefoundry/api/tls.py index fa4548ef8..a0ff8a47f 100644 --- a/messagefoundry/api/tls.py +++ b/messagefoundry/api/tls.py @@ -10,7 +10,9 @@ from __future__ import annotations +import logging import ssl +from pathlib import Path from messagefoundry.auth.trust_anchors import api_client_anchor_spec, enforce_anchor from messagefoundry.config.settings import ApiSettings @@ -21,7 +23,9 @@ harden_verify_flags, ) -__all__ = ["build_api_ssl_context"] +__all__ = ["build_api_ssl_context", "ensure_api_tls_material"] + +log = logging.getLogger(__name__) # Map the validated tls_min_version floor to the SSLContext minimum (TLS < 1.2 is never allowed). _MIN_VERSION = {"1.2": ssl.TLSVersion.TLSv1_2, "1.3": ssl.TLSVersion.TLSv1_3} @@ -68,3 +72,72 @@ def build_api_ssl_context(api: ApiSettings, *, enforcing: bool = True) -> ssl.SS if api.tls_client_crl_file: harden_crl_check(ctx, api.tls_client_crl_file) return ctx + + +#: Filenames for the first-run generated pair, written beside the store database (BACKLOG #1276). +#: **Why there, and the alternative rejected.** That directory is already the engine's own writable +#: state (the database and its WAL live there), it is already operator-controlled via ``--db`` / +#: ``[store].path``, and it is NOT operator-authored configuration -- which is what keeps the engine +#: out of the business of editing an operator's TOML. The rejected alternative was a new +#: ``[api].tls_generated_dir`` setting: a knob for a question with one sensible answer. +_GENERATED_CERT_NAME = "api-generated-cert.pem" +_GENERATED_KEY_NAME = "api-generated-key.pem" + + +def ensure_api_tls_material(api: ApiSettings, *, state_dir: Path) -> tuple[str, str] | None: + """Return the ``(cert_path, key_path)`` the API should serve with, minting on first run. + + **The engine always serves TLS (owner ruling 2026-08-22, superseding ADR 0143's cleartext + loopback premise).** An operator-supplied ``[api].tls_cert_file`` always wins -- this is a + fallback BENEATH it, never a replacement -- so a site that configures its own chain sees no + behaviour change and this function is not even consulted. + + **Returns ``None`` when a reverse proxy terminates TLS upstream** -- see the guard below. + + **Mint-once, then reuse.** The pair is written with :func:`_write_private_key`'s ``O_EXCL`` + + ``0o600`` + Windows-DACL sequence, which REFUSES to overwrite. So a second start finds the + files and loads them; it does not re-mint, and it cannot clobber a key. + + **The generated certificate is a PLACEHOLDER TO BE REPLACED, not an endorsed production + terminator.** It is self-signed, so it carries no chain of trust: strictly better than + cleartext, strictly worse than an operator-supplied chain. A browser reaching the console gets + a trust interstitial until it is imported (``docs/TRAY.md`` documents that import). + + **NOT HANDLED HERE, and it is filed rather than forgotten:** nothing re-mints an EXPIRED + generated pair. ``build_api_ssl_context`` performs no expiry check, so on day 366 the engine + would serve an expired certificate every client rejects. The rotation shape is an open decision + on #1276; until it lands, ``CertExpiryRunner`` alarms on this path like any other served cert. + """ + if api.tls_cert_file: # operator-supplied material always wins + return api.tls_cert_file, api.tls_key_file or "" + + # A DECLARED UPSTREAM TERMINATOR IS NOT AN UNPROTECTED HOP, AND MINTING HERE WOULD BREAK IT. + # `tls_terminated_upstream` (+ trusted_proxies) says a reverse proxy terminates TLS in FRONT of + # the engine and speaks plaintext to it. Serving HTTPS underneath that proxy does not harden the + # deployment -- it breaks the proxy's own hop. "Always serves TLS" means the engine never leaves + # a hop unprotected, NOT that it terminates TLS in every topology. + if api.tls_terminated_upstream: + return None + + cert_path = state_dir / _GENERATED_CERT_NAME + key_path = state_dir / _GENERATED_KEY_NAME + if cert_path.exists() and key_path.exists(): + return str(cert_path), str(key_path) + + from messagefoundry import pki + from messagefoundry.__main__ import _write_private_key + + state_dir.mkdir(parents=True, exist_ok=True) + # 365 days, inheriting the `cert self-signed` CLI default rather than inventing a second + # lifetime for the same primitive. + cert_pem, key_pem = pki.make_self_signed(api.host, [], 365) + _write_private_key(key_path, key_pem) + cert_path.write_bytes(cert_pem) + log.warning( + "no [api].tls_cert_file configured — minted a SELF-SIGNED certificate for %s at %s. It has " + "no chain of trust and is a PLACEHOLDER: browsers will show a trust interstitial until it " + "is imported, and it should be replaced with an operator-supplied chain.", + api.host, + cert_path, + ) + return str(cert_path), str(key_path) diff --git a/scripts/security/crypto_inventory_check.py b/scripts/security/crypto_inventory_check.py index 5a7b530f7..e55c89f53 100644 --- a/scripts/security/crypto_inventory_check.py +++ b/scripts/security/crypto_inventory_check.py @@ -182,6 +182,12 @@ # (ssl.create_default_context), plus opt-in client-cert mTLS (load_cert_chain). Builds the # client-side TLS verification context. "messagefoundry/apiclient/client.py": frozenset({"ssl", "truststore"}), + # BACKLOG #1276 part A: the engine always serves TLS now and mints a self-signed placeholder when + # no operator cert is configured. This harness supplies its own certificate instead — one pair + # minted per run (ssl.create_default_context(cafile=...) pins to it) rather than chasing the one + # the engine mints, so no client ever races a file that doesn't exist until the engine writes it. + # Non-prod only: the pair lives in a per-process temp dir and covers loopback names alone. + "harness/load/tlsmat.py": frozenset({"ssl"}), # ADR 0041 (D3): SHA-256 hashes of the loaded first-party modules vs the wheel dist-info/RECORD at # startup self-attestation — drift detection (integrity/tamper-evidence, not a secret); the engine # alerts by default and (opt-in) fails closed on drift. diff --git a/tests/test_api_tls.py b/tests/test_api_tls.py index f013e424f..7a92ba655 100644 --- a/tests/test_api_tls.py +++ b/tests/test_api_tls.py @@ -28,7 +28,7 @@ require_service_cert, resolve_client_cert_identity, ) -from messagefoundry.api.tls import build_api_ssl_context +from messagefoundry.api.tls import build_api_ssl_context, ensure_api_tls_material from messagefoundry.api.tls_client_cert import ( MF_CLIENT_PEERCERT_STATE_KEY, client_cert_http_protocol_class, @@ -222,7 +222,7 @@ def test_serve_mtls_without_cert_map_keeps_stock_protocol( assert "http" not in captured # stock protocol — the shim is never wired without a map -def test_serve_loopback_without_tls_passes_no_ssl_factory( +def test_serve_loopback_without_a_certificate_now_mints_and_serves_tls( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: from messagefoundry.store.crypto import generate_key @@ -236,7 +236,16 @@ def test_serve_loopback_without_tls_passes_no_ssl_factory( encoding="utf-8", ) assert main(["serve", "--config", str(SAMPLES_CONFIG), "--env", "dev"]) == 0 - assert "ssl_context_factory" not in captured # plaintext loopback: no TLS wiring + # BACKLOG #1276, owner ruling 2026-08-22 (option 3): THIS ASSERTION IS DELIBERATELY INVERTED. + # It previously read `"ssl_context_factory" not in captured # plaintext loopback: no TLS + # wiring`, which pinned the very behaviour the ruling removes -- an unconfigured engine opening + # a cleartext socket. The engine now mints a self-signed placeholder and serves HTTPS. + # + # This is also what SUPERSEDES ADR 0143, whose browser hardening was premised on a "cleartext + # loopback secure-context WITHOUT auto-TLS". Loopback is exactly the case it named. + assert "ssl_context_factory" in captured + assert (tmp_path / "api-generated-cert.pem").exists() + assert (tmp_path / "api-generated-key.pem").exists() # --- WP-15: reverse-proxy / upstream TLS termination ------------------------- @@ -1466,3 +1475,66 @@ def test_a_non_phi_instance_is_not_refused( _posture_probe_toml(tmp_path, public_origin=None, serve_ui=False, synthetic=True) rc = _run_posture_b(tmp_path, monkeypatch, env="prod") assert rc != 2 or "public_origin" not in capsys.readouterr().err + + +# --- BACKLOG #1276: the engine always serves TLS ----------------------------------------------- +# +# Owner ruling 2026-08-22 (option 3), superseding ADR 0143's cleartext-loopback premise. An +# operator certificate always wins; with none configured the engine mints a self-signed +# PLACEHOLDER rather than opening a cleartext socket. + + +def test_an_operator_certificate_always_wins_and_nothing_is_minted(tmp_path: Path) -> None: + # POSITIVE CONTROL for every test below: the generated pair is a fallback BENEATH + # [api].tls_cert_file, never a replacement. A site that configures its own chain must see no + # behaviour change at all -- and must not find engine-minted files appearing in its state dir. + api = ApiSettings(tls_cert_file="operator-cert.pem", tls_key_file="operator-key.pem") + cert, key = ensure_api_tls_material(api, state_dir=tmp_path) + assert (cert, key) == ("operator-cert.pem", "operator-key.pem") + assert list(tmp_path.iterdir()) == [] # nothing minted + + +def test_a_first_run_mints_a_usable_self_signed_pair(tmp_path: Path) -> None: + api = ApiSettings() + assert not api.tls_enabled # the state this fallback exists for + cert, key = ensure_api_tls_material(api, state_dir=tmp_path) + assert Path(cert).exists() and Path(key).exists() + # It must be loadable as a real chain, not merely present: a file that exists but cannot be + # loaded would fail at uvicorn's socket rather than here. + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + ctx.load_cert_chain(cert, key) + + +def test_the_minted_certificate_names_the_bind_host(tmp_path: Path) -> None: + # The CN/SAN must match what a client dials, or every verifying first-party client fails the + # hostname check against the very certificate the engine just minted for them. + api = ApiSettings() + cert, _ = ensure_api_tls_material(api, state_dir=tmp_path) + loaded = x509.load_pem_x509_certificate(Path(cert).read_bytes()) + common_names = [a.value for a in loaded.subject.get_attributes_for_oid(NameOID.COMMON_NAME)] + assert api.host in common_names + + +def test_a_second_start_reuses_the_pair_and_never_re_mints(tmp_path: Path) -> None: + # MINT-ONCE. _write_private_key uses O_EXCL and REFUSES to overwrite, so a re-mint attempt + # would not silently rotate the key -- it would raise. Asserting the bytes are unchanged proves + # the reuse branch is taken rather than the write being attempted and swallowed. + api = ApiSettings() + first_cert, first_key = ensure_api_tls_material(api, state_dir=tmp_path) + cert_bytes = Path(first_cert).read_bytes() + key_bytes = Path(first_key).read_bytes() + second_cert, second_key = ensure_api_tls_material(api, state_dir=tmp_path) + assert (second_cert, second_key) == (first_cert, first_key) + assert Path(second_cert).read_bytes() == cert_bytes + assert Path(second_key).read_bytes() == key_bytes + + +def test_the_minted_pair_builds_a_serving_context(tmp_path: Path) -> None: + # END TO END for this half: the paths the helper returns must survive build_api_ssl_context, + # which is what the serve path actually calls. A pair that mints but cannot build a context + # would move the failure to startup instead of removing it. + api = ApiSettings() + cert, key = ensure_api_tls_material(api, state_dir=tmp_path) + serving = api.model_copy(update={"tls_cert_file": cert, "tls_key_file": key}) + ctx = build_api_ssl_context(serving) + assert ctx.minimum_version is ssl.TLSVersion.TLSv1_2 diff --git a/tests/test_auth_service.py b/tests/test_auth_service.py index e8867043c..e6caf0a3c 100644 --- a/tests/test_auth_service.py +++ b/tests/test_auth_service.py @@ -202,7 +202,33 @@ async def test_admin_reset_does_not_re_arm_retirement_of_a_claimed_bootstrap() - # The reset re-raises must_change_password — that write is correct and must stay; what must # not follow from it is a retirement. after_reset = await store.get_user_by_username("admin") - assert after_reset is not None and after_reset.must_change_password + # BACKLOG #1245 DIAGNOSTIC LIMB. This assertion has failed INTERMITTENTLY, and the recorded + # failure is `must_change_password` FALSE here -- NOT the retirement assertion below. One + # investigation pass was already spent on the wrong assertion; the item says so explicitly. + # + # A bare `assert after_reset.must_change_password` CANNOT DISTINGUISH THREE CAUSES, and all + # three produce an identical red: + # (a) the write never landed + # (b) it landed and something reset it + # (c) the read path is wrong + # A PAIRED READ AT THE SAME INSTANT -- the store record AND the raw row -- splits them on the + # next occurrence. Same `store._db` seam four other tests in this file already use. + # + # This costs two lines and DOES NOT REQUIRE REPRODUCING THE FAILURE FIRST, which is what + # makes it worth adding before anyone tries to. Six candidate mechanisms are already REFUTED + # BY READING in the item; a seventh is deliberately not offered as a story. + cursor = await store._db.execute( + "SELECT must_change_password FROM users WHERE id=?", (admin.id,) + ) + raw_row = await cursor.fetchone() + raw_flag = None if raw_row is None else raw_row[0] + assert after_reset is not None and after_reset.must_change_password, ( + f"#1245 intermittent: store record must_change_password=" + f"{after_reset.must_change_password if after_reset else None!r}, " + f"raw row must_change_password={raw_flag!r}. " + "DISAGREE -> the read path is wrong (c). BOTH falsey -> the write never landed or was " + "reset (a/b), and the raw value tells you which by whether the row exists at all." + ) out = await service.login("admin", temp) # this login is itself a retirement trigger assert out.ok and out.must_change_password still = await store.get_user_by_username("admin") diff --git a/tests/test_bench_batch_two_box.py b/tests/test_bench_batch_two_box.py index 088c5b59a..3610657bd 100644 --- a/tests/test_bench_batch_two_box.py +++ b/tests/test_bench_batch_two_box.py @@ -116,7 +116,7 @@ def test_build_remote_argv_shape() -> None: report_path=Path("out/x.json"), ) assert argv[1:4] == ["-m", "harness", "connscale-remote"] - assert "--engine-url" in argv and "http://10.0.0.5:9001" in argv + assert "--engine-url" in argv and "https://10.0.0.5:9001" in argv assert argv[argv.index("--inbound-base") + 1] == "2604" assert argv[argv.index("--sink-base") + 1] == "40002" assert argv[argv.index("--engine-index-base") + 1] == "2" @@ -211,7 +211,7 @@ def __init__( self.node_id = node_id self.api_port = api_port self.env = dict(env) - self.url = f"http://127.0.0.1:{api_port}" + self.url = f"https://127.0.0.1:{api_port}" counter["pid"] += 1 self.pid: int | None = counter["pid"] rec.node_envs.append(self.env) diff --git a/tests/test_harness_tls_anchor.py b/tests/test_harness_tls_anchor.py new file mode 100644 index 000000000..6d4572818 --- /dev/null +++ b/tests/test_harness_tls_anchor.py @@ -0,0 +1,130 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The harness's TLS trust anchor (BACKLOG #1276 fallout). + +The engine always serves TLS and mints a self-signed placeholder when no operator certificate is +configured. The harness supplies its own certificate instead, so the anchor exists before any engine +is spawned. These pin the three properties that makes that work, each of which had a plausible +way to be silently wrong. +""" + +from __future__ import annotations + +import contextlib +import os +import socket +import ssl +import subprocess +import sys +import threading +from pathlib import Path + +import pytest + +from harness.load.enginepoll import EnginePoller +from harness.load.tlsmat import harness_ssl_context, harness_tls_material + + +def test_the_anchor_is_minted_once_and_reused() -> None: + """Minting per call would hand different nodes different anchors.""" + first = harness_tls_material() + assert harness_tls_material() == first + cert, key = first + assert Path(cert).read_bytes().startswith(b"-----BEGIN CERTIFICATE-----") + assert Path(key).stat().st_size > 0 + + +def test_the_context_completes_a_handshake_a_default_context_rejects() -> None: + """The anchor is proved by an actual handshake, not by inspecting the context. + + ``get_ca_certs()`` reports NOTHING here even though verification works, because it lists only + certificates carrying CA basic constraints and the engine's is a self-signed leaf. Asserting on + it looked like a stronger check and was simply false -- so this stands up a socket serving the + harness's certificate and verifies that our context completes the handshake where a stock + default context (OS trust store) refuses it. + """ + cert, key = harness_tls_material() + server_ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + server_ctx.load_cert_chain(cert, key) + + with socket.socket() as listener: + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + port = listener.getsockname()[1] + + def serve() -> None: + with contextlib.suppress(OSError, ssl.SSLError): + conn, _ = listener.accept() + with ( + contextlib.suppress(OSError, ssl.SSLError), + server_ctx.wrap_socket(conn, server_side=True) as tls, + ): + tls.recv(1) + + for ctx, should_verify in ( + (harness_ssl_context(), True), + (ssl.create_default_context(), False), + ): + thread = threading.Thread(target=serve, daemon=True) + thread.start() + try: + with socket.create_connection(("127.0.0.1", port), timeout=5) as raw: + if should_verify: + with ctx.wrap_socket(raw, server_hostname="127.0.0.1") as tls: + assert tls.getpeercert() is not None + else: + with pytest.raises(ssl.SSLCertVerificationError): + ctx.wrap_socket(raw, server_hostname="127.0.0.1") + finally: + thread.join(timeout=5) + + +def test_a_child_process_inherits_the_parents_anchor() -> None: + """batchbox spawns `connscale-remote` to poll engines the PARENT started. + + A per-process mint would give that child a DIFFERENT certificate from the one those engines + were handed, so every poll would fail verification. No single-process test can see this. + """ + parent_cert, _ = harness_tls_material() + child = subprocess.run( + [ + sys.executable, + "-c", + "from harness.load.tlsmat import harness_tls_material;print(harness_tls_material()[0])", + ], + capture_output=True, + text=True, + env=os.environ, + check=True, + ) + assert child.stdout.strip() == parent_cert + + +@pytest.mark.parametrize( + ("url", "pinned"), + [ + ("https://127.0.0.1:8765", True), + ("https://localhost:8765", True), + # Plain http is the in-process uvicorn path (ingress_probe) -- httpx ignores TLS settings + # there and pinning would be meaningless. + ("http://127.0.0.1:8765", False), + # An engine on ANOTHER box (shardcert's two-box rig) mints its own cert that this process + # has never seen. Pinning ours would break a path that is not ours to fix here. + ("https://10.0.0.5:8765", False), + ], +) +def test_only_a_loopback_https_engine_pins_to_the_harness_anchor(url: str, pinned: bool) -> None: + assert (EnginePoller._cacert_for(url) is not None) is pinned + + +def test_a_spawned_node_is_handed_the_anchor_as_operator_material() -> None: + """The engine honours [api].tls_cert_file FIRST, so handing it ours makes its own mint path + unreachable -- which is what removes the wait-for-a-file-to-appear race.""" + from harness.load.failover import EngineNode + + node = EngineNode("n1", 8765, env={}, config_dir=".", cwd=Path(".")) + cert, key = harness_tls_material() + assert node._env["MEFOR_API_TLS_CERT_FILE"] == cert + assert node._env["MEFOR_API_TLS_KEY_FILE"] == key + assert node.cacert == cert + assert node.url.startswith("https://") diff --git a/tests/test_key_usage_scope_inventory.py b/tests/test_key_usage_scope_inventory.py index 9aa46ffe0..4afa28bfb 100644 --- a/tests/test_key_usage_scope_inventory.py +++ b/tests/test_key_usage_scope_inventory.py @@ -84,6 +84,10 @@ "Console → engine TLS": "a TLS hop configured from the engine's own listener cert (scoped in " "the Cert tooling row)", "Tray → engine TLS": "a tokenless local TLS probe; no engine-held key", + "Load-test harness → spawned-engine TLS": "a TLS hop whose certificate is minted via the same " + "self-signed dev-cert path already scoped in the Cert tooling row (pki.make_self_signed); the " + "harness process holds the pair only long enough to hand it to the child it spawned, in a " + "per-run temp directory, and nothing about it is engine-held or persisted", "Engine-shard lane ownership": "a coordination record, not cryptographic material", "DAST scan-target credential": "a throwaway CSPRNG password for two ephemeral scan identities, " "stored only as an argon2id hash in a temp-directory store the scan destroys; a credential is " diff --git a/tests/test_security_static.py b/tests/test_security_static.py index 924d68924..dcd2f6741 100644 --- a/tests/test_security_static.py +++ b/tests/test_security_static.py @@ -1018,12 +1018,20 @@ def test_xml_import_scanner_sees_indented_imports() -> None: # --- WP-L3-02 (ASVS 11.1.3): cryptographic-discovery gate -------------------- -#: Crypto call sites in :data:`_CRYPTO_ROOTS` that live OUTSIDE ``messagefoundry/`` — i.e. the ones -#: ``crypto_inventory_check.py``'s own ``INVENTORY`` does not cover, because the shipped CLI still -#: defaults to that single package. Recorded here so the walk-roots comment's claim ("walked anyway so -#: a new one cannot appear outside the inventory") is enforced today rather than promised. When -#: BACKLOG #282 widens the gate itself to these roots this pin becomes redundant and should be deleted -#: in favour of the gate's inventory. +#: Crypto call sites in :data:`_CRYPTO_ROOTS` that live OUTSIDE ``messagefoundry/``, kept as a +#: hand-maintained duplicate of the corresponding slice of ``crypto_inventory_check.py``'s own +#: ``INVENTORY`` so drift between the two is caught rather than assumed. +#: +#: STALE CLAIM CORRECTED (measured 2026-08-25): this docstring used to say the gate's own INVENTORY +#: "does not cover" these paths "because the shipped CLI still defaults to" messagefoundry/ alone, +#: and that BACKLOG #282 widening the walk would make this set redundant. #282 already landed -- +#: WALK_ROOTS is ``("messagefoundry", "messagefoundry_webconsole", "harness", "tee", "scripts")`` +#: today, and every entry below already has a matching entry in the gate's own INVENTORY (confirmed +#: by grep, not assumed). So this set is not filling a gap the gate misses; it is a second, +#: independently-maintained copy of one slice of it -- the same redundant-check shape #1301's and +#: #1338's banner guards use elsewhere in this ledger. Left as a TODO rather than deleted here: doing +#: that properly means confirming every remaining entry really is duplicated (not just the one this +#: commit is adding) and is a separate, larger cleanup from the fix this commit is landing. _CRYPTO_SITES_OUTSIDE_THE_PACKAGE = { # ADR 0156: SHA-256 over the ASVS corpus FILE to pin it to the tagged release. No key. "scripts/asvs/scorecard.py": frozenset({"hashlib"}), @@ -1031,6 +1039,10 @@ def test_xml_import_scanner_sees_indented_imports() -> None: # states which revision of the record it read. No key. "scripts/asvs/prove_report.py": frozenset({"hashlib"}), "messagefoundry_webconsole/_security.py": frozenset({"secrets"}), + # BACKLOG #1276 part A: the harness supplies its own TLS certificate for a spawned engine rather + # than chasing the one the engine mints -- see crypto_inventory_check.py's INVENTORY entry for + # the same file, which this duplicates. + "harness/load/tlsmat.py": frozenset({"ssl"}), "tee/__main__.py": frozenset({"ssl"}), "tee/anon/keying.py": frozenset({"hashlib"}), "tee/mefor_api.py": frozenset({"ssl"}),