diff --git a/smartthings_local/protocol/dtls_probe.py b/smartthings_local/protocol/dtls_probe.py index 3fbd5ef..ae073bf 100644 --- a/smartthings_local/protocol/dtls_probe.py +++ b/smartthings_local/protocol/dtls_probe.py @@ -42,7 +42,7 @@ from .auth import _DTLS_CIPHERS, _OCF_ROOT_CA, _load_pem_chain from .coap import split_dtls from .dtls_handshake import _drive_dtls_handshake -from .endpoint import open_connected_udp_socket +from .endpoint import open_host_filtered_udp_socket # DTLS record content types (RFC 6347 ยง4.1) _CT_CHANGE_CIPHER_SPEC = 20 @@ -274,12 +274,12 @@ def _classify_liveness_response(datagram): def _probe_dtls_port_with_flight( host, port, *, flight, timeout, retries, family): - """Send one frozen ClientHello flight on a connected UDP socket.""" + """Send one frozen ClientHello flight on a host-filtered UDP socket.""" attempt_budget = float(timeout) / (retries + 1) attempts = 0 sock = None try: - sock, _endpoint = open_connected_udp_socket( + sock, _endpoint = open_host_filtered_udp_socket( host, port, family=family, @@ -302,9 +302,11 @@ def _probe_dtls_port_with_flight( break response_kind, alert = _parse_liveness_response(datagram) if response_kind is None: - # A connected UDP socket already rejects other peers. An - # unrelated or malformed datagram from the appliance must - # not consume a retransmission or count as DTLS proof. + # The socket already rejects other hosts, and an + # appliance may legitimately answer from a port other than + # the one dialled. An unrelated or malformed datagram from + # it must still not consume a retransmission or count as + # DTLS proof. continue return DtlsLivenessResult( port=port, @@ -591,7 +593,7 @@ def diagnose_dtls_handshake( conn.set_ciphertext_mtu(mtu) try: - sock, _endpoint = open_connected_udp_socket( + sock, _endpoint = open_host_filtered_udp_socket( host, port, family=family, diff --git a/smartthings_local/protocol/dtls_session.py b/smartthings_local/protocol/dtls_session.py index fa53924..e73e8ce 100644 --- a/smartthings_local/protocol/dtls_session.py +++ b/smartthings_local/protocol/dtls_session.py @@ -89,7 +89,7 @@ _drive_dtls_handshake, _HandshakeCancelled, ) -from .endpoint import open_connected_udp_socket +from .endpoint import open_host_filtered_udp_socket # Private compatibility exports used by dtls_probe and existing callers. _DTLS_CIPHERS = _auth._DTLS_CIPHERS @@ -172,7 +172,9 @@ class _MidExchange: # usually works. UDP delivery was never guaranteed, so treat them as # advisory and keep reading. Unconnected sockets never see any of this, # which is why the reader survived them before the connected-socket change -# in d677c72 (v0.1.3). +# in d677c72 (v0.1.3). The session moved back to an unconnected socket for +# issue #66, so this set is now defensive: it costs nothing and still covers +# any caller that supplies a connected socket adapter of its own. _ADVISORY_ERRNOS = frozenset( value for value in ( getattr(errno, name, None) @@ -592,7 +594,7 @@ def connect( remaining = deadline - time.monotonic() if remaining <= 0: raise SessionTimeoutError() - sock, endpoint = open_connected_udp_socket( + sock, endpoint = open_host_filtered_udp_socket( self.host, self.port, family=self.family, @@ -1760,9 +1762,8 @@ def post( except EndpointError: # Attempt 0 is the caller's only datagram, so its # failure is theirs to see. A retransmit is - # best-effort: a connected UDP socket reports the - # ICMP error queued by an earlier send on the next - # one, and the reader treats those same errnos as + # best-effort: a send can still fail locally, + # and the reader treats the same errnos as # advisory. Failing the exchange on one would make # retransmitting less robust than not bothering, # while the original datagram may still be diff --git a/smartthings_local/protocol/endpoint.py b/smartthings_local/protocol/endpoint.py index 9bb2be3..78b169e 100644 --- a/smartthings_local/protocol/endpoint.py +++ b/smartthings_local/protocol/endpoint.py @@ -2,13 +2,16 @@ import math import socket +import time from dataclasses import dataclass from ..errors import EndpointError __all__ = [ + 'HostFilteredUdpSocket', 'ResolvedUdpEndpoint', 'open_connected_udp_socket', + 'open_host_filtered_udp_socket', 'resolve_udp_endpoint', 'resolve_udp_endpoints', ] @@ -162,3 +165,147 @@ def open_connected_udp_socket( pass raise EndpointError() from OSError('UDP socket setup failed') + + +def _host_key(family, sockaddr): + """Return a comparable host identity, ignoring port and flow label.""" + try: + packed = socket.inet_pton(family, sockaddr[0]) + except (OSError, UnicodeError, TypeError, ValueError): + return None + # Retain the IPv6 scope so a link-local reply from another interface is + # not mistaken for the target. + scope = sockaddr[3] if family == socket.AF_INET6 and len(sockaddr) > 3 else 0 + return (family, packed, scope) + + +class HostFilteredUdpSocket: + """UDP socket that accepts replies from any port on one target host. + + A connected UDP socket accepts datagrams only from the exact port it + dialled. RT-OCF binds its DTLS socket to port 0, so an appliance answers + from a kernel-assigned port that need not match the port addressed, and a + connected socket makes a live appliance look silent. This wrapper keeps + sending to the resolved destination and filters inbound datagrams on host + alone, which is what ``ocf_discovery`` already does. + + The exposed surface is the subset of the socket API the DTLS callers use. + Off-path spoofing resistance drops from address-and-port to address only; + the DTLS cookie exchange and handshake authentication remain the real + protection, as they already are for a connected socket. + """ + + __slots__ = ( + '_dest', + '_endpoint', + '_host_key', + '_sock', + '_timeout', + 'observed_reply_port', + ) + + def __init__(self, sock, endpoint): + self._sock = sock + self._endpoint = endpoint + self._dest = endpoint.sockaddr + self._host_key = _host_key(endpoint.family, endpoint.sockaddr) + self._timeout = None + #: Source port of the most recent accepted datagram. Diagnostic only; + #: replies keep going to the port originally dialled. + self.observed_reply_port = None + + @property + def endpoint(self): + return self._endpoint + + def send(self, data): + """Send to the resolved destination, mirroring ``socket.send``.""" + return self._sock.sendto(data, self._dest) + + def recv(self, bufsize): + """Return the next datagram from the target host. + + Datagrams from any other host are discarded without extending the + caller's timeout, so a flood from elsewhere cannot hold the call open + past its deadline. + """ + timeout = self._timeout + deadline = None if timeout is None else time.monotonic() + timeout + while True: + if deadline is not None: + remaining = deadline - time.monotonic() + if remaining <= 0: + raise TimeoutError('timed out') + self._sock.settimeout(remaining) + datagram, address = self._sock.recvfrom(bufsize) + if self._host_key is None or \ + _host_key(self._endpoint.family, address) == self._host_key: + if len(address) > 1: + self.observed_reply_port = address[1] + return datagram + + def settimeout(self, timeout): + self._timeout = timeout + self._sock.settimeout(timeout) + + def gettimeout(self): + return self._timeout + + def fileno(self): + return self._sock.fileno() + + def getsockname(self): + return self._sock.getsockname() + + def close(self): + self._sock.close() + + def __enter__(self): + return self + + def __exit__(self, *_exc): + self.close() + return False + + def __repr__(self): + return f'HostFilteredUdpSocket(family={self._endpoint.family_name})' + + +def open_host_filtered_udp_socket( + host, port, *, family=socket.AF_UNSPEC, local_port=None, timeout=None): + """Open an unconnected UDP socket bound to one target host. + + Mirrors :func:`open_connected_udp_socket`, but the returned socket accepts + a reply from any port on the target rather than only the port dialled. See + :class:`HostFilteredUdpSocket` for why an OCF appliance needs that. + """ + if local_port is not None: + _validate_port(local_port, allow_zero=True) + if timeout is not None: + if isinstance(timeout, bool) or not isinstance(timeout, (int, float)): + raise TypeError('timeout must be a number or None') + if not math.isfinite(timeout) or timeout < 0: + raise ValueError('timeout must be a non-negative number or None') + + endpoints = resolve_udp_endpoints(host, port, family=family) + for endpoint in endpoints: + sock = None + try: + sock = socket.socket( + endpoint.family, socket.SOCK_DGRAM, socket.IPPROTO_UDP) + if local_port is not None: + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + # Bind unconditionally so the local port is fixed before the first + # send, matching the connected path's source-port stability. + sock.bind(endpoint.bind_address(local_port or 0)) + wrapper = HostFilteredUdpSocket(sock, endpoint) + wrapper.settimeout(timeout) + return wrapper, endpoint + except OSError: + if sock is not None: + try: + sock.close() + except OSError: + pass + + raise EndpointError() from OSError('UDP socket setup failed') diff --git a/tests/test_dtls_probe.py b/tests/test_dtls_probe.py index d8b8e76..12d0c3d 100644 --- a/tests/test_dtls_probe.py +++ b/tests/test_dtls_probe.py @@ -251,7 +251,7 @@ def open_socket(host, port, *, family, timeout): fake.settimeout(timeout) return fake, object() - monkeypatch.setattr(p, 'open_connected_udp_socket', open_socket) + monkeypatch.setattr(p, 'open_host_filtered_udp_socket', open_socket) result = p.probe_dtls_port( 'appliance.invalid', 5684, family=socket.AF_INET6, timeout=0.2) @@ -426,7 +426,7 @@ def open_socket(_host, _port, *, family, timeout): sock.settimeout(timeout) return sock, object() - monkeypatch.setattr(p, 'open_connected_udp_socket', open_socket) + monkeypatch.setattr(p, 'open_host_filtered_udp_socket', open_socket) monkeypatch.setattr(p.time, 'monotonic', lambda: now[0]) result = p.diagnose_dtls_handshake( diff --git a/tests/test_endpoint.py b/tests/test_endpoint.py index cd4643b..67937bd 100644 --- a/tests/test_endpoint.py +++ b/tests/test_endpoint.py @@ -7,8 +7,10 @@ from smartthings_local.errors import EndpointError from smartthings_local.protocol import dtls_session from smartthings_local.protocol.endpoint import ( + HostFilteredUdpSocket, ResolvedUdpEndpoint, open_connected_udp_socket, + open_host_filtered_udp_socket, resolve_udp_endpoint, resolve_udp_endpoints, ) @@ -290,7 +292,7 @@ def test_socket_timeout_must_be_finite_and_non_negative(timeout): open_connected_udp_socket('device.example', 5684, timeout=timeout) -def test_session_uses_connected_socket_send_and_recv(monkeypatch): +def test_session_uses_host_filtered_socket_send_and_recv(monkeypatch): endpoint = ResolvedUdpEndpoint( socket.AF_INET6, ('2001:db8::10', 5684, 0, 0)) sock = FakeSocket(socket.AF_INET6) @@ -356,7 +358,7 @@ def open_socket(*args, **kwargs): dtls_session.SSL, 'Connection', lambda *args: connection) monkeypatch.setattr( dtls_session, - 'open_connected_udp_socket', + 'open_host_filtered_udp_socket', open_socket, ) monkeypatch.setattr(dtls_session.time, 'sleep', lambda _delay: None) @@ -421,3 +423,164 @@ def send(self, _data): assert 'UDP send failed' in formatted assert 'credential-value' not in formatted assert 'device.example' not in formatted + + +# --- issue #66: an appliance answering from a port it was not dialled on ---- +# +# RT-OCF binds its DTLS socket to port 0 (rt_udp.c rt_udp_open_server), so the +# secure port is kernel-assigned and a reply legitimately arrives from a source +# port other than the one addressed. A connected UDP socket drops those, which +# made a live appliance report as dead. + + +class _RecordingUdpSocket: + """Underlying socket stub that replays a scripted inbound sequence.""" + + def __init__(self, family, inbound=()): + self.family = family + self.inbound = list(inbound) + self.sent = [] + self.timeouts = [] + self.bound = None + self.closed = False + self.options = [] + + def setsockopt(self, *args): + self.options.append(args) + + def bind(self, address): + self.bound = address + + def settimeout(self, timeout): + self.timeouts.append(timeout) + + def sendto(self, data, address): + self.sent.append((data, address)) + return len(data) + + def recvfrom(self, _size): + if not self.inbound: + raise TimeoutError('timed out') + return self.inbound.pop(0) + + def getsockname(self): + return self.bound + + def fileno(self): + return 7 + + def close(self): + self.closed = True + + +def _v4_endpoint(port=5684): + return ResolvedUdpEndpoint(socket.AF_INET, ('192.0.2.10', port)) + + +def test_reply_from_another_source_port_is_accepted(): + endpoint = _v4_endpoint() + inner = _RecordingUdpSocket( + socket.AF_INET, + inbound=[(b'\x16reply', ('192.0.2.10', 59768))], + ) + sock = HostFilteredUdpSocket(inner, endpoint) + + assert sock.recv(4096) == b'\x16reply' + # The port the appliance answered from is recorded for diagnostics only. + assert sock.observed_reply_port == 59768 + + +def test_send_still_targets_the_dialled_port(): + endpoint = _v4_endpoint() + inner = _RecordingUdpSocket( + socket.AF_INET, + inbound=[(b'\x16reply', ('192.0.2.10', 59768))], + ) + sock = HostFilteredUdpSocket(inner, endpoint) + sock.recv(4096) + sock.send(b'\x16request') + + # Following the reply port would be an unevidenced behaviour change; #66's + # capture shows the appliance still accepting on the port originally used. + assert inner.sent == [(b'\x16request', ('192.0.2.10', 5684))] + + +def test_datagram_from_another_host_is_discarded(): + endpoint = _v4_endpoint() + inner = _RecordingUdpSocket( + socket.AF_INET, + inbound=[ + (b'\x16spoof', ('198.51.100.7', 5684)), + (b'\x16real', ('192.0.2.10', 41234)), + ], + ) + sock = HostFilteredUdpSocket(inner, endpoint) + + assert sock.recv(4096) == b'\x16real' + assert sock.observed_reply_port == 41234 + + +def test_foreign_datagrams_do_not_extend_the_deadline(monkeypatch): + endpoint = _v4_endpoint() + inner = _RecordingUdpSocket( + socket.AF_INET, + inbound=[(b'\x16spoof', ('198.51.100.7', 5684))] * 50, + ) + sock = HostFilteredUdpSocket(inner, endpoint) + sock.settimeout(1.0) + + clock = iter([0.0] + [0.3, 0.6, 0.9, 1.2] + [9.0] * 50) + monkeypatch.setattr( + 'smartthings_local.protocol.endpoint.time.monotonic', + lambda: next(clock)) + + with pytest.raises(TimeoutError): + sock.recv(4096) + + # Each discarded datagram shortens the remaining budget rather than + # restarting it, so a flood cannot hold recv open past its deadline. + assert inner.timeouts[-1] < 1.0 + assert inner.inbound, 'recv consumed the whole flood instead of timing out' + + +def test_ipv6_scope_is_part_of_host_identity(): + endpoint = ResolvedUdpEndpoint( + socket.AF_INET6, ('2001:db8::10', 5684, 0, 2)) + inner = _RecordingUdpSocket( + socket.AF_INET6, + inbound=[ + (b'\x16wrong-if', ('2001:db8::10', 5684, 0, 3)), + (b'\x16right-if', ('2001:db8::10', 59768, 0, 2)), + ], + ) + sock = HostFilteredUdpSocket(inner, endpoint) + + # The same address arriving with a different scope is a different peer, + # which is what keeps a link-local reply from another interface out. + assert sock.recv(4096) == b'\x16right-if' + + +def test_open_host_filtered_udp_socket_binds_without_connecting(monkeypatch): + created = [] + + def factory(family, _socktype, _protocol): + sock = _RecordingUdpSocket(family) + created.append(sock) + return sock + + monkeypatch.setattr( + 'smartthings_local.protocol.endpoint.socket.socket', factory) + monkeypatch.setattr( + 'smartthings_local.protocol.endpoint.socket.getaddrinfo', + lambda *_a, **_k: [_addrinfo(socket.AF_INET, ('192.0.2.10', 5684))]) + + sock, endpoint = open_host_filtered_udp_socket( + '192.0.2.10', 5684, timeout=2.0) + + assert isinstance(sock, HostFilteredUdpSocket) + assert endpoint.port == 5684 + assert created[0].bound == ('', 0) + assert not hasattr(created[0], 'peer') + assert sock.gettimeout() == 2.0 + sock.close() + assert created[0].closed diff --git a/tests/test_errors.py b/tests/test_errors.py index 91ba89e..9567dcb 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -129,7 +129,7 @@ def close(self): dtls_session.socket.AF_INET, ('192.0.2.10', 5684)) monkeypatch.setattr( dtls_session, - 'open_connected_udp_socket', + 'open_host_filtered_udp_socket', lambda *args, **kwargs: (fake_socket, endpoint), ) diff --git a/tests/test_psk_auth.py b/tests/test_psk_auth.py index c4e957a..0a7a065 100644 --- a/tests/test_psk_auth.py +++ b/tests/test_psk_auth.py @@ -363,7 +363,7 @@ def test_psk_handshake_rejection_does_not_expose_credentials(): patch.object(session_module.SSL, "Connection", return_value=connection), patch.object( session_module, - "open_connected_udp_socket", + "open_host_filtered_udp_socket", return_value=(udp_socket, endpoint), ), pytest.raises(SessionError) as captured, diff --git a/tests/test_session_connect_deadline.py b/tests/test_session_connect_deadline.py index 0a35df6..c7c1f3b 100644 --- a/tests/test_session_connect_deadline.py +++ b/tests/test_session_connect_deadline.py @@ -137,7 +137,7 @@ def open_socket(*args, **kwargs): ) monkeypatch.setattr( dtls_session, - "open_connected_udp_socket", + "open_host_filtered_udp_socket", open_socket, ) monkeypatch.setattr(dtls_session.time, "monotonic", clock.monotonic) @@ -241,7 +241,7 @@ def open_socket(*_args, **_kwargs): monkeypatch.setattr(dtls_session.SSL, "Context", lambda *_args: object()) monkeypatch.setattr(dtls_session.SSL, "Connection", lambda *_args: _Connection()) - monkeypatch.setattr(dtls_session, "open_connected_udp_socket", open_socket) + monkeypatch.setattr(dtls_session, "open_host_filtered_udp_socket", open_socket) monkeypatch.setattr(dtls_session.time, "monotonic", clock.monotonic) session = _session(_Auth(clock, configure_delay=0.2)) @@ -274,7 +274,7 @@ def open_socket(*_args, **kwargs): "Connection", lambda *_args: connection, ) - monkeypatch.setattr(dtls_session, "open_connected_udp_socket", open_socket) + monkeypatch.setattr(dtls_session, "open_host_filtered_udp_socket", open_socket) monkeypatch.setattr(dtls_session.time, "monotonic", clock.monotonic) with pytest.raises(SessionTimeoutError): diff --git a/tests/test_session_interruption.py b/tests/test_session_interruption.py index 7cc2396..76cfb1b 100644 --- a/tests/test_session_interruption.py +++ b/tests/test_session_interruption.py @@ -94,7 +94,7 @@ def open_socket(*_args, **_kwargs): ) monkeypatch.setattr( dtls_session, - "open_connected_udp_socket", + "open_host_filtered_udp_socket", open_socket, ) return endpoint @@ -140,7 +140,7 @@ def test_cancel_during_context_setup_stops_before_socket_setup(monkeypatch): monkeypatch.setattr(dtls_session.SSL, "Context", lambda *_args: object()) monkeypatch.setattr( dtls_session, - "open_connected_udp_socket", + "open_host_filtered_udp_socket", lambda *_args, **_kwargs: pytest.fail( "cancelled connect opened a socket" ),