httpd 2.4.x support, request limits, TLS environment, QPACK, 0-RTT - #9
Open
xl32 wants to merge 11 commits into
Open
httpd 2.4.x support, request limits, TLS environment, QPACK, 0-RTT#9xl32 wants to merge 11 commits into
xl32 wants to merge 11 commits into
Conversation
Build against httpd 2.4.52+ as well as trunk. - h3_compat.h keys H3_HAS_RESPONSE_BUCKETS on the same MMN boundary mod_http2 uses (20211221.6). - Without response buckets, remove the core HTTP_HEADER filter from synthesized requests and snapshot response status and headers via h3_response_finalize(), a port of ap_http_header_filter by way of mod_http2's h2_c2_filter.c, minus the HTTP/1.x transport concerns. - Capture status and headers into h3_conn_ctx_t on both paths instead of holding an ap_bucket_response. - Treat the MPM connection-count notifications as optional: stock 2.4.x MPMs lack them, so warn and run in a degraded mode where a graceful child stop does not wait for active QUIC connections to drain, instead of refusing to start. .patches/httpd-2.4.66-pr699.patch adds the notifications to 2.4.x. - Lower the build floor to httpd 2.4.52: ap_create_request (2.4.49), the child_stopping hook (2.4.49) and ap_thread_current (2.4.52) must exist.
RFC 9114 4.1.2 requires a malformed request to be treated as a stream error of type H3_MESSAGE_ERROR. nghttp3 already detects every case (missing, duplicate or invalid pseudo-header fields, connection-specific fields, TE other than trailers, content-length mismatch) and reports it as a non-fatal NGHTTP3_ERR_MALFORMED_HTTP_HEADER/_MESSAGING out of nghttp3_conn_read_stream, but the module treated any negative return as fatal and closed the whole QUIC connection, ending every other request in flight on it. Translate exactly those two errors into a per-stream rejection instead: reset the request stream with the inferred H3_MESSAGE_ERROR code, stop reading it, drop it from nghttp3, and keep the connection serving. All other nghttp3 errors keep the existing connection-error path, as RFC 9114 demands for frame-layer violations. The new checks run only on the error path; the success path is unchanged except for a done-flag test that also stops rejected or already-reset streams from being dispatched to a worker. Tested by test/http3/test_020_malformed.py with a raw QPACK client: each malformed request must yield a stream reset with 0x010E and the same connection must then serve a well-formed request.
The QUIC stacks keep an otherwise idle connection alive on their own: OpenSSL sends a keepalive PING at half the idle interval to hold NAT state open (RFC 9000 s. 10.1.2), which resets both endpoints' idle timers. The transport-level max_idle_timeout therefore never fires and every finished connection stayed on the active list until the peer went away, holding its session pool, its H3MaxConnections slot and, on a patched MPM, its connection count. Track the last application-level progress per session -- a stream accepted, request data read, a request dispatched, response bytes acknowledged -- and close a session with H3_NO_ERROR once it has no worker running and no such progress for H3IdleTimeout seconds. Transport chatter deliberately does not count as progress.
Add H3SocketBufferSize. A UDP receive buffer at the OS default (commonly 208KB) overflows as soon as one connection runs at speed, and each dropped datagram costs a retransmit and a congestion-window cut. The OS caps what it grants, so the module asks, reports a capped or refused grant at info level, and never fails the socket over it. Ported from the pre-h3q tree. The original change also added an optional recv_batch hook to quic_io, implemented with recvmmsg(), to spare the ngtcp2 engine one syscall per datagram. That half is dropped: the ngtcp2 engine no longer exists, and the OpenSSL path drives its own datagram BIO through BIO_recvmmsg() already (see h3q_peer_addr_bio_recvmmsg), so it never used the hook. The unit tests for quic_io_udp go with it.
Session tickets were left entirely to OpenSSL's defaults, so an operator had no way to see or change whether the server hands out resumable sessions, and no answer at all about 0-RTT. Add H3SessionTickets (default on, matching the previous behaviour) which maps to SSL_CTX_set_num_tickets, and set a session id context so tickets this server issues are only resumed against its own sessions. Worth knowing when reading the docs: each worker process keeps its own ticket keys, so a client resumes only when it returns to the process that issued its ticket, and otherwise falls back to a full handshake. 0-RTT is the harder question. OpenSSL's QUIC stack does not accept early data on the server side, so wiring a directive straight to SSL_CTX_set_max_early_data would produce a setting that reads as enabled and does nothing. H3EarlyData therefore defaults to off -- 0-RTT data is replayable by design -- and post_config warns when it is on. Ported from the pre-h3q tree. The original expressed this as an early_data bit in quic_caps that each engine answered for itself. With only the OpenSSL engine left that bit is a constant, so the warning names the OpenSSL stack directly; h3q_config still carries the flag through to the SSL_CTX so an OpenSSL that gains server-side 0-RTT needs no further plumbing here.
mod_ssl does not manage these connections, so a CGI script or a rewrite condition reading SSL_PROTOCOL or SSL_CIPHER over HTTP/3 saw nothing, while the same request over HTTP/1.1 or HTTP/2 answered normally. Only HTTPS=on was set. Add h3q_conn_tls_info(), which reads the negotiated parameters from the connection's SSL object, and set SSL_PROTOCOL, SSL_CIPHER, SSL_CIPHER_USEKEYSIZE, SSL_CIPHER_ALGKEYSIZE, SSL_CIPHER_EXPORT and SSL_SESSION_RESUMED beside the existing HTTPS=on, spelled as mod_ssl spells them, including its "Initial"/"Resumed" wording. The handshake is finished before a session exists, so these values are final at that point: they are formatted once per connection into the session pool, and each request only sets table entries pointing at them. Certificate-derived variables (SSL_SERVER_*, SSL_CLIENT_*) and SSL_SESSION_ID are deliberately not published; they would need X509 parsing and client certificates, which this module does not support. Ported from the pre-h3q tree, where this was an optional conn.tls_info entry in the engine vtable that each engine filled in. With one engine left it is a plain function on h3q_conn.
httpd enforces LimitRequestFields and LimitRequestFieldSize while it parses an HTTP/1 message. HTTP/3 fields arrive here already decoded, so nothing applied either limit: the module advertised nghttp3's default SETTINGS_MAX_FIELD_SECTION_SIZE of (1<<62)-1, and on_recv_header added every field to the stream table with no cap on count or size. A client could grow a stream pool until the child ran out of memory, and the same request over HTTP/1.1 or HTTP/2 was rejected while HTTP/3 accepted it. Count fields and measure each one the way httpd does -- a field sized as "name: value", 0 still meaning unlimited -- and stop storing once either limit is passed, so refusing costs no further memory. The request is answered 431 from the access checker, beside the existing 413 and 400 cases, which keeps it a per-request answer rather than a connection error. Also derive SETTINGS_MAX_FIELD_SECTION_SIZE from the two limits, sizing a field as RFC 9114 4.2.2 does (name + value + 32), so a conforming client stops before it reaches the limit instead of having its request rejected after the fact.
Two ways a client could tie up a child process indefinitely. A client that opened a stream and then stopped reading left its worker thread parked in h3_stream_response_append: the wait had no deadline, so it ended only when the stream was cancelled or the session aborted, and neither happens on its own. The transport stays alive on keepalive PINGs and the idle reaper deliberately skips a session that still has a task running, so with the pool capped at 64 threads, 64 such clients stalled HTTP/3 for that child. Add H3StreamTimeout, defaulting to the server's Timeout the way mod_http2's H2StreamTimeout does, and measure it per chunk of progress rather than over the whole response, so a slow but advancing transfer is untouched while a stalled one is abandoned. Separately, rejecting a malformed request as a stream error -- correct per RFC 9114 4.1.2, and what keeps the connection serving its other streams -- also meant a client could send malformed requests forever, at no cost to itself. Count them per connection and close it past H3MaxStreamErrors (default 8, matching H2MaxStreamErrors) with H3_EXCESSIVE_LOAD.
nghttp3 defaults the QPACK decoder capacity to 0, and the module never
overrode it, so the server told every client it may not use the dynamic
table when encoding request headers. Each request therefore re-sent its
cookies and user-agent literally -- measurably worse header compression
than the same server already gives over HTTP/2, where HPACK's table is
4096 by default. Advertise 4096 with 16 blocked streams, configurable
through H3QpackTableCapacity and H3QpackBlockedStreams; the encoder and
decoder streams this needs were already bound. A capacity of 0 is a
meaningful setting rather than "unset", so it keeps the old behaviour and
the pair tracks whether it was configured separately.
The request worker pool was created with 16 threads growing to 64,
hardcoded, so a large machine could not use more and a small one could
not use fewer. Expose H3MinWorkers, H3MaxWorkers and
H3MaxWorkerIdleSeconds -- the last one had no equivalent at all, and
idle workers were kept forever. Defaults are the previous fixed values,
and a max below min is corrected with a warning rather than being fatal.
Finally, ap_find_linked_module("mod_logio.c") walked the whole loaded
module list comparing names once per connection and again for every
request, to answer a question that cannot change after startup. Resolve
it once from post_config.
Adds reference entries for H3StreamTimeout, H3MaxStreamErrors, H3QpackTableCapacity, H3QpackBlockedStreams, H3MinWorkers, H3MaxWorkers and H3MaxWorkerIdleSeconds, and lists the three that bound resources in the operational limits table. Every registered directive is documented again.
Collapses what were two releases against the pre-h3q tree (v0.0.53 and v0.0.54) into one against upstream v0.0.59. The old numbers are not reused: upstream's own v0.0.52-v0.0.59 are different releases. Two changes from those releases did not come across. Both were ngtcp2-only: the recvmmsg() batch-read hook on quic_io, and resolving the socket's local address once per engine instead of once per pass. Neither has an OpenSSL counterpart -- h3q drives its own datagram BIO through BIO_recvmmsg() and never calls getsockname() -- so nothing was lost with the ngtcp2 engine.
Owner
|
Thanks @xl32. There are a lot of changes so I'll need a few days to review this PR, please be patient with me. Q: Does this PR contain the majority of your proposed changes, or is there other work living on other branches? What of the other PRs? It would help me to review this faster if you can split these changes into isolated versions/PRs to address them piece by piece, and help clarify what depends on what. |
Author
|
@machine-moon thank you! no other work done - all pushed as a proposal for 0.0.60. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.