feat(protocol): discover advertised OCF secure ports

[atc722]

Summary

• add a bounded known-host discover_ocf_secure_ports() API for device-advertised OCF secure ports
• build discovery on the shared CoAP GET, response-classification, option, and Block2 machinery in protocol/coap.py
• query unfiltered /oic/res first for source-bound coaps endpoints, then fall back to /oic/res?rt=oic.r.doxm within the same monotonic deadline
• keep advertisements as candidates only; DTLS liveness and authenticated device identity remain separate checks

What directory discovery adds

probe_dtls_ports() verifies only the candidate ports supplied by its caller. Directory discovery obtains the ports advertised by the device itself, including valid dynamic ports outside a caller's fixed candidate band, and then lets the caller pass those candidates to the existing stateless probe.

This distinction is also visible in the current issue #16 evidence: the advertised secure endpoint can fall outside both the standard OCF port and the bridge's fixed dynamic candidates. Discovery therefore generates candidates; probing verifies listeners. Neither step authenticates a device by itself.

Scope and WD86 context

Following the maintainer feedback, this revision contains only the known-host unicast API. The identity-aware multicast API has been removed from this PR and preserved on atc722:agent/ocf-identity-multicast-held for a later proposal with a concrete caller.

In this discussion, WD86 is shorthand for the observed internal SmartThings/firmware family AWM-KR-M64-24-WD86 of a Samsung washer-dryer combo (oic.d.washer); it is not presented as a retail model number. Redacted testing showed that its OCF host can expose multiple logical device containers, so exact di-to-source-to-endpoint binding is still required by that downstream target-selection path. The package does not yet have a production caller for that multicast API, which is why it is held rather than included here. Device-specific addresses, UUIDs, and advertised port values are intentionally omitted.

Bounds and safety

• one 3-second default monotonic deadline shared by the primary lookup, fallback, and retries
• fresh token, Block2 accumulator, and peer pin for the fallback transfer
• response-source-bound IP-literal coaps endpoints; no DNS lookup of an advertised hostname
• at most 32 Block2 blocks and 64 KiB of accumulated payload
• accepted separate CON responses are acknowledged; malformed, partial, stale-token, and cross-source responses fail closed
• public and internal repr() output omits addresses, tokens, UUIDs, advertised port values, and payload bytes
• no multicast discovery, DTLS probe, authentication attempt, ownership transfer, credential acquisition, control command, or OCF security-resource write

Validation

• python -m pytest -q: 328 passed locally on Python 3.13
• Ruff check on all changed Python files
• git diff --check origin/main...HEAD
• python tools/check_share_safety.py --changed-since origin/main
• wheel and sdist build, distribution-content check, and isolated imports from both artifacts

The repository CI matrix remains responsible for Python 3.11, 3.12, and 3.14 coverage on the updated head.

[QuiteYellow]

Thanks for taking this on.

Two things I'd like to work through before merging, both about scope rather than correctness.

The CoAP layer

ocf_discovery.py carries its own CoAP client: _ResponseBlock, _MAX_BLOCKS = 32, _build_request, _decode_response_block, _option_values, _decode_uint_option. Most of that already exists in protocol/coap.py (build_coap, parse_coap, encode_options, block_value), and the Block2 reassembly loop is a second copy of what DtlsCoapSession.get() does.

Two CoAP implementations in one package means every wire-level fix has to be made twice, and the Samsung-specific ones are the ones that hurt. Token-stable Block2 and the ACK-plus-separate-CON pattern took a long time to get right in the session; a second parser that hasn't met those devices will rediscover them the hard way. Can the discovery path build on coap.py for encode and decode, and keep only the parts that are actually specific to discovery (multicast socket setup, response collection, di matching)?

Two entry points, and what they add over probing

The PR ships discover_ocf_secure_ports() and discover_ocf_secure_ports_multicast() as independent APIs. The reference implementations expose one discovery call with a filter: iotivity-lite does multicast or unicast GET /oic/res, reads the eps array off the returned links, and picks the coaps:// endpoint. Selecting on ?rt=oic.r.doxm is a reasonable fallback for a device that only advertises eps on its security resources, but I'd expect eps to be the primary path and doxm the fallback, not the other way round.

probe_dtls_ports() (added in 1a35cd5) already races the OCF port band and resolves a live secure port in under a second, and the bridge uses it for this today. What does directory discovery do that probing doesn't? Whatever the answer is, put it in the module docstring: it's the reason we'd carry two mechanisms instead of one.

