Skip to content

Bound DTLS handshakes with a monotonic deadline - #34

Merged
QuiteYellow merged 1 commit into
QuiteYellow:mainfrom
Moballo-LLC:codex/py-08a-bounded-connect
Aug 15, 2026
Merged

Bound DTLS handshakes with a monotonic deadline#34
QuiteYellow merged 1 commit into
QuiteYellow:mainfrom
Moballo-LLC:codex/py-08a-bounded-connect

Conversation

@Jason-Morcos

@Jason-Morcos Jason-Morcos commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Problem

connect() measures its handshake budget with time.time(). A clock step during Home Assistant startup can therefore expire a connection immediately or let it run well past its intended bound. The session also maintains a second, fixed-cadence handshake loop beside the diagnostic probe's OpenSSL-timed loop.

Changes

  • Add keyword-only connect(timeout=...), retaining the 12-second default.
  • Validate timeout values as positive, finite numbers.
  • Start one monotonic deadline before authentication context and socket setup.
  • Extract the memory-BIO handshake driver and use it from both connect() and diagnose_dtls_handshake().
  • Let OpenSSL's DTLSv1_get_timeout() / DTLSv1_handle_timeout() schedule retransmissions instead of sleeping 50 ms and retrying on a flat two-second cadence.
  • Cap every network wait to 500 ms and the remaining total budget.
  • Preserve the diagnostic probe's explicit retransmission limit while normal sessions remain bounded by the total deadline.
  • Reject a handshake reported successful at or after the deadline.
  • Leave reader timeouts and post-connect behavior unchanged.

Expiry continues to use the existing SessionTimeoutError.

Validation

  • 186 tests with current dependencies.
  • 186 tests on Python 3.11 with pyOpenSSL 23.1.
  • All 1,070 LocalThings tests against this exact checkout.
  • Focused Ruff, compile, share-safety, distribution-content checks, and isolated wheel/sdist imports.

The deterministic timing tests cover malformed values, context and socket setup time, shortened final waits, success at and after expiry, the default timeout, and an OpenSSL-timed retransmission after a dropped flight.

Independence

This is one signed commit, a44930f, directly on current main (b0d51ab / v0.1.6). It has no dependency on #33. Connection-attempt cancellation remains separate in #35.

@QuiteYellow

Copy link
Copy Markdown
Owner

The wall-clock problem this fixes is worse than the PR body suggests, and I want to record why before anything else.

connect() measures its budget with time.time(). Home Assistant hosts step the clock at boot, right when the integration is opening its first sessions. A forward step of more than twelve seconds expires the budget on the first loop iteration and the handshake fails without a single retry. A backward step runs the loop far past its bound. Issue #37 is titled "will not reconnect on HA startup", and this is one of the mechanisms that produces exactly that shape. Monotonic is the right call.

Before it goes in, one question about how the timing is done.

dtls_probe.py:603-653 already has this loop. It takes a monotonic deadline, clamps each receive to min(0.5, remaining), and services OpenSSL's own DTLS retransmit timer:

to = conn.DTLSv1_get_timeout()
if to is not None and to <= 0:
    if retransmits >= retries:
        break
    conn.DTLSv1_handle_timeout()
    retransmits += 1

The pyOpenSSL floor was raised to 23.1 in 2333899 specifically to guarantee those two calls. connect() uses neither. It sleeps 50 ms per iteration, holds the socket at a flat 2 s, and never touches the retransmit timer, so a dropped flight is only recovered by luck of the cadence.

This PR keeps that cadence and adds a deadline around it, which gives us two hand-rolled timing schemes in one package. Could connect() call the probe's loop instead? Extract it once, both callers use it, and the session gets RFC 6347 §4.2.4.1 backoff instead of a fixed two-second beat. That's how libcoap's OpenSSL backend drives DTLS, and how iotivity-lite drives mbedTLS through mbedtls_ssl_set_timer_cb: the stack owns retransmission, the application owns only the total deadline. The diff should come out smaller than the current one, since time.sleep(0.05) and the flat socket timeout both go away.

I'm raising it now rather than as a follow-up because test_session_connect_deadline.py asserts the two-second cadence in several places. Once that's merged, changing the cadence means rewriting tests that are documenting behaviour we'd rather not have.

On ordering. Your stack has this behind #33, but 5ecfb71 touches dtls_session.py, test_endpoint.py, test_public_api_contract.py and the new deadline tests. No auth.py. So it rebases onto main cleanly, worst case a small conflict in the public-API contract test.

I'd like to do that and land this first. #33 has a confirmed field failure open against it (vmonkey's non-DER leaf) and no consumer in either this repo's bridge or mbillow/localthings today. This PR has users hitting the wall-clock path right now. If #33's parser fix turns around quickly then the order doesn't matter much and I'm happy to take your original sequence. If it takes longer, I don't want the startup-reconnect fix waiting behind a certificate profile nobody is calling yet.

Happy to rebase myself if you'd prefer.

@Jason-Morcos
Jason-Morcos force-pushed the codex/py-08a-bounded-connect branch from 5ecfb71 to a44930f Compare August 14, 2026 21:38
@Jason-Morcos

Copy link
Copy Markdown
Contributor Author

That makes sense. I’ve rebased this directly onto current main, so it no longer depends on #33.

I also removed the separate fixed-cadence handshake loop. connect() and the diagnostic path now use one shared memory-BIO driver: the application owns the total monotonic deadline, while OpenSSL’s DTLSv1_get_timeout() / DTLSv1_handle_timeout() control flight retransmission. The old 50 ms sleep and flat two-second cadence are gone.

Good catch! Let me know if you find anything else

@QuiteYellow
QuiteYellow merged commit 917b0e4 into QuiteYellow:main Aug 15, 2026
8 checks passed
QuiteYellow added a commit that referenced this pull request Aug 15, 2026
- add dtls_handshake.py (from #34) to the repo-layout tree
- point the issue #16 / #20 notes at SamsungServerProfile / ServerCertificateAuth instead of calling that path unsupported
- list the new certificate-profile, connect-deadline, and session-interruption test modules
@QuiteYellow

Copy link
Copy Markdown
Owner

Merged in v0.1.7, thanks.

I smoke-tested the whole stack (#34/#35/#33) on a live dryer and oven before merging. Both handshakes completed ~6s after a container recreate, and steady-state polling held at 0 errors with no forced reconnect. The retransmit servicing you added is new behaviour on these devices (the old connect() loop never called DTLSv1_handle_timeout).

One minor thing for a follow-up, non-blocking: in _drive_dtls_handshake, the success path does return time.monotonic() < deadline right after do_handshake() returns. A handshake that completes a hair past the deadline gets reported as a timeout, so connect() tears down a session that actually completed and raises SessionTimeoutError. Returning True unconditionally once do_handshake() succeeds would keep that completed session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants