Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,11 @@ sess = DtlsCoapSession("192.0.2.100", 49154, auth=auth)

The identity must be the raw 16-byte OCF UUID and the key exactly 16 or 32 bytes. `PskAuth` selects only `ECDHE-PSK-AES128-CBC-SHA256` and does not acquire, derive, provision, rotate, or persist credentials. Ownership transfer and credential discovery are outside this package.

An identity containing a zero byte is rejected, and that limit is OpenSSL's rather than the appliance's. An OCF device takes the identity as bytes with an explicit length, so a zero byte means nothing to it, but OpenSSL's DTLS 1.2 PSK client callback returns the identity as a C string. Measured against OpenSSL 4.0.0, a 16-byte identity with a NUL at byte 8 reaches the wire as 8 bytes and the handshake raises nothing locally, so the appliance answers a truncated identity it has never seen. DTLS 1.2 offers no length-carrying PSK callback, so such a credential is unusable here: roughly 6% of uniformly random 16-byte identities, and about 5% of UUIDv4s, which have two fixed bytes.
An identity containing a zero byte works in a session. A `DtlsCoapSession` carries a PSK credential through this package's own DTLS 1.2 ECDHE-PSK client, which frames the identity with an explicit length, exactly as an OCF device reads it.

Code holding a credential can check it, and report why, before building a provider or storing anything:
One path cannot carry such an identity, and that limit is OpenSSL's rather than the appliance's: `PskAuth.configure_context` raises for it instead of truncating it. OpenSSL's DTLS 1.2 PSK client callback returns the identity as a C string, and measured against OpenSSL 4.0.0 a 16-byte identity with a NUL at byte 8 reaches the wire as 8 bytes with nothing raising locally, so the appliance would answer a truncated identity it has never seen. DTLS 1.2 offers no length-carrying PSK callback. That affects roughly 6% of uniformly random 16-byte identities, and about 5% of UUIDv4s, which have two fixed bytes -- so code that configures a context itself, rather than handing the provider to a session, should expect the refusal.

Code holding a credential can check it, and report why, before building a provider or storing anything. The check is pure -- it reads the identity and nothing else, so the answer does not move with the environment:

