From a0ee86e1b9b3734017171074681ddd80e91ae762 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 30 Jul 2026 20:32:56 -0500 Subject: [PATCH 1/2] fix(dicom): an AE-title list is not a peer control on its own (BACKLOG #252) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fail-closed peer-control gate refuses a non-loopback C-STORE 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. 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. So an SCP whose only peer 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". Server TLS does not close it either: without tls_ca_file there is no client certificate, so the cleartext bind guard (confidentiality) and this gate (authentication) are orthogonal. Option (a) of the three recorded on #252, chosen by the owner: PAIR, do not remove. Off-loopback the gate now requires source_ip_allowlist or mTLS. calling_ae_allowlist is KEPT and still enforced at association time — it catches a misrouted sender and pins intent, which is real value — it simply cannot stand alone. Measured, off-loopback: AE-title-only STARTS -> REFUSED; AE-title PAIRED with an IP allowlist starts; IP-only and mTLS-only unchanged; every loopback bind unchanged (the common dev/single-box case does not regress). Full unfiltered suite: 9518 passed, 0 failed. The refusal detects the unpaired case specifically and explains why an AE Title cannot be verified, and tells the operator to KEEP it and add a control — otherwise the message reads as "but I already set an allowlist" and the obvious wrong move is to delete the AE list. BREAKING for a site whose non-loopback SCP relies on AE-title-alone. The fix is one line (source_ip_allowlist on the inbound(...) call, which for a DICOM SCP is the only authoring surface) and the refusal names it. Recorded in CHANGELOG. A note on the citation. The code and both docs credited this rule to "ADR 0025 §9". ADR 0025 has no section 9 — its decision sections run 1-8, and the "§9" in that file refers to CLAUDE.md §9 — and calling_ae_allowlist appears nowhere in it. The normative statement lives in docs/CONNECTIONS.md and the docs/SECURITY.md decision table, which is what this commit amends. No ADR amendment is possible because there is no ADR text to amend; the new code and tests cite BACKLOG #252, which is real, rather than repeating a pointer that sends a reader somewhere they cannot go. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 25 ++++++++++++++ docs/BACKLOG.md | 2 +- docs/CONNECTIONS.md | 4 ++- docs/SECURITY.md | 2 +- messagefoundry/transports/dicom.py | 54 ++++++++++++++++++------------ tests/test_dicom_scp_security.py | 23 +++++++++++-- 6 files changed, 83 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba178e1d..7aa0a504 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,31 @@ All notable changes to MessageFoundry are documented here. The format follows ## [Unreleased] +### Changed +- **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 #252**. Options considered and declined: an audited opt-out switch, and + documenting the weakness without changing the gate. + ### Fixed - **The load harness's no-loss reconcile did not enforce the `read >= sent // 2` intake guarantee 0.3.2 documented.** The unconfirmed-send excusal is capped at `max(connections, half the run)`, but diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index a4c2e535..084c862d 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -7164,7 +7164,7 @@ The existing test is too weak to catch any of it: `test_insert_comment_reads_bac **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 -> 🔢 **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. diff --git a/docs/CONNECTIONS.md b/docs/CONNECTIONS.md index 5c8e2b00..9728aea9 100644 --- a/docs/CONNECTIONS.md +++ b/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 #252).** 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 — diff --git a/docs/SECURITY.md b/docs/SECURITY.md index d5b43b2c..884e6a27 100644 --- a/docs/SECURITY.md +++ b/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 #252): 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 diff --git a/messagefoundry/transports/dicom.py b/messagefoundry/transports/dicom.py index 5530d7c4..2db73a5e 100644 --- a/messagefoundry/transports/dicom.py +++ b/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 #252): + # 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 #252) — 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 diff --git a/tests/test_dicom_scp_security.py b/tests/test_dicom_scp_security.py index b0989776..2f2da735 100644 --- a/tests/test_dicom_scp_security.py +++ b/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 #252: 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: From 5764e4ff1bd5d15028826d89304ca44f1ae58891 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 31 Jul 2026 08:19:17 -0500 Subject: [PATCH 2/2] fix(ledger): renumber this item 252 -> 316, it collided (BACKLOG #316) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BACKLOG #252 was allocated BEFORE #88 fixed alloc.ps1's floor, which read only origin/main + HEAD despite a docstring promising every local and remote ref. docs/BACKLOG.md is a published baseline of a larger internal ledger, so numbers above #231 exist on refs this branch does not carry — and 252 was one of them: refs/remotes/vaultall/claude/throughput-thursday-4debf6 ## 252. Per-row ingress-stage raw checksum (integrity anchor) So the DICOM peer-control item was a duplicate of an unrelated integrity-anchor item. Re-allocated with the fixed floor, which returns 316 — confirming the jump the fix predicts. Verified 316 is free on every vault ref before taking it. Renumbered in the entry, the gate comment, the refusal message, the test docstring, docs/CONNECTIONS.md, the docs/SECURITY.md decision row and the CHANGELOG. The claim moved with it: 316 taken, 252 released. Deliberately a targeted replacement of "BACKLOG #252" and the "## 252." heading rather than a global 252 -> 316 sweep. CHANGELOG.md mentions cp1252 twice, and a naive replace turns it into cp1316 — a silent corruption in a file nobody re-reads. Both occurrences verified intact. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 2 +- docs/BACKLOG.md | 2 +- docs/CONNECTIONS.md | 2 +- docs/SECURITY.md | 2 +- messagefoundry/transports/dicom.py | 4 ++-- tests/test_dicom_scp_security.py | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7aa0a504..6f0eb318 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ All notable changes to MessageFoundry are documented here. The format follows `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 #252**. Options considered and declined: an audited opt-out switch, and + Tracked as **BACKLOG #316**. Options considered and declined: an audited opt-out switch, and documenting the weakness without changing the gate. ### Fixed diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index b114af89..38738b25 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -7219,7 +7219,7 @@ 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 > ✅ **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). diff --git a/docs/CONNECTIONS.md b/docs/CONNECTIONS.md index 9728aea9..ce8abc3c 100644 --- a/docs/CONNECTIONS.md +++ b/docs/CONNECTIONS.md @@ -1538,7 +1538,7 @@ The **bind interface** is the service-level `[inbound].bind_host` (or a per-conn > **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 #252).** 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. +> ⚠️ **`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 — diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 884e6a27..aab44b52 100644 --- a/docs/SECURITY.md +++ b/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 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 #252): 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 | +| **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 diff --git a/messagefoundry/transports/dicom.py b/messagefoundry/transports/dicom.py index 2db73a5e..7409f189 100644 --- a/messagefoundry/transports/dicom.py +++ b/messagefoundry/transports/dicom.py @@ -177,7 +177,7 @@ 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; tightened by BACKLOG #252): + # 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 @@ -199,7 +199,7 @@ def __init__(self, config: Source) -> None: 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 #252) — keep it as a" + " 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 "" diff --git a/tests/test_dicom_scp_security.py b/tests/test_dicom_scp_security.py index 2f2da735..475bd127 100644 --- a/tests/test_dicom_scp_security.py +++ b/tests/test_dicom_scp_security.py @@ -163,7 +163,7 @@ def test_nonloopback_scp_without_any_peer_control_fails_closed() -> None: def test_nonloopback_scp_with_only_calling_ae_allowlist_fails_closed() -> None: - """BACKLOG #252: an AE-title list no longer satisfies the gate alone. + """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