Skip to content

feat(application): dissect DTLS 1.3 connection-ID unified-header records - #2643

Open
JarryShaw wants to merge 1 commit into
mainfrom
feat/2625-dtls13-cid
Open

JarryShaw wants to merge 1 commit into
mainfrom
feat/2625-dtls13-cid

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • feat — a new feature

Description of your pull request and other information

DTLS 1.3 unified-header records with the C bit set (RFC 9147 §4) now parse as DTLS 1.3 layers. Their connection-ID length comes from #2631's DTLSContext, in one of two ways:

  • IDs that the capture's hellos negotiated, matched by value;
  • a caller-supplied cid_length.

The matcher is #2631's, extended rather than duplicated. A fit needs room for the 16-octet record-number sample, so a shorter learned ID is not misread as a longer one. IDs sent later in DTLS 1.3's encrypted NewConnectionId can come only from the caller; this is documented. With no known length, the record stays Raw quietly. The owner's #2515 rulings hold: memory is per instance, and cid_length is a single value.

Test vectors come from a real wolfSSL 5.9.4 DTLS 1.3 CID exchange, and tshark agrees on every record. Both round trips are byte-exact. The only change to existing output is a cid: None key on unified-header records without a CID; that is option 1 of the open question on #2625. A Sonnet cross-review returned GOOD TO GO, and its hardening and two mutant-killing tests are folded in.

Closes #2625

@JarryShaw JarryShaw added this to the PyPCAPKit v1.6 milestone Nov 2, 2026
@JarryShaw JarryShaw added feat Pull requests that add a new capability (feat: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Nov 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at a0807371b, from a Sonnet cross-review.

  • Layout: it checked the RFC 9147 Figure 3 layout.
  • tls12_cid regression check: 30,000 random match() calls and 6,000 record parses gave the same result on this branch as on main.
  • Prefix adversary: a 16-case prefix grid and 40,000 prefix-heavy records.
  • tshark: re-run against the capture, matching on all 12 frames.
  • Round trips: 5,000 random round trips were byte-exact.

The fix round adds two things the review asked for. The fit now needs room for the 16-octet record-number sample, which removes the prefix ambiguity. Tests now kill the two mutants that survived, M5 (cid_length tried before learned IDs) and M3 (the no-length boundary). The tls12_cid defaults are unchanged.

This branch conflicts with #2638 in DTLS.make(), so whichever merges second needs a rebase.

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 2, 2026
@JarryShaw
JarryShaw force-pushed the feat/2625-dtls13-cid branch from a080737 to e919d1c Compare November 2, 2026 07:57
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Nov 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Fix round and rebase: a0807371b → e919d1c43, now on 16ccc4373 (#2645). Only tests/corekit/test_schema_roundtrip_unit.py changes.

The patch-id changed and the change touches a shared harness, so the label is review: running while a Sonnet re-review checks every schema's chosen instance with and without the pin.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Nov 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES at e919d1c43, from a Sonnet re-review.

  • Required: nothing fails if the pin is removed. With drivers()'s pin deleted, the round-trip module still passes because it builds 2000, the same silent skip this round fixes. Add a guard: no subfield an override sets may be searched, or the case's built octets are 30 01020304 00.
  • Confirmed: across every schema, the pin changes only DTLSCiphertext's two builds, with outcomes identical. PARSE_CONTEXT is needed for the reparse.
  • Optional: a callable override returning a flags dict is not pinned. Reword the docstring to "a dict override", or resolve callables the way _base_values does.

CI on this head is cancelled until the fix round lands.

…rds (#2625)

- A unified header with the C bit set stayed Raw: its CID length is not on
  the wire. DTLSCiphertext gains a cid, read with the length DTLSContext
  finds, and None when C is clear.
- DTLSContext.match takes the unified layout, so both CID record types go
  through the one matcher: learned or caller IDs by value, then cid_length.
  A unified record must leave room for its record-number sample, so a
  shorter ID is not misread as a longer one; tls12_cid is unchanged.
- An unknown length keeps the record Raw, quietly. make() takes cid for a
  unified header; IDs a NewConnectionId issues come from the caller only.
- Schema round trip: an override pins the bit-field subfields it sets, so
  the case sets C; guards check the pin and that PARSE_CONTEXT is read.

Tests: wolfSSL 5.9.4 capture checked with tshark 4.6.9, RFC 9147 vectors.
@JarryShaw
JarryShaw force-pushed the feat/2625-dtls13-cid branch from e919d1c to d86dbdd Compare November 2, 2026 08:11
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Nov 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Fix round: e919d1c43 → d86dbdd1f, same base 16ccc4373.

  • Pin guard: test_an_override_pins_the_bit_field_subfields_it_sets fails when the pin is removed.
  • Context guard: test_each_parse_context_entry_is_read_by_its_case builds each PARSE_CONTEXT case without its context and expects the reparse to fail, so an entry that does nothing fails it.
  • Callable overrides: a new overrides_of resolves them for _base_values, drivers() and the guard alike, so a callable override that returns a flags dict is pinned too.

The label is review: running while the same reviewer checks this round.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Nov 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at d86dbdd1f, from the same Sonnet reviewer.

  • Removing the pin fails both new guards, and removing the case's override fails the context guard.
  • Across every schema, the old and new harness pick identical instances and give identical outcomes. overrides_of resolves exactly as the inline code did, and a callable flags override now pins too.
  • Not blocking: with the pin kept and cid dropped from the override, the case still passes as a degenerate record. Asserting the built octets contain the CID would close it.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Coverage: 94.08% -- tests failure, so this is partial (unit tier, Python 3.15, d86dbdd1f, Unit Tests run failure)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 20561 177 2832 158 98.53%
pcapkit/corekit 2697 74 854 37 96.65%
pcapkit/dumpkit 281 3 100 3 98.43%
pcapkit/foundation 3475 27 1206 32 98.74%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 21439 219 5662 197 98.45%
pcapkit/toolkit 961 28 342 10 96.32%
pcapkit/utilities 512 7 144 5 98.17%
pcapkit/vendor 5256 2260 1198 194 53.61%

Per-file detail: the coverage-html artifact of this run.

This branch has not been deployed

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

Labels

feat Pull requests that add a new capability (feat: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

feat(application): dissect DTLS 1.3 unified-header records that carry a connection ID

1 participant