What I'd like to do

Land the known-host unicast path on its own, built on coap.py, reading eps first with the doxm query as fallback. Hold the multicast identity path until something calls it. That gets a smaller change reviewed properly and gives issue #16 something to test against, and the multicast half stays on the branch rather than being thrown away.

Your bounds work is the strongest part of this and I'd keep all of it: the shared deadline across retries, the per-round datagram cap, the exact normalized di match with no pi fallback, and treating advertisements as candidates that still require a stateless DTLS check. Keeping addresses and UUIDs out of the result reprs matches what endpoint.py does and I'm glad you followed it without being asked.

Happy to look at a reduced version whenever you have time. If you'd rather I did the coap.py rewiring myself and pushed it to your branch, say so and I will.

[atc722]

Thanks — I followed the reduced-scope direction and updated the branch myself.

• The PR now exposes only the known-host unicast API. The identity-aware multicast work is preserved on atc722:agent/ocf-identity-multicast-held and will wait for a concrete consumer.
• The branch is rebuilt on the current main / v0.1.6.
• CoAP GET construction, response classification, ACK-plus-separate-CON handling, and bounded token-stable Block2 assembly now live in protocol/coap.py and are shared with DtlsCoapSession.
• Discovery now reads secure, source-bound eps entries from unfiltered GET /oic/res first, and uses GET /oic/res?rt=oic.r.doxm only as a legacy fallback. Both stages share one monotonic deadline.
• The module docstring now explains the distinction from probe_dtls_ports(): directory discovery learns advertised candidates that the caller may not know to probe, while probing only validates the explicit candidates it receives. Advertisements remain candidates and still require stateless DTLS and authenticated device-identity checks before use.

One clarification on the held multicast path: WD86 is shorthand here for the observed internal SmartThings/firmware family AWM-KR-M64-24-WD86 of a Samsung washer-dryer combo (oic.d.washer), not a retail model number. Redacted testing showed multiple logical OCF device containers on its host, so exact di-to-source-to-endpoint binding remains necessary for that downstream target-selection path. I agree that the public API should return with that concrete caller rather than remain in this PR. Device-specific addresses, UUIDs, and port values are omitted.

Validation on the updated head:

• 239 tests passed locally on Python 3.13
• Ruff, git diff --check, and share-safety passed
• exact-head wheel/sdist build, distribution check, and isolated imports passed

The new GitHub Actions run is currently awaiting maintainer approval, so I am not carrying forward the previous head's 8/8 result.

[atc722]

