Skip to content

fix: correct 0-RTT transport param validation guard and test mock#827

Open
cherylsy wants to merge 3 commits into
alibaba:mainfrom
cherylsy:fix-0rtt-param-validation-guard
Open

fix: correct 0-RTT transport param validation guard and test mock#827
cherylsy wants to merge 3 commits into
alibaba:mainfrom
cherylsy:fix-0rtt-param-validation-guard

Conversation

@cherylsy

@cherylsy cherylsy commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Keep the RFC 9000 0-RTT transport-parameter validation on the real production condition: client 0-RTT was attempted and TLS reports early data accepted at xqc_conn_tls_transport_params_cb() time.
  • Extract xqc_conn_validate_0rtt_transport_params() so unit tests can directly cover the remembered-value comparison without mock TLS/CID/encode-decode setup.
  • Restore the RFC 9221 max_datagram_frame_size 0-RTT remembered-value check outside the TLS early-data guard. This preserves the existing case-test behavior where reducing or disabling DATAGRAM support must raise TRA_0RTT_TRANS_PARAMS_ERROR and clear the saved 0-RTT ticket.
  • Fix xqc_test_packet_encrypt_hp_sample_boundary to guard NULL engine/connection pointers so missing local TLS cert files fail the test instead of crashing the CUnit process.
  • Keep the 0-RTT unit tests registered before hp_sample_boundary so their coverage is not hidden by unrelated cert-path failures.

Root cause

0-RTT validation guard

For the RFC 9000 core transport parameters, the production callback should only validate remembered values when the client attempted 0-RTT and TLS reports early data accepted. The earlier || XQC_CONN_FLAG_0RTT_OK workaround was ineffective in production because 0RTT_OK is set after the transport-parameters callback.

The unit-test problem was that mock connections created by test_engine_connect() do not carry real resumption/early-data TLS state, so the callback guard was unreachable in those tests. Directly testing the extracted validator keeps the unit coverage focused on the comparison logic.

DATAGRAM remembered parameter

max_datagram_frame_size follows RFC 9221 Section 3. If the client remembers this transport parameter for 0-RTT, the value received in the new handshake must not be smaller than the remembered value. That check must remain effective for the existing DATAGRAM case tests, including the path where the server disables DATAGRAM after a ticket was generated with DATAGRAM enabled.

hp_sample_boundary SIGSEGV

xqc_test_packet_encrypt_hp_sample_boundary uses test helpers that load ./server.key and ./server.crt. When those files are absent from the current working directory, engine creation can fail. The test now checks those pointers before dereferencing them, so the suite reports a failed assertion instead of crashing.

Test plan

  • make -C build -j4 on local macOS worktree
  • cd build && ./tests/run_tests: 127 tests passed, 0 failed
  • GitHub Actions Build on Ubuntu for the latest PR head commit: run #875 passed

References

@cherylsy
cherylsy force-pushed the fix-0rtt-param-validation-guard branch from 3dd44d8 to 7f11199 Compare June 25, 2026 12:29
@cherylsy cherylsy changed the title fix: widen 0-RTT transport param validation guard fix: correct 0-RTT transport param validation guard and test mock Jun 25, 2026
…irect unit testing

Extract the 8 parameter comparisons (RFC 9000 §7.4.1 + RFC 9221
max_datagram_frame_size) from the inline block inside
xqc_conn_tls_transport_params_cb into a standalone function
xqc_conn_validate_0rtt_transport_params().  This allows unit tests
to exercise the validation logic directly without TLS state mocking,
CID setup, or encode/decode round-trips.

Rewrite the three 0-RTT param validation tests to call the new
function directly, removing the production-code invasion
(early_data_accepted_override in xqc_tls_t) that the previous
approach required.

Co-Authored-By: Claude (claude-opus-4-6) <noreply@anthropic.com>
@cherylsy
cherylsy force-pushed the fix-0rtt-param-validation-guard branch from 7f11199 to 107eab1 Compare June 26, 2026 08:31
cherylsy and others added 2 commits June 30, 2026 12:23
… TLS certs

The test dereferences engine/conn pointers without NULL checks.  When
server.key / server.crt are absent from CWD, xqc_engine_create returns
NULL and the subsequent xqc_engine_packet_process(NULL, ...) triggers
SIGSEGV, killing the entire test process and preventing later tests
(including the 0-RTT param validation suite) from running.

Changes:
- Add NULL guards and goto-cleanup in xqc_test_packet_encrypt_hp_sample_boundary,
  mirroring the defensive pattern already used by xqc_test_empty_pkt
- Move 0-RTT test registration before hp_sample_boundary in main.c so
  they execute regardless of TLS cert availability

Co-Authored-By: Claude (claude-opus-4-6) <noreply@anthropic.com>
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.

1 participant