From 7a6800cae48068671c8ee554e03f28574583addd Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:17:22 -0700 Subject: [PATCH 1/2] feat(ssl): separate chain validation from hostname verification A consumer pinned to a panel's CA gets one boolean for two different questions -- "is this the panel" and "does its certificate name this address" -- so a panel that moved to a new DHCP lease is indistinguishable from something impersonating one, and both are reported as a verification failure. `build_panel_ssl_context` gains `check_hostname`, which leaves the chain, signature and expiry checks intact and only stops asserting the binding between the certificate and the name used to dial it. An attacker without a key the pinned CA signed still cannot complete the handshake. `leaf_names_host` supplies the other half, hand-written against `getpeercert()` because `ssl.match_hostname` was removed in 3.12. It is stricter than that function was: no wildcards, no `commonName` fallback, DNS and IP entries that never substitute for one another, addresses compared parsed and names casefolded. Both live here rather than on the consumer side for the reason the module docstring already gives about the fingerprint: a security primitive with two implementations is a defect waiting for a firmware upgrade to find it. --- CHANGELOG.md | 10 +++ pyproject.toml | 2 +- src/span_panel_api/__init__.py | 5 +- src/span_panel_api/_ssl.py | 116 ++++++++++++++++++++++++++-- tests/test_public_api_unchanged.py | 6 ++ tests/test_ssl_context.py | 120 ++++++++++++++++++++++++++++- uv.lock | 2 +- 7 files changed, 249 insertions(+), 12 deletions(-) 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..05a4663 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 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,85 @@ 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. + """ + san = peer_cert.get("subjectAltName") + if not isinstance(san, tuple | list): + return False + + candidate = host.strip() + if candidate.endswith("."): + candidate = candidate[:-1] + if not candidate: + return False + + try: + wanted_ip: ipaddress.IPv4Address | ipaddress.IPv6Address | None = ipaddress.ip_address(candidate) + except ValueError: + wanted_ip = None + + for entry in san: + if not isinstance(entry, tuple | list) or len(entry) != 2: + continue + kind, value = entry + if not isinstance(kind, str) or not isinstance(value, str): + continue + if wanted_ip is not None: + if kind != "IP Address": + continue + try: + if ipaddress.ip_address(value.strip()) == wanted_ip: + return True + except ValueError: + continue + else: + if kind != "DNS": + continue + named = value.strip() + if named.endswith("."): + named = named[:-1] + if named and named.casefold() == candidate.casefold(): + 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" }, From 28d9bf07f873365b479cb932f9a57f29322f04d4 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:08:14 -0700 Subject: [PATCH 2/2] refactor(ssl): decompose leaf_names_host into its three rules CodeFactor flagged the matcher as a complex method, and it was: SAN extraction, host normalisation, address parsing and two matching rules in one body, with the branch for which rule applies interleaved through the loop. Split into `_san_entries`, `_names_address` and `_names_dns`, leaving `leaf_names_host` as the dispatcher it should have been -- normalise, then ask exactly one of the two questions. Behaviour is unchanged and the rules now read one per function, which for security code is the point. --- src/span_panel_api/_ssl.py | 73 ++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/src/span_panel_api/_ssl.py b/src/span_panel_api/_ssl.py index 05a4663..e070e65 100644 --- a/src/span_panel_api/_ssl.py +++ b/src/span_panel_api/_ssl.py @@ -21,7 +21,7 @@ import base64 import binascii -from collections.abc import Mapping +from collections.abc import Iterator, Mapping import hashlib import ipaddress import ssl @@ -116,43 +116,62 @@ def leaf_names_host(peer_cert: Mapping[str, object], host: str) -> bool: treat this name as bound to this certificate", and the honest answer to a SAN that cannot be understood is no. """ - san = peer_cert.get("subjectAltName") - if not isinstance(san, tuple | list): - return False - - candidate = host.strip() - if candidate.endswith("."): - candidate = candidate[:-1] + candidate = _without_root_dot(host) if not candidate: return False - + entries = list(_san_entries(peer_cert)) try: - wanted_ip: ipaddress.IPv4Address | ipaddress.IPv6Address | None = ipaddress.ip_address(candidate) + wanted = ipaddress.ip_address(candidate) except ValueError: - wanted_ip = None + 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 not isinstance(kind, str) or not isinstance(value, str): + 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 - if wanted_ip is not None: - if kind != "IP Address": - continue - try: - if ipaddress.ip_address(value.strip()) == wanted_ip: - return True - except ValueError: - continue - else: - if kind != "DNS": - continue - named = value.strip() - if named.endswith("."): - named = named[:-1] - if named and named.casefold() == candidate.casefold(): + 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