diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index f14b03b5449..2aa0617e5da 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -5102,6 +5102,13 @@ HTTP/2 Configuration code of ENHANCE_YOUR_CALM. Any negative value configures no limit to the number of SETTINGS frames received. + SETTINGS frames carrying the ACK flag are not counted against this limit. + They are mandatory protocol responses to SETTINGS frames |TS| sent and + therefore cannot be used by a peer to flood |TS|; counting them would + spuriously close healthy connections (in particular outbound HTTP/2 + sessions where :ts:cv:`proxy.config.http2.flow_control.policy_out` causes + |TS| to send a SETTINGS frame per outbound stream). + .. ts:cv:: CONFIG proxy.config.http2.max_ping_frames_per_minute INT 60 :reloadable: diff --git a/include/proxy/ProxyTransaction.h b/include/proxy/ProxyTransaction.h index 7665392ec50..1b2b010747d 100644 --- a/include/proxy/ProxyTransaction.h +++ b/include/proxy/ProxyTransaction.h @@ -146,6 +146,22 @@ class ProxyTransaction : public VConnection virtual void set_rx_error_code(ProxyError e); virtual void set_tx_error_code(ProxyError e); + /** Whether the request on this transaction is known by the protocol layer to + * be safe to retry on a fresh origin connection. + * + * The default is @c false. A subclass should return @c true only when the + * underlying protocol guarantees the origin did not process (and could not + * have observed) the request -- for example, when an HTTP/2 origin sends a + * GOAWAY whose last_stream_id is below this transaction's stream id, or + * sends a RST_STREAM with the REFUSED_STREAM error code (RFC 9113 6.8 and + * 8.7). HttpSM uses this to allow retrying non-idempotent methods that + * would otherwise be considered too risky to replay. + * + * @return @c true if HttpSM may safely retry the request on a different + * origin connection regardless of method. + */ + virtual bool is_safe_to_retry() const; + bool support_sni() const; void mark_as_tunnel_endpoint() override; diff --git a/include/proxy/http2/Http2CommonSession.h b/include/proxy/http2/Http2CommonSession.h index 0b1d9b8112b..bafd9dbd273 100644 --- a/include/proxy/http2/Http2CommonSession.h +++ b/include/proxy/http2/Http2CommonSession.h @@ -101,14 +101,14 @@ class Http2CommonSession //////////////////// // Accessors - void set_dying_event(int event); - int get_dying_event() const; - bool ready_to_free() const; - bool is_recursing() const; - void set_half_close_local_flag(bool flag); - bool get_half_close_local_flag() const; - bool is_url_pushed(const char *url, int url_len); - void add_url_to_pushed_table(const char *url, int url_len); + void set_dying_event(int event); + int get_dying_event() const; + bool ready_to_free() const; + bool is_recursing() const; + virtual void set_half_close_local_flag(bool flag); + bool get_half_close_local_flag() const; + bool is_url_pushed(const char *url, int url_len); + void add_url_to_pushed_table(const char *url, int url_len); // Record history from Http2ConnectionState void remember(const SourceLocation &location, int event, int reentrant = NO_REENTRANT); diff --git a/include/proxy/http2/Http2ServerSession.h b/include/proxy/http2/Http2ServerSession.h index 5b98c78e8e9..07cf3884673 100644 --- a/include/proxy/http2/Http2ServerSession.h +++ b/include/proxy/http2/Http2ServerSession.h @@ -58,6 +58,20 @@ class Http2ServerSession : public PoolableSession, public Http2CommonSession void add_session() override; void remove_session(); + /** Eagerly drop a half-closed session out of the server-session pool. + * + * Sets the half-close-local flag (delegating to the base class) and, when + * the flag transitions to @c true, immediately removes this session from + * the per-thread server-session pool. Once @c half_close_local is set, + * every subsequent @c create_initiating_stream on this session + * short-circuits with @c REFUSED_STREAM, so leaving it discoverable in + * the pool produces a stream of REFUSED_STREAM aborts on otherwise + * unrelated requests until the session is finally torn down. The pool + * must be made aware as soon as the half-close decision is made, not at + * session destruction time. + */ + void set_half_close_local_flag(bool flag) override; + //////////////////// // Accessors sockaddr const *get_remote_addr() const override; diff --git a/include/proxy/http2/Http2Stream.h b/include/proxy/http2/Http2Stream.h index dbf64edb4b9..953c279f9f9 100644 --- a/include/proxy/http2/Http2Stream.h +++ b/include/proxy/http2/Http2Stream.h @@ -156,6 +156,19 @@ class Http2Stream : public ProxyTransaction void set_rx_error_code(ProxyError e) override; void set_tx_error_code(ProxyError e) override; + bool is_safe_to_retry() const override; + + /** Mark this stream as known-not-processed by the origin. + * + * Called by Http2ConnectionState when the origin has explicitly indicated + * that this stream's request was not (and will not be) processed -- either + * because a GOAWAY arrived whose @c last_stream_id is below this stream's + * id, or because a RST_STREAM with REFUSED_STREAM was received for it (RFC + * 9113 sections 6.8 and 8.7). HttpSM consults @c is_safe_to_retry() when + * deciding whether a non-idempotent request may be retried. + */ + void set_safe_to_retry(); + bool has_request_body(int64_t content_length, bool is_chunked_set) const override; HTTPVersion get_version(HTTPHdr &hdr) const override; @@ -163,6 +176,7 @@ class Http2Stream : public ProxyTransaction void increment_data_length(uint64_t length); bool payload_length_is_valid() const; + void cache_send_request_for_response_validation(); bool is_write_vio_done() const; void update_sent_count(unsigned num_bytes); Http2StreamId get_id() const; @@ -260,6 +274,13 @@ class Http2Stream : public ProxyTransaction /** Whether the stream has been registered with the connection state. */ bool _registered_stream = true; + // Set by Http2ConnectionState when the origin has explicitly indicated + // (via GOAWAY whose last_stream_id is below this stream's id, or via + // RST_STREAM with REFUSED_STREAM) that this stream's request was not + // processed and may be safely retried even for non-idempotent methods. + // See Http2Stream::is_safe_to_retry / set_safe_to_retry. + bool _safe_to_retry = false; + // A brief discussion of similar flags and state variables: _state, closed, terminate_stream // // _state tracks the HTTP2 state of the stream. This field completely coincides with the H2 spec. @@ -286,6 +307,17 @@ class Http2Stream : public ProxyTransaction uint64_t data_length = 0; uint64_t bytes_sent = 0; + // Snapshot of the send-side request taken before `_send_header` is destroyed + // in `update_write_request`. These are used by `payload_length_is_valid` to + // apply the [RFC 9110] 8.6 / [RFC 7230] 3.3.2 payload preclusion rules to + // origin responses on outbound streams. Without this snapshot the request + // method (e.g. HEAD) and the presence of conditional request headers would + // already be lost by the time the response is validated, causing valid HEAD + // and 304 responses with non-zero Content-Length to be rejected as protocol + // errors. + int _cached_send_method_wksidx = -1; + uint64_t _cached_send_conditional_field = 0; + ssize_t _peer_rwnd = 0; ssize_t _local_rwnd = 0; @@ -398,18 +430,50 @@ Http2Stream::increment_data_length(uint64_t length) data_length += length; } +inline void +Http2Stream::cache_send_request_for_response_validation() +{ + // On outbound streams `_send_header` is destroyed in `update_write_request` + // immediately after the request HEADERS frame is encoded and sent. Capture + // the request method and the presence of any conditional request headers + // here so that the response-side `payload_length_is_valid` check can still + // honor the [RFC 9110] 8.6 payload preclusion rules for HEAD responses and + // for 304 responses to conditional GETs. + if (!this->is_outbound_connection() || !_send_header.valid() || _send_header.type_get() != HTTPType::REQUEST) { + return; + } + uint64_t const conditional_mask = (MIME_PRESENCE_IF_UNMODIFIED_SINCE | MIME_PRESENCE_IF_MODIFIED_SINCE | MIME_PRESENCE_IF_RANGE | + MIME_PRESENCE_IF_MATCH | MIME_PRESENCE_IF_NONE_MATCH); + _cached_send_method_wksidx = _send_header.method_get_wksidx(); + _cached_send_conditional_field = _send_header.presence(conditional_mask); +} + inline bool Http2Stream::payload_length_is_valid() const { - uint32_t content_length = _receive_header.get_content_length(); - uint64_t mask = (MIME_PRESENCE_IF_UNMODIFIED_SINCE | MIME_PRESENCE_IF_MODIFIED_SINCE | MIME_PRESENCE_IF_RANGE | - MIME_PRESENCE_IF_MATCH | MIME_PRESENCE_IF_NONE_MATCH); - - // Skip Content-Length check on [RFC 7230] 3.3.2 conditions - bool is_payload_precluded = - this->is_outbound_connection() && (_send_header.method_get_wksidx() == HTTP_WKSIDX_HEAD || - (_send_header.method_get_wksidx() == HTTP_WKSIDX_GET && _send_header.presence(mask) && - _receive_header.status_get() == HTTPStatus::NOT_MODIFIED)); + uint32_t const content_length = _receive_header.get_content_length(); + + // Apply the [RFC 9110] 8.6 / [RFC 7230] 3.3.2 payload preclusion rules to + // origin responses on outbound streams. The send-side `_send_header` may + // already have been torn down by this point, so consult the cached + // request metadata captured by `cache_send_request_for_response_validation`. + bool is_payload_precluded = false; + if (this->is_outbound_connection()) { + if (_cached_send_method_wksidx == HTTP_WKSIDX_HEAD) { + is_payload_precluded = true; + } else if (_cached_send_method_wksidx == HTTP_WKSIDX_GET && _cached_send_conditional_field != 0) { + // `HTTPHdr::status_get()` asserts the underlying header has response + // polarity, but on the outbound origin-response path `_receive_header` + // is still in HTTP/2 form at this point and has not yet been converted, + // so the polarity may not be set. Read the `:status` pseudo-header + // directly to detect a 304 response to a conditional GET. + if (MIMEField const *const status_field = _receive_header.field_find(PSEUDO_HEADER_STATUS); status_field != nullptr) { + auto const sv{status_field->value_get()}; + HTTPStatus const status = http_parse_status(sv.data(), sv.data() + sv.length()); + is_payload_precluded = (status == HTTPStatus::NOT_MODIFIED); + } + } + } if (content_length != 0 && !is_payload_precluded && content_length != data_length) { Warning("Bad payload length content_length=%d data_legnth=%d session_id=%" PRId64, content_length, diff --git a/src/proxy/ProxyTransaction.cc b/src/proxy/ProxyTransaction.cc index 05fb0c8877e..6993c5cdc8f 100644 --- a/src/proxy/ProxyTransaction.cc +++ b/src/proxy/ProxyTransaction.cc @@ -95,6 +95,12 @@ ProxyTransaction::set_tx_error_code(ProxyError e) } } +bool +ProxyTransaction::is_safe_to_retry() const +{ + return false; +} + NetVConnection * ProxyTransaction::get_netvc() const { diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 799f72b771a..8d3c9be5459 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -6430,7 +6430,26 @@ HttpSM::handle_server_setup_error(int event, void *data) [[maybe_unused]] UnixNetVConnection *dbg_vc = nullptr; switch (event) { case VC_EVENT_EOS: - t_state.current.state = HttpTransact::CONNECTION_CLOSED; + // If the underlying transport (e.g. an HTTP/2 stream) has signaled that + // the origin guaranteed it never processed this request -- because of a + // GOAWAY whose last_stream_id is below this stream's id, or a RST_STREAM + // with REFUSED_STREAM (RFC 9113 6.8 and 8.7) -- treat the failure as a + // connection-level error so HttpTransact::is_request_retryable allows + // retrying even non-idempotent methods such as POST. Otherwise this + // would surface to the client as ERR_CLIENT_ABORT despite the request + // being explicitly safe to replay on a fresh connection. + if (server_txn != nullptr && server_txn->is_safe_to_retry()) { + t_state.current.state = HttpTransact::CONNECTION_ERROR; + // A retry with a request body needs to use the complete copy retained + // by request buffering. The first origin attempt clears + // is_buffering_request_body after consuming that copy, so restore the + // flag before setting up the retry's request-body tunnel. + if (this->is_postbuf_valid() && this->get_postbuf_done()) { + is_buffering_request_body = true; + } + } else { + t_state.current.state = HttpTransact::CONNECTION_CLOSED; + } t_state.set_connect_fail(EPIPE); break; case VC_EVENT_ERROR: diff --git a/src/proxy/http/HttpSessionManager.cc b/src/proxy/http/HttpSessionManager.cc index 8827f60c2eb..9a4f4dce5f4 100644 --- a/src/proxy/http/HttpSessionManager.cc +++ b/src/proxy/http/HttpSessionManager.cc @@ -420,19 +420,27 @@ HttpSessionManager::acquire_session(HttpSM *sm, sockaddr const *ip, const char * to_return = nullptr; } - // Otherwise, check the thread pool first - if (this->get_pool_type() == TS_SERVER_SESSION_SHARING_POOL_THREAD || - this->get_pool_type() == TS_SERVER_SESSION_SHARING_POOL_HYBRID) { - retval = _acquire_session(ip, hostname_hash, sm, match_style, TS_SERVER_SESSION_SHARING_POOL_THREAD); - } + // Always check the thread-local pool first. Multiplexing server sessions + // (HTTP/2, HTTP/3) cannot be safely shared across threads -- their state is + // owned by the EThread that drives their connection -- so they are filed + // exclusively in the per-thread pool by `Http2ServerSession::add_session` + // (and analogous code for HTTP/3). If the configured pool type is `global` + // or `global_locked`, only the global pool would otherwise be consulted, + // which means an existing H/2 origin connection on this thread is invisible + // to the lookup. Each new request would then open a fresh TCP+TLS+H/2 + // handshake to the origin, defeating multiplexing entirely. Trying the + // thread-local pool first restores within-thread H/2 origin reuse without + // changing behavior for HTTP/1.x sessions, which fall through to the + // configured pool below on a thread-local miss. + retval = _acquire_session(ip, hostname_hash, sm, match_style, TS_SERVER_SESSION_SHARING_POOL_THREAD); - // If you didn't get a match, and the global pool is an option go there. if (retval != HSMresult_t::DONE) { if (TS_SERVER_SESSION_SHARING_POOL_GLOBAL == this->get_pool_type() || TS_SERVER_SESSION_SHARING_POOL_HYBRID == this->get_pool_type()) { retval = _acquire_session(ip, hostname_hash, sm, match_style, TS_SERVER_SESSION_SHARING_POOL_GLOBAL); - } else if (TS_SERVER_SESSION_SHARING_POOL_GLOBAL_LOCKED == this->get_pool_type()) + } else if (TS_SERVER_SESSION_SHARING_POOL_GLOBAL_LOCKED == this->get_pool_type()) { retval = _acquire_session(ip, hostname_hash, sm, match_style, TS_SERVER_SESSION_SHARING_POOL_GLOBAL_LOCKED); + } } return retval; diff --git a/src/proxy/http2/Http2ConnectionState.cc b/src/proxy/http2/Http2ConnectionState.cc index cb448000145..cae861f9b2f 100644 --- a/src/proxy/http2/Http2ConnectionState.cc +++ b/src/proxy/http2/Http2ConnectionState.cc @@ -710,6 +710,16 @@ Http2ConnectionState::rcv_rst_stream_frame(const Http2Frame &frame) Http2StreamDebug(this->session, stream_id, "Parsed RST_STREAM frame: Error Code: %u", rst_stream.error_code); ATS_PROBE3(http2_rst_stream_rcvd, this->session->get_connection_id(), stream_id, rst_stream.error_code); stream->set_rx_error_code({ProxyErrorClass::TXN, static_cast(rst_stream.error_code)}); + // Per RFC 9113 8.7: REFUSED_STREAM is the one stream-level error code with + // an explicit guarantee that the request was not processed by the peer, + // and so the request is safe to retry on a fresh connection -- including + // for non-idempotent methods. Tag the stream so HttpSM converts the + // resulting EOS into a connection-level retry rather than surfacing it as + // ERR_CLIENT_ABORT to the client. + if (this->session->is_outbound() && + static_cast(rst_stream.error_code) == Http2ErrorCode::HTTP2_ERROR_REFUSED_STREAM) { + stream->set_safe_to_retry(); + } stream->initiating_close(); } @@ -730,18 +740,6 @@ Http2ConnectionState::rcv_settings_frame(const Http2Frame &frame) Warning("Setting frame for zombied session %" PRId64, this->session->get_connection_id()); } - // Update SETTINGS frame count per minute - this->increment_received_settings_frame_count(); - // Close this connection if its SETTINGS frame count exceeds a limit - if (configured_max_settings_frames_per_minute >= 0 && - this->get_received_settings_frame_count() > static_cast(configured_max_settings_frames_per_minute)) { - Metrics::Counter::increment(http2_rsb.max_settings_frames_per_minute_exceeded); - Http2StreamDebug(this->session, stream_id, "Observed too frequent SETTINGS frames: %u frames within a last minute", - this->get_received_settings_frame_count()); - return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM, - "recv settings too frequent SETTINGS frames"); - } - // [RFC 7540] 6.5. The stream identifier for a SETTINGS frame MUST be zero. // If an endpoint receives a SETTINGS frame whose stream identifier field is // anything other than 0x0, the endpoint MUST respond with a connection @@ -754,6 +752,21 @@ Http2ConnectionState::rcv_settings_frame(const Http2Frame &frame) // [RFC 7540] 6.5. Receipt of a SETTINGS frame with the ACK flag set and a // length field value other than 0 MUST be treated as a connection // error of type FRAME_SIZE_ERROR. + // + // SETTINGS-ACK frames are intentionally not counted against + // `max_settings_frames_per_minute`. The rate limit exists to defend + // against a peer flooding us with SETTINGS updates (RFC 9113 6.5: each + // received SETTINGS forces us to apply state changes and queue an ACK), + // but a SETTINGS-ACK is a mandatory protocol response to a SETTINGS we + // ourselves sent and cannot arrive faster than our own send rate. With + // `proxy.config.http2.flow_control.policy_out=2` + // (LARGE_SESSION_AND_DYNAMIC_STREAM) ATS sends a SETTINGS frame at the + // start of every outbound stream, and the origin returns one ACK per + // stream; counting those inbound ACKs against the receive limit can trip + // the limit on a healthy origin connection within seconds and tear down + // a perfectly good multiplexed session with ENHANCE_YOUR_CALM. nghttp2 + // applies the analogous defense (`max_outbound_ack`) to the *outbound* + // ACK queue depth, not to the inbound ACK count, for the same reason. if (frame.header().flags & HTTP2_FLAGS_SETTINGS_ACK) { if (frame.header().length == 0) { return this->_process_incoming_settings_ack_frame(); @@ -763,6 +776,18 @@ Http2ConnectionState::rcv_settings_frame(const Http2Frame &frame) } } + // Update SETTINGS frame count per minute (non-ACK only; see above). + this->increment_received_settings_frame_count(); + // Close this connection if its SETTINGS frame count exceeds a limit. + if (configured_max_settings_frames_per_minute >= 0 && + this->get_received_settings_frame_count() > static_cast(configured_max_settings_frames_per_minute)) { + Metrics::Counter::increment(http2_rsb.max_settings_frames_per_minute_exceeded); + Http2StreamDebug(this->session, stream_id, "Observed too frequent SETTINGS frames: %u frames within a last minute", + this->get_received_settings_frame_count()); + return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM, + "recv settings too frequent SETTINGS frames"); + } + // A SETTINGS frame with a length other than a multiple of 6 octets MUST // be treated as a connection error (Section 5.4.1) of type // FRAME_SIZE_ERROR. @@ -918,6 +943,24 @@ Http2ConnectionState::rcv_goaway_frame(const Http2Frame &frame) Http2StreamDebug(this->session, stream_id, "GOAWAY: last stream id=%d, error code=%d", goaway.last_streamid, static_cast(goaway.error_code)); + // Per RFC 9113 6.8: streams whose id is greater than `last_streamid` were + // not (and will not be) processed by the peer, and the requests they carry + // may be safely retried on a fresh connection. On an outbound H/2 session + // the streams in question are the ones we initiated toward the origin + // (peer-initiated-by-us), which use the "client" stream id space. Tagging + // them here -- before do_io_close() tears the streams down -- lets HttpSM + // decide to retry non-idempotent requests (e.g. POST) that would otherwise + // surface to the client as ERR_CLIENT_ABORT. This is especially important + // for AWS-style origin load balancers that aggressively send + // GOAWAY(last_stream_id=0, NO_ERROR) when draining a connection. + if (this->session->is_outbound()) { + for (Http2Stream *s = stream_list.head; s != nullptr; s = static_cast(s->link.next)) { + if (http2_is_client_streamid(s->get_id()) && s->get_id() > goaway.last_streamid) { + s->set_safe_to_retry(); + } + } + } + this->rx_error_code = {ProxyErrorClass::SSN, static_cast(goaway.error_code)}; this->session->get_proxy_session()->do_io_close(); @@ -1069,12 +1112,26 @@ Http2ConnectionState::rcv_continuation_frame(const Http2Frame &frame) "continuation stream freed with invalid id"); } } else { + bool const is_outbound = this->session->is_outbound(); switch (stream->get_state()) { case Http2StreamState::HTTP2_STREAM_STATE_HALF_CLOSED_REMOTE: return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_STREAM_CLOSED, "continuation half close remote"); case Http2StreamState::HTTP2_STREAM_STATE_IDLE: break; + case Http2StreamState::HTTP2_STREAM_STATE_OPEN: + case Http2StreamState::HTTP2_STREAM_STATE_HALF_CLOSED_LOCAL: + // On outbound (origin-side) connections, response HEADERS may be split + // across CONTINUATION frames. The associated stream is OPEN if our + // request body is still in flight, or HALF_CLOSED_LOCAL once we have + // sent the request with END_STREAM (e.g. for GET or HEAD). [RFC 7540] + // 6.10 only forbids interleaving CONTINUATION with frames of other + // types or other streams, not these stream states. + if (!is_outbound) { + return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR, + "continuation bad state"); + } + break; default: return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR, "continuation bad state"); @@ -1159,13 +1216,24 @@ Http2ConnectionState::rcv_continuation_frame(const Http2Frame &frame) } // Set up the State Machine - SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread()); - stream->mark_milestone(Http2StreamMilestone::START_TXN); - // This should be fine, need to verify whether we need to replace this with the - // "from_early_data" flag from the associated HEADERS frame. - stream->new_transaction(frame.is_from_early_data()); - // Send request header to SM - stream->send_headers(*this); + if (!stream->is_outbound_connection() && !stream->trailing_header_is_possible()) { + SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread()); + stream->mark_milestone(Http2StreamMilestone::START_TXN); + stream->cancel_active_timeout(); + stream->new_transaction(frame.is_from_early_data()); + // Send request header to SM + stream->send_headers(*this); + } else { + // If this is a trailer, first signal to the SM that the body is done + if (stream->trailing_header_is_possible()) { + stream->set_expect_receive_trailer(); + // Propagate the trailer header + stream->send_headers(*this); + } else { + // Propagate the response + stream->send_headers(*this); + } + } // Give a chance to send response before reading next frame. this->session->interrupt_reading_frames(); } else { @@ -2117,7 +2185,15 @@ Http2ConnectionState::delete_stream(Http2Stream *stream) if (http2_is_client_streamid(stream->get_id())) { ink_release_assert(peer_streams_count_in > 0); --peer_streams_count_in; - if (!fini_received && is_peer_concurrent_stream_lb()) { + // Do not put a session that has already entered local half-close back in + // the pool. Once `set_half_close_local_flag(true)` has been called (for + // example because we have started a graceful GOAWAY) every subsequent + // `create_initiating_stream` on this session will fast-fail with + // REFUSED_STREAM, so handing it back out via `acquire_session` only + // causes spurious aborts. The session will be torn down once its + // remaining in-flight streams finish; until then it must stay out of + // the pool. + if (!fini_received && !session->get_half_close_local_flag() && is_peer_concurrent_stream_lb()) { session->add_session(); } } else { diff --git a/src/proxy/http2/Http2ServerSession.cc b/src/proxy/http2/Http2ServerSession.cc index e3e550e1bf9..075127ed86b 100644 --- a/src/proxy/http2/Http2ServerSession.cc +++ b/src/proxy/http2/Http2ServerSession.cc @@ -375,6 +375,27 @@ Http2ServerSession::remove_session() } } +void +Http2ServerSession::set_half_close_local_flag(bool flag) +{ + // Detect the OFF -> ON transition before delegating to the base, which + // is what flips the underlying flag. + bool const transitioning_to_half_close = !this->get_half_close_local_flag() && flag; + Http2CommonSession::set_half_close_local_flag(flag); + if (transitioning_to_half_close) { + // Once `half_close_local` is set, `create_initiating_stream` on this + // session short-circuits to REFUSED_STREAM. Any future + // `acquire_session` lookup that finds this session in the pool will + // therefore hand back a session that immediately fails the next + // origin request with `HTTP/2 stream error code=0x07 refused to + // create new stream, because session is in half_close state`. Evict + // it now so the next request opens (or matches) a healthy session + // instead. The session itself stays alive long enough for any + // already-attached transactions to drain. + this->remove_session(); + } +} + bool Http2ServerSession::is_multiplexing() const { diff --git a/src/proxy/http2/Http2Stream.cc b/src/proxy/http2/Http2Stream.cc index 6be67db03b9..23fd89fe255 100644 --- a/src/proxy/http2/Http2Stream.cc +++ b/src/proxy/http2/Http2Stream.cc @@ -867,14 +867,25 @@ Http2Stream::update_write_request(bool call_update) this->parsing_header_done = true; Http2StreamDebug("update_write_request parsing done, read %d bytes", bytes_used); - // Schedule session shutdown if response header has "Connection: close" - MIMEField *field = this->_send_header.field_find(static_cast(MIME_FIELD_CONNECTION)); - if (field) { - auto value{field->value_get()}; - if (value == static_cast(HTTP_VALUE_CLOSE)) { - SCOPED_MUTEX_LOCK(lock, _proxy_ssn->mutex, this_ethread()); - if (connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NONE) { - connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED, Http2ErrorCode::HTTP2_ERROR_NO_ERROR); + // Only honor `Connection: close` -> graceful-shutdown on the inbound + // (client-facing) H/2 session, where the header lives on a response we + // are about to send back to the client and is a per-client signal to + // drain the H/2 client session. On an outbound (origin-facing) H/2 + // session the same parsing path runs against the outgoing *request*, + // and a single client request carrying `Connection: close` must not + // be allowed to graceful-shutdown a shared H/2 origin connection that + // is multiplexing many other clients' transactions. (`Connection` is + // also a hop-by-hop header forbidden on the H/2 wire by RFC 9113 + // 8.2.2, so it cannot represent an end-to-end origin signal here.) + if (!this->is_outbound_connection()) { + MIMEField *field = this->_send_header.field_find(static_cast(MIME_FIELD_CONNECTION)); + if (field) { + auto value{field->value_get()}; + if (value == static_cast(HTTP_VALUE_CLOSE)) { + SCOPED_MUTEX_LOCK(lock, _proxy_ssn->mutex, this_ethread()); + if (connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NONE) { + connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED, Http2ErrorCode::HTTP2_ERROR_NO_ERROR); + } } } } @@ -890,6 +901,12 @@ Http2Stream::update_write_request(bool call_update) this->parsing_header_done = false; } if (this->is_outbound_connection() || this->_send_header.expect_final_response()) { + // The send-side request header is about to be torn down on outbound + // streams, so snapshot the request method and any conditional-header + // presence first. The snapshot is needed later by + // `payload_length_is_valid` to apply [RFC 9110] 8.6 payload preclusion + // for HEAD responses and 304 responses to conditional GETs. + this->cache_send_request_for_response_validation(); _send_header.destroy(); _send_header.create(this->is_outbound_connection() ? HTTPType::REQUEST : HTTPType::RESPONSE, HTTP_2_0); http_parser_clear(&http_parser); @@ -1320,6 +1337,18 @@ Http2Stream::set_tx_error_code(ProxyError e) } } +bool +Http2Stream::is_safe_to_retry() const +{ + return _safe_to_retry; +} + +void +Http2Stream::set_safe_to_retry() +{ + _safe_to_retry = true; +} + HTTPVersion Http2Stream::get_version(HTTPHdr & /* hdr ATS_UNUSED */) const { diff --git a/tests/gold_tests/h2/continuation_origin.py b/tests/gold_tests/h2/continuation_origin.py new file mode 100644 index 00000000000..88bb1306c0b --- /dev/null +++ b/tests/gold_tests/h2/continuation_origin.py @@ -0,0 +1,141 @@ +#!/usr/bin/env python3 +"""Serve HTTP/2 responses whose header blocks require CONTINUATION frames.""" + +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import argparse +import socket +import ssl +import sys + +from h2.config import H2Configuration +from h2.connection import H2Connection +from h2.events import DataReceived, StreamEnded + +HTTP2_FRAME_HEADER_SIZE = 9 +HTTP2_FRAME_TYPE_CONTINUATION = 0x09 + + +def count_frames(payload: bytes, frame_type: int) -> int: + """Count frames of ``frame_type`` in an HTTP/2 wire payload.""" + count = 0 + offset = 0 + while offset < len(payload): + if len(payload) - offset < HTTP2_FRAME_HEADER_SIZE: + raise RuntimeError("truncated HTTP/2 frame header") + + length = int.from_bytes(payload[offset:offset + 3], "big") + frame_end = offset + HTTP2_FRAME_HEADER_SIZE + length + if frame_end > len(payload): + raise RuntimeError("truncated HTTP/2 frame payload") + if payload[offset + 3] == frame_type: + count += 1 + offset = frame_end + return count + + +def serve_connection(tls_socket: ssl.SSLSocket, expected_responses: int) -> int: + """Serve requests on one HTTP/2 connection.""" + connection = H2Connection(config=H2Configuration(client_side=False, header_encoding="utf-8")) + connection.initiate_connection() + tls_socket.sendall(connection.data_to_send()) + + responses_sent = 0 + # Do not initiate connection shutdown after sending the expected + # responses. A large header block spans multiple TLS records, and closing + # here can race the peer draining the final CONTINUATION and DATA frames. + # Instead, keep servicing the connection until ATS closes it (or AuTest + # terminates this process during cleanup). + while True: + data = tls_socket.recv(65535) + if not data: + return responses_sent + + for event in connection.receive_data(data): + if isinstance(event, DataReceived): + connection.acknowledge_received_data(event.flow_controlled_length, event.stream_id) + elif isinstance(event, StreamEnded): + # Together these values are intentionally larger than the + # default 16 KiB maximum frame size even after HPACK Huffman + # encoding. Each field remains below ATS's per-field limit. + padding_one = f"{event.stream_id:08x}-" + ("0123456789abcdef" * 1024) + padding_two = f"{event.stream_id:08x}-" + ("fedcba9876543210" * 1024) + connection.send_headers( + event.stream_id, + [ + (":status", "200"), + ("content-length", "4"), + ("x-continuation-padding-one", padding_one), + ("x-continuation-padding-two", padding_two), + ], + ) + connection.send_data(event.stream_id, b"okay", end_stream=True) + + wire_bytes = connection.data_to_send() + continuation_frames = count_frames(wire_bytes, HTTP2_FRAME_TYPE_CONTINUATION) + if continuation_frames == 0: + raise RuntimeError("large response header did not generate a CONTINUATION frame") + + print( + f"stream={event.stream_id} sent_continuation_frames={continuation_frames}", + flush=True, + ) + tls_socket.sendall(wire_bytes) + responses_sent += 1 + + +def run_server(port: int, certificate: str, private_key: str, expected_responses: int) -> int: + """Accept TLS connections until ATS closes after the expected responses.""" + context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.load_cert_chain(certificate, private_key) + context.set_alpn_protocols(["h2"]) + + responses_sent = 0 + with socket.create_server(("127.0.0.1", port)) as listener: + listener.settimeout(30) + while responses_sent < expected_responses: + plain_socket, _ = listener.accept() + try: + with context.wrap_socket(plain_socket, server_side=True) as tls_socket: + if tls_socket.selected_alpn_protocol() != "h2": + raise RuntimeError("ATS did not negotiate HTTP/2 with the origin") + responses_sent += serve_connection(tls_socket, expected_responses - responses_sent) + except ssl.SSLError: + # AuTest's readiness probe opens and closes a plain TCP socket. + plain_socket.close() + + return 0 if responses_sent == expected_responses else 1 + + +def parse_args() -> argparse.Namespace: + """Parse command-line arguments.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("port", type=int) + parser.add_argument("certificate") + parser.add_argument("private_key") + parser.add_argument("expected_responses", type=int) + return parser.parse_args() + + +def main() -> int: + """Run the test origin.""" + args = parse_args() + return run_server(args.port, args.certificate, args.private_key, args.expected_responses) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/gold_tests/h2/gold/h2-settings-ack-metrics.gold b/tests/gold_tests/h2/gold/h2-settings-ack-metrics.gold new file mode 100644 index 00000000000..6ce0a1c5ee0 --- /dev/null +++ b/tests/gold_tests/h2/gold/h2-settings-ack-metrics.gold @@ -0,0 +1,2 @@ +proxy.process.http2.max_settings_frames_per_minute_exceeded 0 +proxy.process.http2.connection_errors 0 diff --git a/tests/gold_tests/h2/gold/h2o-pool-reuse-metrics.gold b/tests/gold_tests/h2/gold/h2o-pool-reuse-metrics.gold new file mode 100644 index 00000000000..3cef613120b --- /dev/null +++ b/tests/gold_tests/h2/gold/h2o-pool-reuse-metrics.gold @@ -0,0 +1,2 @@ +proxy.process.http2.total_server_connections 1 +proxy.process.http2.total_server_streams 5 diff --git a/tests/gold_tests/h2/h2_settings_ack_not_counted.test.py b/tests/gold_tests/h2/h2_settings_ack_not_counted.test.py new file mode 100644 index 00000000000..d03c9eabce6 --- /dev/null +++ b/tests/gold_tests/h2/h2_settings_ack_not_counted.test.py @@ -0,0 +1,100 @@ +''' +Verify that SETTINGS frames carrying the ACK flag do not count against +`proxy.config.http2.max_settings_frames_per_minute`. + +A SETTINGS-ACK is a mandatory protocol response to a SETTINGS frame ATS +itself sent (RFC 7540 / 9113 6.5), so it cannot be used by a peer to +flood ATS. Counting inbound ACKs against the per-minute receive limit +spuriously closed otherwise-healthy connections with ENHANCE_YOUR_CALM, +which is especially visible with +`proxy.config.http2.flow_control.policy_in=2` +(LARGE_SESSION_AND_DYNAMIC_STREAM): ATS sends a SETTINGS frame per +inbound stream and the client returns one ACK per stream. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +Test.Summary = ''' +Verify SETTINGS-ACK frames are not counted against +`proxy.config.http2.max_settings_frames_per_minute`. +''' + +Test.ContinueOnFail = True + +replay_file = "replay_h2_settings_ack/settings_ack.replay.yaml" + +server = Test.MakeVerifierServerProcess("settings-ack-origin", replay_file) + +ts = Test.MakeATSProcess("ts", enable_tls=True) +ts.addDefaultSSLFiles() +ts.Disk.records_config.update( + { + 'proxy.config.ssl.server.cert.path': f'{ts.Variables.SSLDir}', + 'proxy.config.ssl.server.private_key.path': f'{ts.Variables.SSLDir}', + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http2_cs', + # Set the per-minute SETTINGS receive limit very low. The verifier + # client will send 1 ACK in response to ATS's preface SETTINGS plus + # 1 ACK per stream that ATS opens via dynamic-window SETTINGS. With + # five separate inbound H/2 sessions, the buggy code would tear at + # least one of them down with ENHANCE_YOUR_CALM after the second + # inbound ACK. With the fix, ACKs are not counted and the limit is + # never tripped. + 'proxy.config.http2.max_settings_frames_per_minute': 2, + # `LARGE_SESSION_AND_DYNAMIC_STREAM` makes ATS send a SETTINGS + # frame whenever a new inbound stream is created (to readjust the + # per-stream window). Each of those SETTINGS frames triggers a + # client ACK -- exactly the pattern that exposed the bug in + # production. + 'proxy.config.http2.flow_control.policy_in': 2, + 'proxy.config.http.cache.http': 0, + }) + +ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{server.Variables.http_port}') +ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + +tr = Test.AddTestRun("Drive 5 H/2 client sessions, each with one request") +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(ts) +tr.AddVerifierClientProcess("client-settings-ack", replay_file, http_ports=[ts.Variables.port], https_ports=[ts.Variables.ssl_port]) +tr.StillRunningAfter = ts +tr.StillRunningAfter = server +tr.TimeOut = 60 + +# Once the client run finishes, assert that ATS never tripped the +# SETTINGS-frames-per-minute limit and never recorded a connection-level +# error caused by it. `stdout_wait` retries until the gold matches, so it +# tolerates the brief settle time between the last response and the +# metric updates. +tr = Test.AddTestRun("Assert SETTINGS-ACK frames did not trip the per-minute limit") +tr.Processes.Default.Command = ( + f"{Test.Variables.AtsTestToolsDir}/stdout_wait" + f" 'traffic_ctl metric get" + f" proxy.process.http2.max_settings_frames_per_minute_exceeded" + f" proxy.process.http2.connection_errors'" + f" {Test.TestDirectory}/gold/h2-settings-ack-metrics.gold") +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +ts.Disk.diags_log.Content = Testers.ExcludesExpression( + "too frequent SETTINGS frames", "must not GOAWAY ENHANCE_YOUR_CALM on inbound SETTINGS-ACK frames") diff --git a/tests/gold_tests/h2/h2_to_origin_continuation.test.py b/tests/gold_tests/h2/h2_to_origin_continuation.test.py new file mode 100644 index 00000000000..8f7850628e5 --- /dev/null +++ b/tests/gold_tests/h2/h2_to_origin_continuation.test.py @@ -0,0 +1,104 @@ +''' +Verify that ATS does not tear down outbound HTTP/2 connections, and does +not crash on a `_sm == nullptr` assertion, when the origin response +headers may be split across HEADERS + CONTINUATION frames on a stream +that has already advanced past IDLE. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import sys + +from ports import get_port + +Test.Summary = ''' +Verify that ATS does not tear down outbound HTTP/2 connections and does +not crash on a `_sm == nullptr` assertion when receiving the origin's +response on a stream that is past the IDLE state. +''' + +Test.ContinueOnFail = True + +replay_file = "replay_h2o_continuation/continuation.replay.yaml" + +tr = Test.AddTestRun("Outbound HTTP/2 CONTINUATION on a non-IDLE stream") +tr.Setup.Copy("continuation_origin.py") + +server = tr.Processes.Process("h2-continuation-origin") +server_port = get_port(server, "https_port") +server_pem = os.path.join(Test.Variables.AtsTestToolsDir, "ssl", "server.pem") +server_key = os.path.join(Test.Variables.AtsTestToolsDir, "ssl", "server.key") +server.Setup.Copy(server_pem) +server.Setup.Copy(server_key) +server.Command = (f"{sys.executable} {tr.RunDirectory}/continuation_origin.py " + f"{server_port} server.pem server.key 2") +server.Ready = When.PortOpen(server_port) +server.ReturnCode = Any(0, -2) +server.Streams.stdout += Testers.ContainsExpression( + r"sent_continuation_frames=[1-9][0-9]*", + "The origin must positively verify that it emitted CONTINUATION frames.", +) + +ts = Test.MakeATSProcess("ts", enable_tls=True) +ts.addDefaultSSLFiles() +ts.Disk.records_config.update( + { + 'proxy.config.ssl.server.cert.path': f'{ts.Variables.SSLDir}', + 'proxy.config.ssl.server.private_key.path': f'{ts.Variables.SSLDir}', + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http2', + 'proxy.config.exec_thread.autoconfig.enabled': 0, + 'proxy.config.exec_thread.limit': 4, + 'proxy.config.ssl.client.alpn_protocols': 'h2,http/1.1', + 'proxy.config.http.server_session_sharing.pool': 'thread', + 'proxy.config.http.server_session_sharing.match': 'ip,sni,cert', + 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', + 'proxy.config.http.cache.http': 0, + # The custom origin emits a header block larger than this value and + # positively verifies that it generated CONTINUATION frames. + 'proxy.config.http2.max_frame_size': 16384, + 'proxy.config.http2.max_header_list_size': 1048576, + 'proxy.config.http.response_header_max_size': 65536, + }) + +ts.Disk.remap_config.AddLine(f'map / https://127.0.0.1:{server_port}') +ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(ts) +tr.AddVerifierClientProcess("client-continuation", replay_file, http_ports=[ts.Variables.port], https_ports=[ts.Variables.ssl_port]) +tr.StillRunningAfter = ts +tr.TimeOut = 60 + +# Regression guards for the two outbound CONTINUATION bugs: +# 1) Connection-level PROTOCOL_ERROR ("continuation bad state") would +# mean rcv_continuation_frame still rejects the OPEN / +# HALF_CLOSED_LOCAL stream states on outbound connections. +# 2) The `_sm == nullptr` assertion (visible as a fatal in +# traffic.out) would mean rcv_continuation_frame called +# `new_transaction` on an outbound stream whose state machine +# already exists (the SM was created when ATS issued the request). +ts.Disk.diags_log.Content = Testers.ExcludesExpression( + "continuation bad state", "ATS must not raise a PROTOCOL_ERROR for outbound CONTINUATION frames in OPEN/HALF_CLOSED_LOCAL") +ts.Disk.traffic_out.Content = Testers.ExcludesExpression( + "_sm == nullptr", "ATS must not re-create an outbound HTTP/2 transaction on receipt of CONTINUATION") diff --git a/tests/gold_tests/h2/h2_to_origin_payload_validation.test.py b/tests/gold_tests/h2/h2_to_origin_payload_validation.test.py new file mode 100644 index 00000000000..fc18db5f0d1 --- /dev/null +++ b/tests/gold_tests/h2/h2_to_origin_payload_validation.test.py @@ -0,0 +1,75 @@ +''' +Verify that ATS forwards origin HTTP/2 responses for HEAD requests and +304 responses to conditional GETs without spurious "Bad payload length" +stream errors when both client and origin use HTTP/2. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +Test.Summary = ''' +Verify outbound HTTP/2 payload-length validation honors RFC 9110 8.6 for +HEAD responses and for 304 responses to conditional GETs. +''' + +Test.ContinueOnFail = True + +replay_file = "replay_h2o_payload_validation/payload_validation.replay.yaml" + +server = Test.MakeVerifierServerProcess("h2-payload-origin", replay_file) + +ts = Test.MakeATSProcess("ts", enable_tls=True) +ts.addDefaultSSLFiles() +ts.Disk.records_config.update( + { + 'proxy.config.ssl.server.cert.path': f'{ts.Variables.SSLDir}', + 'proxy.config.ssl.server.private_key.path': f'{ts.Variables.SSLDir}', + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http2', + 'proxy.config.exec_thread.autoconfig.enabled': 0, + 'proxy.config.exec_thread.limit': 4, + # Negotiate HTTP/2 to the origin via ALPN. + 'proxy.config.ssl.client.alpn_protocols': 'h2,http/1.1', + 'proxy.config.http.server_session_sharing.pool': 'thread', + 'proxy.config.http.server_session_sharing.match': 'ip,sni,cert', + 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', + # Disable caching so the conditional GET reaches the origin. + 'proxy.config.http.cache.http': 0, + }) + +ts.Disk.remap_config.AddLine(f'map / https://127.0.0.1:{server.Variables.https_port}') +ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + +tr = Test.AddTestRun("HEAD/304 with Content-Length over outbound HTTP/2") +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(ts) +tr.AddVerifierClientProcess("client-payload", replay_file, http_ports=[ts.Variables.port], https_ports=[ts.Variables.ssl_port]) +tr.StillRunningAfter = ts +tr.StillRunningAfter = server +tr.TimeOut = 60 + +# These warnings would indicate a regression of the outbound payload +# preclusion logic. They must not appear when ATS correctly recognizes the +# original HEAD or conditional GET request method. +ts.Disk.diags_log.Content = Testers.ExcludesExpression( + "Bad payload length", "ATS must not report a bad payload length for HEAD/304 outbound H2 responses") +ts.Disk.diags_log.Content += Testers.ExcludesExpression( + "recv data bad payload length", "ATS must not raise a stream PROTOCOL_ERROR for HEAD/304 outbound H2 responses") diff --git a/tests/gold_tests/h2/h2_to_origin_pool_reuse.test.py b/tests/gold_tests/h2/h2_to_origin_pool_reuse.test.py new file mode 100644 index 00000000000..84557a1e4a5 --- /dev/null +++ b/tests/gold_tests/h2/h2_to_origin_pool_reuse.test.py @@ -0,0 +1,106 @@ +''' +Verify that ATS multiplexes multiple outbound HTTP/2 origin requests onto +a single H/2 connection when the configured server-session sharing pool is +`global`. This is a regression test for the latent bug where outbound H/2 +sessions were filed exclusively in the per-thread pool while +`HttpSessionManager::acquire_session` was searching only the global pool, +silently disabling H/2 origin reuse and forcing a fresh TCP+TLS+H/2 +handshake for every single request. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +Test.Summary = ''' +Verify outbound HTTP/2 origin connection reuse / multiplexing across +multiple requests when proxy.config.http.server_session_sharing.pool is +`global`. +''' + +Test.ContinueOnFail = True + +replay_file = "replay_h2o_pool_reuse/pool_reuse.replay.yaml" + +server = Test.MakeVerifierServerProcess("h2-pool-origin", replay_file) + +ts = Test.MakeATSProcess("ts", enable_tls=True) +ts.addDefaultSSLFiles() +ts.Disk.records_config.update( + { + 'proxy.config.ssl.server.cert.path': f'{ts.Variables.SSLDir}', + 'proxy.config.ssl.server.private_key.path': f'{ts.Variables.SSLDir}', + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http', + 'proxy.config.exec_thread.autoconfig.enabled': 0, + # Pin to a single net thread so all transactions land on the same + # EThread. Cross-thread sharing of multiplexing H/2 origin sessions + # is intentionally not supported (their state is owned by the + # EThread driving the connection); reuse is only expected within a + # single thread, so single-thread is what the test asserts on. + 'proxy.config.exec_thread.limit': 1, + 'proxy.config.ssl.client.alpn_protocols': 'h2,http/1.1', + # The bug being verified is specific to pool=global: H/2 origin + # sessions live in the per-thread pool but pool=global only consults + # the global pool, so reuse silently fails. With the fix in place, + # `_acquire_session` also checks the thread pool first, so reuse + # works regardless of the configured pool type. + 'proxy.config.http.server_session_sharing.pool': 'global', + 'proxy.config.http.server_session_sharing.match': 'ip,sni,cert', + 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', + 'proxy.config.http.cache.http': 0, + # Keep the original Host header through remap so SNI matching for + # session reuse uses the same hostname for every request rather + # than the IP literal `127.0.0.1` (which TLS will not send as an + # SNI). + 'proxy.config.url_remap.pristine_host_hdr': 1, + }) + +ts.Disk.remap_config.AddLine(f'map / https://127.0.0.1:{server.Variables.https_port}') +ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + +tr = Test.AddTestRun("Drive 5 sequential H/2 requests over a single client session") +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(ts) +tr.AddVerifierClientProcess("client-pool-reuse", replay_file, http_ports=[ts.Variables.port], https_ports=[ts.Variables.ssl_port]) +tr.StillRunningAfter = ts +tr.StillRunningAfter = server +tr.TimeOut = 60 + +# `stdout_wait` retries the command until its output matches the gold file +# (or the run times out), so we don't need a separate settling step -- +# once ATS finishes releasing all 5 streams the metrics will agree. +tr = Test.AddTestRun("Assert exactly one H/2 origin connection carried all 5 streams") +tr.Processes.Default.Command = ( + f"{Test.Variables.AtsTestToolsDir}/stdout_wait" + f" 'traffic_ctl metric get" + f" proxy.process.http2.total_server_connections" + f" proxy.process.http2.total_server_streams'" + f" {Test.TestDirectory}/gold/h2o-pool-reuse-metrics.gold") +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# Regression guard: if the bug is reintroduced, every transaction opens a +# fresh outbound H/2 connection and `Add session to pool` is logged once +# per connection. Catch the case of more than 5 outbound H/2 sessions. +ts.Disk.diags_log.Content = Testers.ExcludesExpression( + r"(?:.*Add session to pool.*\n.*){5,}.*Add session to pool", + "must not open more than 5 outbound H/2 origin sessions for 5 requests") diff --git a/tests/gold_tests/h2/h2_to_origin_safe_retry.test.py b/tests/gold_tests/h2/h2_to_origin_safe_retry.test.py new file mode 100644 index 00000000000..731042bad27 --- /dev/null +++ b/tests/gold_tests/h2/h2_to_origin_safe_retry.test.py @@ -0,0 +1,115 @@ +''' +Verify ATS retries non-idempotent outbound HTTP/2 requests when the origin +guarantees that it did not process them. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import sys + +from ports import get_port + +Test.Summary = ''' +Verify that ATS safely retries a POST after the HTTP/2 origin either sends +RST_STREAM(REFUSED_STREAM) or GOAWAY(last_stream_id=0). +''' + +Test.ContinueOnFail = True + + +class SafeRetryScenario: + """Configure one HTTP/2 safe-retry scenario.""" + + replay_file = "replay_h2o_safe_retry/safe_retry.replay.yaml" + + def __init__(self, mode: str, replay_key: str) -> None: + tr = Test.AddTestRun(f"Safe POST retry after outbound HTTP/2 {mode}") + tr.Setup.Copy("safe_retry_origin.py") + + server = tr.Processes.Process(f"safe-retry-origin-{mode}") + server_port = get_port(server, "https_port") + server_pem = os.path.join(Test.Variables.AtsTestToolsDir, "ssl", "server.pem") + server_key = os.path.join(Test.Variables.AtsTestToolsDir, "ssl", "server.key") + server.Setup.Copy(server_pem) + server.Setup.Copy(server_key) + server.Command = ( + f"{sys.executable} {tr.RunDirectory}/safe_retry_origin.py " + f"{mode} {server_port} server.pem server.key") + server.Ready = When.PortOpen(server_port) + server.ReturnCode = Any(0, -2) + + action = "REFUSED_STREAM" if mode == "rst" else "GOAWAY" + server.Streams.stdout += Testers.ContainsExpression( + rf"action={action} attempt=1", + f"The origin must reject the first POST with {action}.", + ) + server.Streams.stdout += Testers.ContainsExpression( + r"retry_succeeded attempts=2 method=POST body=request-body", + "ATS must retry the POST once, including its request body.", + ) + + ts = tr.MakeATSProcess(f"ts-safe-retry-{mode}", enable_tls=True, enable_cache=False) + ts.addDefaultSSLFiles() + ts.Disk.records_config.update( + { + 'proxy.config.ssl.server.cert.path': f'{ts.Variables.SSLDir}', + 'proxy.config.ssl.server.private_key.path': f'{ts.Variables.SSLDir}', + 'proxy.config.ssl.client.alpn_protocols': 'h2,http/1.1', + 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', + 'proxy.config.http.server_session_sharing.pool': 'thread', + # The RST_STREAM retry may reuse the healthy H2 connection. + # Match only on the loopback IP because an IP-literal remap + # does not send SNI, while ATS proposes the literal as the + # lookup SNI when it searches the session pool. + 'proxy.config.http.server_session_sharing.match': 'ip', + 'proxy.config.http.connect_attempts_max_retries': 3, + # Retrying a request with a body requires ATS's existing + # request buffer so the second origin attempt can replay it. + 'proxy.config.http.request_buffer_enabled': 1, + 'proxy.config.http.post_copy_size': 4096, + 'proxy.config.exec_thread.autoconfig.enabled': 0, + 'proxy.config.exec_thread.limit': 1, + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|http2', + }) + ts.Disk.remap_config.AddLine(f'map / https://127.0.0.1:{server_port}') + ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + + tr.Processes.Default.StartBefore(server) + tr.Processes.Default.StartBefore(ts) + tr.AddVerifierClientProcess( + f"safe-retry-client-{mode}", + self.replay_file, + http_ports=[ts.Variables.port], + keys=replay_key, + ) + tr.TimeOut = 60 + + ts.Disk.traffic_out.Content = Testers.ExcludesExpression( + "ERR_CLIENT_ABORT", + "A request guaranteed unprocessed by the origin must not be returned as a client abort.", + ) + + +SafeRetryScenario("rst", "rst-refused") +SafeRetryScenario("goaway", "goaway-last-stream-zero") diff --git a/tests/gold_tests/h2/replay_h2_settings_ack/settings_ack.replay.yaml b/tests/gold_tests/h2/replay_h2_settings_ack/settings_ack.replay.yaml new file mode 100644 index 00000000000..2da2a10c0c0 --- /dev/null +++ b/tests/gold_tests/h2/replay_h2_settings_ack/settings_ack.replay.yaml @@ -0,0 +1,187 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +meta: + version: '1.0' + +# Five separate H/2 client sessions, each issuing a single request. Each +# session generates: +# * 1 SETTINGS-ACK in response to ATS's connection-preface SETTINGS, +# * 1 SETTINGS-ACK in response to the per-stream dynamic-window SETTINGS +# that ATS sends with `flow_control.policy_in=2`, +# i.e. ~2 inbound SETTINGS-ACK frames per session. Across the five +# sessions ATS receives ~10 inbound SETTINGS-ACK frames in well under a +# minute. With `max_settings_frames_per_minute=2` and the buggy +# implementation, ATS would close the connection with ENHANCE_YOUR_CALM +# and the verifier client would observe its stream being torn down. With +# the fix in place, ACKs are not counted toward the limit, the limit +# stays at 0, and all five sessions complete cleanly. + +sessions: + - protocol: + - name: http + version: '2' + - name: tls + sni: settings-ack.example.com + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, settings-ack-1 ]]}} + client-request: + version: '2' + scheme: https + method: GET + url: /settings-ack/1 + headers: + encoding: esc_json + fields: + - [ Host, settings-ack.example.com ] + - [ uuid, settings-ack-1 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'one1' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '2' + - name: tls + sni: settings-ack.example.com + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, settings-ack-2 ]]}} + client-request: + version: '2' + scheme: https + method: GET + url: /settings-ack/2 + headers: + encoding: esc_json + fields: + - [ Host, settings-ack.example.com ] + - [ uuid, settings-ack-2 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'two2' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '2' + - name: tls + sni: settings-ack.example.com + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, settings-ack-3 ]]}} + client-request: + version: '2' + scheme: https + method: GET + url: /settings-ack/3 + headers: + encoding: esc_json + fields: + - [ Host, settings-ack.example.com ] + - [ uuid, settings-ack-3 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 6 ] + content: { encoding: plain, data: 'three3' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '2' + - name: tls + sni: settings-ack.example.com + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, settings-ack-4 ]]}} + client-request: + version: '2' + scheme: https + method: GET + url: /settings-ack/4 + headers: + encoding: esc_json + fields: + - [ Host, settings-ack.example.com ] + - [ uuid, settings-ack-4 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 5 ] + content: { encoding: plain, data: 'four4' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '2' + - name: tls + sni: settings-ack.example.com + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, settings-ack-5 ]]}} + client-request: + version: '2' + scheme: https + method: GET + url: /settings-ack/5 + headers: + encoding: esc_json + fields: + - [ Host, settings-ack.example.com ] + - [ uuid, settings-ack-5 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 5 ] + content: { encoding: plain, data: 'five5' } + proxy-response: + status: 200 diff --git a/tests/gold_tests/h2/replay_h2o_continuation/continuation.replay.yaml b/tests/gold_tests/h2/replay_h2o_continuation/continuation.replay.yaml new file mode 100644 index 00000000000..7eb1f981b53 --- /dev/null +++ b/tests/gold_tests/h2/replay_h2o_continuation/continuation.replay.yaml @@ -0,0 +1,134 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +meta: + version: '1.0' + +# Two outbound HTTP/2 transactions whose responses are generated by the +# custom origin in `continuation_origin.py`. The origin sends a header block +# larger than 16 KiB and positively verifies that it emits CONTINUATION +# frames. This drives the outbound response path on streams that are past +# IDLE so the regression guards in `h2_to_origin_continuation.test.py` catch: +# +# - "continuation bad state" PROTOCOL_ERROR if the outbound +# CONTINUATION state-machine guard is ever re-tightened. +# - The `_sm == nullptr` assertion if `rcv_continuation_frame` ever +# unconditionally calls `new_transaction` on outbound streams again. +sessions: + - protocol: + - name: http + version: '2' + - name: tls + version: TLSv1.3 + sni: cont.h2o.example.com + proxy-verify-mode: 0 + proxy-provided-cert: true + - name: tcp + - name: ip + version: '4' + + transactions: + - all: { headers: { fields: [[ uuid, h2o-cont-1 ]]}} + + client-request: + version: '2' + scheme: https + method: GET + url: /continuation/first + headers: + encoding: esc_json + fields: + - [ Host, cont.h2o.example.com ] + content: + encoding: plain + size: 0 + + proxy-request: + version: '2' + scheme: https + method: GET + url: + - [ path, { value: /continuation/first, as: equal } ] + headers: + encoding: esc_json + fields: + - [ Host, cont.h2o.example.com ] + + server-response: + version: '2' + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: + encoding: plain + data: 'okay' + + proxy-response: + version: '2' + status: 200 + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + + - all: { headers: { fields: [[ uuid, h2o-cont-2 ]]}} + + client-request: + version: '2' + scheme: https + method: GET + url: /continuation/second + headers: + encoding: esc_json + fields: + - [ Host, cont.h2o.example.com ] + content: + encoding: plain + size: 0 + + proxy-request: + version: '2' + scheme: https + method: GET + url: + - [ path, { value: /continuation/second, as: equal } ] + headers: + encoding: esc_json + fields: + - [ Host, cont.h2o.example.com ] + + server-response: + version: '2' + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: + encoding: plain + data: 'okay' + + proxy-response: + version: '2' + status: 200 + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] diff --git a/tests/gold_tests/h2/replay_h2o_payload_validation/payload_validation.replay.yaml b/tests/gold_tests/h2/replay_h2o_payload_validation/payload_validation.replay.yaml new file mode 100644 index 00000000000..3df925b33a6 --- /dev/null +++ b/tests/gold_tests/h2/replay_h2o_payload_validation/payload_validation.replay.yaml @@ -0,0 +1,146 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +meta: + version: '1.0' + +# +# These transactions exercise the payload-length validation code path on +# outbound (origin-side) HTTP/2 streams. They cover the [RFC 9110] 8.6 cases +# where a non-zero Content-Length is permitted with no message body: +# +# * Test 1: A HEAD request with a Content-Length response from the origin. +# * Test 2: A conditional GET (If-None-Match) that the origin satisfies +# with a 304 (Not Modified) response carrying Content-Length. +# +# Without the Http2Stream send-side request snapshot, ATS would tear down +# these streams with a STREAM PROTOCOL_ERROR ("recv data bad payload length") +# because `_send_header` is destroyed before the response is validated. + +sessions: + - protocol: + - name: http + version: '2' + - name: tls + version: TLSv1.3 + sni: payload.h2o.example.com + proxy-verify-mode: 0 + proxy-provided-cert: true + - name: tcp + - name: ip + version: '4' + + transactions: + # + # Test 1: HEAD request with a non-zero Content-Length response. + # + - all: { headers: { fields: [[ uuid, head-with-content-length ]]}} + + client-request: + version: '2' + scheme: https + method: HEAD + url: /payload/head + headers: + encoding: esc_json + fields: + - [ Host, payload.h2o.example.com ] + content: + encoding: plain + size: 0 + + proxy-request: + version: '2' + scheme: https + method: HEAD + url: + - [ path, { value: /payload/head, as: equal } ] + headers: + encoding: esc_json + fields: + - [ Host, payload.h2o.example.com ] + + server-response: + version: '2' + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 7 ] + content: + encoding: plain + size: 0 + + proxy-response: + version: '2' + status: 200 + headers: + encoding: esc_json + fields: + - [ Content-Length, 7 ] + + # + # Test 2: Conditional GET satisfied with a 304 carrying Content-Length. + # + - all: { headers: { fields: [[ uuid, conditional-get-304 ]]}} + + client-request: + version: '2' + scheme: https + method: GET + url: /payload/conditional + headers: + encoding: esc_json + fields: + - [ Host, payload.h2o.example.com ] + - [ If-None-Match, '"abc"' ] + content: + encoding: plain + size: 0 + + proxy-request: + version: '2' + scheme: https + method: GET + url: + - [ path, { value: /payload/conditional, as: equal } ] + headers: + encoding: esc_json + fields: + - [ Host, payload.h2o.example.com ] + - [ If-None-Match, { value: '"abc"', as: equal } ] + + server-response: + version: '2' + status: 304 + reason: Not Modified + headers: + encoding: esc_json + fields: + - [ Content-Length, 42 ] + - [ ETag, '"abc"' ] + content: + encoding: plain + size: 0 + + proxy-response: + version: '2' + status: 304 + headers: + encoding: esc_json + fields: + - [ Content-Length, 42 ] diff --git a/tests/gold_tests/h2/replay_h2o_pool_reuse/pool_reuse.replay.yaml b/tests/gold_tests/h2/replay_h2o_pool_reuse/pool_reuse.replay.yaml new file mode 100644 index 00000000000..742bddef3c6 --- /dev/null +++ b/tests/gold_tests/h2/replay_h2o_pool_reuse/pool_reuse.replay.yaml @@ -0,0 +1,173 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +meta: + version: '1.0' + +# Five separate H/1 client sessions, each issuing a single request to the +# same origin (matching SNI + IP + cert). This is the production-style +# pattern that exposes the H/2 origin session pooling bug: every new +# client session triggers a fresh `acquire_session` lookup with no +# session attached to the user-agent transaction, so the pool search is +# what determines whether the existing H/2 origin connection is reused. +# If reuse works, all 5 requests should be multiplexed onto a single H/2 +# origin connection (1 connection, 5 streams). + +sessions: + - protocol: + - name: http + version: '1.1' + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, pool-reuse-1 ]]}} + client-request: + version: '1.1' + scheme: http + method: GET + url: /pool/reuse/1 + headers: + encoding: esc_json + fields: + - [ Host, pool.h2o.example.com ] + - [ uuid, pool-reuse-1 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'one1' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '1.1' + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, pool-reuse-2 ]]}} + client-request: + version: '1.1' + scheme: http + method: GET + url: /pool/reuse/2 + headers: + encoding: esc_json + fields: + - [ Host, pool.h2o.example.com ] + - [ uuid, pool-reuse-2 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'two2' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '1.1' + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, pool-reuse-3 ]]}} + client-request: + version: '1.1' + scheme: http + method: GET + url: /pool/reuse/3 + headers: + encoding: esc_json + fields: + - [ Host, pool.h2o.example.com ] + - [ uuid, pool-reuse-3 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'thr3' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '1.1' + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, pool-reuse-4 ]]}} + client-request: + version: '1.1' + scheme: http + method: GET + url: /pool/reuse/4 + headers: + encoding: esc_json + fields: + - [ Host, pool.h2o.example.com ] + - [ uuid, pool-reuse-4 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'fou4' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '1.1' + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, pool-reuse-5 ]]}} + client-request: + version: '1.1' + scheme: http + method: GET + url: /pool/reuse/5 + headers: + encoding: esc_json + fields: + - [ Host, pool.h2o.example.com ] + - [ uuid, pool-reuse-5 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'fiv5' } + proxy-response: + status: 200 diff --git a/tests/gold_tests/h2/replay_h2o_safe_retry/safe_retry.replay.yaml b/tests/gold_tests/h2/replay_h2o_safe_retry/safe_retry.replay.yaml new file mode 100644 index 00000000000..a089a3718ac --- /dev/null +++ b/tests/gold_tests/h2/replay_h2o_safe_retry/safe_retry.replay.yaml @@ -0,0 +1,67 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +meta: + version: '1.0' + +# The custom HTTP/2 origin rejects the first copy of each POST with a signal +# that guarantees the request was not processed. A successful response proves +# that ATS retries the non-idempotent method and replays its body. +sessions: + - transactions: + - all: { headers: { fields: [[ uuid, rst-refused ]]}} + client-request: + method: POST + version: '1.1' + scheme: http + url: /rst-refused + headers: + fields: + - [ Host, safe-retry.example.com ] + - [ Content-Length, 12 ] + content: + encoding: plain + data: request-body + proxy-response: + status: 200 + headers: + fields: + - [ Content-Length, 7 ] + content: + encoding: plain + data: retried + + - all: { headers: { fields: [[ uuid, goaway-last-stream-zero ]]}} + client-request: + method: POST + version: '1.1' + scheme: http + url: /goaway-last-stream-zero + headers: + fields: + - [ Host, safe-retry.example.com ] + - [ Content-Length, 12 ] + content: + encoding: plain + data: request-body + proxy-response: + status: 200 + headers: + fields: + - [ Content-Length, 7 ] + content: + encoding: plain + data: retried diff --git a/tests/gold_tests/h2/safe_retry_origin.py b/tests/gold_tests/h2/safe_retry_origin.py new file mode 100644 index 00000000000..7ecc864498f --- /dev/null +++ b/tests/gold_tests/h2/safe_retry_origin.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +"""Exercise safe retries after HTTP/2 REFUSED_STREAM and GOAWAY.""" + +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import argparse +import socket +import ssl +import sys + +from h2.config import H2Configuration +from h2.connection import H2Connection +from h2.errors import ErrorCodes +from h2.events import ConnectionTerminated, DataReceived, RequestReceived, StreamEnded + +EXPECTED_BODY = b"request-body" +RESPONSE_BODY = b"retried" + + +class RetryOrigin: + """An HTTP/2 origin that rejects the first POST and accepts its retry.""" + + def __init__(self, mode: str) -> None: + self.mode = mode + self.attempts = 0 + + def _validate_request(self, headers: dict[str, str], body: bytes) -> None: + """Verify that ATS replayed the non-idempotent request intact.""" + if headers.get(":method") != "POST": + raise RuntimeError(f"expected POST, got {headers.get(':method')}") + if body != EXPECTED_BODY: + raise RuntimeError(f"expected body {EXPECTED_BODY!r}, got {body!r}") + + def _reject_first_attempt(self, connection: H2Connection, stream_id: int) -> bool: + """Reject the first request and return whether to close the socket.""" + if self.mode == "rst": + connection.reset_stream(stream_id, error_code=ErrorCodes.REFUSED_STREAM) + print("action=REFUSED_STREAM attempt=1", flush=True) + return False + + connection.close_connection( + error_code=ErrorCodes.NO_ERROR, + last_stream_id=0, + ) + print("action=GOAWAY attempt=1 last_stream_id=0", flush=True) + return True + + def serve_connection(self, tls_socket: ssl.SSLSocket) -> bool: + """Serve one HTTP/2 connection and report whether the retry succeeded.""" + connection = H2Connection(config=H2Configuration(client_side=False, header_encoding="utf-8")) + connection.initiate_connection() + tls_socket.sendall(connection.data_to_send()) + + headers_by_stream: dict[int, dict[str, str]] = {} + bodies_by_stream: dict[int, bytearray] = {} + + while True: + data = tls_socket.recv(65535) + if not data: + return False + + close_socket = False + retry_succeeded = False + for event in connection.receive_data(data): + if isinstance(event, RequestReceived): + headers_by_stream[event.stream_id] = dict(event.headers) + bodies_by_stream[event.stream_id] = bytearray() + elif isinstance(event, DataReceived): + bodies_by_stream[event.stream_id].extend(event.data) + connection.acknowledge_received_data(event.flow_controlled_length, event.stream_id) + elif isinstance(event, StreamEnded): + headers = headers_by_stream[event.stream_id] + body = bytes(bodies_by_stream[event.stream_id]) + self._validate_request(headers, body) + self.attempts += 1 + + if self.attempts == 1: + close_socket = self._reject_first_attempt(connection, event.stream_id) + elif self.attempts == 2: + connection.send_headers( + event.stream_id, + [ + (":status", "200"), + ("content-length", str(len(RESPONSE_BODY))), + ], + ) + connection.send_data(event.stream_id, RESPONSE_BODY, end_stream=True) + retry_succeeded = True + else: + raise RuntimeError(f"received unexpected request attempt {self.attempts}") + elif isinstance(event, ConnectionTerminated): + close_socket = True + + wire_bytes = connection.data_to_send() + if wire_bytes: + tls_socket.sendall(wire_bytes) + + if retry_succeeded: + print( + f"retry_succeeded attempts={self.attempts} method=POST body={EXPECTED_BODY.decode()}", + flush=True, + ) + return True + if close_socket: + return False + + +def run_server(mode: str, port: int, certificate: str, private_key: str) -> int: + """Accept connections until ATS successfully retries the request.""" + context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.load_cert_chain(certificate, private_key) + context.set_alpn_protocols(["h2"]) + + origin = RetryOrigin(mode) + with socket.create_server(("127.0.0.1", port)) as listener: + listener.settimeout(30) + while origin.attempts < 2: + plain_socket, _ = listener.accept() + try: + with context.wrap_socket(plain_socket, server_side=True) as tls_socket: + if tls_socket.selected_alpn_protocol() != "h2": + raise RuntimeError("ATS did not negotiate HTTP/2 with the origin") + if origin.serve_connection(tls_socket): + return 0 + except ssl.SSLError: + # AuTest's readiness probe opens and closes a plain TCP socket. + plain_socket.close() + + return 1 + + +def parse_args() -> argparse.Namespace: + """Parse command-line arguments.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("mode", choices=("rst", "goaway")) + parser.add_argument("port", type=int) + parser.add_argument("certificate") + parser.add_argument("private_key") + return parser.parse_args() + + +def main() -> int: + """Run the test origin.""" + args = parse_args() + return run_server(args.mode, args.port, args.certificate, args.private_key) + + +if __name__ == "__main__": + sys.exit(main())