brbzull0 opened a new pull request, #13631:
URL: https://github.com/apache/trafficserver/pull/13631
`Http3SessionAccept::accept()` dispatched on the negotiated ALPN with an
if/else-if chain ending in:
```c
} else {
ink_abort("Negotiated App Name is unknown");
}
```
An absent or empty ALPN lands in that final arm, so a QUIC handshake that
completes without an ALPN extension aborts the process rather than closing
the
connection. `08d1896d6a64` ("Reject TLS if client offers alpn with no match",
#7981) removed the earlier `alpn.empty()` arm that used to catch this case
before it reached the abort.
That the empty state is expected elsewhere in the same path is visible in
both
`_start_application()` implementations, which already have null-ALPN
fallbacks.
### Change
- `Http3SessionAccept.h` / `.cc`: add an `AppType` enum and a static
`select_app_type(std::string_view)` helper, and turn the dispatch into a
`switch`. The `UNKNOWN` arm logs and returns `false`;
`Http3SessionAccept::mainEvent()` already converts a `false` return into
`netvc->do_io_close()`, so no new teardown path is introduced.
- `QUICNetVConnection.cc`: `negotiated_application_name()` declared
`const uint8_t *name;` uninitialised and passed it to
`quiche_conn_application_proto()`, which leaves it untouched when no
protocol
was negotiated -- so the `std::string_view` was constructed over an
indeterminate pointer. Initialise to `nullptr` and return an empty
`string_view` in that case. This also makes `alpn.data()` non-null for the
`%.*s` in the new log line.
Extracting `select_app_type()` keeps the tag-to-application mapping in one
place
and makes it directly unit-testable, though no unit test is added here (see
below).
### Test
Honest summary: **no existing test fails without this change, and a targeted
new
test is not portable.**
An unrecognised tag such as `banana` never reaches `accept()` -- the TLS
layer
answers `no_application_protocol` first, which is what
`tests/gold_tests/tls/tls_bad_alpn.test.py` already covers over TCP. The
empty
ALPN case only completes a handshake on a backend that does not enforce
RFC 9001 s8.1, so a gold test for it would pass or skip inconsistently across
BoringSSL, QUICTLS and the OpenSSL QUIC-TLS-callbacks shim. Rather than add a
backend-dependent test, this is submitted as hardening.
What was run, 9/9 pass: `h3_active_timeout`, `h3_flow_control`,
`h3_go_client`,
`h3_proxy_verifier`, `h3_python_client`, `h3_sni_check`,
`h3_stream_lifetime`,
`quic_no_activity_timeout`, `tls_bad_alpn`. Three of those assert on the
exact
`start HTTP/3 app (ALPN=h3)` debug line, so they confirm the switch
conversion
did not disturb the happy path.
Built on both QUIC backends. Under `ENABLE_QUICHE=ON`,
`src/iocore/net/QUICNetVConnection.cc` compiles clean -- that file is only
built
under `elseif(TS_HAS_QUICHE)` in `src/iocore/net/CMakeLists.txt`, so the
OpenSSL-QUIC build does not cover it. The quiche configuration was
compile-verified only; the autests above ran against the OpenSSL-QUIC build.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
