Repository navigation
Carry a PSK credential through the library's own DTLS client - #121
Conversation
_handle_record admitted a DTLS 1.0-framed HelloVerifyRequest only in sent_hello, while _handle_hello_verify_request answers one in sent_cookie_hello too. So a server that frames its challenges that way got its first answered and its second dropped, which is the OpenSSL behaviour that handler exists to avoid: both sides retransmit to the deadline with no alert. Found by @Jason-Morcos on #117. The four existing repeated-cookie tests call _handle_hello_verify_request directly, so none of them crossed the record layer where the gate sits. That is the same shape as the audit test which asserted the original repeated-HVR bug was correct. The version check also reads only the first message in the record, while _handle_handshake_fragment walks every message in it. A 1.0-framed record leading with a HelloVerifyRequest therefore carried a ServerHello straight into got_server_hello, which the framing test in test_dtls_psk_interop.py claims is refused -- true only while the ServerHello arrived in its own record. The exemption now travels with the record and admits one message. No peer is known to send a second 1.0-framed challenge: an OpenSSL server asked to re-challenge sends fatal alert 40 and one cookie, and the appliance frames 1.2. This is a consistency repair, not a measured break. Eight tests, five through the record layer; three fail without the fix and two pin behaviour it must not change.
PskAuth now answers the connection factory the previous commit added, and a session reaches the pure-Python ECDHE-PSK engine instead of OpenSSL. The credential class that was 100% unusable here is the one that gains it: an OwnerPSK whose identity happens to contain a zero byte. The guard moves rather than going away. 475178e measured the reason it exists -- OpenSSL 4.0.0 puts a 16-byte identity with a NUL at byte 8 on the wire as 8 bytes, raising nothing locally -- and that is a fact about OpenSSL's callback, not about the credential. So configure_context refuses such an identity explicitly and names the path that does carry it, while validate_identity stops refusing it and goes back to being pure: a caller showing a user why their credential was rejected cannot have that answer move with the environment, which is the mistake the closed #115 made by checking for a shared library on disk. No OpenSSL callback is built for an unpresentable identity at all, so the guard is the absence of the thing that would truncate rather than a flag beside it. The measured figures stay in the docstring: ~6% of uniformly random 16-byte identities, ~5% of UUIDv4s. Credential material still reaches no attribute, public or private. The engine factory is a closure captured in __init__, the same way the OpenSSL callback already was, because test_psk_auth asserts even _identity and _key are absent. getattr dispatch on a private name is the existing house pattern here, not a new one: connect() already looks up _authenticated_server_identity that way. A session's PSK path is now covered end to end against a real OpenSSL DTLS server behind a fake socket, with and without cookie exchange, carrying an identity that leads with a zero byte: the full flight, the identity read back out of the ClientKeyExchange in the clear, application data both ways, and one record per datagram as TizenRT requires. Those tests fail if the factory is removed. Public API change, so docs/api.md, README.md and the contract test move with it. Downstream still has its own copy of the restriction -- localthings credentials.py raises PskIdentityZeroByte -- so a user does not see this until that goes too.
The engine commit added cryptography>=38.0 to dependencies, but the floor job pins cbor2, pyOpenSSL and pytest and then installs with --no-deps, so nothing ever resolved against that floor: cryptography arrived as whatever pyOpenSSL 23.1.0 happened to pull, around 40.x. A declared floor no job exercises is not a floor. Verified at 38.0.0 on Python 3.11 with the pinned cbor2 and pyOpenSSL: the whole suite passes, which is unsurprising -- AES-CBC, HMAC, EC key generation and from_encoded_point all long predate it.
The close machinery stays above the seam, which was the explicit ask, but nothing exercised it on this path. It needs to: close_notify matters more for a PSK session than for a certificate one. CAdecryptSsl runs SetupCipher only for an endpoint with no existing peer entry (ca_adapter_net_ssl.c:2177-2191), and the suite list it builds is one file-static array (:323) rebuilt in place (:1542) and handed to mbedtls_ssl_conf_ciphersuites as a pointer (:1586), which ssl_srv.c reads live while parsing a ClientHello. So a ClientHello landing on a reused peer entry is matched against whatever the last SetupCipher left there. Our ClientHello offers exactly one suite, so that is binary: present and selected, or handshake_failure with nothing naming the stale entry as the cause. A certificate ClientHello offers a list and degrades more gently. Verified: one datagram, one alert record, epoch 1 so encrypted under the session keys, and the OpenSSL server reports an orderly close rather than our own byte inspection of it.
DtlsCoapSession took mtu straight onto an attribute without checking it, and OpenSSL consults the value only when it has to fragment a flight, so an unusable number was invisible: measured here, set_ciphertext_mtu accepts 100 and 70000 alike and the ClientHello goes out at 212 bytes either way. That stopped being harmless when a session gained a second backend. DtlsPskClient validates 256-65535 of its own, so mtu=100 constructed and then raised from connect() on a PSK provider while a certificate provider carried on -- the same argument meaning two different things depending on the credential, reported from the wrong place. dtls_probe has validated 576-16384 since it was written, with the message "mtu is outside the safe UDP range", so the project already had an answer and the session was the outlier. That check moves to dtls_handshake, which both callers already import from, and the session uses it: one rule, three callers, the failure at the constructor where the argument was supplied. Nothing in the repository or the bridge passes a non-default mtu, and the range the session now enforces is inside the engine's, so the engine's own check is unreachable from a session and stays as a guard for direct use.
# Conflicts: # tests/test_dtls_connection_seam.py
pr-115-purepy-psk/oven_psk_test.py tested the engine through a standalone probe that owned its own socket and CoAP correlation. This one imports nothing of its own: it drives the installed smartthings-local, so what it exercises is the shipped path -- the connection seam, the shared handshake driver, the fixed local source port, CoAP correlation and close(). It runs two sessions on the same local port. The second is the interesting one: a first that works and a second that does not would point at the appliance keeping a peer entry across our close, which is a prediction read out of ca_adapter_net_ssl.c and never tested on hardware. Verified before being handed to anyone: parses on 3.11 through 3.14 (the first draft had a backslash inside an f-string expression, which is a syntax error before 3.12), and runs end to end against an in-process OpenSSL PSK server -- both handshakes complete, start_reader works, both closes send close_notify. The GET times out there because that server speaks DTLS and not CoAP, which also exercises the failure path. Says at the top to stop the integration first: a second DTLS client to one appliance gets silence rather than an error, so an HA session would make a healthy device look dead.
|
@KRZ303 There is a hardware run here that nothing on my side can stand in for, if you have the time? Your October run drove the engine through the standalone probe, which owned its own socket and did its own CoAP correlation. What is new is the engine reached through The branch is A throwaway environment is the safer place for this. The test touches no HA code, so installing a dev branch into your HA one buys nothing and puts the certificate washer you mentioned on #575 behind the same library. In a disposable venv the rollback is One thing that will otherwise cost you a confusing result: make sure HA is not holding a session to the oven while this runs — disable that entry, or stop HA. Measured on my own appliances, a second DTLS client to one device gets silence rather than an error, so an active integration session makes a healthy oven look dead. The script is on the scratch branch, the way the others are: The script's own output is what to paste, verbatim: it labels each step and prints a summary, and the difference between failure modes lives in that text. It is built to be safe to paste — your address is stripped from everything it prints, including the library's own log lines, which do carry it, and That second connect is the interesting one. In the appliance firmware, The opposite experiment, killing the process without One change that might catch you out if you reuse an old script: |
|
@QuiteYellow Both sessions passed on my Samsung oven. I used a disposable Python 3.13.5 virtualenv on a separate LAN VM, with Home Assistant and my SmartThings hub powered off. I installed I ran your script at Here is the script's output verbatim: The close path ran between sessions, and the second connection and read succeeded. I did not capture alert delivery or inspect the appliance's peer table, so this does not establish peer-entry removal. |
|
@KRZ303 Thank you! Your two non-claims stand as you wrote them. With no alert capture and no peer-table read, peer-entry removal is unproven, so the Both #117 and #121 have merged since your run. Your |
|
@QuiteYellow @KRZ303 I checked the released 0.1.23 source against the oven-tested My targeted released-source run had 551 passed and one failure: One migration detail for downstream callers: Also, the LocalThings handoff has landed as #591, carrying forward #575 with the 0.1.23 floor. The cookie wording and factory-boundary corrections on #117/#120 look resolved from my side. Thanks for carrying the reproductions into the tests. |
3 participants
PskAuthnow reaches the pure-Python DTLS 1.2 ECDHE-PSK engine instead of OpenSSL, so the credential class that is currently unusable here gains a working path: an OwnerPSK whose identity contains a zero byte.Relationship to the other PRs. The connection seam landed in #120 and is in
main, so it is not in this diff. The engine itself is #117, and its commits are carried here, because the wiring is meaningless without them. Either #117 merges first and this reduces to the wiring, or #117 closes in favour of this. @Jason-Morcos has reviewed the engine on #117 (339 tests) and the seam on #120 (207 tests, no blocker).What the wiring does
PskAuthanswers the seam's private_create_dtls_connectionhook, holding the engine factory in a closure rather than an attribute —test_psk_authasserts even_identityand_keyare absent, so credential material still reaches no attribute, public or private.The NUL guard moves rather than disappearing
475178emeasured why it exists: OpenSSL 4.0.0 puts a 16-byte identity with a NUL at byte 8 on the wire as 8 bytes, raising nothing locally. That is a fact about OpenSSL's callback, not about the credential. Soconfigure_contextraises for such an identity and names the path that does carry it, whilevalidate_identitystops refusing it and goes back to being a pure, environment-independent check. No OpenSSL callback is built for an unpresentable identity at all, so the guard is the absence of the thing that would truncate. The measured figures stay in the docstring: roughly 6% of uniformly random 16-byte identities, about 5% of UUIDv4s.This is a public API change, so
docs/api.md,README.mdand the contract test move with it.One mtu rule instead of three
dtls_probehas validated576-16384since it was written; the session validated nothing, andDtlsPskClientvalidates256-65535of its own. Somtu=100used to construct and then raise fromconnect()on a PSK provider while a certificate provider carried on. The probe's check moves todtls_handshake, which both already import from, and the session uses it. Measured:set_ciphertext_mtuconsults the value only when a flight fragments, and OpenSSL takes 100 and 70000 alike, emitting the same 212-byte ClientHello — so the number was invisible until something large went out. Nothing in the library or the bridge passes a non-defaultmtu.Coverage
1316 tests. A session completes a handshake against a real OpenSSL PSK server behind a fake socket, with and without cookie exchange, carrying an identity that leads with a zero byte: the full flight, the identity read back out of the ClientKeyExchange in the clear, application data both ways, one record per datagram as the appliance firmware requires, and a clean
close()emitting one encrypted alert record that the server reports as an orderly close. Those fail if the hook is removed.No appliance hardware has run any of this. @KRZ303's oven is the only PSK hardware in the project and holds the only demonstrated recovered OwnerPSK.
Downstream
localthings#575 already delegates to
PskAuth.validate_identity(), so this release is what makes that PR's behaviour reachable. Their floor issmartthings-local>=0.1.20.