feat(tls): let the HTTP egress family and both Vault hops name an internal CA (BACKLOG #1180) - #878
Open
wshallwshall wants to merge 3 commits into
Open
feat(tls): let the HTTP egress family and both Vault hops name an internal CA (BACKLOG #1180)#878wshallwshall wants to merge 3 commits into
wshallwshall wants to merge 3 commits into
Conversation
added 3 commits
September 4, 2026 18:33
…ernal CA (BACKLOG #1180)
resolve_trust_anchor reached mllp, dicom and remotefile and nothing else. The
whole HTTP egress family exposed only a verify_tls boolean, and both
hvac.Client sites passed no CA argument at all, so an operator who set
[tls].internal_ca_file had it honoured on some hops and silently ignored on
every https one. This is the expressibility limb of ASVS 12.3.4: the anchor
could not be spoken there, not merely that it ships off.
Threaded through one construction point. build_anchored_https_handler in
config/tls_policy.py is the only place an HTTP-family https handler is now
built, so the five call sites cannot drift.
- system / loopback / no internal CA -> build_asserted_https_handler
verbatim: urllib's OWN context, asserted in place, unchanged.
- augment -> urllib's own context with the internal CA loaded into it.
Adding a root needs no new context, so nothing is rebuilt or replayed.
- pinned (and a per-connection CA, the same shape) -> the one arm that
substitutes a context, because it must trust ONLY the internal CA and an
SSLContext cannot unload roots urllib already loaded. It replays urllib's
two deltas over create_default_context so pinning a hop does not silently
drop its ALPN advertisement and post-handshake auth.
Reached by RestDestination, SoapDestination (including the mutual-TLS opener,
which builds its own context), FhirDestination, DicomWebDestination and the
fhir_lookup executor's per-connection opener map. The lookup executor has no
Destination to carry the policy, so the runner threads it in explicitly at
both the live and build-check construction sites.
The Vault hops take MEFOR_STORE_VAULT_CA_FILE and MEFOR_SECRETS_VAULT_CA_FILE,
resolved through the same resolve_trust_anchor and mapped to requests' single
verify= bundle path. augment is REFUSED there rather than silently narrowed:
one path cannot say "public roots plus this CA". The keyword is omitted when
nothing is configured, so the stock client is constructed exactly as before.
Note the direction on that hop: hvac rides requests, whose default bundle is
the PUBLIC certifi roots, not the OS store. An internal-CA Vault failed closed
rather than being broadly trusted, so what was missing was the ability to
reach such a Vault at all.
Nothing changes for an operator who configures nothing. A stock https
destination is still handed the shared _NO_REDIRECT_OPENER by identity; a hop
takes a per-connection opener only when it carries an extra handler or an
anchor that narrows.
Twelve mutations were run against the new assertions and all twelve go red,
including two negative controls: one that makes an unanchored hop substitute a
context, and one that gives every hop a per-connection opener.
The connected-systems table said the two hvac hops carry "hvac/requests' own default -- the engine sets no explicit client TLS options here". That is no longer true when MEFOR_STORE_VAULT_CA_FILE or MEFOR_SECRETS_VAULT_CA_FILE is set, and a security table asserting the engine sets nothing where it now sets an anchor is the stale-doc shape section 11 forbids. Both rows now say what the default is (requests' PUBLIC bundle, not the OS store) and name the env var that narrows it. The secrets-provider section of CONFIGURATION.md gains the same pointer beside the address and token it belongs with.
…still open Appends a progress note to #1180: what was re-measured (and with which positive control), what was wired, the two of the item's own claims the work corrects, and the five things measured and deliberately not done. The status banner is NOT flipped. This item closes by a vault scorecard re-score plus a lander flip, neither of which a builder may do, and the note says in terms that expressibility is not a pass: the cell's own re-score trigger fires on exactly this change, so re-scoring on it while every default is still permissive is the move the record already rules out.
Collaborator
Author
|
Manager note. No Read this for, named rather than left to be found:
|
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.
What this is
The expressibility limb of BACKLOG #1180 (ASVS 12.3.4).
resolve_trust_anchorreachedmllp,dicomandremotefileand nothing else, so an operator who set[tls].internal_ca_filehad it honoured on some hops and silently ignored on every https one. This makes the anchor speakable there. It does not flip a default and it does not settle the posture question.The measurement, and its positive control
Re-measured by execution at HEAD
a2eef0f37, because this item's research is dated 2026-08-20 and a probe that finds nothing everywhere is indistinguishable from a clean repo.transports/rest.pytransports/soap.pytransports/fhir.pytransports/dicomweb.pytransports/mllp.pytransports/dicom.pytransports/remotefile.pyThe three controls fire on all four probes, so the probe can see the thing.
For the Vault hops, a recording fake
hvacwas installed and the real factories called. Both_build_clientsites passed exactly{url, token, allow_redirects}and no CA-bearing keyword — and the recorder is demonstrably working, because it saw those three. Peer PR 760/#1317'sassert_hvac_tls_suitesis not onmain.Both stated shortfalls reproduce.
What was wired
One construction point:
build_anchored_https_handlerinconfig/tls_policy.pynow builds every HTTP-family https handler, so the call sites cannot drift.build_asserted_https_handlerverbatim: urllib's OWN context, asserted in place.SSLContextcannot unload roots urllib has already loaded. That arm replays urllib's two measured deltas (set_alpn_protocols(["http/1.1"]),post_handshake_auth) so pinning a hop does not silently change its handshake.Reached by
RestDestination,SoapDestination(including the mutual-TLS opener, which builds its own context),FhirDestination,DicomWebDestination, and thefhir_lookupexecutor's per-connection opener map. That last has noDestinationto carry the policy, soRegistryRunnerthreads it in explicitly at both the live andbuild_checksites.Both hvac hops take
MEFOR_STORE_VAULT_CA_FILE/MEFOR_SECRETS_VAULT_CA_FILE, resolved through the sameresolve_trust_anchorand mapped ontorequests' singleverify=bundle path.augmentis refused there rather than silently narrowed — one path cannot say "public roots plus this CA".Note the direction on that hop: hvac rides
requests, whose default bundle is the PUBLIC certifi roots, not the OS store. An internal-CA Vault failed closed rather than being broadly trusted, so what was missing was the ability to reach one at all. That corrects this item's severity clause, as the 2026-08-20 research already noted.How the default is proved unchanged
Not asserted about — asserted.
_NO_REDIRECT_OPENERby object identity, including under a[tls]block insystemmode and under apinnedpolicy on a loopback host.{url, token, allow_redirects}when no CA is configured — the keyword is omitted, not passed as an explicit default.Mutation proof
Twelve mutations, each applied and restored in turn, full suite re-run each time. All twelve go red, and the baseline and restored runs are green.
M11 and M12 are the ones that matter for the negative claim: they break the default, and the default assertions catch them.
Checks run
Locally, on this branch:
ruff checkandruff format --checkovermessagefoundryandtests— clean.mypy messagefoundrystrict — clean, 267 files.pytest:test_tls_trust_anchor,test_tls_cipher_assertion_sites,test_rest_transport,test_soap_transport,test_soap_wssecurity,test_soap_body_secrets,test_fhir_transport,test_fhir_lookup,test_dicomweb,test_keyprovider_vault,test_keyprovider,test_secretprovider,test_smart_backend,test_outbound_forward_proxy,test_ech_egress,test_hop_refusal_http,test_hop_refusal_revocation,test_hop_refusal_wiring,test_connection_tls_loosenings,test_connection_schema,test_egress_allowlist,test_api_tls,test_mllp_tls,test_alert_smtp_tls,test_harness_tls_anchor— all green.scripts/docs/backlog_status_check.py— OK, 664 items, each declaring exactly one status.Not run locally, please read on CI: the full suite (the box is contended; a full local run reached 4 percent in 18 minutes when measured), the SQL Server and Postgres store legs, the load leg, the IDE leg, and
windows-service-smoke. The store legs are worth a look —store/keyprovider_vault.pygained a module-scope import ofconfig/tls_policy(stdlib-only, andconfig/settings.pyalready imports it at module scope, so it is warm before this module loads by name).One pre-existing test needed a one-line fix
tests/test_soap_wssecurity.py::test_client_cert_opener_loads_chain_and_floors_tlsmonkeypatchedssl.create_default_contextwith a zero-argument lambda. The SOAP mTLS opener now delegates tobuild_verifying_client_context, which passes thessl.Purposepositionally the way the stdlib is called everywhere else. The stub now takes the real signature's arguments. No assertion was weakened.Measured and deliberately NOT done
Named so nobody reads this PR as the limb closed.
tls_ca_fileparameter onRest(),Soap(),FHIR(),DICOMweb()andFhirLookup(). The code reads that key, precedence rule 1 ofresolve_trust_anchorhonours it, and no authoring surface can set it — exactly what this item's own factory probe found forFtp(). The instance[tls]anchor is what this pass made expressible; the connection-scoped half is a separate signature change that flows into the introspected GUI schema.transports/smart.py), built from inside the veryFhirDestination.__init__this pass changes. Inpinnedmode a FHIR destination now anchors its data hop while the hop carrying itsclient_assertionstill verifies against the OS store. It is not a one-line thread-through: the anchor must resolve for the TOKEN host, which has its own loopback and proxy decisions (ADR 0126 already splits it), so it needs the policy passed intotoken_provider_from_settings, not the destination's resolved anchor.pipeline/alert_sinks.py, whosenotifier_from_settingsalready receives atrust_anchor_policyand threads it toEmailTransportonly.[tls].internal_ca_filereaching the two Vault hops. Neither provider has that section in scope — they hold aStoreSettings/SecretsSettingsand build from the environment — so it needs threading throughresolve_key_provider/resolve_secret_provider. That is the "new settings" this item's research already named. The env vars are the honest minimum today, andvault_client_verify_kwargssays so in its docstring rather than claiming more.SSLContextat construction, andbuild_checkbuilds a second it discards. Startup-only, and caching a mutableSSLContextacross connectors is its own hazard, so it was left alone rather than done quietly.Out of scope, untouched
trust_anchor_modestill defaults to"system"; the pinned-without-a-CA validator is untouched.The trap this PR fires by construction — please read before re-scoring
This item records that cell 12.3.4's own re-score trigger watches for the anchor resolver or a CA parameter appearing on these HTTP factories. This change fires that trigger deliberately. Expressibility is not a pass. Re-scoring 12.3.4 on this alone, while every default is still permissive, would be wiring an existing toolkit into the pipeline so an absence claim stops firing — which the record already rules out. The BACKLOG note says the same thing in the same words, and the status banner is not flipped.
Recommendation on the posture ruling (a recommendation only — this is an owner call)
Building this changed my view of the question, so it is worth recording.
Do not refuse an anchor-absent internal hop. The shipped ladder ALLOWs a verified-and-authenticated hop by written decision (
forward_hop_disposition's first branch, with the in-code comment "an encrypted+authenticated hop, nothing to gate"), and refusing an unnarrowed but verified hop inverts that. That is a new policy, not an extension of an existing one.Report it instead. The cheaper and more honest control is the public-chain probe this item's research already identifies, and the work here makes it reachable on almost every hop it covers: validate the peer chain against a public-only bundle. If it validates, the verb's condition demonstrably does not obtain. If it does not, an internal or self-signed certificate is demonstrably in use and a specific anchor is required. That is a measurable predicate, and it needs no operator attestation, which matters because
checks.py:1529already records the pressure a blocking declaration creates.One thing the build makes concrete for whoever takes the ruling:
augmentdoes not satisfy the word ONLY. It keeps the public roots, so a hop inaugmentmode is not narrowed in the verb's sense. This PR refusesaugmenton the tworequests-based hops because it is not expressible there, but on the urllib hops it is accepted and it does not meet the verb. That disposition is on this item's proposed-work list and is still owed.