diff --git a/CHANGELOG.md b/CHANGELOG.md index 72b7996..5c5847d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), Pre-releases are not listed separately. A beta is a step towards the next public version, so its changes are folded into that version's entry as they land and are described against the **last public release**, never against the beta before it. What one beta corrected in an earlier beta does not appear at all: from the point of view of somebody upgrading between released versions, it never happened. +## [3.2.0] + +A consumer pinned to a panel's CA cannot currently tell a panel that has moved from something impersonating one, because the two produce the same verification failure. This release splits the question. + +### Added + +- **`build_panel_ssl_context` takes `check_hostname`**, so a caller can verify that a peer holds a key the pinned CA signed without also asserting that the certificate names the address it was dialled by. +- **`leaf_names_host` decides the name binding on its own**, hand-written against `getpeercert()` because `ssl.match_hostname` was removed in Python 3.12, and stricter than that function was: no wildcards, no `commonName` fallback, and DNS and IP entries + that never stand in for one another. + ## [3.1.1] A follow-up to 3.1.0's security work, with no API change and no adapter move required. diff --git a/pyproject.toml b/pyproject.toml index 8afbb5a..abba0ee 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "span-panel-api" -version = "3.1.1" +version = "3.2.0" description = "A client library for SPAN Panel API" authors = [ {name = "SpanPanel"} diff --git a/src/span_panel_api/__init__.py b/src/span_panel_api/__init__.py index af66103..2d798e4 100644 --- a/src/span_panel_api/__init__.py +++ b/src/span_panel_api/__init__.py @@ -6,7 +6,7 @@ from importlib.metadata import version as _pkg_version -from ._ssl import build_panel_ssl_context, ca_fingerprint +from ._ssl import build_panel_ssl_context, ca_fingerprint, leaf_names_host from .auth import ( delete_fqdn, download_ca_cert, @@ -154,6 +154,9 @@ # both live here rather than being reimplemented on the other side. "build_panel_ssl_context", "ca_fingerprint", + # Added 2026-08-28: the hostname half of verification, split out so a + # caller using a relaxed context can still establish the name binding. + "leaf_names_host", "delete_fqdn", "download_ca_cert", "get_fqdn", diff --git a/src/span_panel_api/_ssl.py b/src/span_panel_api/_ssl.py index dabdee6..e070e65 100644 --- a/src/span_panel_api/_ssl.py +++ b/src/span_panel_api/_ssl.py @@ -6,18 +6,24 @@ lives in ``auth.download_ca_cert``, and deciding whether a fetched PEM may be trusted lives with the caller. -Public rather than private (``_ssl`` is a module-name convention here, and both -names are re-exported from the package root) because the consumer needs exactly -these two: it builds the same context for its own HTTPS calls, and it prints and -compares the same fingerprint string. Two implementations of a fingerprint that -must agree byte-for-byte is a defect waiting for a firmware upgrade to find it. +Public rather than private (``_ssl`` is a module-name convention here, and every +name is re-exported from the package root) because the consumer needs all three: +it builds the same context for its own HTTPS calls, it prints and compares the +same fingerprint string, and it applies the same hostname rules when it has to +judge a name binding for itself. Two implementations of a fingerprint that must +agree byte-for-byte is a defect waiting for a firmware upgrade to find it, and +the same is true of a hand-written hostname matcher -- more so, since that one +is security-relevant and has no standard-library implementation left to defer +to since ``ssl.match_hostname`` was removed in Python 3.12. """ from __future__ import annotations import base64 import binascii +from collections.abc import Iterator, Mapping import hashlib +import ipaddress import ssl from .exceptions import SpanPanelValidationError @@ -26,7 +32,7 @@ _PEM_FOOTER = "-----END CERTIFICATE-----" -def build_panel_ssl_context(ca_pem: str) -> ssl.SSLContext: +def build_panel_ssl_context(ca_pem: str, *, check_hostname: bool = True) -> ssl.SSLContext: """Build an SSLContext that trusts only the provided panel CA. The panel issues a private CA and a server cert signed by it. We do @@ -42,7 +48,28 @@ def build_panel_ssl_context(ca_pem: str) -> ssl.SSLContext: This does not weaken the parts of verification that matter for this connection: the trust anchor is still only the panel's own CA, hostname - checking stays enabled, and signature/expiry validation is unchanged. + checking stays enabled by default, and signature/expiry validation is + unchanged. + + ``check_hostname=False`` asks a narrower question: *does the peer hold a + private key whose certificate chains to this anchor?* The chain, the + signature and the expiry are still verified -- only the binding between + the certificate and the name used to dial it is left unasserted. That is + a real distinction and not a relaxation of trust: an attacker without a + CA-signed key cannot complete the handshake either way. + + It exists because the two failures are otherwise indistinguishable, and + they call for opposite responses. A panel that has moved to a new DHCP + lease serves a perfectly good certificate that no longer names the + address it is reached at; something impersonating a panel serves one that + chains to nothing. Collapsing both into "verification failed" tells a + user their panel has been intercepted when its address merely changed. + + Never pass ``check_hostname=False`` for a connection that carries data. + The name binding is what stops a validated certificate being replayed by + a host it was not issued to, so a relaxed context belongs only in code + that is deciding *which* host to talk to, paired with + :func:`leaf_names_host` to establish the binding separately. Raises: ssl.SSLError: ``ca_pem`` is not a certificate the ssl module accepts. @@ -50,12 +77,104 @@ def build_panel_ssl_context(ca_pem: str) -> ssl.SSLContext: """ ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) ctx.verify_mode = ssl.CERT_REQUIRED - ctx.check_hostname = True + ctx.check_hostname = check_hostname ctx.verify_flags &= ~ssl.VERIFY_X509_STRICT ctx.load_verify_locations(cadata=ca_pem) return ctx +def leaf_names_host(peer_cert: Mapping[str, object], host: str) -> bool: + """Whether a validated peer certificate names ``host`` in its SAN. + + The hostname half of what ``check_hostname=True`` does in one step, split + out so a caller that built a relaxed context can still ask the question + and act on the answer. ``peer_cert`` is what ``SSLSocket.getpeercert()`` + returns, which is populated only for a certificate the handshake already + validated -- so this function decides naming, never trust. + + Hand-written because ``ssl.match_hostname`` was removed in Python 3.12 + and nothing replaced it as public API. The rules here are deliberately + stricter than the ones it implemented, because a panel's leaf is + machine-generated from a fixed template and needs none of the latitude a + general-purpose matcher owes the public web: + + - **No wildcards.** ``*.example.com`` is not matched against anything. A + panel names literal addresses, so a wildcard in one of its certificates + would be an anomaly rather than a case to support. + - **No ``commonName`` fallback.** Deprecated for two decades, and every + certificate this library meets carries a SAN. + - **IP and DNS entries are not interchangeable.** A host that parses as an + IP address is matched only against ``IP Address`` entries and a name + only against ``DNS`` entries, so a certificate naming the *string* + "10.0.0.5" in a DNS entry does not authorise the address 10.0.0.5. + - **Addresses compare parsed, names compare casefolded.** ``::1`` and + ``0:0:0:0:0:0:0:1`` are one address; ``Panel.local`` and ``panel.local`` + are one name. A single trailing dot is insignificant on both sides. + + Returns False for anything it cannot read -- a certificate with no SAN, a + malformed entry, an unparseable address. The caller's question is "may I + treat this name as bound to this certificate", and the honest answer to a + SAN that cannot be understood is no. + """ + candidate = _without_root_dot(host) + if not candidate: + return False + entries = list(_san_entries(peer_cert)) + try: + wanted = ipaddress.ip_address(candidate) + except ValueError: + return _names_dns(entries, candidate) + return _names_address(entries, wanted) + + +def _without_root_dot(name: str) -> str: + """Strip surrounding space and a single root dot, which is not significant.""" + stripped = name.strip() + return stripped[:-1] if stripped.endswith(".") else stripped + + +def _san_entries(peer_cert: Mapping[str, object]) -> Iterator[tuple[str, str]]: + """Yield the readable ``(kind, value)`` pairs of a certificate's SAN. + + Anything malformed is skipped rather than rejected wholesale, so one broken + entry cannot hide a good one sitting beside it. + """ + san = peer_cert.get("subjectAltName") + if not isinstance(san, tuple | list): + return + for entry in san: + if not isinstance(entry, tuple | list) or len(entry) != 2: + continue + kind, value = entry + if isinstance(kind, str) and isinstance(value, str): + yield kind, value + + +def _names_address(entries: list[tuple[str, str]], wanted: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool: + """Whether an ``IP Address`` entry denotes ``wanted``, compared as addresses.""" + for kind, value in entries: + if kind != "IP Address": + continue + try: + if ipaddress.ip_address(value.strip()) == wanted: + return True + except ValueError: + continue + return False + + +def _names_dns(entries: list[tuple[str, str]], candidate: str) -> bool: + """Whether a ``DNS`` entry equals ``candidate``, casefolded and exact.""" + folded = candidate.casefold() + for kind, value in entries: + if kind != "DNS": + continue + named = _without_root_dot(value) + if named and named.casefold() == folded: + return True + return False + + def ca_fingerprint(ca_pem: str) -> str: """SHA-256 over the certificate's DER bytes, lowercase hex, no separators. diff --git a/tests/test_public_api_unchanged.py b/tests/test_public_api_unchanged.py index eb628bd..1009ad6 100644 --- a/tests/test_public_api_unchanged.py +++ b/tests/test_public_api_unchanged.py @@ -95,6 +95,12 @@ # being reimplemented on the far side of the pin where they could drift. "build_panel_ssl_context", "ca_fingerprint", + # Added 2026-08-28: the hostname half of verification, needed by a caller + # that built a relaxed context to decide *which* host to talk to and must + # still establish the name binding. Here for the same reason as the two + # above -- a hand-written SAN matcher reimplemented on the far side of the + # pin is the drift this module exists to prevent. + "leaf_names_host", "delete_fqdn", "download_ca_cert", "get_fqdn", diff --git a/tests/test_ssl_context.py b/tests/test_ssl_context.py index 84ad1a0..bf0680d 100644 --- a/tests/test_ssl_context.py +++ b/tests/test_ssl_context.py @@ -20,7 +20,7 @@ import pytest -from span_panel_api._ssl import build_panel_ssl_context +from span_panel_api._ssl import build_panel_ssl_context, leaf_names_host cryptography = pytest.importorskip("cryptography", reason="cryptography needed to mint a test CA") @@ -232,3 +232,121 @@ def test_conventional_ca_still_loads(self) -> None: def test_malformed_pem_raises(self) -> None: with pytest.raises((ssl.SSLError, ValueError)): build_panel_ssl_context("-----BEGIN CERTIFICATE-----\nnot base64\n-----END CERTIFICATE-----\n") + + +class TestRelaxedHostnameContext: + """`check_hostname=False` separates "is this the panel" from "is this its name". + + The scenario throughout is the one a DHCP move produces: a leaf that names + `localhost` and nothing else, reached at `127.0.0.1`. The certificate is + perfectly good and signed by the pinned anchor; only the name it was dialled + by is absent from it. + """ + + def test_default_context_refuses_a_host_the_leaf_does_not_name(self) -> None: + """The premise. With hostname checking on, the two failures look alike.""" + ca_pem, leaf_pem, leaf_key_pem = _ca_and_leaf(with_aki=False) + ctx = build_panel_ssl_context(ca_pem) + + with _tls_server(leaf_pem, leaf_key_pem) as (host, port): + with socket.create_connection((host, port), timeout=5) as raw: + with pytest.raises(ssl.SSLCertVerificationError): + ctx.wrap_socket(raw, server_hostname="127.0.0.1") + + def test_relaxed_context_completes_and_yields_the_certificate(self) -> None: + """The same connection succeeds, and hands back the leaf to judge.""" + ca_pem, leaf_pem, leaf_key_pem = _ca_and_leaf(with_aki=False) + ctx = build_panel_ssl_context(ca_pem, check_hostname=False) + + with _tls_server(leaf_pem, leaf_key_pem) as (host, port): + with socket.create_connection((host, port), timeout=5) as raw: + with ctx.wrap_socket(raw, server_hostname="127.0.0.1") as tls: + peer = tls.getpeercert() + + assert peer is not None + # Chain-valid, and demonstrably not named by the address it was reached + # at -- the two facts the caller has to tell apart. + assert leaf_names_host(peer, "localhost") is True + assert leaf_names_host(peer, "127.0.0.1") is False + + def test_relaxed_context_still_rejects_an_untrusted_chain(self) -> None: + """The load-bearing guarantee: relaxing the name does not relax trust. + + An attacker without a key the pinned CA signed must still fail, or the + tri-state would be a hole rather than a classification. + """ + ca_pem, _, _ = _ca_and_leaf(with_aki=False) + _, other_leaf, other_key = _ca_and_leaf(with_aki=False) + ctx = build_panel_ssl_context(ca_pem, check_hostname=False) + + with _tls_server(other_leaf, other_key) as (host, port): + with socket.create_connection((host, port), timeout=5) as raw: + with pytest.raises(ssl.SSLCertVerificationError): + ctx.wrap_socket(raw, server_hostname="localhost") + + def test_relaxed_context_keeps_peer_verification_required(self) -> None: + ctx = build_panel_ssl_context(_self_signed_ca(with_aki=False), check_hostname=False) + + assert ctx.check_hostname is False + assert ctx.verify_mode is ssl.CERT_REQUIRED + assert not (ctx.verify_flags & ssl.VERIFY_X509_STRICT) + + def test_default_is_unchanged(self) -> None: + """Every existing caller keeps hostname checking without asking for it.""" + ctx = build_panel_ssl_context(_self_signed_ca(with_aki=False)) + assert ctx.check_hostname is True + + +class TestLeafNamesHost: + def test_exact_dns_match(self) -> None: + assert leaf_names_host({"subjectAltName": (("DNS", "panel.local"),)}, "panel.local") + + def test_dns_match_is_case_insensitive(self) -> None: + assert leaf_names_host({"subjectAltName": (("DNS", "Panel.LOCAL"),)}, "panel.local") + + def test_trailing_dot_is_insignificant_on_both_sides(self) -> None: + assert leaf_names_host({"subjectAltName": (("DNS", "panel.local."),)}, "panel.local") + assert leaf_names_host({"subjectAltName": (("DNS", "panel.local"),)}, "panel.local.") + + def test_wildcards_are_not_matched(self) -> None: + """A panel names literal addresses; a wildcard would be an anomaly.""" + assert not leaf_names_host({"subjectAltName": (("DNS", "*.local"),)}, "panel.local") + + def test_ipv4_match(self) -> None: + assert leaf_names_host({"subjectAltName": (("IP Address", "10.0.0.5"),)}, "10.0.0.5") + + def test_ipv6_compares_parsed_not_textually(self) -> None: + san = {"subjectAltName": (("IP Address", "0:0:0:0:0:0:0:1"),)} + assert leaf_names_host(san, "::1") + + def test_ip_and_dns_entries_are_not_interchangeable(self) -> None: + """Naming the string in a DNS entry must not authorise the address.""" + assert not leaf_names_host({"subjectAltName": (("DNS", "10.0.0.5"),)}, "10.0.0.5") + assert not leaf_names_host({"subjectAltName": (("IP Address", "10.0.0.5"),)}, "panel.local") + + def test_no_common_name_fallback(self) -> None: + """A SAN-less certificate names nothing, whatever its subject says.""" + peer = {"subject": ((("commonName", "panel.local"),),)} + assert not leaf_names_host(peer, "panel.local") + + def test_absent_or_unreadable_san_is_not_a_match(self) -> None: + assert not leaf_names_host({}, "panel.local") + assert not leaf_names_host({"subjectAltName": "not-a-sequence"}, "panel.local") + + def test_malformed_entries_are_skipped_not_fatal(self) -> None: + """A good entry beside a broken one still matches.""" + peer = { + "subjectAltName": ( + ("DNS",), + ("IP Address", "not-an-ip"), + (None, "panel.local"), + ("DNS", "panel.local"), + ) + } + assert leaf_names_host(peer, "panel.local") + + def test_unparseable_ip_entry_does_not_match(self) -> None: + assert not leaf_names_host({"subjectAltName": (("IP Address", "999.1.1.1"),)}, "10.0.0.5") + + def test_empty_host_matches_nothing(self) -> None: + assert not leaf_names_host({"subjectAltName": (("DNS", "panel.local"),)}, "") diff --git a/uv.lock b/uv.lock index eac7b5b..7416132 100644 --- a/uv.lock +++ b/uv.lock @@ -975,7 +975,7 @@ wheels = [ [[package]] name = "span-panel-api" -version = "3.1.1" +version = "3.2.0" source = { editable = "." } dependencies = [ { name = "httpx" },