diff --git a/Dockerfile b/Dockerfile index 8f7f9ee..9f4ff04 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,4 +6,4 @@ FROM ghcr.io/home-assistant/home-assistant:stable # repeats the install attempt on every container recreate. Baking # smartthings-local into the image keeps the dev container usable # offline and avoids relying on that runtime install path. -RUN pip3 install --no-cache-dir "smartthings-local>=0.1.2" +RUN pip3 install --no-cache-dir "smartthings-local>=0.1.6" diff --git a/custom_components/localthings/config_flow.py b/custom_components/localthings/config_flow.py index d965d7b..1c0ae0d 100644 --- a/custom_components/localthings/config_flow.py +++ b/custom_components/localthings/config_flow.py @@ -493,27 +493,75 @@ _CERT_ALERTS = frozenset( } ) -# OpenSSL renders a received fatal alert into its error text as e.g. -# "tlsv1 alert unknown ca", which DtlsCoapSession.connect() wraps in a -# ConnectionError. Reading it back tells us what the appliance objected to. -# -# Deliberately not the library's diagnostic probe (stateless=False): that -# mode commits association state on the device, and an orphaned association -# makes the next attempt time out (RFC 6347 §4.2.8) -- a bad trade on a -# path the user is about to retry. +# Older smartthings-local (< 0.1.3) rendered a received fatal alert straight +# into the handshake exception's text, e.g. "tlsv1 alert unknown ca" wrapped +# in a ConnectionError -- reading it back told us what the appliance +# objected to. 0.1.3's "redacted typed failures" removed that: connect()'s +# exceptions now carry a fixed, non-sensitive message with the real OpenSSL +# text neither included nor chained (see smartthings_local.errors -- +# "backend errors can contain remote endpoints, local paths, or credential +# metadata"). _alert_name is kept as a harmless fallback for exception text +# that does carry it; _resolve_alert below is what actually classifies a +# failure against a current library. _ALERT_RE = re.compile(r"alert ([a-z0-9 ]+)") def _alert_name(exc: Exception) -> str | None: - """The TLS alert an appliance sent, if this failure carried one.""" + """The TLS alert an appliance sent, if this failure's exception text + carried one (only ever true against smartthings-local < 0.1.3).""" match = _ALERT_RE.search(str(exc).lower()) return match.group(1).strip().replace(" ", "_") if match else None +def _diagnostic_alert(host: str, port: int, cert_pem: str, key_pem: str): + """One opt-in stateful handshake against `port`, using our real + credentials, so a fatal Alert can be classified from the raw record + itself rather than parsed out of an exception's text. + + This is the library's diagnose_dtls_handshake -- deliberately not used + for the primary candidate scan (it commits association state on the + device, and an orphaned association makes the *next* attempt time out + per RFC 6347 §4.2.8). Here it only runs once every real candidate has + already failed, to explain a failure that's happening either way -- + one more orphaned association is a fair trade for a message that says + why, on a path the user is about to retry regardless. + + Imported lazily, like `_clienthello_probe`, so an install whose + smartthings-local predates this API degrades to a generic message + instead of failing to load the config flow at all. + """ + from smartthings_local.protocol.dtls_probe import diagnose_dtls_handshake + + return diagnose_dtls_handshake( + host, + port, + cert_pem=cert_pem, + key_pem=key_pem, + timeout=CLIENTHELLO_PROBE_TIMEOUT_S, + retries=CLIENTHELLO_PROBE_RETRIES, + ) + + +def _resolve_alert(exc: Exception, host: str, port: int, cert_pem: str, key_pem: str) -> str | None: + """The TLS alert `port`'s failed handshake carried, if any -- the + exception's own text first (cheap, and all an older library ever + offers), then one bounded diagnostic handshake against a current one + that redacts it (see _diagnostic_alert).""" + name = _alert_name(exc) + if name is not None: + return name + try: + result = _diagnostic_alert(host, port, cert_pem, key_pem) + except Exception: + return None + return result.alert[1] if result.alert else None + + def _classify_handshake_failure( host: str, scan: _PortScan, failures: list[tuple[int, Exception]], + alerts: dict[int, str] | None = None, ) -> CannotConnect: """Turn "no port worked" into the most specific thing we can honestly say, in rough order of how much the evidence tells us: an alert means @@ -521,13 +569,21 @@ def _classify_handshake_failure( certificate); a confirmed DTLS port that then timed out is likely still holding a session from a previous attempt; otherwise the sweep's own shape is the evidence. + + `alerts` is the per-port classification `_handshake_and_read` already + resolved (exception text, or a diagnostic handshake -- see + _resolve_alert); a caller with only raw failures (or an older library) + still gets `_alert_name`'s exception-text reading as a fallback. """ - alerts = [name for name in (_alert_name(exc) for _, exc in failures) if name] - cert_alerts = [name for name in alerts if name in _CERT_ALERTS] + resolved = dict(alerts or {}) + for port, exc in failures: + resolved.setdefault(port, _alert_name(exc)) + alert_names = [name for name in resolved.values() if name] + cert_alerts = [name for name in alert_names if name in _CERT_ALERTS] if cert_alerts: return CertRejected(f"{host} rejected our certificate (alert {cert_alerts[0]})") - if alerts: - return HandshakeFailed(f"{host} refused the DTLS handshake (alert {alerts[0]})") + if alert_names: + return HandshakeFailed(f"{host} refused the DTLS handshake (alert {alert_names[0]})") if scan.confirmed: return HandshakeTimeout( f"DTLS server confirmed on {host}:{scan.confirmed} but the handshake never completed" @@ -631,6 +687,7 @@ def _handshake_and_read(host: str, scan: _PortScan, cert_pem: str, key_pem: str) from smartthings_local.protocol.dtls_session import DtlsCoapSession failures: list[tuple[int, Exception]] = [] + alerts: dict[int, str] = {} for port in scan.candidates: sess = None try: @@ -645,11 +702,14 @@ def _handshake_and_read(host: str, scan: _PortScan, cert_pem: str, key_pem: str) except Exception as exc: failures.append((port, exc)) _LOGGER.debug("port %d failed: %s", port, exc) + alert = _resolve_alert(exc, host, port, cert_pem, key_pem) + if alert is not None: + alerts[port] = alert finally: if sess is not None: with contextlib.suppress(Exception): sess.close() - raise _classify_handshake_failure(host, scan, failures) + raise _classify_handshake_failure(host, scan, failures, alerts) def _probe_and_validate( diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index d0a7ada..f10ff54 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -103,10 +103,15 @@ def _local_source_port(host: str) -> int: time per RFC 6347 §4.2.8, instead of holding it 5-15 min. See DTLS_LOCAL_PORT_BASE. Requires smartthings-local >= 0.1.1. - Must stay unique per device on this host too: the library's socket is - unconnected, so two devices sharing a port would mis-demux each other's - datagrams. Last IPv4 octet as offset for the common case; a stable - CRC32 fold otherwise. + Must stay unique per device on this host too. That used to be load- + bearing for demuxing: an unconnected socket handed every device's + datagrams to whichever recvfrom() happened to be listening on their + shared port. smartthings-local >= 0.1.3 connect()s its UDP socket + instead (see endpoint.py's open_connected_udp_socket), so the kernel + already filters incoming datagrams to each session's own resolved peer + -- but a distinct port per device keeps that guarantee from ever + depending on it, and keeps captures/logs unambiguous. Last IPv4 octet + as offset for the common case; a stable CRC32 fold otherwise. """ try: offset = int(ipaddress.IPv4Address(host)) & 0xFF diff --git a/custom_components/localthings/manifest.json b/custom_components/localthings/manifest.json index 60c97e6..0a8f5ea 100644 --- a/custom_components/localthings/manifest.json +++ b/custom_components/localthings/manifest.json @@ -10,7 +10,7 @@ "requirements": [ "cbor2>=5.4.6", "pyOpenSSL>=23.0", - "smartthings-local>=0.1.2" + "smartthings-local>=0.1.6" ], "version": "0.21.2" } diff --git a/requirements-dev.txt b/requirements-dev.txt index 6aa6f59..088db5f 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -6,7 +6,7 @@ pytest-homeassistant-custom-component>=0.13.316 # Integration runtime deps, needed to import the component under test # (also declared in custom_components/localthings/manifest.json). -smartthings-local>=0.1.2 +smartthings-local>=0.1.6 cbor2>=5.4.6 pyOpenSSL>=23.0 cryptography>=41.0 diff --git a/tests/localthings/test_config_flow.py b/tests/localthings/test_config_flow.py index a3d8e85..a20d0e6 100644 --- a/tests/localthings/test_config_flow.py +++ b/tests/localthings/test_config_flow.py @@ -238,6 +238,11 @@ class FakeSession: instances: ClassVar[list[FakeSession]] = [] reject_certs: ClassVar[set[str]] = set() + # smartthings-local >= 0.1.3 ("redacted typed failures") no longer puts + # the alert in connect()'s exception text -- set True to model that, so + # a test can check the diagnostic-handshake fallback (_resolve_alert) + # instead of the legacy _alert_name text-parsing path. + redact_rejection: ClassVar[bool] = False def __init__(self, host, port, cert_pem=None, key_pem=None, **kwargs): self.host, self.port, self.cert_pem = host, port, cert_pem @@ -245,6 +250,10 @@ class FakeSession: def connect(self): if self.cert_pem in FakeSession.reject_certs: + if FakeSession.redact_rejection: + from smartthings_local.errors import SessionError + + raise SessionError() raise ConnectionError( "DTLS handshake error: [('SSL routines', '', 'sslv3 alert bad certificate')]" ) @@ -268,6 +277,7 @@ def fake_dtls(monkeypatch): FakeSession.instances = [] FakeSession.reject_certs = set() + FakeSession.redact_rejection = False monkeypatch.setattr(config_flow, "_fetch_samsung_uuid", lambda: "test-uuid") monkeypatch.setattr( config_flow, @@ -480,6 +490,44 @@ async def test_rejected_reused_leaf_is_reminted( assert [s.cert_pem for s in FakeSession.instances] == [MOCK_LEAF_CERT_PEM, "FULLCHAIN"] +async def test_rejected_reused_leaf_is_reminted_against_a_redacted_library( + hass: HomeAssistant, monkeypatch, fake_dtls +) -> None: + """Same flow as test_rejected_reused_leaf_is_reminted, but against a + connect() failure shaped like smartthings-local >= 0.1.3 -- a fixed, + redacted exception with no alert text at all (see errors.py's + "Classified errors"). The re-mint decision has to come from + _resolve_alert's diagnostic-handshake fallback instead of _alert_name.""" + from custom_components.localthings import config_flow + + existing = MockConfigEntry(domain=DOMAIN, data=ENTRY_DATA, unique_id="localthings_other") + existing.add_to_hass(hass) + _patch_clienthello(monkeypatch, {49154}) + FakeSession.reject_certs = {MOCK_LEAF_CERT_PEM} + FakeSession.redact_rejection = True + + class _DiagnosticResult: + alert = (2, "bad_certificate") + + diagnosed: list[int] = [] + + def _diagnostic_alert(host, port, cert_pem, key_pem): + diagnosed.append(port) + return _DiagnosticResult() + + monkeypatch.setattr(config_flow, "_diagnostic_alert", _diagnostic_alert) + + result = await hass.config_entries.flow.async_init(DOMAIN, context={"source": "user"}) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_HOST: MOCK_HOST} + ) + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"][CONF_LEAF_CERT_PEM] == "FULLCHAIN" + assert [s.cert_pem for s in FakeSession.instances] == [MOCK_LEAF_CERT_PEM, "FULLCHAIN"] + assert diagnosed == [49154] + + async def test_unconfirmed_port_failure_is_not_reminted( hass: HomeAssistant, monkeypatch, fake_dtls ) -> None: @@ -554,6 +602,72 @@ def test_cert_alert_is_reported_as_a_certificate_problem() -> None: assert err.error_key == "cert_rejected" +def test_classify_handshake_failure_uses_a_resolved_alert_over_exception_text() -> None: + """_handshake_and_read passes its own resolved `alerts` mapping (built + via _resolve_alert, which is what actually classifies a failure against + smartthings-local >= 0.1.3's redacted exceptions) -- it must win even + when the exception text itself says nothing.""" + from custom_components.localthings.config_flow import ( + CertRejected, + _classify_handshake_failure, + ) + + err = _classify_handshake_failure( + MOCK_HOST, + _scan(confirmed=[49154]), + [(49154, RuntimeError("session operation failed"))], + {49154: "bad_certificate"}, + ) + assert isinstance(err, CertRejected) + assert err.error_key == "cert_rejected" + + +def test_resolve_alert_prefers_exception_text_over_the_diagnostic_handshake() -> None: + """A library still stamping the alert into its exception text (< 0.1.3) + answers for free; the diagnostic handshake must not run at all then.""" + from custom_components.localthings.config_flow import _resolve_alert + + def _must_not_run(*args, **kwargs): + raise AssertionError("must not run the diagnostic handshake") + + with patch("custom_components.localthings.config_flow._diagnostic_alert", _must_not_run): + name = _resolve_alert( + _openssl_alert("tlsv1 alert unknown ca"), MOCK_HOST, 49154, "CERT", "KEY" + ) + assert name == "unknown_ca" + + +def test_resolve_alert_falls_back_to_the_diagnostic_handshake() -> None: + """smartthings-local >= 0.1.3 redacts the exception text (see errors.py's + "Classified errors"), so the only way left to learn *why* a handshake + failed is the library's own classification of the raw alert record.""" + from smartthings_local.errors import SessionError + + from custom_components.localthings import config_flow + + class _Result: + alert = (2, "bad_certificate") + + with patch.object(config_flow, "_diagnostic_alert", lambda *a, **k: _Result()): + name = config_flow._resolve_alert(SessionError(), MOCK_HOST, 49154, "CERT", "KEY") + assert name == "bad_certificate" + + +def test_resolve_alert_is_none_when_the_diagnostic_handshake_also_fails() -> None: + """A best-effort extra probe: its own failure must not raise out of + _resolve_alert, it just leaves the caller with no alert to report.""" + from smartthings_local.errors import SessionError, SessionTimeoutError + + from custom_components.localthings import config_flow + + def _boom(*args, **kwargs): + raise SessionTimeoutError() + + with patch.object(config_flow, "_diagnostic_alert", _boom): + name = config_flow._resolve_alert(SessionError(), MOCK_HOST, 49154, "CERT", "KEY") + assert name is None + + def test_non_cert_alert_is_kept_distinct_from_a_cert_problem() -> None: """A cipher or version mismatch is also a deliberate refusal, but no amount of fiddling with CA credentials will fix it."""