From bbbac610ed88b5433fbf1b2bfbe4eb0a1dee52c9 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Mon, 31 Aug 2026 16:03:31 -0700 Subject: [PATCH] security(rest): put the schema fetch behind the pinned CA, on its own port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `panel_http_port` served two transports with opposite security properties: the bridge's CA download, plaintext by design because it fetches the very anchor everything else is checked against, and the client's schema fetch, which should ride the pinned HTTPS transport whenever the caller holds one. One parameter fed both, so a consumer that pinned a CA could not move its schema fetch to HTTPS without pointing the CA download at a TLS port it speaks plaintext to (SpanPanel/span#264). `SpanMqttClient` takes `panel_https_port`, and both schema fetches — connect and every redispatch refetch — go HTTPS on it whenever an `ssl_context` is supplied; naming the port without an anchor is refused rather than left silently plaintext. `create_span_client` routes its one `port` into the slot matching its transport, and documents the diagnostic-CA-read corollary. A verification failure gets a name: `SpanPanelTLSVerificationError`, a subclass of `SpanPanelConnectionError` so every existing except clause keeps its meaning, raised only when `ssl.SSLCertVerificationError` is in the cause chain because ambiguous evidence must not look terminal. The redispatch retry loop leaves it to raise — it cannot succeed on a later attempt under the same anchor — and its wrapper logs it as what it is instead of blaming a slow boot. The CA download itself no longer emits the plaintext-transport warning and no longer spends the once-per-host slot: the fetch of the anchor is unverifiable by construction, carries no credential in either direction, and every caller states its own trust posture — the warning as it stood named credentials that call never carries, which is the log line the issue reported. Every other bootstrap call warns exactly as before. --- CHANGELOG.md | 21 ++ pyproject.toml | 2 +- src/span_panel_api/__init__.py | 6 + src/span_panel_api/_http.py | 58 ++++- src/span_panel_api/auth.py | 4 +- src/span_panel_api/exceptions.py | 19 ++ src/span_panel_api/factory.py | 17 +- src/span_panel_api/mqtt/client.py | 58 ++++- tests/test_plaintext_warning.py | 48 +++- tests/test_public_api_unchanged.py | 5 + tests/test_schema_fetch_transport_split.py | 276 +++++++++++++++++++++ uv.lock | 2 +- 12 files changed, 499 insertions(+), 17 deletions(-) create mode 100644 tests/test_schema_fetch_transport_split.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e2201b..eee383e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,27 @@ 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.4.0] + +A consumer that pinned the panel's CA could not put its schema fetches behind that pin, because the one port `SpanMqttClient` took served two transports with opposite security properties — the schema fetch, which should ride the pinned HTTPS transport, and +the bridge's CA download, which is plaintext by design because it fetches the very anchor everything else is checked against. This release splits them. + +### Added + +- **`SpanMqttClient` takes `panel_https_port`**, and its schema fetches — the one at connect and every redispatch refetch — move to HTTPS on that port whenever an `ssl_context` is supplied, leaving the bridge's deliberately-plaintext CA fetches on + `panel_http_port` exactly where they were. Naming the HTTPS port without an anchor is refused rather than left silently plaintext, for the same reason `_build_url` refuses port 80 with a context. +- **`SpanPanelTLSVerificationError` names a bootstrap REST call that failed certificate verification**, as a subclass of `SpanPanelConnectionError` so every existing except clause keeps its meaning — raised only when an `ssl.SSLCertVerificationError` is in + the cause chain, because ambiguous evidence must not look terminal, and exported so a consumer that fails closed on an untrusted certificate can catch it before the parent. + +### Changed + +- **`create_span_client`'s `port` lands in the slot its transport needs**: with an `ssl_context` it was already read as the HTTPS port by every REST call the factory makes, so it now reaches the client's HTTPS slot and the CA download takes the plaintext + default, instead of the TLS port being handed to a plaintext fetch. +- **The redispatch schema refetch no longer retries a certificate-verification failure**, which cannot succeed on a later attempt under the same anchor; it is left to raise and logged once per trigger, instead of a background task fetching every thirty + seconds forever while the log blames a slow boot. +- **The CA download no longer emits the plaintext-transport warning**, because the fetch of the anchor itself is unverifiable by construction and carries no credential — its trust posture is stated by each caller in its own voice, and the warning as it + stood named credentials that call never carries. Every other bootstrap call still warns, and the CA download no longer spends the once-per-host slot a genuinely plaintext call needs later. + ## [3.3.0] A pinned panel that has moved is no longer reported the same way as a panel whose clock reset, so a consumer can put the remedy in front of a user instead of retrying in silence. diff --git a/pyproject.toml b/pyproject.toml index 128c33a..dab4056 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "span-panel-api" -version = "3.3.0" +version = "3.4.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 9670691..7a812fd 100644 --- a/src/span_panel_api/__init__.py +++ b/src/span_panel_api/__init__.py @@ -30,6 +30,7 @@ SpanPanelServerError, SpanPanelStaleDataError, SpanPanelTimeoutError, + SpanPanelTLSVerificationError, SpanPanelValidationError, ) from .factory import create_span_client @@ -200,6 +201,11 @@ "SpanPanelError", "SpanPanelServerError", "SpanPanelStaleDataError", + # Added 2026-08-31 (3.4.0): a bootstrap REST call that failed verification + # rather than connection. A subclass of SpanPanelConnectionError, so every + # existing except clause keeps its meaning; a consumer that fails closed on + # an untrusted certificate catches this one before the parent. + "SpanPanelTLSVerificationError", "SpanPanelTimeoutError", "SpanPanelValidationError", ] diff --git a/src/span_panel_api/_http.py b/src/span_panel_api/_http.py index bed455e..20b90bb 100644 --- a/src/span_panel_api/_http.py +++ b/src/span_panel_api/_http.py @@ -12,7 +12,13 @@ import httpx -from .exceptions import SpanPanelAPIError, SpanPanelConnectionError, SpanPanelTimeoutError, SpanPanelValidationError +from .exceptions import ( + SpanPanelAPIError, + SpanPanelConnectionError, + SpanPanelTimeoutError, + SpanPanelTLSVerificationError, + SpanPanelValidationError, +) _LOGGER = logging.getLogger(__name__) @@ -28,6 +34,10 @@ #: each, so the two cannot drift apart the way their parsers had. V2_STATUS_PATH = "/api/v2/status" +#: The one bootstrap path exempt from the plaintext warning, named here because +#: the transport is what grants the exemption. See `_warn_plaintext_transport`. +CA_CERT_PATH = "/api/v2/certificate/ca" + #: The verbs the bootstrap API uses. Spelled as a `Literal` rather than passed #: through to `client.request()` so the dispatch below stays exhaustive and each #: call still reaches the named httpx method. @@ -161,9 +171,23 @@ def _reset_plaintext_warnings() -> None: _warned_plaintext_hosts.clear() -def _warn_plaintext_transport(host: str, ssl_context: ssl.SSLContext | None) -> None: +def _warn_plaintext_transport(host: str, path: str, ssl_context: ssl.SSLContext | None) -> None: """Say out loud, once per panel, that its bootstrap traffic is not encrypted. + **The CA download is exempt, and does not claim the once-per-host slot.** + The warning exists so an operator can tell a security property is off when + it could be on, and for that endpoint there is no "on": verifying the fetch + of the anchor would require the anchor being fetched, an unverified-TLS + wrapping is readable and forgeable by the same active on-path attacker, and + the payload is a public certificate carrying no credential in either + direction — its authenticity control is the leaf check callers run *after* + the fetch. Each caller also states its own trust posture in its own voice: + the bridge's unpinned warning, a config flow's fingerprint confirmation, a + consumer's trust-on-first-use log. Warning here anyway named credentials the + call never carries, which is the line issue span#264 reported. Not marking + the host matters as much as not warning: a pinned consumer's diagnostic + re-read must not spend the slot a genuinely plaintext call needs later. + In the same voice as the MQTT bridge's unpinned-CA warning, and for the same reason: a security property that is off by default is only a decision if the operator can tell it is off. ``ssl_context=None`` puts the request on @@ -190,6 +214,8 @@ def _warn_plaintext_transport(host: str, ssl_context: ssl.SSLContext | None) -> """ if ssl_context is not None: return + if path == CA_CERT_PATH: + return if host in _warned_plaintext_hosts: return _warned_plaintext_hosts.add(host) @@ -299,7 +325,7 @@ async def _request( caller that supplied it. """ url = _build_url(host, port, path, ssl_context) - _warn_plaintext_transport(host, ssl_context) + _warn_plaintext_transport(host, path, ssl_context) try: async with _get_client(httpx_client, timeout, ssl_context) as client: match method: @@ -314,5 +340,31 @@ async def _request( except httpx.TimeoutException as exc: raise SpanPanelTimeoutError(f"Timed out connecting to {host}") from exc except httpx.TransportError as exc: + if _is_certificate_verification_failure(exc): + raise SpanPanelTLSVerificationError( + f"{host} answered {path} with a certificate the supplied trust anchor rejects: {exc}" + ) from exc raise SpanPanelConnectionError(f"Cannot reach panel at {host}: {exc}") from exc return _Reply(host=host, endpoint=path, response=response) + + +def _is_certificate_verification_failure(exc: BaseException) -> bool: + """Whether this transport failure is demonstrably about certificate verification. + + httpx wraps the underlying ``ssl.SSLCertVerificationError`` rather than + exposing it, so the evidence lives in the cause chain. Only that exact class + counts: a handshake that dies any other way -- a reset, a protocol mismatch, + an alert from a peer that is not TLS at all -- is indistinguishable from a + panel mid-reboot, and calling ambiguous evidence "verification failed" would + let a transient outage masquerade as the one failure consumers treat as + terminal. The walk is capped because ``__context__`` chains are + caller-assembled and nothing here should trust one to be finite. + """ + seen = 0 + current: BaseException | None = exc + while current is not None and seen < 10: + if isinstance(current, ssl.SSLCertVerificationError): + return True + current = current.__cause__ if current.__cause__ is not None else current.__context__ + seen += 1 + return False diff --git a/src/span_panel_api/auth.py b/src/span_panel_api/auth.py index a7dfc90..14f9efb 100644 --- a/src/span_panel_api/auth.py +++ b/src/span_panel_api/auth.py @@ -18,7 +18,7 @@ import httpx -from ._http import V2_STATUS_PATH, _request +from ._http import CA_CERT_PATH, V2_STATUS_PATH, _request from .exceptions import SpanPanelAPIError, SpanPanelAuthError, SpanPanelServerError from .models import HomieSchemaTypes, V2AuthResponse, V2HomieSchema, V2StatusInfo @@ -373,7 +373,7 @@ async def download_ca_cert( "GET", host, port, - "/api/v2/certificate/ca", + CA_CERT_PATH, timeout=timeout, httpx_client=httpx_client, ssl_context=ssl_context, diff --git a/src/span_panel_api/exceptions.py b/src/span_panel_api/exceptions.py index 9e63049..a80b921 100644 --- a/src/span_panel_api/exceptions.py +++ b/src/span_panel_api/exceptions.py @@ -13,6 +13,25 @@ class SpanPanelConnectionError(SpanPanelError): """Connection to SPAN panel failed.""" +class SpanPanelTLSVerificationError(SpanPanelConnectionError): + """Something answered a bootstrap REST call with a certificate the supplied anchor rejects. + + A subclass of `SpanPanelConnectionError` on purpose: every consumer that + catches the parent and retries keeps doing exactly what it did, because + nothing raised this before an `ssl_context` reached the bootstrap calls. The + subclass exists for the consumer that wants the opposite of a retry — a + verification failure is not "the panel is not up yet", it is "whatever is up + does not hold a key the pin signs", and retrying that is waiting to succeed + against whatever is answering. Catch this before the parent to fail closed. + + Raised only when the failure is demonstrably about verification — an + `ssl.SSLCertVerificationError` in the cause chain. Every other transport + failure, TLS handshakes that die for other reasons included, stays a plain + `SpanPanelConnectionError`, because ambiguous evidence must not look + terminal. + """ + + class SpanPanelTimeoutError(SpanPanelError): """Request timed out.""" diff --git a/src/span_panel_api/factory.py b/src/span_panel_api/factory.py index 5ab4464..7ec907f 100644 --- a/src/span_panel_api/factory.py +++ b/src/span_panel_api/factory.py @@ -45,7 +45,14 @@ async def create_span_client( serial_number: Panel serial number (extracted from detection/registration if omitted). port: Port of the panel bootstrap API used for registration, detection and the schema fetch. ``None`` takes the scheme default -- 80 plaintext, 443 with a - context. + context. It reaches the constructed client in the slot matching its + transport: ``panel_https_port`` with a context, ``panel_http_port`` without + -- so a pinned client's plaintext CA fetches never dial the TLS port. + The corollary is stated rather than hidden: under a context the bridge's + diagnostic CA re-read takes the plaintext default, port 80. A pinned + caller whose panel serves plaintext on a nonstandard port has no way to + say so through this factory; construct ``SpanMqttClient`` directly and + pass both ports. httpx_client: Optional shared ``httpx.AsyncClient``, used for every request this makes and handed to the client it builds. Not closed here; its timeouts and limits are the caller's, which is why the per-call ``timeout`` defaults are @@ -115,11 +122,17 @@ async def create_span_client( # `adapters` — none of it is safe to run on an event loop. adapter_cls = await asyncio.to_thread(resolve_adapter, adapter_key, dispatch_reason) + # `port` follows the transport the factory's own REST calls just used it + # for: with an ssl_context it was the HTTPS port (`_build_url` accepts no + # other reading), so it lands in the HTTPS slot and the bridge's + # deliberately-plaintext CA download keeps its own default. Without one it + # is the plaintext port, exactly as before. client = SpanMqttClient( host, serial_number, mqtt_config, - panel_http_port=port, + panel_http_port=None if ssl_context is not None else port, + panel_https_port=port if ssl_context is not None else None, adapter_factory=adapter_cls, data_model_version=schema.data_model_version, schema_dispatch_reason=dispatch_reason, diff --git a/src/span_panel_api/mqtt/client.py b/src/span_panel_api/mqtt/client.py index b2f4e86..fdfba12 100644 --- a/src/span_panel_api/mqtt/client.py +++ b/src/span_panel_api/mqtt/client.py @@ -33,6 +33,8 @@ SpanPanelServerError, SpanPanelStaleDataError, SpanPanelTimeoutError, + SpanPanelTLSVerificationError, + SpanPanelValidationError, ) from ..models import AdoptedProperty, ControlTarget, FieldMetadata, HomieSchemaTypes, SpanPanelSnapshot, V2HomieSchema from ..protocol import PanelCapability, SchemaAdapter @@ -131,6 +133,7 @@ def __init__( broker_config: MqttClientConfig, snapshot_interval: float = 1.0, panel_http_port: int | None = None, + panel_https_port: int | None = None, adapter_factory: Callable[[str, V2HomieSchema], SchemaAdapter] | None = None, data_model_version: str | None = None, schema_dispatch_reason: str | None = None, @@ -139,11 +142,29 @@ def __init__( ssl_context: ssl.SSLContext | None = None, control_deadlines: ControlDeadlines | None = None, ) -> None: + if panel_https_port is not None and ssl_context is None: + # A TLS port with nothing to verify against is a decision nobody + # made: accepted silently, the schema fetch would run plaintext HTTP + # against a port the caller believes is TLS. The same misreading + # `_build_url` refuses for port 80 with a context, from the other + # direction. + raise SpanPanelValidationError( + f"panel_https_port={panel_https_port} was passed without an ssl_context for {host}. " + "Supply the pinned CA as ssl_context, or omit panel_https_port to stay on plaintext HTTP." + ) self._host = host self._serial_number = serial_number self._broker_config = broker_config self._snapshot_interval = snapshot_interval + # Two ports because they serve transports with opposite security + # properties. `panel_http_port` is the plaintext one, and it belongs to + # the bridge: the CA download is unauthenticated by construction — it + # fetches the very anchor everything else is checked against — so it + # never follows the pin. `panel_https_port` carries this client's own + # schema fetches once an `ssl_context` anchors them; `None` with a + # context means `_build_url`'s TLS default, 443. self._panel_http_port = panel_http_port + self._panel_https_port = panel_https_port self._adapter_factory = adapter_factory # Shared by the caller, owned by the caller: never closed here, and its # policy -- timeouts, limits, headers -- is whatever the caller set. That @@ -222,10 +243,18 @@ async def _fetch_schema(self) -> V2HomieSchema: four arguments spelled out separately, and adding the trust anchor to one and not the other is exactly how a session ends up bootstrapping over HTTPS and refetching over HTTP for the rest of its life. One call site. + + The port follows the transport. With an anchor the fetch is HTTPS and + takes ``panel_https_port``; without one it is plaintext and takes + ``panel_http_port``, exactly as it always did. Handing the HTTP port to + a TLS call is the combination ``_build_url`` refuses, and handing the + TLS port to the plaintext one is the constructor refusal -- so by the + time this runs, the pairing is already known good. """ + port = self._panel_https_port if self._ssl_context is not None else self._panel_http_port return await get_homie_schema( self._host, - port=self._panel_http_port, + port=port, httpx_client=self._httpx_client, ssl_context=self._ssl_context, ) @@ -1465,6 +1494,16 @@ async def _fetch_schema_with_retry(self) -> V2HomieSchema | None: while True: try: return await self._fetch_schema() + except SpanPanelTLSVerificationError: + # Before its parent, which the next clause would retry forever. + # A verification failure cannot succeed on a later attempt -- + # the anchor is fixed for the session -- so it is precisely the + # "error that is not the panel still coming up" this loop's + # contract leaves to raise. The redispatch wrapper logs it once + # per trigger, and escalation belongs to the MQTT side: a + # rotated CA surfaces through the bridge's own diagnosis and + # fatal-error channel, which reconnects share with this trigger. + raise except ( SpanPanelConnectionError, SpanPanelTimeoutError, @@ -1516,6 +1555,23 @@ async def _redispatch_if_generation_changed(self) -> None: """ try: await self._redispatch_once() + except SpanPanelTLSVerificationError: + # Before the catch-all, whose text blames a slow boot. This is the + # one refetch failure that is not one: the schema endpoint answered + # with a certificate the session's anchor rejects, which cannot fix + # itself on a later attempt and must not steer a user investigating + # an interception toward waiting. Still non-fatal here for the + # catch-all's reason -- nothing may escape a fire-and-forget task -- + # and the next reconnect edge re-arms the attempt. + _LOGGER.error( + "Could not follow the panel's schema-generation change: the schema refetch " + "failed certificate verification against the pinned CA, so the %r parser is " + "unchanged. If the panel's CA rotated with the firmware, the broker " + "connection will surface it; otherwise check what answers the panel's " + "HTTPS port.", + self._data_model_version, + exc_info=True, + ) except Exception: # pylint: disable=broad-exception-caught # Nothing may escape here. This runs as a fire-and-forget task, so an # escaping exception becomes "Task exception was never retrieved" in diff --git a/tests/test_plaintext_warning.py b/tests/test_plaintext_warning.py index 782d371..5c33e92 100644 --- a/tests/test_plaintext_warning.py +++ b/tests/test_plaintext_warning.py @@ -135,13 +135,23 @@ async def test_two_calls_on_one_client_warn_once(self, caplog: pytest.LogCapture assert len(_warnings(caplog)) == 1 @pytest.mark.asyncio - async def test_repeated_ca_fetches_on_fresh_clients_warn_once(self, caplog: pytest.LogCaptureFixture) -> None: - """The reconnect path, and the reason this is scoped to the panel. - - `download_ca_cert` is called on every MQTT reconnect with no injected - client, so each call builds its own. Anything keyed on the client object - would warn once per reconnect — exactly the log the bridge's own - once-per-bridge warning exists to avoid. + async def test_the_ca_download_never_warns(self, caplog: pytest.LogCaptureFixture) -> None: + """Reversed in 3.4.0, deliberately: this endpoint is unverifiable by construction. + + The warning exists so an operator can tell a security property is *off* + when it could be on, and for the first fetch of the anchor itself there + is no "on": any verification would require the anchor being fetched, an + unverified-TLS wrapping would be readable and forgeable by the same + active on-path attacker, and the payload is a public certificate with no + credential in either direction — the authenticity control is the leaf + check its callers run *after* the fetch. Every caller also already says + so in its own voice: the bridge's once-per-bridge unpinned warning, the + config flow's fingerprint confirmation, and the deferred pin's + trust-on-first-use log line. Until 3.3.x this endpoint warned like the + rest, naming credentials it never carries; that line is what issue + span#264 reported. A caller that already holds the anchor and wants a + verified second copy passes ``ssl_context``, and no warning was ever in + question there. """ with patch("span_panel_api._http.httpx.AsyncClient") as mock_cls: dedicated = AsyncMock() @@ -154,6 +164,30 @@ async def test_repeated_ca_fetches_on_fresh_clients_warn_once(self, caplog: pyte for _ in range(5): await download_ca_cert(HOST) + assert len(_warnings(caplog)) == 0 + + @pytest.mark.asyncio + async def test_the_ca_download_does_not_swallow_a_later_warning(self, caplog: pytest.LogCaptureFixture) -> None: + """Skipping the warning must not mark the host as already warned. + + The diagnostic CA re-read runs on a *pinned* entry whose other calls are + HTTPS; if it claimed the once-per-host slot, a genuinely plaintext call + made later — a reauth on an entry that lost its pin — would say nothing. + """ + answer = _json_response({"ebusBrokerPassword": "new-pass"}, method="PUT") + # Built before the patch below replaces `httpx.AsyncClient`; a spec + # against the patched class is a spec against a Mock, which mock refuses. + injected = _client("put", answer) + with patch("span_panel_api._http.httpx.AsyncClient") as mock_cls: + dedicated = AsyncMock() + dedicated.__aenter__ = AsyncMock(return_value=dedicated) + dedicated.__aexit__ = AsyncMock(return_value=False) + dedicated.get = AsyncMock(return_value=_text_response(PEM)) + mock_cls.return_value = dedicated + with caplog.at_level(logging.WARNING): + await download_ca_cert(HOST) + await regenerate_passphrase(HOST, "jwt", httpx_client=injected) + assert len(_warnings(caplog)) == 1 @pytest.mark.asyncio diff --git a/tests/test_public_api_unchanged.py b/tests/test_public_api_unchanged.py index 9b11b4c..56ffcde 100644 --- a/tests/test_public_api_unchanged.py +++ b/tests/test_public_api_unchanged.py @@ -155,6 +155,11 @@ "SpanPanelError", "SpanPanelServerError", "SpanPanelStaleDataError", + # Added 2026-08-31 (3.4.0): a bootstrap REST call that failed *verification* + # rather than connection. A subclass of SpanPanelConnectionError, so every + # existing except clause keeps its meaning; a consumer that wants to fail + # closed on an untrusted certificate catches this one first. + "SpanPanelTLSVerificationError", "SpanPanelTimeoutError", "SpanPanelValidationError", } diff --git a/tests/test_schema_fetch_transport_split.py b/tests/test_schema_fetch_transport_split.py new file mode 100644 index 0000000..c9ffc7d --- /dev/null +++ b/tests/test_schema_fetch_transport_split.py @@ -0,0 +1,276 @@ +"""The schema fetch and the CA download need different ports, and had one. + +`panel_http_port` serves two transports with opposite security properties: the +bridge's CA download, which is plaintext by design because it fetches the very +anchor everything else is checked against, and the client's schema fetch, which +should ride the pinned HTTPS transport whenever the caller holds one. One +parameter fed both, so a consumer that pinned a CA could not move its schema +fetch to HTTPS without simultaneously pointing the CA download at a TLS port it +speaks plaintext to. + +The split: `panel_https_port` carries the schema fetch when an `ssl_context` is +supplied, and `panel_http_port` keeps the bridge's deliberately-plaintext CA +fetches exactly where they were. + +Alongside it, a TLS verification failure on a bootstrap REST call gets its own +exception class. A consumer that fails closed on an untrusted certificate needs +to tell "something answered with a certificate the pin does not sign" apart from +"nothing answered" — the first is terminal and needs a person, the second clears +itself when the panel finishes rebooting. +""" + +from __future__ import annotations + +import asyncio +import contextlib +import logging +import ssl +from unittest.mock import AsyncMock, MagicMock, patch + +import httpx +import pytest + +from span_panel_api._http import _reset_plaintext_warnings +from span_panel_api.auth import get_homie_schema +from span_panel_api.exceptions import ( + SpanPanelConnectionError, + SpanPanelTLSVerificationError, + SpanPanelValidationError, +) +from span_panel_api.mqtt import MqttClientConfig +from span_panel_api.mqtt.client import SpanMqttClient + +from conftest import flat_schema + +HOST = "192.168.1.1" +SERIAL = "sp3-242424-001" + + +@pytest.fixture(autouse=True) +def _fresh_warning_state() -> None: + """Keep the once-per-host warning set from leaking between tests.""" + _reset_plaintext_warnings() + + +@pytest.fixture +def context() -> ssl.SSLContext: + """Any context object will do — nothing here completes a handshake.""" + return ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + + +class _Schema: + def __init__(self, version: str | None) -> None: + self.data_model_version = version + + +def _client(**kwargs: object) -> SpanMqttClient: + return SpanMqttClient( + host=HOST, + serial_number=SERIAL, + broker_config=MqttClientConfig(broker_host="broker.local", username="u", password="p"), + **kwargs, # type: ignore[arg-type] + ) + + +async def _run_connect_fetch(client: SpanMqttClient, fetch: AsyncMock) -> None: + """Drive connect() far enough to make its schema fetch, and no further.""" + with ( + patch("span_panel_api.mqtt.client.get_homie_schema", fetch), + patch.object(client, "_preload_adapter", AsyncMock()), + patch.object(client, "_build_adapter", MagicMock()), + ): + with contextlib.suppress(Exception): + await client.connect() + assert fetch.await_count >= 1 + + +class TestSchemaFetchPortSelection: + @pytest.mark.asyncio + async def test_a_pinned_client_fetches_the_schema_on_the_https_port(self, context: ssl.SSLContext) -> None: + client = _client(panel_http_port=80, panel_https_port=8443, ssl_context=context) + fetch = AsyncMock(return_value=_Schema("1.0")) + await _run_connect_fetch(client, fetch) + assert fetch.await_args.kwargs["port"] == 8443 + assert fetch.await_args.kwargs["ssl_context"] is context + + @pytest.mark.asyncio + async def test_a_pinned_client_without_a_named_port_takes_the_scheme_default(self, context: ssl.SSLContext) -> None: + """`None` reaches `_build_url`, whose default for a TLS call is 443.""" + client = _client(panel_http_port=80, ssl_context=context) + fetch = AsyncMock(return_value=_Schema("1.0")) + await _run_connect_fetch(client, fetch) + assert fetch.await_args.kwargs["port"] is None + assert fetch.await_args.kwargs["ssl_context"] is context + + @pytest.mark.asyncio + async def test_an_unpinned_client_keeps_the_plaintext_port(self) -> None: + client = _client(panel_http_port=8080) + fetch = AsyncMock(return_value=_Schema("1.0")) + await _run_connect_fetch(client, fetch) + assert fetch.await_args.kwargs["port"] == 8080 + assert fetch.await_args.kwargs["ssl_context"] is None + + @pytest.mark.asyncio + async def test_the_upgrade_refetch_uses_the_same_transport(self, context: ssl.SSLContext) -> None: + """One call site for both fetches is the point; prove the retry loop kept it.""" + client = _client(panel_http_port=80, panel_https_port=8443, ssl_context=context) + client._loop = asyncio.get_running_loop() + fetch = AsyncMock(return_value=_Schema("1.0")) + with patch("span_panel_api.mqtt.client.get_homie_schema", fetch): + assert await client._fetch_schema_with_retry() is not None + assert fetch.await_args.kwargs["port"] == 8443 + assert fetch.await_args.kwargs["ssl_context"] is context + + def test_an_https_port_without_an_anchor_is_refused(self) -> None: + """A TLS port with nothing to verify against is a decision nobody made. + + Accepting it silently would put the schema fetch on plaintext HTTP at a + port the caller believes is TLS — the same misreading `_build_url` + refuses for port 80 with a context, from the other direction. + """ + with pytest.raises(SpanPanelValidationError): + _client(panel_https_port=8443) + + @pytest.mark.asyncio + async def test_the_bridge_keeps_the_plaintext_port_when_the_schema_fetch_moves(self, context: ssl.SSLContext) -> None: + """The CA download is plaintext by design and must not follow the pin.""" + client = _client(panel_http_port=8080, panel_https_port=8443, ssl_context=context) + # A real schema, because this test needs connect() to get all the way + # to bridge construction rather than stopping at the fetch. + fetch = AsyncMock(return_value=flat_schema(32)) + bridge_cls = MagicMock() + with ( + patch("span_panel_api.mqtt.client.get_homie_schema", fetch), + patch("span_panel_api.mqtt.client.AsyncMqttBridge", bridge_cls), + patch.object(client, "_preload_adapter", AsyncMock()), + patch.object(client, "_build_adapter", MagicMock()), + ): + with contextlib.suppress(Exception): + await client.connect() + assert bridge_cls.call_args is not None + assert bridge_cls.call_args.kwargs["panel_http_port"] == 8080 + + +class TestFactoryPortRouting: + """`create_span_client`'s one `port` lands in the slot its transport needs. + + The factory's REST calls already read `port` as "the HTTPS port" when an + `ssl_context` rides along — `_build_url` refuses anything else — but it then + handed the same number to `panel_http_port`, pointing the bridge's + deliberately-plaintext CA download at a TLS port. + """ + + @pytest.mark.asyncio + async def test_a_pinned_factory_call_routes_its_port_to_the_https_slot(self, context: ssl.SSLContext) -> None: + from span_panel_api.adapters import _reset_adapter_cache + from span_panel_api.factory import create_span_client + + _reset_adapter_cache() + config = MqttClientConfig(broker_host="broker.local", username="u", password="p") + with ( + patch("span_panel_api.factory.SpanMqttClient") as mock_cls, + patch("span_panel_api.factory.get_homie_schema", return_value=_Schema(None)), + ): + mock_cls.return_value.connect = AsyncMock() + await create_span_client( + HOST, + mqtt_config=config, + serial_number=SERIAL, + port=8443, + ssl_context=context, + ) + kwargs = mock_cls.call_args.kwargs + assert kwargs["panel_https_port"] == 8443 + assert kwargs["ssl_context"] is context + # The CA download takes the plaintext default rather than the TLS port. + assert kwargs["panel_http_port"] is None + + @pytest.mark.asyncio + async def test_an_unpinned_factory_call_keeps_its_port_on_the_plaintext_slot(self) -> None: + from span_panel_api.adapters import _reset_adapter_cache + from span_panel_api.factory import create_span_client + + _reset_adapter_cache() + config = MqttClientConfig(broker_host="broker.local", username="u", password="p") + with ( + patch("span_panel_api.factory.SpanMqttClient") as mock_cls, + patch("span_panel_api.factory.get_homie_schema", return_value=_Schema(None)), + ): + mock_cls.return_value.connect = AsyncMock() + await create_span_client(HOST, mqtt_config=config, serial_number=SERIAL, port=8080) + kwargs = mock_cls.call_args.kwargs + assert kwargs["panel_http_port"] == 8080 + assert kwargs.get("panel_https_port") is None + + +class TestRedispatchRetryDoesNotRetryVerificationFailures: + """The retry loop's contract is 'the panel still coming up', and this is not that. + + A verification failure cannot succeed on a later attempt under the same + context — the anchor is fixed for the session — so retrying it is a + background task GETting the panel every thirty seconds forever while the + log blames a slow boot. Left to raise, the redispatch wrapper logs the + failure once per trigger, and the MQTT side owns the escalation: a rotated + CA surfaces through the bridge's own diagnosis and fatal-error channel. + """ + + @pytest.mark.asyncio + async def test_a_verification_failure_raises_out_of_the_retry_loop(self, context: ssl.SSLContext) -> None: + client = _client(panel_https_port=8443, ssl_context=context) + client._loop = asyncio.get_running_loop() + fetch = AsyncMock(side_effect=SpanPanelTLSVerificationError("cert rejected by the pin")) + with patch("span_panel_api.mqtt.client.get_homie_schema", fetch): + # wait_for, because the defect this guards against is an infinite + # retry loop with real backoff sleeps: without it, a regression + # hangs the suite instead of failing it. + with pytest.raises(SpanPanelTLSVerificationError): + await asyncio.wait_for(client._fetch_schema_with_retry(), timeout=5) + # One attempt, no backoff loop: the failure is not retried at all. + assert fetch.await_count == 1 + + @pytest.mark.asyncio + async def test_the_escaping_failure_is_logged_as_what_it_is( + self, context: ssl.SSLContext, caplog: pytest.LogCaptureFixture + ) -> None: + """The wrapper's catch-all blamed a slow boot; verification is not that. + + 'Reload once the panel is fully back up' is accidentally workable advice + — a reload does surface the repair — but the sentence points at the + wrong cause, and the wrong cause is the one a user investigating an + interception must not be steered away from. + """ + client = _client(panel_https_port=8443, ssl_context=context) + client._loop = asyncio.get_running_loop() + failure = SpanPanelTLSVerificationError("cert rejected by the pin") + with patch.object(client, "_redispatch_once", AsyncMock(side_effect=failure)): + with caplog.at_level(logging.ERROR): + await client._redispatch_if_generation_changed() + assert "certificate verification" in caplog.text + assert "fully back up" not in caplog.text + + +class TestTLSVerificationFailureIsNamed: + @pytest.mark.asyncio + async def test_a_certificate_verification_failure_is_its_own_error(self) -> None: + """The one transport failure that must not be retried into submission.""" + verify_failure = ssl.SSLCertVerificationError("certificate verify failed: unable to get local issuer certificate") + wrapped = httpx.ConnectError("[SSL: CERTIFICATE_VERIFY_FAILED]") + wrapped.__cause__ = verify_failure + injected = MagicMock() + injected.get = AsyncMock(side_effect=wrapped) + + with pytest.raises(SpanPanelTLSVerificationError) as excinfo: + await get_homie_schema(HOST, httpx_client=injected) + # Still a connection error, so a consumer holding the 3.x contract — + # catch SpanPanelConnectionError, retry — keeps working unchanged. + assert isinstance(excinfo.value, SpanPanelConnectionError) + + @pytest.mark.asyncio + async def test_an_ordinary_connect_failure_stays_a_connection_error(self) -> None: + """A refused socket is 'not up yet', and must not look terminal.""" + injected = MagicMock() + injected.get = AsyncMock(side_effect=httpx.ConnectError("connection refused")) + + with pytest.raises(SpanPanelConnectionError) as excinfo: + await get_homie_schema(HOST, httpx_client=injected) + assert not isinstance(excinfo.value, SpanPanelTLSVerificationError) diff --git a/uv.lock b/uv.lock index 46355a3..553b402 100644 --- a/uv.lock +++ b/uv.lock @@ -975,7 +975,7 @@ wheels = [ [[package]] name = "span-panel-api" -version = "3.3.0" +version = "3.4.0" source = { editable = "." } dependencies = [ { name = "httpx" },