From 53b0de3efdd002c6bfe799f3c77d6cbcaf56f3a7 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 16:25:26 -0500 Subject: [PATCH 1/3] fix(auth): an EMPTY oidc_client_secret is a MISSING one (ASVS 6.x config validation) The guard that makes a missing OIDC client secret fail at config load read if self.oidc_client_secret is None and self.oidc_client_secret_ref is None which is not the shape a missing secret usually takes. `MEFOR_AUTH_OIDC_CLIENT_SECRET=` in a service wrapper, or an NSSM environment entry with an empty value, yields `""` -- and `"" is not None`, so oidc_enabled loaded happily with no client secret at all. Reproduced before the fix, with a working control so the repro proves something: secret='client-s3cret' ACCEPTED (control) secret='' ACCEPTED oidc_client_secret='' secret=' ' ACCEPTED oidc_client_secret=' ' secret absent refused "oidc_enabled requires a client secret" Only the ABSENT case fired. Note the `missing` list ten lines above in the same validator already uses `if not value` for all five pinned endpoints -- this one line was the only place in the validator that disagreed with its own siblings, which is why it read as correct. Severity, stated accurately rather than inflated: this is NOT an auth bypass. An empty client_secret reaches the IdP at the token exchange and is rejected there, so the effect is a federated login that is broken instead of refused, diagnosed from a proxy log rather than from a startup message. The defect is that a fail-at-load guard silently became a fail-at-first-login one. Whitespace is stripped for the emptiness TEST only; the value is never rewritten, and a secret with meaningful leading/trailing whitespace still binds byte-for-byte (asserted). The same test now applies to oidc_client_secret_ref, or the fix would be half a fix -- an empty ref would otherwise satisfy "one of the two is set" while naming no provider entry. Both mutations killed against the shipped, formatted source: M1 restore `is None` -> RED (4 failed) on the empty cases M2 make the guard reject EVERY secret -> RED on the POSITIVE CONTROL M2 is the one that matters: without a positive control, both refusal tests would stay green under a guard that refuses everything, and "a raise happened" is not the same claim as "the right input was refused". --- messagefoundry/config/settings.py | 18 ++++++++-- tests/test_settings.py | 57 +++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/messagefoundry/config/settings.py b/messagefoundry/config/settings.py index ca4ddec4..7cee18a4 100644 --- a/messagefoundry/config/settings.py +++ b/messagefoundry/config/settings.py @@ -2120,10 +2120,22 @@ def _require_oidc_fields(self) -> AuthSettings: if missing: raise ValueError(f"oidc_enabled requires: {', '.join(missing)}") - if self.oidc_client_secret is None and self.oidc_client_secret_ref is None: + # EMPTY, not just absent. `is None` was the test here, and it let the most common shape of a + # missing secret straight through: an env var exported with no value. `MEFOR_AUTH_OIDC_CLIENT_SECRET=` + # in a service wrapper or an NSSM environment entry produces `""`, which is not None, so the guard + # that exists to make a missing client secret fail at CONFIG LOAD did not fire — the failure moved + # to the first token exchange, as an IdP rejection an operator has to go read a proxy log to + # understand. `if not value` is already the emptiness test used by the `missing` list ten lines + # above; this line was the only one in the validator that disagreed. Whitespace is stripped for the + # test only — the value itself is never rewritten. + if ( + not (self.oidc_client_secret or "").strip() + and not (self.oidc_client_secret_ref or "").strip() + ): raise ValueError( - "oidc_enabled requires a client secret: set oidc_client_secret (via " - "MEFOR_AUTH_OIDC_CLIENT_SECRET) or oidc_client_secret_ref (a [secrets].provider reference)" + "oidc_enabled requires a NON-EMPTY client secret: set oidc_client_secret (via " + "MEFOR_AUTH_OIDC_CLIENT_SECRET) or oidc_client_secret_ref (a [secrets].provider " + "reference). An env var exported with no value counts as missing." ) # Every pinned URL must be https (no dev escape — this is an off-box trust boundary) and its diff --git a/tests/test_settings.py b/tests/test_settings.py index a7ce7df9..ba8edd84 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -605,6 +605,63 @@ def test_auth_oidc_refusals_name_the_key( load_settings(config_path=cfg, environ=env) +@pytest.mark.parametrize( + ("secret", "label"), + [ + ("", "an env var exported with NO value"), + (" ", "whitespace only"), + ("\t\n", "whitespace only, non-space"), + ], +) +def test_auth_oidc_empty_client_secret_is_refused(tmp_path: Path, secret: str, label: str) -> None: + """An EMPTY client secret must be refused at config load, not just an absent one. + + The guard used to read ``if self.oidc_client_secret is None``, which is not the shape a missing + secret usually takes. ``MEFOR_AUTH_OIDC_CLIENT_SECRET=`` in a service wrapper — or an NSSM + environment entry with an empty value — yields ``""``, and ``"" is not None``, so federated login + started up "configured" with no client secret and failed later at the token exchange as an IdP + rejection. Every sibling field in the same validator already used ``if not value``; this one line + disagreed. + + Reproduced before the fix: ``""`` and ``" "`` both loaded successfully with + ``oidc_client_secret == ''``. Mutation: restore ``is None``. Red on all three cases. + """ + cfg = _write(tmp_path / "messagefoundry.toml", _OIDC_AD + _OIDC_BLOCK) + env = dict(_OIDC_ENV) | {"MEFOR_AUTH_OIDC_CLIENT_SECRET": secret} + with pytest.raises(ValidationError, match="NON-EMPTY client secret"): + load_settings(config_path=cfg, environ=env) + + +def test_auth_oidc_empty_secret_ref_is_refused(tmp_path: Path) -> None: + """The same emptiness test must apply to the ``_ref`` alternative, or the fix is half a fix. + + An empty ``oidc_client_secret_ref`` would otherwise satisfy the "one of the two is set" guard while + naming no provider entry at all. + """ + cfg = _write( + tmp_path / "messagefoundry.toml", + _OIDC_AD + _OIDC_BLOCK + 'oidc_client_secret_ref = ""\n', + ) + env = {k: v for k, v in _OIDC_ENV.items() if k != "MEFOR_AUTH_OIDC_CLIENT_SECRET"} + with pytest.raises(ValidationError, match="NON-EMPTY client secret"): + load_settings(config_path=cfg, environ=env) + + +def test_auth_oidc_a_real_secret_still_loads(tmp_path: Path) -> None: + """Positive control for the two refusals above. + + Without it, tightening the emptiness test to something that rejects EVERY secret would leave both + refusal tests green — they only assert that a raise happens. + """ + cfg = _write(tmp_path / "messagefoundry.toml", _OIDC_AD + _OIDC_BLOCK) + s = load_settings(config_path=cfg, environ=dict(_OIDC_ENV)) + assert s.auth.oidc_client_secret == "client-s3cret" + # A secret with meaningful surrounding whitespace is NOT rewritten — stripping is for the + # emptiness test only, because an IdP secret can legitimately contain any byte. + padded = dict(_OIDC_ENV) | {"MEFOR_AUTH_OIDC_CLIENT_SECRET": " s3cret "} + assert load_settings(config_path=cfg, environ=padded).auth.oidc_client_secret == " s3cret " + + def test_auth_oidc_requires_public_origin(tmp_path: Path) -> None: cfg = _write(tmp_path / "messagefoundry.toml", _OIDC_AD + _OIDC_BLOCK) env = {k: v for k, v in _OIDC_ENV.items() if k != "MEFOR_SECURITY_WEB_CONSOLE_PUBLIC_ADDRESS"} From af9f382b81305fdc68832647575e56c084308044 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 16:51:45 -0500 Subject: [PATCH 2/3] fix(tls): the approved KEX groups were never pinned -- stop claiming they were (ASVS 11.6.2) harden_kex_groups pins SSLContext.set_groups "where available (Python 3.13+)". set_groups is a Python 3.15 API. typeshed guards it at sys.version_info >= (3, 15), alongside the get_groups that will finally make the pin assertable. Measured on this tree -- Python 3.14.6 / OpenSSL 3.5.7, hasattr(ctx, "set_groups") is False -- so APPROVED_KEX_GROUPS reaches ZERO of its six call sites and every built context inherits OpenSSL's default group list. That much was already suspected. What was not: client pinned to result (real handshakes vs build_api_ssl_context, identical at tls_min_version 1.2 and 1.3) X25519 accepted <- positive control secp384r1 accepted <- positive control prime256v1 accepted <- positive control ffdhe2048 ACCEPTED ffdhe3072 ACCEPTED secp521r1 ACCEPTED secp224r1 refused (NO_SUITABLE_GROUPS) sect571r1 refused (NO_SUITABLE_GROUPS) ADR 0092 section 4(b) asserts the opposite: "a real handshake test proves a client offering only a non-approved FFDHE group is refused -- runtime enforcement". That test reached its assertion only through a CLIENT-side set_groups, so it hit pytest.skip on every interpreter this project runs on. A skip was concealing a false assertion, and the ADR cited it as proof. That is worse than an untested claim, because it reads as evidence. Method, stated because the probe could have lied too: the client pins ONE group via set_ecdh_curve, which was validated as a genuine constraint FIRST -- a server pinned to prime256v1 refuses a client pinned to secp384r1 (NO_SUITABLE_KEY_SHARE) while the unpinned-server control accepts it. Without that check, a probe that constrained nothing would have reported "everything accepted" for a context that was in fact restrictive. SEVERITY, stated accurately rather than inflated: the inherited list is forward-secret, so the property the TLS 1.2+ floor exists to guarantee still holds, and the genuinely weak curves are refused. This is a conformance gap against an internal allow-list -- wider than policy, not weak. ASVS 11.6.2 is Partial in the scorecard of record and stays Partial. The headline count does not move. What changes is whether it can be defended. What lands: harden_kex_groups now RETURNS the list it actually pinned (None today). A security control that cannot report whether it did anything reports success forever -- that is how a call at six sites with zero effect survived three assessments. The failure path returns None too: previously a pin that RAISED logged a warning and then fell off the end, so a caller could not distinguish "pinned" from "tried and failed". Its `# pragma: no cover` is gone, because a stand-in context now drives that branch instead of leaving it to an unusual OpenSSL build. The three unit tests that stood over this asserted (a) the call does not raise, (b) it no-ops on an object without the API, (c) a string constant has three colon-separated names. All three pass identically whether or not a single group is ever pinned. Replaced with an unconditional receipt asserting the None, whose failure message is the re-derivation instruction for whoever trips it. Written unconditionally on purpose: an `if hasattr(ctx, "set_groups")` branch would restore exactly the property being removed. The skipping handshake test is replaced by one that MEASURES the accepted-group set and cannot skip. Asserted as invariants rather than an exact table, since the accepted set comes from the linked OpenSSL and a CI leg may differ: every approved group must get in (else the server is over-restricted and the next assertion would pass for the wrong reason) and some non-approved group must get in (proving the list is not enforced). The measured table prints on failure. Five mutations, both directions: M1 report a pin never made -> inertness receipt RED M2 revert to the old no-return shape -> reporting test RED M3 failed pin reports the list -> raising test RED M4 probe accepts everything -> measurement RED M5 probe refuses everything -> measurement RED M2 is the one that justifies its own test: under M2 the INERTNESS test stays GREEN (measured: rc=0), because a function with no return statement also yields None. Asserting `is None` therefore does not prove the reporting works, which is why the pinning path is driven separately with a stand-in context. Documents corrected in the same change, because the claim had spread to five: - config/tls_policy.py -- the "3.13+" docstring and the module summary. - docs/PHI.md section 4 -- "Approved groups pinned where supported" -> inherited, with the measured accepted set. - docs/ASVS-L2-PHASE0-CHANGES.md -- the same sentence, same error. - docs/adr/0092 section 4(b) -- struck through in place and WITHDRAWN by a dated amendment naming all three wrong assertions. The ADR's decision is unaffected: the forward secrecy it relies on comes from the TLS 1.2+ floor, which IS enforced. Sections 4(a) and 4(c) were re-confirmed and stand. - docs/BACKLOG.md -- the "runtime-KEX enforcement ... handshake-tested" clause inside a shipped banner. Prose-only edit, banner invariant untouched, test_backlog_status_check.py green. Deliberately NOT touched: docs/security/ASVS-L3-ASSESSMENT.md and the other dated assessments carry the same claim and are marked "Retained unmodified for diffing and audit history". They stay unmodified; the scorecard-of-record and register corrections are vaulted separately (docs/security/ is git-ignored here). Also NOT done, and it is a real option rather than an oversight: SSLContext.set_ecdh_curve exists and genuinely pins, but takes exactly ONE OpenSSL curve short name -- it cannot express a preference list, and pinning through it would refuse two of the three approved groups. It is a costed hardening knob for a future lane, not a drop-in, and shipping it as though it closed 11.6.2 would be the kind of lenient reading this project has already retracted twice. Also note secp256r1 is a valid group-list alias but NOT a valid EC curve name (that spelling is prime256v1) -- do not "normalise" the constant. Swept the sibling helpers, because "is anything else inert?" is the obvious next question and a claim is not an answer. tls_policy.py has exactly three getattr-guarded best-effort call sites; all three were driven on this interpreter: set_groups (harden_kex_groups) INERT -- the subject of this commit VERIFY_X509_STRICT (harden_verify_flags) LIVE -- present (32), and the flag is verifiably ORed into verify_flags (32768 -> 32800) _hashlib.get_fips_mode (fips_attestation) LIVE -- returns False, not None; None would have meant undeterminable, i.e. a vacuous attestation So this was the only one of the three, and that is measured rather than assumed. --- docs/ASVS-L2-PHASE0-CHANGES.md | 9 +- docs/BACKLOG.md | 2 +- docs/PHI.md | 26 ++++-- ...hop-refusal-refuse-the-insecure-phi-hop.md | 42 +++++++++- messagefoundry/config/tls_policy.py | 63 ++++++++++---- tests/test_api_tls.py | 84 +++++++++++++++---- tests/test_tls_policy.py | 78 ++++++++++++++++- 7 files changed, 255 insertions(+), 49 deletions(-) diff --git a/docs/ASVS-L2-PHASE0-CHANGES.md b/docs/ASVS-L2-PHASE0-CHANGES.md index 1ddc19c7..c432f7b6 100644 --- a/docs/ASVS-L2-PHASE0-CHANGES.md +++ b/docs/ASVS-L2-PHASE0-CHANGES.md @@ -218,9 +218,12 @@ value. Under `[security].enforcement=ENFORCE` a DEK past its max-age + grace **e For the off-loopback transports the TLS floor is **1.2+** (`tls_min_version`), which constrains key exchange to forward-secret **(EC)DHE** suites. The **code-half is now built** (WP-L3-10 free lift): -[config/tls_policy.py](../../messagefoundry/config/tls_policy.py) pins the approved key-exchange groups -(`X25519:secp384r1:secp256r1` via `SSLContext.set_groups` on Python ≥3.13; OpenSSL already defaults to -these on 3.11/3.12) on every built API + MLLP TLS context (`harden_kex_groups`), and a `[api].tls_ciphers` +[config/tls_policy.py](../../messagefoundry/config/tls_policy.py) *attempts* the approved key-exchange +group pin (`X25519:secp384r1:secp256r1` via `SSLContext.set_groups`) on every built API + MLLP TLS +context (`harden_kex_groups`) — but **corrected 2026-07-29: `set_groups` is a Python 3.15 API, not 3.13, +so today it pins nothing and those contexts inherit OpenSSL's default group list** (forward-secret, but +wider than the approved three — see [PHI.md](PHI.md) §4 for the measured accepted set). The helper now +reports what it pinned so the gap is visible rather than assumed. A `[api].tls_ciphers` settings validator (`validate_tls_ciphers`) rejects any non-forward-secret (static-RSA/DH) operator cipher string at config load. On the default `127.0.0.1` bind no TLS is presented, so this is immaterial today; the controls take effect once the engine terminates TLS off-loopback. diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 102ca00e..cbd0440b 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -6293,7 +6293,7 @@ Two findings are worth surfacing here. **Posture B scores worse on Fails than Po ## 200. Transport enforcement: make the code refuse the insecure hop -> ✅ **SHIPPED 2026-07-13 (ADR 0092 + 2026-07-13 amendment) — the posture-keyed transport-hop refusal and ALL its DEFERRED residuals are closed.** The core (2026-07-11): the #200 cleartext-hop refusal **enforces on the primary `serve`/`reload` path, not only at `build_check`** — the live connector-build sites in `pipeline/wiring_runner.py` (`_start_outbound`, `_start_inbound_unsafe`, `_reconcile_outbounds`) stamp the derived `active_hop_posture`, so the raw-TCP/X12/MLLP/DICOM/anon-ftp guards **refuse a production-PHI cleartext outbound at serve**, and the strict verify-off cells (engine⇄store weakened TLS, MLLP/FTPS `tls_verify=false`, credentialed plain-ftp) route `MEFOR_ALLOW_INSECURE_TLS` through the **production-PHI clamp** (`config.settings.weakened_tls_escape_permitted`) so the escape can no longer relax a production-PHI hop. **Residuals now closed (2026-07-13):** (1) the **API PHI-read data-path guard** — `create_app` derives the API serve-hop disposition via the new pure `tls_policy.phi_read_hop_disposition` (reusing the ONE authority + the production-PHI clamp) and `api/security.enforce_phi_read_hop` (folded into `require_phi_read`; explicit on the step-up `search` route) **refuses (403, PHI-free)** a raw-view/attachment-download/summary read on a prod-PHI instance whose serve hop is not proven secure — loopback/TLS/proxy-terminated/synthetic/no-`[ai]` stay byte-identical; (2) the **`db_lookup`/`fhir_lookup` live-read posture stamp** — `_build_lookup_executor`/`_build_fhir_lookup_executor` now wrap construction in `active_hop_posture(self._hop_posture)`, so a prod-PHI weakened-TLS live read is refused (it previously keyed on the UNCLAMPED escape, posture unstamped) and a synthetic cleartext read is no longer false-closed; (3) **`messagefoundry check`** now runs the posture-stamped `build_check_registry` (new required `build-check` in `checks.py`; fail-safe SKIP with no `messagefoundry.toml`), so a prod-PHI cleartext hop is caught at commit/CI; (4) the **Posture-B tails** — a cert-authenticated `GET /service/identity` writes a `service_cert_auth` audit row, runtime-KEX enforcement (`harden_kex_groups` on the in-process context) + a real mutual-TLS handshake are handshake-tested. All compose with the #201 revocation guard / #199 cleartext-egress / #129 expiry-relaxation and never double-refuse a legitimate lane. Tests: `tests/test_hop_refusal_residuals.py` + `tests/test_api_tls.py`. **Genuinely deferred (infra-bound):** a full uvicorn-on-a-real-socket mTLS handshake through the live serve bind (Windows TLS CI legs) — the handshake tests exercise the same `build_api_ssl_context` context, so only the uvicorn wiring is uncovered. _(Re-scored 2026-07-10 → P2; filed by the ASVS 5.0 L3 re-score, PR #854.)_ +> ✅ **SHIPPED 2026-07-13 (ADR 0092 + 2026-07-13 amendment) — the posture-keyed transport-hop refusal and ALL its DEFERRED residuals are closed.** The core (2026-07-11): the #200 cleartext-hop refusal **enforces on the primary `serve`/`reload` path, not only at `build_check`** — the live connector-build sites in `pipeline/wiring_runner.py` (`_start_outbound`, `_start_inbound_unsafe`, `_reconcile_outbounds`) stamp the derived `active_hop_posture`, so the raw-TCP/X12/MLLP/DICOM/anon-ftp guards **refuse a production-PHI cleartext outbound at serve**, and the strict verify-off cells (engine⇄store weakened TLS, MLLP/FTPS `tls_verify=false`, credentialed plain-ftp) route `MEFOR_ALLOW_INSECURE_TLS` through the **production-PHI clamp** (`config.settings.weakened_tls_escape_permitted`) so the escape can no longer relax a production-PHI hop. **Residuals now closed (2026-07-13):** (1) the **API PHI-read data-path guard** — `create_app` derives the API serve-hop disposition via the new pure `tls_policy.phi_read_hop_disposition` (reusing the ONE authority + the production-PHI clamp) and `api/security.enforce_phi_read_hop` (folded into `require_phi_read`; explicit on the step-up `search` route) **refuses (403, PHI-free)** a raw-view/attachment-download/summary read on a prod-PHI instance whose serve hop is not proven secure — loopback/TLS/proxy-terminated/synthetic/no-`[ai]` stay byte-identical; (2) the **`db_lookup`/`fhir_lookup` live-read posture stamp** — `_build_lookup_executor`/`_build_fhir_lookup_executor` now wrap construction in `active_hop_posture(self._hop_posture)`, so a prod-PHI weakened-TLS live read is refused (it previously keyed on the UNCLAMPED escape, posture unstamped) and a synthetic cleartext read is no longer false-closed; (3) **`messagefoundry check`** now runs the posture-stamped `build_check_registry` (new required `build-check` in `checks.py`; fail-safe SKIP with no `messagefoundry.toml`), so a prod-PHI cleartext hop is caught at commit/CI; (4) the **Posture-B tails** — a cert-authenticated `GET /service/identity` writes a `service_cert_auth` audit row, a real mutual-TLS handshake is handshake-tested (**corrected 2026-07-29:** the companion "runtime-KEX enforcement" claim here was wrong — `SSLContext.set_groups` is a Python 3.15 API, so `harden_kex_groups` pins nothing on today's interpreters and the test that asserted the FFDHE refusal self-skipped; measured, the context ACCEPTS ffdhe2048. Both fixed and the residual is now asserted — see the ADR 0092 2026-07-29 amendment and PHI.md §4). All compose with the #201 revocation guard / #199 cleartext-egress / #129 expiry-relaxation and never double-refuse a legitimate lane. Tests: `tests/test_hop_refusal_residuals.py` + `tests/test_api_tls.py`. **Genuinely deferred (infra-bound):** a full uvicorn-on-a-real-socket mTLS handshake through the live serve bind (Windows TLS CI legs) — the handshake tests exercise the same `build_api_ssl_context` context, so only the uvicorn wiring is uncovered. _(Re-scored 2026-07-10 → P2; filed by the ASVS 5.0 L3 re-score, PR #854.)_ **Cluster:** Security & Compliance. **Priority:** P2. **Verdict:** build. **Severity:** medium. diff --git a/docs/PHI.md b/docs/PHI.md index 08c5d525..5b3b3660 100644 --- a/docs/PHI.md +++ b/docs/PHI.md @@ -635,17 +635,27 @@ and a reverse-proxy / forwarded-header alternative are designed in [ADR 0002](adr/0002-phase2-transport-security-and-strong-auth.md) (*Proposed* — build gated on a scheduled off-loopback exposure). -**Key-exchange parameters `[BUILT — WP-L3-10 code half]` (ASVS 11.6.2).** Every TLS context the engine +**Key-exchange parameters `[PARTIAL — the 1.2+ floor and the cipher validator are enforced; the group +pin is INERT until Python 3.15]` (ASVS 11.6.2).** Every TLS context the engine builds — the API/WebSocket listener ([api/tls.py](../messagefoundry/api/tls.py)) and the per-connection MLLP server/client contexts ([transports/mllp.py](../messagefoundry/transports/mllp.py)) — enforces a **TLS 1.2+ floor**, which constrains 1.2 to **(EC)DHE** key exchange and makes 1.3 ECDHE-only: forward- -secret key establishment, never static RSA/DH. Two controls in -[config/tls_policy.py](../messagefoundry/config/tls_policy.py) pin the *parameters*: - -- **Approved groups pinned where supported.** Built contexts call `harden_kex_groups`, which sets the - approved ECDHE groups `X25519:secp384r1:secp256r1` via `SSLContext.set_groups` on Python ≥ 3.13. On - 3.11/3.12 there is no public group-pinning API and OpenSSL's defaults already lead with exactly these - curves, so it is a deliberate no-op, not a downgrade. +secret key establishment, never static RSA/DH. **That floor is the enforced control.** Two further +controls in [config/tls_policy.py](../messagefoundry/config/tls_policy.py) address the *parameters* — and +only the second of them actually takes effect on today's interpreters: + +- **Approved groups are *inherited*, not pinned — corrected 2026-07-29.** Built contexts call + `harden_kex_groups`, which pins the approved ECDHE groups `X25519:secp384r1:secp256r1` via + `SSLContext.set_groups` — an API that lands in **Python 3.15**. This bullet previously said "≥ 3.13", + and the practical effect of the error is that on every interpreter this project currently runs on + (measured: 3.14.6 / OpenSSL 3.5.7) the helper pins **nothing** and every built context inherits + OpenSSL's default group list. That default *is* forward-secret — the property the TLS 1.2+ floor + above exists to guarantee — but it is **wider than the approved list**: measured against the real API + context, it also accepts `ffdhe2048`, `ffdhe3072` and `secp521r1`. It refuses `secp224r1` and + `sect571r1`, so the gap is *wider than policy*, not *weak*. `harden_kex_groups` now **returns the + list it actually pinned** — `None` today — and `tests/test_tls_policy.py` asserts that `None` + unconditionally, so the first interpreter with the API turns the test red instead of letting the + claim drift back. - **`tls_ciphers` is validated, not trusted.** An operator `[api].tls_ciphers` string is rejected at config load if it would admit a **non-forward-secret** (static-RSA/DH) suite, so a misconfiguration cannot widen the key exchange below policy. diff --git a/docs/adr/0092-posture-keyed-transport-hop-refusal-refuse-the-insecure-phi-hop.md b/docs/adr/0092-posture-keyed-transport-hop-refusal-refuse-the-insecure-phi-hop.md index 06e90a7b..ef48ba82 100644 --- a/docs/adr/0092-posture-keyed-transport-hop-refusal-refuse-the-insecure-phi-hop.md +++ b/docs/adr/0092-posture-keyed-transport-hop-refusal-refuse-the-insecure-phi-hop.md @@ -166,10 +166,11 @@ authority above: 4. **Posture-B tails.** (a) A **cert-authenticated intra-service auth** is now **audited**: the `GET /service/identity` route (the only `require_service_cert` surface) writes a `service_cert_auth` row into the tamper-evident chain naming the mapped principal (PHI/secret-free — auth plane + route - only). (b) **Runtime KEX enforcement** is verified: when the engine terminates TLS in-process, + only). (b) ~~**Runtime KEX enforcement** is verified: when the engine terminates TLS in-process, `build_api_ssl_context` pins the approved forward-secret groups (`harden_kex_groups`), and a real handshake test proves a client offering only a non-approved FFDHE group is refused — runtime - enforcement, not the operator attestation the proxy-terminated (Posture-B) case still relies on. (c) A + enforcement, not the operator attestation the proxy-terminated (Posture-B) case still relies on.~~ + **WITHDRAWN 2026-07-29 — see the amendment below.** (c) A real **mutual-TLS handshake** test exercises the exact server context the serve path builds (`CERT_REQUIRED`): a trusted client cert completes, a missing one is refused. **Genuinely deferred (infra-bound):** a full uvicorn-on-a-real-socket mTLS handshake through the live serve bind is left to @@ -207,3 +208,40 @@ every escape-clamp is **byte-identical** to the former production-PHI behaviour two carve-outs of the 2026-07-20 amendment re-key to `enforcement`, and the keyless-PHI ack is renamed `allow_unencrypted_phi_in_production` → `allow_unencrypted_phi_under_strict_enforcement` (see the ADR 0140 amendment). The four hard-refused floor items and the unconditional ePHI audit are unchanged. + +## Amendment (2026-07-29) — §4(b) "runtime KEX enforcement" is WITHDRAWN as a measurement error + +The 2026-07-13 amendment's §4(b) made three assertions. **All three are wrong**, re-measured on the +shipping interpreter (Python 3.14.6 / OpenSSL 3.5.7): + +1. *"`build_api_ssl_context` pins the approved forward-secret groups"* — it does not. + `SSLContext.set_groups` is a **Python 3.15** addition (typeshed guards it at + `sys.version_info >= (3, 15)`), so `harden_kex_groups` returns without pinning at **all six** of its + call sites and the built contexts inherit OpenSSL's default group list. +2. *"a real handshake test proves…"* — that test + (`test_kex_allow_list_enforced_at_runtime`) reached its assertion only through a **client-side** + `set_groups`, so on every interpreter this project runs on it hit `pytest.skip`. It never executed. +3. *"…a client offering only a non-approved FFDHE group is refused"* — the opposite is true. Measured by + pinning a client to one group at a time via `set_ecdh_curve` and handshaking against the real + `build_api_ssl_context`, at both `tls_min_version` 1.2 and 1.3: `ffdhe2048`, `ffdhe3072` and + `secp521r1` are all **accepted**. (`secp224r1` and `sect571r1` are refused.) + +A skip was concealing a false assertion, and this ADR cited it as proof. That is worse than an untested +claim, because it reads as evidence. + +**What this ADR relies on is unaffected.** The forward-secrecy property comes from the **TLS 1.2+ floor**, +which *is* enforced, and the inherited default list is itself forward-secret — the residual is *wider than +policy*, not *weak*. §4(a) (the `service_cert_auth` audit row) and §4(c) (the real mutual-TLS handshake +against the built `CERT_REQUIRED` context) were re-confirmed and stand. + +**Fixed alongside this amendment:** `harden_kex_groups` now **returns the group list it actually pinned** +(`None` today), so the inertness is observable rather than silent; a failed pin returns `None` too, where +it previously fell through as if it had succeeded. The skipping test is replaced by one that *measures* +the accepted-group set and cannot skip, plus an unconditional receipt asserting the `None` — so the first +interpreter to grow the API turns the suite red and forces this amendment, `PHI.md` §4 and the ASVS +11.6.2 row to be re-derived together. See [PHI.md](../PHI.md) §4. + +**Do not re-assert a KEX pin anywhere until `harden_kex_groups` returns non-`None`.** +`SSLContext.set_ecdh_curve` does exist and genuinely constrains the TLS 1.3 `supported_groups`, but it +takes exactly one OpenSSL curve short name — it cannot express a preference list, and pinning through it +would refuse two of the three approved groups. It is a costed hardening option, not a drop-in. diff --git a/messagefoundry/config/tls_policy.py b/messagefoundry/config/tls_policy.py index 219aee0d..66ab3b16 100644 --- a/messagefoundry/config/tls_policy.py +++ b/messagefoundry/config/tls_policy.py @@ -8,9 +8,11 @@ * :func:`validate_tls_ciphers` — reject an operator ``tls_ciphers`` string that would admit a non-forward-secret (non-ECDHE/DHE) key exchange, so a misconfiguration cannot widen the suite below policy. Run from the ``[api].tls_ciphers`` settings validator, so a bad value fails loud at load. -* :func:`harden_kex_groups` — pin the approved ECDHE groups on a built context where the runtime - supports it (``SSLContext.set_groups``, Python 3.13+); on older interpreters OpenSSL already leads - with these groups, so it is a best-effort no-op rather than a downgrade. +* :func:`harden_kex_groups` — *attempt* to pin the approved ECDHE groups on a built context, and + **report whether it managed to**. ``SSLContext.set_groups`` is a **Python 3.15** API (this said + "3.13+" and was wrong), so today it pins nothing on every supported runtime and the contexts inherit + OpenSSL's default group list — forward-secret, but wider than policy. See its docstring for the + measured accepted set; do not cite the groups as "pinned" while it returns ``None``. * :func:`harden_verify_flags` — OR ``ssl.VERIFY_X509_STRICT`` into a verifying context's ``verify_flags`` so a presented chain must be RFC 5280-conformant (ASVS 12.1.4 strict path validation). Revocation itself is delegated to the org PKI / OCSP-must-staple proxy + OS trust store @@ -109,23 +111,54 @@ def fips_attestation() -> tuple[bool | None, str]: return fips_mode, ssl.OPENSSL_VERSION -def harden_kex_groups(ctx: ssl.SSLContext) -> None: - """Best-effort pin ``ctx`` to :data:`APPROVED_KEX_GROUPS`. - - Uses ``SSLContext.set_groups`` where available (Python 3.13+). On older interpreters there is no - public API to pin groups and OpenSSL's defaults already lead with X25519/P-256/P-384, so this is a - deliberate no-op rather than a weakening. A runtime that rejects the group list (an unusual OpenSSL - build) is logged and left at its secure defaults.""" +def harden_kex_groups(ctx: ssl.SSLContext) -> str | None: + """Pin ``ctx`` to :data:`APPROVED_KEX_GROUPS`; return the list pinned, or ``None`` if nothing was. + + **On every runtime this project currently supports this pins NOTHING and returns ``None``.** + ``SSLContext.set_groups`` is a **Python 3.15** addition — typeshed guards it at + ``sys.version_info >= (3, 15)``, alongside the ``get_groups`` that will finally make the pin + *assertable*. This docstring previously said "Python 3.13+"; that was wrong, and it was repeated + into `docs/PHI.md`, `docs/ASVS-L2-PHASE0-CHANGES.md`, ADR 0092 §4(b) and the ASVS scorecard. + Measured on this tree: Python 3.14.6 / OpenSSL 3.5.7, ``hasattr(ctx, "set_groups")`` is ``False``, + so :data:`APPROVED_KEX_GROUPS` reaches **zero** of this function's six call sites and every built + context falls back to OpenSSL's default group list. + + That default *is* forward-secret — which is the property the TLS 1.2+ floor guarantees and the one + ASVS 11.6.2's first clause is about — but it is **wider than the approved list**. Measured against + the real ``build_api_ssl_context``, at both ``tls_min_version`` 1.2 and 1.3, it also accepts + ``ffdhe2048``, ``ffdhe3072`` and ``secp521r1``; it does refuse ``secp224r1`` and ``sect571r1``. So + the residual is *wider than policy*, not *weak*. Until this returns non-``None``, do not describe + the engine's key-exchange groups as "pinned" anywhere — they are **inherited**. + + The **return value is the point.** A security control that cannot report whether it did anything + reports success forever; that is how a call at six sites with zero effect survived three + assessments. ``tests/test_tls_policy.py`` asserts the ``None`` *unconditionally*, so the first + interpreter that grows the API turns that test red — which is the signal to re-derive this + docstring, `docs/PHI.md` §4 and the 11.6.2 row, and to switch the test to ``get_groups()``. + + Two traps for whoever does that work: + + * ``SSLContext.set_ecdh_curve`` *does* exist and genuinely constrains the TLS 1.3 + ``supported_groups`` (verified: a server pinned to ``prime256v1`` refuses a client pinned to + ``secp384r1`` with ``NO_SUITABLE_KEY_SHARE``, while the unpinned control accepts it). It is not a + substitute here — it takes exactly ONE OpenSSL curve short name, so it cannot express a + preference list, and pinning through it would refuse two of the three approved groups. + * ``secp256r1`` is a valid OpenSSL *group-list* alias but **not** a valid EC curve name (that + spelling is ``prime256v1``, and ``set_ecdh_curve("secp256r1")`` raises). Both names are correct + in their own API. Do not "normalise" the constant. + """ set_groups = getattr(ctx, "set_groups", None) if set_groups is None: - return + return None try: set_groups(APPROVED_KEX_GROUPS) - except ( - ssl.SSLError, - ValueError, - ) as exc: # pragma: no cover - depends on the linked OpenSSL build + except (ssl.SSLError, ValueError) as exc: + # None, not the list: a pin that RAISED must never read back as a pin that took. (The former + # `# pragma: no cover` here is gone — this branch is now driven by a stand-in context in + # tests/test_tls_policy.py rather than left to an unusual OpenSSL build to exercise.) logger.warning("Could not pin TLS key-exchange groups %r: %s", APPROVED_KEX_GROUPS, exc) + return None + return APPROVED_KEX_GROUPS def harden_verify_flags(ctx: ssl.SSLContext) -> None: diff --git a/tests/test_api_tls.py b/tests/test_api_tls.py index 8106b819..deb0efbe 100644 --- a/tests/test_api_tls.py +++ b/tests/test_api_tls.py @@ -1253,28 +1253,80 @@ def _verifying_client_ctx(ca: Path) -> ssl.SSLContext: return ssl.create_default_context(ssl.Purpose.SERVER_AUTH, cafile=str(ca)) -def test_kex_allow_list_enforced_at_runtime(tmp_path: Path) -> None: - # The engine terminates TLS in-process, so it ENFORCES the approved forward-secret KEX groups at - # runtime (harden_kex_groups on the built context), not merely as an operator attestation. Prove it: - # a client pinned to ONLY a non-approved FFDHE group cannot complete against the pinned server, while - # a default client negotiates a forward-secret TLS 1.3 suite. +#: Key-exchange groups OUTSIDE `APPROVED_KEX_GROUPS` that a client can pin via `set_ecdh_curve`. +#: `x448` and `X25519MLKEM768` are deliberately absent — they are TLS groups but not EC curve NAMES, so +#: `set_ecdh_curve` raises on them and this technique cannot measure them. +_NON_APPROVED_KEX_PROBES = ("ffdhe2048", "ffdhe3072", "secp521r1", "secp224r1", "sect571r1") + +#: The approved groups, spelled for `set_ecdh_curve` — `prime256v1`, never `secp256r1` (see +#: APPROVED_KEX_GROUPS: the group-list alias and the EC curve name differ for exactly this curve). +_APPROVED_KEX_PROBES = ("X25519", "secp384r1", "prime256v1") + + +def _kex_group_accepted(server_ctx: ssl.SSLContext, ca: Path, group: str) -> bool: + """Does ``server_ctx`` complete a handshake with a client offering ONLY ``group``?""" + client = _verifying_client_ctx(ca) + # A ValueError here is a bug in this test's group table, not a finding — let it raise loudly rather + # than degrade into "not accepted", which would silently manufacture the result we want. + client.set_ecdh_curve(group) + try: + return bool(_handshake(server_ctx, client, client_cert=None)) + except OSError: + return False + + +def test_which_kex_groups_the_built_context_actually_accepts(tmp_path: Path) -> None: + """MEASURE the built context's key-exchange groups (ASVS 11.6.2). + + This replaces a test that asserted the opposite of the truth and stayed green by skipping. It read: + "a client offering ONLY ffdhe2048 ... shares no key-exchange group with the pinned server, so the + handshake is refused — runtime enforcement". It reached that assertion only through client-side + ``set_groups``, a **Python 3.15** API, so on every interpreter this project runs on it hit + ``pytest.skip``. And the claim was false: measured, this context ACCEPTS ffdhe2048, because + ``harden_kex_groups`` pins nothing. A skip was concealing a wrong assertion, which is worse than no + test — ADR 0092 §4(b) cited it as proof. + + ``set_ecdh_curve`` is the API that does exist, and it genuinely constrains the TLS 1.3 + ``supported_groups``: verified separately that a server pinned to ``prime256v1`` refuses a client + pinned to ``secp384r1`` (``NO_SUITABLE_KEY_SHARE``) while the unpinned control accepts it. One group + per handshake is enough to enumerate what the server will take. + + Asserted as INVARIANTS, not as an exact table: the accepted set comes from the linked OpenSSL's + default group list and a CI leg may link a different build. What must hold is that *every* approved + group gets in (else the server is over-restricted, or the harness is broken, and the second + assertion would pass for the wrong reason) and that *some* non-approved group gets in (proving the + approved list is not enforced). The measured table is printed on failure either way. + + Mutation: make ``_kex_group_accepted`` return True unconditionally. Red — the "genuinely weak curves + stay out" assertions fail, so a probe that cannot distinguish anything is caught. + """ ca, cert, key = _strict_ca_and_leaf(tmp_path) server_ctx = build_api_ssl_context( ApiSettings(tls_cert_file=str(cert), tls_key_file=str(key), tls_min_version="1.3") ) - # Positive control: a normal client (approved groups) handshakes and negotiates a TLS 1.3 suite. + + # Positive control first: a default client must negotiate a forward-secret TLS 1.3 suite at all. cipher = _handshake(server_ctx, _verifying_client_ctx(ca), client_cert=None) - assert cipher and cipher.startswith("TLS_") # a TLS 1.3 (ECDHE, forward-secret) suite + assert cipher and cipher.startswith("TLS_") - # Negative: a client offering ONLY ffdhe2048 (a valid TLS 1.3 group the server does NOT list) shares - # no key-exchange group with the pinned server, so the handshake is refused — runtime enforcement. - bad_client = _verifying_client_ctx(ca) - try: - bad_client.set_groups("ffdhe2048") - except (AttributeError, ssl.SSLError, ValueError): - pytest.skip("runtime does not support set_groups on the client side") - with pytest.raises(OSError): - _handshake(server_ctx, bad_client, client_cert=None) + approved = {g: _kex_group_accepted(server_ctx, ca, g) for g in _APPROVED_KEX_PROBES} + non_approved = {g: _kex_group_accepted(server_ctx, ca, g) for g in _NON_APPROVED_KEX_PROBES} + table = f"approved={approved} non_approved={non_approved}" + + assert all(approved.values()), f"the built context refuses an APPROVED group — {table}" + + # The residual of record: the approved list is NOT enforced, because a group outside it gets in. + leaked = sorted(g for g, ok in non_approved.items() if ok) + assert leaked, ( + "no non-approved key-exchange group was accepted, so the built context now DOES constrain " + "groups to the approved list. That is an improvement, not a test failure — re-score ASVS " + f"11.6.2, update docs/PHI.md §4 and the register, then tighten this test. {table}" + ) + + # ...and the genuinely weak curves stay out, so the residual is "wider than policy", not "insecure". + # If either of these fails it is a real finding on that OpenSSL build, not a flake. + assert not non_approved["secp224r1"], f"a 112-bit-strength curve was accepted — {table}" + assert not non_approved["sect571r1"], f"a binary-field curve was accepted — {table}" def test_real_mutual_tls_handshake_on_built_context(tmp_path: Path) -> None: diff --git a/tests/test_tls_policy.py b/tests/test_tls_policy.py index 8e5dd758..014639af 100644 --- a/tests/test_tls_policy.py +++ b/tests/test_tls_policy.py @@ -111,17 +111,87 @@ def test_validate_rejects_non_forward_secret() -> None: # --- harden_kex_groups ------------------------------------------------------------------------- def test_harden_does_not_raise_on_real_context() -> None: ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) - harden_kex_groups(ctx) # no-op pre-3.13, set_groups on 3.13+ — either way must not raise + harden_kex_groups(ctx) # must never raise, whatever the runtime can or cannot pin -def test_harden_is_noop_without_set_groups() -> None: - # A runtime/object lacking set_groups is handled gracefully (older interpreters). +def test_the_group_pin_is_inert_on_this_runtime_and_says_so() -> None: + """A liveness receipt for ASVS 11.6.2 — written to FAIL on the interpreter upgrade. + + ``SSLContext.set_groups`` is a **Python 3.15** addition, so ``harden_kex_groups`` pins nothing on + any interpreter this project runs on and ``APPROVED_KEX_GROUPS`` reaches zero of its six call + sites. That was already true; what was missing was any way to NOTICE. The tests that stood here + asserted (a) that the call does not raise, (b) that it no-ops on an object without the API, and (c) + the contents of a string constant — all three pass identically whether or not a single group is + ever pinned. That is how the docstring came to claim "Python 3.13+" and how ADR 0092 §4(b), PHI.md + §4 and the ASVS scorecard all came to assert a control that does not execute. + + Asserted **unconditionally** on purpose: an ``if hasattr(ctx, "set_groups")`` branch here would + restore exactly the property being removed — a test that cannot fail. + """ + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + pinned = harden_kex_groups(ctx) + assert pinned is None, ( + f"harden_kex_groups pinned {pinned!r}, so this interpreter HAS a group-list API and the ASVS " + f"11.6.2 residual has CHANGED. This is good news, not a bug. Re-derive, in one change: the " + f"harden_kex_groups docstring, docs/PHI.md §4, docs/ASVS-L2-PHASE0-CHANGES.md, the ADR 0092 " + f"§4(b) amendment and the 11.6.2 register row — then rewrite this test to assert the pin TOOK, " + f"via ctx.get_groups(), which lands in the same Python version." + ) + assert not hasattr(ctx, "set_groups"), ( + "set_groups EXISTS but harden_kex_groups still returned None, so the pin is failing silently — " + "worse than being unavailable, because the docs would read as satisfied. See the " + "logger.warning path." + ) + + +def test_harden_reports_none_without_set_groups() -> None: + # An object lacking the API must be handled gracefully AND report that nothing was pinned. fake = types.SimpleNamespace() - harden_kex_groups(fake) # type: ignore[arg-type] + assert harden_kex_groups(fake) is None # type: ignore[arg-type] + + +def test_harden_reports_the_list_it_pinned_when_the_api_exists() -> None: + """The return value must be REAL, not incidentally-``None``. + + ``test_the_group_pin_is_inert_on_this_runtime_and_says_so`` asserts ``is None`` — which a function + with no ``return`` statement at all also satisfies. On its own it therefore does NOT prove the + reporting works: reverting this helper to its old ``-> None`` signature would leave it green. Drive + the pinning path with a stand-in context that *has* ``set_groups`` so both halves of the contract + are covered, and assert the group list actually reached it. + """ + + class _Pinnable: + def __init__(self) -> None: + self.pinned: list[str] = [] + + def set_groups(self, grouplist: str) -> None: + self.pinned.append(grouplist) + + ctx = _Pinnable() + assert harden_kex_groups(ctx) == APPROVED_KEX_GROUPS # type: ignore[arg-type] + assert ctx.pinned == [APPROVED_KEX_GROUPS], "the group list never reached set_groups" + + +def test_a_pin_that_raises_reports_nothing_pinned() -> None: + """An OpenSSL build that REJECTS the group list must report ``None``, not the list. + + This was the shipped bug: the helper logged the warning and then fell off the end, so the caller + could not distinguish "pinned" from "tried and failed" — and once the helper started reporting, the + failure path returning the list would have been a lie with a warning line next to it. + """ + + class _Rejecting: + def set_groups(self, grouplist: str) -> None: + raise ValueError("this OpenSSL build rejects the group list") + + assert harden_kex_groups(_Rejecting()) is None # type: ignore[arg-type] def test_approved_groups_are_ecdhe_curves() -> None: assert APPROVED_KEX_GROUPS.split(":") == ["X25519", "secp384r1", "secp256r1"] + # NB `secp256r1` is a valid OpenSSL group-list alias but NOT a valid EC curve name — that spelling + # is `prime256v1`, and set_ecdh_curve("secp256r1") raises ValueError. Both are correct in their own + # API; do not "normalise" them to one. # --- harden_verify_flags ----------------------------------------------------------------------- From f70b0fc273b77d70201d716ef34572e50d21247b Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 16:52:06 -0500 Subject: [PATCH 3/3] docs(adr-0034): record the py/insecure-protocol dismissal on the 12.1.1 TLS probe CodeQL alert 145 -- py/insecure-protocol, HIGH, messagefoundry/config/tls_probe.py -- was raised by PR #53 and dismissed `won't fix` on the repo. ADR 0034's policy is that every finding is triaged to Fix or Dismiss-with-a-recorded-reason, with the per-alert rationale in the GitHub comment and the class rationale in the ADR. The GitHub comment is capped at 280 characters, so the ADR is where the real argument has to live; this is that entry. The rule is factually right and does not apply: tls_probe.py offers TLS 1.0/1.1 BECAUSE a successful handshake at a withdrawn version is the finding it exists to produce. Two flagged settings are load-bearing and mutation-proven -- SECLEVEL=0 (without it OpenSSL sends no ClientHello, so the probe would measure our refusal to ask rather than their refusal to answer) and CERT_NONE (an untrusted internal CA would abort before the version was settled, reporting "TLS 1.0 refused" for a door never knocked on). What makes the dismissal safe is scope, so the row states it: client contexts only, built in that module, one handshake, never returned to a caller, no application data, no PHI. Not a data path, and the settings must never be reused for one -- which the module docstring and the crypto-inventory row both already say, at the two places a future author would actually look. Also recorded per this ADR's own convergence rule: the anchor is tls_probe.py:146 inside _offer_context, and that module is new and small, so this WILL re-fire as a fresh alert number the first time anything is inserted above line 146. Re-dismiss with this rationale rather than re-triaging from scratch. --- ...is-triage-policy-accepted-risk-register.md | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/adr/0034-static-analysis-triage-policy-accepted-risk-register.md b/docs/adr/0034-static-analysis-triage-policy-accepted-risk-register.md index 8333f3e3..c376bcbd 100644 --- a/docs/adr/0034-static-analysis-triage-policy-accepted-risk-register.md +++ b/docs/adr/0034-static-analysis-triage-policy-accepted-risk-register.md @@ -221,3 +221,35 @@ an execution. Before the next tag, run `security.yml`'s sbom job via `workflow_d log — the install command there is byte-identical to `release.yml`'s, and `test_sbom_install_is_byte_identical_in_release_and_security` now enforces that identity, because the dry-run is evidence about the release step only for as long as the two commands are the same command. + +## Amendment — 2026-07-29: `py/insecure-protocol` on the ASVS 12.1.1 TLS-floor probe + +**Alert 145 — `py/insecure-protocol`, HIGH, `messagefoundry/config/tls_probe.py`. Dismissed `won't fix`.** + +CodeQL is factually right and the finding does not apply. `tls_probe.py` is the **measurement** for ASVS +12.1.1: at startup, on a PHI instance behind a *declared* upstream TLS terminator under `enforce`, it dials +the operator's own `public_origin` and offers TLS 1.0 and 1.1. **A successful handshake is the finding** — +it proves the front door accepts a protocol NIST SP 800-52r2 withdrew, and the engine refuses to serve. +Offering the withdrawn version *is* the control; there is no implementation that measures whether a peer +accepts TLS 1.0 without asking it to. + +Two settings the rule flags are load-bearing and mutation-proven in `tests/test_tls_floor_probe.py`: + +- `minimum_version == maximum_version == TLSv1` **plus `ALL:@SECLEVEL=0`** — without the security-level + drop, modern OpenSSL will not even *send* the ClientHello, so the probe would measure **our** refusal to + ask rather than **their** refusal to answer, and a permissive front door would read as clean. Dropping + `SECLEVEL=0` is one of the five mutations that turn the suite red. +- `CERT_NONE` — the probe measures the **protocol floor**. An internal CA the engine does not trust would + abort the handshake *before the version was settled*, reporting "TLS 1.0 refused" for a door that was + never knocked on. Chain validation is a separate control (12.1.4 / `harden_verify_flags`). + +**Scope, which is what makes the dismissal safe:** client contexts only, constructed in this module, used +for exactly one handshake, never returned to a caller, carrying no application data and no PHI. This is +**not a data path** and these settings must never be reused for one — the module docstring says so, and +the crypto-inventory row (`scripts/security/crypto_inventory_check.py`) repeats the warning at the place a +future author would look. Every TLS scanner (`testssl.sh`, `sslyze`, `nmap ssl-enum-ciphers`) is built the +same way; suppressing this rule for a scanner is the industry-standard disposition, not a local shortcut. + +**Convergence note (per the rule above):** the anchor is `tls_probe.py:146`, inside `_offer_context`. That +module is new and small, so expect this to re-fire as a fresh alert number the first time anything is +inserted above line 146. Re-dismiss with this rationale rather than re-triaging from scratch.