Update to smartthings-local 0.1.6, handle redacted typed errors
Bumps the smartthings-local floor from >=0.1.2 to >=0.1.6 (manifest,
requirements-dev.txt, Dockerfile) and adopts the interface/behavior
changes introduced along the way:
- 0.1.3 ("redacted typed failures", PR #23) replaced connect()'s
ConnectionError(f"DTLS handshake error: {e}") with fixed, redacted
exceptions (SessionError, SessionTimeoutError, etc.) that never carry
backend text -- including the TLS alert name. The config flow's
_classify_handshake_failure relied on parsing that text out of the
exception (_alert_name) to tell a rejected certificate from any other
handshake failure; against a current library that regex never matches
again, silently downgrading every setup failure to the generic
"cannot_connect" message.
Fixed by adding _resolve_alert: it still tries _alert_name first (a
harmless fallback if it ever matches), then falls back to one bounded
smartthings_local.protocol.dtls_probe.diagnose_dtls_handshake() call
against the specific port that failed, which classifies the fatal
Alert straight from the raw record instead of an exception string.
_handshake_and_read now threads the resolved per-port alerts into
_classify_handshake_failure, so CertRejected vs. HandshakeFailed keeps
working the way it did before the redaction.
- 0.1.3 also moved the DTLS session onto a connected UDP socket (see
endpoint.py's open_connected_udp_socket), which changes why
coordinator._local_source_port needs a unique port per device -- the
kernel now demuxes by the full local-port/remote-peer tuple instead of
relying on an unconnected recvfrom(). Docstring updated to match.
- 0.1.6's reader-thread fail-fast fix (_check_live/_reader_running) makes
a dead reader raise SessionClosedError immediately instead of hanging
a request out to its timeout. No code change needed: SessionClosedError
is a ConnectionError subclass (not TimeoutError), so the coordinator's
existing _defer_reconnect_for/isinstance(e, TimeoutError) split already
routes it to the immediate-reconnect path.
Every other new/changed piece (endpoint.py, dtls_probe.py bounded
probing, auth.py's CertificateAuth/PskAuth providers) stays behind
compatible built-in exception types and unchanged get()/post()/
subscribe()/ping() signatures, per the library's own compatibility
table, so the coordinator's and observe.py's broad exception handling
needed no changes.
Tests: added coverage for _resolve_alert's exception-text vs.
diagnostic-handshake fallback, _classify_handshake_failure with a
resolved alerts mapping, and an end-to-end config-flow re-mint test
against a FakeSession that raises the new redacted SessionError instead
of the old text-bearing ConnectionError.
Full suite (1517 tests), ruff, and ty all pass against smartthings-local
0.1.6 installed from PyPI.
This commit is contained in:
1 parent
cbe881818b
commit
3f9789512d
6 files changed
+200
-21
No files matched your search
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
Reference in new issue
Block a user