feat(tls): outbound CRL revocation checking and the blanket-attestation clamp (BACKLOG #299 limb A) - #1208
feat(tls): outbound CRL revocation checking and the blanket-attestation clamp (BACKLOG #299 limb A)#1208wshallwshall wants to merge 5 commits into
Conversation
… an enforcing hop (BACKLOG #299) `revocation_hop_disposition` returned ALLOW on `attested` BEFORE its `enforcing -> REFUSE` arm, and `RevocationHopGuard.capture` OR'd the process-wide `MEFOR_TLS_REVOCATION_ATTESTED` env into that same `attested` argument. So one environment variable, set once, returned ALLOW for EVERY verifying outbound TLS hop in the instance -- defeating an enforcing posture on hops the operator never enumerated. On first deployment that would let a revoked-but-unexpired peer certificate be accepted on any such hop while the instance reported an enforcing posture. The two claims are now separate inputs and are ranked differently. A per-connection `tls_revocation_attested` is a claim about one named hop a reviewer can check against that hop's PKI, and it still crosses an enforcing hop. The blanket env is a claim about all of them at once, and it now ranks BELOW the enforcing refusal. Under a non-enforcing posture it still ALLOWs, byte-identical to the pre-clamp behaviour, so nothing outside an enforcing posture moves. Adds `crl_checked` as a relaxation in the same gradient, ahead of the refusal: a hop whose OWN context loads a CRL and sets VERIFY_CRL_CHECK_LEAF does check revocation, so the gap this gate exists for is closed rather than declared away. `context_checks_revocation` reads that flag off the context the handshake actually uses, never off a setting -- an instance-wide CRL must not silence the guard on a hop whose handshake never consults it. The CRL wiring that supplies such a context lands next. Five existing tests pinned the pre-clamp ALLOW (guard, MLLP, the four HTTP-family cells, SMTP). Each now asserts the refusal and records at its docstring that it asserted the opposite. NOT in this commit: the Postgres store hop keeps reading the env as its attestation. `[store]` has no per-connection attestation surface, so clamping it before it has a CRL of its own would refuse every enforcing Postgres-store instance with no configuration that fixes it.
…ps that resolve a trust anchor (BACKLOG #299) harden_crl_check shipped, and all three of its call sites were INBOUND listeners: api/tls.py, mllp.py's server context, and dicom.py's SCP mTLS branch. Every OUTBOUND hop verified its peer and checked no revocation, so on first deployment a revoked-but-unexpired server certificate would be accepted on a hop the instance reported as verified. That is the gap the #201 RevocationHopGuard refuses on, and until now an operator had no way to CLOSE it -- only to declare it away. [tls].crl_file rides the trust-anchor policy already threaded onto every outbound Destination, so the CRL reaches each hop through resolve_trust_anchor and lands on that hop's OWN context in build_verifying_client_context / build_anchored_https_handler. Covered: the MLLP outbound client, the DICOM SCU, FTPS, SMTP, and the HTTP family (REST / SOAP / FHIR / DICOMweb / the fhir_lookup opener). Per-hop scoping is the risk the item records as binding, and two things carry it. TrustAnchor.narrows now counts a CRL, because the five HTTP-family call sites reuse a module-level opener built at import time whenever the anchor does not narrow -- a CRL-only anchor that answered "nothing to narrow" would leave the hop on that unrevoked opener. And every test asserts VERIFY_CRL_CHECK_LEAF on the context the connector really hands to wrap_socket, via context_checks_revocation, not on the setting: an ldap3.Tls or a truststore context is built by a library that never sees the policy, and a settings-shaped assertion would grade those green too. Loopback hops are exempt, for a stronger reason than the internal-CA exemption they inherit it from. VERIFY_CRL_CHECK_LEAF refuses a peer whose ISSUER has no CRL in the store, not only a revoked one, and an on-box peer is normally issued by a local PKI the org CRL does not cover -- so applying it there would break working traffic to close a gap the revocation guard already treats as closed. requests_verify_from_anchor REFUSES a CRL rather than dropping it. verify= names one bundle path and carries no revocation flag, so honouring the CA while losing the CRL would report a revocation-checked hop that checks nothing. Unreachable today (both Vault callers build a default policy) and written anyway, because that silent drop is the failure that lands the day the policy is threaded there. email.py builds its TLS context BEFORE capturing the revocation guard now, since the guard reads the CRL flag off that context. The refusals ahead of it are unmoved, so an unverified or credential-leaking SMTP hop is still rejected first. Every test added here was confirmed to FAIL on a tree with only the test changes applied: 8 in test_tls_policy.py and 9 in test_hop_refusal_revocation.py, checked by stashing the source and re-running.
…LS forwarder (BACKLOG #299) The RFC 5425 syslog forwarder builds its own ssl context and resolves no trust anchor, so the [tls].crl_file added for the anchor-resolving hops never reaches it. Letting it inherit that setting silently would be the per-hop scoping error #299 warns about -- an instance-wide CRL reported as covering a handshake that never consults it. So it gets its own [logging].forward_tls_crl_file. Guarded on forward_tls_verify. The opt-out arm is CERT_NONE, where there is no chain to check a CRL against and setting VERIFY_CRL_CHECK_LEAF would refuse every collector while claiming a check. Loaded after the CA and any client chain, so harden_crl_check's "the CRL really landed in the store" assertion answers for the final trust store. Tests assert the flag on the context the handler wraps its socket with, with a negative control on the same builder, and were confirmed to fail on a tree carrying only the test changes.
… hop (BACKLOG #299) The OIDC relying party's opener builds its own stdlib context and resolves no trust anchor, so the [tls].crl_file added for the anchor-resolving hops cannot reach it. [auth].oidc_tls_crl_file is its own knob rather than a silent inheritance, for the reason #299 records as binding: an instance-wide CRL reported as covering a handshake it never reaches is worse than no CRL at all. A revoked certificate matters more on this hop than on a data hop. This is the leg that carries the client secret, the authorization code and the identity assertion, so accepting a revoked-but-unexpired IdP certificate would be an authentication-material exposure rather than a confidentiality one. The CRL loads after the trust store is final and before harden_cipher_suites, so harden_crl_check's "the CRL really landed" assertion answers for the store this handshake uses. Both existing branches verify, so there is no CERT_NONE arm to guard. The setting joins its two siblings in the security-doc-drift reviewed-non-inputs list, for their reason: it decides whether the ENGINE trusts the IdP's certificate, not what the engine decides about a request it receives. A certificate it rejects never yields an identity at all. The AuthService seam test now pins the CRL path alongside the pin and the dial -- a setting that never reaches the builder is a knob that does nothing, and only the seam test can catch that. Both new tests in test_auth_oidc_http.py were confirmed to fail on a tree carrying only the test changes.
… (BACKLOG #299) context_checks_revocation tests VERIFY_CRL_CHECK_LEAF and claims that answers for VERIFY_CRL_CHECK_CHAIN as well. That claim is a measurement rather than a reading of the two names, so it is pinned by a test instead of left in a docstring: on CPython 3.14 / OpenSSL 3.5.7, LEAF is 0x4 and CHAIN is 0xc, because CHAIN is LEAF OR'd with X509_V_FLAG_CRL_CHECK_ALL. If they ever stop overlapping, a context set to check the whole chain would read as checking nothing, every revocation guard on it would wrongly refuse, and nothing else in the suite would notice. The docstring now cites the measurement and the test that holds it.
LANDER INSPECTION -- blocking, on the wiring rather than the machineryPosted under the korus The CRL machinery is the strongest thing in this PR and I want it. What I cannot land is the The CRL check is genuinely fail-closed, and that is not a small thing
There is no fetch to fail: the CRL is a local PEM path, read at construction. I checked every added The blocker: the relaxation cannot reach the HTTP family, structurally
And it is not a forgotten keyword. In The guard runs ten lines before the context exists, so it could not receive one. The same order So with Why that is a blocker and not a follow-up
Remaining routes for an off-box REST/FHIR/SOAP/DICOMweb destination: move it to loopback, or set The PR body states the hazard itself -- "Clamping the env without building the CRL would have left Two more, both feeding the same conclusionNo owner ruling is cited. ADR 0173 is mentioned zero times in the PR body. A default that Zero documentation files changed. Why no check caught itAll 29 CI checks pass. The HTTP-family test calls Minimum to unblock
Verdict: block. |
1 participant
BACKLOG #299 limb A: outbound CRL revocation, plus the attestation clamp
This closes part of BACKLOG #299. It does not close the row. The scope table below says which
limbs landed here, which were already shipped before this branch started, which is limb B, and which
is vault work no builder can do.
MessageFoundry has zero deployments, so every defect below is written in the conditional. Nothing
here is a live exposure.
What was wrong
Two defects, and the second is what made the first unfixable.
1. A blanket environment variable defeated an enforcing posture.
revocation_hop_dispositionreturned ALLOW onattestedbefore itsenforcing -> REFUSEarm, andRevocationHopGuard.captureOR'd the process-wideMEFOR_TLS_REVOCATION_ATTESTEDenv into that sameattestedargument. One environment variable, set once, returned ALLOW for every verifyingoutbound TLS hop in the instance. On first deployment that would accept a revoked-but-unexpired peer
certificate on any such hop while the instance reported an enforcing posture.
2. Revocation checking existed, and reached no outbound hop.
harden_crl_checkshipped under BACKLOG #1005, and all three of its call sites are INBOUND listeners:api/tls.py,mllp.py's server context, anddicom.py's SCP mTLS branch. There was no[tls].crl_fileat all. So an operator whose hop the revocation guard refused had no way to close the gap, only to
declare it away with the very env var defect 1 describes. Clamping the env without building the CRL
would have left enforcing instances with no route forward.
What this branch builds
[tls].crl_filerides the trust-anchor policy already threaded onto every outboundDestination,so the CRL reaches each hop through
resolve_trust_anchorand lands on that hop's own contextin
build_verifying_client_context/build_anchored_https_handler.[logging].forward_tls_crl_fileand[auth].oidc_tls_crl_filefor the two hops that build theirown context and resolve no anchor.
tls_revocation_attestedand the blanket env are now separate inputswith different ranks. Per-connection still crosses an enforcing hop; the env ranks below the
enforcing refusal. Under a non-enforcing posture the env still ALLOWs, byte-identical to before, so
nothing outside an enforcing posture moves.
crl_checkedas a relaxation ahead of the refusal, so a hop that really checks a CRL stops beingrefused. It is read off the context via
context_checks_revocation, never off the setting.Per-hop scoping, which the row records as the binding risk
The row warns that one global
crl_filemust never silence guards on hops whose handshakes neverconsult it. Two things carry that here.
Every assertion reads
VERIFY_CRL_CHECK_LEAFoff the object the handshake uses, not off thesetting. An
ldap3.Tlsor atruststorecontext is built by a library that never sees the policy, soa settings-shaped assertion would grade those green alongside the hops that really got a CRL.
TrustAnchor.narrowsnow counts a CRL. The five HTTP-family call sites reuse a module-levelopener whenever the anchor does not narrow, and that opener is built at import time, before any
config exists. A CRL-only anchor that answered "nothing to narrow" would have left the hop on that
unrevoked shared opener while its guard was told revocation was checked. That is exactly the failure
the row names, and
test_a_crl_alone_makes_the_anchor_narrowpins it.How I enumerated the egress hops
The brief named six and said to treat that as a starting point. Method, in three passes over
messagefoundry/:create_default_context,ssl.SSLContext(,truststore.SSLContext);build_verifying_client_context,build_anchored_https_handler,build_asserted_https_handler,build_smtp_tls_context);ldap3.Tls,hvac/requests,asyncpg,httpx).Then each hit was classified as inbound listener, outbound client, or not TLS. Characterising the
class rather than trusting the list is the point: an enumeration is always missing one.
build_verifying_client_context[tls].crl_filebuild_verifying_client_context[tls].crl_filebuild_verifying_client_context[tls].crl_file[tls].crl_file[tls].crl_file[tls].crl_file[tls].crl_filefhir_lookupread[tls].crl_filebuild_smtp_tls_context[tls].crl_file[logging].forward_tls_crl_file[auth].oidc_tls_crl_fileldap3.Tlsbuilds its ownrequestsverify=build_asserted_https_handler, no anchortruststore/ stdlibInbound listeners already had CRL support and are out of limb A's scope:
api/tls.py, the MLLP servercontext, the DICOM SCP mTLS branch.
Named gaps, with reasons rather than silence
ldap3.ldap3.Tlstakesca_certs_file,validate,versionandciphersand builds its own context internally; it accepts nossl_context. Wiring aCRL there means substituting a private context for the library's, which is a different change with
its own risk. Named, not faked.
clamped.
[store]has no per-connection attestation surface, so clamping it before giving it aCRL of its own would refuse every enforcing Postgres-store instance with no configuration that
fixes it. That is a real residual of the clamp and it should be closed by giving the store hop its
own CRL setting.
requeststakes one bundle path and no revocationflag. Unreachable today (both callers build a default policy), written anyway, because the silent
drop is the failure that would land the day the policy is threaded there.
tls_crl_fileis not built. It would need an edit in each connector'ssettings read, and those files belong to other builders this wave. The instance-wide setting is
what landed.
Operational note, which is the other half of this setting
VERIFY_CRL_CHECK_LEAFrefuses a peer whose issuer has no CRL in the store, not only a revokedone. So a configured CRL must cover every issuer the covered hops present, and must be refreshed
before its
nextUpdate. Both failures are fail-CLOSED — the handshake is refused, nothing crossesunverified — and
harden_crl_checkrefuses an absent, unloadable or already-expired CRL atconstruction rather than at the first partner handshake.
Loopback hops are exempt for that reason. An on-box peer is normally issued by a local PKI the org
CRL does not cover, and the revocation guard already ALLOWs a loopback hop, so applying a CRL there
would break working traffic to close a gap the gate does not consider open.
Scope: what landed, what did not
17d8c8229/8a61e4a41. Not rebuilt.4c68c28eb(PR 878, BACKLOG #1180). Not rebuilt. The row's premise that "today's context is implicit stdlib and unreachable by harden calls" is false at HEAD and I relied on the seam being there.docs/security/is gitignored in the engine and absent from this checkout, so a builder physically cannot do it. The closing act for this row is a vault re-score I did not and could not perform.Tests
Every new test was confirmed to FAIL on a tree carrying only the test changes, by stashing
messagefoundry/and re-running:tests/test_tls_policy.py(anchor resolution, both context builders,narrows, the requests refusal)tests/test_hop_refusal_revocation.py(per-hop coverage on MLLP, DICOM SCU, FTPS, SMTP, the four HTTP cells, and the loopback exemption)tests/test_logging.py(syslog)tests/test_auth_oidc_http.py(OIDC)Each carries a negative control on the same builder, because a flag assertion alone cannot tell a
working check from a context that refuses everyone.
Seven existing tests asserted the pre-clamp ALLOW and now assert the refusal (the guard, MLLP, the
four HTTP-family cells, SMTP). Each says so at its own docstring. They were not relaxed; they
documented the defect.
Two gates needed updating for the new settings, both in tests:
tests/test_auth_oidc_service.py(the seam spy now pins the CRL path — a setting that never reachesthe builder is a knob that does nothing) and
tests/test_security_doc_drift.py(oidc_tls_crl_filejoins its two siblings in the reviewed-non-inputs list).
Checks run
ruff format --check,ruff check,mypy messagefoundry(strict, 275 files) all clean. Targetedsuites green: the TLS policy, hop-refusal, trust-anchor, MLLP TLS, email, remotefile, DICOM, DICOMweb,
REST, FHIR, cert-expiry, forward-proxy, logging, cipher-assertion-site, auth and settings suites
(886 + 1067 + 2013 + 112 passed across those passes). That includes a full sweep of the doc-drift,
settings, config, inventory, contract, parity, ASVS and crypto gates -- 2013 passed -- which is where
a settings change usually breaks.
The full local suite was STARTED AND DELIBERATELY STOPPED at roughly 7 percent. Four builders are
running on this one box and it was going to take hours while starving the other three. That is a
skipped check, not a passed one: the full-suite verdict must be read from the hosted legs.
One local failure is box-level and not from this branch:
tests/test_selfheal_installed_parity.py::test_the_installed_selfheal_payload_matches_the_committed_sourcecompares this checkout's
scripts/worktree/worktree-selfheal.ps1against a user-scope installed copyat
C:\Users\Scott\.claude-hooks\. This branch touches nothing underscripts/, and there is noinstalled copy on a hosted runner.
Legs to read after this session exits: the Windows and Linux test legs,
windows-service-smoke(NSSM, never visible to a builder), and the crypto-inventory / ASVS 11.1.3 discovery gate — this
branch adds CRL code to
logging_setup.pyandauth/oidc_http.py, and the local pre-commit hookpassed, but the CI gate is the authority.
Files, for the merge read
config/tls_policy.pyandconfig/settings.pycarry the substance.transports/mllp.pygains oneargument.
transports/email.pyreorders its guard to run after the context exists, because the guardnow reads the CRL flag off that context; the refusals ahead of it are unmoved.
logging_setup.py,auth/oidc_http.py,auth/service.pyand__main__.pycarry the twoown-context hops.
transports/rest.pywas NOT edited, and that was a design goal rather than an accident: puttingthe CRL on the resolved
TrustAnchormeans every anchor-resolving hop picks it up with no connectoredit, so
rest.py,soap.py,fhir.pyanddicomweb.pyare untouched.apiclient/client.pywasnot edited either; PR 1198 has since merged, so that contention is moot.
Open questions for the next brief
[store].ssl_crl_fileso the clamp can extend to it?I left it reading the blanket env, because clamping it with no escape would refuse every enforcing
Postgres instance. That is a judgement call and it deserves an owner or a follow-up item.
SSLContextforldap3's internal one acceptable for the AD hop?It is the only route to revocation checking there.
[tls].crl_filewant a per-connection override? The row's "per-connection" wordingsuggests yes, and it needs connector edits this wave's file ownership did not allow.
Collision check against the other PRs open this wave
This branch touches none of the contended files. Changed here:
config/tls_policy.py,config/settings.py,logging_setup.py,auth/oidc_http.py,auth/service.py,__main__.py,transports/mllp.py,transports/email.py, and six test files.Deliberately NOT touched, and the design is why: putting the CRL on the resolved
TrustAnchormeansevery anchor-resolving hop picks it up with no connector edit, so
transports/rest.py,transports/soap.py,transports/fhir.pyandtransports/dicomweb.pyall gain revocation checkingwhile staying byte-identical on this branch. The brief permitted one last-resort edit to
refuse_unrevoked_verified_hopinrest.py; it was not needed and was not made.Also untouched:
transports/database.py,transports/base.py,transports/signing.py,transports/smart.py,transports/http_auth.py,auth/oidc/flow.py,apiclient/client.py,api/tls.py,docs/CONNECTIONS.md.auth/oidc/flow.pyis an egress hop I identified and deliberately did not wire, and it needs nowiring. Its
exchange_codedoes open a socket, but through an injectedopener— the onebuild_idp_openerbuilds. So the[auth].oidc_tls_crl_fileadded here already covers thathandshake, and editing
flow.pywould have added nothing but a conflict. The pureauth.oidcpackage holding no network policy of its own is exactly what makes that true.
Note for whoever works
logging_setup.pynext, passed on rather than acted on:RedactionFilter'sdocstring says it scrubs chained
__cause__and__context__, but it renders through the defaulttraceback printer, which honours
__suppress_context__, so it never reaches a suppressed chain. Thefilter is not defective — the wording invites a wrong assumption. This branch does not go near that
docstring and left it alone.