I’ve updated the branch again after rebasing the reduced known-host-only PR onto current main (bc4465b, including #40).

• The PR still contains only the known-host unicast discovery API; the held multicast branch is unchanged.
• The shared CoAP refactor now preserves the current bounded handshake/cancellation and OBSERVE Block2 behavior, including ETag restart and SZX downshift handling.
• I also added a deterministic regression for a stale Block2 response interleaving with the requested current block; the current response is no longer cleared or retransmitted.
• Exact-head local validation: 312 tests passed on Python 3.13, changed-file Ruff, git diff --check, share-safety, wheel/sdist content checks, and isolated imports from both artifacts.

The updated head is 9d221a9. Its new Validate run is awaiting maintainer approval, so I’m not treating the previous head’s successful checks as evidence for this revision: https://github.com/QuiteYellow/SmartThings-Local/actions/runs/32010439678

[vmonkey]

Tested this branch against two older-firmware Samsung devices — a soundbar (oic.d.networkaudio) and a Tizen TV (oic.d.tv). Neither is on the 5683/5684 behaviour from #16. Read-only, no DTLS.

Both work, via the doxm fallback: found=True, one port each, matching the p.port I get decoding /oic/res by hand.

Neither device has an eps array anywhere. Every secured link advertises through p.port / p.x.org.iotivity.tls instead — the older OCF policy-port form. So on these devices the primary path always finds nothing and the fallback always does the work.

That costs a full wasted directory fetch each time: /oic/res unfiltered is ~9 KiB over 9 Block2 blocks, downloaded and discarded, before the fallback asks ?rt=oic.r.doxm and gets the answer in a single 149-byte block. 10 attempts total per device. Both honour the rt= filter, so for a device with no eps, the fallback query alone would have been enough.

One other thing: neither device advertises its plaintext port — unsecured links are "sec": false with no port key. On this firmware the port is ephemeral and changes on every reboot, and 5683 isn't listening, so discovery_port has to be established before this API can be called. The intended route is multicast GET /oic/res to 224.0.1.187, where the reply's source port is the answer; that worked intermittently for me (my AP's IGMP snooping entry for the group ages out, and the TV never joins at all), so I fell back to sweeping with a CON GET /oic/res probe, ~11 s for the full range. Might be worth a docstring line, since the discovery_port=5683 default quietly assumes this is already solved.

Happy to re-run anything against either device.

[atc722]

@vmonkey Thanks — the two-device results and the concrete Block2 measurements are very helpful.

Both observations are valid.

• On legacy devices without eps, an eps-first lookup must consume the complete /oic/res representation before it can safely conclude that no advertised endpoint exists. I kept that ordering because it is the review scope requested above and avoids introducing an automatic firmware heuristic in this PR. A legacy-first opt-in can remain a separate follow-up if the maintainers want that trade-off.
• I have now clarified the public-port contract in the module and function docstrings and in the README: discovery_port=5683 is only a convenience default. If a target does not listen there, the caller must locate its public CoAP request port separately and pass it explicitly. A different response source port is still accepted after target-address and token correlation. feat(protocol): discover known-host OCF responder ports #47 proposes the complementary bounded responder-port discovery path for that case.

The documentation update is now at 8bd4697; it makes no runtime or public API behavior changes.

[atc722]

@QuiteYellow The current head (8bd4697) remains the reduced known-host-only revision requested above: it uses the shared coap.py request, response, and Block2 machinery, reads unfiltered eps first, and uses the DOXM query only as a fallback under the same deadline.

The latest change only clarifies the discovery_port contract in the docstrings and README; it makes no runtime or public API behavior changes.

Additional Family Hub testing also confirmed the value of this path against a moved secure port:
#16 (comment)

Could you approve the new Validate workflow and confirm whether the current reduced revision matches the requested scope?

https://github.com/QuiteYellow/SmartThings-Local/actions/runs/32460555370

[QuiteYellow]

The workflow is approved and green on 8bd4697.

Scope matches. eps first, DOXM as a bounded fallback. The bounds work is the strongest part: one deadline across retries, the per-round datagram cap, exact normalized di matching, and every advertisement still having to survive a DTLS check.

What I do need changed

Four things, all in dtls_session.py, all checked against 8bd4697 today. They are about the session rather than discovery, which is why I want them settled before this lands.

1. _pending_get_mids is not cleared when the reader dies. close() clears it at :533, but the reader-death finally at :667 walks only self._pending. The two maps are torn down together on one path and not the other, so a reader death leaves MID records behind that :717 can still match against.

2. container.clear() on BLOCK2_DUPLICATE drops acknowledged. At :1020, consuming the stale slot wipes the whole container, including the acknowledged flag set at :720. The long separate-response wait at :972 is guarded on that flag, so after a duplicate block the next attempt falls back to _BLOCK_ACK_TIMEOUT. That is a shorter wait on exactly the devices that answer with an empty ACK and a separate CON, which the module docstring calls out as what RT-OCF does for the larger responses. Clearing the response fields while preserving acknowledged would fix it.

3. The Samsung behaviour is now the non-default path. The session builds its accumulator with strict_metadata=False, allow_mid_transfer_error=True and allow_szx_downshift=True at :924, while discovery takes the strict defaults at ocf_discovery.py:400. Each of those flags is a workaround that #38 and #40 arrived at against real appliances. As defaults-off options they read as opt-in leniency, and the next person adding a caller gets the strict behaviour unless they know to ask. I would rather the appliance behaviour stayed the default and the flags did not exist.

4. The test-compatibility branch at :985. The CoapMessage built by hand for "tests/integrations that inject the legacy pending-container fields" is a shape only the tests use, sitting in the request path. That belongs on the test side of the line.

On point 1, hold off before changing anything. #56 covers it: @mbillow's #54 adds _inflight_mids for the write path, which is the same registry from the other end. I would rather the two converged than have you tidy _pending_get_mids on its own.

Point 2 is a live bug rather than style, so I would like that one regardless of how #56 lands. If you would rather split the session changes into their own PR and let the discovery module land on top, that works too, and it would make both easier to review.

The # noqa comments and the socket.timeout to TimeoutError swap are unrelated to either, and dropping them would take a chunk out of the diff for free.