From f07c84216992360600d5e882c3a08f11cb718646 Mon Sep 17 00:00:00 2001 From: Scott Hall Date: Tue, 27 Oct 2026 17:16:39 -0500 Subject: [PATCH 1/9] CRL signature check at start: a CA with no SKI no longer rescues a CRL its named CA proves bad A start now refuses the mixed case. The CA the CRL names by key identifier is checked and proven not to verify it, and beside it sits a same-name CA with no Subject Key Identifier that the engine cannot check. That read "uncheckable" and the start loaded the file with a WARNING. A named CA that cannot be checked still warns, and so does a lone CA with no SKI. The running-hop reload is unchanged. pki._signature_state ranks it; the CrlSignature docstring, the start's refusal and warning texts, the CA-file monitor's text and CONFIGURATION.md describe it. Tests: the mixed-case refusal, (a) the strict-context premise, and controls (b) and (c). Vault BACKLOG #4159 steps 1, 2 and 5; #2905 item 1 as decision 6428364937 reads it. --- changelog.d/4159.security.md | 9 ++ docs/CONFIGURATION.md | 14 ++- messagefoundry/config/tls_policy.py | 13 ++- messagefoundry/pipeline/cert_expiry.py | 2 +- messagefoundry/pki.py | 27 +++-- tests/test_crl_signature_at_start.py | 154 +++++++++++++++++++++++++ 6 files changed, 204 insertions(+), 15 deletions(-) create mode 100644 changelog.d/4159.security.md diff --git a/changelog.d/4159.security.md b/changelog.d/4159.security.md new file mode 100644 index 0000000000..a374f42dfd --- /dev/null +++ b/changelog.d/4159.security.md @@ -0,0 +1,9 @@ +- **A start now refuses a CRL that the CA it names by key identifier proves badly signed, even + beside a CA of the same name that has no subject key identifier and cannot be checked.** Before, + that second CA made the signature check read "cannot check", so the start loaded the file with a + WARNING. Such a CA cannot make the CRL valid at a handshake: every verifying context the engine + builds uses strict verification, and OpenSSL 3 refuses a chain through a CA with no subject key + identifier. A CA the CRL names that cannot be checked still warns, and so does a lone CA with no + subject key identifier that cannot be checked. The running-hop reload is unchanged. An OpenSSL + build without that strict check could take such a CA for the CRL's issuer; the engine sets no + OpenSSL version floor. (vault BACKLOG #4159) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 2b6fb4ad3e..453f91ae89 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2001,7 +2001,13 @@ otherwise (the first bullet below says what that leaves out). The CRL names that key identifier. The engine ran the signature check with that key, and the check failed. Where several listed CAs share that name, the check must run and fail for each of them that the engine finds to fit the CRL's -authority key identifier. A handshake that OpenSSL judges by such a CRL fails with +authority key identifier and that carries a subject key identifier, and no CA of that name may +verify the CRL. A fitting CA with no subject key identifier that the engine cannot check does not +stop the refusal (vault BACKLOG #4159). Every verifying context the engine builds uses strict +verification, under which OpenSSL 3 refuses a chain through such a CA, so it cannot make the CRL +valid at a handshake. That rests on the OpenSSL build: the engine sets no OpenSSL version floor, +and a build without that strict check could take such a CA for the CRL's issuer. A handshake +that OpenSSL judges by such a CRL fails with `CRL signature failure`, an error that does not name the file. The refusal names the setting, the file and the CRL. Give the setting the CRL that CA signed. No setting turns this check off, as for an expired CRL. @@ -2036,8 +2042,10 @@ three cases warn: renewed its key can read this way while handshakes still pass. That was measured for one chain shape, where the peer sends the renewed CA certified by the old key. - The engine cannot check the signature with the listed CA's key, even where the CRL names that key. - Two measured examples are an ECDSA signature on a CRL that names an RSA key, and an RSA signature - over SHA-1 that the CA did make. + Where the CRL names a listed CA's key, only that CA counts here; a CA of that name with no subject + key identifier that cannot be checked does not (the refusal above says why). Two measured + examples are an ECDSA signature on a CRL that names an RSA key, and an RSA signature over SHA-1 + that the CA did make. If the check itself fails, the engine logs an ERROR that says so. It loads the file unless a CRL it judged before the failure is proved badly signed, which refuses the file. Where it loads, the diff --git a/messagefoundry/config/tls_policy.py b/messagefoundry/config/tls_policy.py index e81a98987b..8d0b808a3c 100644 --- a/messagefoundry/config/tls_policy.py +++ b/messagefoundry/config/tls_policy.py @@ -570,7 +570,10 @@ def _refuse_crl_count(loaded: int, blocks: Sequence[CrlBlock], *, label: str) -> "verify the CRL, but the CRL does not " "name that CA's key, so a renewed CA key could explain it" ), - "uncheckable": "the engine cannot check its signature with the listed CA's key", + "uncheckable": ( + "the engine cannot check its signature with the listed CA's key, where that CA carries " + "the key identifier the CRL names, or the CRL names no listed CA's key" + ), } #: How many CRLs that warning describes before it only counts the rest. _UNCONFIRMED_SHOWN = 3 @@ -589,8 +592,8 @@ def _check_crl_signatures( unverified CRL already failed closed there, so what this adds is the diagnosis, at start. **Everything short of proof loads, with one warning.** That covers at least a CRL that names - no key, a key the engine cannot check with, and a CRL the engine matches to no listed CA. The - last is the usual case in a two-tier PKI, where the peer sends the intermediate that issued + no key, a named key the engine cannot check with, and a CRL the engine matches to no listed + CA. The last is the usual case in a two-tier PKI, where the peer sends the intermediate that issued the CRL, so it must not log per hop. The warning is one line for the file, and :func:`~messagefoundry.config.loaded_crls.first_unconfirmed_signature` decides how often it is logged. That function is asked only when there is a line to log. So it holds the last file @@ -642,7 +645,9 @@ def _check_crl_signatures( raise ValueError( f"{label}: {bad.where} names the key of a CA certificate this hop trusts, and that " "key does not verify the CRL's signature. A handshake OpenSSL judges by this CRL " - "fails with 'CRL signature failure'. Give this setting the CRL that CA signed, or " + "fails with 'CRL signature failure'. A listed CA of that name with no Subject Key " + "Identifier does not change that: strict verification refuses a chain through such " + "a CA. Give this setting the CRL that CA signed, or " "remove this CRL if a good one from that issuer is already in the file" ) unconfirmed = [signature for signature in judged if signature.state != "verified"] diff --git a/messagefoundry/pipeline/cert_expiry.py b/messagefoundry/pipeline/cert_expiry.py index 25a64e2301..eedc18e408 100644 --- a/messagefoundry/pipeline/cert_expiry.py +++ b/messagefoundry/pipeline/cert_expiry.py @@ -770,7 +770,7 @@ def _ca_file_crl_refusal( # (tls_policy._check_crl_signatures). OpenSSL can prefer a newer good CRL from the issuer. # The fix and its claim are one sentence, short enough for the alert list's cut (#3512). return ( - "whose CA file now holds a CRL its issuer's key does not verify", + "whose CA file now holds a CRL that the CA key it names does not verify", "Replace the CRL in the CA file with one its CA signed before restarting: a handshake " "OpenSSL judges by it fails. If a good CRL from that issuer is already in the file, " "remove this one instead", diff --git a/messagefoundry/pki.py b/messagefoundry/pki.py index 0965d4310a..a9161030fb 100644 --- a/messagefoundry/pki.py +++ b/messagefoundry/pki.py @@ -441,10 +441,18 @@ class CrlSignature: * ``verified``: a CA that fits verifies the CRL. * ``bad``: **proven** badly signed. The CRL's key identifier equals a fitting CA's Subject Key - Identifier, and every fitting CA's key was checked and does not verify the CRL. The CRL names - that key, and that key did not sign it. - * ``uncheckable``: some fitting CA could not be checked, and none verifies. "Cannot check" is - not "does not verify" (:func:`_verifies`). + Identifier, every fitting CA so named was checked and its key does not verify the CRL, and + no fitting CA verifies it. The CRL names that key, and that key did not sign it. A fitting + CA with no Subject Key Identifier that could not be checked does not lift this to + ``uncheckable`` (vault BACKLOG #4159, the reading of #2905 item 1 decided 2026-10-26): every + verifying engine context sets ``VERIFY_X509_STRICT``, under which OpenSSL 3 refuses a chain + through a CA with no Subject Key Identifier, so that CA cannot make the CRL valid at a + handshake. Measured 2026-10-27 on CPython 3.14.7 with OpenSSL 3.5.7. An OpenSSL build or a + context without that strict check could still take such a CA for the issuer; the engine + sets no OpenSSL floor. + * ``uncheckable``: no fitting CA verifies, and some fitting CA could not be checked: one the + CRL names by key identifier, or, where the CRL names no fitting CA's key, any. "Cannot check" + is not "does not verify" (:func:`_verifies`). * ``unproven``: every fitting CA was checked and none verifies, but the CRL names none of them by key identifier. A CRL with no key identifier fits every CA of that name. So a CA that renewed its key reads this way, while the peer's own chain may still carry the CA that @@ -859,6 +867,7 @@ def _signature_state( fitting = False named = False # the CRL names some fitting CA's key unchecked = False + named_unchecked = False # a CA whose key the CRL names could not be checked for ca in named_cas: try: names_key = _akid_fit(akid, ca) @@ -872,11 +881,15 @@ def _signature_state( if answer: return "verified" unchecked = unchecked or answer is None + named_unchecked = named_unchecked or (answer is None and names_key) if not fitting: return "unlisted" - if unchecked: - return "uncheckable" - return "bad" if named else "unproven" + # Vault BACKLOG #4159: proof outranks a CA with no Subject Key Identifier that could not be + # checked. Where the CRL names a key, _akid_fit drops a CA whose identifier differs, so every + # fitting CA it does not name carries none, and no strict context chains through such a CA. + if named and not named_unchecked: + return "bad" + return "uncheckable" if unchecked else "unproven" #: Why a CRL reads :class:`CrlSignature` state ``unlisted``, for a start's warning and a reload's diff --git a/tests/test_crl_signature_at_start.py b/tests/test_crl_signature_at_start.py index 96d247f6f1..10ae5f0440 100644 --- a/tests/test_crl_signature_at_start.py +++ b/tests/test_crl_signature_at_start.py @@ -24,6 +24,7 @@ from __future__ import annotations import datetime +import functools import logging import ssl import warnings @@ -50,6 +51,7 @@ from messagefoundry.pipeline import crl_reload from messagefoundry.pipeline.crl_reload import BAD_SIGNATURE, FIX, reload_replaced_crls from messagefoundry.pki import crl_signature_refusal, judge_crl_signatures +from tests.test_crl_bundle_anchors import _ku from tests.test_crl_held_copy_alert import _bundle_scan from tests.test_crl_reload import _ca_cert, _key_pem, _leaf from tests.test_trust_anchor_byte_binding import _handshake @@ -406,6 +408,158 @@ def test_a_ca_that_does_not_parse_is_ignored(twins: _Twins) -> None: assert [signature.state for signature in judged] == ["verified"] +# --- vault BACKLOG #4159: a CA with no Subject Key Identifier cannot rescue a proven bad CRL ------ + + +def _ca_without_ski(key: _CaKey) -> x509.Certificate: + """A self-signed CA certificate for ``key`` named :data:`_NAME` that carries no Subject Key + Identifier, and so no Authority Key Identifier either: nothing else here names a key.""" + return ( + x509.CertificateBuilder() + .subject_name(_NAME) + .issuer_name(_NAME) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(_NOW - _DAY) + .not_valid_after(_NOW + 365 * _DAY) + .add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True) + .add_extension(_ku(ca=True), critical=True) + .sign(key, hashes.SHA256()) + ) + + +@functools.cache +def _rsa_key() -> rsa.RSAPrivateKey: + """One RSA key for every test that needs only a key the engine cannot check an ECDSA + signature with. Generated once: a 2048-bit key takes a noticeable fraction of a second.""" + return rsa.generate_private_key(public_exponent=65537, key_size=2048) + + +def _names_key(pem: bytes, ca: x509.Certificate) -> bool | None: + """How ``ca`` fits the CRL ``pem``'s authority key identifier: ``None`` when it does not fit, + and otherwise whether the CRL names its key (:func:`messagefoundry.pki._akid_fit`).""" + crl = x509.load_pem_x509_crl(pem) + listed = pki_module._listed_ca(ca.public_bytes(serialization.Encoding.DER)) + return pki_module._akid_fit(pki_module._crl_extensions(crl).akid, listed) + + +def _assert_uncheckable_and_the_start_warns( + tmp_path: Path, pem: bytes, caplog: pytest.LogCaptureFixture +) -> None: + """The warn arm for the CRL ``pem`` against the CAs in ``tmp_path / "ca.pem"``: the state is + ``uncheckable``, and a start loads the file with one WARNING that says so.""" + assert [s.state for s in judge_crl_signatures(pem, _ders(tmp_path / "ca.pem"))] == [ + "uncheckable" + ] + with caplog.at_level("WARNING"): + hop = _hop(tmp_path / "ca.pem", tmp_path / "crl.pem") + assert hop.verify_flags & ssl.VERIFY_CRL_CHECK_LEAF + assert len(logged := _warnings(caplog)) == 1 + assert "cannot check its signature" in logged[0] + + +@pytest.mark.parametrize("no_ski_first", [False, True], ids=["named-ca-first", "no-ski-ca-first"]) +def test_a_crl_its_named_ca_proves_bad_refuses_beside_an_unchecked_ca_with_no_ski( + twins: _Twins, tmp_path: Path, no_ski_first: bool +) -> None: + """The mixed case. The new CA carries the key identifier the CRL names, and its key is proven + not to verify the CRL. Beside it sits a CA of the same name with no Subject Key Identifier and + an RSA key, which fits the CRL and cannot check its ECDSA signature. Before vault BACKLOG #4159 + that read ``uncheckable`` and the start loaded the file with a WARNING.""" + no_ski = _ca_without_ski(_rsa_key()) + named = twins.new_ca.read_bytes() + mixed = tmp_path / "mixed.pem" + mixed.write_bytes( + no_ski.public_bytes(_PEM) + named if no_ski_first else named + no_ski.public_bytes(_PEM) + ) + pem = twins.write_crl("stranger", "new") + + # The case's premises: the CA with no Subject Key Identifier fits without being named, and + # the engine cannot check the signature with its key. + assert _names_key(pem, no_ski) is False + assert pki_module._verifies(x509.load_pem_x509_crl(pem), no_ski) is None + + assert [s.state for s in judge_crl_signatures(pem, _ders(mixed))] == ["bad"] + with pytest.raises(ValueError, match="does not verify the CRL's signature") as refused: + _hop(mixed, twins.crl) + assert "with no Subject Key Identifier does not change that" in str(refused.value) + + # The running-hop reload is unchanged: it held this file back before and still does. + verdict = crl_signature_refusal(pem, _ders(mixed)) + assert verdict is not None and verdict[1] is True + + # Control: OpenSSL itself fails the handshake on this CRL, whichever CA the file lists first, + # so the refusal refuses nothing that works on this interpreter's OpenSSL. + with pytest.raises(ssl.SSLError, match="signature"): + _handshake(_raw_hop(mixed, twins.crl), twins.server(sends_renewed=False)) + + +def test_a_strict_engine_context_refuses_a_chain_through_a_ca_with_no_ski(tmp_path: Path) -> None: + """(a) The premise the mixed-case refusal rests on: an engine verifying context sets + ``VERIFY_X509_STRICT``, and under it OpenSSL refuses a chain through a CA with no Subject Key + Identifier, so such a CA cannot make a CRL valid at a handshake. One builder is measured here, + the outbound one.""" + key = ec.generate_private_key(ec.SECP256R1()) + (tmp_path / "ca.pem").write_bytes(_ca_without_ski(key).public_bytes(_PEM)) + leaf, leaf_key = _leaf(_NAME, key) + (tmp_path / "leaf.pem").write_bytes(leaf.public_bytes(_PEM)) + (tmp_path / "leaf-key.pem").write_bytes(_key_pem(leaf_key)) + server = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + server.load_cert_chain(tmp_path / "leaf.pem", tmp_path / "leaf-key.pem") + client = build_verifying_client_context( + TrustAnchor(cafile=str(tmp_path / "ca.pem"), load_system_roots=False) + ) + + assert client.verify_flags & ssl.VERIFY_X509_STRICT + with pytest.raises(ssl.SSLError, match="Missing Subject Key Identifier"): + _handshake(client, server) + + # Control: the same chain passes once the strict flag is cleared, so it is that check, and + # not some other fault in the chain, that refuses it. This is the residual: a context + # without the flag could take such a CA for a CRL's issuer. + client.verify_flags &= ~ssl.VERIFY_X509_STRICT + _handshake(client, server) # raises if refused + + +def test_a_named_ca_that_cannot_be_checked_still_warns_beside_a_ca_with_no_ski( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """(b) The CA the CRL names by key identifier has an RSA key, and the CRL carries an ECDSA + signature, so that CA cannot be checked. A CA of the same name with no Subject Key + Identifier and an EC key sits beside it, and its key is proven not to verify the CRL. The + named CA could still have signed it, so the state stays ``uncheckable`` and the start warns.""" + rsa_key = _rsa_key() + named = _ca(rsa_key, rsa_key) + no_ski = _ca_without_ski(ec.generate_private_key(ec.SECP256R1())) + (tmp_path / "ca.pem").write_bytes(named.public_bytes(_PEM) + no_ski.public_bytes(_PEM)) + pem = _crl(ec.generate_private_key(ec.SECP256R1()), names=rsa_key) + (tmp_path / "crl.pem").write_bytes(pem) + crl = x509.load_pem_x509_crl(pem) + + assert _names_key(pem, named) is True and pki_module._verifies(crl, named) is None + assert _names_key(pem, no_ski) is False and pki_module._verifies(crl, no_ski) is False + + _assert_uncheckable_and_the_start_warns(tmp_path, pem, caplog) + + +def test_a_lone_ca_with_no_ski_that_cannot_be_checked_still_warns( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """(c) The only CA of that name carries no Subject Key Identifier and has an RSA key. The CRL + names a key by identifier and carries an ECDSA signature, so that CA fits, is not named, and + cannot be checked. No CA is proven bad, so the start warns.""" + no_ski = _ca_without_ski(_rsa_key()) + (tmp_path / "ca.pem").write_bytes(no_ski.public_bytes(_PEM)) + signer = ec.generate_private_key(ec.SECP256R1()) + pem = _crl(signer, names=signer) + (tmp_path / "crl.pem").write_bytes(pem) + + assert _names_key(pem, no_ski) is False + assert pki_module._verifies(x509.load_pem_x509_crl(pem), no_ski) is None + + _assert_uncheckable_and_the_start_warns(tmp_path, pem, caplog) + + # --- the warn arm: one line for the file, however many contexts load it ------------------------- From f9647dfb689869077cd1f015ac9861a08c8cdcf3 Mon Sep 17 00:00:00 2001 From: Scott Hall Date: Tue, 27 Oct 2026 17:37:25 -0500 Subject: [PATCH 2/9] CRL signature check: scope the no-SKI premise to what was measured, and record its residuals Review round-a9f9bcb3ff repairs. OpenSSL skips its strict checks on a one-certificate chain, so a peer presenting a listed self-signed CA with no SKI can pass a handshake on a CRL the start now refuses (measured on CPython 3.14.7, OpenSSL 3.5.7). The prose now claims strict refusal only for chains of two or more certificates and names that residual beside the OpenSSL-floor one. The rule itself is unchanged. The reload's BAD_SIGNATURE remedy no longer says every uncheckable signature loads at start. The refusal text is back to main's. The warning text is clearer, CONFIGURATION.md says "fitting CA", and a test pins the CA-file monitor's remedy for the mixed case. Vault BACKLOG #4159. --- changelog.d/4159.security.md | 14 ++++++++------ docs/CONFIGURATION.md | 15 +++++++++------ messagefoundry/config/tls_policy.py | 8 +++----- messagefoundry/pipeline/crl_reload.py | 7 ++++--- messagefoundry/pki.py | 26 +++++++++++++++----------- tests/test_crl_signature_at_start.py | 21 ++++++++++++++++----- 6 files changed, 55 insertions(+), 36 deletions(-) diff --git a/changelog.d/4159.security.md b/changelog.d/4159.security.md index a374f42dfd..234cb8f846 100644 --- a/changelog.d/4159.security.md +++ b/changelog.d/4159.security.md @@ -1,9 +1,11 @@ - **A start now refuses a CRL that the CA it names by key identifier proves badly signed, even beside a CA of the same name that has no subject key identifier and cannot be checked.** Before, that second CA made the signature check read "cannot check", so the start loaded the file with a - WARNING. Such a CA cannot make the CRL valid at a handshake: every verifying context the engine - builds uses strict verification, and OpenSSL 3 refuses a chain through a CA with no subject key - identifier. A CA the CRL names that cannot be checked still warns, and so does a lone CA with no - subject key identifier that cannot be checked. The running-hop reload is unchanged. An OpenSSL - build without that strict check could take such a CA for the CRL's issuer; the engine sets no - OpenSSL version floor. (vault BACKLOG #4159) + WARNING. Every verifying context the engine builds uses strict verification, and OpenSSL 3 + refuses a chain of two or more certificates through a CA with no subject key identifier. A CA + the CRL names that cannot be checked still warns, and so does a lone CA with no subject key + identifier that cannot be checked. The running-hop reload still holds such a file back. The + start can refuse a CRL a handshake could pass in at least two cases: a peer that presents such a + CA alone, as a one-certificate chain, which OpenSSL does not check strictly, and an OpenSSL + build without that strict check; the engine sets no OpenSSL version floor. (vault BACKLOG + #4159) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 453f91ae89..b17a2c2af2 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2001,14 +2001,17 @@ otherwise (the first bullet below says what that leaves out). The CRL names that key identifier. The engine ran the signature check with that key, and the check failed. Where several listed CAs share that name, the check must run and fail for each of them that the engine finds to fit the CRL's -authority key identifier and that carries a subject key identifier, and no CA of that name may +authority key identifier and that carries a subject key identifier, and no fitting CA may verify the CRL. A fitting CA with no subject key identifier that the engine cannot check does not stop the refusal (vault BACKLOG #4159). Every verifying context the engine builds uses strict -verification, under which OpenSSL 3 refuses a chain through such a CA, so it cannot make the CRL -valid at a handshake. That rests on the OpenSSL build: the engine sets no OpenSSL version floor, -and a build without that strict check could take such a CA for the CRL's issuer. A handshake -that OpenSSL judges by such a CRL fails with -`CRL signature failure`, an error that does not name the file. The refusal names the setting, the +verification, under which OpenSSL 3 refuses a chain of two or more certificates through such a +CA, so it cannot make the CRL valid for that chain. At least two cases are left where the start +refuses a CRL that a handshake could pass. OpenSSL skips its strict checks when the peer presents +one certificate: a peer whose certificate is such a self-signed CA, listed for the hop, was +measured to pass a strict handshake judged by a CRL that CA signed. And an OpenSSL build without +the strict check could take such a CA for the CRL's issuer; the engine sets no OpenSSL version +floor. A handshake that OpenSSL judges by a CRL the start refuses through a listed CA's chain +fails with `CRL signature failure`, an error that does not name the file. The refusal names the setting, the file and the CRL. Give the setting the CRL that CA signed. No setting turns this check off, as for an expired CRL. diff --git a/messagefoundry/config/tls_policy.py b/messagefoundry/config/tls_policy.py index 8d0b808a3c..70215726ca 100644 --- a/messagefoundry/config/tls_policy.py +++ b/messagefoundry/config/tls_policy.py @@ -571,8 +571,8 @@ def _refuse_crl_count(loaded: int, blocks: Sequence[CrlBlock], *, label: str) -> "name that CA's key, so a renewed CA key could explain it" ), "uncheckable": ( - "the engine cannot check its signature with the listed CA's key, where that CA carries " - "the key identifier the CRL names, or the CRL names no listed CA's key" + "the engine cannot check its signature with the key of the listed CA the CRL names by key " + "identifier, or, where it names none, of a listed CA of that name" ), } #: How many CRLs that warning describes before it only counts the rest. @@ -645,9 +645,7 @@ def _check_crl_signatures( raise ValueError( f"{label}: {bad.where} names the key of a CA certificate this hop trusts, and that " "key does not verify the CRL's signature. A handshake OpenSSL judges by this CRL " - "fails with 'CRL signature failure'. A listed CA of that name with no Subject Key " - "Identifier does not change that: strict verification refuses a chain through such " - "a CA. Give this setting the CRL that CA signed, or " + "fails with 'CRL signature failure'. Give this setting the CRL that CA signed, or " "remove this CRL if a good one from that issuer is already in the file" ) unconfirmed = [signature for signature in judged if signature.state != "verified"] diff --git a/messagefoundry/pipeline/crl_reload.py b/messagefoundry/pipeline/crl_reload.py index 75297dd31a..13f6e78781 100644 --- a/messagefoundry/pipeline/crl_reload.py +++ b/messagefoundry/pipeline/crl_reload.py @@ -200,9 +200,10 @@ "refuses the file too only where it proves the signature bad. Short of that proof the " "signature does not stop a start: unless another rule refuses the file, a start loads it " "with a warning, and each handshake then decides. That covers at least a CRL that names no " - "key and a signature the engine cannot check. The handshake passes where the peer's chain " - "carries the CA that signed the CRL, as after a CA renews its key, and fails where it does " - "not" + "key and a signature the engine cannot check with the key the CRL names, or, where it names " + "no listed CA's key, with any listed CA of that name. The handshake passes where the peer's " + "chain carries the CA that signed the CRL, as after a CA renews its key, and fails where it " + "does not" ) NO_ISSUER = ( "Restart the engine to apply the file. The reload checks a CRL's signature against the hop's " diff --git a/messagefoundry/pki.py b/messagefoundry/pki.py index a9161030fb..fcd96b902e 100644 --- a/messagefoundry/pki.py +++ b/messagefoundry/pki.py @@ -444,12 +444,17 @@ class CrlSignature: Identifier, every fitting CA so named was checked and its key does not verify the CRL, and no fitting CA verifies it. The CRL names that key, and that key did not sign it. A fitting CA with no Subject Key Identifier that could not be checked does not lift this to - ``uncheckable`` (vault BACKLOG #4159, the reading of #2905 item 1 decided 2026-10-26): every - verifying engine context sets ``VERIFY_X509_STRICT``, under which OpenSSL 3 refuses a chain - through a CA with no Subject Key Identifier, so that CA cannot make the CRL valid at a - handshake. Measured 2026-10-27 on CPython 3.14.7 with OpenSSL 3.5.7. An OpenSSL build or a - context without that strict check could still take such a CA for the issuer; the engine - sets no OpenSSL floor. + ``uncheckable`` (vault BACKLOG #4159, the reading of #2905 item 1 decided 2026-10-26). The + reason: every verifying engine context sets ``VERIFY_X509_STRICT``, under which OpenSSL 3 + refuses a chain of two or more certificates through a CA with no Subject Key Identifier, + so such a CA cannot make the CRL valid for that chain. Measured 2026-10-27 on CPython + 3.14.7 with OpenSSL 3.5.7. **At least two residuals, where the start refuses a CRL a + handshake could pass.** OpenSSL skips its strict checks on a one-certificate chain: a peer + that presents a self-signed CA with no Subject Key Identifier, listed here, passed a strict + handshake judged by a CRL that CA signed, measured the same day. And an OpenSSL build or a + context without the strict check could take such a CA for the issuer; the engine sets no + OpenSSL floor. A fitting CA with no Subject Key Identifier that verifies the CRL still + makes it ``verified``. * ``uncheckable``: no fitting CA verifies, and some fitting CA could not be checked: one the CRL names by key identifier, or, where the CRL names no fitting CA's key, any. "Cannot check" is not "does not verify" (:func:`_verifies`). @@ -884,11 +889,10 @@ def _signature_state( named_unchecked = named_unchecked or (answer is None and names_key) if not fitting: return "unlisted" - # Vault BACKLOG #4159: proof outranks a CA with no Subject Key Identifier that could not be - # checked. Where the CRL names a key, _akid_fit drops a CA whose identifier differs, so every - # fitting CA it does not name carries none, and no strict context chains through such a CA. - if named and not named_unchecked: - return "bad" + if named: + # Where the CRL names a key, _akid_fit drops a CA whose identifier differs, so a fitting + # CA it does not name carries none; CrlSignature says why that one cannot lift ``bad``. + return "uncheckable" if named_unchecked else "bad" return "uncheckable" if unchecked else "unproven" diff --git a/tests/test_crl_signature_at_start.py b/tests/test_crl_signature_at_start.py index 10ae5f0440..b56191de84 100644 --- a/tests/test_crl_signature_at_start.py +++ b/tests/test_crl_signature_at_start.py @@ -27,6 +27,7 @@ import functools import logging import ssl +import time import warnings import weakref from dataclasses import dataclass @@ -48,7 +49,7 @@ harden_crl_check, ) from messagefoundry.keywrap import der_children, read_der_tlv -from messagefoundry.pipeline import crl_reload +from messagefoundry.pipeline import cert_expiry, crl_reload from messagefoundry.pipeline.crl_reload import BAD_SIGNATURE, FIX, reload_replaced_crls from messagefoundry.pki import crl_signature_refusal, judge_crl_signatures from tests.test_crl_bundle_anchors import _ku @@ -480,16 +481,26 @@ def test_a_crl_its_named_ca_proves_bad_refuses_beside_an_unchecked_ca_with_no_sk assert pki_module._verifies(x509.load_pem_x509_crl(pem), no_ski) is None assert [s.state for s in judge_crl_signatures(pem, _ders(mixed))] == ["bad"] - with pytest.raises(ValueError, match="does not verify the CRL's signature") as refused: + with pytest.raises(ValueError, match="does not verify the CRL's signature"): _hop(mixed, twins.crl) - assert "with no Subject Key Identifier does not change that" in str(refused.value) + + # The CA-file monitor gives the proven remedy, where it asked for a check before. + verdict_of_scan = cert_expiry._ca_file_crl_refusal( + mixed.read_bytes() + pem, + time.time(), + None, + ca_file_only=True, + on_unchecked=lambda: None, + ) + assert verdict_of_scan is not None + assert verdict_of_scan[1].startswith("Replace the CRL in the CA file with one its CA signed") # The running-hop reload is unchanged: it held this file back before and still does. verdict = crl_signature_refusal(pem, _ders(mixed)) assert verdict is not None and verdict[1] is True - # Control: OpenSSL itself fails the handshake on this CRL, whichever CA the file lists first, - # so the refusal refuses nothing that works on this interpreter's OpenSSL. + # Control: OpenSSL itself fails this chain's handshake on this CRL, in either listing order. + # One chain shape only, a leaf under the named CA; pki.CrlSignature names the residuals. with pytest.raises(ssl.SSLError, match="signature"): _handshake(_raw_hop(mixed, twins.crl), twins.server(sends_renewed=False)) From c801cdc7cdc242c2127db362f62c4bc12289be02 Mon Sep 17 00:00:00 2001 From: Scott Hall Date: Tue, 27 Oct 2026 17:59:51 -0500 Subject: [PATCH 3/9] CRL signature check: pin the one-certificate residual, tighten the control, soften completeness claims Review round-0125698bbc repairs, the second and last round. A test now pins the measured residual: a listed self-signed CA with no SKI, presented alone, passes a strict engine handshake judged by its own CRL, and revoking its serial refuses it. The mixed-case control now matches "CRL signature failure". The "every verifying context" claims now say what was read and what was measured. The reload remedy says "some listed CA that fits it". ADR 0173 names the narrowed warn case. Vault BACKLOG #4159. --- changelog.d/4159.security.md | 2 +- docs/CONFIGURATION.md | 20 ++++---- ...ss-terminating-and-originating-surfaces.md | 5 +- messagefoundry/pipeline/crl_reload.py | 2 +- messagefoundry/pki.py | 8 +-- tests/test_crl_signature_at_start.py | 51 +++++++++++++++---- 6 files changed, 62 insertions(+), 26 deletions(-) diff --git a/changelog.d/4159.security.md b/changelog.d/4159.security.md index 234cb8f846..0cf8793979 100644 --- a/changelog.d/4159.security.md +++ b/changelog.d/4159.security.md @@ -1,7 +1,7 @@ - **A start now refuses a CRL that the CA it names by key identifier proves badly signed, even beside a CA of the same name that has no subject key identifier and cannot be checked.** Before, that second CA made the signature check read "cannot check", so the start loaded the file with a - WARNING. Every verifying context the engine builds uses strict verification, and OpenSSL 3 + WARNING. The verifying contexts the engine builds use strict verification, and OpenSSL 3 refuses a chain of two or more certificates through a CA with no subject key identifier. A CA the CRL names that cannot be checked still warns, and so does a lone CA with no subject key identifier that cannot be checked. The running-hop reload still holds such a file back. The diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index b17a2c2af2..f885ac5d89 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2003,16 +2003,16 @@ several listed CAs share that name, the check must run and fail for each of them that the engine finds to fit the CRL's authority key identifier and that carries a subject key identifier, and no fitting CA may verify the CRL. A fitting CA with no subject key identifier that the engine cannot check does not -stop the refusal (vault BACKLOG #4159). Every verifying context the engine builds uses strict -verification, under which OpenSSL 3 refuses a chain of two or more certificates through such a -CA, so it cannot make the CRL valid for that chain. At least two cases are left where the start -refuses a CRL that a handshake could pass. OpenSSL skips its strict checks when the peer presents -one certificate: a peer whose certificate is such a self-signed CA, listed for the hop, was -measured to pass a strict handshake judged by a CRL that CA signed. And an OpenSSL build without -the strict check could take such a CA for the CRL's issuer; the engine sets no OpenSSL version -floor. A handshake that OpenSSL judges by a CRL the start refuses through a listed CA's chain -fails with `CRL signature failure`, an error that does not name the file. The refusal names the setting, the -file and the CRL. Give the setting the CRL that CA signed. No setting turns this check off, as for +stop the refusal (vault BACKLOG #4159). The verifying contexts the engine builds use strict +verification, as read from their call sites and measured for at least the outbound one. Under it +OpenSSL 3 refuses a chain of two or more certificates through such a CA, so it cannot make the +CRL valid for that chain. At least two cases are left where the start refuses a CRL that a +handshake could pass. OpenSSL skips its strict checks when the peer presents one certificate: a +peer whose certificate is such a self-signed CA, listed for the hop, was measured to pass a strict +handshake judged by a CRL that CA signed. And an OpenSSL build without the strict check could take +such a CA for the CRL's issuer; the engine sets no OpenSSL version floor. A handshake whose chain +runs through the named CA, judged by such a CRL, fails with `CRL signature failure`, an error that +does not name the file. The refusal names the setting, the file and the CRL. Give the setting the CRL that CA signed. No setting turns this check off, as for an expired CRL. The check reads only the file a CRL setting names. A CRL can also reach a hop through its CA file diff --git a/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md b/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md index e4e29ae2d2..90cf2e5183 100644 --- a/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md +++ b/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md @@ -1279,7 +1279,10 @@ hop's own CA list. It refuses one case only: the CRL names a listed CA's key by engine ran the signature check with that key, and the check failed (corrected under vault BACKLOG #3506; it read "that key did not sign it", which is also true of a signature of another kind than the key, and that loads). Anything short of that proof loads with a WARNING, at least an issuer the -hop does not list, a CRL that names no key, and a key the engine cannot check with. `docs/CONFIGURATION.md`, +hop does not list, a CRL that names no key, and a key the engine cannot check with. Since vault +BACKLOG #4159, that last case loads only where the unchecked key is the one the CRL names, or the +CRL names no listed key: a same-name CA with no subject key identifier that cannot be checked no +longer stops the refusal. `docs/CONFIGURATION.md`, under "CRL files", is the one place that says how often it is logged and what a refusal stops. So the reload refuses more than a start does, on purpose. The ruling's reason is a CA that renewed its key: its CRL can fail the engine's check while handshakes still pass, because OpenSSL diff --git a/messagefoundry/pipeline/crl_reload.py b/messagefoundry/pipeline/crl_reload.py index 13f6e78781..da1e317324 100644 --- a/messagefoundry/pipeline/crl_reload.py +++ b/messagefoundry/pipeline/crl_reload.py @@ -201,7 +201,7 @@ "signature does not stop a start: unless another rule refuses the file, a start loads it " "with a warning, and each handshake then decides. That covers at least a CRL that names no " "key and a signature the engine cannot check with the key the CRL names, or, where it names " - "no listed CA's key, with any listed CA of that name. The handshake passes where the peer's " + "no listed CA's key, with some listed CA that fits it. The handshake passes where the peer's " "chain carries the CA that signed the CRL, as after a CA renews its key, and fails where it " "does not" ) diff --git a/messagefoundry/pki.py b/messagefoundry/pki.py index fcd96b902e..d2608f121d 100644 --- a/messagefoundry/pki.py +++ b/messagefoundry/pki.py @@ -445,10 +445,10 @@ class CrlSignature: no fitting CA verifies it. The CRL names that key, and that key did not sign it. A fitting CA with no Subject Key Identifier that could not be checked does not lift this to ``uncheckable`` (vault BACKLOG #4159, the reading of #2905 item 1 decided 2026-10-26). The - reason: every verifying engine context sets ``VERIFY_X509_STRICT``, under which OpenSSL 3 - refuses a chain of two or more certificates through a CA with no Subject Key Identifier, - so such a CA cannot make the CRL valid for that chain. Measured 2026-10-27 on CPython - 3.14.7 with OpenSSL 3.5.7. **At least two residuals, where the start refuses a CRL a + reason: the verifying engine contexts set ``VERIFY_X509_STRICT``, by the decision's + reading of the call sites, and under it OpenSSL 3 refuses a chain of two or more + certificates through a CA with no Subject Key Identifier, so such a CA cannot make the CRL + valid for that chain. Measured 2026-10-27 on CPython 3.14.7 with OpenSSL 3.5.7. **At least two residuals, where the start refuses a CRL a handshake could pass.** OpenSSL skips its strict checks on a one-certificate chain: a peer that presents a self-signed CA with no Subject Key Identifier, listed here, passed a strict handshake judged by a CRL that CA signed, measured the same day. And an OpenSSL build or a diff --git a/tests/test_crl_signature_at_start.py b/tests/test_crl_signature_at_start.py index b56191de84..c2686d3ec5 100644 --- a/tests/test_crl_signature_at_start.py +++ b/tests/test_crl_signature_at_start.py @@ -412,10 +412,11 @@ def test_a_ca_that_does_not_parse_is_ignored(twins: _Twins) -> None: # --- vault BACKLOG #4159: a CA with no Subject Key Identifier cannot rescue a proven bad CRL ------ -def _ca_without_ski(key: _CaKey) -> x509.Certificate: +def _ca_without_ski(key: _CaKey, *, localhost: bool = False) -> x509.Certificate: """A self-signed CA certificate for ``key`` named :data:`_NAME` that carries no Subject Key - Identifier, and so no Authority Key Identifier either: nothing else here names a key.""" - return ( + Identifier, and so no Authority Key Identifier either: nothing else here names a key. With + ``localhost`` it also names ``localhost``, so a server can present it as its own.""" + builder = ( x509.CertificateBuilder() .subject_name(_NAME) .issuer_name(_NAME) @@ -425,8 +426,12 @@ def _ca_without_ski(key: _CaKey) -> x509.Certificate: .not_valid_after(_NOW + 365 * _DAY) .add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True) .add_extension(_ku(ca=True), critical=True) - .sign(key, hashes.SHA256()) ) + if localhost: + builder = builder.add_extension( + x509.SubjectAlternativeName([x509.DNSName("localhost")]), False + ) + return builder.sign(key, hashes.SHA256()) @functools.cache @@ -500,16 +505,17 @@ def test_a_crl_its_named_ca_proves_bad_refuses_beside_an_unchecked_ca_with_no_sk assert verdict is not None and verdict[1] is True # Control: OpenSSL itself fails this chain's handshake on this CRL, in either listing order. - # One chain shape only, a leaf under the named CA; pki.CrlSignature names the residuals. - with pytest.raises(ssl.SSLError, match="signature"): + # One chain shape only, a leaf under the named CA, with a no-SKI CA whose key is of another + # kind than the leaf's signature; pki.CrlSignature names the residuals. + with pytest.raises(ssl.SSLError, match="CRL signature failure"): _handshake(_raw_hop(mixed, twins.crl), twins.server(sends_renewed=False)) def test_a_strict_engine_context_refuses_a_chain_through_a_ca_with_no_ski(tmp_path: Path) -> None: """(a) The premise the mixed-case refusal rests on: an engine verifying context sets - ``VERIFY_X509_STRICT``, and under it OpenSSL refuses a chain through a CA with no Subject Key - Identifier, so such a CA cannot make a CRL valid at a handshake. One builder is measured here, - the outbound one.""" + ``VERIFY_X509_STRICT``, and under it OpenSSL refuses a chain of two or more certificates + through a CA with no Subject Key Identifier, so such a CA cannot make a CRL valid for that + chain. One builder is measured here, the outbound one.""" key = ec.generate_private_key(ec.SECP256R1()) (tmp_path / "ca.pem").write_bytes(_ca_without_ski(key).public_bytes(_PEM)) leaf, leaf_key = _leaf(_NAME, key) @@ -532,6 +538,33 @@ def test_a_strict_engine_context_refuses_a_chain_through_a_ca_with_no_ski(tmp_pa _handshake(client, server) # raises if refused +def test_a_one_certificate_chain_through_a_ca_with_no_ski_passes_a_strict_handshake( + tmp_path: Path, +) -> None: + """The residual pki.CrlSignature records: OpenSSL skips its strict checks on a chain of one + certificate. A peer that presents a listed self-signed CA with no Subject Key Identifier as + its own certificate passes a strict engine handshake judged by a CRL that CA signed, so that + CA can make a CRL valid for that chain. If this goes red, the residual has moved: correct + the texts that name it (the CrlSignature docstring, CONFIGURATION.md, the changelog).""" + key = ec.generate_private_key(ec.SECP256R1()) + ca = _ca_without_ski(key, localhost=True) + (tmp_path / "ca.pem").write_bytes(ca.public_bytes(_PEM)) + (tmp_path / "ca-key.pem").write_bytes(_key_pem(key)) + (tmp_path / "crl.pem").write_bytes(_crl(key, names=key)) + server = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + server.load_cert_chain(tmp_path / "ca.pem", tmp_path / "ca-key.pem") + + client = _hop(tmp_path / "ca.pem", tmp_path / "crl.pem") + assert client.verify_flags & ssl.VERIFY_X509_STRICT + assert client.verify_flags & ssl.VERIFY_CRL_CHECK_LEAF + _handshake(client, server) # raises if refused + + # Control: the CRL is live for this chain. Revoking the peer's own serial refuses it. + (tmp_path / "crl.pem").write_bytes(_crl(key, names=key, revoke=ca.serial_number)) + with pytest.raises(ssl.SSLError, match="revoked"): + _handshake(_hop(tmp_path / "ca.pem", tmp_path / "crl.pem"), server) + + def test_a_named_ca_that_cannot_be_checked_still_warns_beside_a_ca_with_no_ski( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: From e044dd95fc85505c61a73bb096eee3ba49592997 Mon Sep 17 00:00:00 2001 From: Scott Hall Date: Tue, 27 Oct 2026 20:32:03 -0500 Subject: [PATCH 4/9] CRL signature check: a CA with no SKI counts for nothing once the CRL names a key; a key that accepts the signature verifies it Fixes the two defects in the Lander's covering review of engine PR 2585 (round-f2be79dfb0), and findings 3, 4, 5, 6, 8, 9 and 10. - pki._signature_state reads every fit first. Where the CRL names a fitting CA's key, only the named CAs are judged, so a fitting CA with no SKI no longer rescues a CRL the named CA proves bad, whether its key verifies, fails or cannot be checked, in either CA-file order. - pki._verifies returns True for a signature the key accepts after is_signature_valid refused it (SHA-1). None now means only a key or algorithm the engine cannot check. One signer, one verdict per digest. - Wording: the start refusal, the CA-file remedy, the uncheckable warning, the _check_crl_signatures docstring, the changelog. The premise and residuals are stated once, in CONFIGURATION.md. - Tests: the verifying no-SKI case, the residual set up whole, the SHA-1 verdict with OpenSSL's handshake as the premise, and one signer under SHA-256 and SHA-1. vault BACKLOG #4159 --- changelog.d/4159.security.md | 21 +- docs/CONFIGURATION.md | 49 +++-- ...ss-terminating-and-originating-surfaces.md | 4 +- messagefoundry/config/tls_policy.py | 18 +- messagefoundry/pipeline/cert_expiry.py | 4 +- messagefoundry/pki.py | 94 ++++---- tests/test_crl_held_copy_alert.py | 7 +- tests/test_crl_reload.py | 2 +- tests/test_crl_signature_at_start.py | 208 ++++++++++++++---- 9 files changed, 280 insertions(+), 127 deletions(-) diff --git a/changelog.d/4159.security.md b/changelog.d/4159.security.md index 0cf8793979..df7681736c 100644 --- a/changelog.d/4159.security.md +++ b/changelog.d/4159.security.md @@ -1,11 +1,14 @@ - **A start now refuses a CRL that the CA it names by key identifier proves badly signed, even - beside a CA of the same name that has no subject key identifier and cannot be checked.** Before, - that second CA made the signature check read "cannot check", so the start loaded the file with a - WARNING. The verifying contexts the engine builds use strict verification, and OpenSSL 3 - refuses a chain of two or more certificates through a CA with no subject key identifier. A CA - the CRL names that cannot be checked still warns, and so does a lone CA with no subject key - identifier that cannot be checked. The running-hop reload still holds such a file back. The - start can refuse a CRL a handshake could pass in at least two cases: a peer that presents such a - CA alone, as a one-certificate chain, which OpenSSL does not check strictly, and an OpenSSL - build without that strict check; the engine sets no OpenSSL version floor. (vault BACKLOG + beside a CA of the same name that has no subject key identifier.** Before, that second CA could + rescue the CRL: where its key verified the CRL, the start loaded the file with no warning, and + where the engine could not check its key, the start loaded the file with a WARNING. Now such a + CA counts for nothing once the CRL names another listed CA's key, and the running-hop reload + holds such a file back too. A CA the CRL names that cannot be checked still warns, and so does a + lone CA with no subject key identifier that cannot be checked. A signature the CA's key accepts + is now verified even where the `cryptography` library's own check refuses it, as for an RSA + signature over SHA-1 the CA made, so the verdict no longer depends on the digest: a start loads + such a CRL with no warning, and the reload applies it where it held it back before. The premise, + that the engine's verifying contexts use strict verification, and the cases where the start can + refuse a CRL a handshake could pass are stated in `docs/CONFIGURATION.md`, in the paragraph + that starts "Why a CA with no subject key identifier counts for nothing there". (vault BACKLOG #4159) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index f885ac5d89..648ff0844a 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2001,19 +2001,29 @@ otherwise (the first bullet below says what that leaves out). The CRL names that key identifier. The engine ran the signature check with that key, and the check failed. Where several listed CAs share that name, the check must run and fail for each of them that the engine finds to fit the CRL's -authority key identifier and that carries a subject key identifier, and no fitting CA may -verify the CRL. A fitting CA with no subject key identifier that the engine cannot check does not -stop the refusal (vault BACKLOG #4159). The verifying contexts the engine builds use strict -verification, as read from their call sites and measured for at least the outbound one. Under it -OpenSSL 3 refuses a chain of two or more certificates through such a CA, so it cannot make the -CRL valid for that chain. At least two cases are left where the start refuses a CRL that a -handshake could pass. OpenSSL skips its strict checks when the peer presents one certificate: a -peer whose certificate is such a self-signed CA, listed for the hop, was measured to pass a strict -handshake judged by a CRL that CA signed. And an OpenSSL build without the strict check could take -such a CA for the CRL's issuer; the engine sets no OpenSSL version floor. A handshake whose chain -runs through the named CA, judged by such a CRL, fails with `CRL signature failure`, an error that -does not name the file. The refusal names the setting, the file and the CRL. Give the setting the CRL that CA signed. No setting turns this check off, as for -an expired CRL. +authority key identifier and that carries the subject key identifier the CRL names. A fitting CA +with no subject key identifier counts for nothing then, whether its key verifies the CRL, does +not, or cannot be checked (vault BACKLOG #4159). A handshake whose chain runs through the named CA +and that OpenSSL judges by such a CRL fails with `CRL signature failure`, an error that does not +name the file. The refusal names the setting, the file and the CRL. Give the setting the CRL that +CA signed. No setting turns this check off, as for an expired CRL. + +**Why a CA with no subject key identifier counts for nothing there, and where that refuses a CRL a +handshake could pass.** This paragraph is the one statement of that premise and its residuals; +the code and the changelog point here. The verifying contexts the engine builds set OpenSSL's +strict verification, as read from their call sites and measured for at least the outbound one. +Under it OpenSSL 3 refuses a verified chain of two or more certificates through a CA with no +subject key identifier, so such a CA cannot make the CRL valid for that chain. At least two cases +are left where the start refuses a CRL that a handshake could pass: + +- OpenSSL skips those strict checks where the chain it verifies is one certificate long. That is + the length of the chain it builds, which ends at a listed CA, not the number of certificates the + peer sends. So a peer whose own certificate is such a CA, self-signed and listed for the hop, + passes. It was measured, on CPython 3.14.7 with OpenSSL 3.5.7, to pass a strict handshake judged + by a CRL that CA signed and that names the key of another listed CA of the same name, a CRL the + start refuses. +- An OpenSSL build or a verifying context without the strict check could take such a CA for the + CRL's issuer. The engine sets no OpenSSL version floor. The check reads only the file a CRL setting names. A CRL can also reach a hop through its CA file alone, on at least the hops that the paragraph on a CRL inside a CA file, further down, names. The @@ -2044,11 +2054,14 @@ three cases warn: - A listed CA has the CRL's issuer name and does not verify it, but the CRL names no key. A CA that renewed its key can read this way while handshakes still pass. That was measured for one chain shape, where the peer sends the renewed CA certified by the old key. -- The engine cannot check the signature with the listed CA's key, even where the CRL names that key. - Where the CRL names a listed CA's key, only that CA counts here; a CA of that name with no subject - key identifier that cannot be checked does not (the refusal above says why). Two measured - examples are an ECDSA signature on a CRL that names an RSA key, and an RSA signature over SHA-1 - that the CA did make. +- The engine cannot check the signature with a fitting listed CA's key, because it does not support + that algorithm or that pairing of key and signature. Where the CRL names a listed CA's key, only + that CA counts here (the refusal above says why). Where it names no listed CA's key, as when it + names a key no listed CA carries, every fitting CA counts, one with no subject key identifier + included. A measured example is an ECDSA signature on a CRL that names an RSA key. A signature + the CA's key accepts is verified even where the `cryptography` library's own check refuses it, + as for an RSA signature over SHA-1 that the CA did make; OpenSSL 3.5.7 was measured to judge a + peer by such a CRL (vault BACKLOG #4159). If the check itself fails, the engine logs an ERROR that says so. It loads the file unless a CRL it judged before the failure is proved badly signed, which refuses the file. Where it loads, the diff --git a/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md b/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md index 90cf2e5183..a6e617e6f6 100644 --- a/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md +++ b/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md @@ -1281,8 +1281,8 @@ engine ran the signature check with that key, and the check failed (corrected un the key, and that loads). Anything short of that proof loads with a WARNING, at least an issuer the hop does not list, a CRL that names no key, and a key the engine cannot check with. Since vault BACKLOG #4159, that last case loads only where the unchecked key is the one the CRL names, or the -CRL names no listed key: a same-name CA with no subject key identifier that cannot be checked no -longer stops the refusal. `docs/CONFIGURATION.md`, +CRL names no listed key: a same-name CA with no subject key identifier no longer stops the +refusal, whether its key verifies the CRL or cannot be checked. `docs/CONFIGURATION.md`, under "CRL files", is the one place that says how often it is logged and what a refusal stops. So the reload refuses more than a start does, on purpose. The ruling's reason is a CA that renewed its key: its CRL can fail the engine's check while handshakes still pass, because OpenSSL diff --git a/messagefoundry/config/tls_policy.py b/messagefoundry/config/tls_policy.py index 70215726ca..fa749a75d2 100644 --- a/messagefoundry/config/tls_policy.py +++ b/messagefoundry/config/tls_policy.py @@ -572,7 +572,8 @@ def _refuse_crl_count(loaded: int, blocks: Sequence[CrlBlock], *, label: str) -> ), "uncheckable": ( "the engine cannot check its signature with the key of the listed CA the CRL names by key " - "identifier, or, where it names none, of a listed CA of that name" + "identifier or, where the CRL names no listed CA's key, with the key of some listed CA " + "that fits it" ), } #: How many CRLs that warning describes before it only counts the rest. @@ -592,9 +593,11 @@ def _check_crl_signatures( unverified CRL already failed closed there, so what this adds is the diagnosis, at start. **Everything short of proof loads, with one warning.** That covers at least a CRL that names - no key, a named key the engine cannot check with, and a CRL the engine matches to no listed - CA. The last is the usual case in a two-tier PKI, where the peer sends the intermediate that issued - the CRL, so it must not log per hop. The warning is one line for the file, and + no key, a key the engine cannot check with, and a CRL the engine matches to no listed CA. The + key is the one the CRL names or, where it names no listed CA's key, that of any listed CA + that fits it, one with no Subject Key Identifier included. A CRL matched to no listed CA is + the usual case in a two-tier PKI, where the peer sends the intermediate that issued the CRL, + so it must not log per hop. The warning is one line for the file, and :func:`~messagefoundry.config.loaded_crls.first_unconfirmed_signature` decides how often it is logged. That function is asked only when there is a line to log. So it holds the last file version that logged one, and an earlier version is logged again only when it returns after @@ -644,9 +647,10 @@ def _check_crl_signatures( if (bad := next((s for s in judged if s.state == "bad"), None)) is not None: raise ValueError( f"{label}: {bad.where} names the key of a CA certificate this hop trusts, and that " - "key does not verify the CRL's signature. A handshake OpenSSL judges by this CRL " - "fails with 'CRL signature failure'. Give this setting the CRL that CA signed, or " - "remove this CRL if a good one from that issuer is already in the file" + "key does not verify the CRL's signature. A handshake whose chain runs through that " + "CA and that OpenSSL judges by this CRL fails with 'CRL signature failure'. Give this " + "setting the CRL that CA signed, or remove this CRL if a good one from that issuer is " + "already in the file" ) unconfirmed = [signature for signature in judged if signature.state != "verified"] finding = tuple(signature.state for signature in judged) diff --git a/messagefoundry/pipeline/cert_expiry.py b/messagefoundry/pipeline/cert_expiry.py index eedc18e408..010d4b1234 100644 --- a/messagefoundry/pipeline/cert_expiry.py +++ b/messagefoundry/pipeline/cert_expiry.py @@ -772,8 +772,8 @@ def _ca_file_crl_refusal( return ( "whose CA file now holds a CRL that the CA key it names does not verify", "Replace the CRL in the CA file with one its CA signed before restarting: a handshake " - "OpenSSL judges by it fails. If a good CRL from that issuer is already in the file, " - "remove this one instead", + "OpenSSL judges by it through the CA it names fails. If a good CRL from that issuer " + "is already in the file, remove this one instead", ) if states - _PASSING - _UNCONFIRMED: # A state this scan does not name, beside any other: never a pass and never the weaker diff --git a/messagefoundry/pki.py b/messagefoundry/pki.py index d2608f121d..8e3aa2b22b 100644 --- a/messagefoundry/pki.py +++ b/messagefoundry/pki.py @@ -439,25 +439,21 @@ class CrlSignature: :func:`_issuer_name_key` names, so every text that describes ``unlisted`` must stay true of a CA OpenSSL takes for the issuer and this test does not. - * ``verified``: a CA that fits verifies the CRL. - * ``bad``: **proven** badly signed. The CRL's key identifier equals a fitting CA's Subject Key - Identifier, every fitting CA so named was checked and its key does not verify the CRL, and - no fitting CA verifies it. The CRL names that key, and that key did not sign it. A fitting - CA with no Subject Key Identifier that could not be checked does not lift this to - ``uncheckable`` (vault BACKLOG #4159, the reading of #2905 item 1 decided 2026-10-26). The - reason: the verifying engine contexts set ``VERIFY_X509_STRICT``, by the decision's - reading of the call sites, and under it OpenSSL 3 refuses a chain of two or more - certificates through a CA with no Subject Key Identifier, so such a CA cannot make the CRL - valid for that chain. Measured 2026-10-27 on CPython 3.14.7 with OpenSSL 3.5.7. **At least two residuals, where the start refuses a CRL a - handshake could pass.** OpenSSL skips its strict checks on a one-certificate chain: a peer - that presents a self-signed CA with no Subject Key Identifier, listed here, passed a strict - handshake judged by a CRL that CA signed, measured the same day. And an OpenSSL build or a - context without the strict check could take such a CA for the issuer; the engine sets no - OpenSSL floor. A fitting CA with no Subject Key Identifier that verifies the CRL still - makes it ``verified``. - * ``uncheckable``: no fitting CA verifies, and some fitting CA could not be checked: one the - CRL names by key identifier, or, where the CRL names no fitting CA's key, any. "Cannot check" - is not "does not verify" (:func:`_verifies`). + Where the CRL's key identifier equals some fitting CA's Subject Key Identifier, the CRL + **names** that CA, and only the named CAs are judged; otherwise every fitting CA is. A + fitting CA with no Subject Key Identifier then counts for nothing, whether its key verifies + the CRL, does not, or cannot be checked (vault BACKLOG #4159, the reading of #2905 item 1 + decided 2026-10-26). Why, and the residuals where that refuses at start a CRL a handshake + could pass, are stated once, in + ``docs/CONFIGURATION.md``, in the paragraph that starts "Why a CA with no subject key + identifier counts for nothing there". + + * ``verified``: a CA that is judged verifies the CRL. + * ``bad``: **proven** badly signed. The CRL names a fitting CA, and every named CA was checked + and its key does not verify the CRL. The CRL names that key, and that key did not sign it. + * ``uncheckable``: no judged CA verifies, and some judged CA could not be checked: one the CRL + names, or, where it names none, any fitting CA. "Cannot check" is not "does not verify" + (:func:`_verifies`). * ``unproven``: every fitting CA was checked and none verifies, but the CRL names none of them by key identifier. A CRL with no key identifier fits every CA of that name. So a CA that renewed its key reads this way, while the peer's own chain may still carry the CA that @@ -869,31 +865,31 @@ def _signature_state( akid = _crl_extensions(crl).akid # once per CRL, not once per CA except _UNREADABLE_CA: return "unlisted" # its extensions do not read: no CA is shown to fit - fitting = False - named = False # the CRL names some fitting CA's key - unchecked = False - named_unchecked = False # a CA whose key the CRL names could not be checked + fitting: list[x509.Certificate] = [] + named: list[x509.Certificate] = [] # the fitting CAs whose key the CRL names for ca in named_cas: try: names_key = _akid_fit(akid, ca) except _UNREADABLE_CA: continue # the CA's extensions or its issuer name do not read: not shown to fit - if names_key is None: - continue - fitting = True - named = named or names_key - answer = _verifies(crl, ca.cert) + if names_key is not None: + fitting.append(ca.cert) + if names_key: + named.append(ca.cert) + if not fitting: + return "unlisted" + # Where the CRL names a key, _akid_fit drops a CA whose identifier differs, so a fitting CA it + # does not name carries none. Only the named CAs are tried then, whatever the other's key + # would say, and whatever the CA-file order (vault BACKLOG #4159); CrlSignature says why. + unchecked = False + for cert in named or fitting: + answer = _verifies(crl, cert) if answer: return "verified" unchecked = unchecked or answer is None - named_unchecked = named_unchecked or (answer is None and names_key) - if not fitting: - return "unlisted" - if named: - # Where the CRL names a key, _akid_fit drops a CA whose identifier differs, so a fitting - # CA it does not name carries none; CrlSignature says why that one cannot lift ``bad``. - return "uncheckable" if named_unchecked else "bad" - return "uncheckable" if unchecked else "unproven" + if unchecked: + return "uncheckable" + return "bad" if named else "unproven" #: Why a CRL reads :class:`CrlSignature` state ``unlisted``, for a start's warning and a reload's @@ -911,10 +907,10 @@ def crl_signature_refusal(pem: bytes, ca_ders: Iterable[bytes]) -> tuple[str, bo """Why a CA in ``ca_ders`` is not confirmed to have signed some CRL in ``pem``, or ``None`` when each CRL is confirmed. - The flag is True when a listed CA fits the CRL and none verifies it, whether the check failed - or could not run, and False when no listed CA fits. :class:`CrlSignature` says what "fits" - means. A block with the True flag is reported over one with the False flag, wherever it sits - in the file. OpenSSL checks a CRL's signature at the handshake, not at the load. So OpenSSL + The flag is True when a listed CA fits the CRL and no CA judged verifies it, whether the check + failed or could not run, and False when no listed CA fits. :class:`CrlSignature` says what + "fits" means and which CAs are judged. A block with the True flag is reported over one with + the False flag, wherever it sits in the file. OpenSSL checks a CRL's signature at the handshake, not at the load. So OpenSSL itself loads a CRL with the right issuer name and Authority Key Identifier but a bad signature, and then every handshake it judges fails with ``CRL signature failure``. The running-hop reload calls this before a live load, because a load cannot be undone @@ -983,12 +979,16 @@ def _verifies(crl: x509.CertificateRevocationList, ca: x509.Certificate) -> bool ``is_signature_valid`` answers False for a signature it cannot check as well as for one that fails, because it turns every error inside its own verification into False. So a False from it is not proof. The key is then asked directly (:func:`_verify_with`), and only its - ``InvalidSignature`` counts as proof. Everything else is ``None``: a key or an algorithm - ``cryptography`` does not support, a key of another kind than the signature, and a - ``ValueError`` or ``TypeError`` from either call. So is a signature the key accepts after - ``is_signature_valid`` refused it. Measured 2026-10-13 with ``cryptography`` 50.0.2: an RSA - PKCS1v15 signature over SHA-1 that the CA did make reads ``None`` that way, and a forged one - reads False.""" + ``InvalidSignature`` counts as proof. ``None`` is only what the key cannot be asked: a key or + an algorithm ``cryptography`` does not support, a key of another kind than the signature, and + a ``ValueError`` or ``TypeError`` from either call. + + **A signature the key accepts is True, even where ``is_signature_valid`` refused it** (vault + BACKLOG #4159). The key's own check passed, so the key made the signature, and a verdict that + followed the refusal would differ for one signer by digest alone. Measured 2026-10-27 + with ``cryptography`` 50.0.2 and OpenSSL 3.5.7: ``is_signature_valid`` refuses an RSA PKCS1v15 + signature over SHA-1 that the CA did make, and a handshake takes that CRL and judges the peer + by it. Before #4159 such a signature read ``None``. A forged one reads False.""" try: key = ca.public_key() if crl.is_signature_valid(key): # type: ignore[arg-type] @@ -998,7 +998,7 @@ def _verifies(crl: x509.CertificateRevocationList, ca: x509.Certificate) -> bool return False except (ValueError, TypeError, UnsupportedAlgorithm): return None - return None # the key accepts what ``is_signature_valid`` refused: no proof either way + return True # the key accepts what ``is_signature_valid`` refused, as for SHA-1 above _DSA_SIGNATURES = frozenset( diff --git a/tests/test_crl_held_copy_alert.py b/tests/test_crl_held_copy_alert.py index f5a7e7010a..75a40528dc 100644 --- a/tests/test_crl_held_copy_alert.py +++ b/tests/test_crl_held_copy_alert.py @@ -648,7 +648,7 @@ def test_a_badly_signed_crl_put_in_the_ca_file_is_not_answered_with_a_restart( detail = _bundle_detail(bundle) assert detail.startswith("Replace the CRL in the CA file with one its CA signed") - assert "a handshake OpenSSL judges by it fails" in detail + assert _PROVEN_BAD_CLAIM in detail assert "Restart the engine" not in detail assert hop.verify_flags & ssl.VERIFY_CRL_CHECK_LEAF @@ -687,6 +687,9 @@ def test_a_ca_file_crl_from_a_renewed_ca_key_is_unconfirmed_not_failing( #: The lead of the remedy for a CRL in a CA file whose signature is proven bad. _PROVEN_BAD = "Replace the CRL in the CA file with one its CA signed" +#: The claim the remedy for a proven bad CRL in a CA file makes about handshakes. +_PROVEN_BAD_CLAIM = "a handshake OpenSSL judges by it through the CA it names fails" + #: The whole line the scan logs, naming no file, when it could not check a CA file's CRL. _SCAN_FAILED_LINE = "cert_expiry: could not check the CRL in a CA file" @@ -720,7 +723,7 @@ def _states(pem: bytes, cas: object) -> list[CrlSignature]: detail = _bundle_detail(bundle) assert detail.startswith(lead), detail - assert ("a handshake OpenSSL judges by it fails" in detail) == ("bad" in states), detail + assert (_PROVEN_BAD_CLAIM in detail) == ("bad" in states), detail assert hop.verify_flags & ssl.VERIFY_CRL_CHECK_LEAF diff --git a/tests/test_crl_reload.py b/tests/test_crl_reload.py index af97d5de99..9a200dc056 100644 --- a/tests/test_crl_reload.py +++ b/tests/test_crl_reload.py @@ -206,7 +206,7 @@ def _ca_cert( def _leaf( - issuer: x509.Name, signer: ec.EllipticCurvePrivateKey + issuer: x509.Name, signer: CertificateIssuerPrivateKeyTypes ) -> tuple[x509.Certificate, ec.EllipticCurvePrivateKey]: """A ``localhost`` leaf for server and client auth, issued by ``issuer`` and signed by ``signer``, and its key.""" diff --git a/tests/test_crl_signature_at_start.py b/tests/test_crl_signature_at_start.py index c2686d3ec5..983ee21091 100644 --- a/tests/test_crl_signature_at_start.py +++ b/tests/test_crl_signature_at_start.py @@ -210,6 +210,16 @@ def _raw_hop(ca_file: Path, crl: Path) -> ssl.SSLContext: return ctx +def _server(tmp_path: Path, chain: bytes, key: ec.EllipticCurvePrivateKey) -> ssl.SSLContext: + """A server context that presents the PEM certificates ``chain``, the first of them its own + and ``key``'s.""" + (tmp_path / "server.pem").write_bytes(chain) + (tmp_path / "server-key.pem").write_bytes(_key_pem(key)) + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + ctx.load_cert_chain(tmp_path / "server.pem", tmp_path / "server-key.pem") + return ctx + + _WARNING = "could not confirm at start" @@ -435,9 +445,10 @@ def _ca_without_ski(key: _CaKey, *, localhost: bool = False) -> x509.Certificate @functools.cache -def _rsa_key() -> rsa.RSAPrivateKey: - """One RSA key for every test that needs only a key the engine cannot check an ECDSA - signature with. Generated once: a 2048-bit key takes a noticeable fraction of a second.""" +def _rsa_key(which: int = 0) -> rsa.RSAPrivateKey: + """RSA key number ``which``, shared by every test that asks for it: a key the engine cannot + check an ECDSA signature with, or a key to sign over SHA-1 with. Generated once each: a + 2048-bit key takes a noticeable fraction of a second.""" return rsa.generate_private_key(public_exponent=65537, key_size=2048) @@ -511,6 +522,48 @@ def test_a_crl_its_named_ca_proves_bad_refuses_beside_an_unchecked_ca_with_no_sk _handshake(_raw_hop(mixed, twins.crl), twins.server(sends_renewed=False)) +@pytest.mark.parametrize("no_ski_first", [False, True], ids=["named-ca-first", "no-ski-ca-first"]) +def test_a_crl_its_named_ca_proves_bad_refuses_beside_a_ca_with_no_ski_that_verifies_it( + twins: _Twins, tmp_path: Path, no_ski_first: bool +) -> None: + """Finding 1 of the Lander's covering review on engine PR 2585. The CRL names the new CA's key + and the stranger signed it. Beside the new CA sits a CA of the same name with no Subject Key + Identifier and the stranger's key, so its key DOES verify the CRL. That read ``verified``, and + the start loaded the file with no warning, while a handshake through the named CA failed. Such + a CA counts for nothing now, in either CA-file order (pki.CrlSignature).""" + no_ski = _ca_without_ski(twins.stranger) + named = twins.new_ca.read_bytes() + mixed = tmp_path / "mixed.pem" + mixed.write_bytes( + no_ski.public_bytes(_PEM) + named if no_ski_first else named + no_ski.public_bytes(_PEM) + ) + pem = twins.write_crl("stranger", "new") + + # The case's premises: that CA fits without being named, and its key verifies the CRL. + assert _names_key(pem, no_ski) is False + assert pki_module._verifies(x509.load_pem_x509_crl(pem), no_ski) is True + + assert [s.state for s in judge_crl_signatures(pem, _ders(mixed))] == ["bad"] + with pytest.raises(ValueError, match="does not verify the CRL's signature"): + _hop(mixed, twins.crl) + # The running-hop reload holds it back now too: it loaded it live before, as ``verified``. + verdict = crl_signature_refusal(pem, _ders(mixed)) + assert verdict is not None and verdict[1] is True + + # Control: OpenSSL fails a strict handshake through the named CA. Listed first, the CA with no + # SKI lets the handshake reach the CRL, and it fails there; a CRL the named CA signed passes. + # Listed after the named CA, a CA with no SKI and a key of the same kind fails the chain before + # the CRL is reached, whatever the CRL (open question 4 in the PR body). + raw = _raw_hop(mixed, twins.crl) + assert raw.verify_flags & ssl.VERIFY_X509_STRICT + expected = "CRL signature failure" if no_ski_first else "Missing Subject Key Identifier" + with pytest.raises(ssl.SSLError, match=expected): + _handshake(raw, twins.server(sends_renewed=False)) + if no_ski_first: + twins.write_crl("new", "new") + _handshake(_raw_hop(mixed, twins.crl), twins.server(sends_renewed=False)) + + def test_a_strict_engine_context_refuses_a_chain_through_a_ca_with_no_ski(tmp_path: Path) -> None: """(a) The premise the mixed-case refusal rests on: an engine verifying context sets ``VERIFY_X509_STRICT``, and under it OpenSSL refuses a chain of two or more certificates @@ -519,10 +572,7 @@ def test_a_strict_engine_context_refuses_a_chain_through_a_ca_with_no_ski(tmp_pa key = ec.generate_private_key(ec.SECP256R1()) (tmp_path / "ca.pem").write_bytes(_ca_without_ski(key).public_bytes(_PEM)) leaf, leaf_key = _leaf(_NAME, key) - (tmp_path / "leaf.pem").write_bytes(leaf.public_bytes(_PEM)) - (tmp_path / "leaf-key.pem").write_bytes(_key_pem(leaf_key)) - server = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) - server.load_cert_chain(tmp_path / "leaf.pem", tmp_path / "leaf-key.pem") + server = _server(tmp_path, leaf.public_bytes(_PEM), leaf_key) client = build_verifying_client_context( TrustAnchor(cafile=str(tmp_path / "ca.pem"), load_system_roots=False) ) @@ -538,31 +588,47 @@ def test_a_strict_engine_context_refuses_a_chain_through_a_ca_with_no_ski(tmp_pa _handshake(client, server) # raises if refused -def test_a_one_certificate_chain_through_a_ca_with_no_ski_passes_a_strict_handshake( - tmp_path: Path, +@pytest.mark.parametrize("sends_more", [False, True], ids=["sends-itself", "sends-another-too"]) +def test_the_one_certificate_residual_a_start_refuses_a_crl_a_strict_handshake_passes( + twins: _Twins, tmp_path: Path, sends_more: bool ) -> None: - """The residual pki.CrlSignature records: OpenSSL skips its strict checks on a chain of one - certificate. A peer that presents a listed self-signed CA with no Subject Key Identifier as - its own certificate passes a strict engine handshake judged by a CRL that CA signed, so that - CA can make a CRL valid for that chain. If this goes red, the residual has moved: correct - the texts that name it (the CrlSignature docstring, CONFIGURATION.md, the changelog).""" - key = ec.generate_private_key(ec.SECP256R1()) - ca = _ca_without_ski(key, localhost=True) - (tmp_path / "ca.pem").write_bytes(ca.public_bytes(_PEM)) - (tmp_path / "ca-key.pem").write_bytes(_key_pem(key)) - (tmp_path / "crl.pem").write_bytes(_crl(key, names=key)) - server = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) - server.load_cert_chain(tmp_path / "ca.pem", tmp_path / "ca-key.pem") + """The first residual ``docs/CONFIGURATION.md`` states for the rule above, set up whole. The + hop lists the new CA, which the CRL names, and beside it a self-signed CA of the same name + with no Subject Key Identifier and the stranger's key, which signed the CRL. The start + refuses the file. A peer whose own certificate is that CA passes a strict handshake judged by + the CRL: OpenSSL skips its strict checks on a verified chain of one certificate, and that is + the chain it builds, not the certificates the peer sends. If this goes red, the residual has + moved: correct that paragraph.""" + no_ski = _ca_without_ski(twins.stranger, localhost=True) + listed = tmp_path / "listed.pem" + listed.write_bytes(twins.new_ca.read_bytes() + no_ski.public_bytes(_PEM)) + sent = no_ski.public_bytes(_PEM) + (twins.new_ca.read_bytes() if sends_more else b"") + server = _server(tmp_path, sent, twins.stranger) + twins.write_crl("stranger", "new") - client = _hop(tmp_path / "ca.pem", tmp_path / "crl.pem") - assert client.verify_flags & ssl.VERIFY_X509_STRICT - assert client.verify_flags & ssl.VERIFY_CRL_CHECK_LEAF - _handshake(client, server) # raises if refused + # The engine's half: the state is ``bad``, and the start refuses the file. + assert [s.state for s in judge_crl_signatures(twins.crl.read_bytes(), _ders(listed))] == ["bad"] + with pytest.raises(ValueError, match="does not verify the CRL's signature"): + _hop(listed, twins.crl) + + def strict_hop() -> ssl.SSLContext: + """The engine's outbound context without the CRL setting, with the CRL loaded and leaf + revocation checking on, as the setting would have done had the start not refused it.""" + ctx = build_verifying_client_context( + TrustAnchor(cafile=str(listed), load_system_roots=False) + ) + ctx.load_verify_locations(cafile=str(twins.crl)) + ctx.verify_flags |= ssl.VERIFY_CRL_CHECK_LEAF + return ctx + + # OpenSSL's half: the same CRL passes the handshake. + assert strict_hop().verify_flags & ssl.VERIFY_X509_STRICT + _handshake(strict_hop(), server) # raises if refused # Control: the CRL is live for this chain. Revoking the peer's own serial refuses it. - (tmp_path / "crl.pem").write_bytes(_crl(key, names=key, revoke=ca.serial_number)) + twins.crl.write_bytes(_crl(twins.stranger, names=twins.new_key, revoke=no_ski.serial_number)) with pytest.raises(ssl.SSLError, match="revoked"): - _handshake(_hop(tmp_path / "ca.pem", tmp_path / "crl.pem"), server) + _handshake(strict_hop(), server) def test_a_named_ca_that_cannot_be_checked_still_warns_beside_a_ca_with_no_ski( @@ -923,15 +989,16 @@ def _resigned( return x509.load_der_x509_crl(der[: -len(signature)] + signature).public_bytes(_PEM) -def test_an_rsa_sha1_crl_the_ca_signed_is_cannot_check_and_a_forged_one_is_bad( - tmp_path: Path, +def test_an_rsa_sha1_crl_the_ca_signed_is_verified_and_a_forged_one_is_bad( + tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: - """A CRL that names the CA's key and that the reload holds back, while a start loads it. The - reload's remedy used to say a start refuses such a file. The SHA-384 CRL is the control for - the hand-made encoding: the same swap under a digest the library accepts reads ``verified``. - So ``uncheckable`` is about SHA-1, not about the swap.""" - key = rsa.generate_private_key(public_exponent=65537, key_size=2048) - other = rsa.generate_private_key(public_exponent=65537, key_size=2048) + """A CRL that names the CA's key, signed over SHA-1. ``cryptography``'s own check refuses it + and the CA's key accepts it, so since vault BACKLOG #4159 it is ``verified``. It read + ``uncheckable`` before: a start warned, and the reload held it back. The premise is OpenSSL's: + it takes that CRL and judges the peer by it. The SHA-384 CRL is the control for the hand-made + encoding: the same swap under a digest the library accepts reads ``verified`` too.""" + key = _rsa_key() + other = _rsa_key(1) ca = _ca(key, key) ders = [ca.public_bytes(serialization.Encoding.DER)] @@ -939,21 +1006,84 @@ def test_an_rsa_sha1_crl_the_ca_signed_is_cannot_check_and_a_forged_one_is_bad( forged = _resigned(_crl(other, names=key), other, _SHA1_WITH_RSA, hashes.SHA1()) control = _resigned(_crl(key, names=key), key, _SHA384_WITH_RSA, hashes.SHA384()) + # The premise on the library's side: its own check refuses the CRL the CA made. + assert x509.load_pem_x509_crl(good).is_signature_valid(key.public_key()) is False assert [s.state for s in judge_crl_signatures(control, ders)] == ["verified"] - assert [s.state for s in judge_crl_signatures(good, ders)] == ["uncheckable"] + assert [s.state for s in judge_crl_signatures(good, ders)] == ["verified"] assert [s.state for s in judge_crl_signatures(forged, ders)] == ["bad"] - verdict = crl_signature_refusal(good, ders) - assert verdict is not None and verdict[1] is True # the reload's BAD_SIGNATURE case + assert crl_signature_refusal(good, ders) is None # the reload applies it ca_file, crl_file = tmp_path / "ca.pem", tmp_path / "crl.pem" ca_file.write_bytes(ca.public_bytes(_PEM)) crl_file.write_bytes(good) - assert _hop(ca_file, crl_file).verify_flags & ssl.VERIFY_CRL_CHECK_LEAF + with caplog.at_level("WARNING"): + hop = _hop(ca_file, crl_file) + assert _warnings(caplog) == [] + + # The premise on OpenSSL's side: it judges a peer under this CA by the SHA-1 CRL. Control: + # the same CRL with the peer's serial listed refuses the handshake. + leaf, leaf_key = _leaf(_NAME, key) + server = _server(tmp_path, leaf.public_bytes(_PEM), leaf_key) + _handshake(hop, server) # raises if refused + crl_file.write_bytes( + _resigned( + _crl(key, names=key, revoke=leaf.serial_number), key, _SHA1_WITH_RSA, hashes.SHA1() + ) + ) + with pytest.raises(ssl.SSLError, match="revoked"): + _handshake(_hop(ca_file, crl_file), server) + crl_file.write_bytes(forged) with pytest.raises(ValueError, match="does not verify the CRL's signature"): _hop(ca_file, crl_file) +#: The digests :func:`test_one_signer_gets_one_verdict_under_sha256_and_sha1` signs with. +_SHA256_AND_SHA1 = [ + pytest.param(_SHA256_WITH_RSA, hashes.SHA256(), id="sha256"), + pytest.param(_SHA1_WITH_RSA, hashes.SHA1(), id="sha1"), +] + +#: (the CAs the hop lists, in order, the CA whose key signs, the CA whose key the CRL names, the +#: verdict). "named" carries a Subject Key Identifier, and "no-ski" carries none. +_ONE_SIGNER = [ + pytest.param(("named",), "named", "named", "verified", id="the-named-ca-signs"), + pytest.param(("named",), "named", None, "verified", id="the-crl-names-no-key"), + pytest.param(("no-ski",), "no-ski", "no-ski", "verified", id="a-lone-ca-with-no-ski-signs"), + pytest.param( + ("named", "no-ski"), "no-ski", "named", "bad", id="no-ski-ca-signs-after-the-named-ca" + ), + pytest.param( + ("no-ski", "named"), "no-ski", "named", "bad", id="no-ski-ca-signs-before-the-named-ca" + ), +] + + +@pytest.mark.parametrize(("oid", "digest"), _SHA256_AND_SHA1) +@pytest.mark.parametrize(("listed", "signer", "names", "state"), _ONE_SIGNER) +def test_one_signer_gets_one_verdict_under_sha256_and_sha1( + listed: tuple[str, ...], + signer: str, + names: str | None, + state: str, + oid: bytes, + digest: hashes.HashAlgorithm, +) -> None: + """Finding 2 of the Lander's covering review on engine PR 2585. A ``None`` from + :func:`messagefoundry.pki._verifies` meant either a key the engine cannot check or a signature + the key accepts that ``cryptography``'s own check refused, and SHA-1 is refused there. So one + signer's verdict could differ by digest alone: a CA with no SKI signing beside the named CA + read ``verified`` over SHA-256 and ``bad`` over SHA-1, and the named CA signing read + ``verified`` and ``uncheckable``. Each row now gives one verdict for both digests.""" + keys = {"named": _rsa_key(), "no-ski": _rsa_key(1)} + cas = {"named": _ca(keys["named"], keys["named"]), "no-ski": _ca_without_ski(keys["no-ski"])} + shape = _crl(keys[signer], names=None if names is None else keys[names]) + pem = _resigned(shape, keys[signer], oid, digest) + ders = [cas[who].public_bytes(serialization.Encoding.DER) for who in listed] + + assert [s.state for s in judge_crl_signatures(pem, ders)] == [state] + + def test_a_version_that_logs_nothing_in_between_does_not_reset_the_record( twins: _Twins, caplog: pytest.LogCaptureFixture ) -> None: From 1c4f026e1f375e9876527dd821ba14ec7eea01d7 Mon Sep 17 00:00:00 2001 From: Scott Hall Date: Tue, 27 Oct 2026 21:03:48 -0500 Subject: [PATCH 5/9] CRL signature check: repairs from the Builder's review round 1 (round-b36c54e35d) - The verifying no-SKI test asserts the CA-file monitor's remedy, and its handshake control no longer assumes a same-name CA sort order. - The one-certificate residual test runs both CA-file orders; the lone no-SKI CA's engine-path handshake test is back. - _rsa_key takes its number positionally, with no default. - The reload's refusal text says which CA's key it checked. - CONFIGURATION.md: the warn bullet points at the "Why" paragraph, hedges its causes, and states that verified means the CA's key made the signature, not that every TLS library or policy takes it. - The changelog no longer claims the verdict is digest-independent. vault BACKLOG #4159 --- changelog.d/4159.security.md | 14 +++--- docs/CONFIGURATION.md | 21 +++++--- messagefoundry/pki.py | 13 ++++- tests/test_crl_signature_at_start.py | 75 +++++++++++++++++++++------- 4 files changed, 88 insertions(+), 35 deletions(-) diff --git a/changelog.d/4159.security.md b/changelog.d/4159.security.md index df7681736c..6d36e0ad9a 100644 --- a/changelog.d/4159.security.md +++ b/changelog.d/4159.security.md @@ -6,9 +6,11 @@ holds such a file back too. A CA the CRL names that cannot be checked still warns, and so does a lone CA with no subject key identifier that cannot be checked. A signature the CA's key accepts is now verified even where the `cryptography` library's own check refuses it, as for an RSA - signature over SHA-1 the CA made, so the verdict no longer depends on the digest: a start loads - such a CRL with no warning, and the reload applies it where it held it back before. The premise, - that the engine's verifying contexts use strict verification, and the cases where the start can - refuse a CRL a handshake could pass are stated in `docs/CONFIGURATION.md`, in the paragraph - that starts "Why a CA with no subject key identifier counts for nothing there". (vault BACKLOG - #4159) + signature over SHA-1 the CA made, so that refusal no longer decides the verdict: a start loads + such a CRL with no warning, and the reload applies it where it held it back before. A digest the + engine's own check cannot use still reads as one it cannot check, and a TLS library or system + policy that refuses SHA-1 can still fail the handshake on such a CRL (`docs/CONFIGURATION.md`, + the bullet on a signature the engine cannot check). The premise, that the engine's verifying + contexts use strict verification, and the cases where the start can refuse a CRL a handshake + could pass are stated in `docs/CONFIGURATION.md`, in the paragraph that starts "Why a CA with no + subject key identifier counts for nothing there". (vault BACKLOG #4159) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 648ff0844a..276ef033a6 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2054,14 +2054,19 @@ three cases warn: - A listed CA has the CRL's issuer name and does not verify it, but the CRL names no key. A CA that renewed its key can read this way while handshakes still pass. That was measured for one chain shape, where the peer sends the renewed CA certified by the old key. -- The engine cannot check the signature with a fitting listed CA's key, because it does not support - that algorithm or that pairing of key and signature. Where the CRL names a listed CA's key, only - that CA counts here (the refusal above says why). Where it names no listed CA's key, as when it - names a key no listed CA carries, every fitting CA counts, one with no subject key identifier - included. A measured example is an ECDSA signature on a CRL that names an RSA key. A signature - the CA's key accepts is verified even where the `cryptography` library's own check refuses it, - as for an RSA signature over SHA-1 that the CA did make; OpenSSL 3.5.7 was measured to judge a - peer by such a CRL (vault BACKLOG #4159). +- The engine cannot check the signature with a fitting listed CA's key, at least where it does + not support that algorithm or that pairing of key and signature, or where reading the key or + checking with it raises an error. Where the CRL names a listed CA's key, only that CA counts + here (the paragraph that starts "Why a CA with no subject key identifier" says why). Where it + names no listed CA's key, as when it names a key no listed CA carries, every fitting CA counts, + one with no subject key identifier included. A measured example is an ECDSA signature on a CRL + that names an RSA key. A signature the CA's key accepts is verified even where the + `cryptography` library's own check refuses it, as for an RSA signature over SHA-1 that the CA + did make; OpenSSL 3.5.7 was measured to judge a peer by such a CRL (vault BACKLOG #4159). + Verified means the CA's key made the signature, not that every TLS library takes it: the + engine checks with the `cryptography` library's OpenSSL, the handshake runs on Python's, and a + build or system policy that refuses the digest, such as SHA-1, fails the handshake on a CRL + that started with no warning and that a reload applies. If the check itself fails, the engine logs an ERROR that says so. It loads the file unless a CRL it judged before the failure is proved badly signed, which refuses the file. Where it loads, the diff --git a/messagefoundry/pki.py b/messagefoundry/pki.py index 8e3aa2b22b..b0aaaa45c7 100644 --- a/messagefoundry/pki.py +++ b/messagefoundry/pki.py @@ -931,7 +931,9 @@ def crl_signature_refusal(pem: bytes, ca_ders: Iterable[bytes]) -> tuple[str, bo elif signature.state != "verified": return ( f"{signature.where}: the engine could not confirm its signature with the key " - "of the CA certificate this hop trusts for that issuer. The check failed or " + "of the CA certificate this hop trusts for that issuer: the one the CRL names by " + "key identifier, where it names a listed CA's key, and a CA of that name with no " + "subject key identifier does not count then. The check failed or " "could not run. Where that key did not sign the CRL, a handshake that OpenSSL " "checks against that CA's key by this CRL fails with 'CRL signature failure'", True, @@ -988,7 +990,14 @@ def _verifies(crl: x509.CertificateRevocationList, ca: x509.Certificate) -> bool followed the refusal would differ for one signer by digest alone. Measured 2026-10-27 with ``cryptography`` 50.0.2 and OpenSSL 3.5.7: ``is_signature_valid`` refuses an RSA PKCS1v15 signature over SHA-1 that the CA did make, and a handshake takes that CRL and judges the peer - by it. Before #4159 such a signature read ``None``. A forged one reads False.""" + by it. Before #4159 such a signature read ``None``. A forged one reads False. + + **True says the CA's key made the signature, not that every TLS library takes it.** The key + is asked through ``cryptography``'s own OpenSSL, and the handshake runs on the ``ssl`` + module's, which can be another build with another policy. A policy that refuses the digest, + such as a system crypto policy that refuses SHA-1 signatures, fails the handshake on a CRL + read ``verified`` here. That holds for any verified signature, not only one + ``is_signature_valid`` refused; ``docs/CONFIGURATION.md`` states it beside the warn cases.""" try: key = ca.public_key() if crl.is_signature_valid(key): # type: ignore[arg-type] diff --git a/tests/test_crl_signature_at_start.py b/tests/test_crl_signature_at_start.py index 983ee21091..7c4331f60e 100644 --- a/tests/test_crl_signature_at_start.py +++ b/tests/test_crl_signature_at_start.py @@ -445,10 +445,11 @@ def _ca_without_ski(key: _CaKey, *, localhost: bool = False) -> x509.Certificate @functools.cache -def _rsa_key(which: int = 0) -> rsa.RSAPrivateKey: +def _rsa_key(which: int, /) -> rsa.RSAPrivateKey: """RSA key number ``which``, shared by every test that asks for it: a key the engine cannot check an ECDSA signature with, or a key to sign over SHA-1 with. Generated once each: a - 2048-bit key takes a noticeable fraction of a second.""" + 2048-bit key takes a noticeable fraction of a second. No default, so one number names one + key however it is written.""" return rsa.generate_private_key(public_exponent=65537, key_size=2048) @@ -483,7 +484,7 @@ def test_a_crl_its_named_ca_proves_bad_refuses_beside_an_unchecked_ca_with_no_sk not to verify the CRL. Beside it sits a CA of the same name with no Subject Key Identifier and an RSA key, which fits the CRL and cannot check its ECDSA signature. Before vault BACKLOG #4159 that read ``uncheckable`` and the start loaded the file with a WARNING.""" - no_ski = _ca_without_ski(_rsa_key()) + no_ski = _ca_without_ski(_rsa_key(0)) named = twins.new_ca.read_bytes() mixed = tmp_path / "mixed.pem" mixed.write_bytes( @@ -549,19 +550,28 @@ def test_a_crl_its_named_ca_proves_bad_refuses_beside_a_ca_with_no_ski_that_veri # The running-hop reload holds it back now too: it loaded it live before, as ``verified``. verdict = crl_signature_refusal(pem, _ders(mixed)) assert verdict is not None and verdict[1] is True + # The CA-file monitor gives the proven remedy, where it raised no alert before. + verdict_of_scan = cert_expiry._ca_file_crl_refusal( + mixed.read_bytes() + pem, + time.time(), + None, + ca_file_only=True, + on_unchecked=lambda: None, + ) + assert verdict_of_scan is not None + assert verdict_of_scan[1].startswith("Replace the CRL in the CA file with one its CA signed") - # Control: OpenSSL fails a strict handshake through the named CA. Listed first, the CA with no - # SKI lets the handshake reach the CRL, and it fails there; a CRL the named CA signed passes. - # Listed after the named CA, a CA with no SKI and a key of the same kind fails the chain before - # the CRL is reached, whatever the CRL (open question 4 in the PR body). + # Control: OpenSSL fails a strict handshake through the named CA. Which error depends on the + # order OpenSSL sorts same-name CAs in, which differs by C runtime, not on the file order. + # Where it tries the named CA first, the handshake reaches this CRL and fails on it. Where it + # tries the CA with no SKI first, whose key is of the leaf's kind, the strict check fails the + # chain on the missing SKI before the CRL, whatever the CRL (open question 4 in the PR body). + # The CRL's own failure, in both orders, is the unchecked-CA test's control above; a CA with + # no SKI whose key could also verify this CRL must be of the CRL's kind, and so of the leaf's. raw = _raw_hop(mixed, twins.crl) assert raw.verify_flags & ssl.VERIFY_X509_STRICT - expected = "CRL signature failure" if no_ski_first else "Missing Subject Key Identifier" - with pytest.raises(ssl.SSLError, match=expected): + with pytest.raises(ssl.SSLError, match="CRL signature failure|Missing Subject Key Identifier"): _handshake(raw, twins.server(sends_renewed=False)) - if no_ski_first: - twins.write_crl("new", "new") - _handshake(_raw_hop(mixed, twins.crl), twins.server(sends_renewed=False)) def test_a_strict_engine_context_refuses_a_chain_through_a_ca_with_no_ski(tmp_path: Path) -> None: @@ -588,9 +598,10 @@ def test_a_strict_engine_context_refuses_a_chain_through_a_ca_with_no_ski(tmp_pa _handshake(client, server) # raises if refused +@pytest.mark.parametrize("no_ski_first", [False, True], ids=["named-ca-first", "no-ski-ca-first"]) @pytest.mark.parametrize("sends_more", [False, True], ids=["sends-itself", "sends-another-too"]) def test_the_one_certificate_residual_a_start_refuses_a_crl_a_strict_handshake_passes( - twins: _Twins, tmp_path: Path, sends_more: bool + twins: _Twins, tmp_path: Path, sends_more: bool, no_ski_first: bool ) -> None: """The first residual ``docs/CONFIGURATION.md`` states for the rule above, set up whole. The hop lists the new CA, which the CRL names, and beside it a self-signed CA of the same name @@ -598,10 +609,12 @@ def test_the_one_certificate_residual_a_start_refuses_a_crl_a_strict_handshake_p refuses the file. A peer whose own certificate is that CA passes a strict handshake judged by the CRL: OpenSSL skips its strict checks on a verified chain of one certificate, and that is the chain it builds, not the certificates the peer sends. If this goes red, the residual has - moved: correct that paragraph.""" + moved: correct that paragraph. Both CA-file orders run, because OpenSSL sorts same-name CAs + in an order that differs by C runtime, and the two file orders cover both.""" no_ski = _ca_without_ski(twins.stranger, localhost=True) listed = tmp_path / "listed.pem" - listed.write_bytes(twins.new_ca.read_bytes() + no_ski.public_bytes(_PEM)) + pair = [twins.new_ca.read_bytes(), no_ski.public_bytes(_PEM)] + listed.write_bytes(b"".join(reversed(pair) if no_ski_first else pair)) sent = no_ski.public_bytes(_PEM) + (twins.new_ca.read_bytes() if sends_more else b"") server = _server(tmp_path, sent, twins.stranger) twins.write_crl("stranger", "new") @@ -631,6 +644,30 @@ def strict_hop() -> ssl.SSLContext: _handshake(strict_hop(), server) +def test_a_lone_ca_with_no_ski_judges_its_own_one_certificate_chain_through_the_crl_setting( + tmp_path: Path, +) -> None: + """The lone-CA shape of the case above, on the engine's own path: no other CA of that name is + listed, so the CRL names no listed CA's key, the CA with no SKI is judged, it verifies the CRL, + and the start loads the file through the CRL setting. A peer that presents that CA as its own + certificate passes the engine's strict handshake, and the CRL is live for it.""" + key = ec.generate_private_key(ec.SECP256R1()) + ca = _ca_without_ski(key, localhost=True) + (tmp_path / "ca.pem").write_bytes(ca.public_bytes(_PEM)) + (tmp_path / "crl.pem").write_bytes(_crl(key, names=key)) + server = _server(tmp_path, ca.public_bytes(_PEM), key) + + client = _hop(tmp_path / "ca.pem", tmp_path / "crl.pem") + assert client.verify_flags & ssl.VERIFY_X509_STRICT + assert client.verify_flags & ssl.VERIFY_CRL_CHECK_LEAF + _handshake(client, server) # raises if refused + + # Control: the CRL is live for this chain. Revoking the peer's own serial refuses it. + (tmp_path / "crl.pem").write_bytes(_crl(key, names=key, revoke=ca.serial_number)) + with pytest.raises(ssl.SSLError, match="revoked"): + _handshake(_hop(tmp_path / "ca.pem", tmp_path / "crl.pem"), server) + + def test_a_named_ca_that_cannot_be_checked_still_warns_beside_a_ca_with_no_ski( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: @@ -638,7 +675,7 @@ def test_a_named_ca_that_cannot_be_checked_still_warns_beside_a_ca_with_no_ski( signature, so that CA cannot be checked. A CA of the same name with no Subject Key Identifier and an EC key sits beside it, and its key is proven not to verify the CRL. The named CA could still have signed it, so the state stays ``uncheckable`` and the start warns.""" - rsa_key = _rsa_key() + rsa_key = _rsa_key(0) named = _ca(rsa_key, rsa_key) no_ski = _ca_without_ski(ec.generate_private_key(ec.SECP256R1())) (tmp_path / "ca.pem").write_bytes(named.public_bytes(_PEM) + no_ski.public_bytes(_PEM)) @@ -658,7 +695,7 @@ def test_a_lone_ca_with_no_ski_that_cannot_be_checked_still_warns( """(c) The only CA of that name carries no Subject Key Identifier and has an RSA key. The CRL names a key by identifier and carries an ECDSA signature, so that CA fits, is not named, and cannot be checked. No CA is proven bad, so the start warns.""" - no_ski = _ca_without_ski(_rsa_key()) + no_ski = _ca_without_ski(_rsa_key(0)) (tmp_path / "ca.pem").write_bytes(no_ski.public_bytes(_PEM)) signer = ec.generate_private_key(ec.SECP256R1()) pem = _crl(signer, names=signer) @@ -997,7 +1034,7 @@ def test_an_rsa_sha1_crl_the_ca_signed_is_verified_and_a_forged_one_is_bad( ``uncheckable`` before: a start warned, and the reload held it back. The premise is OpenSSL's: it takes that CRL and judges the peer by it. The SHA-384 CRL is the control for the hand-made encoding: the same swap under a digest the library accepts reads ``verified`` too.""" - key = _rsa_key() + key = _rsa_key(0) other = _rsa_key(1) ca = _ca(key, key) ders = [ca.public_bytes(serialization.Encoding.DER)] @@ -1075,7 +1112,7 @@ def test_one_signer_gets_one_verdict_under_sha256_and_sha1( signer's verdict could differ by digest alone: a CA with no SKI signing beside the named CA read ``verified`` over SHA-256 and ``bad`` over SHA-1, and the named CA signing read ``verified`` and ``uncheckable``. Each row now gives one verdict for both digests.""" - keys = {"named": _rsa_key(), "no-ski": _rsa_key(1)} + keys = {"named": _rsa_key(0), "no-ski": _rsa_key(1)} cas = {"named": _ca(keys["named"], keys["named"]), "no-ski": _ca_without_ski(keys["no-ski"])} shape = _crl(keys[signer], names=None if names is None else keys[names]) pem = _resigned(shape, keys[signer], oid, digest) From 79ea366edbb11c8138f4c068008038af5afef9fe Mon Sep 17 00:00:00 2001 From: Scott Hall Date: Tue, 27 Oct 2026 21:28:14 -0500 Subject: [PATCH 6/9] CRL signature check: repairs from the Builder's review round 2 (round-4da251fc03) - The verifying no-SKI test's handshake control now discriminates: in whichever same-name sort order this runtime takes, a CRL the named CA signed either passes (so the bad CRL was the fault) or fails on the same missing SKI (so the CRL was never consulted). - The uncheckable warning and CONFIGURATION.md say "a listed CA that fits it and whose key the CRL names", matching the code. - _verifies links to the residual instead of restating it. - The same-signer test states main's verdicts. - The BAD_SIGNATURE remedy reads "with the key of some listed CA". - A docstring line reflowed. vault BACKLOG #4159 --- docs/CONFIGURATION.md | 8 ++++---- messagefoundry/config/tls_policy.py | 6 +++--- messagefoundry/pipeline/crl_reload.py | 2 +- messagefoundry/pki.py | 13 +++++-------- tests/test_crl_signature_at_start.py | 21 +++++++++++++++++---- 5 files changed, 30 insertions(+), 20 deletions(-) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 276ef033a6..04fcfe43b5 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2056,10 +2056,10 @@ three cases warn: shape, where the peer sends the renewed CA certified by the old key. - The engine cannot check the signature with a fitting listed CA's key, at least where it does not support that algorithm or that pairing of key and signature, or where reading the key or - checking with it raises an error. Where the CRL names a listed CA's key, only that CA counts - here (the paragraph that starts "Why a CA with no subject key identifier" says why). Where it - names no listed CA's key, as when it names a key no listed CA carries, every fitting CA counts, - one with no subject key identifier included. A measured example is an ECDSA signature on a CRL + checking with it raises an error. Where the CRL names the key of a listed CA that fits it, only + such a CA counts here (the paragraph that starts "Why a CA with no subject key identifier" says + why). Where it names no such CA's key, as when it names a key no listed CA carries, every + fitting CA counts, one with no subject key identifier included. A measured example is an ECDSA signature on a CRL that names an RSA key. A signature the CA's key accepts is verified even where the `cryptography` library's own check refuses it, as for an RSA signature over SHA-1 that the CA did make; OpenSSL 3.5.7 was measured to judge a peer by such a CRL (vault BACKLOG #4159). diff --git a/messagefoundry/config/tls_policy.py b/messagefoundry/config/tls_policy.py index fa749a75d2..1865a26b91 100644 --- a/messagefoundry/config/tls_policy.py +++ b/messagefoundry/config/tls_policy.py @@ -571,9 +571,9 @@ def _refuse_crl_count(loaded: int, blocks: Sequence[CrlBlock], *, label: str) -> "name that CA's key, so a renewed CA key could explain it" ), "uncheckable": ( - "the engine cannot check its signature with the key of the listed CA the CRL names by key " - "identifier or, where the CRL names no listed CA's key, with the key of some listed CA " - "that fits it" + "the engine cannot check its signature with the key of a listed CA that fits it and whose " + "key the CRL names by key identifier or, where it names no such CA's key, with the key of " + "some listed CA that fits it" ), } #: How many CRLs that warning describes before it only counts the rest. diff --git a/messagefoundry/pipeline/crl_reload.py b/messagefoundry/pipeline/crl_reload.py index da1e317324..2b8fa1733b 100644 --- a/messagefoundry/pipeline/crl_reload.py +++ b/messagefoundry/pipeline/crl_reload.py @@ -201,7 +201,7 @@ "signature does not stop a start: unless another rule refuses the file, a start loads it " "with a warning, and each handshake then decides. That covers at least a CRL that names no " "key and a signature the engine cannot check with the key the CRL names, or, where it names " - "no listed CA's key, with some listed CA that fits it. The handshake passes where the peer's " + "no listed CA's key, with the key of some listed CA that fits it. The handshake passes where the peer's " "chain carries the CA that signed the CRL, as after a CA renews its key, and fails where it " "does not" ) diff --git a/messagefoundry/pki.py b/messagefoundry/pki.py index b0aaaa45c7..0dc77d2a5d 100644 --- a/messagefoundry/pki.py +++ b/messagefoundry/pki.py @@ -910,8 +910,8 @@ def crl_signature_refusal(pem: bytes, ca_ders: Iterable[bytes]) -> tuple[str, bo The flag is True when a listed CA fits the CRL and no CA judged verifies it, whether the check failed or could not run, and False when no listed CA fits. :class:`CrlSignature` says what "fits" means and which CAs are judged. A block with the True flag is reported over one with - the False flag, wherever it sits in the file. OpenSSL checks a CRL's signature at the handshake, not at the load. So OpenSSL - itself loads a CRL with the right issuer name and Authority Key Identifier but a bad + the False flag, wherever it sits in the file. OpenSSL checks a CRL's signature at the + handshake, not at the load. So OpenSSL itself loads a CRL with the right issuer name and Authority Key Identifier but a bad signature, and then every handshake it judges fails with ``CRL signature failure``. The running-hop reload calls this before a live load, because a load cannot be undone (BACKLOG #299). @@ -992,12 +992,9 @@ def _verifies(crl: x509.CertificateRevocationList, ca: x509.Certificate) -> bool signature over SHA-1 that the CA did make, and a handshake takes that CRL and judges the peer by it. Before #4159 such a signature read ``None``. A forged one reads False. - **True says the CA's key made the signature, not that every TLS library takes it.** The key - is asked through ``cryptography``'s own OpenSSL, and the handshake runs on the ``ssl`` - module's, which can be another build with another policy. A policy that refuses the digest, - such as a system crypto policy that refuses SHA-1 signatures, fails the handshake on a CRL - read ``verified`` here. That holds for any verified signature, not only one - ``is_signature_valid`` refused; ``docs/CONFIGURATION.md`` states it beside the warn cases.""" + True says the CA's key made the signature, not that every TLS library or policy takes it; + ``docs/CONFIGURATION.md`` states that residual, in the bullet on a signature the engine + cannot check.""" try: key = ca.public_key() if crl.is_signature_valid(key): # type: ignore[arg-type] diff --git a/tests/test_crl_signature_at_start.py b/tests/test_crl_signature_at_start.py index 7c4331f60e..4837f722c8 100644 --- a/tests/test_crl_signature_at_start.py +++ b/tests/test_crl_signature_at_start.py @@ -570,8 +570,20 @@ def test_a_crl_its_named_ca_proves_bad_refuses_beside_a_ca_with_no_ski_that_veri # no SKI whose key could also verify this CRL must be of the CRL's kind, and so of the leaf's. raw = _raw_hop(mixed, twins.crl) assert raw.verify_flags & ssl.VERIFY_X509_STRICT - with pytest.raises(ssl.SSLError, match="CRL signature failure|Missing Subject Key Identifier"): + with pytest.raises( + ssl.SSLError, match="CRL signature failure|Missing Subject Key Identifier" + ) as on_bad: _handshake(raw, twins.server(sends_renewed=False)) + # Which order this runtime took decides what a CRL the named CA signed must do. Where this + # CRL was reached and failed, that one passes, so the CRL was the fault. Where the chain failed + # on the missing SKI, that one fails the same way, so the CRL was never consulted. + twins.write_crl("new", "new") + good = _raw_hop(mixed, twins.crl) + if "CRL signature failure" in str(on_bad.value): + _handshake(good, twins.server(sends_renewed=False)) # raises if refused + else: + with pytest.raises(ssl.SSLError, match="Missing Subject Key Identifier"): + _handshake(good, twins.server(sends_renewed=False)) def test_a_strict_engine_context_refuses_a_chain_through_a_ca_with_no_ski(tmp_path: Path) -> None: @@ -1109,9 +1121,10 @@ def test_one_signer_gets_one_verdict_under_sha256_and_sha1( """Finding 2 of the Lander's covering review on engine PR 2585. A ``None`` from :func:`messagefoundry.pki._verifies` meant either a key the engine cannot check or a signature the key accepts that ``cryptography``'s own check refused, and SHA-1 is refused there. So one - signer's verdict could differ by digest alone: a CA with no SKI signing beside the named CA - read ``verified`` over SHA-256 and ``bad`` over SHA-1, and the named CA signing read - ``verified`` and ``uncheckable``. Each row now gives one verdict for both digests.""" + signer's verdict could differ by digest alone. On main, a CA with no SKI signing beside the + named CA read ``verified`` over SHA-256 and ``uncheckable`` over SHA-1, and so did the named + CA signing; the head the review read gave ``bad`` over SHA-1 for the first. Each row now + gives one verdict for both digests.""" keys = {"named": _rsa_key(0), "no-ski": _rsa_key(1)} cas = {"named": _ca(keys["named"], keys["named"]), "no-ski": _ca_without_ski(keys["no-ski"])} shape = _crl(keys[signer], names=None if names is None else keys[names]) From ca368b7e0f0abeb025555f8df59d873dc78ed274 Mon Sep 17 00:00:00 2001 From: Scott Hall Date: Tue, 27 Oct 2026 21:52:46 -0500 Subject: [PATCH 7/9] CRL signature check: a SHA-1 CRL the CA made reads "cannot check" again pki._verifies returns None, not True, where the CA's key accepts a signature that cryptography's is_signature_valid refused, as for an RSA signature over SHA-1. A genuine SHA-1 CRL from a judged CA reads uncheckable again: a start warns, and the running-hop reload holds it back until a restart. A forged one that names the CA's key still reads bad. Finding 2 stays fixed: the named-CA filter keeps a no-SKI signer beside the named CA bad under both digests. The trade-off is stated in CONFIGURATION.md: on a host that accepts SHA-1 a revocation in such a CRL waits for a restart; in exchange the reload never applies live a CRL that a host refusing SHA-1 would fail every handshake on, and the engine cannot tell the two hosts apart. Tests: the SHA-1 test is back to uncheckable, warning and held back; the one-signer table carries a verdict per digest, renamed test_sha1_moves_a_signers_verdict_only_from_verified_to_uncheckable. vault BACKLOG #4159 --- changelog.d/4159.security.md | 12 ++-- docs/CONFIGURATION.md | 22 ++++--- messagefoundry/pki.py | 36 ++++++----- tests/test_crl_signature_at_start.py | 97 +++++++++++++++++----------- 4 files changed, 100 insertions(+), 67 deletions(-) diff --git a/changelog.d/4159.security.md b/changelog.d/4159.security.md index 6d36e0ad9a..0cf372f7eb 100644 --- a/changelog.d/4159.security.md +++ b/changelog.d/4159.security.md @@ -4,12 +4,12 @@ where the engine could not check its key, the start loaded the file with a WARNING. Now such a CA counts for nothing once the CRL names another listed CA's key, and the running-hop reload holds such a file back too. A CA the CRL names that cannot be checked still warns, and so does a - lone CA with no subject key identifier that cannot be checked. A signature the CA's key accepts - is now verified even where the `cryptography` library's own check refuses it, as for an RSA - signature over SHA-1 the CA made, so that refusal no longer decides the verdict: a start loads - such a CRL with no warning, and the reload applies it where it held it back before. A digest the - engine's own check cannot use still reads as one it cannot check, and a TLS library or system - policy that refuses SHA-1 can still fail the handshake on such a CRL (`docs/CONFIGURATION.md`, + lone CA with no subject key identifier that cannot be checked. An RSA signature over SHA-1 that + a CA counted here made still reads as one the engine cannot check, as before: a start warns, + and the reload holds it back until a restart, so a revocation in it waits for that restart on a + host that accepts SHA-1. That avoids applying live a CRL on which a host that refuses SHA-1 + would fail every handshake, since the engine cannot tell which kind of host it is on. A forged + SHA-1 signature that names a listed CA's key still refuses the file (`docs/CONFIGURATION.md`, the bullet on a signature the engine cannot check). The premise, that the engine's verifying contexts use strict verification, and the cases where the start can refuse a CRL a handshake could pass are stated in `docs/CONFIGURATION.md`, in the paragraph that starts "Why a CA with no diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 04fcfe43b5..e655e5ec1f 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2059,14 +2059,20 @@ three cases warn: checking with it raises an error. Where the CRL names the key of a listed CA that fits it, only such a CA counts here (the paragraph that starts "Why a CA with no subject key identifier" says why). Where it names no such CA's key, as when it names a key no listed CA carries, every - fitting CA counts, one with no subject key identifier included. A measured example is an ECDSA signature on a CRL - that names an RSA key. A signature the CA's key accepts is verified even where the - `cryptography` library's own check refuses it, as for an RSA signature over SHA-1 that the CA - did make; OpenSSL 3.5.7 was measured to judge a peer by such a CRL (vault BACKLOG #4159). - Verified means the CA's key made the signature, not that every TLS library takes it: the - engine checks with the `cryptography` library's OpenSSL, the handshake runs on Python's, and a - build or system policy that refuses the digest, such as SHA-1, fails the handshake on a CRL - that started with no warning and that a reload applies. + fitting CA counts, one with no subject key identifier included. Two measured examples are an + ECDSA signature on a CRL that names an RSA key, and an RSA signature over SHA-1 that the CA did + make. The CA's key accepts that second one, and the `cryptography` library's own check refuses + its digest, so the engine reads it as one it cannot check. + + **That SHA-1 case is a trade-off (vault BACKLOG #4159).** A start loads such a CRL with the + WARNING, and the running-hop reload holds it back, so it is applied at the next restart. The + cost is revocation freshness: on a host whose TLS library accepts SHA-1, as OpenSSL 3.5.7 was + measured to, judging a peer by such a CRL, a revocation in a new copy of it waits for a + restart. What it avoids is applying live a CRL on which a host that refuses SHA-1, by its build + or its system policy, would fail every handshake through that CA. The engine checks with the + `cryptography` library's OpenSSL and the handshake runs on Python's, so it cannot tell which + kind of host it is on. A forged SHA-1 signature that names a listed CA's key is still proved + bad, and refuses the file. If the check itself fails, the engine logs an ERROR that says so. It loads the file unless a CRL it judged before the failure is proved badly signed, which refuses the file. Where it loads, the diff --git a/messagefoundry/pki.py b/messagefoundry/pki.py index 0dc77d2a5d..c904beaedb 100644 --- a/messagefoundry/pki.py +++ b/messagefoundry/pki.py @@ -453,7 +453,10 @@ class CrlSignature: and its key does not verify the CRL. The CRL names that key, and that key did not sign it. * ``uncheckable``: no judged CA verifies, and some judged CA could not be checked: one the CRL names, or, where it names none, any fitting CA. "Cannot check" is not "does not verify" - (:func:`_verifies`). + (:func:`_verifies`). It covers a signature over SHA-1 that a judged CA did make, which a + start loads with a warning and the reload holds back until a restart; a forged one is + ``bad``. ``docs/CONFIGURATION.md`` states that trade-off, in the bullet on a signature the + engine cannot check. * ``unproven``: every fitting CA was checked and none verifies, but the CRL names none of them by key identifier. A CRL with no key identifier fits every CA of that name. So a CA that renewed its key reads this way, while the peer's own chain may still carry the CA that @@ -919,7 +922,8 @@ def crl_signature_refusal(pem: bytes, ca_ders: Iterable[bytes]) -> tuple[str, bo **The flag is wider than what a start refuses (vault BACKLOG #2905).** It is True for every :class:`CrlSignature` state but ``verified`` and ``unlisted``. A start refuses only ``bad``, the proven case, and loads the other two with a warning. So the reload holds back some files - that a start would load. + that a start would load, at least a genuine SHA-1 CRL from a judged CA, whose revocations + then wait for a restart (the ``uncheckable`` state says why). A CA that does not parse is ignored. ``pem`` must be bytes :func:`judge_every_crl` accepted; a block that does not parse raises ``ValueError``.""" @@ -981,20 +985,18 @@ def _verifies(crl: x509.CertificateRevocationList, ca: x509.Certificate) -> bool ``is_signature_valid`` answers False for a signature it cannot check as well as for one that fails, because it turns every error inside its own verification into False. So a False from it is not proof. The key is then asked directly (:func:`_verify_with`), and only its - ``InvalidSignature`` counts as proof. ``None`` is only what the key cannot be asked: a key or - an algorithm ``cryptography`` does not support, a key of another kind than the signature, and - a ``ValueError`` or ``TypeError`` from either call. - - **A signature the key accepts is True, even where ``is_signature_valid`` refused it** (vault - BACKLOG #4159). The key's own check passed, so the key made the signature, and a verdict that - followed the refusal would differ for one signer by digest alone. Measured 2026-10-27 - with ``cryptography`` 50.0.2 and OpenSSL 3.5.7: ``is_signature_valid`` refuses an RSA PKCS1v15 - signature over SHA-1 that the CA did make, and a handshake takes that CRL and judges the peer - by it. Before #4159 such a signature read ``None``. A forged one reads False. - - True says the CA's key made the signature, not that every TLS library or policy takes it; - ``docs/CONFIGURATION.md`` states that residual, in the bullet on a signature the engine - cannot check.""" + ``InvalidSignature`` counts as proof. Everything else is ``None``, "cannot check": a key or an + algorithm ``cryptography`` does not support, a key of another kind than the signature, and a + ``ValueError`` or ``TypeError`` from either call. + + **So is a signature the key accepts after ``is_signature_valid`` refused it**: the key accepts + it, and the library's digest policy refuses it. Measured 2026-10-27 with ``cryptography`` + 50.0.2 and OpenSSL 3.5.7: an RSA PKCS1v15 signature over SHA-1 that the CA did make reads + ``None`` that way, and a forged one reads False. So a genuine SHA-1 CRL from a judged CA + (:class:`CrlSignature`) reads ``uncheckable``: a start warns, and the running-hop reload + holds it back. A forged one that names the CA's key still reads ``bad``. That is a + trade-off; its cost and what it avoids are in ``docs/CONFIGURATION.md``, in the bullet on a + signature the engine cannot check (vault BACKLOG #4159).""" try: key = ca.public_key() if crl.is_signature_valid(key): # type: ignore[arg-type] @@ -1004,7 +1006,7 @@ def _verifies(crl: x509.CertificateRevocationList, ca: x509.Certificate) -> bool return False except (ValueError, TypeError, UnsupportedAlgorithm): return None - return True # the key accepts what ``is_signature_valid`` refused, as for SHA-1 above + return None # the key accepts what ``is_signature_valid`` refused, as for SHA-1 above _DSA_SIGNATURES = frozenset( diff --git a/tests/test_crl_signature_at_start.py b/tests/test_crl_signature_at_start.py index 4837f722c8..b909dc93bc 100644 --- a/tests/test_crl_signature_at_start.py +++ b/tests/test_crl_signature_at_start.py @@ -1038,14 +1038,17 @@ def _resigned( return x509.load_der_x509_crl(der[: -len(signature)] + signature).public_bytes(_PEM) -def test_an_rsa_sha1_crl_the_ca_signed_is_verified_and_a_forged_one_is_bad( +def test_an_rsa_sha1_crl_the_ca_signed_is_cannot_check_and_a_forged_one_is_bad( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: """A CRL that names the CA's key, signed over SHA-1. ``cryptography``'s own check refuses it - and the CA's key accepts it, so since vault BACKLOG #4159 it is ``verified``. It read - ``uncheckable`` before: a start warned, and the reload held it back. The premise is OpenSSL's: - it takes that CRL and judges the peer by it. The SHA-384 CRL is the control for the hand-made - encoding: the same swap under a digest the library accepts reads ``verified`` too.""" + and the CA's key accepts it, so it reads ``uncheckable``: a start loads it with the warning, + and the reload holds it back. That is the trade-off ``docs/CONFIGURATION.md`` states in the + bullet on a signature the engine cannot check (vault BACKLOG #4159). The handshake below is + its cost on this host: OpenSSL takes the CRL and judges the peer by it, so the reload held + back a CRL this host could use. The SHA-384 CRL is the control for the hand-made encoding: + the same swap under a digest the library accepts reads ``verified``. So ``uncheckable`` is + about SHA-1, not about the swap.""" key = _rsa_key(0) other = _rsa_key(1) ca = _ca(key, key) @@ -1055,25 +1058,26 @@ def test_an_rsa_sha1_crl_the_ca_signed_is_verified_and_a_forged_one_is_bad( forged = _resigned(_crl(other, names=key), other, _SHA1_WITH_RSA, hashes.SHA1()) control = _resigned(_crl(key, names=key), key, _SHA384_WITH_RSA, hashes.SHA384()) - # The premise on the library's side: its own check refuses the CRL the CA made. - assert x509.load_pem_x509_crl(good).is_signature_valid(key.public_key()) is False + # The premise on the library's side: its own check refuses the CRL the CA made, and the + # CA's key accepts it, so the engine cannot check it rather than proving it bad. + good_crl = x509.load_pem_x509_crl(good) + assert good_crl.is_signature_valid(key.public_key()) is False + assert pki_module._verifies(good_crl, ca) is None assert [s.state for s in judge_crl_signatures(control, ders)] == ["verified"] - assert [s.state for s in judge_crl_signatures(good, ders)] == ["verified"] assert [s.state for s in judge_crl_signatures(forged, ders)] == ["bad"] - assert crl_signature_refusal(good, ders) is None # the reload applies it + verdict = crl_signature_refusal(good, ders) + assert verdict is not None and verdict[1] is True # the reload holds it back ca_file, crl_file = tmp_path / "ca.pem", tmp_path / "crl.pem" ca_file.write_bytes(ca.public_bytes(_PEM)) crl_file.write_bytes(good) - with caplog.at_level("WARNING"): - hop = _hop(ca_file, crl_file) - assert _warnings(caplog) == [] + _assert_uncheckable_and_the_start_warns(tmp_path, good, caplog) - # The premise on OpenSSL's side: it judges a peer under this CA by the SHA-1 CRL. Control: - # the same CRL with the peer's serial listed refuses the handshake. + # The cost, on this host: OpenSSL judges a peer under this CA by the SHA-1 CRL the start + # loaded. Control: the same CRL with the peer's serial listed refuses the handshake. leaf, leaf_key = _leaf(_NAME, key) server = _server(tmp_path, leaf.public_bytes(_PEM), leaf_key) - _handshake(hop, server) # raises if refused + _handshake(_hop(ca_file, crl_file), server) # raises if refused crl_file.write_bytes( _resigned( _crl(key, names=key, revoke=leaf.serial_number), key, _SHA1_WITH_RSA, hashes.SHA1() @@ -1087,51 +1091,72 @@ def test_an_rsa_sha1_crl_the_ca_signed_is_verified_and_a_forged_one_is_bad( _hop(ca_file, crl_file) -#: The digests :func:`test_one_signer_gets_one_verdict_under_sha256_and_sha1` signs with. +#: The digests :func:`test_sha1_moves_a_signers_verdict_only_from_verified_to_uncheckable` signs +#: with: the OID, the digest, and which verdict of a row applies. _SHA256_AND_SHA1 = [ - pytest.param(_SHA256_WITH_RSA, hashes.SHA256(), id="sha256"), - pytest.param(_SHA1_WITH_RSA, hashes.SHA1(), id="sha1"), + pytest.param(_SHA256_WITH_RSA, hashes.SHA256(), 0, id="sha256"), + pytest.param(_SHA1_WITH_RSA, hashes.SHA1(), 1, id="sha1"), ] #: (the CAs the hop lists, in order, the CA whose key signs, the CA whose key the CRL names, the -#: verdict). "named" carries a Subject Key Identifier, and "no-ski" carries none. +#: verdicts over SHA-256 and over SHA-1). "named" carries a Subject Key Identifier, and "no-ski" +#: carries none. The test's docstring says why the verdicts differ where they do. _ONE_SIGNER = [ - pytest.param(("named",), "named", "named", "verified", id="the-named-ca-signs"), - pytest.param(("named",), "named", None, "verified", id="the-crl-names-no-key"), - pytest.param(("no-ski",), "no-ski", "no-ski", "verified", id="a-lone-ca-with-no-ski-signs"), pytest.param( - ("named", "no-ski"), "no-ski", "named", "bad", id="no-ski-ca-signs-after-the-named-ca" + ("named",), "named", "named", ("verified", "uncheckable"), id="the-named-ca-signs" + ), + pytest.param(("named",), "named", None, ("verified", "uncheckable"), id="the-crl-names-no-key"), + pytest.param( + ("no-ski",), + "no-ski", + "no-ski", + ("verified", "uncheckable"), + id="a-lone-ca-with-no-ski-signs", + ), + pytest.param( + ("named", "no-ski"), + "no-ski", + "named", + ("bad", "bad"), + id="no-ski-ca-signs-after-the-named-ca", ), pytest.param( - ("no-ski", "named"), "no-ski", "named", "bad", id="no-ski-ca-signs-before-the-named-ca" + ("no-ski", "named"), + "no-ski", + "named", + ("bad", "bad"), + id="no-ski-ca-signs-before-the-named-ca", ), ] -@pytest.mark.parametrize(("oid", "digest"), _SHA256_AND_SHA1) -@pytest.mark.parametrize(("listed", "signer", "names", "state"), _ONE_SIGNER) -def test_one_signer_gets_one_verdict_under_sha256_and_sha1( +@pytest.mark.parametrize(("oid", "digest", "column"), _SHA256_AND_SHA1) +@pytest.mark.parametrize(("listed", "signer", "names", "states"), _ONE_SIGNER) +def test_sha1_moves_a_signers_verdict_only_from_verified_to_uncheckable( listed: tuple[str, ...], signer: str, names: str | None, - state: str, + states: tuple[str, str], oid: bytes, digest: hashes.HashAlgorithm, + column: int, ) -> None: - """Finding 2 of the Lander's covering review on engine PR 2585. A ``None`` from - :func:`messagefoundry.pki._verifies` meant either a key the engine cannot check or a signature - the key accepts that ``cryptography``'s own check refused, and SHA-1 is refused there. So one - signer's verdict could differ by digest alone. On main, a CA with no SKI signing beside the - named CA read ``verified`` over SHA-256 and ``uncheckable`` over SHA-1, and so did the named - CA signing; the head the review read gave ``bad`` over SHA-1 for the first. Each row now - gives one verdict for both digests.""" + """Finding 2 of the Lander's covering review on engine PR 2585: one signer's verdict differed + by digest alone. On main, a CA with no SKI signing beside the named CA read ``verified`` over + SHA-256 and ``uncheckable`` over SHA-1, and the head the review read gave ``bad`` over SHA-1. + The named-CA filter (pki.CrlSignature) makes that signer ``bad`` under both digests: only the + named CA is judged, and its key is proven not to verify under either. Where the signer is + judged, SHA-1 still moves its verdict, but only to ``uncheckable``, never to ``bad``: its key + accepts the signature, ``cryptography``'s own check refuses the digest, and pki._verifies + reads that as "cannot check". That difference is the trade-off docs/CONFIGURATION.md states + in the bullet on a signature the engine cannot check.""" keys = {"named": _rsa_key(0), "no-ski": _rsa_key(1)} cas = {"named": _ca(keys["named"], keys["named"]), "no-ski": _ca_without_ski(keys["no-ski"])} shape = _crl(keys[signer], names=None if names is None else keys[names]) pem = _resigned(shape, keys[signer], oid, digest) ders = [cas[who].public_bytes(serialization.Encoding.DER) for who in listed] - assert [s.state for s in judge_crl_signatures(pem, ders)] == [state] + assert [s.state for s in judge_crl_signatures(pem, ders)] == [states[column]] def test_a_version_that_logs_nothing_in_between_does_not_reset_the_record( From dc2c709e2b8cd7794779a6df7f34df57dd75cd16 Mon Sep 17 00:00:00 2001 From: Scott Hall Date: Tue, 27 Oct 2026 22:29:19 -0500 Subject: [PATCH 8/9] CRL signature check: repairs from review round-56109fe24d - CONFIGURATION.md and the changelog state the SHA-1 trade-off's cost in full: a held-back copy can lapse with no restart, and on a host that refuses SHA-1 the restart still loads the CRL with the warning. - The SHA-1 test's OpenSSL half skips on a host that refuses the SHA-1 CRL, and the forged-CRL refusal runs before it. - Two lines over 100 columns rewrapped, string content unchanged. - A test comment points at CONFIGURATION.md for the residuals. vault BACKLOG #4159 --- changelog.d/4159.security.md | 8 +++++--- docs/CONFIGURATION.md | 11 +++++++---- messagefoundry/pipeline/crl_reload.py | 6 +++--- messagefoundry/pki.py | 5 +++-- tests/test_crl_signature_at_start.py | 22 ++++++++++++++-------- 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/changelog.d/4159.security.md b/changelog.d/4159.security.md index 0cf372f7eb..0f318e3578 100644 --- a/changelog.d/4159.security.md +++ b/changelog.d/4159.security.md @@ -6,9 +6,11 @@ holds such a file back too. A CA the CRL names that cannot be checked still warns, and so does a lone CA with no subject key identifier that cannot be checked. An RSA signature over SHA-1 that a CA counted here made still reads as one the engine cannot check, as before: a start warns, - and the reload holds it back until a restart, so a revocation in it waits for that restart on a - host that accepts SHA-1. That avoids applying live a CRL on which a host that refuses SHA-1 - would fail every handshake, since the engine cannot tell which kind of host it is on. A forged + and the reload holds it back until a restart, so on a host that accepts SHA-1 a revocation in + it waits for that restart, and the copy the hop holds can lapse first. That avoids applying + live, with nobody acting, a CRL on which a host that refuses SHA-1 would fail every handshake, + since the engine cannot tell which kind of host it is on; a restart there still loads it with + the warning. A forged SHA-1 signature that names a listed CA's key still refuses the file (`docs/CONFIGURATION.md`, the bullet on a signature the engine cannot check). The premise, that the engine's verifying contexts use strict verification, and the cases where the start can refuse a CRL a handshake diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index e655e5ec1f..c0984d96c7 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2066,10 +2066,13 @@ three cases warn: **That SHA-1 case is a trade-off (vault BACKLOG #4159).** A start loads such a CRL with the WARNING, and the running-hop reload holds it back, so it is applied at the next restart. The - cost is revocation freshness: on a host whose TLS library accepts SHA-1, as OpenSSL 3.5.7 was - measured to, judging a peer by such a CRL, a revocation in a new copy of it waits for a - restart. What it avoids is applying live a CRL on which a host that refuses SHA-1, by its build - or its system policy, would fail every handshake through that CA. The engine checks with the + cost falls on a host whose TLS library accepts SHA-1, as OpenSSL 3.5.7 was measured to, judging + a peer by such a CRL. There a revocation in a new copy waits for a restart, and if nobody + restarts, the copy the hop holds lapses at its next update and the hop then refuses the peers + it judges by it, as for any held-back file. What it avoids is applying live, with nobody acting, a CRL on which a + host that refuses SHA-1, by its build or its system policy, would fail every handshake through + that CA. It does not prevent that failure: on such a host the restart loads the CRL with the + WARNING, and the handshakes through that CA then fail. The engine checks with the `cryptography` library's OpenSSL and the handshake runs on Python's, so it cannot tell which kind of host it is on. A forged SHA-1 signature that names a listed CA's key is still proved bad, and refuses the file. diff --git a/messagefoundry/pipeline/crl_reload.py b/messagefoundry/pipeline/crl_reload.py index 2b8fa1733b..00b01718bc 100644 --- a/messagefoundry/pipeline/crl_reload.py +++ b/messagefoundry/pipeline/crl_reload.py @@ -201,9 +201,9 @@ "signature does not stop a start: unless another rule refuses the file, a start loads it " "with a warning, and each handshake then decides. That covers at least a CRL that names no " "key and a signature the engine cannot check with the key the CRL names, or, where it names " - "no listed CA's key, with the key of some listed CA that fits it. The handshake passes where the peer's " - "chain carries the CA that signed the CRL, as after a CA renews its key, and fails where it " - "does not" + "no listed CA's key, with the key of some listed CA that fits it. The handshake passes where " + "the peer's chain carries the CA that signed the CRL, as after a CA renews its key, and fails " + "where it does not" ) NO_ISSUER = ( "Restart the engine to apply the file. The reload checks a CRL's signature against the hop's " diff --git a/messagefoundry/pki.py b/messagefoundry/pki.py index c904beaedb..cafb66db9d 100644 --- a/messagefoundry/pki.py +++ b/messagefoundry/pki.py @@ -914,8 +914,9 @@ def crl_signature_refusal(pem: bytes, ca_ders: Iterable[bytes]) -> tuple[str, bo failed or could not run, and False when no listed CA fits. :class:`CrlSignature` says what "fits" means and which CAs are judged. A block with the True flag is reported over one with the False flag, wherever it sits in the file. OpenSSL checks a CRL's signature at the - handshake, not at the load. So OpenSSL itself loads a CRL with the right issuer name and Authority Key Identifier but a bad - signature, and then every handshake it judges fails with ``CRL signature failure``. The + handshake, not at the load. So OpenSSL itself loads a CRL with the right issuer name and + Authority Key Identifier but a bad signature, and then every handshake it judges fails with + ``CRL signature failure``. The running-hop reload calls this before a live load, because a load cannot be undone (BACKLOG #299). diff --git a/tests/test_crl_signature_at_start.py b/tests/test_crl_signature_at_start.py index b909dc93bc..54ab7b2bb5 100644 --- a/tests/test_crl_signature_at_start.py +++ b/tests/test_crl_signature_at_start.py @@ -518,7 +518,7 @@ def test_a_crl_its_named_ca_proves_bad_refuses_beside_an_unchecked_ca_with_no_sk # Control: OpenSSL itself fails this chain's handshake on this CRL, in either listing order. # One chain shape only, a leaf under the named CA, with a no-SKI CA whose key is of another - # kind than the leaf's signature; pki.CrlSignature names the residuals. + # kind than the leaf's signature; docs/CONFIGURATION.md names the residuals. with pytest.raises(ssl.SSLError, match="CRL signature failure"): _handshake(_raw_hop(mixed, twins.crl), twins.server(sends_renewed=False)) @@ -1070,14 +1070,24 @@ def test_an_rsa_sha1_crl_the_ca_signed_is_cannot_check_and_a_forged_one_is_bad( ca_file, crl_file = tmp_path / "ca.pem", tmp_path / "crl.pem" ca_file.write_bytes(ca.public_bytes(_PEM)) + crl_file.write_bytes(forged) + with pytest.raises(ValueError, match="does not verify the CRL's signature"): + _hop(ca_file, crl_file) crl_file.write_bytes(good) _assert_uncheckable_and_the_start_warns(tmp_path, good, caplog) - # The cost, on this host: OpenSSL judges a peer under this CA by the SHA-1 CRL the start - # loaded. Control: the same CRL with the peer's serial listed refuses the handshake. + # The cost, on a host that accepts SHA-1: OpenSSL judges a peer under this CA by the SHA-1 + # CRL the start loaded. Control: the same CRL with the peer's serial listed refuses the + # handshake. A host whose OpenSSL policy refuses SHA-1 is the other side of the trade-off, + # so this half skips there rather than failing; the engine's half above has already run. leaf, leaf_key = _leaf(_NAME, key) server = _server(tmp_path, leaf.public_bytes(_PEM), leaf_key) - _handshake(_hop(ca_file, crl_file), server) # raises if refused + try: + _handshake(_hop(ca_file, crl_file), server) + except ssl.SSLError as refused: + if "CRL signature failure" not in str(refused): + raise + pytest.skip(f"this host's OpenSSL refuses the SHA-1 CRL: {refused}") crl_file.write_bytes( _resigned( _crl(key, names=key, revoke=leaf.serial_number), key, _SHA1_WITH_RSA, hashes.SHA1() @@ -1086,10 +1096,6 @@ def test_an_rsa_sha1_crl_the_ca_signed_is_cannot_check_and_a_forged_one_is_bad( with pytest.raises(ssl.SSLError, match="revoked"): _handshake(_hop(ca_file, crl_file), server) - crl_file.write_bytes(forged) - with pytest.raises(ValueError, match="does not verify the CRL's signature"): - _hop(ca_file, crl_file) - #: The digests :func:`test_sha1_moves_a_signers_verdict_only_from_verified_to_uncheckable` signs #: with: the OID, the digest, and which verdict of a row applies. From 663aa64a4ca7f64681ccfc29b85e3b61cb608326 Mon Sep 17 00:00:00 2001 From: Scott Hall Date: Tue, 27 Oct 2026 22:57:45 -0500 Subject: [PATCH 9/9] CRL signature check: repairs from review round-f2a71675a6 - CrlSignature's uncheckable bullet says "at least an RSA signature over SHA-1", and qualifies which forged one is bad. - CONFIGURATION.md: where the held copy is itself an earlier SHA-1 CRL from that CA, holding the new one back gains nothing on a host that refuses SHA-1. - The changelog summarises the SHA-1 trade-off and points at CONFIGURATION.md instead of restating it. - A test comment no longer says the same-name CA order ignores the CA-file order, which was measured false on one host. vault BACKLOG #4159 --- changelog.d/4159.security.md | 11 ++++------- docs/CONFIGURATION.md | 4 +++- messagefoundry/pki.py | 9 +++++---- tests/test_crl_signature_at_start.py | 4 ++-- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/changelog.d/4159.security.md b/changelog.d/4159.security.md index 0f318e3578..bd0bcb8c75 100644 --- a/changelog.d/4159.security.md +++ b/changelog.d/4159.security.md @@ -6,13 +6,10 @@ holds such a file back too. A CA the CRL names that cannot be checked still warns, and so does a lone CA with no subject key identifier that cannot be checked. An RSA signature over SHA-1 that a CA counted here made still reads as one the engine cannot check, as before: a start warns, - and the reload holds it back until a restart, so on a host that accepts SHA-1 a revocation in - it waits for that restart, and the copy the hop holds can lapse first. That avoids applying - live, with nobody acting, a CRL on which a host that refuses SHA-1 would fail every handshake, - since the engine cannot tell which kind of host it is on; a restart there still loads it with - the warning. A forged - SHA-1 signature that names a listed CA's key still refuses the file (`docs/CONFIGURATION.md`, - the bullet on a signature the engine cannot check). The premise, that the engine's verifying + and the reload holds it back until a restart. That trades revocation freshness on a host that + accepts SHA-1 against applying live a CRL that a host refusing SHA-1 would fail every handshake + on; `docs/CONFIGURATION.md`, in the bullet on a signature the engine cannot check, states the + trade-off. The premise, that the engine's verifying contexts use strict verification, and the cases where the start can refuse a CRL a handshake could pass are stated in `docs/CONFIGURATION.md`, in the paragraph that starts "Why a CA with no subject key identifier counts for nothing there". (vault BACKLOG #4159) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index c0984d96c7..d3a9a6a2b9 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2072,7 +2072,9 @@ three cases warn: it judges by it, as for any held-back file. What it avoids is applying live, with nobody acting, a CRL on which a host that refuses SHA-1, by its build or its system policy, would fail every handshake through that CA. It does not prevent that failure: on such a host the restart loads the CRL with the - WARNING, and the handshakes through that CA then fail. The engine checks with the + WARNING, and the handshakes through that CA then fail. Where the copy the hop holds is itself an + earlier SHA-1 CRL from that CA, such a host fails those handshakes already, and holding the new + copy back gains nothing there. The engine checks with the `cryptography` library's OpenSSL and the handshake runs on Python's, so it cannot tell which kind of host it is on. A forged SHA-1 signature that names a listed CA's key is still proved bad, and refuses the file. diff --git a/messagefoundry/pki.py b/messagefoundry/pki.py index cafb66db9d..f683a5bb19 100644 --- a/messagefoundry/pki.py +++ b/messagefoundry/pki.py @@ -453,10 +453,11 @@ class CrlSignature: and its key does not verify the CRL. The CRL names that key, and that key did not sign it. * ``uncheckable``: no judged CA verifies, and some judged CA could not be checked: one the CRL names, or, where it names none, any fitting CA. "Cannot check" is not "does not verify" - (:func:`_verifies`). It covers a signature over SHA-1 that a judged CA did make, which a - start loads with a warning and the reload holds back until a restart; a forged one is - ``bad``. ``docs/CONFIGURATION.md`` states that trade-off, in the bullet on a signature the - engine cannot check. + (:func:`_verifies`). It covers at least an RSA signature over SHA-1 that a judged CA did + make, which a start loads with a warning and the reload holds back until a restart; a + forged one that names a listed CA's key of the signature's kind is ``bad``. + ``docs/CONFIGURATION.md`` states that trade-off, in the bullet on a signature the engine + cannot check. * ``unproven``: every fitting CA was checked and none verifies, but the CRL names none of them by key identifier. A CRL with no key identifier fits every CA of that name. So a CA that renewed its key reads this way, while the peer's own chain may still carry the CA that diff --git a/tests/test_crl_signature_at_start.py b/tests/test_crl_signature_at_start.py index 54ab7b2bb5..64615a6e61 100644 --- a/tests/test_crl_signature_at_start.py +++ b/tests/test_crl_signature_at_start.py @@ -562,8 +562,8 @@ def test_a_crl_its_named_ca_proves_bad_refuses_beside_a_ca_with_no_ski_that_veri assert verdict_of_scan[1].startswith("Replace the CRL in the CA file with one its CA signed") # Control: OpenSSL fails a strict handshake through the named CA. Which error depends on the - # order OpenSSL sorts same-name CAs in, which differs by C runtime, not on the file order. - # Where it tries the named CA first, the handshake reaches this CRL and fails on it. Where it + # order OpenSSL tries same-name CAs in, which was measured to follow the CA-file order on one + # host and may differ by C runtime, so both file orders run. Where it tries the named CA first, the handshake reaches this CRL and fails on it. Where it # tries the CA with no SKI first, whose key is of the leaf's kind, the strict check fails the # chain on the missing SKI before the CRL, whatever the CRL (open question 4 in the PR body). # The CRL's own failure, in both orders, is the unchecked-CA test's control above; a CA with