diff --git a/messagefoundry/auth/trust_anchors.py b/messagefoundry/auth/trust_anchors.py index 5493b5fda3..81a8a50a14 100644 --- a/messagefoundry/auth/trust_anchors.py +++ b/messagefoundry/auth/trust_anchors.py @@ -543,19 +543,18 @@ def anchor_cadata(data: bytes, spec: AnchorSpec) -> str: **An encryption header name after the first BEGIN refuses, before that load** (BACKLOG #2270), whether or not it sits inside a block. A ``Proc-Type: 4,ENCRYPTED`` header sends OpenSSL to its password callback, and ``cadata=`` sets - none, so OpenSSL falls back to its default one, which reads the terminal. The POSIX terminal run - was done later, off-version (BACKLOG #3090, 2026-10-12): on CPython 3.12.3 / OpenSSL 3.0.13 on - Linux with a pseudo-terminal, the same ``cadata=`` load wrote ``Enter PEM pass phrase:`` and was - still waiting at 5 seconds, while ``cafile=`` refused at once with ``PEM lib``. - ``tests/test_encrypted_pem_cafile_pin.py`` holds that table and pins it on each CI platform. - What was measured first, on Windows, on - CPython 3.14.6 / OpenSSL 3.5.7 in a child process with stdin on the null device: the + none, so OpenSSL falls back to its default one, which reads the terminal. Measured on Windows, + on CPython 3.14.6 / OpenSSL 3.5.7 in a child process with stdin on the null device: the ``cadata=`` load of a certificate block carrying ``Proc-Type: 4,ENCRYPTED`` and ``DEK-Info:`` did not return within 20 seconds and was killed, while ``cafile=`` on the same bytes refused at - once with ``PEM lib``. So a start or a reload could hang on this file. A CA certificate never - carries an RFC 1421 encryption header, so ``Proc-Type:`` and ``DEK-Info:`` both refuse, in any - case, anywhere after the first BEGIN of the text the load would get, and OpenSSL is never - handed that text (:func:`has_encryption_header`). That is wider than the hang: a private-key + once with ``PEM lib``. So a start or a reload could hang on this file. A later run, off-version, + under a POSIX pseudo-terminal (BACKLOG #3090) saw the same ``cadata=`` load write the password + prompt and wait. ``tests/test_encrypted_pem_cafile_pin.py`` holds that run's table, with its + versions. It pins the path loads on each CI platform, and the pseudo-terminal case on POSIX + only. A CA certificate never carries an RFC 1421 encryption header, so ``Proc-Type:`` and + ``DEK-Info:`` both refuse, in any case, anywhere after the first BEGIN of the text the load + would get, and OpenSSL is never handed that text (:func:`has_encryption_header`). That is wider + than the hang: a private-key block encrypted in that legacy header format beside a certificate loads through ``cadata=``, and refuses here, and so does the header name in a comment between two blocks or after the last one. The diff --git a/messagefoundry/transports/strict_requests.py b/messagefoundry/transports/strict_requests.py index f15ff8756e..10549b0521 100644 --- a/messagefoundry/transports/strict_requests.py +++ b/messagefoundry/transports/strict_requests.py @@ -310,19 +310,16 @@ def _refuse_encrypted_ca_text(self) -> None: **The refusal is a** :class:`VaultHopRefused`, **like the proxy leg's CA load refusal in** :meth:`_narrow_the_proxy_leg`. urllib3 does not catch that type, so it - reaches the caller as it is, and :func:`vault_failure_text` keeps its text. The base - ``InsecureHopRefused`` would reach the caller too, but that function would give only - its type name; - :class:`VaultHopRefused` says why. A urllib3 ``SSLError`` raised here would not reach - the caller as it is. Measured with urllib3 2.8.0, one raised before the proxy has - connected comes out of requests as a ``ProxyError``. The providers would then report a - type name only. + reaches the caller as it is, and :func:`vault_failure_text` keeps its text. A urllib3 + ``SSLError`` raised here would not reach the caller as it is. Measured with urllib3 + 2.8.0, one raised before the proxy has connected comes out of requests as a + ``ProxyError``. The providers would then report a type name only. **A CA file is not checked, because the load itself refuses it.** That holds for the encryption header only: a CA file holding a private key loads, and nothing here - refuses it (BACKLOG #3745 covers the CA text alone). ``ca_certs`` is a - path. Measured, a path load of an encrypted ``CERTIFICATE`` or ``X509 CRL`` block raised - ``PEM lib`` at once and wrote no prompt. A check here would also open the file twice. + refuses it (BACKLOG #3745 covers the CA text alone). ``ca_certs`` is a path. Measured, + a path load of an encrypted ``CERTIFICATE`` or ``X509 CRL`` block raised ``PEM lib`` + at once, and no prompt was seen. A check here would also open the file twice. ``tests/test_encrypted_pem_cafile_pin.py`` holds both measurements, with the versions and terminal conditions, and pins them on each platform CI runs. @@ -330,19 +327,20 @@ def _refuse_encrypted_ca_text(self) -> None: reaches OpenSSL as ``capath``. OpenSSL reads a directory's files when it verifies a peer, not when the directory is named, so a check of the text handed to the load, like the one above, does not cover one. So it was measured, for Python's ``ssl`` only, - with a real handshake (2026-10-12), on CPython 3.14.6 / OpenSSL 3.5.7 on - Windows and, off-version, on CPython 3.12.3 / OpenSSL 3.0.13 on Linux. A directory - file holding an encrypted ``CERTIFICATE`` or ``X509 CRL`` block, under the name - OpenSSL looks up, wrote no prompt and never waited, in every terminal condition - tried, a pseudo-terminal among them. The handshake refused the peer at once with a - verification error, so the hop fails closed. ``tests/test_encrypted_pem_capath_pin.py`` - holds that table and its controls, and pins it on each platform CI runs. A directory - does reach both loads today. With neither ``MEFOR_STORE_VAULT_CA_FILE`` nor - ``MEFOR_SECRETS_VAULT_CA_FILE`` set, hvac and requests choose the CA. At least two - routes can name a directory: hvac's ``VAULT_CAPATH``, used when ``VAULT_CACERT`` is not - set, and requests' ``REQUESTS_CA_BUNDLE`` or ``CURL_CA_BUNDLE``. requests passes a - directory on as ``ca_cert_dir``. A refusal here would break a hop that works and stop - nothing that was seen to wait, so the directory is loaded as before.""" + with a real handshake on Windows and, off-version, on Linux. A directory file holding + an encrypted ``CERTIFICATE`` or ``X509 CRL`` block, under the name OpenSSL looks up, + never waited, in every terminal condition tried, a pseudo-terminal among them. The + handshake refused the peer at once with a verification error, so the hop fails + closed. No prompt was seen either. On Windows a console prompt is not visible to the + caller. ``tests/test_encrypted_pem_capath_pin.py`` says what the Windows reading rests + on. It holds the table, with its versions and controls, and pins it on each platform + CI runs. A directory does reach both loads today. With neither + ``MEFOR_STORE_VAULT_CA_FILE`` nor ``MEFOR_SECRETS_VAULT_CA_FILE`` set, hvac and + requests choose the CA. At least two routes can name a directory: hvac's + ``VAULT_CAPATH``, used when ``VAULT_CACERT`` is not set, and requests' + ``REQUESTS_CA_BUNDLE`` or ``CURL_CA_BUNDLE``. requests passes a directory on as + ``ca_cert_dir``. A refusal here would break a hop that works and stop nothing that was + seen to wait, so the directory is loaded as before.""" if isinstance(self.ca_cert_data, str) and has_encryption_header(self.ca_cert_data): # Fixed text: it names no part of the CA text it refuses. raise VaultHopRefused( @@ -762,9 +760,9 @@ def vault_failure_text(exc: BaseException) -> str: A refusal from this hop keeps its own text: a :class:`VaultHopRefused` from the hop checks, or an :class:`~messagefoundry.transports.bounded_read.EgressReplyError` from the reply reader. Both are built here and in ``bounded_read`` from the hop's fixed connector label and fixed - reason text. They name no part of the address, the proxy URL, the token or the reply. Any - other failure gives only its type name, as before, because its text can echo ciphertext, a - request body or a URL. + reason text. What the hop's text never names is listed once, in :class:`VaultHopRefused`. + Any other failure gives only its type name, as + before, because its text can echo ciphertext, a request body or a URL. **A bare** ``InsecureHopRefused`` **gives only its type name too (vault BACKLOG #3247).** :class:`VaultHopRefused` says why. diff --git a/tests/test_revocation_guard_census.py b/tests/test_revocation_guard_census.py index d3a40cb690..44f4ee1780 100644 --- a/tests/test_revocation_guard_census.py +++ b/tests/test_revocation_guard_census.py @@ -19,15 +19,22 @@ the listener gates are a different control. **What "guarded" proves.** The site's function, or the one its entry names, calls a name in -:data:`_GUARDS`, and every such name reaches ``RevocationHopGuard.capture``. That ties the site to a -guard call. It does not prove that every branch reaches the call, or that the call refuses: the -behaviour tests (at least ``tests/test_revocation_guard_remaining_hops.py`` and -``tests/test_hop_refusal_revocation.py``) prove that. A second hop added inside a function that is -already guarded passes here too, so read the function in review. +:data:`_GUARDS`, and every such name reaches a guard in :data:`_GUARD_ROOTS`. That ties the site +to a guard call. It does not prove that every branch reaches the call, or that the call refuses, +or that a guard checks a CRL: the behaviour tests (at least +``tests/test_revocation_guard_remaining_hops.py``, ``tests/test_hop_refusal_revocation.py`` and +``tests/test_vault_revocation_hop_guard.py``) prove that. A second hop added inside a function that +is already guarded passes here too, so read the function in review. **It goes red.** Delete the guard call from one guarded site and :func:`test_every_guarded_site_calls_a_guard` fails naming that site; measured when this file was written, by deleting the ``RevocationHopGuard.capture`` call in ``DicomScuDestination.__init__``. +Measured again when the two Vault ``_build_client`` sites became Guarded (vault BACKLOG #3587): +renaming the ``mount_strict_reply_adapter`` call in either one failed that test naming it, and +renaming either link of the chain from the mount to the Vault guard failed +:func:`test_every_guard_name_reaches_a_root`. So did a second def named ``refuse_an_unrevoked_hop`` +in another module, and renaming the Vault guard's ``_crossed_unrevoked`` call failed +:func:`test_the_vault_root_decides_through_the_authority`. """ from __future__ import annotations @@ -38,7 +45,7 @@ from pathlib import Path from messagefoundry.config.tls_policy import RevocationHopGuard -from tests._ast_sites import callee_name +from tests._ast_sites import callee_name, calls_to, find_funcs, named_func, parse_source _ROOT = Path(__file__).resolve().parents[1] _PACKAGE = _ROOT / "messagefoundry" @@ -84,16 +91,29 @@ #: The stdlib and truststore names. Matched only as a call: a bare ``SSLContext`` also names a type. _STDLIB_BUILDERS = frozenset({"create_default_context", "SSLContext"}) -#: Names whose call runs a revocation guard. Each one other than ``capture`` must reach ``capture`` -#: through the others (:func:`test_every_guard_name_reaches_the_guard`). -_GUARDS = frozenset( +#: The guards every other name in :data:`_GUARDS` must reach. The Vault hop guard, in +#: ``transports/strict_requests.py`` (vault BACKLOG #2983), decides through +#: ``tls_policy.revocation_hop_disposition`` and never calls ``RevocationHopGuard``; its docstring +#: says why. +_GUARD_ROOTS = frozenset( { "capture", # RevocationHopGuard.capture, matched only on that receiver + "_refuse_an_unrevoked_vault_hop", + } +) + +#: Names whose call runs a revocation guard. Each one outside :data:`_GUARD_ROOTS` must reach a root +#: through the others (:func:`test_every_guard_name_reaches_a_root`). +_GUARDS = _GUARD_ROOTS | frozenset( + { "refuse_unrevoked_verified_hop", "_refuse_store_revocation", "_refuse_forward_revocation", "_refuse_idp_revocation", "idp_revocation_guards", + # transports/strict_requests.py, the Vault hops (vault BACKLOG #2983) + "refuse_an_unrevoked_hop", # StrictReplyAdapter's, at the mount and before each send + "mount_strict_reply_adapter", } ) @@ -101,6 +121,14 @@ _UNGATED = ( "an ungated verifying hop, listed in docs/DEPLOYMENT.md under 'Ungated verifying TLS hop'" ) +#: What the two Vault entries' guard covers, and what this census does not follow there. +_VAULT_GUARD = ( + "through mount_strict_reply_adapter, which runs _refuse_an_unrevoked_vault_hop at " + "construction (vault BACKLOG #2983). No CRL lever is built for these hops, so off the box " + "the hop crosses an enforcing posture only on its vault_revocation_attested pair. This census " + "follows the construction-time call only, not the adapter's check before each send, and not " + "_refuse_an_unrevoked_proxy_leg (vault BACKLOG #3561), handed on as a value" +) _TRAY = "the Windows tray probing the engine's own API on this box" _LISTENER = ( "a SERVER context for an inbound listener. Its client-certificate revocation gate is " @@ -162,6 +190,12 @@ def __post_init__(self) -> None: note="the SMART and OAuth2 token hops" ), "messagefoundry/transports/soap.py::SoapDestination.__init__": Guarded(), + "messagefoundry/config/secretprovider_vault.py::_build_client": Guarded( + note=f"the Vault KV secret provider hop, {_VAULT_GUARD}" + ), + "messagefoundry/store/keyprovider_vault.py::_build_client": Guarded( + note=f"the Vault Transit key provider hop, which the Transit cipher shares, {_VAULT_GUARD}" + ), # --- exempt: ungated verifying hops ------------------------------------------------------------ "messagefoundry/auth/ldap.py::LdapAuthenticator.__init__": Exempt(f"LDAPS: {_UNGATED}"), "messagefoundry/pipeline/alert_sinks.py::_build_no_redirect_opener": Exempt( @@ -178,15 +212,6 @@ def __post_init__(self) -> None: "messagefoundry/transports/ai_broker.py::AiBroker.__init__": Exempt( f"the AI-broker endpoint: {_UNGATED}" ), - "messagefoundry/config/secretprovider_vault.py::_build_client": Exempt( - "the Vault KV secret provider hop. Ungated, and NOT in docs/DEPLOYMENT.md's ungated table: " - "its requests-based client takes one CA bundle and no revocation flag, so no CRL can reach " - "it (tls_policy refuses [tls].crl_file there). Found by this census; unfiled" - ), - "messagefoundry/store/keyprovider_vault.py::_build_client": Exempt( - "the Vault Transit key provider hop. Ungated, and NOT in docs/DEPLOYMENT.md's ungated " - "table, for the same reason as the KV provider above. Found by this census; unfiled" - ), # --- exempt: not a partner hop ----------------------------------------------------------------- "messagefoundry/apiclient/client.py::_build_verify_context": Exempt( "the engine's own HTTP client library (ADR 0088), dialing the engine's API from a client " @@ -346,9 +371,10 @@ def test_every_site_is_in_the_census() -> None: unlisted = sorted(_scan().sites - _CENSUS.keys()) assert not unlisted, ( f"verifying client TLS context(s) with no census entry: {unlisted}. Add a guard " - f"(RevocationHopGuard.capture, or refuse_unrevoked_verified_hop for an https hop) and list " - f"the site as Guarded, or list it as Exempt with the reason. See ADR 0173 and " - f"docs/DEPLOYMENT.md, 'Revocation-guard behavior'." + f"(RevocationHopGuard.capture, refuse_unrevoked_verified_hop for an https hop, or " + f"mount_strict_reply_adapter for an hvac Vault client, which must not use " + f"RevocationHopGuard) and list the site as Guarded, or list it as Exempt with the reason. " + f"See ADR 0173 and docs/DEPLOYMENT.md, 'Revocation-guard behavior'." ) @@ -373,19 +399,38 @@ def test_every_guarded_site_calls_a_guard() -> None: assert not unguarded, f"census sites marked Guarded whose guard call is gone: {unguarded}" -def test_every_guard_name_reaches_the_guard() -> None: +def _def_count(name: str) -> int: + """How many ``def``s named ``name`` the package holds, nested ones and methods included.""" + return sum( + len(find_funcs(parse_source(source), name)) + for path in sorted(_PACKAGE.rglob("*.py")) + if name in (source := path.read_text(encoding="utf-8")) + ) + + +def test_every_guard_name_reaches_a_root() -> None: """A wrapper that stopped calling the guard would leave every site it covers green above. - Followed through the wrappers to ``capture``, so two wrappers that only call each other are - hollow too. Keyed by bare function name, so a same-named def elsewhere in the package could - stand in for a wrapper; the guard names here are unique today.""" + Followed through the wrappers to a root, so two wrappers that only call each other are hollow + too. Keyed by bare function name, so each guard name but ``capture`` must have exactly one def + in the package: a second same-named def could otherwise stand in for a wrapper. ``capture`` is + matched only on ``RevocationHopGuard``, and ``transports/mllp.py`` defines another one.""" defs = _scan().defs assert callable(getattr(RevocationHopGuard, "capture", None)) - # 6 guard names when this floor was set, so 4 lets two go with no edit here (BACKLOG #3512). It - # fails a list emptied or cut to "capture" alone, over which the check below passes on nothing. - assert "capture" in _GUARDS, sorted(_GUARDS) - assert len(_GUARDS) >= 4, f"only {len(_GUARDS)} guard names, floor 4" - reaches = {"capture"} + # Pinned exactly, so a hollow wrapper moved into the roots needs an edit here that review sees. + assert sorted(_GUARD_ROOTS) == ["_refuse_an_unrevoked_vault_hop", "capture"], _GUARD_ROOTS + # Counted from the source, not imported: the Vault guard's module needs the [vault] extra. + counts = {name: _def_count(name) for name in sorted(_GUARDS - {"capture"})} + # 8 names when this floor was set; 4 is the Vault root plus the wrapper floor below. It fails a + # walk over nothing, over which the one-def check would pass. + assert len(counts) >= 4, f"only {len(counts)} guard names besides capture, floor 4" + not_one = {name: n for name, n in counts.items() if n != 1} + assert not not_one, f"guard name(s) without exactly one def in messagefoundry/: {not_one}" + # 5 wrappers when this floor was set, so 3 lets two go with no edit here (BACKLOG #3512). It + # fails a list cut to the roots alone, over which the check below passes on nothing. + wrappers = _GUARDS - _GUARD_ROOTS + assert len(wrappers) >= 3, f"only {len(wrappers)} guard names outside the roots, floor 3" + reaches = set(_GUARD_ROOTS) grew = True while grew: grew = False @@ -394,7 +439,27 @@ def test_every_guard_name_reaches_the_guard() -> None: reaches.add(name) grew = True hollow = sorted(_GUARDS - reaches) - assert not hollow, f"guard name(s) that never reach RevocationHopGuard.capture: {hollow}" + assert not hollow, f"guard name(s) that never reach a root in {sorted(_GUARD_ROOTS)}: {hollow}" + + +def test_the_vault_root_decides_through_the_authority() -> None: + """The Vault root is not ``RevocationHopGuard``, so its tie to the decision is checked here. + + It must call ``_crossed_unrevoked``, and that must call ``revocation_hop_disposition``, the + authority every other revocation guard uses. This shows the calls exist, not that their answer + is used: a root that returns first stays green here, and the behaviour tests in + ``tests/test_vault_revocation_hop_guard.py`` catch that.""" + tree = parse_source( + (_PACKAGE / "transports" / "strict_requests.py").read_text(encoding="utf-8") + ) + root = named_func(tree, "_refuse_an_unrevoked_vault_hop") + decider = named_func(tree, "_crossed_unrevoked") + assert calls_to(root, {"_crossed_unrevoked"}, bare_only=True), ( + "the Vault root no longer decides" + ) + assert calls_to(decider, {"revocation_hop_disposition"}, bare_only=True), ( + "_crossed_unrevoked no longer asks tls_policy.revocation_hop_disposition" + ) def test_every_builder_name_is_still_defined() -> None: diff --git a/tests/test_vault_hop_refusal_marker.py b/tests/test_vault_hop_refusal_marker.py index 982dcd6906..afe8755d3f 100644 --- a/tests/test_vault_hop_refusal_marker.py +++ b/tests/test_vault_hop_refusal_marker.py @@ -140,15 +140,16 @@ def _is_fixed_text(call: ast.Call, constants: frozenset[str]) -> bool: def test_every_marked_refusal_is_built_from_fixed_text() -> None: - """The marker means the text names no address, proxy URL, token or reply. This holds each - site to a shape that cannot interpolate one. It does not read a listed helper's own text.""" + """The marker means the text names nothing the ``VaultHopRefused`` docstring says it never + names. This holds each site to a shape built only from the connector label, literals, module + constants and the listed helpers. It does not read a listed helper's own text.""" tree = parse_source(_HOME.read_text(encoding="utf-8")) constants = _string_constants(tree) sites = call_sites(tree, _MARKER) - # 15 marked refusals when this floor was last set (BACKLOG #3458, #3745). A scan that sees few - # of them, or none, leaves the rest unchecked. Lower the floor only if refusals were really + # 17 marked refusals when this floor was last set (BACKLOG #3489). A scan that sees few of + # them, or none, leaves the rest unchecked. Lower the floor only if refusals were really # merged or removed, and raise it when one is added. - assert len(sites) >= 15, f"the scan saw only {len(sites)} marked refusal(s) in {_HOME.name}" + assert len(sites) >= 17, f"the scan saw only {len(sites)} marked refusal(s) in {_HOME.name}" loose = [site.lineno for site in sites if not _is_fixed_text(site, constants)] assert loose == [], f"a marked refusal interpolates something at lines {loose}" # The constants the sites use today are found, so the lookup is not passing on an empty set.