diff --git a/changelog.d/4159.security.md b/changelog.d/4159.security.md new file mode 100644 index 0000000000..bd0bcb8c75 --- /dev/null +++ b/changelog.d/4159.security.md @@ -0,0 +1,15 @@ +- **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.** 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. 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. 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 2b6fb4ad3e..d3a9a6a2b9 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2001,10 +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. 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. +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 @@ -2035,9 +2054,30 @@ 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. - 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, 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 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. 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 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. 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. 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 e4e29ae2d2..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 @@ -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 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 e81a98987b..1865a26b91 100644 --- a/messagefoundry/config/tls_policy.py +++ b/messagefoundry/config/tls_policy.py @@ -570,7 +570,11 @@ 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 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. _UNCONFIRMED_SHOWN = 3 @@ -590,8 +594,10 @@ def _check_crl_signatures( **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 - the CRL, so it must not log per hop. The warning is one line for the file, and + 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 @@ -641,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 25a64e2301..010d4b1234 100644 --- a/messagefoundry/pipeline/cert_expiry.py +++ b/messagefoundry/pipeline/cert_expiry.py @@ -770,10 +770,10 @@ 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", + "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/pipeline/crl_reload.py b/messagefoundry/pipeline/crl_reload.py index 75297dd31a..00b01718bc 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 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 0965d4310a..f683a5bb19 100644 --- a/messagefoundry/pki.py +++ b/messagefoundry/pki.py @@ -439,12 +439,25 @@ 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, 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`). + 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`). 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 @@ -856,24 +869,28 @@ 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 + 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 - if not fitting: - return "unlisted" if unchecked: return "uncheckable" return "bad" if named else "unproven" @@ -894,19 +911,21 @@ 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 - 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 + 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 (BACKLOG #299). **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``.""" @@ -918,7 +937,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, @@ -966,12 +987,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. 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. 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] @@ -981,7 +1008,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 None # 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 96d247f6f1..64615a6e61 100644 --- a/tests/test_crl_signature_at_start.py +++ b/tests/test_crl_signature_at_start.py @@ -24,8 +24,10 @@ from __future__ import annotations import datetime +import functools import logging import ssl +import time import warnings import weakref from dataclasses import dataclass @@ -47,9 +49,10 @@ 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 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 @@ -207,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" @@ -406,6 +419,306 @@ 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, *, 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. With + ``localhost`` it also names ``localhost``, so a server can present it as its own.""" + builder = ( + 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) + ) + if localhost: + builder = builder.add_extension( + x509.SubjectAlternativeName([x509.DNSName("localhost")]), False + ) + return builder.sign(key, hashes.SHA256()) + + +@functools.cache +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. No default, so one number names one + key however it is written.""" + 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(0)) + 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"): + _hop(mixed, twins.crl) + + # 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 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; 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)) + + +@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 + # 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. Which error depends on the + # 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 + # 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" + ) 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: + """(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 + 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) + 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) + ) + + 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 + + +@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, 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 + 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. 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" + 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") + + # 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. + twins.crl.write_bytes(_crl(twins.stranger, names=twins.new_key, revoke=no_ski.serial_number)) + with pytest.raises(ssl.SSLError, match="revoked"): + _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: + """(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(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)) + 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(0)) + (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 ------------------------- @@ -726,14 +1039,18 @@ def _resigned( def test_an_rsa_sha1_crl_the_ca_signed_is_cannot_check_and_a_forged_one_is_bad( - tmp_path: Path, + 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 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) ders = [ca.public_bytes(serialization.Encoding.DER)] @@ -741,19 +1058,111 @@ 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, 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)] == ["uncheckable"] 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 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) - assert _hop(ca_file, crl_file).verify_flags & ssl.VERIFY_CRL_CHECK_LEAF 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 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) + 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() + ) + ) + with pytest.raises(ssl.SSLError, match="revoked"): + _handshake(_hop(ca_file, crl_file), server) + + +#: 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(), 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 +#: 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", "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", "bad"), + id="no-ski-ca-signs-before-the-named-ca", + ), +] + + +@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, + states: tuple[str, str], + oid: bytes, + digest: hashes.HashAlgorithm, + column: int, +) -> None: + """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)] == [states[column]] def test_a_version_that_logs_nothing_in_between_does_not_reset_the_record(