diff --git a/docs/ASVS-L2-PHASE0-CHANGES.md b/docs/ASVS-L2-PHASE0-CHANGES.md index aa40278c..bd0f09e7 100644 --- a/docs/ASVS-L2-PHASE0-CHANGES.md +++ b/docs/ASVS-L2-PHASE0-CHANGES.md @@ -351,9 +351,9 @@ tables in [`CONNECTIONS.md`](CONNECTIONS.md) §"Resource management & limits" (A | HashiCorp Vault Transit — store DEK envelope-decrypt (ADR 0019) | outbound | HTTPS via `hvac` (the `[vault]` extra); port from the address | TLS verification is `hvac`/`requests`' own default — the engine sets no explicit client TLS options here | a Vault token — `MEFOR_STORE_VAULT_TOKEN` (`hvac` falls back to `VAULT_TOKEN` when unset) | **yes** — `MEFOR_STORE_VAULT_ADDR` (opt-in; fail-closed) | `MEFOR_STORE_VAULT_ADDR`, `MEFOR_STORE_VAULT_TOKEN`, `MEFOR_STORE_VAULT_TRANSIT_KEY` | | HashiCorp Vault Transit — **bulk at-rest cipher** (`[store].cipher_provider = vault_transit`, ADR 0138) | outbound (**per store operation**) | HTTPS via the same shared `hvac` client build; port from the address. **One `encrypt_data` / `decrypt_data` round trip per encrypted CELL** on every store write and read, plus one `generate_hmac` per audit row — not a startup-only hop | as the DEK hop: `hvac`/`requests` defaults, no engine-set client TLS options | the same Vault token (`MEFOR_STORE_VAULT_TOKEN`) | **yes** — `MEFOR_STORE_VAULT_ADDR` (shared with the DEK hop) | `[store].cipher_provider`, `MEFOR_STORE_TRANSIT_KEY`, `MEFOR_STORE_TRANSIT_AUDIT_KEY` | | DR backup destination (ADR 0049) | outbound (scheduled + on-demand) | local filesystem, or **SMB/CIFS over TCP when `[backup].destination` is a UNC path** (the OS redirector owns the port); a cloud URL is **rejected at load** | n/a — no engine-terminated TLS on this hop; SMB dialect security is the OS's | the engine service account's **own** identity — `[backup]` exposes no `credential_*` impersonation knob, unlike the FILE connector | **yes** — `[backup].destination` | `[backup].enabled`, `[backup].destination`, `schedule_at`, `retention_keep`, `snapshot_method`, `allow_unencrypted` | -| Security-event notification email, per user | outbound | SMTP through the **same** `[alerts]` transport and default port 587, but a **second, independent** background dispatcher — its own 1000-item queue and its own drain task — mailing each affected USER's own address, not the operator `email_to` list | STARTTLS, as the operator sink | as the operator sink | **yes** — the same `[alerts].email_smtp_host` | `[auth].notify_security_events`, `[alerts].email_*` | +| Security-event notification email, per user | outbound | SMTP through the **same** `[alerts]` transport and default port 587, but a **second, independent** background dispatcher — its own 1000-item queue and its own drain task — mailing each affected USER's own address, not the operator `email_to` list | STARTTLS **and certificate verification**, as the operator sink — plumbed at this call site in its own right (`pipeline/security_notify.py`), not inherited implicitly | as the operator sink | **yes** — the same `[alerts].email_smtp_host` | `[auth].notify_security_events`, `[alerts].email_*` | | HashiCorp Vault KV v2 — connector-credential secrets provider (ADR 0019) | outbound | HTTPS via `hvac`, a **separate client** from the Transit one behind the same extra | as above | a Vault token — `MEFOR_SECRETS_VAULT_TOKEN` (falls back to `VAULT_TOKEN`) | **yes** — `MEFOR_SECRETS_VAULT_ADDR` (opt-in; fail-closed) | `MEFOR_SECRETS_VAULT_ADDR`, `MEFOR_SECRETS_VAULT_TOKEN`, `[secrets].provider` | -| Alerts — SMTP notification sink | outbound | SMTP, **default port 587**, STARTTLS by default. Distinct from the `email` message connector | STARTTLS (`email_use_tls`) | optional `email_username` / `email_password` (env `MEFOR_ALERTS_EMAIL_PASSWORD`, never the file) | **yes** — `[alerts].email_smtp_host`, gated by `[alerts].smtp_allowed_hosts` (empty = any) | `[alerts].email_smtp_host`, `email_smtp_port`, `email_from`, `email_to`, `email_timeout`, `smtp_allowed_hosts` | +| Alerts — SMTP notification sink | outbound | SMTP, **default port 587**, STARTTLS by default. Distinct from the `email` message connector | STARTTLS (`email_use_tls`). The server certificate **is verified** (`email_tls_verify` default true, #323) — chain + hostname + strict RFC 5280, TLS 1.2 floor, anchored to `email_tls_ca_file` / `[tls].internal_ca_file` / the OS roots | optional `email_username` / `email_password` (env `MEFOR_ALERTS_EMAIL_PASSWORD`, never the file) | **yes** — `[alerts].email_smtp_host`, gated by `[alerts].smtp_allowed_hosts` (empty = any) | `[alerts].email_smtp_host`, `email_smtp_port`, `email_from`, `email_to`, `email_timeout`, `smtp_allowed_hosts`, `email_tls_verify`, `email_tls_ca_file` | | Alerts — webhook sink | outbound | HTTP(S) POST of the event as JSON through a **no-redirect** opener (a 3xx cannot divert the POST) | a plaintext `http://` target is **refused** unless `MEFOR_ALLOW_INSECURE_TLS` | none — the URL is the credential | **yes** — `[alerts].webhook_url`, gated by `[alerts].webhook_allowed_hosts` | `[alerts].webhook_url`, `webhook_timeout`, `webhook_allowed_hosts` | | Off-box syslog log forwarder (ADR 0080) | outbound | `udp` (**the default**, plaintext RFC 5426), `tcp` (RFC 6587) or `tls` (RFC 5425); **default port 514** | on `tls`, only `forward_tls_ca_file` is trusted (system roots are **not** loaded) with hostname checking (`forward_tls_verify` default true) and optional mutual TLS via `forward_tls_client_cert`. A plaintext / unverified collector hop is REFUSED on an enforcing production-PHI instance unless `forward_hop_attested` | none on `udp`/`tcp` (network trust); the client certificate under mutual TLS | **yes** — `[logging].forward_host` | `[logging].forward_enabled`, `forward_host`, `forward_port`, `forward_protocol`, `forward_format`, `forward_tls_ca_file`, `forward_tls_verify`, `forward_tls_client_cert`, `forward_hop_attested` | | SNTP / NTP startup clock-sync probe (ADR 0080, ASVS 16.2.2) | outbound | UDP, **port 123** — one 48-byte stdlib SNTP request at startup, never on the message path | none — SNTP, not NTS | **unauthenticated by design**: a coarse drift check for a trusted management network | **yes** — `[logging].ntp_peer` | `[logging].ntp_peer`, `require_time_sync` (both are needed to enable it; default = a no-op), `time_sync_max_skew_seconds`, `time_sync_fail_closed` | diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index d48a56cd..98ed3799 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -5263,13 +5263,13 @@ sourced — **#1 (SQL Server concurrency)** and **#2 (console off-thread)** — **Trigger:** build when a partner mandates an SMTP server whose TLS certificate cannot be validated. **Prefer fixing the trust chain.** -**Why:** Real gap. The alert SMTP sink (EmailTransport in pipeline/alert_sinks.py) calls starttls() with the default SSL context and exposes only email_use_tls plus the global MEFOR_ALLOW_INSECURE_TLS cleartext escape, so there is no per-mail-server option to keep TLS on yet unconditionally trust a self-signed/mismatched certificate. +**Why (AS FILED — both halves are now false; see the correction below).** Filed as a real gap: the alert SMTP sink called `starttls()` with the default SSL context and exposed only `email_use_tls`, so there was no per-mail-server option to keep TLS on yet trust an unvalidatable certificate. [#323](#323) layer 3 changed both facts — the hop now builds a **verifying** context, and `[alerts].email_tls_verify` **is** that per-server override. **Why it is an anti-feature:** unconditionally trusting an SMTP server's certificate defeats TLS. The only escape (`MEFOR_ALLOW_INSECURE_TLS`) is global and deliberately loud. Recorded for parity completeness, not as a want. -> ⚠️ **CORRECTED 2026-08-01 — this item previously asserted "The engine's `EmailAlertSink` uses STARTTLS with a verifying context by design." That was FALSE**, and it is the shape [`CLAUDE.md`](../CLAUDE.md) §11 names as worst: *a compensating control resting on a false premise*. `smtplib.starttls()` with no context falls back to `ssl._create_stdlib_context`, which **is** `ssl._create_unverified_context` (`CERT_NONE`, `check_hostname=False`) — so the alert sink was encrypting without authenticating, and a reader of this item would have concluded alert email was TLS-verified when it was not. #323 fixed the two **connectors** (`EmailDestination` / `DirectDestination`); it did **not** fix the alert sink, so the **Why:** paragraph above remains accurate and this item's premise stays false until #323's alerts-cell residual lands. Do not re-assert verification here before then. +> ⚠️ **CORRECTED 2026-08-01, RESOLVED 2026-08-02.** This item once asserted "The engine's `EmailAlertSink` uses STARTTLS with a verifying context by design." That was **FALSE when written** — the shape [`CLAUDE.md`](../CLAUDE.md) §11 names as worst, *a compensating control resting on a false premise*: `smtplib.starttls()` with no context falls back to `ssl._create_stdlib_context`, which **is** `ssl._create_unverified_context` (`CERT_NONE`, `check_hostname=False`), so the sink encrypted without authenticating and a reader would have concluded alert email was TLS-verified when it was not. [#323](#323) layer 3 has now landed, so **the sentence is true for the first time** — verification is built, not assumed, and `tests/test_alert_smtp_tls.py` asserts it against a negative control. The standing instruction is therefore lifted, with one condition: state it as **built and tested**, never as "by design". ⚠️ **This item stays DECLINED.** #323 built *verification*; #139 asks for the **anti-feature** — unconditionally trusting any certificate. That capability now exists as `email_tls_verify = false`, but it is deliberately **not** the per-mail-server knob this item wanted: it is instance-wide, it is a named loosening, and on an enforcing PHI instance it refuses to start without `[security].allow_unverified_alert_smtp_tls`. If a partner ever mandates an unvalidatable relay, the answer is `email_tls_ca_file`, not this item. -**Nearest existing mechanism:** EmailTransport / send_plain_email in pipeline/alert_sinks.py (SMTP alert sink, built by notifier_from_settings from AlertsSettings in config/settings.py); its only TLS knob is email_use_tls (STARTTLS on/off) plus the global MEFOR_ALLOW_INSECURE_TLS / insecure_tls_allowed() escape, which permits CLEARTEXT SMTP — not a keep-TLS-but-trust-any-cert override. +**Nearest existing mechanism (UPDATED 2026-08-02):** `EmailTransport` / `send_plain_email` in `pipeline/alert_sinks.py`. Its TLS knobs are now `email_use_tls` (STARTTLS vs cleartext), **`email_tls_verify`** (authenticate the relay — the keep-TLS-but-trust-any-cert override this item described, though instance-wide rather than per-server) and **`email_tls_ca_file`** (the preferred answer: trust the relay's own CA and keep verification on), gated by `[security].allow_unverified_alert_smtp_tls`. ⚠️ The old text here claimed the global `MEFOR_ALLOW_INSECURE_TLS` / `insecure_tls_allowed()` escape applied to this cell; it never did — measured, that escape is read in `alert_sinks.py` **only** on the webhook `http://` branch, so cleartext alert SMTP was gated by nothing at all until #323 layer 3's serve gate covered it. **Source:** Corepoint v8.1.0 help-export coverage sweep (2026-07-09) — five adversarially-verified passes over the full help export (1,569 pages); absence re-verified against `origin/main` before filing. Cross-ref [#52](#52-corepoint-capability-parity-gaps--prioritized-roadmap-input-2026-06-27). @@ -7403,11 +7403,17 @@ MEFOR_FORBIDDEN_TOKENS=scripts/security/scan-tokens.local.txt.example \ ## 323. SMTP TLS is unverified on all three send paths -> 🚧 **Status 2026-08-01 — PARTIALLY SHIPPED, 2 of 3 cells** (PR #132). The two **connectors** verify: `EmailDestination` and `DirectDestination` build an explicit verifying context via the new `tls_policy.build_smtp_tls_context()` and pass it on both `smtplib` arms, with per-connection `tls_verify` / `tls_ca_file` / `tls_check_hostname` on the `Email()` and `Direct()` factories. Proven by **negative control**: with the code change stashed and the tests kept, all eight new assertions in `tests/test_email_destination.py` go **red** — the pre-existing "STARTTLS was issued" assertions stayed green throughout the insecure period and could never have caught this. ⚠️ **The alerts cell is NOT fixed**: `pipeline/alert_sinks.py:384` still calls `smtp.starttls()` bare, so alert + security-notify email remains unverified. That is the residual below, and it is why [#139](#139)'s premise is still false. -> -> 🔢 **Filed 2026-08-01.** Value **8/10** · Difficulty **4/10** · _fill-in_. All SMTP send paths called `starttls()` / `SMTP_SSL()` with **no** SSL context, so Python 3.14's stdlib default applied (`ssl._create_stdlib_context` **is** `ssl._create_unverified_context` — `CERT_NONE`, `check_hostname=False`) — the EMAIL destination put Handler PHI *and* the SMTP AUTH password over an encrypted-but-unauthenticated hop, while the #201 revocation guard, the cleartext-credential rule and `DEPLOYMENT.md` all described that same hop as **verified**. +> ✅ **SHIPPED 2026-08-02 — all three cells.** Connectors in PR #132 (layers 1–2); the **alerts cell** in layer 3. `send_plain_email` builds an explicit verifying context via `tls_policy.build_smtp_tls_context()` and passes it to `starttls()`, from new `[alerts].email_tls_verify` / `email_tls_ca_file`, plumbed through **three** construction seams — `EmailTransport` (ops alerts), `SecurityEventNotifier` (per-user security email — a genuinely separate call site, not an inheritor), and the hand-rolled transport inside `POST /alerts/test-email`, which had to be plumbed too or the operator's "test my mail server" button would have exercised a *different* TLS posture than live alerts. Gated by a `[security].allow_unverified_alert_smtp_tls` **acknowledgment switch** at the serve gate, registered in `security_loosenings()` (two entries — see below), and reported by a new `alert-smtp-tls` `checks.py` advisory. Proven by **negative control**: with the production change stashed and the tests kept, 7 assertions go red at `assert None is not None` — including the pre-existing `test_email_transport_sends_via_smtp`, whose `sent["tls"] is True` assertion stayed green for the entire insecure period and whose fake had *already* been widened to accept `context=` by PR #132 without the production code passing one. Filed 2026-08-01: Value **8/10** · Difficulty **4/10**. All SMTP send paths called `starttls()` / `SMTP_SSL()` with **no** SSL context, so Python 3.14's stdlib default applied (`ssl._create_stdlib_context` **is** `ssl._create_unverified_context` — `CERT_NONE`, `check_hostname=False`) — the EMAIL destination put Handler PHI *and* the SMTP credential over an encrypted-but-unauthenticated hop, while the #201 revocation guard, the cleartext-credential rule and `DEPLOYMENT.md` all described that same hop as **verified**. + +**Layer 3 as built, and the two places it departs from the residual as filed.** Both departures are recorded here rather than left as silent omissions. + +1. **The serve gate also refuses `[alerts].email_use_tls=false` (cleartext), which is broader than this item asked for.** Measured: `insecure_tls_allowed()` is read in `alert_sinks.py` **only** on the webhook `http://` branch, so cleartext alert SMTP was gated by *nothing*. Refusing verify-off while permitting cleartext would have handed an operator a bypass onto the strictly **worse** posture, so the gate's condition is “this hop does not authenticate the relay”, which is true of both. Strictly **adds** refusals (ADR 0092 decision 5); byte-identical on the shipped defaults. + +2. **The `refuse_unverified_smtp_tls()` helper this item called for was NOT built.** Two reasons, and the second is the load-bearing one. (a) The alerts cell refuses at the **serve gate**, not against the clamp, so it would never call the helper — it would ship with zero callers. (b) The two connectors' verify-off refusals are **not** identical (measured by `ast.Raise` collection with the cell name normalised: 483 vs 542 chars — `Direct` truncates `Email`'s first sentence and inserts an S/MIME-specific harm sentence), so “extracting” them would be a **rewording of a shipped operator-facing refusal**, and would reverse the written decision at `transports/email.py:180-185` that a third spelling is how the next bug gets written. If a helper is ever wanted here, the sibling **credential** refusals are the safe mechanical extraction — those *are* identical modulo the cell name. -**Residual — the alerts cell (~1 layer).** `pipeline/alert_sinks.py` `send_plain_email` and `pipeline/security_notify.py` need the same context, plumbed from new `[alerts].email_tls_verify` / `email_tls_ca_file`, plus a `[security].allow_unverified_alert_smtp_tls` acknowledgment switch at the serve gate. It needs an **acknowledgment switch rather than the clamp** because the contextvar hop posture is never stamped for that cell — which is why it was deferred rather than folded in. The shared `refuse_unverified_smtp_tls()` helper belongs in `config/settings.py` at that point; the connectors currently **inline** the refusal, matching the `mllp.py` / `remotefile.py` house style. Then: register the deviation in `security_loosenings()` (see [#333](#333)), add a `checks.py` advisory, and correct `docs/PHI.md` and [#139](#139). +**`security_loosenings()` gained a 5th REQUIRED `alerts` parameter and TWO entries, not one.** The deviation and the acknowledgment of it are different facts: under `enforcement=warn` an operator can run verify-off with **no** acknowledgment at all, so keying the report on the switch alone would have left the actual weakening invisible — the exact failure mode the registry exists to prevent. Required rather than optional per the function's own contract (“an optional parameter is a detector that silently fails to fire”). Unlike `cleartext_accepted` this deviation is **settings-scoped**, so `security show` and a graphless `GET /security/posture` report it completely rather than declaring a gap. + +**Still open, and NOT closed by this item:** `[alerts].email_smtp_port=465` (implicit TLS) does not work on this cell and never did — `send_plain_email` has no `SMTP_SSL` arm at all. Stated, not fixed. The connection-scoped `unverified_smtp_hops()` reader that would report the **connectors'** `tls_verify=false` belongs to [#333](#333), which stays open. **Also landed, called out rather than folded in silently.** `transports/direct.py`'s cleartext arm read the **unclamped** `insecure_tls_allowed()` while its sibling arm one branch away read the clamped `weakened_tls_escape_permitted_here()`. Two different escapes in one connector is how the next bug gets written, so it now reads the clamped one. Strictly **adds** refusals (ADR 0092 decision 5). Partially closes the [#329](#329) concern. @@ -7415,7 +7421,7 @@ MEFOR_FORBIDDEN_TOKENS=scripts/security/scan-tokens.local.txt.example \ **Cluster:** Security & Compliance. **Priority:** P1. **Verdict:** build. **Severity:** high. -**What:** Every SMTP send in the product opens TLS with no `context` argument: +**What (AS FILED — every row below is now historical).** All five call sites were fixed: the four connector rows by PR #132, the `alert_sinks.py` row by layer 3. Kept as the record of what was found, not as a description of live code: | Site | Call | |---|---| @@ -7447,7 +7453,7 @@ Nothing in the product moves that default: grep for `_create_default_https_conte The same false premise is published to operators: `docs/DEPLOYMENT.md:129` counts *"SMTP/EMAIL"* among *"**seven** verifying outbound TLS hops"*, and #201's shipped banner (`docs/BACKLOG.md:6430`) says the guard *"fires only on a VERIFYING hop"*. -`docs/PHI.md:916` is the one place that is **honest** — it states the `send_plain_email` defect plainly and correctly notes *"PR #1163 hardened the EMAIL message destination connector, not the `[alerts]` SMTP path"*. That doc is right about the alerts path and stays; what it does not say is that the destination connector it points at has the identical defect. +`docs/PHI.md` stream 11 was the one place that was **honest** — it stated the `send_plain_email` defect plainly and warned readers not to generalise it to the connectors. ⚠️ The sentence this item quoted verbatim from it (*"PR #1163 hardened the EMAIL message destination connector, not the `[alerts]` SMTP path"*) **no longer exists** — PR #132 rewrote that row, so the quotation was already stale before layer 3 touched it. Layer 3 rewrote the row again, to the verifying posture. What that doc did not say is that the destination connector it points at has the identical defect. **Why:** The EMAIL destination is a PHI egress — the Handler payload **is** the body (`transports/email.py:197-205`). "Encrypted but unauthenticated" means an active on-path attacker terminates the TLS session with any self-signed certificate and reads the PHI in clear, then relays. The engine's own posture machinery cannot see this: #200 keys on `use_tls=false` / `tls_verify=false` and #201 keys on revocation, so a `use_tls=true` EMAIL connection passes **every** hop gate as a secure hop. @@ -7467,9 +7473,7 @@ The reason to rate this high anyway is the second-order damage: a green posture 2. Expose per-connection `tls_verify` + `ca_cert` on the `Email()` / `Direct()` factories and the `[alerts].email_*` settings, so a private-CA or self-signed mail server is served by **trust configuration**, not by silent non-verification. 3. Route any resulting `tls_verify=false` through the **same** #200 posture gate as MLLP/FTPS (`docs/BACKLOG.md:6410`), so a production-PHI instance refuses it. Then #201's `RevocationHopGuard` on this path becomes true rather than aspirational. 4. Correct the false-premise prose in the same commit: `transports/email.py:173`, the guard labels at `:184-185`, `config/tls_policy.py:703`, `docs/DEPLOYMENT.md:129`, and the #201 banner at `docs/BACKLOG.md:6430`. Update `docs/PHI.md:916` to cover all three paths once fixed. -5. Tests: the three fake SMTPs (`tests/test_email_destination.py:57`, `tests/test_direct_transport.py:182`, `tests/test_alert_sinks.py:247`) all declare `def starttls(self) -> None:` with **no** `context` parameter and none asserts verification — they will need the kwarg, plus a positive assertion that the passed context has `check_hostname=True` / `verify_mode=CERT_REQUIRED`. A real-handshake test against a wrong-host certificate is the one that would actually have caught this. - -**Migration risk, stated plainly:** turning verification on is a **breaking change** for any deployment currently pointed at a self-signed or private-CA mail server — those sends begin failing at `starttls()`. Step 2 is the mitigation and must land in the same release, with a release note. +5. Tests: ⚠️ **the parameter half of this step was already false when layer 3 started** — PR #132 widened all three fakes (including the alerts one) to accept `context`, without the alerts production code ever passing one. So the fakes accepted the kwarg and **discarded** it, and the assertion half was the part that mattered: none asserted verification — they will need the kwarg, plus a positive assertion that the passed context has `check_hostname=True` / `verify_mode=CERT_REQUIRED`. A real-handshake test against a wrong-host certificate is the one that would actually have caught this. **Related:** [`messagefoundry/transports/email.py`](../messagefoundry/transports/email.py), [`messagefoundry/transports/direct.py`](../messagefoundry/transports/direct.py), [`messagefoundry/pipeline/alert_sinks.py`](../messagefoundry/pipeline/alert_sinks.py), [`messagefoundry/config/tls_policy.py`](../messagefoundry/config/tls_policy.py), [ADR 0029](adr/0029-email-smtp-destination.md), [ADR 0078](adr/0078-certificate-revocation-posture.md), [ADR 0085](adr/0085-direct-hisp-smime-connector.md), [ADR 0092](adr/0092-posture-keyed-transport-hop-refusal-refuse-the-insecure-phi-hop.md), [`docs/PHI.md`](PHI.md) §7 stream 11, [`docs/DEPLOYMENT.md`](DEPLOYMENT.md), #200, #201, #139. @@ -7964,6 +7968,8 @@ Honestly bounded: **this is build-time only.** No PHI path, no running-engine su *(Filed as one item, not two: both are fixed by the same edits to the same four files — a reader beside `accepted_cleartext_hops`, an entry in `security_loosenings()`, an advisory beside `_check_cleartext_accepted`, and threading at `api/app.py`. Stated once rather than twice, per the docs rule against restating a load-bearing fact.)* +**Partial delivery from [#323](#323) layer 3 (2026-08-02) — this item stays OPEN.** #323 added the `[alerts]` SMTP hop's deviations (`email_use_tls` / `email_tls_verify`) to `security_loosenings()`, which required giving it a 5th **required** `alerts` parameter, and added an `alert-smtp-tls` advisory beside `_check_cleartext_accepted`. So the *shape* this item asks for now exists and has a worked precedent — but **neither of this item's two deviations is closed**: `tls_allow_expired` on the six outbound connectors and the generic-ODBC DATABASE hop are both still invisible, and no connection-scoped `unverified_smtp_hops()` reader was built (that would report the **connectors'** `tls_verify=false`, which is this item's territory, not #323's). The completeness floor in `tests/test_security_posture_defaults.py` still iterates `SecuritySettings.model_fields` only, so per-connection settings and `[alerts]` fields remain structurally invisible to it — #323's entries are guarded by hand-written tests, not by the floor. Do not read this note as progress toward closure; read it as one worked example to copy. + **Cluster:** Security / Posture reporting. **Priority:** P2. **Verdict:** build. **Severity:** low. **What:** two different weakenings, one shared blind spot. `security_loosenings()` ([`config/settings.py:3955-4140`](../messagefoundry/config/settings.py)) covers neither: the function was read end to end, and the last entry is the ADR 0153 `cleartext_accepted` block at `:4130-4139`, immediately followed by `return out` at `:4140`. Its own scope paragraph enumerates the deviations it covers from outside `[security]` — `settings.py:3967-3968`: diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 9108c60a..fdb709e4 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -1006,7 +1006,9 @@ best-effort and runs on a background task, so it never blocks or hangs a deliver | `email_smtp_port` | int | 587 | SMTP port | | `email_from` | str | _unset_ | sender address (required for email) | | `email_to` | list | _unset_ | recipient(s) (required for email). Via env: comma-separated `MEFOR_ALERTS_EMAIL_TO` | -| `email_use_tls` | bool | `true` | issue STARTTLS before sending | +| `email_use_tls` | bool | `true` | issue STARTTLS before sending. Selects TLS vs **cleartext** — it does not by itself decide whether the relay is authenticated; that is `email_tls_verify` | +| `email_tls_verify` | bool | `true` | verify the relay's certificate on that STARTTLS hop — chain + hostname + strict RFC 5280, TLS 1.2 floor ([#323](BACKLOG.md)). `false` keeps the session encrypted but accepts **any** certificate (MITM-able). Both `false` values are **loosenings**: `security_loosenings()` names them, `messagefoundry check`'s `alert-smtp-tls` advisory reports them, and on an enforcing PHI instance `serve` **refuses to start** unless `[security].allow_unverified_alert_smtp_tls` is also set | +| `email_tls_ca_file` | str | *(unset)* | PEM bundle of trust anchors for that hop. Unset = `[tls].internal_ca_file` if configured, else the OS trust store. A path, not a secret | | `email_username` | str | _unset_ | SMTP login user (omit for unauthenticated relays) | | `email_password` | str | _unset_ | **secret** — supply via `MEFOR_ALERTS_EMAIL_PASSWORD`, never the file (or use `email_password_secret`) | | `email_password_secret` | str | _unset_ | connector `SecretProvider` reference (ADR 0019 §5) — when set and `[secrets].provider` is configured, the SMTP password is resolved from that backend (e.g. a Vault KV `path#field`) instead of `email_password`. A reference, not a secret. | @@ -1468,6 +1470,7 @@ and a PHI weakening under **strict enforcement** (`enforcement = enforce`, the d | `allow_unencrypted_phi` | bool | `false` | audited escape: start a PHI instance with **no** key | | `allow_unencrypted_phi_under_strict_enforcement` | bool | `false` | the **second acknowledgment** required to start a PHI instance keyless under strict enforcement ([ADR 0140](adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md)). Under `enforcement = enforce`, `allow_unencrypted_phi = true` on its own is **not** enough — `serve` still refuses to start (exit 2) unless this is also set, so the highest-risk posture (real PHI + strict enforcement) is never one flag away from plaintext at rest. Under `enforcement = warn` the single `allow_unencrypted_phi` flag still governs. With both set the instance starts with PHI bodies, summary/metadata and the error columns **unencrypted at rest**, and the startup AUDIT line names **both** flags. A **loosening** — `security_loosenings()` reports it, so it is never silent | | `allow_single_factor_admin_when_exposed` | bool | `false` | permit **single-factor admin on an exposed PHI instance** (ADR 0140). With `require_sign_in` on, `require_mfa` explicitly off, and the operator surface exposed — a **non-loopback bind**, or the console reached through a declared TLS-terminating proxy **with `serve_web_console` explicitly `true`** — a PHI instance under `enforcement = enforce` **refuses to start** (exit 2) — the Administrator role would authenticate with a single factor over the network. Setting this permits that start; it is recorded in a WARNING-level AUDIT line and the ordinary exposure warning still prints. A **loosening** — `security_loosenings()` reports it. **Blind spot — do not treat this refusal as your MFA control.** The proxy arm keys on the console actually being *served*, and a **default-on** console auto-degrades to JSON-only on an exposed instance (ADR 0143, flipped in place at startup), so the topology this document recommends — loopback bind behind a declared terminator, `serve_web_console` left at its default — does **not** trip the refusal even on a PHI instance under `enforce` with `require_mfa = false`. The JSON operator API is still reachable off-box through that proxy with single-factor admin. Set `require_mfa = true` there regardless; the gate will not catch you. **Prefer `require_mfa = true` — and know its scope.** Under the shipped `require_mfa_scope = "every_local_account"` it requires a second factor from **every** local account, *not* only Administrators, so a non-interactive **local** bearer-token service account becomes MFA-pending and cannot enrol unattended. **There are two remedies, not three.** Either make it a **directory (AD/Kerberos) principal** — those are out of scope under either value, their factor delegated to the directory — or set `require_mfa_scope = "administrators"` (itself reported as a loosening, and it leaves every local Administrator in scope) — see that row below. **mTLS is *not* the third.** A `[api].tls_client_cert_identities` mapping does grant a cert-identity that never meets the MFA gate, but that plane is admitted on exactly **one** route (`GET /service/identity`, `require_service_cert`) and carries no session, so an account "moved to mTLS" can read back its own identity and nothing else — it cannot replay, purge, poll status, or do any work a service account exists for. The `[api].tls_client_cert_identities` row above is the authority on that reach. Directory identities being out of scope also means an AD-only deployment is safe **for its AD users**; its local bootstrap admin and any local service accounts are still in scope | +| `allow_unverified_alert_smtp_tls` | bool | `false` | the **acknowledgment** required to start an enforcing PHI instance whose `[alerts]` SMTP hop does not authenticate the relay — i.e. `[alerts].email_use_tls = false` (cleartext) or `[alerts].email_tls_verify = false` (encrypted but accepts any certificate) ([#323](BACKLOG.md)). Covers BOTH shapes deliberately: cleartext is strictly worse than unauthenticated TLS, so gating only the second would hand an operator a bypass onto the worse posture. Without it `serve` refuses to start (exit 2); with it the start is permitted and named in a WARNING-level `AUDIT:` line. An **acknowledgment switch rather than the clamped `MEFOR_ALLOW_INSECURE_TLS` escape** the connectors use, because this cell is constructed outside the `active_hop_posture` scope where that clamp would be inert. A **loosening** — `security_loosenings()` reports it, so it is never silent | | `memory_encryption_operator_declared` | bool | `false` | **`[BUILT]` ([ADR 0152](adr/0152-in-use-data-protection-for-phi-platform-memory-encryption-attestation-asvs-11-7-1.md) rung 2, ASVS 11.7.1):** the operator's **declaration** that this host provides hardware memory encryption (AMD SEV-SNP / Intel TDX), so PHI is protected in RAM **while it is being processed**. The engine cannot verify it — a local CPU flag is emitted by the OS whose integrity the requirement protects against — so this records **who took responsibility**, the same discipline as `MEFOR_TLS_REVOCATION_ATTESTED`. It is deliberately **not** called "attested": in confidential computing that word means a CPU-signed quote verified against the silicon vendor's root PKI (ADR 0152 rung 3, **not built**). An **exposed** PHI instance without it **warns and starts** — on every environment, at both `enforcement` settings; it refuses only if `require_memory_encryption_declaration` is also set. A **positive platform read-out does not substitute for it** (a read-out must never relax a control). **Loopback and synthetic instances are byte-identical** (never consulted). If the platform read-out positively contradicts this, the contradiction is **warned at start and reported** as `memory_encryption_readout_contradicts_declaration` on `GET /security/posture` — but **never refused** (the read-out is a self-report, not evidence, and has known false negatives: driver not loaded, container without the device node mapped, Azure CVM paravisor). **Setting this does not make the instance ASVS 11.7.1-compliant** — see the read-out note below the table. Env: `MEFOR_SECURITY_MEMORY_ENCRYPTION_OPERATOR_DECLARED` | | `require_memory_encryption_declaration` | bool | `false` | **`[BUILT]` (ADR 0152 rung 2):** turn the row-12 warning above into a **refusal** — an **exposed** PHI instance with no `memory_encryption_operator_declared` then **refuses to start** under `enforcement=enforce` (and still warns under `warn`). **Opt-in by design, and the default is load-bearing:** the property is a **host** property that no operator can satisfy on Windows (the read-out is always `null` there), and "exposed" includes the recommended loopback-behind-proxy topology, so a refusal by default would stop working dev/staging/prod deployments from booting on upgrade over something they cannot change. Same scoping rule as `[security].allowed_client_networks`' companion refusal (ADR 0151): a new refusal fires only on a new opt-in. Set it in an estate that has standardized on confidential-computing hosts and wants a missing declaration to be fatal. Env: `MEFOR_SECURITY_REQUIRE_MEMORY_ENCRYPTION_DECLARATION` | | `require_sign_in` | bool | `true` | authenticate every request | diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index f590c5c0..13782a03 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -572,6 +572,7 @@ on the shipped posture. | **LDAPS** (`[auth].ad_tls_verify`, default true) | verifying directory bind | | **`[logging]` TLS syslog forwarder** (`forward_tls_verify`, default true) | CA-anchored RFC 5425 hop | | **Webhook alert sink** and the **AI-broker endpoint** | verifying https openers | +| **`[alerts]` SMTP sink** and the **per-user security-event notifier** | `email_tls_verify` defaults **true** ([#323](BACKLOG.md)) — one verifying context, two call sites. Deliberately carries **no** `RevocationHopGuard`: it is constructed outside the `active_hop_posture` scope those guards read, so a guard here could not see the instance posture. Its verify-off / cleartext deviations are gated by `[security].allow_unverified_alert_smtp_tls` at the serve gate instead | For every hop in that table, revocation is exactly what the ASVS row above calls *delegated* — **your PKI's or your egress proxy's job, written into your runbook**. The engine will not make you say so, and diff --git a/docs/PHI.md b/docs/PHI.md index 09c50abc..27c8fbb3 100644 --- a/docs/PHI.md +++ b/docs/PHI.md @@ -913,8 +913,8 @@ with materially different PHI profiles, so they get their own rows; stream 4 is | **8. `alert_instance` table — default on wherever an `[alerts]` notifier exists** | resolvable operator alerts: `connection_stopped`, `queue_buildup`, `lane_stuck`, `message_stall`, `saturation`, `connection_error`, `content_match`, `storage_threshold`, `cert_expiry`, `secret_rotation`, `bootstrap_admin_expiring` (the UNCLAIMED first-run bootstrap admin nearing its auto-disable deadline — ASVS 6.4.5; its payload carries only the ISO deadline plus whole hours remaining, never the password or any secret), `integrity_drift`, `update_available`, `backup_failed`, `rcsi_off_degraded`, `leadership_acquired`, `dr_activated`, `gcm_invocations` (the per-key AES-GCM invocation bound crossing its 2^31 soft warn — ASVS 11.3.4; its payload carries a one-way `key_id` fingerprint plus counters, never key bytes) The three reachable **inverse** signals — `connection_restored`, `leadership_lost`, `dr_released` — are never rows here: `_record_state` routes an inverse through `_AUTO_RESOLVE` to `resolve_alert_instances_for`, never to `upsert_alert_instance`. (A fourth mapped key, `connection_started`, is emitted by no code path today.) | rows: `event_type`, `connection`, `severity`, `status`, `first_seen`, `last_seen`, `count`, `reason`, `acked_by`, `acked_at`, `resolved_at`, `suspended_until`, `escalation_tier` | the store database, **all three backends** | the operator alert list — acknowledge / resolve / suspend. Durable state is recorded **before** any suppression or throttle return, so a muted alert still leaves a record | `GET /alerts/active` under **`monitoring:diagnose`** (**not** a PHI permission) with the same per-channel scope; ack/resolve/suspend/**resume** are POSTs on the same tier, and the separate read-only `GET /alerts/rules` view sits on its own gate | shares the connection-event window; **only RESOLVED instances are DELETEd**, by `resolved_at` — an open or acknowledged condition is never aged out from under an operator | **`reason` is free text** taken from the event's `detail`/`reason`/`label`: `safe_text(reason)[:200]` then cipher-encrypted (AAD `("alert_instance","reason",event_type,connection)` — the de-dup grain, so one AAD covers both the INSERT and the re-fire UPDATE). `content_match` is **PHI-free by contract**: the sink method takes no value parameter, only the connection, an operator label and an optional rule id | | **9. `response` rows with `kind='ack_sent'` — DEFAULT ON** (`[diagnostics].response_sent = true`) | the ACK/NAK the engine returned to an inbound sender, under a sentinel destination `\x1fack:` | rows: `ack_code` (`AA`/`AE`/`AR`/`CA`/`CE`/`CR`), `ack_phase` (`decode`/`parse`/`strict`/`ingest`), `outcome`, `body`, `detail` | the store database | "what did we actually reply, and why" — the operator's answer to a sender disputing an ACK | `GET /messages/{id}/responses` under `messages:read` + `require_phi_read`; the `body` only for a caller who also holds `messages:view_raw`; every read writes a `response.read` audit row | `body`, `detail` and `resp_headers` are set to `NULL` in place by `purge_message_bodies` on the message-body window, on all three backends | **PHI fail-safe:** the ACK **body** is stored **only when the store cipher is active** — on a keyless store it is `NULL` rather than plaintext — and every NAK passes no body at all, so the offending field value is never persisted. The disposition metadata (`ack_code`/`ack_phase`/`outcome`) is non-PHI and always captured; `detail` is `safe_text`-scrubbed, 200-char bounded and encrypted | | **10. `[alerts]` webhook transport** (off by default — `webhook_url` unset) | one HTTPS POST per alert, carrying every non-underscore event key as JSON | JSON | the operator's webhook endpoint (Slack/Teams/PagerDuty/custom) | operator notification | **`https` only** — a plaintext `http://` webhook URL is refused at construction unless the raw `MEFOR_ALLOW_INSECURE_TLS` escape is set (and then a warning is logged); note this path reads the **unclamped** escape, unlike the connectors. Redirects are refused; an optional `webhook_allowed_hosts` egress allowlist gates the host | the endpoint's | **carries the alert's `detail`/`reason` free text** (`safe_exc()`-scrubbed at the emit sites, but **not** re-run through `safe_text` on this path). Internal `_`-prefixed keys (per-rule recipients, rule id, cooldown) are stripped before send, so recipient addresses never cross the wire | -| **11. `[alerts]` SMTP transport — operator alert list** (off unless `email_smtp_host` + `email_from` + ≥1 `email_to`) | one email per alert; default subject `[MessageFoundry] — `, default body every non-underscore event key as `k: v` | plain text (always kept — never HTML-only); optional HTML alternative | the operators' mailboxes | operator notification | `smtp_allowed_hosts` egress allowlist; the SMTP password comes from `MEFOR_ALERTS_EMAIL_PASSWORD` or a `[secrets]` provider, never the config file; per-send timeout `email_timeout` | the mail system's | carries the same `detail`/`reason` free text as the webhook. #138 operator templates are constrained to a **closed non-PHI variable allowlist** validated fail-closed at config load. **Transport caveat, stated plainly — and this path is now the ONLY one left:** `send_plain_email` calls `smtp.starttls()` with **no SSL context**, so Python's stdlib default applies (`ssl._create_stdlib_context` **is** `ssl._create_unverified_context` — `check_hostname = False`, `verify_mode = CERT_NONE`). The hop is therefore **encrypted but unauthenticated (MITM-able)** when `email_use_tls` is true (the default), and cleartext when it is false. There is **no hop gradient or attestation on this path**. ⚠️ **Do not generalise this row to the message connectors.** [#323](BACKLOG.md) fixed the **EMAIL** and **DIRECT** *message destinations* — both now build an explicit verifying context (chain + hostname + strict RFC 5280, anchored to the OS roots, a per-connection `tls_ca_file`, or `[tls].internal_ca_file`), with `tls_verify=false` refused unless the **clamped** escape permits it. The `[alerts]` cell was deliberately deferred because it needs an acknowledgment switch rather than that clamp (the contextvar hop posture is never stamped here), so it is the residual on #323 — not an oversight, and not evidence that SMTP is unverified engine-wide | -| **12. Per-user security-event SMTP notifier** — **posture-mandatory on a PHI instance** | `account_locked`, `login_after_failures`, `password_changed`, `password_reset`, `email_changed`, `roles_changed`, `account_disabled`, `mfa_enabled`, `mfa_disabled`, `admin_action_new_ip` | plain-text email | the **affected user's own** mailbox | ASVS 6.3.5 / 6.3.7 out-of-band notification of security-relevant account changes | shares stream 11's SMTP transport and therefore its caveat. On a PHI instance with auth enabled `serve` **refuses to start (exit 2) under `[security].enforcement = enforce`** when no effective channel exists; the explicit, **audited** opt-out is `[alerts].security_notifications_required = false` | the mail system's | the body carries the account username, a fixed description, optionally the failed-attempt count or the new email on file, and the source IP — **no message data, no secrets**. Dispatch is a bounded background queue; a failed send is logged, never raised (the event is still in `audit_log`) | +| **11. `[alerts]` SMTP transport — operator alert list** (off unless `email_smtp_host` + `email_from` + ≥1 `email_to`) | one email per alert; default subject `[MessageFoundry] — `, default body every non-underscore event key as `k: v` | plain text (always kept — never HTML-only); optional HTML alternative | the operators' mailboxes | operator notification | `smtp_allowed_hosts` egress allowlist; the SMTP password comes from `MEFOR_ALERTS_EMAIL_PASSWORD` or a `[secrets]` provider, never the config file; per-send timeout `email_timeout` | the mail system's | carries the same `detail`/`reason` free text as the webhook. #138 operator templates are constrained to a **closed non-PHI variable allowlist** validated fail-closed at config load. **Transport posture:** `send_plain_email` builds an explicit **verifying** context (chain + hostname + strict RFC 5280, TLS 1.2 floor) via `tls_policy.build_smtp_tls_context()` and passes it to `starttls()`, anchored to the OS roots, `[alerts].email_tls_ca_file`, or `[tls].internal_ca_file` — the same factory the EMAIL and DIRECT *message destinations* use, so all three SMTP cells now share one policy ([#323](BACKLOG.md), closed 2026-08-02). Before that this call passed **no** context and Python's stdlib default applied (`ssl._create_stdlib_context` **is** `ssl._create_unverified_context` — `CERT_NONE`, `check_hostname = False`), leaving the hop encrypted but unauthenticated. There is still **no hop gradient or attestation on this path** — unlike the connectors, this cell is constructed outside the `active_hop_posture` scope, so its deviations (`email_use_tls = false`, or `email_tls_verify = false`) are gated by a `[security].allow_unverified_alert_smtp_tls` **acknowledgment switch at the serve gate** rather than by the clamped escape: on an enforcing PHI instance `serve` refuses to start without it, and permits + `AUDIT`-logs the start with it. Both deviations are named by `security_loosenings()` and reported by `messagefoundry check`'s `alert-smtp-tls` advisory | +| **12. Per-user security-event SMTP notifier** — **posture-mandatory on a PHI instance** | `account_locked`, `login_after_failures`, `password_changed`, `password_reset`, `email_changed`, `roles_changed`, `account_disabled`, `mfa_enabled`, `mfa_disabled`, `admin_action_new_ip` | plain-text email | the **affected user's own** mailbox | ASVS 6.3.5 / 6.3.7 out-of-band notification of security-relevant account changes | shares stream 11's SMTP transport and therefore its verifying context and its `[alerts].email_tls_*` knobs — note this is a **separate call site** (`pipeline/security_notify.py`), plumbed in its own right rather than inheriting by accident. On a PHI instance with auth enabled `serve` **refuses to start (exit 2) under `[security].enforcement = enforce`** when no effective channel exists; the explicit, **audited** opt-out is `[alerts].security_notifications_required = false` | the mail system's | the body carries the account username, a fixed description, optionally the failed-attempt count or the new email on file, and the source IP — **no message data, no secrets**. Dispatch is a bounded background queue; a failed send is logged, never raised (the event is still in `audit_log`) | | **13. `LoggingAlertSink` fallback** (when no `[alerts]` transport is configured) | every alert **this state-less sink implements**, at `WARNING` — `leadership_lost` / `dr_released` at `INFO`, and `connection_restored` is a **deliberate no-op** (a recovery needs no page and there is no instance to auto-resolve), so a lane recovery produces no record on this stream at all. `content_match` exists only on `NotifierAlertSink` and has no fallback-path record | — | folds into stream 1 | so alerts are never silent | inherits stream 1's | inherits stream 1's | includes the `detail`/`reason` free text, and therefore inherits stream 1's filters, ACL, forwarder and retention | | **14. `messagefoundry support-bundle` archive** (operator-invoked CLI, never automatic) | `app-log.txt` — the trailing **500** lines (`DEFAULT_LOG_TAIL_LINES`) of the configured app log — plus a secret-free `config-summary.json` (counts/names only) and a metadata-only `status.json` | text members inside a `.zip` | the operator-supplied `--out` path — **outside** the store and outside the NSSM DataDir ACL | hand-off to support: this stream exists precisely to leave the box | **none once written.** Filesystem permissions on wherever `--out` points are the only control; the CLI carries no RBAC and writes no audit row | **none** — never swept by `[retention].app_log_days` or anything else; the operator owns the file | Inherits stream 1's residual and passes a **fourth** redactor, `support/redact.py::redact_log_line` — **not** the three handler filters. Treat a bundle as a copy of stream 1, at stream 1's PHI class | diff --git a/docs/SECURITY-LOOSENING.md b/docs/SECURITY-LOOSENING.md index 7c8317f4..444a78f6 100644 --- a/docs/SECURITY-LOOSENING.md +++ b/docs/SECURITY-LOOSENING.md @@ -53,6 +53,7 @@ section reference. | Sign-in & identity | `require_sign_in` | `true` | | | `require_mfa` | `true` | | | `allow_single_factor_admin_when_exposed` | `false` | +| Alert transport | `allow_unverified_alert_smtp_tls` | `false` | | | `sign_out_after_idle_minutes` | `30` | | | `max_session_hours` | `12` | | Data handling | `block_unlisted_outbound` | `true` | @@ -251,6 +252,27 @@ trail. strict enforcement is a deliberate, slight **tightening** ([ADR 0140](adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md); the ack was renamed from `allow_unencrypted_phi_in_production` by [ADR 0148](adr/0148-phi-default-posture-and-an-explicit-security-enforcement-level.md) when the dial moved from the `production` tier to `enforcement`). +### `allow_unverified_alert_smtp_tls = true` — permit an unauthenticated `[alerts]` SMTP hop +- **What you lose:** the hop carrying operator alert bodies, **every per-user security-event email** + (lockout, password/roles change, new-IP admin action) and the SMTP login credential no longer + authenticates the mail relay. It covers **both** unauthenticated shapes: `[alerts].email_use_tls = false` + (cleartext) and `[alerts].email_tls_verify = false` (encrypted, but any certificate is accepted). An + on-path attacker who can answer for the relay reads all of it — and, since stream 12 is the ASVS + 6.3.5/6.3.7 out-of-band channel, can also *deny* a user the notice that their account was just taken over. +- **When acceptable:** a lab/dev relay with a self-signed certificate you cannot re-issue, on a trusted + network. Prefer pointing `[alerts].email_tls_ca_file` or `[tls].internal_ca_file` at that relay's CA — + that keeps verification on and needs no deviation at all. +- **Compensating controls:** trusted-network placement; a relay on the same host; a startup **AUDIT** line + records the override, `security_loosenings()` names it, and `messagefoundry check`'s `alert-smtp-tls` + advisory prints the hop's posture and whether it is acknowledged. +- **Why an acknowledgment and not the clamped escape:** the EMAIL/DIRECT connectors key their verify-off + refusal on `MEFOR_ALLOW_INSECURE_TLS` read through the **clamped** + `weakened_tls_escape_permitted_here()`, which reads the construction-time hop posture. The alerts + notifier is built in the API lifespan, **outside** `build_check_registry`'s `active_hop_posture` scope, + where that clamp degrades to the *unclamped* escape and would provide no refusal at all. So this cell + gets an explicit `[security]` acknowledgment instead — the first verify-off hop governed that way. +- **Still refused:** nothing here relaxes the connectors. This switch reaches the `[alerts]` cell only. + ### `enforcement = warn` — warn instead of refuse on the PHI serve-gate floor - **What you lose:** the serve-gate **refuse/warn dial** flips from *refuse* to *warn-and-continue*, and the [ADR 0092](adr/0092-posture-keyed-transport-hop-refusal-refuse-the-insecure-phi-hop.md) blunt escapes @@ -373,9 +395,12 @@ trail. invisible — an accepted risk that stops being visible has stopped being accepted and started being forgotten. It also cannot relax a hop ADR 0153 does not govern: inbound binds are still decided by the exposed-gates, and **revocation / weakened-TLS (`verify_tls = false`) refusals are unaffected** — a - verify-off hop is encrypted-but-unauthenticated, not cleartext, so it keeps the clamped - `MEFOR_ALLOW_INSECURE_TLS` escape and this declaration does not reach it. Nor does it reach an SMTP - `AUTH` over cleartext, which is refused outright. + verify-off hop is encrypted-but-unauthenticated, not cleartext, so this declaration does not reach it. + At least the connector verify-off cells keep the clamped `MEFOR_ALLOW_INSECURE_TLS` escape; the + `[alerts]` SMTP hop is governed instead by the `allow_unverified_alert_smtp_tls` acknowledgment above + (its construction sits outside the posture scope the clamp reads), so "the clamped escape" is not a + universal statement about verify-off hops and should not be read as one. Nor does this declaration + reach an SMTP `AUTH` over cleartext, which is refused outright. --- diff --git a/docs/adr/0029-email-smtp-destination.md b/docs/adr/0029-email-smtp-destination.md index e6ddb2ee..1cfafcb4 100644 --- a/docs/adr/0029-email-smtp-destination.md +++ b/docs/adr/0029-email-smtp-destination.md @@ -31,6 +31,34 @@ --- +## Amendment 2026-08-02 — the SMTP TLS posture this ADR describes is now VERIFIED on all three cells + +D3 below says the connector takes "the same posture `send_plain_email` already takes". That sentence was +written when both cells passed **no** SSL context to `starttls()` — which meant both fell back to +`ssl._create_stdlib_context`, and that **is** `ssl._create_unverified_context` (`CERT_NONE`, +`check_hostname=False`). So "STARTTLS by default" was, for the whole life of this ADR, encryption without +authentication: an on-path attacker presenting any certificate read the Handler payload and the SMTP +credential. + +[BACKLOG #323](../BACKLOG.md) closed that in two steps, and the sentence is true again — but for a +different reason than it was written for: + +- **Layers 1–2 (PR #132)** gave `EmailDestination` / `DirectDestination` an explicit verifying context + from `tls_policy.build_smtp_tls_context()`, with per-connection `tls_verify` / `tls_ca_file` / + `tls_check_hostname`, refused against the **clamped** `weakened_tls_escape_permitted_here()`. +- **Layer 3** gave the `[alerts]` cell the same factory, from `[alerts].email_tls_verify` / + `email_tls_ca_file`. That cell is gated differently on purpose: it is constructed outside + `build_check_registry`'s `active_hop_posture` scope, where the connectors' clamp reads a `None` posture + and degrades to the *unclamped* escape — i.e. would provide no refusal at all. It is therefore governed + by a `[security].allow_unverified_alert_smtp_tls` **acknowledgment switch at the serve gate**, which + refuses an enforcing PHI instance whose alert hop does not authenticate the relay. + +**D3's escape wording is also narrower than it reads.** It names `insecure_tls_allowed()` for the +cleartext arm; that raw call was replaced by the clamped reader in both connectors, and the `[alerts]` +cell never consulted either — its cleartext arm was ungated until layer 3's serve gate covered it. + +--- + ## Context MessageFoundry has no email transport. A common integration ask — "fan a result/alert out as an email to a diff --git a/messagefoundry/__main__.py b/messagefoundry/__main__.py index 628579eb..55729857 100644 --- a/messagefoundry/__main__.py +++ b/messagefoundry/__main__.py @@ -1465,7 +1465,9 @@ def _serve(args: argparse.Namespace) -> int: # cleartext-hop list. That is not a silent subset: every ADR 0153 acceptance is reported moments # later — per connection, with its reason — by the construction gate's own loud WARN + audit record, # and completely by `messagefoundry check` and GET /security/posture, which both have the graph. - _loosenings = security_loosenings(settings.security, settings.store, settings.auth, ()) + _loosenings = security_loosenings( + settings.security, settings.store, settings.auth, settings.alerts, () + ) if _loosenings: _seclog = logging.getLogger(__name__) _seclog.warning( @@ -2164,6 +2166,70 @@ def _serve(args: argparse.Namespace) -> int: file=sys.stderr, ) + # --- #323 layer 3: the alerts / security-event SMTP hop must AUTHENTICATE the relay ------------- + # The [alerts] SMTP transport carries operator alert bodies and every per-user security-event email + # (lockout, password/email/roles change, new-IP admin action) — and the SMTP AUTH password. Before + # #323 that hop called starttls() with NO context, so smtplib's fallback (ssl._create_stdlib_context, + # which IS _create_unverified_context) accepted ANY certificate: encrypted, unauthenticated, and an + # on-path attacker read all of it. The connectors (EMAIL/DIRECT, layers 1-2) key their refusal on the + # CLAMPED weakened_tls_escape_permitted_here(), but that mechanism is INERT here — this notifier is + # built in the API lifespan, outside build_check_registry's active_hop_posture scope, so + # current_hop_posture() is None and the clamp degrades to the unclamped escape. Hence an explicit + # acknowledgment switch at this gate instead, in the shape of the keyless-PHI second ack (ADR 0140). + # + # IT COVERS BOTH UNAUTHENTICATED SHAPES, DELIBERATELY. email_use_tls=false (no TLS at all) is + # strictly worse than email_tls_verify=false (TLS that authenticates nothing), and it was previously + # ungated. A gate that refused only the second would hand an operator a bypass that lands them on + # the WORSE posture — so the condition is "this hop does not authenticate the relay", which is true + # of both. Strictly ADDS refusals (ADR 0092 decision 5); byte-identical on the shipped defaults, + # which verify. + # + # Gated on a CONFIGURED transport: with no email_smtp_host/email_from there is no hop to protect, + # and the #188 gate above already owns the "no channel at all" case. + if ( + data_class is DataClass.PHI + and settings.alerts.email_smtp_host + and settings.alerts.email_from + ): + if not settings.alerts.email_use_tls: + hop_desc = "[alerts].email_use_tls=false (the SMTP hop is CLEARTEXT)" + elif not settings.alerts.email_tls_verify: + hop_desc = "[alerts].email_tls_verify=false (the SMTP hop verifies no certificate)" + else: + hop_desc = "" + if hop_desc: + if enforcing and not settings.security.allow_unverified_alert_smtp_tls: + print( + f"error: {hop_desc} on a {'production ' if production else ''}PHI instance " + f"({env_name!r}); refusing to start — operator alert bodies, every per-user " + "security-event email, and the SMTP AUTH password would cross a hop that does not " + "authenticate the relay, so an on-path attacker can read them. Remove the override " + "(the default verifies), or point [alerts].email_tls_ca_file / [tls].internal_ca_file " + "at the relay's CA; or set [security].allow_unverified_alert_smtp_tls=true to " + "deliberately accept an unauthenticated alert hop (audited).", + file=sys.stderr, + ) + return 2 + if enforcing: + # Explicitly acknowledged under strict enforcement — a loud WARNING-level AUDIT line + # (captured by NSSM stdout/SIEM), then the shared warn posture below. Never silent. + logging.getLogger(__name__).warning( + "AUDIT: starting a %sPHI instance (environment %r) with %s, permitted because " + "[security].allow_unverified_alert_smtp_tls=true — alert bodies, security-event " + "email and the SMTP AUTH credential cross an UNAUTHENTICATED hop " + "(alert-SMTP-TLS verification opt-out override).", + "production " if production else "", + env_name, + hop_desc, + ) + print( + f"warning: {hop_desc} in a PHI-carrying environment ({env_name!r}) — alert bodies, " + "per-user security-event email and the SMTP AUTH password cross a hop that does not " + "authenticate the relay (MITM-able). Remove the override, or trust the relay's CA via " + "[alerts].email_tls_ca_file / [tls].internal_ca_file.", + file=sys.stderr, + ) + # --- ADR 0152 rung 2: in-USE PHI protection (ASVS 11.7.1) ------------------------------------ # Placed LAST in the posture ladder on purpose (extend, never weaken): every more SPECIFIC # refusal — cleartext bind, revocation, Posture-B, /ui exposure, MFA-at-exposure, retention, @@ -4304,6 +4370,7 @@ def _security(args: argparse.Namespace) -> int: from messagefoundry.config import security_edit from messagefoundry.config.settings import ( + AlertsSettings, AuthSettings, SecuritySettings, StoreSettings, @@ -4319,14 +4386,14 @@ def _security(args: argparse.Namespace) -> int: # the shipped defaults and SAY SO via the emitted `loosenings_partial` marker, rather than silently # reporting a subset as if it were everything. _loosenings_partial = False - _store, _auth = StoreSettings(), AuthSettings() + _store, _auth, _alerts = StoreSettings(), AuthSettings(), AlertsSettings() if Path(path).exists(): # An ABSENT file is not a degraded read — the shipped defaults ARE the effective posture there, # and `security show` is expected to work offline before any config exists. Only a file that # exists and will not resolve is partial. try: _full = load_settings(config_path=path) - _store, _auth = _full.store, _full.auth + _store, _auth, _alerts = _full.store, _full.auth, _full.alerts except (ValidationError, tomllib.TOMLDecodeError, OSError, ValueError): # The specific ways a settings file fails to resolve: a schema/cross-field violation, # malformed TOML, an unreadable path, and the plain ValueErrors load_settings raises for a @@ -4339,7 +4406,10 @@ def _loosenings(sec: SecuritySettings) -> list[dict[str, str]]: # 0153 per-connection cleartext_accepted declarations — it passes an empty list and declares the # gap in `loosenings_scope` below, instead of reporting a settings-only view as if it were the # whole posture. `messagefoundry check` and GET /security/posture are the complete surfaces. - return [{"switch": s, "risk": r} for s, r in security_loosenings(sec, _store, _auth, ())] + return [ + {"switch": s, "risk": r} + for s, r in security_loosenings(sec, _store, _auth, _alerts, ()) + ] #: Emitted alongside every loosening list this subcommand prints, so a reader can never mistake a #: degraded or settings-only report for a complete one. `partial` means [store]/[auth] could not be @@ -4347,7 +4417,7 @@ def _loosenings(sec: SecuritySettings) -> list[dict[str, str]]: _loosenings_scope = { "loosenings_partial": _loosenings_partial, "loosenings_scope": ( - "settings only ([security]/[store]/[auth]); per-connection cleartext_accepted " + "settings only ([security]/[store]/[auth]/[alerts]); per-connection cleartext_accepted " "declarations are NOT included — see `messagefoundry check` or GET /security/posture" ), } diff --git a/messagefoundry/api/app.py b/messagefoundry/api/app.py index 4cba84d7..7c78d63c 100644 --- a/messagefoundry/api/app.py +++ b/messagefoundry/api/app.py @@ -1492,6 +1492,10 @@ async def security_posture( # [store]/[auth] carry posture switches too (ADR 0148: one posture, loosen only), so the registry # needs them to report a COMPLETE list. Same stash-or-default pattern as `store` above. auth_settings = getattr(request.app.state, "auth_settings", None) or AuthSettings() + # #323 layer 3: [alerts] carries the SMTP-hop deviation (cleartext / verification off). Same + # stash-or-default pattern — settings-scoped, so this route reports it completely even with no + # graph loaded, unlike the connection-scoped cleartext_accepted set below. + alerts_settings = getattr(request.app.state, "alerts_settings", None) or AlertsSettings() # ADR 0153: the ONE connection-scoped deviation. Read LIVE off the running graph (so a reload is # reflected) — this route is where an operator learns a cleartext hop is being crossed by # declaration, and a stale or absent list would understate the posture. An engine with no @@ -1514,7 +1518,9 @@ async def security_posture( ) loosenings = [ SecurityLoosening(switch=name, risk=risk) - for name, risk in security_loosenings(security, store, auth_settings, cleartext_hops) + for name, risk in security_loosenings( + security, store, auth_settings, alerts_settings, cleartext_hops + ) ] synthetic_relaxation = ( "strict PHI-only controls (at-rest-encryption refusal, deny-by-default egress, bounded " @@ -2522,6 +2528,18 @@ async def test_alert_email( subject_template=alerts.email_subject_template, body_template=alerts.email_body_template, html_template=alerts.email_html_template, + # #323 layer 3: the test send MUST use the same TLS posture as a real alert, or this + # diagnostic passes against a relay that live alerts would refuse (or vice versa) — a + # compensating control resting on a false premise, which is the defect class #323 is + # about. Same fields, same source, same factory. + tls_verify=alerts.email_tls_verify, + tls_ca_file=alerts.email_tls_ca_file, + trust_anchor_policy=( + tls_settings.policy() + if (tls_settings := getattr(request.app.state, "tls_settings", None)) + is not None + else None + ), ) # A fixed synthetic event — carries only a type/severity/connection label + a static detail # string; NO message body, NO PHI. The template-value allowlist maps these safely too. @@ -5332,7 +5350,13 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: resolve_secret_provider(secrets_settings) if secrets_settings is not None else None ) notifier = ( - notifier_from_settings(alerts_settings, secret_provider=secret_provider) + notifier_from_settings( + alerts_settings, + secret_provider=secret_provider, + # #323 layer 3: the instance [tls] internal-CA policy reaches the alerts SMTP hop too, so + # an estate on a private CA needs no per-alert CA path. + trust_anchor_policy=tls_settings.policy() if tls_settings else None, + ) if alerts_settings is not None else None ) @@ -5512,6 +5536,9 @@ async def _alert_control(action: str, target: str) -> None: # email_password_secret reference (fail-closed) exactly as notifier_from_settings does. None on # the embedded/test path — then only a plain env-sourced email_password can be tested. app.state.secret_provider = secret_provider + # #323 layer 3: expose the [tls] trust-anchor policy so POST /alerts/test-email builds its + # EmailTransport with the SAME anchors as the live notifier. None on the embedded/test path. + app.state.tls_settings = tls_settings app.state.service_settings = service_settings # back GET /service/status (L6a) app.state.log_dir = log_dir # back GET /status app-log metering (#50) app.state.approval_gate = _build_approval_gate( @@ -5561,7 +5588,9 @@ async def _audit_upload_prune(meta: UploadedFileMeta) -> None: # after the engine in the finally below). if auth_settings.notify_security_events and alerts_settings is not None: security_notifier = security_notifier_from_settings( - alerts_settings, secret_provider=secret_provider + alerts_settings, + secret_provider=secret_provider, + trust_anchor_policy=tls_settings.policy() if tls_settings else None, ) if security_notifier is not None: security_notifier.start() diff --git a/messagefoundry/checks.py b/messagefoundry/checks.py index 9cf71301..2a1c084f 100644 --- a/messagefoundry/checks.py +++ b/messagefoundry/checks.py @@ -47,6 +47,11 @@ So is ``accepts-candidate`` — an AST scan that flags a ``@handler`` opening with a guard-filter (``if : return []``), a filter that belongs in an ``accepts=`` router-stage predicate (ADR 0084) where it costs 0 transactions instead of 2; also advisory (prints, never blocks). +So is ``alert-smtp-tls`` (#323 layer 3) — it reports whether the ``[alerts]`` SMTP hop AUTHENTICATES the +relay, naming the trust anchor when it does and the acknowledgment state when it does not. Advisory +because the **serve gate** is what refuses an unauthenticated alert hop on an enforcing PHI instance; +this only makes the hop's posture readable in review. It states the secure case out loud rather than +going quiet, so a passing line is never confused with a check that did not run. Exit-code policy lives in the CLI (``__main__._check``): 0 iff no required check failed. """ @@ -155,6 +160,13 @@ def run_checks( # ADR 0153: name every outbound that declares cleartext_accepted, so the accepted set is visible # in review rather than discoverable only by reading each connection. Advisory — see the check. _check_cleartext_accepted(config_dir), + # #323 layer 3: report whether the [alerts] SMTP hop authenticates the relay. The defect this + # closes was invisible for exactly as long as nothing reported it. Advisory — see the check. + _check_alert_smtp_tls( + config_dir, + service_config=service_config, + suppress_search=suppress_service_toml_search, + ), ] if run_lint: results.append(_run_tool("ruff", ["ruff", "check", str(config_dir)])) @@ -1318,6 +1330,109 @@ def _check_build( ) +def _check_alert_smtp_tls( + config_dir: str | Path, + *, + service_config: str | Path | None = None, + suppress_search: bool = False, +) -> CheckResult: + """Surface whether the ``[alerts]`` SMTP hop **authenticates the relay** (#323 layer 3). + + That hop carries operator alert bodies, every per-user security-event email, and the SMTP AUTH + password. Before #323 it issued ``starttls()`` with no context, so it accepted any certificate — + encrypted, unauthenticated, and invisible: nothing in the product reported it, which is why the + defect survived in three docs that described the hop as verified. This check is the review surface + that makes the hop's posture readable without opening the code. + + Advisory (``required=False``) for the same reason as ``cleartext-accepted``: a deliberate, + acknowledged deviation is a reasoned choice, not a config error, and the **serve gate** is what + refuses it on an enforcing PHI instance. Blocking here would duplicate that refusal at the wrong + altitude and punish a synthetic/dev instance where the deviation is legitimate. + + States the SECURE case explicitly rather than going quiet — an absent line is indistinguishable + from a check that did not run. + + Service-toml resolution is :func:`_check_posture`'s, verbatim; no ``messagefoundry.toml`` or + settings that will not load → SKIP (``validate``/``posture`` report those).""" + from pydantic import ValidationError + + from messagefoundry.config.settings import load_settings + + if service_config is not None: + toml: Path | None = Path(service_config) if Path(service_config).is_file() else None + elif suppress_search: + candidate = Path(config_dir) / "messagefoundry.toml" + toml = candidate if candidate.is_file() else None + else: + toml = _find_service_toml(config_dir) + if toml is None: + return CheckResult( + "alert-smtp-tls", ok=True, required=False, skipped=True, detail="no messagefoundry.toml" + ) + try: + settings = load_settings(config_path=toml) + except (FileNotFoundError, ValueError, ValidationError, OSError) as exc: + return CheckResult( + "alert-smtp-tls", + ok=True, + required=False, + skipped=True, + detail=f"settings did not load: {exc}", + ) + alerts = settings.alerts + if not (alerts.email_smtp_host and alerts.email_from): + return CheckResult( + "alert-smtp-tls", + ok=True, + required=False, + detail="no [alerts] SMTP transport configured — no hop to report", + ) + acked = settings.security.allow_unverified_alert_smtp_tls + ack_note = ( + " — acknowledged by [security].allow_unverified_alert_smtp_tls" + if acked + else " — NOT acknowledged; an enforcing PHI instance will REFUSE to start" + ) + if not alerts.email_use_tls: + return CheckResult( + "alert-smtp-tls", + ok=True, + required=False, + detail=( + f"[alerts].email_use_tls=false — the SMTP hop to {alerts.email_smtp_host} is " + f"CLEARTEXT{ack_note}" + ), + ) + if not alerts.email_tls_verify: + return CheckResult( + "alert-smtp-tls", + ok=True, + required=False, + detail=( + f"[alerts].email_tls_verify=false — the SMTP hop to {alerts.email_smtp_host} is " + f"encrypted but accepts ANY certificate{ack_note}" + ), + ) + anchor = ( + f"[alerts].email_tls_ca_file ({alerts.email_tls_ca_file})" + if alerts.email_tls_ca_file + else ( + f"[tls].internal_ca_file ({settings.tls.internal_ca_file})" + if settings.tls.internal_ca_file + else "the OS trust store" + ) + ) + return CheckResult( + "alert-smtp-tls", + ok=True, + required=False, + detail=( + f"the [alerts] SMTP hop to {alerts.email_smtp_host} verifies the relay certificate " + f"against {anchor}" + ), + ) + + def _check_cleartext_accepted( config_dir: str | Path, ) -> CheckResult: diff --git a/messagefoundry/config/settings.py b/messagefoundry/config/settings.py index 49752885..321c49df 100644 --- a/messagefoundry/config/settings.py +++ b/messagefoundry/config/settings.py @@ -2775,6 +2775,16 @@ class AlertsSettings(_Section): email_from: str | None = None email_to: list[str] = [] email_use_tls: bool = True # STARTTLS + # #323 layer 3: whether that STARTTLS hop VERIFIES the relay's certificate. True (the default) builds + # an explicit verifying context via tls_policy.build_smtp_tls_context; before #323 there was no + # context at all and smtplib's fallback verified NOTHING (CERT_NONE, check_hostname=False), so the + # hop was encrypted but unauthenticated. FALSE is an audited loosening: it is named by + # security_loosenings() and REFUSED at the serve gate on an enforcing PHI instance unless + # [security].allow_unverified_alert_smtp_tls is also set. Only meaningful when email_use_tls=true. + email_tls_verify: bool = True + # PEM bundle of trust anchors for that hop. None (the default) = the [tls] internal-CA policy if one + # is configured, else the OS trust store. A path, not a secret — same status as [tls].internal_ca_file. + email_tls_ca_file: str | None = None email_username: str | None = None email_password: str | None = None # secret — supply via MEFOR_ALERTS_EMAIL_PASSWORD # Connector SecretProvider reference (ADR 0019 §5, BACKLOG #196). When set AND [secrets].provider is @@ -3573,6 +3583,21 @@ class SecuritySettings(_Section): # enforcement=warn keeps it a warning even when this is set. Not a loosening (it tightens). require_memory_encryption_declaration: bool = False + # ── Outbound alert email (data in transit) ─────────────────────── + # #323 layer 3: the SECOND acknowledgment required to run the alerts / security-event SMTP hop with + # certificate verification OFF ([alerts].email_tls_verify=false) on an ENFORCING PHI instance. + # AN ACKNOWLEDGMENT SWITCH, NOT THE CLAMP — and the difference is the whole reason this cell shipped + # separately from the EMAIL/DIRECT connectors. Those are built inside build_check_registry's + # `active_hop_posture` scope, so they read the CLAMPED weakened_tls_escape_permitted_here() and an + # enforcing PHI hop can never be relaxed. The alerts notifier is constructed in the API lifespan, + # OUTSIDE that scope (measured: the contextvar is stamped only in pipeline/wiring_runner.py), where + # current_hop_posture() is None and the clamp degrades to the UNCLAMPED escape — i.e. the connectors' + # mechanism would silently provide no refusal at all here. So the refusal is keyed on this explicit + # switch at the serve gate instead, in the shape of allow_unencrypted_phi_under_strict_enforcement. + # Default FALSE and byte-identical when unset. Setting it TRUE is a LOOSENING: security_loosenings() + # names it, so the opt-out is never silent. + allow_unverified_alert_smtp_tls: bool = False + # ── Sign-in & identity ─────────────────────────────────────────── require_sign_in: bool = True # authenticate every request require_mfa: bool = True # second factor, enforced as an ACCESS gate (ASVS 6.3.3) @@ -3971,6 +3996,7 @@ def security_loosenings( sec: SecuritySettings, store: StoreSettings, auth: AuthSettings, + alerts: AlertsSettings, cleartext_hops: Sequence[str], ) -> list[tuple[str, str]]: """The ``[security]`` switches at their INSECURE value, plus the enumerated deviations outside that @@ -3980,7 +4006,8 @@ def security_loosenings( ``[security]`` switch — pinned by a completeness floor in ``tests/test_security_posture_defaults.py`` that iterates ``SecuritySettings.model_fields`` and fails on an unreported, unexempted one — plus an ENUMERATED set of deviations that live elsewhere: ``[store].aad_bind``, - ``[auth].ad_session_recheck_seconds``, and the per-connection ``cleartext_accepted``. It is NOT yet + ``[auth].ad_session_recheck_seconds``, ``[alerts].email_use_tls``/``email_tls_verify`` (#323 + layer 3), and the per-connection ``cleartext_accepted``. It is NOT yet an exhaustive registry of every security-relevant switch in every section; ``[store]``/``[auth]`` carry others (``encrypt``, ``trust_server_certificate``, ``enabled``, ``require_mfa``, ``ad_tls_verify``, ``ad_allow_insecure_ldap``, ``oidc_require_mfa_claim``, @@ -4139,6 +4166,39 @@ def security_loosenings( "live engine sessions until they expire on their own", ) ) + # --- the [alerts] SMTP hop (#323 layer 3). Two SEPARATE entries, deliberately: the deviation and the + # acknowledgment of it are different facts and an operator can hold either without the other. A hop + # with verification off under enforcement=warn needs no acknowledgment to run, so keying the report + # on the switch alone would leave the actual weakening invisible — which is the failure mode this + # registry exists to prevent. Reported at every call site (unlike cleartext_accepted, this is + # settings-scoped, so `security show` and a graphless GET /security/posture see it completely). + if alerts.email_smtp_host and alerts.email_from: + if not alerts.email_use_tls: + out.append( + ( + "email_use_tls", + "the [alerts] SMTP hop is CLEARTEXT — operator alert bodies, every per-user " + "security-event email (lockout, password/roles change) and the SMTP login " + "credential cross it unencrypted and readable by anything on the path", + ) + ) + elif not alerts.email_tls_verify: + out.append( + ( + "email_tls_verify", + "the [alerts] SMTP hop is encrypted but UNAUTHENTICATED — it accepts any " + "certificate, so an on-path attacker presenting one reads the alert bodies, the " + "per-user security-event email and the SMTP login credential", + ) + ) + if sec.allow_unverified_alert_smtp_tls: + out.append( + ( + "allow_unverified_alert_smtp_tls", + "an unauthenticated [alerts] SMTP hop is permitted to start an enforcing PHI instance " + "— the serve gate that would otherwise refuse it is acknowledged away", + ) + ) # --- the one CONNECTION-scoped deviation (ADR 0153 decision 2). It is not a [security] switch, but # it is a declared departure from the one shipped posture, so it belongs in the one registry an # operator reads — a deviation the registry cannot see is a second posture by the back door. diff --git a/messagefoundry/pipeline/alert_sinks.py b/messagefoundry/pipeline/alert_sinks.py index ce70b4ed..550a787d 100644 --- a/messagefoundry/pipeline/alert_sinks.py +++ b/messagefoundry/pipeline/alert_sinks.py @@ -45,6 +45,7 @@ EscalationTier, insecure_tls_allowed, ) +from messagefoundry.config.tls_policy import TrustAnchorPolicy, build_smtp_tls_context __all__ = [ "AlertTransport", @@ -363,15 +364,52 @@ def send_plain_email( timeout: float = 30.0, allowed_hosts: tuple[str, ...] = (), html_body: str | None = None, + tls_verify: bool = True, + tls_ca_file: str | None = None, + trust_anchor_policy: TrustAnchorPolicy | None = None, ) -> None: """Send one email via SMTP (STARTTLS by default). Blocking — call via ``asyncio.to_thread``. Shared by :class:`EmailTransport` (ops alerts) and the per-user security-event notifier. An optional ``allowed_hosts`` egress allowlist gates the SMTP host. ``body`` (plain text) is ALWAYS the primary part; ``html_body`` (#138) adds an HTML **alternative** (``multipart/alternative``) when provided — - the message is never HTML-only.""" + the message is never HTML-only. + + **The STARTTLS hop is VERIFIED (#323, layer 3).** ``smtplib.starttls()`` takes no context by + default and falls back to :func:`ssl._create_stdlib_context`, which **is** + ``ssl._create_unverified_context`` — measured on CPython 3.14.6: ``verify_mode=CERT_NONE``, + ``check_hostname=False``. So this hop was encrypted but **unauthenticated**: an on-path attacker + presenting any certificate read the alert body and the SMTP AUTH credential. It now builds an + explicit verifying context through the same :func:`~messagefoundry.config.tls_policy. + build_smtp_tls_context` factory as the EMAIL and DIRECT connectors (layers 1–2), so all three SMTP + cells share one policy. + + ``tls_verify=False`` is the audited escape and is **not** gated here. This cell is built OUTSIDE + ``build_check_registry``'s ``active_hop_posture`` scope — measured: the contextvar is stamped only + in ``pipeline/wiring_runner.py`` — so the connectors' clamped + ``weakened_tls_escape_permitted_here()`` would fall back to the UNCLAMPED escape here and an + enforcing PHI instance would get no refusal at all. The refusal is therefore an acknowledgment + switch at the **serve gate** (``[security].allow_unverified_alert_smtp_tls``) instead, which is + also why layer 3 was deferred out of layers 1–2 rather than folded in. + + There is deliberately **no** ``SMTP_SSL`` arm: this cell has never had one, so + ``[alerts].email_smtp_port=465`` (implicit TLS) does not work here and is out of scope — see #323's + residual, which states it rather than fixing it silently.""" allowed = tuple(h.lower() for h in allowed_hosts) if allowed and host.lower() not in allowed: raise ValueError(f"SMTP host {host!r} is not in the configured allowlist") + # Built AFTER the allowlist check, deliberately: tests/test_alerts_test_email.py runs the REAL + # function and depends on a disallowed host raising before ANY other work happens. + tls_context = ( + build_smtp_tls_context( + host=host, + cell="alerts SMTP transport", + verify=tls_verify, + ca_file=tls_ca_file, + trust_anchor_policy=trust_anchor_policy, + ) + if use_tls + else None + ) msg = EmailMessage() msg["Subject"] = subject msg["From"] = sender @@ -381,7 +419,8 @@ def send_plain_email( msg.add_alternative(html_body, subtype="html") with smtplib.SMTP(host, port, timeout=timeout) as smtp: if use_tls: - smtp.starttls() + # context= is REQUIRED (#323): starttls()'s own default verifies NOTHING. + smtp.starttls(context=tls_context) if username is not None: smtp.login(username, password or "") smtp.send_message(msg) @@ -407,6 +446,9 @@ def __init__( subject_template: str | None = None, body_template: str | None = None, html_template: str | None = None, + tls_verify: bool = True, + tls_ca_file: str | None = None, + trust_anchor_policy: TrustAnchorPolicy | None = None, ) -> None: self.host = host self.port = port @@ -416,6 +458,11 @@ def __init__( self.username = username self.password = password self.timeout = timeout + # #323 layer 3: carried as PLAIN DATA, not a pre-built ssl.SSLContext, so this module and + # security_notify.py never name `ssl` and the single context factory stays in config/tls_policy.py. + self.tls_verify = tls_verify + self.tls_ca_file = tls_ca_file + self.trust_anchor_policy = trust_anchor_policy # Optional egress allowlist (lower-cased) for the SMTP host; empty = any (WP-11c, parity with # the webhook allowlist). The alert payload carries no PHI, so this is general egress control. self.allowed_hosts = tuple(h.lower() for h in allowed_hosts) @@ -469,6 +516,9 @@ def _send( timeout=self.timeout, allowed_hosts=self.allowed_hosts, html_body=html_body, + tls_verify=self.tls_verify, + tls_ca_file=self.tls_ca_file, + trust_anchor_policy=self.trust_anchor_policy, ) @@ -1114,7 +1164,10 @@ def configured_alert_transport_names(alerts: AlertsSettings) -> set[str]: def notifier_from_settings( - alerts: AlertsSettings, *, secret_provider: SecretProvider | None = None + alerts: AlertsSettings, + *, + secret_provider: SecretProvider | None = None, + trust_anchor_policy: TrustAnchorPolicy | None = None, ) -> NotifierAlertSink | None: """Build a :class:`NotifierAlertSink` from ``[alerts]`` settings, or ``None`` when no transport is configured (the caller then leaves the engine on its default logging sink). @@ -1126,7 +1179,11 @@ def notifier_from_settings( ``secret_provider`` (ADR 0019 §5) resolves the SMTP password from a ``[secrets].provider`` when ``email_password_secret`` is set (fail-closed); ``None``/no reference → the env-sourced - ``email_password`` is used, byte-identical to before.""" + ``email_password`` is used, byte-identical to before. + + ``trust_anchor_policy`` (#190, ADR 0093) supplies the instance ``[tls]`` internal-CA fallback to the + SMTP hop when ``[alerts].email_tls_ca_file`` names none of its own (#323 layer 3). It only chooses + WHICH roots verify the relay — it never turns verification off.""" smtp_password = resolve_connector_secret( secret_provider, ref=alerts.email_password_secret, @@ -1158,6 +1215,11 @@ def notifier_from_settings( subject_template=alerts.email_subject_template, body_template=alerts.email_body_template, html_template=alerts.email_html_template, + # #323 layer 3: the STARTTLS hop verifies by default. email_tls_verify=false is the + # audited escape, refused at the serve gate unless [security].allow_unverified_alert_smtp_tls. + tls_verify=alerts.email_tls_verify, + tls_ca_file=alerts.email_tls_ca_file, + trust_anchor_policy=trust_anchor_policy, ) ) # Fail loud at config time if a rule routes to a transport that isn't configured (a typo or a diff --git a/messagefoundry/pipeline/security_notify.py b/messagefoundry/pipeline/security_notify.py index 5cadf535..14309d5f 100644 --- a/messagefoundry/pipeline/security_notify.py +++ b/messagefoundry/pipeline/security_notify.py @@ -33,6 +33,7 @@ ) from messagefoundry.config.secretprovider import SecretProvider, resolve_connector_secret from messagefoundry.config.settings import AlertsSettings +from messagefoundry.config.tls_policy import TrustAnchorPolicy from messagefoundry.pipeline.alert_sinks import _BackgroundDispatcher, send_plain_email log = logging.getLogger(__name__) @@ -104,6 +105,9 @@ def __init__( password: str | None = None, timeout: float = 30.0, allowed_hosts: tuple[str, ...] = (), + tls_verify: bool = True, + tls_ca_file: str | None = None, + trust_anchor_policy: TrustAnchorPolicy | None = None, ) -> None: super().__init__() self._host = host @@ -114,6 +118,11 @@ def __init__( self._password = password self._timeout = timeout self._allowed_hosts = allowed_hosts + # #323 layer 3: the STARTTLS hop that carries a user's security-event mail now VERIFIES the + # relay's certificate. Carried as plain data — send_plain_email builds the one context. + self._tls_verify = tls_verify + self._tls_ca_file = tls_ca_file + self._trust_anchor_policy = trust_anchor_policy async def notify(self, event: SecurityEvent) -> None: # No deliverable address (common for local accounts / unset email) → nothing to email; the @@ -149,11 +158,17 @@ def _send(self, event: SecurityEvent) -> None: password=self._password, timeout=self._timeout, allowed_hosts=self._allowed_hosts, + tls_verify=self._tls_verify, + tls_ca_file=self._tls_ca_file, + trust_anchor_policy=self._trust_anchor_policy, ) def security_notifier_from_settings( - alerts: AlertsSettings, *, secret_provider: SecretProvider | None = None + alerts: AlertsSettings, + *, + secret_provider: SecretProvider | None = None, + trust_anchor_policy: TrustAnchorPolicy | None = None, ) -> SecurityEventNotifier | None: """Build the per-user security notifier from ``[alerts]`` SMTP settings, or ``None`` when no SMTP server/sender is configured (then only the ``/me/security-events`` feed records events). @@ -178,4 +193,7 @@ def security_notifier_from_settings( password=smtp_password, timeout=alerts.email_timeout, allowed_hosts=tuple(alerts.smtp_allowed_hosts), + tls_verify=alerts.email_tls_verify, + tls_ca_file=alerts.email_tls_ca_file, + trust_anchor_policy=trust_anchor_policy, ) diff --git a/scripts/security/crypto_inventory_check.py b/scripts/security/crypto_inventory_check.py index d38b935a..601af932 100644 --- a/scripts/security/crypto_inventory_check.py +++ b/scripts/security/crypto_inventory_check.py @@ -24,6 +24,26 @@ The walk-set (:data:`WALK_ROOTS`) is byte-identical to ``tests/test_security_static.py``'s ``_CRYPTO_ROOTS`` (BACKLOG #283 owns that pin; this gate consumes it). +**Adding a TLS/crypto call site? Read this first — it is cheaper than a red CI leg.** #323's opening +commit `e4728d7f` failed ``Tests (pytest)`` on all three OS legs by adding ``import ssl`` to +``transports/{email,direct}.py`` without an entry here; layer 3 then avoided the gate entirely rather +than feeding it. Four facts, each measured, that are not obvious from the code below: + +* **The gate is BIDIRECTIONAL, so "just register the file" is not a free fix.** An unregistered file + that imports a trigger fails one way (*undocumented crypto use*); a registered file that STOPS + importing it fails the other (*inventory lists ['ssl'] but the file no longer imports it*). A + registration is a standing commitment, not a one-time appeasement. +* **``if TYPE_CHECKING: import ssl`` does NOT hide the import.** Under ``from __future__ import + annotations`` the annotation still trips the scanner. There is no cheap way to keep the name and + dodge the gate — nor should there be. +* **The only real escape is not naming the type.** Hold the *inputs* (``tls_verify`` / ``tls_ca_file`` + / a ``TrustAnchorPolicy``) as plain data and let one inventoried builder produce the context into a + **bare local** with no annotation. Then no ``ssl`` name exists in the calling module at all. +* **For SMTP that builder already exists:** :func:`~messagefoundry.config.tls_policy.build_smtp_tls_context`. + All three SMTP cells (EMAIL, DIRECT, the ``[alerts]`` sink) route through it, so exactly one file is + registered here and the ``pipeline/`` call sites stay ``ssl``-free. That is centralization, not + evasion: one place decides the TLS policy for every SMTP hop in the product. + Stdlib only (no install), like ``scripts/security/scan_forbidden.py`` — runnable as a CI step and a pytest. Usage:: diff --git a/tests/test_alert_sinks.py b/tests/test_alert_sinks.py index 74025703..d37fd158 100644 --- a/tests/test_alert_sinks.py +++ b/tests/test_alert_sinks.py @@ -247,6 +247,10 @@ def __exit__(self, *a: object) -> None: def starttls(self, context: ssl.SSLContext | None = None) -> None: sent["tls"] = True + # #323 layer 3: RECORD the context. This fake accepted the kwarg from PR #132 onward but + # threw it away, so `sent["tls"] is True` below stayed green for the entire period the hop + # was accepting any certificate. "STARTTLS was issued" is not a security assertion. + sent["context"] = context def login(self, user: str, password: str) -> None: sent["login"] = (user, password) @@ -268,6 +272,12 @@ def send_message(self, msg: Any) -> None: asyncio.run(t.send({"type": "connection_stopped", "connection": "OB_X", "detail": "boom"})) assert sent["host"] == "smtp.example" assert sent["tls"] is True + # #323 layer 3: the hop must AUTHENTICATE the relay, not merely encrypt to it. Without a context + # smtplib falls back to ssl._create_stdlib_context, which IS _create_unverified_context + # (CERT_NONE / check_hostname=False) — so these two lines are the whole point of the fix. + assert sent["context"] is not None + assert sent["context"].verify_mode is ssl.CERT_REQUIRED + assert sent["context"].check_hostname is True assert sent["login"] == ("mf", "secret") assert "OB_X" in sent["subject"] assert sent["to"] == "ops@example, oncall@example" diff --git a/tests/test_alert_smtp_tls.py b/tests/test_alert_smtp_tls.py new file mode 100644 index 00000000..48a137d5 --- /dev/null +++ b/tests/test_alert_smtp_tls.py @@ -0,0 +1,396 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""BACKLOG #323 layer 3 — the ``[alerts]`` SMTP hop AUTHENTICATES the relay. + +``smtplib.starttls()`` takes no context by default and falls back to ``ssl._create_stdlib_context``, +which **is** ``ssl._create_unverified_context`` (``CERT_NONE``, ``check_hostname=False`` — measured on +CPython 3.14.6). So this hop was encrypted but unauthenticated: an on-path attacker presenting any +certificate read the alert bodies, every per-user security-event email, and the SMTP AUTH password. +Layers 1–2 (PR #132) fixed the EMAIL and DIRECT *connectors*; this is the third and last cell. + +**Every assertion here is a NEGATIVE CONTROL.** Stash the production change and each one goes red — the +failure mode is named in each test's comment. That discipline is the point: the pre-existing +``sent["tls"] is True`` assertion in ``test_alert_sinks.py`` stayed green throughout the insecure +period, and the fake had *already* been widened to accept ``context=`` by PR #132 without the +production code passing one. A test that passes both ways proves nothing. +""" + +from __future__ import annotations + +import asyncio +import ssl +from pathlib import Path +from typing import Any + +import pytest + +from messagefoundry.auth.notifications import ACCOUNT_LOCKED, SecurityEvent +from messagefoundry.config.settings import ( + AlertsSettings, + AuthSettings, + SecuritySettings, + StoreSettings, + security_loosenings, +) +from messagefoundry.pipeline.alert_sinks import EmailTransport +from messagefoundry.pipeline.security_notify import SecurityEventNotifier + + +class _RecordingSMTP: + """An SMTP fake that RECORDS the TLS context instead of discarding it.""" + + captured: dict[str, Any] = {} + + def __init__(self, host: str, port: int, timeout: float | None = None) -> None: + _RecordingSMTP.captured = {"host": host, "port": port, "context": None, "tls": False} + + def __enter__(self) -> _RecordingSMTP: + return self + + def __exit__(self, *a: object) -> None: + return None + + def starttls(self, context: ssl.SSLContext | None = None) -> None: + _RecordingSMTP.captured["tls"] = True + _RecordingSMTP.captured["context"] = context + + def login(self, user: str, password: str) -> None: + _RecordingSMTP.captured["login"] = (user, password) + + def send_message(self, msg: Any) -> None: + _RecordingSMTP.captured["subject"] = msg["Subject"] + + +@pytest.fixture +def smtp(monkeypatch: pytest.MonkeyPatch) -> type[_RecordingSMTP]: + monkeypatch.setattr("messagefoundry.pipeline.alert_sinks.smtplib.SMTP", _RecordingSMTP) + return _RecordingSMTP + + +# -------------------------------------------------------------------------------------------------- +# The two production call sites. These are SEPARATE code paths (alert_sinks.EmailTransport and +# security_notify.SecurityEventNotifier), so one passing does not imply the other. +# -------------------------------------------------------------------------------------------------- + + +def test_the_ops_alert_hop_verifies_the_relay_certificate(smtp: type[_RecordingSMTP]) -> None: + # WITHOUT THE FIX: context is None → red on the first assert. That is the whole defect. + t = EmailTransport(host="smtp.example", port=587, sender="mf@example", recipients=["ops@x"]) + asyncio.run(t.send({"type": "connection_stopped", "connection": "OB_X", "detail": "boom"})) + ctx = smtp.captured["context"] + assert ctx is not None, "starttls() was called with NO context — the hop verifies nothing" + assert ctx.verify_mode is ssl.CERT_REQUIRED + assert ctx.check_hostname is True + + +def test_the_security_event_hop_verifies_the_relay_certificate(smtp: type[_RecordingSMTP]) -> None: + # A DISTINCT call site from the one above — pipeline/security_notify.py builds its own transport, + # so the ops-alert test passing tells us nothing about this one. WITHOUT THE FIX: context is None. + n = SecurityEventNotifier(host="smtp.example", port=587, sender="mf@example") + n._send(SecurityEvent(event_type=ACCOUNT_LOCKED, username="dr.who", email="dr.who@example")) + ctx = smtp.captured["context"] + assert ctx is not None, "the security-event hop verifies nothing" + assert ctx.verify_mode is ssl.CERT_REQUIRED + assert ctx.check_hostname is True + + +def test_strict_x509_is_on_so_a_malformed_chain_is_rejected(smtp: type[_RecordingSMTP]) -> None: + # ASVS 12.1.4: harden_verify_flags ORs VERIFY_X509_STRICT into the context, so a presented chain + # must be RFC 5280-conformant. WITHOUT THE FIX there is no context to carry the flag. + t = EmailTransport(host="smtp.example", port=587, sender="mf@example", recipients=["ops@x"]) + asyncio.run(t.send({"type": "connection_stopped", "connection": "OB_X", "detail": "boom"})) + ctx = smtp.captured["context"] + assert ctx is not None + assert ctx.verify_flags & ssl.VERIFY_X509_STRICT + + +def test_tls_1_2_is_the_negotiated_floor(smtp: type[_RecordingSMTP]) -> None: + # NIST SP 800-52r2. WITHOUT THE FIX: no context, so no floor is asserted at all. + t = EmailTransport(host="smtp.example", port=587, sender="mf@example", recipients=["ops@x"]) + asyncio.run(t.send({"type": "connection_stopped", "connection": "OB_X", "detail": "boom"})) + assert smtp.captured["context"].minimum_version is ssl.TLSVersion.TLSv1_2 + + +# -------------------------------------------------------------------------------------------------- +# The audited escape still works, and is visibly different from the secure default. +# -------------------------------------------------------------------------------------------------- + + +def test_tls_verify_false_produces_an_unverified_context(smtp: type[_RecordingSMTP]) -> None: + # The escape is REAL — it must actually turn verification off, or an operator who set it would be + # running a posture nobody described. It is gated at the SERVE gate, not here (this cell is built + # outside the connectors' active_hop_posture scope, so their clamp is inert on it). + t = EmailTransport( + host="smtp.example", port=587, sender="mf@example", recipients=["ops@x"], tls_verify=False + ) + asyncio.run(t.send({"type": "connection_stopped", "connection": "OB_X", "detail": "boom"})) + ctx = smtp.captured["context"] + assert ctx is not None + assert ctx.verify_mode is ssl.CERT_NONE + assert ctx.check_hostname is False + + +def test_use_tls_false_passes_no_context_because_there_is_no_tls( + smtp: type[_RecordingSMTP], +) -> None: + t = EmailTransport( + host="smtp.example", port=587, sender="mf@example", recipients=["ops@x"], use_tls=False + ) + asyncio.run(t.send({"type": "connection_stopped", "connection": "OB_X", "detail": "boom"})) + assert smtp.captured["tls"] is False + assert smtp.captured["context"] is None + + +def test_a_connection_ca_file_is_loaded_as_the_only_anchor( + smtp: type[_RecordingSMTP], tmp_path: Path +) -> None: + # [alerts].email_tls_ca_file pins the relay's own CA. Assert it LOADED it: an empty cert store + # would mean the path was accepted and ignored, which looks identical from the outside. + ca = tmp_path / "relay-ca.pem" + ca.write_bytes(_self_signed_ca_pem()) + t = EmailTransport( + host="smtp.example", + port=587, + sender="mf@example", + recipients=["ops@x"], + tls_ca_file=str(ca), + ) + asyncio.run(t.send({"type": "connection_stopped", "connection": "OB_X", "detail": "boom"})) + ctx = smtp.captured["context"] + assert ctx is not None + assert ctx.verify_mode is ssl.CERT_REQUIRED + assert len(ctx.get_ca_certs()) == 1, "the CA file was accepted but no anchor was loaded" + + +# -------------------------------------------------------------------------------------------------- +# The loosening registry SEES the deviation. Without this the weakening is invisible to every operator +# surface — which is exactly how the original defect survived. +# -------------------------------------------------------------------------------------------------- + + +def _names(**kw: Any) -> list[str]: + alerts = AlertsSettings( + email_smtp_host="smtp.example", email_from="mf@example", **kw.pop("alerts", {}) + ) + sec = SecuritySettings(**kw.pop("security", {})) + return [n for n, _ in security_loosenings(sec, StoreSettings(), AuthSettings(), alerts, ())] + + +def test_the_shipped_alert_defaults_are_not_a_loosening() -> None: + assert "email_tls_verify" not in _names() + assert "email_use_tls" not in _names() + assert "allow_unverified_alert_smtp_tls" not in _names() + + +def test_verification_off_is_reported_even_without_the_acknowledgment() -> None: + # THE POINT OF A SEPARATE ENTRY: under enforcement=warn an operator can run verify-off with no + # acknowledgment at all. Keying the report on the ack switch alone would leave the actual + # weakening invisible. WITHOUT THE FIX: security_loosenings() cannot even see AlertsSettings. + assert "email_tls_verify" in _names(alerts={"email_tls_verify": False}) + + +def test_cleartext_alert_smtp_is_reported() -> None: + assert "email_use_tls" in _names(alerts={"email_use_tls": False}) + + +def test_the_acknowledgment_switch_is_itself_reported() -> None: + # Armed by the completeness floor in tests/test_security_posture_defaults.py, which iterates + # SecuritySettings.model_fields — this asserts the entry it demands actually exists. + assert "allow_unverified_alert_smtp_tls" in _names( + security={"allow_unverified_alert_smtp_tls": True} + ) + + +def test_an_unconfigured_alert_transport_reports_no_hop_deviation() -> None: + # No SMTP host/sender = no hop to weaken. Reporting one would be a false positive an operator + # cannot act on. + bare = AlertsSettings(email_use_tls=False, email_tls_verify=False) + names = [ + n + for n, _ in security_loosenings( + SecuritySettings(), StoreSettings(), AuthSettings(), bare, () + ) + ] + assert "email_use_tls" not in names + assert "email_tls_verify" not in names + + +def _self_signed_ca_pem() -> bytes: + """A throwaway self-signed CA, generated fresh in-process for the anchor-loading test. + + Generated rather than embedded so the file carries no certificate blob a reader has to trust, and + so it can never expire and turn this into a time-bomb failure. The private key is discarded when + this function returns — nothing can be signed with it. ``cryptography`` is a base dependency + (``transports/direct.py`` S/MIME), so this needs no extra.""" + import datetime + + from cryptography import x509 + from cryptography.hazmat.primitives import hashes, serialization + from cryptography.hazmat.primitives.asymmetric import ec + from cryptography.x509.oid import NameOID + + key = ec.generate_private_key(ec.SECP256R1()) + name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, "mf-test-relay-ca")]) + not_before = datetime.datetime(2026, 1, 1, tzinfo=datetime.UTC) + cert = ( + x509.CertificateBuilder() + .subject_name(name) + .issuer_name(name) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(not_before) + .not_valid_after(not_before + datetime.timedelta(days=3650)) + .add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True) + .sign(key, hashes.SHA256()) + ) + return cert.public_bytes(serialization.Encoding.PEM) + + +# -------------------------------------------------------------------------------------------------- +# The serve gate. An ACKNOWLEDGMENT switch, not the connectors' clamp — the contextvar hop posture is +# never stamped for this cell, so weakened_tls_escape_permitted_here() would be inert here. +# -------------------------------------------------------------------------------------------------- + +SAMPLES_CONFIG = Path(__file__).resolve().parents[1] / "samples" / "config" + + +def _prod_phi_toml(*, alerts_lines: str = "", security_lines: str = "") -> str: + """A production-PHI service config that clears every gate AHEAD of the #323 one. + + Loopback bind, so the exposure-keyed ladder (Posture-B, MFA-at-exposure, /ui) never fires and a + refusal here can only be the alerts-SMTP gate. `[alerts]` is configured because the #188 gate + refuses a PHI instance with no notification channel at all — which would make a refusal test pass + for the wrong reason. Extra `[security]` lines are spliced in as dotted keys BEFORE the first table + header: appended after `[alerts]` they would land IN `[alerts]`, load clean, and silently do + nothing.""" + return ( + "security.block_unlisted_outbound = true\n" + "security.delete_message_bodies_after_days = 30\n" + + security_lines + + "[retention]\ndead_letter_days = 30\n" + + '[alerts]\nemail_smtp_host = "smtp.example.org"\nemail_from = "sec@example.org"\n' + + alerts_lines + ) + + +def _serve(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, toml: str, *, env: str = "prod") -> int: + from messagefoundry.__main__ import main + + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("MEFOR_STORE_ENCRYPTION_KEY", "x" * 44) + (tmp_path / "messagefoundry.toml").write_text(toml, encoding="utf-8") + monkeypatch.setattr("messagefoundry.api.create_managed_app", lambda **kw: object()) + monkeypatch.setattr("uvicorn.run", lambda *a, **k: None) + return main(["serve", "--config", str(SAMPLES_CONFIG), "--env", env]) + + +def test_the_shipped_alert_defaults_start_a_production_phi_instance( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # The gate must be BYTE-IDENTICAL on the secure defaults. A gate that refuses a stock config is a + # gate that gets disabled. + assert _serve(tmp_path, monkeypatch, _prod_phi_toml()) == 0 + + +def test_verification_off_refuses_a_production_phi_instance( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + # WITHOUT THE GATE: returns 0 and the instance starts with an unauthenticated alert hop. + rc = _serve(tmp_path, monkeypatch, _prod_phi_toml(alerts_lines="email_tls_verify = false\n")) + assert rc == 2 + err = capsys.readouterr().err + assert "email_tls_verify=false" in err + assert "refusing to start" in err + # It must name the way OUT, not just the problem — an operator reading this gets nothing else. + assert "allow_unverified_alert_smtp_tls" in err + + +def test_cleartext_alert_smtp_refuses_a_production_phi_instance( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + # The BYPASS this gate exists to close: refusing verify-off while permitting use_tls=false would + # push an operator onto the STRICTLY WORSE posture to escape the refusal. + rc = _serve(tmp_path, monkeypatch, _prod_phi_toml(alerts_lines="email_use_tls = false\n")) + assert rc == 2 + assert "CLEARTEXT" in capsys.readouterr().err + + +def test_the_acknowledgment_permits_the_start_and_audits_it( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + rc = _serve( + tmp_path, + monkeypatch, + _prod_phi_toml( + alerts_lines="email_tls_verify = false\n", + security_lines="security.allow_unverified_alert_smtp_tls = true\n", + ), + ) + assert rc == 0 + # Permitted is not the same as silent: the deliberate weakening must leave a durable record. + assert "warning:" in capsys.readouterr().err + + +def test_a_synthetic_instance_is_not_gated(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + # No PHI, no refusal — the gate is keyed on data_class, like every sibling in the ladder. + # + # The posture is declared EXPLICITLY rather than inferred from `--env dev`. Measured: the samples + # config resolves `dev` to data_class=phi, so an env-name-based version of this test failed with + # rc=2 — and had the gate been (wrongly) keyed on the env NAME instead of the derived data_class, + # that version would have passed while proving nothing. + rc = _serve( + tmp_path, + monkeypatch, + _prod_phi_toml( + alerts_lines="email_tls_verify = false\n", + security_lines="security.handles_real_patient_data = false\n", + ), + env="dev", + ) + assert rc == 0 + + +# -------------------------------------------------------------------------------------------------- +# The `messagefoundry check` advisory — the review surface. The defect was invisible for exactly as +# long as nothing reported it. +# -------------------------------------------------------------------------------------------------- + + +def _advisory(tmp_path: Path, toml: str) -> Any: + from messagefoundry.checks import _check_alert_smtp_tls + + (tmp_path / "messagefoundry.toml").write_text(toml, encoding="utf-8") + return _check_alert_smtp_tls(tmp_path, service_config=tmp_path / "messagefoundry.toml") + + +def test_the_advisory_states_the_secure_case_out_loud(tmp_path: Path) -> None: + # An absent line is indistinguishable from a check that did not run, so the PASSING case must say + # something. This is the leak-gate-blindness rule applied to the report itself. + r = _advisory(tmp_path, _prod_phi_toml()) + assert r.ok and not r.required and not r.skipped + assert "verifies" in r.detail and "OS trust store" in r.detail + + +def test_the_advisory_names_an_unacknowledged_deviation(tmp_path: Path) -> None: + r = _advisory(tmp_path, _prod_phi_toml(alerts_lines="email_tls_verify = false\n")) + assert r.ok and not r.required # advisory: the serve gate refuses, this only reports + assert "ANY certificate" in r.detail + assert "NOT acknowledged" in r.detail + + +def test_the_advisory_reports_the_acknowledgment_when_it_is_set(tmp_path: Path) -> None: + r = _advisory( + tmp_path, + _prod_phi_toml( + alerts_lines="email_tls_verify = false\n", + security_lines="security.allow_unverified_alert_smtp_tls = true\n", + ), + ) + assert "acknowledged by [security].allow_unverified_alert_smtp_tls" in r.detail + + +def test_the_advisory_skips_rather_than_inventing_a_result(tmp_path: Path) -> None: + from messagefoundry.checks import _check_alert_smtp_tls + + r = _check_alert_smtp_tls(tmp_path) + assert r.skipped and r.ok diff --git a/tests/test_client_network_allowlist.py b/tests/test_client_network_allowlist.py index 92d74ca5..f2a75567 100644 --- a/tests/test_client_network_allowlist.py +++ b/tests/test_client_network_allowlist.py @@ -31,6 +31,7 @@ from messagefoundry.auth import Role from messagefoundry.auth.service import AuthService from messagefoundry.config.settings import ( + AlertsSettings, ApiSettings, AuthSettings, SecuritySettings, @@ -49,7 +50,7 @@ def _loosenings(sec: SecuritySettings) -> list[tuple[str, str]]: The registry takes all four inputs as REQUIRED arguments deliberately (ADR 0148: one posture, and a deviation the registry cannot see is a second posture by the back door). The tests below are about the ``[security]`` switches specifically, so the other three are pinned at shipped values here.""" - return security_loosenings(sec, StoreSettings(), AuthSettings(), ()) + return security_loosenings(sec, StoreSettings(), AuthSettings(), AlertsSettings(), ()) PW = "a-strong-test-passphrase" # >=15, no app/vendor terms — satisfies the ASVS policy diff --git a/tests/test_memory_encryption_readout.py b/tests/test_memory_encryption_readout.py index f4dfc2ae..18510305 100644 --- a/tests/test_memory_encryption_readout.py +++ b/tests/test_memory_encryption_readout.py @@ -42,6 +42,7 @@ platform_memory_encryption_readout, ) from messagefoundry.config.settings import ( + AlertsSettings, AuthSettings, SecuritySettings, StoreSettings, @@ -56,7 +57,7 @@ def _loosenings(sec: SecuritySettings) -> list[tuple[str, str]]: The registry takes all four inputs as REQUIRED arguments deliberately (ADR 0148: one posture, and a deviation the registry cannot see is a second posture by the back door). The tests below are about the ``[security]`` switches specifically, so the other three are pinned at shipped values here.""" - return security_loosenings(sec, StoreSettings(), AuthSettings(), ()) + return security_loosenings(sec, StoreSettings(), AuthSettings(), AlertsSettings(), ()) SAMPLES_CONFIG = Path(__file__).resolve().parents[1] / "samples" / "config" diff --git a/tests/test_security_config.py b/tests/test_security_config.py index a696d024..80301bad 100644 --- a/tests/test_security_config.py +++ b/tests/test_security_config.py @@ -18,6 +18,7 @@ from messagefoundry.__main__ import main from messagefoundry.config.ai_policy import DataClass from messagefoundry.config.settings import ( + AlertsSettings, AuthSettings, SecuritySettings, ServiceSettings, @@ -33,7 +34,7 @@ def _loosenings(sec: SecuritySettings) -> list[tuple[str, str]]: The registry takes all four inputs as REQUIRED arguments deliberately (ADR 0148: one posture, and a deviation the registry cannot see is a second posture by the back door). The tests below are about the ``[security]`` switches specifically, so the other three are pinned at shipped values here.""" - return security_loosenings(sec, StoreSettings(), AuthSettings(), ()) + return security_loosenings(sec, StoreSettings(), AuthSettings(), AlertsSettings(), ()) SAMPLES_CONFIG = Path(__file__).resolve().parents[1] / "samples" / "config" diff --git a/tests/test_security_posture_defaults.py b/tests/test_security_posture_defaults.py index 89931f52..79709bca 100644 --- a/tests/test_security_posture_defaults.py +++ b/tests/test_security_posture_defaults.py @@ -26,6 +26,7 @@ from messagefoundry.api import create_app from messagefoundry.config.settings import ( + AlertsSettings, AuthSettings, SecuritySettings, ServiceSettings, @@ -53,6 +54,7 @@ def _names( sec: SecuritySettings | None = None, store: StoreSettings | None = None, auth: AuthSettings | None = None, + alerts: AlertsSettings | None = None, cleartext_hops: tuple[str, ...] = (), ) -> list[str]: """The loosening SWITCH NAMES for a settings combination (defaults where not overridden).""" @@ -62,6 +64,7 @@ def _names( sec or SecuritySettings(), store or StoreSettings(), auth or AuthSettings(), + alerts or AlertsSettings(), cleartext_hops, ) ] @@ -88,7 +91,9 @@ def test_the_shipped_defaults_are_not_themselves_loosenings() -> None: def test_aad_bind_off_is_a_named_loosening() -> None: named = dict( - security_loosenings(SecuritySettings(), StoreSettings(aad_bind=False), AuthSettings(), ()) + security_loosenings( + SecuritySettings(), StoreSettings(aad_bind=False), AuthSettings(), AlertsSettings(), () + ) ) assert "aad_bind" in named # The risk text must say what is actually lost — cell binding, i.e. at-rest INTEGRITY binding — not @@ -102,7 +107,9 @@ def test_aad_bind_loosening_names_its_no_op_caveat() -> None: text says so. Reporting it as a live weakness on a keyless dev box would train operators to ignore the list — the failure mode a loosening registry can least afford.""" named = dict( - security_loosenings(SecuritySettings(), StoreSettings(aad_bind=False), AuthSettings(), ()) + security_loosenings( + SecuritySettings(), StoreSettings(aad_bind=False), AuthSettings(), AlertsSettings(), () + ) ) assert "no effect without a store key" in named["aad_bind"] @@ -112,7 +119,9 @@ def test_aad_bind_loosening_names_its_no_op_caveat() -> None: def test_recheck_zero_with_ad_enabled_is_a_named_loosening() -> None: auth = _ad(ad_session_recheck_seconds=0) - named = dict(security_loosenings(SecuritySettings(), StoreSettings(), auth, ())) + named = dict( + security_loosenings(SecuritySettings(), StoreSettings(), auth, AlertsSettings(), ()) + ) assert "ad_session_recheck_seconds" in named assert "revocation" in named["ad_session_recheck_seconds"] @@ -260,7 +269,11 @@ def test_cleartext_accepted_is_a_named_loosening() -> None: a second posture by the back door.""" named = dict( security_loosenings( - SecuritySettings(), StoreSettings(), AuthSettings(), ("OB_LEGACY", "OB_LAB") + SecuritySettings(), + StoreSettings(), + AuthSettings(), + AlertsSettings(), + ("OB_LEGACY", "OB_LAB"), ) ) assert "cleartext_accepted" in named