```python
try:
Expand Down
2 changes: 1 addition & 1 deletion docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ PskAuth(*, identity: bytes, key: bytes)
*class*: DTLS authentication using an existing OCF PSK credential.

- `configure_context(context: OpenSSL.SSL.Context) -> None`: Configure one context for the narrow Samsung OCF PSK profile.
- `validate_identity(identity: bytes) -> None`: Raise unless `identity` is one OpenSSL can put on the wire.
- `validate_identity(identity: bytes) -> None`: Raise unless `identity` is one a session can put on the wire.

#### `SamsungServerProfile`

Expand Down
98 changes: 73 additions & 25 deletions smartthings_local/protocol/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
from cryptography.x509.oid import ExtensionOID
from OpenSSL import SSL, _util, crypto

from ._dtls_psk import DtlsPskClient

logger = logging.getLogger(__name__)

_OCF_ROOT_CA = str(Path(__file__).with_name("ocf_root_ca.pem"))
Expand Down Expand Up @@ -556,45 +558,47 @@ def _authenticated_server_identity(self, connection) -> UUID | None:
class PskAuth:
"""DTLS authentication using an existing OCF PSK credential.

The identity must be a raw 16-byte OCF UUID that OpenSSL can present, as
described on :meth:`validate_identity`. The key must contain 16 or 32
bytes. Credential material is intentionally not exposed as public
The identity must be a raw 16-byte OCF UUID; the key must contain 16 or
32 bytes. Credential material is intentionally not exposed as public
attributes and is never included in this provider's representation. A
configured context must not outlive this provider; ``DtlsCoapSession``
enforces that lifetime by retaining its provider.

A session reaches this credential through the library's own DTLS 1.2
ECDHE-PSK client, which carries the identity with an explicit length and
so takes any 16 bytes. :meth:`configure_context` is the OpenSSL path,
kept for direct callers, and it cannot carry an identity containing a
zero byte -- see :meth:`validate_identity`.
"""

__slots__ = ("_callback",)
__slots__ = ("_callback", "_engine")

@staticmethod
def validate_identity(identity: bytes) -> None:
"""Raise unless ``identity`` is one OpenSSL can put on the wire.
"""Raise unless ``identity`` is one a session can put on the wire.

A caller holding a credential can check it here, and report the
reason, before building a provider or storing anything.

An OCF appliance takes the identity as bytes with an explicit length,
so a zero byte is unremarkable to the device. OpenSSL's DTLS 1.2 PSK
client callback returns the identity as a C string, which leaves no
way to express one: a NUL truncates the identity on the wire, and the
handshake then fails against the truncated value with no local error
to point at the cause. Roughly 6% of uniformly random 16-byte
identities carry a zero byte, and about 5% of UUIDv4s, whose version
and variant bytes can never be zero. There is no length-carrying PSK
callback for DTLS 1.2 to fall back on, so such a credential cannot be
used through this library.
reason, before building a provider or storing anything. This is a
pure check: it depends on the identity and nothing else.

A zero byte is accepted. An OCF appliance takes the identity as
bytes with an explicit length, so a zero byte is unremarkable to the
device, and the library's own DTLS client frames it the same way.
What cannot carry it is OpenSSL: its DTLS 1.2 PSK client callback
returns the identity as a C string, so a NUL truncates it on the
wire and the handshake fails against the truncated value with no
local error to point at the cause. That constraint belongs to one
backend rather than to the credential, so :meth:`configure_context`
refuses such an identity explicitly and a session does not.

The size of that difference, measured: roughly 6% of uniformly
random 16-byte identities carry a zero byte, and about 5% of
UUIDv4s, whose version and variant bytes can never be zero.
"""
if type(identity) is not bytes:
raise TypeError("identity must be bytes")
if len(identity) != 16:
raise ValueError("identity must be a raw 16-byte OCF UUID")
if b"\x00" in identity:
raise ValueError(
"identity cannot contain a NUL byte: OpenSSL presents a "
"DTLS 1.2 PSK identity as a C string, so a NUL truncates it "
"and the appliance would be sent a shorter identity than the "
"one supplied"
)

def __init__(self, *, identity: bytes, key: bytes) -> None:
if type(identity) is not bytes or type(key) is not bytes:
Expand All @@ -603,6 +607,22 @@ def __init__(self, *, identity: bytes, key: bytes) -> None:
if len(key) not in (16, 32):
raise ValueError("key must be 16 or 32 bytes")

def engine(*, mtu: int) -> DtlsPskClient:
return DtlsPskClient(identity, key, mtu)

# Held in a closure rather than an attribute, the same way the
# OpenSSL callback below is: no attribute, public or private, holds
# credential material, and test_psk_auth pins that.
object.__setattr__(self, "_engine", engine)

if b"\x00" in identity:
# OpenSSL would truncate this identity at the zero byte, so no
# callback is built for it at all and configure_context refuses.
# The engine a session uses carries an explicit length and is
# unaffected; see validate_identity.
object.__setattr__(self, "_callback", None)
return

ffi = _util.ffi

@ffi.callback(_PSK_CLIENT_CALLBACK_CDEF)
Expand Down Expand Up @@ -644,8 +664,36 @@ def __repr__(self) -> str:
"""Return a representation that never includes credential material."""
return "PskAuth()"

def _create_dtls_connection(self, *, mtu: int):
"""Return a DTLS connection carrying this credential.

Private, and found by ``DtlsCoapSession`` and the diagnostic through
``getattr`` rather than through ``AuthenticationProvider``: that
Protocol is ``runtime_checkable`` and gated on with ``isinstance``,
so a required method here would reject third-party providers.

The engine speaks one ciphersuite and no X.509, and it frames the
identity with an explicit length, which is the whole reason this
path exists alongside :meth:`configure_context`.
"""
return self._engine(mtu=mtu)

def configure_context(self, context: SSL.Context) -> None:
"""Configure one context for the narrow Samsung OCF PSK profile."""
"""Configure one context for the narrow Samsung OCF PSK profile.

Raises when the identity contains a zero byte. OpenSSL presents a
DTLS 1.2 PSK identity as a C string, so it would send a shorter
identity than the one supplied and nothing local would say so. A
session does not come through here and has no such limit.
"""
if self._callback is None:
raise ValueError(
"this identity contains a NUL byte and cannot be presented "
"through OpenSSL, which carries a DTLS 1.2 PSK identity as a "
"C string: it truncates there, and the appliance would be "
"sent a shorter identity than the one supplied. A "
"DtlsCoapSession carries this credential without OpenSSL"
)
setter = getattr(_util.lib, "SSL_CTX_set_psk_client_callback", None)
if setter is None:
raise RuntimeError(
Expand Down
21 changes: 21 additions & 0 deletions smartthings_local/protocol/dtls_handshake.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,27 @@
_DTLS_EPOCH_ZERO = b'\x00\x00'
_DTLS_VERSIONS = frozenset((b'\xfe\xff', b'\xfe\xfd'))
_MAX_CLEANUP_TRANSCRIPT_RECORDS = 32
# The probe has validated this range since it was written; the session never
# did, and the two have to agree now that a session can carry an engine with
# a range of its own.
_MIN_MTU = 576
_MAX_MTU = 16384


def _validate_mtu(mtu):
"""Raise unless ``mtu`` is a datagram size every DTLS backend here takes.

One rule rather than one per caller. OpenSSL silently accepts anything
and only consults the value when it has to fragment a flight, so an
out-of-range mtu used to be invisible until something large went out.
The pure-Python PSK engine validates its own argument, which would
otherwise make the same number raise on one provider and pass on
another, from connect() rather than from where it was supplied.
"""
if isinstance(mtu, bool) or not isinstance(mtu, int):
raise TypeError('mtu must be an integer')
if not _MIN_MTU <= mtu <= _MAX_MTU:
raise ValueError('mtu is outside the safe UDP range')


def _complete_epoch_zero_handshake_types(record):
Expand Down
7 changes: 2 additions & 5 deletions smartthings_local/protocol/dtls_probe.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
from ..errors import ProbeError
from .auth import PskAuth, _DTLS_CIPHERS, _OCF_ROOT_CA, _load_pem_chain
from .coap import split_dtls
from .dtls_handshake import _drive_dtls_handshake
from .dtls_handshake import _drive_dtls_handshake, _validate_mtu
from .endpoint import open_host_filtered_udp_socket

# DTLS record content types (RFC 6347 §4.1)
Expand Down Expand Up @@ -187,10 +187,7 @@ def _validate_liveness_options(port, retries, timeout, mtu):
raise TypeError('timeout must be a number')
if not math.isfinite(timeout) or not 0 < timeout <= 30:
raise ValueError('timeout must be greater than zero and at most 30')
if isinstance(mtu, bool) or not isinstance(mtu, int):
raise TypeError('mtu must be an integer')
if not 576 <= mtu <= 16384:
raise ValueError('mtu is outside the safe UDP range')
_validate_mtu(mtu)


def _validate_probe_family(family):
Expand Down
2 changes: 2 additions & 0 deletions smartthings_local/protocol/dtls_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
_HvrPeerCleanupTranscript,
_drive_dtls_handshake,
_HandshakeCancelled,
_validate_mtu,
)
from .endpoint import open_host_filtered_udp_socket

Expand Down Expand Up @@ -570,6 +571,7 @@ def __init__(self, host, port, cert_path=None, key_path=None, *,
# cannot express which delivery answered the register CON, nor
# which query-qualified relation it belongs to.
self.on_observe_delivery = on_observe_delivery
_validate_mtu(mtu)
self.mtu = mtu
self._min_req_interval = 1.0 / rate_limit_rps
self._write_max_attempts = max(1, int(write_max_attempts))
Expand Down
39 changes: 39 additions & 0 deletions tests/test_dtls_connection_seam.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,45 @@ def test_port_probe_flight_stays_on_openssl():
parameters = inspect.signature(dtls_probe._client_hello_flight).parameters
assert set(parameters) == {"mtu"}


# -- one mtu rule, so a number cannot mean different things per provider ----


@pytest.mark.parametrize("mtu", [100, 575, 16385, 70000])
def test_out_of_range_mtu_is_refused_for_every_provider(mtu):
"""The divergence this closes: the session never validated mtu.

DtlsPskClient requires 256-65535 and OpenSSL silently accepts anything,
consulting it only when it has to fragment. So mtu=100 used to construct,
then raise from connect() on a PSK provider while a certificate provider
carried on. Now both are refused where the argument was supplied, under
the range dtls_probe has validated since it was written.
"""
for auth in (_ContextAuth(), _EngineAuth(_Connection())):
with pytest.raises(ValueError, match="safe UDP range"):
DtlsCoapSession("device.example", 5684, auth=auth, mtu=mtu)


@pytest.mark.parametrize("mtu", [576, 1200, 16384])
def test_in_range_mtu_is_accepted_for_every_provider(mtu):
for auth in (_ContextAuth(), _EngineAuth(_Connection())):
assert DtlsCoapSession(
"device.example", 5684, auth=auth, mtu=mtu).mtu == mtu


@pytest.mark.parametrize("mtu", [True, 1200.0, "1200", None])
def test_mtu_must_be_an_integer(mtu):
with pytest.raises(TypeError, match="mtu must be an integer"):
DtlsCoapSession("device.example", 5684, auth=_ContextAuth(), mtu=mtu)


def test_the_session_and_the_probe_share_one_mtu_rule():
"""Two copies of a range drift. The probe's message is the shared one."""
from smartthings_local.protocol.dtls_handshake import _validate_mtu

for module_validator in (dtls_probe._validate_mtu,
dtls_session._validate_mtu):
assert module_validator is _validate_mtu
# -- what the cancellation checks actually guarantee ------------------------


Expand Down
28 changes: 25 additions & 3 deletions tests/test_dtls_probe.py
Original file line number Diff line number Diff line change
Expand Up @@ -838,18 +838,40 @@ def test_cli_refuses_a_certificate_and_a_psk_together(capsys):


def test_cli_reports_an_unusable_psk_credential_without_a_traceback(capsys):
# A NUL in the identity is rejected by PskAuth, and the CLI has to render
# that as a usage error rather than an exception.
# A malformed credential is rejected by PskAuth, and the CLI has to
# render that as a usage error rather than an exception.
result = p._main([
'127.0.0.1', '5684', '--diagnostic',
'--psk-identity', '0102030405060708000a0b0c0d0e0f10',
'--psk-identity', '0102030405060708',
'--psk-key', '00' * 16,
])

assert result == 2
assert 'invalid PSK credential' in capsys.readouterr().out


def test_cli_accepts_an_identity_containing_a_zero_byte(monkeypatch, capsys):
# It used to be a usage error, because OpenSSL could not carry it. The
# diagnostic now reaches the same engine a session uses, so the
# credential is valid and the run has to proceed to the appliance.
diagnosed = {}

def record(host, port, **kwargs):
diagnosed['auth'] = kwargs.get('auth')
return p.ProbeResult(host, port)

monkeypatch.setattr(p, 'diagnose_dtls_handshake', record)
result = p._main([
'127.0.0.1', '5684', '--diagnostic',
'--psk-identity', '0102030405060708000a0b0c0d0e0f10',
'--psk-key', '00' * 16,
])

assert 'invalid PSK credential' not in capsys.readouterr().out
assert result != 2
assert diagnosed['auth'] is not None


def test_encrypted_records_do_not_invent_handshake_messages(monkeypatch):
# A handshake record after ChangeCipherSpec is encrypted, so its first
# byte is ciphertext. Read as a message type it names whatever it
Expand Down
Loading
Loading