feat(tls): assert forward secrecy on every built context, don't inherit it (ASVS 12.1.2) - #49
Merged
Merged
Conversation
…it it (ASVS 12.1.2)
validate_tls_ciphers already rejects a CONFIGURED tls_ciphers/proxy_tls_ciphers
that admits static RSA/DH -- but it only fires when an operator sets the knob. A
context built without one inherits the interpreter's default suite list and
nothing checked it.
That inheritance-without-assertion is the real residual. The residual of record
-- "No cipher knob on MLLP/DICOM/FTPS/REST/SOAP at all" -- OVERSTATES the
defect, and correcting our own residual is part of this cell's evidence: an
assessor who runs get_ciphers() sees the correction came from us.
harden_cipher_suites(ctx, connector=...) asserts and raises at all six context
sites (API/UI listener, DICOM listener + destination, MLLP listener +
destination, FTPS).
MEASURED, not asserted: on CPython 3.14.6 / OpenSSL 3.5.7 all four shipped
context shapes resolve to 17 suites with ZERO non-forward-secret. So this raises
on no supported configuration today -- it converts an inherited property into a
checked one, at zero interop risk.
Deliberately NOT shipped: the obvious set_ciphers("ECDHE+AESGCM:ECDHE+CHACHA20:
DHE+AESGCM:!aNULL:!eNULL") preference string. Measured against the shipped
default it REMOVES six CBC-SHA2 suites that real MLLP/DICOM hospital peers still
speak -- a genuine interop regression -- and ADDS two DSS suites the default did
not enable. The default order already leads with TLS_AES_256_GCM_SHA384, so
"strongest first" holds without touching it. Dropping that item is what makes
this S rather than M.
The evidence test PRINTS what it measured ("17 suites examined, 0
non-forward-secret" per shape) rather than swallowing it through capsys:
capturing would verify the report was produced while hiding it from the reader,
which defeats the point. `pytest -s -k forward_secret` IS the artefact.
Four mutations, each red for its own reason. Two of them exist because the first
pass MISSED them: with the assertion wired at six sites and two tests passing,
deleting the call from an MLLP site stayed GREEN -- both tests exercised the
FUNCTION, not its WIRING, so a new listener could have shipped an unasserted
context unnoticed, which is exactly how the original residual arose. Added a
DERIVED call-site guard (every site pinning kex groups must also assert;
harden_kex_groups already marks every place the engine builds a context) plus a
liveness receipt so the rglob cannot pass over nothing.
Untouched on purpose: harden_kex_groups. `ssl.SSLContext.set_groups` does not
exist on 3.14.6, so it pins nothing at any of its six call sites; the proposed
"call it on the five uncovered contexts" fix would add five more no-ops behind a
test that can self-skip. That is a separate, honest correction -- not this cell.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ASVS 12.1.2 — cipher suites / forward secrecy. The cheapest genuine Pass in the remediation
packet, and the measurement that makes it safe was re-verified on this tree before building.
The actual residual
validate_tls_ciphersalready rejects a configuredtls_ciphers/proxy_tls_ciphersthat admitsstatic RSA/DH — but it only fires when an operator sets the knob. A context built without one
inherits the interpreter's default suite list and nothing checked it.
That inheritance-without-assertion is the real gap. The residual of record — "No cipher knob on
MLLP/DICOM/FTPS/REST/SOAP at all" — overstates the defect, and correcting our own residual is part
of this cell's evidence: an assessor who runs
get_ciphers()sees the correction came from us.harden_cipher_suites(ctx, connector=...)now asserts and raises at all six context sites (API/UIlistener, DICOM listener + destination, MLLP listener + destination, FTPS).
Measured, not asserted
On CPython 3.14.6 / OpenSSL 3.5.7 all four shipped context shapes resolve to 17 suites, zero
non-forward-secret. This raises on no supported configuration today — it converts an inherited
property into a checked one, at zero interop risk.
The evidence test prints what it measured (
17 suites examined, 0 non-forward-secretper shape)rather than swallowing it through
capsys: capturing would verify the report was produced whilehiding it from the reader, which defeats the point.
pytest -s -k forward_secretis the artefact.Deliberately not shipped
The obvious
set_ciphers("ECDHE+AESGCM:ECDHE+CHACHA20:DHE+AESGCM:!aNULL:!eNULL")preference string.Measured against the shipped default it removes six CBC-SHA2 suites real MLLP/DICOM hospital peers
still speak — a genuine interop regression — and adds two DSS suites the default did not enable, on
the one cell whose entire evidence story is "we assert exactly what is enabled". The default order
already leads with
TLS_AES_256_GCM_SHA384, so "strongest first" holds without touching it.Also untouched:
harden_kex_groups.ssl.SSLContext.set_groupsdoes not exist on 3.14.6, so itpins nothing at any of its six call sites; the proposed "call it on the five uncovered contexts" fix
would add five more no-ops behind a test that can self-skip. Separate, honest correction — not this
cell.
The guard did not work when first written
With the assertion wired at six sites and two tests passing, deleting the call from an MLLP site
stayed green — both tests exercised the function, not its wiring. A new listener could have
shipped an unasserted context unnoticed, which is exactly how the original residual arose.
Added a derived call-site guard — every site that pins kex groups must also assert, since
harden_kex_groupsalready marks every place the engine builds and hardens a context — plus a livenessreceipt so the
rglobcannot pass over nothing. Four mutations now red: neutralise the assertion ·break the FS predicate · drop the call from an MLLP site · drop it from the API listener.
Verification
9571 passed. Two failures, both environmental and both reproduced without this change:
test_installed_metadata_matches_dunder_version(this worktree has no.venv, so a stale0.3.0install is measured against
0.3.2source) andtest_the_installed_gate_matches_the_committed_source(main's #43 changed the gate source; the installed user-scope snapshot needs
install-gate.ps1re-run). This diff is six TLS files and touches no gate file.
🤖 Generated with Claude Code