fix(dicom): an AE-title list is not a peer control on its own (BACKLOG #252)

CHANGELOG.md

@@ -39,6 +39,29 @@ All notable changes to MessageFoundry are documented here. The format follows
   `block_unlisted_outbound = false` is unchanged and still refused, because there the other six
   transports stay allow-any; the refusal now names that override as the reason. **No shipped refusal
   stops firing.**
+- **BREAKING — a non-loopback DICOM C-STORE SCP now requires a *verifiable* peer control;
+  `calling_ae_allowlist` no longer satisfies the gate on its own.** The fail-closed peer-control check
+  refuses a remotely-reachable SCP that has no peer control, and it accepted any one of three:
+  `calling_ae_allowlist`, `source_ip_allowlist`, or mTLS. It **counted** them rather than weighing
+  them. But a Calling AE Title is a string the caller asserts about **itself** in the association
+  request — no key, no signature, nothing to verify — and AE Titles are published in conformance
+  statements and visible in any capture. An SCP whose only control was an AE-title list was therefore
+  reachable by anyone who could route to it and knew one string, while passing a check named
+  "fail-closed peer controls". Server TLS does not close this: without `tls_ca_file` there is no client
+  certificate, so the cleartext bind guard (confidentiality) and this gate (authentication) are
+  orthogonal.
+  **What changed:** off-loopback, the gate now requires `source_ip_allowlist` **or** mTLS
+  (`tls` + `tls_ca_file`). `calling_ae_allowlist` is **kept and still enforced** at association time —
+  it is a genuinely useful filter that catches a misrouted sender and pins intent — it simply has to be
+  **paired** with one of the two. Measured: AE-title-only off-loopback goes from starting to refused;
+  AE-title **paired** with an IP allowlist starts; IP-only and mTLS-only are unchanged; and every
+  loopback bind (the common dev/single-box case) is unchanged.
+  **Who this bites:** a site running a non-loopback SCP whose only peer control is
+  `calling_ae_allowlist`. It starts today and will refuse after upgrading. The fix is one line — add
+  `source_ip_allowlist=[...]` to the `inbound(...)` call, which for a DICOM SCP is the only authoring
+  surface — and the refusal names it. Keep the AE list; it is still doing work.
+  Tracked as **BACKLOG #316**. Options considered and declined: an audited opt-out switch, and
+  documenting the weakness without changing the gate.
 
 ### Fixed
 - **The DICOM C-STORE SCP's fail-closed refusal named a settings key that does not exist.** It told

docs/BACKLOG.md

@@ -7219,9 +7219,9 @@ The existing test is too weak to catch any of it: `test_insert_comment_reads_bac
 **Related:** #250 (the ops API this would sit on), #238, #26 (the declined visual-authoring line — why a declarative artifact interpreted by a second execution path is out).
 
 **Source:** Windmill/Kestra evaluation (2026-07-30), "Anvil Ops Tasks" design. Recorded here because the design memo holding the conditions has been deleted.
-## 252. DICOM SCP peer-control gate counts a spoofable AE-title list as sufficient
+## 316. DICOM SCP peer-control gate counts a spoofable AE-title list as sufficient
 
-> 🔢 **Filed 2026-07-30.** P2. Surfaced while fixing the refusal message that named a non-existent settings key (that half is fixed; this half is a contract change and was deliberately not ridden in on it).
+> ✅ **SHIPPED — 2026-07-30. Option (a): pair, do not remove.** Off-loopback, the gate now requires a **verifiable** control — `source_ip_allowlist` or mTLS. `calling_ae_allowlist` no longer satisfies it alone, but is **kept and still enforced** at association time as a filter, so nothing that was useful about it is lost. Measured: AE-title-only off-loopback goes STARTS → REFUSED; AE-title **paired** with an IP allowlist starts; IP-only and mTLS-only are unchanged; every loopback case is unchanged. Breaking for a site relying on AE-title-alone off-loopback — see CHANGELOG. Options (b) audited-opt-out and (c) document-only were declined by the owner in favour of (a).
 
 **Type:** security hardening — authentication strength of a fail-closed gate.
 

docs/CONNECTIONS.md

@@ -1536,7 +1536,9 @@ MWL, Query/Retrieve (C-FIND/C-MOVE/C-GET), and pixel-data handling.
 
 The **bind interface** is the service-level `[inbound].bind_host` (or a per-connection `bind_address`) and the **peer-IP gate** is the per-connection **`source_ip_allowlist`** — both are set on the `inbound(...)` call, not as `DICOM()` arguments. ⚠️ **`source_ip_allowlist` is *not* a key of the `[inbound]` section in `messagefoundry.toml`.** That section carries only `bind_host`, `ack_after` and `stream_inflight_budget_bytes`, and every settings section is pydantic `extra="ignore"` — so writing `source_ip_allowlist` under `[inbound]` in the service TOML is **accepted silently and does nothing**. (Verified: `InboundSettings.model_fields` is exactly those three; a loaded `[inbound].source_ip_allowlist` leaves no attribute behind, while a sibling `bind_host` survives.) `bind_address` is the same story — a per-connection keyword, not a `[inbound]` key. The reachable forms are `inbound("IB_…", DICOM(...), source_ip_allowlist=["10.20.0.0/16"])` and, for the transports available as data, the **top-level** `source_ip_allowlist` key in `connections.toml` (shown in the [`connections.toml` example](#connections-as-data--connectionstoml-adr-0007) above) — `DICOM()` is code-first only, so for a SCP it is the `inbound(...)` keyword. A non-loopback cleartext SCP is **refused at startup** unless `tls=true` (the generalized [cleartext] bind-guard — `check_dimse_tls_exposure`). `serve --allow-insecure-bind` downgrades that refusal to a warning, but the flag is **clamped** exactly as it is for the MLLP/HTTP/TCP listeners: on a PHI-classified instance under the default `[security].enforcement = enforce` the bind is refused *even with it*, so on a stock instance `tls=true` is the only way to bind off-loopback. (`host` / `called_ae_title` / `connect_timeout` on `DICOM()` are for the **Phase-2 outbound SCU** and are unused by the inbound SCP.)
 
-> **Fail-closed peer controls (deny-by-default, ADR 0025 §9).** DICOM has no transport authentication on its own, so a **non-loopback** SCP **MUST** set at least one peer control — `calling_ae_allowlist` (a `DICOM()` argument), a per-connection `source_ip_allowlist` (an `inbound(...)` keyword — **not** a `[inbound]` service-TOML key, see the ⚠️ above), or **mTLS** (`tls=true` **and** `tls_ca_file`, which makes the SCP require + verify a client cert). With **none** of the three set, a non-loopback SCP is **refused at construction** (the connection degrades per ADR 0031 startup fault isolation; surfaced under `check`/dry-run). This is the **authentication** analog of the `check_dimse_tls_exposure` cleartext bind-guard above (which is the orthogonal **confidentiality** guard): TLS-without-mTLS encrypts the channel but does **not** authenticate the peer. A **loopback** bind (`127.0.0.1`/`localhost`/`::1`, the common dev/single-box case) is exempt.
+> **Fail-closed peer controls (deny-by-default).** DICOM has no transport authentication on its own, so a **non-loopback** SCP **MUST** set a **verifiable** peer control — either a per-connection `source_ip_allowlist` (an `inbound(...)` keyword — **not** a `[inbound]` service-TOML key, see the ⚠️ above), or **mTLS** (`tls=true` **and** `tls_ca_file`, which makes the SCP require + verify a client cert). With **neither** set, a non-loopback SCP is **refused at construction** (the connection degrades per ADR 0031 startup fault isolation; surfaced under `check`/dry-run). This is the **authentication** analog of the `check_dimse_tls_exposure` cleartext bind-guard above (which is the orthogonal **confidentiality** guard): TLS-without-mTLS encrypts the channel but does **not** authenticate the peer. A **loopback** bind (`127.0.0.1`/`localhost`/`::1`, the common dev/single-box case) is exempt.
+>
+> ⚠️ **`calling_ae_allowlist` does not satisfy this gate on its own (BACKLOG #316).** It used to: the three controls were counted as co-equal. But a Calling AE Title is a string the caller asserts about **itself** in the association request — no key, no signature, nothing to verify — and AE Titles are published in conformance statements and visible in any capture. An SCP whose only control was an AE-title list was reachable by anyone who could route to it and knew one string, while passing a check named "fail-closed peer controls". **Keep it — it is still enforced at association time and is a genuinely useful filter** (it catches a misrouted sender and pins intent). It simply has to be **paired** with `source_ip_allowlist` or mTLS off-loopback.
 >
 > ⚠️ **The construction gate counts controls, so the wrong spelling passes it.** Set
 > `calling_ae_allowlist` (AE titles are attacker-chosen strings on an unauthenticated association —

docs/SECURITY.md

@@ -1118,7 +1118,7 @@ listen source. The refusal action differs materially per listener, so each has i
 | **MLLP / HTTP / DICOM** — peer client certificate | the TLS peer certificate presented at handshake | `tls = true` **and** `tls_ca_file` set → `ssl.CERT_REQUIRED` plus strict RFC 5280 verify flags; no client certificate, or one not issued by that CA | **DENY** — the TLS handshake fails and the connection **never reaches the accept path**, so there is **no** connection event and no allow-list evaluation. `tls_ca_file` unset → server-only TLS and no peer-certificate decision. TCP and X12 have no inbound TLS at this release |
 | **DICOM** — calling AE | the requesting AE's Calling AE Title, at **association negotiation** | `calling_ae_allowlist` set and the title is not in it | **DENY** — the association is rejected by pynetdicom before any C-STORE callback runs (`ae.require_calling_aet`). `None` = any AE the peer-IP allow-list admits |
 | **DICOM** — called AE | the AE Title the peer addressed the association to | not this engine's own `ae_title` | **DENY** at negotiation (`ae.require_called_aet`); **default `require_called_ae_title = true`** |
-| **DICOM** — peer-control construction gate | the SCP's bind host × the presence of any peer control | non-loopback bind with **none** of `calling_ae_allowlist`, `source_ip_allowlist` (an `inbound(...)` keyword — for a DICOM SCP the ONLY surface, since `DICOM()` is not authorable in `connections.toml`), or mTLS (`tls` + `tls_ca_file` → `CERT_REQUIRED`) | **DENY at construction** (ValueError). The connection degrades per ADR 0031 startup fault isolation and the fault surfaces under `messagefoundry check` / dry-run. Loopback hosts are exempt |
+| **DICOM** — peer-control construction gate | the SCP's bind host × the presence of a **verifiable** peer control | non-loopback bind with **neither** `source_ip_allowlist` (an `inbound(...)` keyword — for a DICOM SCP the ONLY surface, since `DICOM()` is not authorable in `connections.toml`) **nor** mTLS (`tls` + `tls_ca_file` → `CERT_REQUIRED`). ⚠️ `calling_ae_allowlist` does **not** satisfy this gate alone (BACKLOG #316): an AE Title is caller-asserted with no cryptographic binding, so it is still enforced as a filter but must be **paired** with one of the two above | **DENY at construction** (ValueError). The connection degrades per ADR 0031 startup fault isolation and the fault surfaces under `messagefoundry check` / dry-run. Loopback hosts are exempt |
 
 > **Telemetry honesty.** The `peer_not_allowlisted` connection event is durable when the connection's
 > `capture_connection_errors` is `true`, **or is unset (`None`, the default) and the

messagefoundry/transports/dicom.py

@@ -177,30 +177,42 @@ def __init__(self, config: Source) -> None:
         self._timeout = float(s.get("timeout_seconds", 30.0))
         # Build the TLS context now so a bad cert/key fails at build, not at bind (like MLLP/LDAPS).
         self._ssl = _server_ssl_context(s)
-        # Fail-closed peer controls (SEC-012, deny-by-default per ADR 0025 §9): a non-loopback SCP with
-        # NO peer authentication is refused at construction. DIMSE has no transport auth on its own, so
-        # a remotely-reachable SCP must gate peers by at least one of: the calling-AE allowlist, the
-        # per-connection source_ip_allowlist (an inbound(...) keyword — NOT an [inbound] service-settings
-        # key, which does not exist and is discarded silently), or mTLS (tls + tls_ca_file → CERT_REQUIRED in
+        # Fail-closed peer controls (SEC-012, deny-by-default; tightened by BACKLOG #316):
+        # a non-loopback SCP with no VERIFIABLE peer control is refused at construction. DIMSE has no
+        # transport auth of its own, so a remotely-reachable SCP must gate peers by the per-connection
+        # source_ip_allowlist (an inbound(...) keyword — NOT an [inbound] service-settings key, which does
+        # not exist and is discarded silently) or mTLS (tls + tls_ca_file → CERT_REQUIRED in
         # _server_ssl_context). This is the AUTHENTICATION analog of check_dimse_tls_exposure's cleartext
-        # bind guard (which is the orthogonal CONFIDENTIALITY guard). Raising here integrates with
-        # ADR-0031 startup fault isolation (the connection degrades, not the engine) and surfaces under
-        # check/dry-run. Loopback binds (dev/single-box) are exempt.
+        # bind guard (the orthogonal CONFIDENTIALITY guard). Raising here integrates with ADR-0031 startup
+        # fault isolation (the connection degrades, not the engine) and surfaces under check/dry-run.
+        # Loopback binds (dev/single-box) are exempt.
+        #
+        # calling_ae_allowlist deliberately does NOT satisfy this gate on its own. The original rule
+        # counted the three controls as co-equal, but a Calling AE Title is a string the caller asserts
+        # about ITSELF in the association request — no key, no signature, nothing to verify, and AE Titles
+        # are published in conformance statements and visible in any capture. An SCP whose only control
+        # was an AE-title list was reachable by anyone who could route to it and knew one string, while
+        # passing a check named "fail-closed peer controls". It remains a useful FILTER (it catches a
+        # misrouted sender and pins intent) and is still enforced at association time — it just has to be
+        # PAIRED with a control that can actually be verified.
         mtls_on = bool(s.get("tls")) and bool(s.get("tls_ca_file"))
-        if self._host not in _LOOPBACK_HOSTS and not (
-            self._calling_ae_allowlist or self._source_ip_allowlist or mtls_on
-        ):
+        if self._host not in _LOOPBACK_HOSTS and not (self._source_ip_allowlist or mtls_on):
+            unpaired = (
+                " You set calling_ae_allowlist, but an AE Title is asserted by the caller and cannot be"
+                " verified, so it no longer satisfies this gate alone (BACKLOG #316) — keep it as a"
+                " filter and add one of the two controls above."
+                if self._calling_ae_allowlist
+                else ""
+            )
             raise ValueError(
-                f"DICOM C-STORE SCP bound non-loopback host {self._host!r} with no peer controls: "
-                "set at least one of mTLS (tls + tls_ca_file), source_ip_allowlist, or "
-                "calling_ae_allowlist to fail closed (egress deny-by-default ethos, ADR 0025 §9), or "
-                "bind 127.0.0.1. Authoring surface: pass them to inbound(...), e.g. "
-                'inbound("pacs_in", DICOM(...), source_ip_allowlist=["10.0.0.0/8"]). That is the ONLY '
-                "surface for a DICOM SCP — the [inbound] section of messagefoundry.toml has no "
-                "source_ip_allowlist key and discards it silently, and while connections.toml "
-                "[[inbound]] tables do accept the key, none of their transports is DICOM. Note "
-                "calling_ae_allowlist alone gates on a caller-asserted AE Title with no cryptographic "
-                "binding; prefer mTLS or the IP allowlist where the peer is not trusted."
+                f"DICOM C-STORE SCP bound non-loopback host {self._host!r} with no verifiable peer "
+                "control: set source_ip_allowlist, or mTLS (tls + tls_ca_file), to fail closed "
+                f"(deny-by-default, ADR 0025 §9), or bind 127.0.0.1.{unpaired} Authoring surface: pass "
+                'them to inbound(...), e.g. inbound("pacs_in", DICOM(...), '
+                'source_ip_allowlist=["10.0.0.0/8"]). That is the ONLY surface for a DICOM SCP — the '
+                "[inbound] section of messagefoundry.toml has no source_ip_allowlist key and discards it "
+                "silently, and while connections.toml [[inbound]] tables do accept the key, none of "
+                "their transports is DICOM."
             )
         self._handler: InboundHandler | None = None
         self._loop: asyncio.AbstractEventLoop | None = None

tests/test_dicom_scp_security.py

@@ -158,13 +158,30 @@ def test_nonloopback_scp_without_any_peer_control_fails_closed() -> None:
     with pytest.raises(ValueError) as exc:
         _scp("0.0.0.0")
     msg = str(exc.value)
-    assert "calling_ae_allowlist" in msg
     assert "source_ip_allowlist" in msg
     assert "mTLS" in msg
 
 
-def test_nonloopback_scp_with_calling_ae_allowlist_ok() -> None:
-    _scp("0.0.0.0", calling_ae_allowlist=["MOD1"])  # no raise
+def test_nonloopback_scp_with_only_calling_ae_allowlist_fails_closed() -> None:
+    """BACKLOG #316: an AE-title list no longer satisfies the gate alone.
+
+    A Calling AE Title is asserted by the caller in the association request — no key, no signature,
+    nothing to verify — and AE Titles are published in conformance statements. Before the amendment
+    this construction succeeded, so an SCP reachable by anyone who knew one string passed a check
+    named "fail-closed peer controls". The refusal must say specifically why the AE list is not
+    enough, or the operator just re-reads it as "I already set an allowlist".
+    """
+    with pytest.raises(ValueError) as exc:
+        _scp("0.0.0.0", calling_ae_allowlist=["MOD1"])
+    msg = str(exc.value)
+    assert "calling_ae_allowlist" in msg
+    assert "cannot be verified" in msg
+    assert "filter" in msg  # tells them to KEEP it, not delete it
+
+
+def test_nonloopback_scp_with_paired_ae_and_ip_allowlist_ok() -> None:
+    # The amendment PAIRS rather than removes: the AE list stays useful, alongside a verifiable control.
+    _scp("0.0.0.0", calling_ae_allowlist=["MOD1"], source_ip_allowlist=["10.0.0.0/8"])  # no raise
 
 
 def test_nonloopback_scp_with_source_ip_allowlist_ok() -> None: