diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index 9110962936cd..7ea1916839b6 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -34,16 +34,6 @@ CodecClient::CodecClient(Type type, Network::ClientConnectionPtr&& connection, connection_->addConnectionCallbacks(*this); connection_->addReadFilter(Network::ReadFilterSharedPtr{new CodecReadFilter(*this)}); - // In general, codecs are handed new not-yet-connected connections, but in the - // case of ALPN, the codec may be handed an already connected connection. - if (!connection_->connecting()) { - ASSERT(connection_->state() == Network::Connection::State::Open); - connected_ = true; - } else { - ENVOY_CONN_LOG(debug, "connecting", *connection_); - connection_->connect(); - } - if (idle_timeout_) { idle_timer_ = dispatcher.createTimer([this]() -> void { onIdleTimeout(); }); enableIdleTimer(); @@ -54,7 +44,23 @@ CodecClient::CodecClient(Type type, Network::ClientConnectionPtr&& connection, connection_->noDelay(true); } -CodecClient::~CodecClient() = default; +CodecClient::~CodecClient() { + ASSERT(connect_called_, "CodecClient::connect() is not called through out the life time."); +} + +void CodecClient::connect() { + connect_called_ = true; + ASSERT(codec_ != nullptr); + // In general, codecs are handed new not-yet-connected connections, but in the + // case of ALPN, the codec may be handed an already connected connection. + if (!connection_->connecting()) { + ASSERT(connection_->state() == Network::Connection::State::Open); + connected_ = true; + } else { + ENVOY_CONN_LOG(debug, "connecting", *connection_); + connection_->connect(); + } +} void CodecClient::close() { connection_->close(Network::ConnectionCloseType::NoFlush); } @@ -168,7 +174,6 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne Event::Dispatcher& dispatcher, Random::RandomGenerator& random_generator) : CodecClient(type, std::move(connection), host, dispatcher) { - switch (type) { case Type::HTTP1: { codec_ = std::make_unique( @@ -185,10 +190,13 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne } case Type::HTTP3: { #ifdef ENVOY_ENABLE_QUIC + auto& quic_session = dynamic_cast(*connection_); codec_ = std::make_unique( - dynamic_cast(*connection_), *this, - host->cluster().http3CodecStats(), host->cluster().http3Options(), + quic_session, *this, host->cluster().http3CodecStats(), host->cluster().http3Options(), Http::DEFAULT_MAX_REQUEST_HEADERS_KB); + // Initialize the session after max request header size is changed in above http client + // connection creation. + quic_session.Initialize(); break; #else // Should be blocked by configuration checking at an earlier point. @@ -196,6 +204,7 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne #endif } } + connect(); } } // namespace Http diff --git a/source/common/http/codec_client.h b/source/common/http/codec_client.h index d0c5e486f076..07eb9724d607 100644 --- a/source/common/http/codec_client.h +++ b/source/common/http/codec_client.h @@ -47,7 +47,7 @@ class CodecClientCallbacks { * This is an HTTP client that multiple stream management and underlying connection management * across multiple HTTP codec types. */ -class CodecClient : Logger::Loggable, +class CodecClient : protected Logger::Loggable, public Http::ConnectionCallbacks, public Network::ConnectionCallbacks, public Event::DeferredDeletable { @@ -140,6 +140,12 @@ class CodecClient : Logger::Loggable, CodecClient(Type type, Network::ClientConnectionPtr&& connection, Upstream::HostDescriptionConstSharedPtr host, Event::Dispatcher& dispatcher); + /** + * Connect to the host. + * Needs to be called after codec_ is instantiated. + */ + void connect(); + // Http::ConnectionCallbacks void onGoAway(GoAwayErrorCode error_code) override { if (codec_callbacks_) { @@ -257,6 +263,7 @@ class CodecClient : Logger::Loggable, bool connected_{}; bool remote_closed_{}; bool protocol_error_{false}; + bool connect_called_{false}; }; using CodecClientPtr = std::unique_ptr; diff --git a/source/common/quic/BUILD b/source/common/quic/BUILD index d46b4b3bc28c..9e4dae98192f 100644 --- a/source/common/quic/BUILD +++ b/source/common/quic/BUILD @@ -180,8 +180,8 @@ envoy_cc_library( hdrs = ["quic_filter_manager_connection_impl.h"], tags = ["nofips"], deps = [ - ":envoy_quic_connection_lib", ":envoy_quic_simulated_watermark_buffer_lib", + ":quic_network_connection_lib", ":send_buffer_monitor_lib", "//include/envoy/event:dispatcher_interface", "//include/envoy/network:connection_interface", @@ -192,6 +192,7 @@ envoy_cc_library( "//source/common/http/http3:codec_stats_lib", "//source/common/network:connection_base_lib", "//source/common/stream_info:stream_info_lib", + "@com_googlesource_quiche//:quic_core_connection_lib", ], ) @@ -208,6 +209,7 @@ envoy_cc_library( tags = ["nofips"], deps = [ ":envoy_quic_proof_source_lib", + ":envoy_quic_server_connection_lib", ":envoy_quic_stream_lib", ":envoy_quic_utils_lib", ":quic_filter_manager_connection_lib", @@ -255,17 +257,13 @@ envoy_cc_library( ) envoy_cc_library( - name = "envoy_quic_connection_lib", - srcs = ["envoy_quic_connection.cc"], - hdrs = ["envoy_quic_connection.h"], + name = "quic_network_connection_lib", + srcs = ["quic_network_connection.cc"], + hdrs = ["quic_network_connection.h"], tags = ["nofips"], deps = [ - ":quic_io_handle_wrapper_lib", "//include/envoy/network:connection_interface", "//source/common/network:listen_socket_lib", - "//source/common/quic:envoy_quic_utils_lib", - "//source/extensions/transport_sockets:well_known_names", - "@com_googlesource_quiche//:quic_core_connection_lib", ], ) @@ -275,8 +273,12 @@ envoy_cc_library( hdrs = ["envoy_quic_server_connection.h"], tags = ["nofips"], deps = [ - ":envoy_quic_connection_lib", + ":quic_io_handle_wrapper_lib", + ":quic_network_connection_lib", + "//source/common/quic:envoy_quic_utils_lib", + "//source/extensions/transport_sockets:well_known_names", "//source/server:connection_handler_lib", + "@com_googlesource_quiche//:quic_core_connection_lib", ], ) @@ -286,11 +288,12 @@ envoy_cc_library( hdrs = ["envoy_quic_client_connection.h"], tags = ["nofips"], deps = [ - ":envoy_quic_connection_lib", ":envoy_quic_packet_writer_lib", + ":quic_network_connection_lib", "//include/envoy/event:dispatcher_interface", "//source/common/network:socket_option_factory_lib", "//source/common/network:udp_packet_writer_handler_lib", + "@com_googlesource_quiche//:quic_core_connection_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) @@ -354,6 +357,8 @@ envoy_cc_library( "//source/common/network:address_lib", "//source/common/network:listen_socket_lib", "//source/common/network:socket_option_factory_lib", + "//source/common/quic:quic_io_handle_wrapper_lib", + "//source/extensions/transport_sockets:well_known_names", "@com_googlesource_quiche//:quic_core_http_header_list_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], diff --git a/source/common/quic/active_quic_listener.cc b/source/common/quic/active_quic_listener.cc index ba987c33ae8a..6fe1af4b38d8 100644 --- a/source/common/quic/active_quic_listener.cc +++ b/source/common/quic/active_quic_listener.cc @@ -46,6 +46,9 @@ ActiveQuicListener::ActiveQuicListener( kernel_worker_routing_(kernel_worker_routing) { // This flag fix a QUICHE issue which may crash Envoy during connection close. SetQuicReloadableFlag(quic_single_ack_in_packet2, true); + // Do not include 32-byte per-entry overhead while counting header size. + quiche::FlagRegistry::getInstance(); + ASSERT(!GetQuicFlag(FLAGS_quic_header_size_limit_includes_overhead)); if (Runtime::LoaderSingleton::getExisting()) { enabled_.emplace(Runtime::FeatureFlag(enabled, Runtime::LoaderSingleton::get())); diff --git a/source/common/quic/client_connection_factory_impl.cc b/source/common/quic/client_connection_factory_impl.cc index 8034c74e4741..ffb6951237f9 100644 --- a/source/common/quic/client_connection_factory_impl.cc +++ b/source/common/quic/client_connection_factory_impl.cc @@ -22,7 +22,9 @@ PersistentQuicInfoImpl::PersistentQuicInfoImpl( static_cast(server_addr->ip()->port()), false}, crypto_config_( std::make_unique(std::make_unique( - stats_scope, getConfig(transport_socket_factory), time_source))) {} + stats_scope, getConfig(transport_socket_factory), time_source))) { + quiche::FlagRegistry::getInstance(); +} namespace { // TODO(alyssawilk, danzh2010): This is mutable static info that is required for the QUICHE code. @@ -53,7 +55,6 @@ createQuicNetworkConnection(Http::PersistentQuicInfo& info, Event::Dispatcher& d static_info.quic_config_, info_impl->supported_versions_, std::move(connection), info_impl->server_id_, info_impl->crypto_config_.get(), &static_info.push_promise_index_, dispatcher, 0); - ret->Initialize(); return ret; } diff --git a/source/common/quic/codec_impl.cc b/source/common/quic/codec_impl.cc index bbabca605cd2..5cdae4dd8e96 100644 --- a/source/common/quic/codec_impl.cc +++ b/source/common/quic/codec_impl.cc @@ -22,7 +22,7 @@ QuicHttpServerConnectionImpl::QuicHttpServerConnectionImpl( EnvoyQuicServerSession& quic_session, Http::ServerConnectionCallbacks& callbacks, Http::Http3::CodecStats& stats, const envoy::config::core::v3::Http3ProtocolOptions& http3_options, - const uint32_t /*max_request_headers_kb*/, + const uint32_t max_request_headers_kb, envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction headers_with_underscores_action) : QuicHttpConnectionImplBase(quic_session, stats), quic_server_session_(quic_session) { @@ -30,6 +30,7 @@ QuicHttpServerConnectionImpl::QuicHttpServerConnectionImpl( quic_session.setHttp3Options(http3_options); quic_session.setHeadersWithUnderscoreAction(headers_with_underscores_action); quic_session.setHttpConnectionCallbacks(callbacks); + quic_session.set_max_inbound_header_list_size(max_request_headers_kb * 1024u); } void QuicHttpServerConnectionImpl::onUnderlyingConnectionAboveWriteBufferHighWatermark() { @@ -68,11 +69,12 @@ QuicHttpClientConnectionImpl::QuicHttpClientConnectionImpl( EnvoyQuicClientSession& session, Http::ConnectionCallbacks& callbacks, Http::Http3::CodecStats& stats, const envoy::config::core::v3::Http3ProtocolOptions& http3_options, - const uint32_t /*max_request_headers_kb*/) + const uint32_t max_request_headers_kb) : QuicHttpConnectionImplBase(session, stats), quic_client_session_(session) { session.setCodecStats(stats); session.setHttp3Options(http3_options); session.setHttpConnectionCallbacks(callbacks); + session.set_max_inbound_header_list_size(max_request_headers_kb * 1024); } Http::RequestEncoder& diff --git a/source/common/quic/envoy_quic_client_connection.cc b/source/common/quic/envoy_quic_client_connection.cc index 3b914b91f630..158fa4d2244b 100644 --- a/source/common/quic/envoy_quic_client_connection.cc +++ b/source/common/quic/envoy_quic_client_connection.cc @@ -42,12 +42,12 @@ EnvoyQuicClientConnection::EnvoyQuicClientConnection( quic::QuicAlarmFactory& alarm_factory, quic::QuicPacketWriter* writer, bool owns_writer, const quic::ParsedQuicVersionVector& supported_versions, Event::Dispatcher& dispatcher, Network::ConnectionSocketPtr&& connection_socket) - : EnvoyQuicConnection(server_connection_id, quic::QuicSocketAddress(), - envoyIpAddressToQuicSocketAddress( - connection_socket->addressProvider().remoteAddress()->ip()), - helper, alarm_factory, writer, owns_writer, quic::Perspective::IS_CLIENT, - supported_versions, std::move(connection_socket)), - dispatcher_(dispatcher) {} + : quic::QuicConnection(server_connection_id, quic::QuicSocketAddress(), + envoyIpAddressToQuicSocketAddress( + connection_socket->addressProvider().remoteAddress()->ip()), + &helper, &alarm_factory, writer, owns_writer, + quic::Perspective::IS_CLIENT, supported_versions), + QuicNetworkConnection(std::move(connection_socket)), dispatcher_(dispatcher) {} void EnvoyQuicClientConnection::processPacket( Network::Address::InstanceConstSharedPtr local_address, diff --git a/source/common/quic/envoy_quic_client_connection.h b/source/common/quic/envoy_quic_client_connection.h index 8fa3b18100e7..dc3587a3b05c 100644 --- a/source/common/quic/envoy_quic_client_connection.h +++ b/source/common/quic/envoy_quic_client_connection.h @@ -3,13 +3,28 @@ #include "envoy/event/dispatcher.h" #include "common/network/utility.h" -#include "common/quic/envoy_quic_connection.h" +#include "common/quic/envoy_quic_utils.h" +#include "common/quic/quic_network_connection.h" + +#if defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-parameter" +#pragma GCC diagnostic ignored "-Winvalid-offsetof" +#endif + +#include "quiche/quic/core/quic_connection.h" + +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif namespace Envoy { namespace Quic { // A client QuicConnection instance managing its own file events. -class EnvoyQuicClientConnection : public EnvoyQuicConnection, public Network::UdpPacketProcessor { +class EnvoyQuicClientConnection : public quic::QuicConnection, + public QuicNetworkConnection, + public Network::UdpPacketProcessor { public: // A connection socket will be created with given |local_addr|. If binding // port not provided in |local_addr|, pick up a random port. diff --git a/source/common/quic/envoy_quic_client_session.cc b/source/common/quic/envoy_quic_client_session.cc index 7c969926fa1e..6f0484c9535e 100644 --- a/source/common/quic/envoy_quic_client_session.cc +++ b/source/common/quic/envoy_quic_client_session.cc @@ -11,23 +11,21 @@ EnvoyQuicClientSession::EnvoyQuicClientSession( quic::QuicCryptoClientConfig* crypto_config, quic::QuicClientPushPromiseIndex* push_promise_index, Event::Dispatcher& dispatcher, uint32_t send_buffer_limit) - : QuicFilterManagerConnectionImpl(*connection, dispatcher, send_buffer_limit), + : QuicFilterManagerConnectionImpl(*connection, connection->connection_id(), dispatcher, + send_buffer_limit), quic::QuicSpdyClientSession(config, supported_versions, connection.release(), server_id, crypto_config, push_promise_index), - host_name_(server_id.host()) { - // HTTP/3 header limits should be configurable, but for now hard-code to Envoy defaults. - set_max_inbound_header_list_size(Http::DEFAULT_MAX_REQUEST_HEADERS_KB * 1000); -} + host_name_(server_id.host()) {} EnvoyQuicClientSession::~EnvoyQuicClientSession() { ASSERT(!connection()->connected()); - quic_connection_ = nullptr; + network_connection_ = nullptr; } absl::string_view EnvoyQuicClientSession::requestedServerName() const { return host_name_; } void EnvoyQuicClientSession::connect() { - dynamic_cast(quic_connection_)->setUpConnectionSocket(); + dynamic_cast(network_connection_)->setUpConnectionSocket(); // Start version negotiation and crypto handshake during which the connection may fail if server // doesn't support the one and only supported version. CryptoConnect(); @@ -41,7 +39,8 @@ void EnvoyQuicClientSession::OnConnectionClosed(const quic::QuicConnectionCloseF void EnvoyQuicClientSession::Initialize() { quic::QuicSpdyClientSession::Initialize(); - quic_connection_->setEnvoyConnection(*this); + initialized_ = true; + network_connection_->setEnvoyConnection(*this); } void EnvoyQuicClientSession::OnCanWrite() { @@ -102,6 +101,14 @@ EnvoyQuicClientSession::CreateIncomingStream(quic::PendingStream* /*pending*/) { bool EnvoyQuicClientSession::hasDataToWrite() { return HasDataToWrite(); } +const quic::QuicConnection* EnvoyQuicClientSession::quicConnection() const { + return initialized_ ? connection() : nullptr; +} + +quic::QuicConnection* EnvoyQuicClientSession::quicConnection() { + return initialized_ ? connection() : nullptr; +} + void EnvoyQuicClientSession::OnTlsHandshakeComplete() { raiseConnectionEvent(Network::ConnectionEvent::Connected); } diff --git a/source/common/quic/envoy_quic_client_session.h b/source/common/quic/envoy_quic_client_session.h index 54398175b83c..638d99ffcb80 100644 --- a/source/common/quic/envoy_quic_client_session.h +++ b/source/common/quic/envoy_quic_client_session.h @@ -77,6 +77,9 @@ class EnvoyQuicClientSession : public QuicFilterManagerConnectionImpl, // QuicFilterManagerConnectionImpl bool hasDataToWrite() override; + // Used by base class to access quic connection after initialization. + const quic::QuicConnection* quicConnection() const override; + quic::QuicConnection* quicConnection() override; private: // These callbacks are owned by network filters and quic session should outlive diff --git a/source/common/quic/envoy_quic_connection.cc b/source/common/quic/envoy_quic_connection.cc deleted file mode 100644 index 5a51ada8cc19..000000000000 --- a/source/common/quic/envoy_quic_connection.cc +++ /dev/null @@ -1,27 +0,0 @@ -#include "common/quic/envoy_quic_connection.h" - -#include "common/quic/envoy_quic_utils.h" - -namespace Envoy { -namespace Quic { - -EnvoyQuicConnection::EnvoyQuicConnection(const quic::QuicConnectionId& server_connection_id, - quic::QuicSocketAddress initial_self_address, - quic::QuicSocketAddress initial_peer_address, - quic::QuicConnectionHelperInterface& helper, - quic::QuicAlarmFactory& alarm_factory, - quic::QuicPacketWriter* writer, bool owns_writer, - quic::Perspective perspective, - const quic::ParsedQuicVersionVector& supported_versions, - Network::ConnectionSocketPtr&& connection_socket) - : quic::QuicConnection(server_connection_id, initial_self_address, initial_peer_address, - &helper, &alarm_factory, writer, owns_writer, perspective, - supported_versions), - connection_socket_(std::move(connection_socket)) {} - -EnvoyQuicConnection::~EnvoyQuicConnection() { connection_socket_->close(); } - -uint64_t EnvoyQuicConnection::id() const { return envoy_connection_->id(); } - -} // namespace Quic -} // namespace Envoy diff --git a/source/common/quic/envoy_quic_dispatcher.cc b/source/common/quic/envoy_quic_dispatcher.cc index 6a7caeba272d..862683019404 100644 --- a/source/common/quic/envoy_quic_dispatcher.cc +++ b/source/common/quic/envoy_quic_dispatcher.cc @@ -3,6 +3,7 @@ #include "common/http/utility.h" #include "common/quic/envoy_quic_server_connection.h" #include "common/quic/envoy_quic_server_session.h" +#include "common/quic/envoy_quic_utils.h" namespace Envoy { namespace Quic { @@ -48,21 +49,34 @@ void EnvoyQuicDispatcher::OnConnectionClosed(quic::QuicConnectionId connection_i std::unique_ptr EnvoyQuicDispatcher::CreateQuicSession( quic::QuicConnectionId server_connection_id, const quic::QuicSocketAddress& self_address, - const quic::QuicSocketAddress& peer_address, absl::string_view /*alpn*/, - const quic::ParsedQuicVersion& version, absl::string_view /*sni*/) { + const quic::QuicSocketAddress& peer_address, absl::string_view alpn, + const quic::ParsedQuicVersion& version, absl::string_view sni) { quic::QuicConfig quic_config = config(); // TODO(danzh) setup flow control window via config. quic_config.SetInitialStreamFlowControlWindowToSend( Http2::Utility::OptionsLimits::MIN_INITIAL_STREAM_WINDOW_SIZE); quic_config.SetInitialSessionFlowControlWindowToSend( 1.5 * Http2::Utility::OptionsLimits::MIN_INITIAL_STREAM_WINDOW_SIZE); + + Network::ConnectionSocketPtr connection_socket = createServerConnectionSocket( + listen_socket_.ioHandle(), self_address, peer_address, std::string(sni), alpn); + const Network::FilterChain* filter_chain = + listener_config_.filterChainManager().findFilterChain(*connection_socket); + auto quic_connection = std::make_unique( server_connection_id, self_address, peer_address, *helper(), *alarm_factory(), writer(), - /*owns_writer=*/false, quic::ParsedQuicVersionVector{version}, listen_socket_); + /*owns_writer=*/false, quic::ParsedQuicVersionVector{version}, std::move(connection_socket)); auto quic_session = std::make_unique( quic_config, quic::ParsedQuicVersionVector{version}, std::move(quic_connection), this, session_helper(), crypto_config(), compressed_certs_cache(), dispatcher_, - listener_config_.perConnectionBufferLimitBytes(), listener_config_); + listener_config_.perConnectionBufferLimitBytes()); + if (filter_chain != nullptr) { + const bool has_filter_initialized = + listener_config_.filterChainFactory().createNetworkFilterChain( + *quic_session, filter_chain->networkFilterFactories()); + // QUIC listener must have HCM filter configured. Otherwise, stream creation later will fail. + ASSERT(has_filter_initialized); + } quic_session->Initialize(); // Filter chain can't be retrieved here as self address is unknown at this // point. diff --git a/source/common/quic/envoy_quic_proof_source.cc b/source/common/quic/envoy_quic_proof_source.cc index bc0dfb8c8de8..fd9554f1a7a8 100644 --- a/source/common/quic/envoy_quic_proof_source.cc +++ b/source/common/quic/envoy_quic_proof_source.cc @@ -82,16 +82,12 @@ EnvoyQuicProofSource::getTlsCertConfigAndFilterChain(const quic::QuicSocketAddre const quic::QuicSocketAddress& client_address, const std::string& hostname) { ENVOY_LOG(trace, "Getting cert chain for {}", hostname); - Network::ConnectionSocketImpl connection_socket( - std::make_unique(listen_socket_.ioHandle()), - quicAddressToEnvoyAddressInstance(server_address), - quicAddressToEnvoyAddressInstance(client_address)); - connection_socket.setDetectedTransportProtocol( - Extensions::TransportSockets::TransportProtocolNames::get().Quic); - connection_socket.setRequestedServerName(hostname); - connection_socket.setRequestedApplicationProtocols({"h2"}); + // TODO(danzh) modify QUICHE to make quic session or ALPN accessible to avoid hard-coded ALPN. + Network::ConnectionSocketPtr connection_socket = createServerConnectionSocket( + listen_socket_.ioHandle(), server_address, client_address, hostname, "h3-29"); const Network::FilterChain* filter_chain = - filter_chain_manager_.findFilterChain(connection_socket); + filter_chain_manager_.findFilterChain(*connection_socket); + if (filter_chain == nullptr) { listener_stats_.no_filter_chain_match_.inc(); ENVOY_LOG(warn, "No matching filter chain found for handshake."); diff --git a/source/common/quic/envoy_quic_server_connection.cc b/source/common/quic/envoy_quic_server_connection.cc index 290b60e0fbb1..b17a9a88663f 100644 --- a/source/common/quic/envoy_quic_server_connection.cc +++ b/source/common/quic/envoy_quic_server_connection.cc @@ -14,29 +14,26 @@ EnvoyQuicServerConnection::EnvoyQuicServerConnection( quic::QuicSocketAddress initial_self_address, quic::QuicSocketAddress initial_peer_address, quic::QuicConnectionHelperInterface& helper, quic::QuicAlarmFactory& alarm_factory, quic::QuicPacketWriter* writer, bool owns_writer, - const quic::ParsedQuicVersionVector& supported_versions, Network::Socket& listen_socket) - : EnvoyQuicConnection(server_connection_id, initial_self_address, initial_peer_address, helper, - alarm_factory, writer, owns_writer, quic::Perspective::IS_SERVER, - supported_versions, - std::make_unique( - // Wraps the real IoHandle instance so that if the connection socket - // gets closed, the real IoHandle won't be affected. - std::make_unique(listen_socket.ioHandle()), - nullptr, quicAddressToEnvoyAddressInstance(initial_peer_address))) {} + const quic::ParsedQuicVersionVector& supported_versions, + Network::ConnectionSocketPtr connection_socket) + : quic::QuicConnection(server_connection_id, initial_self_address, initial_peer_address, + &helper, &alarm_factory, writer, owns_writer, + quic::Perspective::IS_SERVER, supported_versions), + QuicNetworkConnection(std::move(connection_socket)) {} bool EnvoyQuicServerConnection::OnPacketHeader(const quic::QuicPacketHeader& header) { - if (!EnvoyQuicConnection::OnPacketHeader(header)) { + quic::QuicSocketAddress old_self_address = self_address(); + if (!quic::QuicConnection::OnPacketHeader(header)) { return false; } - if (connectionSocket()->addressProvider().localAddress() != nullptr) { + if (old_self_address == self_address()) { return true; } + // Update local address if QUICHE has updated the self address. ASSERT(self_address().IsInitialized()); - // Self address should be initialized by now. connectionSocket()->addressProvider().setLocalAddress( quicAddressToEnvoyAddressInstance(self_address())); - connectionSocket()->setDetectedTransportProtocol( - Extensions::TransportSockets::TransportProtocolNames::get().Quic); + return true; } diff --git a/source/common/quic/envoy_quic_server_connection.h b/source/common/quic/envoy_quic_server_connection.h index 62903793d985..04c1a4b8f80d 100644 --- a/source/common/quic/envoy_quic_server_connection.h +++ b/source/common/quic/envoy_quic_server_connection.h @@ -2,14 +2,27 @@ #include "envoy/network/listener.h" -#include "common/quic/envoy_quic_connection.h" +#include "common/quic/envoy_quic_utils.h" +#include "common/quic/quic_network_connection.h" #include "server/connection_handler_impl.h" +#if defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-parameter" +#pragma GCC diagnostic ignored "-Winvalid-offsetof" +#endif + +#include "quiche/quic/core/quic_connection.h" + +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif + namespace Envoy { namespace Quic { -class EnvoyQuicServerConnection : public EnvoyQuicConnection { +class EnvoyQuicServerConnection : public quic::QuicConnection, public QuicNetworkConnection { public: EnvoyQuicServerConnection(const quic::QuicConnectionId& server_connection_id, quic::QuicSocketAddress initial_self_address, @@ -18,9 +31,9 @@ class EnvoyQuicServerConnection : public EnvoyQuicConnection { quic::QuicAlarmFactory& alarm_factory, quic::QuicPacketWriter* writer, bool owns_writer, const quic::ParsedQuicVersionVector& supported_versions, - Network::Socket& listen_socket); + Network::ConnectionSocketPtr connection_socket); - // EnvoyQuicConnection + // QuicNetworkConnection // Overridden to set connection_socket_ with initialized self address and retrieve filter chain. bool OnPacketHeader(const quic::QuicPacketHeader& header) override; }; diff --git a/source/common/quic/envoy_quic_server_session.cc b/source/common/quic/envoy_quic_server_session.cc index d13da33e4110..44208d4fb3f3 100644 --- a/source/common/quic/envoy_quic_server_session.cc +++ b/source/common/quic/envoy_quic_server_session.cc @@ -11,21 +11,19 @@ namespace Quic { EnvoyQuicServerSession::EnvoyQuicServerSession( const quic::QuicConfig& config, const quic::ParsedQuicVersionVector& supported_versions, - std::unique_ptr connection, quic::QuicSession::Visitor* visitor, + std::unique_ptr connection, quic::QuicSession::Visitor* visitor, quic::QuicCryptoServerStream::Helper* helper, const quic::QuicCryptoServerConfig* crypto_config, quic::QuicCompressedCertsCache* compressed_certs_cache, Event::Dispatcher& dispatcher, - uint32_t send_buffer_limit, Network::ListenerConfig& listener_config) + uint32_t send_buffer_limit) : quic::QuicServerSessionBase(config, supported_versions, connection.get(), visitor, helper, crypto_config, compressed_certs_cache), - QuicFilterManagerConnectionImpl(*connection, dispatcher, send_buffer_limit), - quic_connection_(std::move(connection)), listener_config_(listener_config) { - // HTTP/3 header limits should be configurable, but for now hard-code to Envoy defaults. - set_max_inbound_header_list_size(Http::DEFAULT_MAX_REQUEST_HEADERS_KB * 1000); -} + QuicFilterManagerConnectionImpl(*connection, connection->connection_id(), dispatcher, + send_buffer_limit), + quic_connection_(std::move(connection)) {} EnvoyQuicServerSession::~EnvoyQuicServerSession() { ASSERT(!quic_connection_->connected()); - QuicFilterManagerConnectionImpl::quic_connection_ = nullptr; + QuicFilterManagerConnectionImpl::network_connection_ = nullptr; } absl::string_view EnvoyQuicServerSession::requestedServerName() const { @@ -89,6 +87,7 @@ void EnvoyQuicServerSession::OnConnectionClosed(const quic::QuicConnectionCloseF void EnvoyQuicServerSession::Initialize() { quic::QuicServerSessionBase::Initialize(); + initialized_ = true; quic_connection_->setEnvoyConnection(*this); } @@ -109,29 +108,23 @@ void EnvoyQuicServerSession::SetDefaultEncryptionLevel(quic::EncryptionLevel lev if (level != quic::ENCRYPTION_FORWARD_SECURE) { return; } - maybeCreateNetworkFilters(); // This is only reached once, when handshake is done. raiseConnectionEvent(Network::ConnectionEvent::Connected); } bool EnvoyQuicServerSession::hasDataToWrite() { return HasDataToWrite(); } -void EnvoyQuicServerSession::OnTlsHandshakeComplete() { - quic::QuicServerSessionBase::OnTlsHandshakeComplete(); - maybeCreateNetworkFilters(); - raiseConnectionEvent(Network::ConnectionEvent::Connected); +const quic::QuicConnection* EnvoyQuicServerSession::quicConnection() const { + return initialized_ ? connection() : nullptr; } -void EnvoyQuicServerSession::maybeCreateNetworkFilters() { - auto proof_source_details = - dynamic_cast(GetCryptoStream()->ProofSourceDetails()); - ASSERT(proof_source_details != nullptr, - "ProofSource didn't provide ProofSource::Details. No filter chain will be installed."); +quic::QuicConnection* EnvoyQuicServerSession::quicConnection() { + return initialized_ ? connection() : nullptr; +} - const bool has_filter_initialized = - listener_config_.filterChainFactory().createNetworkFilterChain( - *this, proof_source_details->filterChain().networkFilterFactories()); - ASSERT(has_filter_initialized); +void EnvoyQuicServerSession::OnTlsHandshakeComplete() { + quic::QuicServerSessionBase::OnTlsHandshakeComplete(); + raiseConnectionEvent(Network::ConnectionEvent::Connected); } size_t EnvoyQuicServerSession::WriteHeadersOnHeadersStream( diff --git a/source/common/quic/envoy_quic_server_session.h b/source/common/quic/envoy_quic_server_session.h index e80e42c9e62f..1e4900e5632a 100644 --- a/source/common/quic/envoy_quic_server_session.h +++ b/source/common/quic/envoy_quic_server_session.h @@ -19,6 +19,7 @@ #include "common/quic/send_buffer_monitor.h" #include "common/quic/quic_filter_manager_connection_impl.h" +#include "common/quic/envoy_quic_server_connection.h" #include "common/quic/envoy_quic_server_stream.h" namespace Envoy { @@ -33,13 +34,12 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase, public: EnvoyQuicServerSession(const quic::QuicConfig& config, const quic::ParsedQuicVersionVector& supported_versions, - std::unique_ptr connection, + std::unique_ptr connection, quic::QuicSession::Visitor* visitor, quic::QuicCryptoServerStreamBase::Helper* helper, const quic::QuicCryptoServerConfig* crypto_config, quic::QuicCompressedCertsCache* compressed_certs_cache, - Event::Dispatcher& dispatcher, uint32_t send_buffer_limit, - Network::ListenerConfig& listener_config); + Event::Dispatcher& dispatcher, uint32_t send_buffer_limit); ~EnvoyQuicServerSession() override; @@ -87,13 +87,14 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase, // QuicFilterManagerConnectionImpl bool hasDataToWrite() override; + // Used by base class to access quic connection after initialization. + const quic::QuicConnection* quicConnection() const override; + quic::QuicConnection* quicConnection() override; private: void setUpRequestDecoder(EnvoyQuicServerStream& stream); - void maybeCreateNetworkFilters(); - std::unique_ptr quic_connection_; - Network::ListenerConfig& listener_config_; + std::unique_ptr quic_connection_; // These callbacks are owned by network filters and quic session should out live // them. Http::ServerConnectionCallbacks* http_connection_callbacks_{nullptr}; diff --git a/source/common/quic/envoy_quic_utils.cc b/source/common/quic/envoy_quic_utils.cc index 3da9c12702ea..8f9460db3e6c 100644 --- a/source/common/quic/envoy_quic_utils.cc +++ b/source/common/quic/envoy_quic_utils.cc @@ -1,11 +1,15 @@ #include "common/quic/envoy_quic_utils.h" +#include + #include "envoy/common/platform.h" #include "envoy/config/core/v3/base.pb.h" #include "common/network/socket_option_factory.h" #include "common/network/utility.h" +#include "extensions/transport_sockets/well_known_names.h" + namespace Envoy { namespace Quic { @@ -223,5 +227,21 @@ int deduceSignatureAlgorithmFromPublicKey(const EVP_PKEY* public_key, std::strin return sign_alg; } +Network::ConnectionSocketPtr +createServerConnectionSocket(Network::IoHandle& io_handle, + const quic::QuicSocketAddress& self_address, + const quic::QuicSocketAddress& peer_address, + const std::string& hostname, absl::string_view alpn) { + auto connection_socket = std::make_unique( + std::make_unique(io_handle), + quicAddressToEnvoyAddressInstance(self_address), + quicAddressToEnvoyAddressInstance(peer_address)); + connection_socket->setDetectedTransportProtocol( + Extensions::TransportSockets::TransportProtocolNames::get().Quic); + connection_socket->setRequestedServerName(hostname); + connection_socket->setRequestedApplicationProtocols({alpn}); + return connection_socket; +} + } // namespace Quic } // namespace Envoy diff --git a/source/common/quic/envoy_quic_utils.h b/source/common/quic/envoy_quic_utils.h index 8898bd1e4971..f30c582b7d53 100644 --- a/source/common/quic/envoy_quic_utils.h +++ b/source/common/quic/envoy_quic_utils.h @@ -7,6 +7,7 @@ #include "common/http/header_map_impl.h" #include "common/network/address_impl.h" #include "common/network/listen_socket_impl.h" +#include "common/quic/quic_io_handle_wrapper.h" #if defined(__GNUC__) #pragma GCC diagnostic push @@ -125,5 +126,13 @@ bssl::UniquePtr parseDERCertificate(const std::string& der_bytes, std::str // not supported, return 0 with error_details populated correspondingly. int deduceSignatureAlgorithmFromPublicKey(const EVP_PKEY* public_key, std::string* error_details); +// Return a connection socket which read and write via io_handle, but doesn't close it when the +// socket gets closed nor set options on the socket. +Network::ConnectionSocketPtr +createServerConnectionSocket(Network::IoHandle& io_handle, + const quic::QuicSocketAddress& self_address, + const quic::QuicSocketAddress& peer_address, + const std::string& hostname, absl::string_view alpn); + } // namespace Quic } // namespace Envoy diff --git a/source/common/quic/platform/quiche_flags_impl.cc b/source/common/quic/platform/quiche_flags_impl.cc index 6b2f9a638df4..641f31ae0b2f 100644 --- a/source/common/quic/platform/quiche_flags_impl.cc +++ b/source/common/quic/platform/quiche_flags_impl.cc @@ -33,9 +33,8 @@ absl::flat_hash_map makeFlagMap() { #define QUIC_PROTOCOL_FLAG(type, flag, ...) flags.emplace(FLAGS_##flag->name(), FLAGS_##flag); #include "quiche/quic/core/quic_protocol_flags_list.h" #undef QUIC_PROTOCOL_FLAG - - // TODO(danzh) Re-enable TLS resumption after #15912 is checked in. - FLAGS_quic_disable_server_tls_resumption->setValue(true); + // Do not include 32-byte per-entry overhead while counting header size. + FLAGS_quic_header_size_limit_includes_overhead->setValue(false); return flags; } diff --git a/source/common/quic/quic_filter_manager_connection_impl.cc b/source/common/quic/quic_filter_manager_connection_impl.cc index b58b0a4a25a5..e47c27080d1d 100644 --- a/source/common/quic/quic_filter_manager_connection_impl.cc +++ b/source/common/quic/quic_filter_manager_connection_impl.cc @@ -1,17 +1,18 @@ #include "common/quic/quic_filter_manager_connection_impl.h" +#include #include namespace Envoy { namespace Quic { -QuicFilterManagerConnectionImpl::QuicFilterManagerConnectionImpl(EnvoyQuicConnection& connection, - Event::Dispatcher& dispatcher, - uint32_t send_buffer_limit) +QuicFilterManagerConnectionImpl::QuicFilterManagerConnectionImpl( + QuicNetworkConnection& connection, const quic::QuicConnectionId& connection_id, + Event::Dispatcher& dispatcher, uint32_t send_buffer_limit) // Using this for purpose other than logging is not safe. Because QUIC connection id can be // 18 bytes, so there might be collision when it's hashed to 8 bytes. - : Network::ConnectionImplBase(dispatcher, /*id=*/connection.connection_id().Hash()), - quic_connection_(&connection), filter_manager_(*this, *connection.connectionSocket()), + : Network::ConnectionImplBase(dispatcher, /*id=*/connection_id.Hash()), + network_connection_(&connection), filter_manager_(*this, *connection.connectionSocket()), stream_info_(dispatcher.timeSource(), connection.connectionSocket()->addressProviderSharedPtr()), write_buffer_watermark_simulation_( @@ -61,10 +62,15 @@ bool QuicFilterManagerConnectionImpl::aboveHighWatermark() const { } void QuicFilterManagerConnectionImpl::close(Network::ConnectionCloseType type) { - if (quic_connection_ == nullptr) { + if (network_connection_ == nullptr) { // Already detached from quic connection. return; } + if (!initialized_) { + // Delay close till the first OnCanWrite() call. + close_type_during_initialize_ = type; + return; + } const bool delayed_close_timeout_configured = delayed_close_timeout_.count() > 0; if (hasDataToWrite() && type != Network::ConnectionCloseType::NoFlush) { if (delayed_close_timeout_configured) { @@ -105,7 +111,7 @@ void QuicFilterManagerConnectionImpl::close(Network::ConnectionCloseType type) { const Network::ConnectionSocket::OptionsSharedPtr& QuicFilterManagerConnectionImpl::socketOptions() const { - return quic_connection_->connectionSocket()->options(); + return network_connection_->connectionSocket()->options(); } Ssl::ConnectionInfoConstSharedPtr QuicFilterManagerConnectionImpl::ssl() const { @@ -134,6 +140,10 @@ void QuicFilterManagerConnectionImpl::updateBytesBuffered(size_t old_buffered_by void QuicFilterManagerConnectionImpl::maybeApplyDelayClosePolicy() { if (!inDelayedClose()) { + if (close_type_during_initialize_.has_value()) { + close(close_type_during_initialize_.value()); + close_type_during_initialize_ = absl::nullopt; + } return; } if (hasDataToWrite() || delayed_close_state_ == DelayedCloseState::CloseAfterFlushAndWait) { @@ -151,21 +161,21 @@ void QuicFilterManagerConnectionImpl::onConnectionCloseEvent( const quic::QuicConnectionCloseFrame& frame, quic::ConnectionCloseSource source) { transport_failure_reason_ = absl::StrCat(quic::QuicErrorCodeToString(frame.quic_error_code), " with details: ", frame.error_details); - if (quic_connection_ != nullptr) { + if (network_connection_ != nullptr) { // Tell network callbacks about connection close if not detached yet. raiseConnectionEvent(source == quic::ConnectionCloseSource::FROM_PEER ? Network::ConnectionEvent::RemoteClose : Network::ConnectionEvent::LocalClose); - ASSERT(quic_connection_ != nullptr); - quic_connection_ = nullptr; + ASSERT(network_connection_ != nullptr); + network_connection_ = nullptr; } } void QuicFilterManagerConnectionImpl::closeConnectionImmediately() { - if (quic_connection_ == nullptr) { + if (quicConnection() == nullptr) { return; } - quic_connection_->CloseConnection(quic::QUIC_NO_ERROR, "Closed by application", + quicConnection()->CloseConnection(quic::QUIC_NO_ERROR, "Closed by application", quic::ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); } diff --git a/source/common/quic/quic_filter_manager_connection_impl.h b/source/common/quic/quic_filter_manager_connection_impl.h index a1532222ddb5..91cfd98b8e23 100644 --- a/source/common/quic/quic_filter_manager_connection_impl.h +++ b/source/common/quic/quic_filter_manager_connection_impl.h @@ -5,11 +5,23 @@ #include "envoy/event/dispatcher.h" #include "envoy/network/connection.h" +#if defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-parameter" +#pragma GCC diagnostic ignored "-Winvalid-offsetof" +#endif + +#include "quiche/quic/core/quic_connection.h" + +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif + #include "common/common/empty_string.h" #include "common/common/logger.h" #include "common/http/http3/codec_stats.h" #include "common/network/connection_impl_base.h" -#include "common/quic/envoy_quic_connection.h" +#include "common/quic/quic_network_connection.h" #include "common/quic/envoy_quic_simulated_watermark_buffer.h" #include "common/quic/send_buffer_monitor.h" #include "common/stream_info/stream_info_impl.h" @@ -24,8 +36,9 @@ namespace Quic { class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, public SendBufferMonitor { public: - QuicFilterManagerConnectionImpl(EnvoyQuicConnection& connection, Event::Dispatcher& dispatcher, - uint32_t send_buffer_limit); + QuicFilterManagerConnectionImpl(QuicNetworkConnection& connection, + const quic::QuicConnectionId& connection_id, + Event::Dispatcher& dispatcher, uint32_t send_buffer_limit); ~QuicFilterManagerConnectionImpl() override = default; // Network::FilterManager @@ -56,10 +69,10 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, void detectEarlyCloseWhenReadDisabled(bool /*value*/) override { ASSERT(false); } bool readEnabled() const override { return true; } const Network::SocketAddressSetter& addressProvider() const override { - return quic_connection_->connectionSocket()->addressProvider(); + return network_connection_->connectionSocket()->addressProvider(); } Network::SocketAddressProviderSharedPtr addressProviderSharedPtr() const override { - return quic_connection_->connectionSocket()->addressProviderSharedPtr(); + return network_connection_->connectionSocket()->addressProviderSharedPtr(); } absl::optional unixSocketPeerCredentials() const override { @@ -69,20 +82,20 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, void setConnectionStats(const Network::Connection::ConnectionStats& stats) override { // TODO(danzh): populate stats. Network::ConnectionImplBase::setConnectionStats(stats); - quic_connection_->setConnectionStats(stats); + network_connection_->setConnectionStats(stats); } Ssl::ConnectionInfoConstSharedPtr ssl() const override; Network::Connection::State state() const override { - if (quic_connection_ != nullptr && quic_connection_->connected()) { + if (!initialized_ || (quicConnection() != nullptr && quicConnection()->connected())) { return Network::Connection::State::Open; } return Network::Connection::State::Closed; } bool connecting() const override { - if (quic_connection_ != nullptr && !quic_connection_->IsHandshakeComplete()) { - return true; + if (initialized_ && (quicConnection() == nullptr || quicConnection()->IsHandshakeComplete())) { + return false; } - return false; + return true; } void write(Buffer::Instance& /*data*/, bool /*end_stream*/) override { // All writes should be handled by Quic internally. @@ -138,11 +151,16 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, virtual bool hasDataToWrite() PURE; - EnvoyQuicConnection* quic_connection_{nullptr}; + // Returns a QuicConnection interface if initialized_ is true, otherwise nullptr. + virtual const quic::QuicConnection* quicConnection() const = 0; + virtual quic::QuicConnection* quicConnection() = 0; + + QuicNetworkConnection* network_connection_{nullptr}; absl::optional> codec_stats_; absl::optional> http3_options_; + bool initialized_{false}; private: friend class Envoy::TestPauseFilterForQuic; @@ -167,6 +185,7 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, // send buffer. EnvoyQuicSimulatedWatermarkBuffer write_buffer_watermark_simulation_; Buffer::OwnedImpl empty_buffer_; + absl::optional close_type_during_initialize_; }; } // namespace Quic diff --git a/source/common/quic/quic_network_connection.cc b/source/common/quic/quic_network_connection.cc new file mode 100644 index 000000000000..577178c9cc32 --- /dev/null +++ b/source/common/quic/quic_network_connection.cc @@ -0,0 +1,14 @@ +#include "common/quic/quic_network_connection.h" + +namespace Envoy { +namespace Quic { + +QuicNetworkConnection::QuicNetworkConnection(Network::ConnectionSocketPtr&& connection_socket) + : connection_socket_(std::move(connection_socket)) {} + +QuicNetworkConnection::~QuicNetworkConnection() { connection_socket_->close(); } + +uint64_t QuicNetworkConnection::id() const { return envoy_connection_->id(); } + +} // namespace Quic +} // namespace Envoy diff --git a/source/common/quic/envoy_quic_connection.h b/source/common/quic/quic_network_connection.h similarity index 54% rename from source/common/quic/envoy_quic_connection.h rename to source/common/quic/quic_network_connection.h index d853252e777f..66c51a7d5215 100644 --- a/source/common/quic/envoy_quic_connection.h +++ b/source/common/quic/quic_network_connection.h @@ -1,40 +1,21 @@ #pragma once -#if defined(__GNUC__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wunused-parameter" -#pragma GCC diagnostic ignored "-Winvalid-offsetof" -#endif - -#include "quiche/quic/core/quic_connection.h" - -#if defined(__GNUC__) -#pragma GCC diagnostic pop -#endif - #include -#include "common/common/logger.h" #include "envoy/network/connection.h" +#include "common/common/logger.h" + namespace Envoy { namespace Quic { -// Derived for network filter chain, stats and QoS. This is used on both client -// and server side. -class EnvoyQuicConnection : public quic::QuicConnection, - protected Logger::Loggable { +// A base class of both the client and server connections which keeps stats and +// connection socket. +class QuicNetworkConnection : protected Logger::Loggable { public: - EnvoyQuicConnection(const quic::QuicConnectionId& server_connection_id, - quic::QuicSocketAddress initial_self_address, - quic::QuicSocketAddress initial_peer_address, - quic::QuicConnectionHelperInterface& helper, - quic::QuicAlarmFactory& alarm_factory, quic::QuicPacketWriter* writer, - bool owns_writer, quic::Perspective perspective, - const quic::ParsedQuicVersionVector& supported_versions, - Network::ConnectionSocketPtr&& connection_socket); + QuicNetworkConnection(Network::ConnectionSocketPtr&& connection_socket); - ~EnvoyQuicConnection() override; + virtual ~QuicNetworkConnection(); // Called by EnvoyQuicSession::setConnectionStats(). void setConnectionStats(const Network::Connection::ConnectionStats& stats) { diff --git a/test/common/http/common.h b/test/common/http/common.h index c031767ef347..3a28e9b369b8 100644 --- a/test/common/http/common.h +++ b/test/common/http/common.h @@ -22,6 +22,7 @@ class CodecClientForTest : public Http::CodecClient { Upstream::HostDescriptionConstSharedPtr host, Event::Dispatcher& dispatcher) : CodecClient(type, std::move(connection), host, dispatcher), destroy_cb_(destroy_cb) { codec_.reset(codec); + connect(); } ~CodecClientForTest() override { if (destroy_cb_) { diff --git a/test/common/quic/BUILD b/test/common/quic/BUILD index 38d7fa11a252..679a669af537 100644 --- a/test/common/quic/BUILD +++ b/test/common/quic/BUILD @@ -274,6 +274,8 @@ envoy_cc_test_library( external_deps = ["bazel_runfiles"], tags = ["nofips"], deps = [ + "//source/common/quic:envoy_quic_client_connection_lib", + "//source/common/quic:envoy_quic_server_connection_lib", "//source/common/quic:quic_filter_manager_connection_lib", "//test/test_common:environment_lib", "@com_googlesource_quiche//:quic_core_http_spdy_session_lib", diff --git a/test/common/quic/active_quic_listener_test.cc b/test/common/quic/active_quic_listener_test.cc index 6ed403a415fd..d4cfe8a54231 100644 --- a/test/common/quic/active_quic_listener_test.cc +++ b/test/common/quic/active_quic_listener_test.cc @@ -126,7 +126,8 @@ class ActiveQuicListenerTest : public QuicMultiVersionTest { })); listener_factory_ = createQuicListenerFactory(yamlForQuicConfig()); - EXPECT_CALL(listener_config_, filterChainManager()).WillOnce(ReturnRef(filter_chain_manager_)); + EXPECT_CALL(listener_config_, filterChainManager()) + .WillRepeatedly(ReturnRef(filter_chain_manager_)); quic_listener_ = staticUniquePointerCast(listener_factory_->createActiveUdpListener( 0, connection_handler_, *dispatcher_, listener_config_)); @@ -155,9 +156,9 @@ class ActiveQuicListenerTest : public QuicMultiVersionTest { } void maybeConfigureMocks(int connection_count) { - if (quic_version_.UsesTls()) { - return; - } + EXPECT_CALL(filter_chain_manager_, findFilterChain(_)) + .Times(connection_count) + .WillRepeatedly(Return(filter_chain_)); EXPECT_CALL(listener_config_, filterChainFactory()).Times(connection_count); EXPECT_CALL(listener_config_.filter_chain_factory_, createNetworkFilterChain(_, _)) .Times(connection_count) @@ -167,8 +168,10 @@ class ActiveQuicListenerTest : public QuicMultiVersionTest { Server::Configuration::FilterChainUtility::buildFilterChain(connection, filter_factories); return true; })); - EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::Connected)) - .Times(connection_count); + if (!quic_version_.UsesTls()) { + EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::Connected)) + .Times(connection_count); + } EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::LocalClose)) .Times(connection_count); diff --git a/test/common/quic/envoy_quic_dispatcher_test.cc b/test/common/quic/envoy_quic_dispatcher_test.cc index 0a3e6a0ce492..5b4f7b4b76b6 100644 --- a/test/common/quic/envoy_quic_dispatcher_test.cc +++ b/test/common/quic/envoy_quic_dispatcher_test.cc @@ -109,22 +109,7 @@ class EnvoyQuicDispatcherTest : public QuicMultiVersionTest, dispatcher_->run(Event::Dispatcher::RunType::NonBlock); } - void processValidChloPacketAndCheckStatus(bool should_buffer) { - quic::QuicSocketAddress peer_addr(version_ == Network::Address::IpVersion::v4 - ? quic::QuicIpAddress::Loopback4() - : quic::QuicIpAddress::Loopback6(), - 54321); - quic::QuicBufferedPacketStore* buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(&envoy_quic_dispatcher_); - if (!should_buffer) { - // Set QuicDispatcher::new_sessions_allowed_per_event_loop_ to - // |kNumSessionsToCreatePerLoopForTests| so that received CHLOs can be - // processed immediately. - envoy_quic_dispatcher_.ProcessBufferedChlos(kNumSessionsToCreatePerLoopForTests); - EXPECT_FALSE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(buffered_packets->HasBufferedPackets(connection_id_)); - } - + void processValidChloPacket(const quic::QuicSocketAddress& peer_addr) { // Create a Quic Crypto or TLS1.3 CHLO packet. EnvoyQuicClock clock(*dispatcher_); Buffer::OwnedImpl payload = generateChloPacketToSend( @@ -142,7 +127,25 @@ class EnvoyQuicDispatcherTest : public QuicMultiVersionTest, envoy_quic_dispatcher_.ProcessPacket( envoyIpAddressToQuicSocketAddress(listen_socket_->addressProvider().localAddress()->ip()), peer_addr, *received_packet); + } + void processValidChloPacketAndCheckStatus(bool should_buffer) { + quic::QuicSocketAddress peer_addr(version_ == Network::Address::IpVersion::v4 + ? quic::QuicIpAddress::Loopback4() + : quic::QuicIpAddress::Loopback6(), + 54321); + quic::QuicBufferedPacketStore* buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(&envoy_quic_dispatcher_); + if (!should_buffer) { + // Set QuicDispatcher::new_sessions_allowed_per_event_loop_ to + // |kNumSessionsToCreatePerLoopForTests| so that received CHLOs can be + // processed immediately. + envoy_quic_dispatcher_.ProcessBufferedChlos(kNumSessionsToCreatePerLoopForTests); + EXPECT_FALSE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(buffered_packets->HasBufferedPackets(connection_id_)); + } + + processValidChloPacket(peer_addr); if (should_buffer) { // Incoming CHLO packet is buffered, because ProcessPacket() is called before // ProcessBufferedChlos(). @@ -170,6 +173,7 @@ class EnvoyQuicDispatcherTest : public QuicMultiVersionTest, ASSERT(envoy_connection->addressProvider().localAddress() != nullptr); EXPECT_EQ(*listen_socket_->addressProvider().localAddress(), *envoy_connection->addressProvider().localAddress()); + EXPECT_EQ(64 * 1024, envoy_connection->max_inbound_header_list_size()); } void processValidChloPacketAndInitializeFilters(bool should_buffer) { @@ -189,6 +193,23 @@ class EnvoyQuicDispatcherTest : public QuicMultiVersionTest, read_filter->callbacks_->connection().setConnectionStats( {read_total, read_current, write_total, write_current, nullptr, nullptr}); }}); + EXPECT_CALL(listener_config_, filterChainManager()).WillOnce(ReturnRef(filter_chain_manager)); + EXPECT_CALL(filter_chain_manager, findFilterChain(_)) + .WillOnce(Invoke([this](const Network::ConnectionSocket& socket) { + switch (GetParam().second) { + case QuicVersionType::GquicQuicCrypto: + EXPECT_EQ("", socket.requestedApplicationProtocols()[0]); + break; + case QuicVersionType::GquicTls: + EXPECT_EQ("h3-T051", socket.requestedApplicationProtocols()[0]); + break; + case QuicVersionType::Iquic: + EXPECT_EQ("h3-29", socket.requestedApplicationProtocols()[0]); + break; + } + EXPECT_EQ("test.example.org", socket.requestedServerName()); + return &proof_source_->filterChain(); + })); EXPECT_CALL(proof_source_->filterChain(), networkFilterFactories()) .WillOnce(ReturnRef(filter_factory)); EXPECT_CALL(listener_config_, filterChainFactory()); @@ -197,12 +218,17 @@ class EnvoyQuicDispatcherTest : public QuicMultiVersionTest, const std::vector& filter_factories) { EXPECT_EQ(1u, filter_factories.size()); Server::Configuration::FilterChainUtility::buildFilterChain(connection, filter_factories); + dynamic_cast(connection) + .set_max_inbound_header_list_size(64 * 1024); return true; })); EXPECT_CALL(*read_filter, onNewConnection()) // Stop iteration to avoid calling getRead/WriteBuffer(). .WillOnce(Return(Network::FilterStatus::StopIteration)); - EXPECT_CALL(network_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)); + if (!quicVersionUsesTls()) { + // The test utility can't generate 0-RTT packet for Quic TLS handshake yet. + EXPECT_CALL(network_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)); + } processValidChloPacketAndCheckStatus(should_buffer); EXPECT_CALL(network_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); @@ -236,24 +262,62 @@ INSTANTIATE_TEST_SUITE_P(EnvoyQuicDispatcherTests, EnvoyQuicDispatcherTest, testing::ValuesIn(generateTestParam()), testParamsToString); TEST_P(EnvoyQuicDispatcherTest, CreateNewConnectionUponCHLO) { - if (quicVersionUsesTls()) { - // QUICHE doesn't support 0-RTT TLS1.3 handshake yet. - processValidChloPacketAndCheckStatus(false); - // Shutdown() to close the connection. - envoy_quic_dispatcher_.Shutdown(); - return; - } processValidChloPacketAndInitializeFilters(false); } -TEST_P(EnvoyQuicDispatcherTest, CreateNewConnectionUponBufferedCHLO) { - if (quicVersionUsesTls()) { - // QUICHE doesn't support 0-RTT TLS1.3 handshake yet. - processValidChloPacketAndCheckStatus(true); - // Shutdown() to close the connection. - envoy_quic_dispatcher_.Shutdown(); - return; +TEST_P(EnvoyQuicDispatcherTest, CloseConnectionDuringFilterInstallation) { + Network::MockFilterChainManager filter_chain_manager; + std::shared_ptr read_filter(new Network::MockReadFilter()); + Network::MockConnectionCallbacks network_connection_callbacks; + testing::StrictMock read_total; + testing::StrictMock read_current; + testing::StrictMock write_total; + testing::StrictMock write_current; + + std::vector filter_factory( + {[&](Network::FilterManager& filter_manager) { + filter_manager.addReadFilter(read_filter); + read_filter->callbacks_->connection().addConnectionCallbacks(network_connection_callbacks); + read_filter->callbacks_->connection().setConnectionStats( + {read_total, read_current, write_total, write_current, nullptr, nullptr}); + // This will not close connection right away, but after it processes the first packet. + read_filter->callbacks_->connection().close(Network::ConnectionCloseType::NoFlush); + }}); + EXPECT_CALL(listener_config_, filterChainManager()).WillOnce(ReturnRef(filter_chain_manager)); + EXPECT_CALL(filter_chain_manager, findFilterChain(_)) + .WillOnce(Return(&proof_source_->filterChain())); + EXPECT_CALL(proof_source_->filterChain(), networkFilterFactories()) + .WillOnce(ReturnRef(filter_factory)); + EXPECT_CALL(listener_config_, filterChainFactory()); + EXPECT_CALL(listener_config_.filter_chain_factory_, createNetworkFilterChain(_, _)) + .WillOnce(Invoke([](Network::Connection& connection, + const std::vector& filter_factories) { + EXPECT_EQ(1u, filter_factories.size()); + Server::Configuration::FilterChainUtility::buildFilterChain(connection, filter_factories); + return true; + })); + EXPECT_CALL(*read_filter, onNewConnection()) + // Stop iteration to avoid calling getRead/WriteBuffer(). + .WillOnce(Return(Network::FilterStatus::StopIteration)); + + if (!quicVersionUsesTls()) { + EXPECT_CALL(network_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)); } + + EXPECT_CALL(network_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); + quic::QuicSocketAddress peer_addr(version_ == Network::Address::IpVersion::v4 + ? quic::QuicIpAddress::Loopback4() + : quic::QuicIpAddress::Loopback6(), + 54321); + // Set QuicDispatcher::new_sessions_allowed_per_event_loop_ to + // |kNumSessionsToCreatePerLoopForTests| so that received CHLOs can be + // processed immediately. + envoy_quic_dispatcher_.ProcessBufferedChlos(kNumSessionsToCreatePerLoopForTests); + + processValidChloPacket(peer_addr); +} + +TEST_P(EnvoyQuicDispatcherTest, CreateNewConnectionUponBufferedCHLO) { processValidChloPacketAndInitializeFilters(true); } diff --git a/test/common/quic/envoy_quic_proof_source_test.cc b/test/common/quic/envoy_quic_proof_source_test.cc index 898678ff374f..bd41d2991544 100644 --- a/test/common/quic/envoy_quic_proof_source_test.cc +++ b/test/common/quic/envoy_quic_proof_source_test.cc @@ -155,7 +155,7 @@ class EnvoyQuicProofSourceTest : public ::testing::Test { *connection_socket.addressProvider().remoteAddress()); EXPECT_EQ(Extensions::TransportSockets::TransportProtocolNames::get().Quic, connection_socket.detectedTransportProtocol()); - EXPECT_EQ("h2", connection_socket.requestedApplicationProtocols()[0]); + EXPECT_EQ("h3-29", connection_socket.requestedApplicationProtocols()[0]); return &filter_chain_; })); EXPECT_CALL(filter_chain_, transportSocketFactory()) @@ -244,7 +244,7 @@ TEST_F(EnvoyQuicProofSourceTest, GetProofFailNoCertConfig) { *connection_socket.addressProvider().remoteAddress()); EXPECT_EQ(Extensions::TransportSockets::TransportProtocolNames::get().Quic, connection_socket.detectedTransportProtocol()); - EXPECT_EQ("h2", connection_socket.requestedApplicationProtocols()[0]); + EXPECT_EQ("h3-29", connection_socket.requestedApplicationProtocols()[0]); return &filter_chain_; })); EXPECT_CALL(filter_chain_, transportSocketFactory()) diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc index c2c93111a1dd..fe441dbc25cb 100644 --- a/test/common/quic/envoy_quic_server_session_test.cc +++ b/test/common/quic/envoy_quic_server_session_test.cc @@ -55,28 +55,6 @@ using testing::ReturnRef; namespace Envoy { namespace Quic { -class TestEnvoyQuicServerConnection : public EnvoyQuicServerConnection { -public: - TestEnvoyQuicServerConnection(quic::QuicConnectionHelperInterface& helper, - quic::QuicAlarmFactory& alarm_factory, - quic::QuicPacketWriter& writer, - const quic::ParsedQuicVersionVector& supported_versions, - Network::Socket& listen_socket) - : EnvoyQuicServerConnection(quic::test::TestConnectionId(), - quic::QuicSocketAddress(quic::QuicIpAddress::Any4(), 12345), - quic::QuicSocketAddress(quic::QuicIpAddress::Loopback4(), 12345), - helper, alarm_factory, &writer, /*owns_writer=*/false, - supported_versions, listen_socket) {} - - Network::Connection::ConnectionStats& connectionStats() const { - return EnvoyQuicConnection::connectionStats(); - } - - MOCK_METHOD(void, SendConnectionClosePacket, - (quic::QuicErrorCode, quic::QuicIetfTransportErrorCodes, const std::string&)); - MOCK_METHOD(bool, SendControlFrame, (const quic::QuicFrame& frame)); -}; - // Derive to have simpler priority mechanism. class TestEnvoyQuicServerSession : public EnvoyQuicServerSession { public: @@ -154,16 +132,15 @@ class EnvoyQuicServerSessionTest : public testing::TestWithParam { SetQuicReloadableFlag(quic_disable_version_draft_29, !GetParam()); return quic::ParsedVersionOfIndex(quic::CurrentSupportedVersions(), 0); }()), - quic_connection_(new TestEnvoyQuicServerConnection( + quic_connection_(new MockEnvoyQuicServerConnection( connection_helper_, alarm_factory_, writer_, quic_version_, *listener_config_.socket_)), crypto_config_(quic::QuicCryptoServerConfig::TESTING, quic::QuicRandom::GetInstance(), std::make_unique(), quic::KeyExchangeSource::Default()), envoy_quic_session_(quic_config_, quic_version_, - std::unique_ptr(quic_connection_), + std::unique_ptr(quic_connection_), /*visitor=*/nullptr, &crypto_stream_helper_, &crypto_config_, &compressed_certs_cache_, *dispatcher_, - /*send_buffer_limit*/ quic::kDefaultFlowControlSendWindow * 1.5, - listener_config_), + /*send_buffer_limit*/ quic::kDefaultFlowControlSendWindow * 1.5), stats_({ALL_HTTP3_CODEC_STATS( POOL_COUNTER_PREFIX(listener_config_.listenerScope(), "http3."), POOL_GAUGE_PREFIX(listener_config_.listenerScope(), "http3."))}) { @@ -270,7 +247,7 @@ class EnvoyQuicServerSessionTest : public testing::TestWithParam { quic::ParsedQuicVersionVector quic_version_; testing::NiceMock writer_; testing::NiceMock listener_config_; - TestEnvoyQuicServerConnection* quic_connection_; + MockEnvoyQuicServerConnection* quic_connection_; quic::QuicConfig quic_config_; quic::QuicCryptoServerConfig crypto_config_; testing::NiceMock crypto_stream_helper_; @@ -800,29 +777,8 @@ TEST_P(EnvoyQuicServerSessionTest, GoAway) { http_connection_->goAway(); } -TEST_P(EnvoyQuicServerSessionTest, InitializeFilterChain) { - read_filter_ = std::make_shared(); - Network::MockFilterChain filter_chain; - crypto_stream_->setProofSourceDetails( - std::make_unique(filter_chain)); - std::vector filter_factory{[this]( - Network::FilterManager& filter_manager) { - filter_manager.addReadFilter(read_filter_); - read_filter_->callbacks_->connection().addConnectionCallbacks(network_connection_callbacks_); - read_filter_->callbacks_->connection().setConnectionStats( - {read_total_, read_current_, write_total_, write_current_, nullptr, nullptr}); - }}; - EXPECT_CALL(filter_chain, networkFilterFactories()).WillOnce(ReturnRef(filter_factory)); - EXPECT_CALL(*read_filter_, onNewConnection()) - // Stop iteration to avoid calling getRead/WriteBuffer(). - .WillOnce(Return(Network::FilterStatus::StopIteration)); - EXPECT_CALL(listener_config_.filter_chain_factory_, createNetworkFilterChain(_, _)) - .WillOnce(Invoke([](Network::Connection& connection, - const std::vector& filter_factories) { - EXPECT_EQ(1u, filter_factories.size()); - Server::Configuration::FilterChainUtility::buildFilterChain(connection, filter_factories); - return true; - })); +TEST_P(EnvoyQuicServerSessionTest, ConnectedAfterHandshake) { + installReadFilter(); EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::Connected)); if (!quic_version_[0].UsesTls()) { envoy_quic_session_.SetDefaultEncryptionLevel(quic::ENCRYPTION_FORWARD_SECURE); diff --git a/test/common/quic/envoy_quic_server_stream_test.cc b/test/common/quic/envoy_quic_server_stream_test.cc index 3ec95ee9e2c6..d9a5643f6f0a 100644 --- a/test/common/quic/envoy_quic_server_stream_test.cc +++ b/test/common/quic/envoy_quic_server_stream_test.cc @@ -50,11 +50,8 @@ class EnvoyQuicServerStreamTest : public testing::TestWithParam { listener_stats_({ALL_LISTENER_STATS(POOL_COUNTER(listener_config_.listenerScope()), POOL_GAUGE(listener_config_.listenerScope()), POOL_HISTOGRAM(listener_config_.listenerScope()))}), - quic_connection_(quic::test::TestConnectionId(), - quic::QuicSocketAddress(quic::QuicIpAddress::Any6(), 123), - quic::QuicSocketAddress(quic::QuicIpAddress::Any6(), 12345), - connection_helper_, alarm_factory_, &writer_, - /*owns_writer=*/false, {quic_version_}, *listener_config_.socket_), + quic_connection_(connection_helper_, alarm_factory_, writer_, + quic::ParsedQuicVersionVector{quic_version_}, *listener_config_.socket_), quic_session_(quic_config_, {quic_version_}, &quic_connection_, *dispatcher_, quic_config_.GetInitialStreamFlowControlWindowToSend() * 2), stream_id_(VersionUsesHttp3(quic_version_.transport_version) ? 4u : 5u), @@ -111,6 +108,8 @@ class EnvoyQuicServerStreamTest : public testing::TestWithParam { EXPECT_CALL(quic_session_, MaybeSendRstStreamFrame(_, _, _)).Times(testing::AtMost(1u)); EXPECT_CALL(quic_session_, MaybeSendStopSendingFrame(_, quic::QUIC_STREAM_NO_ERROR)) .Times(testing::AtMost(1u)); + EXPECT_CALL(quic_connection_, + SendConnectionClosePacket(_, quic::NO_IETF_QUIC_ERROR, "Closed by application")); quic_session_.close(Network::ConnectionCloseType::NoFlush); } } @@ -166,7 +165,7 @@ class EnvoyQuicServerStreamTest : public testing::TestWithParam { quic::QuicConfig quic_config_; testing::NiceMock listener_config_; Server::ListenerStats listener_stats_; - EnvoyQuicServerConnection quic_connection_; + testing::NiceMock quic_connection_; MockEnvoyQuicSession quic_session_; quic::QuicStreamId stream_id_; Http::Http3::CodecStats stats_; @@ -605,6 +604,8 @@ TEST_P(EnvoyQuicServerStreamTest, ConnectionCloseDuringEncoding) { receiveRequest(request_body_, true, request_body_.size() * 2); quic_stream_->encodeHeaders(response_headers_, /*end_stream=*/false); std::string response(16 * 1024 + 1, 'a'); + EXPECT_CALL(quic_connection_, + SendConnectionClosePacket(_, quic::NO_IETF_QUIC_ERROR, "Closed in WriteHeaders")); EXPECT_CALL(quic_session_, WritevData(_, _, _, _, _, _)) .Times(testing::AtLeast(1u)) .WillRepeatedly( @@ -645,6 +646,8 @@ TEST_P(EnvoyQuicServerStreamTest, ConnectionCloseDuringEncoding) { // onResetStream() callbacks. TEST_P(EnvoyQuicServerStreamTest, ConnectionCloseAfterEndStreamEncoded) { receiveRequest(request_body_, true, request_body_.size() * 2); + EXPECT_CALL(quic_connection_, + SendConnectionClosePacket(_, quic::NO_IETF_QUIC_ERROR, "Closed in WriteHeaders")); EXPECT_CALL(quic_session_, WritevData(_, _, _, _, _, _)) .WillOnce( Invoke([this](quic::QuicStreamId, size_t, quic::QuicStreamOffset, diff --git a/test/common/quic/test_utils.h b/test/common/quic/test_utils.h index 4650eeb9f6dd..0da7ee95ba92 100644 --- a/test/common/quic/test_utils.h +++ b/test/common/quic/test_utils.h @@ -1,5 +1,7 @@ #pragma once +#include "common/quic/envoy_quic_client_connection.h" +#include "common/quic/envoy_quic_server_connection.h" #include "common/quic/quic_filter_manager_connection_impl.h" #if defined(__GNUC__) @@ -28,17 +30,58 @@ namespace Envoy { namespace Quic { +class MockEnvoyQuicServerConnection : public EnvoyQuicServerConnection { +public: + MockEnvoyQuicServerConnection(quic::QuicConnectionHelperInterface& helper, + quic::QuicAlarmFactory& alarm_factory, + quic::QuicPacketWriter& writer, + const quic::ParsedQuicVersionVector& supported_versions, + Network::Socket& listen_socket) + : MockEnvoyQuicServerConnection( + helper, alarm_factory, writer, + quic::QuicSocketAddress(quic::QuicIpAddress::Any4(), 12345), + quic::QuicSocketAddress(quic::QuicIpAddress::Loopback4(), 12345), supported_versions, + listen_socket) {} + + MockEnvoyQuicServerConnection(quic::QuicConnectionHelperInterface& helper, + quic::QuicAlarmFactory& alarm_factory, + quic::QuicPacketWriter& writer, + quic::QuicSocketAddress self_address, + quic::QuicSocketAddress peer_address, + const quic::ParsedQuicVersionVector& supported_versions, + Network::Socket& listen_socket) + : EnvoyQuicServerConnection( + quic::test::TestConnectionId(), self_address, peer_address, helper, alarm_factory, + &writer, /*owns_writer=*/false, supported_versions, + createServerConnectionSocket(listen_socket.ioHandle(), self_address, peer_address, + "example.com", "h3-29")) {} + + Network::Connection::ConnectionStats& connectionStats() const { + return QuicNetworkConnection::connectionStats(); + } + + MOCK_METHOD(void, SendConnectionClosePacket, + (quic::QuicErrorCode, quic::QuicIetfTransportErrorCodes, const std::string&)); + MOCK_METHOD(bool, SendControlFrame, (const quic::QuicFrame& frame)); +}; + class MockEnvoyQuicSession : public quic::QuicSpdySession, public QuicFilterManagerConnectionImpl { public: MockEnvoyQuicSession(const quic::QuicConfig& config, const quic::ParsedQuicVersionVector& supported_versions, - EnvoyQuicConnection* connection, Event::Dispatcher& dispatcher, + EnvoyQuicServerConnection* connection, Event::Dispatcher& dispatcher, uint32_t send_buffer_limit) : quic::QuicSpdySession(connection, /*visitor=*/nullptr, config, supported_versions), - QuicFilterManagerConnectionImpl(*connection, dispatcher, send_buffer_limit) { + QuicFilterManagerConnectionImpl(*connection, connection->connection_id(), dispatcher, + send_buffer_limit) { crypto_stream_ = std::make_unique(this); } + void Initialize() override { + quic::QuicSpdySession::Initialize(); + initialized_ = true; + } + // From QuicSession. MOCK_METHOD(quic::QuicSpdyStream*, CreateIncomingStream, (quic::QuicStreamId id)); MOCK_METHOD(quic::QuicSpdyStream*, CreateIncomingStream, (quic::PendingStream * pending)); @@ -70,6 +113,10 @@ class MockEnvoyQuicSession : public quic::QuicSpdySession, public QuicFilterMana protected: bool hasDataToWrite() override { return HasDataToWrite(); } + const quic::QuicConnection* quicConnection() const override { + return initialized_ ? connection() : nullptr; + } + quic::QuicConnection* quicConnection() override { return initialized_ ? connection() : nullptr; } private: std::unique_ptr crypto_stream_; @@ -80,14 +127,20 @@ class MockEnvoyQuicClientSession : public quic::QuicSpdyClientSession, public: MockEnvoyQuicClientSession(const quic::QuicConfig& config, const quic::ParsedQuicVersionVector& supported_versions, - EnvoyQuicConnection* connection, Event::Dispatcher& dispatcher, + EnvoyQuicClientConnection* connection, Event::Dispatcher& dispatcher, uint32_t send_buffer_limit) : quic::QuicSpdyClientSession(config, supported_versions, connection, quic::QuicServerId("example.com", 443, false), &crypto_config_, nullptr), - QuicFilterManagerConnectionImpl(*connection, dispatcher, send_buffer_limit), + QuicFilterManagerConnectionImpl(*connection, connection->connection_id(), dispatcher, + send_buffer_limit), crypto_config_(quic::test::crypto_test_utils::ProofVerifierForTesting()) {} + void Initialize() override { + quic::QuicSpdyClientSession::Initialize(); + initialized_ = true; + } + // From QuicSession. MOCK_METHOD(quic::QuicSpdyClientStream*, CreateIncomingStream, (quic::QuicStreamId id)); MOCK_METHOD(quic::QuicSpdyClientStream*, CreateIncomingStream, (quic::PendingStream * pending)); @@ -110,6 +163,10 @@ class MockEnvoyQuicClientSession : public quic::QuicSpdyClientSession, protected: bool hasDataToWrite() override { return HasDataToWrite(); } + const quic::QuicConnection* quicConnection() const override { + return initialized_ ? connection() : nullptr; + } + quic::QuicConnection* quicConnection() override { return initialized_ ? connection() : nullptr; } private: quic::QuicCryptoClientConfig crypto_config_; diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index f0a67fe7a6cb..94ab19fd1969 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -589,6 +589,7 @@ void FakeUpstream::threadRoutine() { { absl::MutexLock lock(&lock_); new_connections_.clear(); + quic_connections_.clear(); consumed_connections_.clear(); } } diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index ce92bcb06f5e..04bccb733b66 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -805,12 +805,12 @@ class FakeUpstream : Logger::Loggable, Event::DispatcherPtr dispatcher_; Network::ConnectionHandlerPtr handler_; std::list new_connections_ ABSL_GUARDED_BY(lock_); - std::list quic_connections_ ABSL_GUARDED_BY(lock_); // When a QueuedConnectionWrapper is popped from new_connections_, ownership is transferred to // consumed_connections_. This allows later the Connection destruction (when the FakeUpstream is // deleted) on the same thread that allocated the connection. std::list consumed_connections_ ABSL_GUARDED_BY(lock_); + std::list quic_connections_ ABSL_GUARDED_BY(lock_); const FakeUpstreamConfig config_; bool read_disable_on_new_connection_; const bool enable_half_close_; diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 6228dd40b0a0..03cd14677936 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -771,11 +771,6 @@ TEST_P(DownstreamProtocolIntegrationTest, RetryAttemptCountHeader) { // The retry priority will always target P1, which would otherwise never be hit due to P0 being // healthy. TEST_P(DownstreamProtocolIntegrationTest, RetryPriority) { - if (upstreamProtocol() == FakeHttpConnection::Type::HTTP2 && - downstreamProtocol() == Http::CodecClient::Type::HTTP3) { - // TODO(alyssawilk) investigate why this combination doesn't work. - return; - } EXCLUDE_UPSTREAM_HTTP3; // Timed out waiting for new stream. const Upstream::HealthyLoad healthy_priority_load({0u, 100u}); const Upstream::DegradedLoad degraded_priority_load({0u, 100u}); @@ -1325,9 +1320,18 @@ TEST_P(ProtocolIntegrationTest, MissingStatus) { // Validate that lots of tiny cookies doesn't cause a DoS (single cookie header). TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { - // TODO(danzh) re-enable this test after quic headers size become configurable. - EXCLUDE_DOWNSTREAM_HTTP3 - EXCLUDE_UPSTREAM_HTTP3; // QUIC_STREAM_EXCESSIVE_LOAD + if (downstreamProtocol() == Http::CodecClient::Type::HTTP3) { + // QUICHE Qpack splits concatenated cookies into crumbs to increase + // compression ratio. On the receiver side, the total size of these crumbs + // may be larger than coalesced cookie headers. Increase the max request + // header size to avoid QUIC_HEADERS_TOO_LARGE stream error. + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) -> void { hcm.mutable_max_request_headers_kb()->set_value(96); }); + } + if (upstreamProtocol() == FakeHttpConnection::Type::HTTP3) { + setMaxRequestHeadersKb(96); + } initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -1350,9 +1354,6 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) { // Validate that lots of tiny cookies doesn't cause a DoS (many cookie headers). TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingMany) { - // TODO(danzh) re-enable this test after quic headers size become configurable. - EXCLUDE_DOWNSTREAM_HTTP3 - EXCLUDE_UPSTREAM_HTTP3; // QUIC_STREAM_EXCESSIVE_LOAD // Set header count limit to 2010. uint32_t max_count = 2010; config_helper_.addConfigModifier( @@ -1561,9 +1562,6 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlRejected) { } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlAccepted) { - // TODO(danzh) re-enable this test after quic headers size become configurable. - EXCLUDE_DOWNSTREAM_HTTP3 - EXCLUDE_UPSTREAM_HTTP3; // requires configurable header limits. // Send one 95 kB URL with limit 96 kB headers. testLargeRequestUrl(95, 96); } @@ -1574,22 +1572,22 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersRejected) { } TEST_P(DownstreamProtocolIntegrationTest, VeryLargeRequestHeadersRejected) { - EXCLUDE_DOWNSTREAM_HTTP3 + EXCLUDE_DOWNSTREAM_HTTP3; EXCLUDE_UPSTREAM_HTTP3; // Send one very large 2048 kB (2 MB) header with limit 1024 kB (1 MB) and 100 headers. testLargeRequestHeaders(2048, 1, 1024, 100); } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersAccepted) { - EXCLUDE_DOWNSTREAM_HTTP3 - EXCLUDE_UPSTREAM_HTTP3; // requires configurable header limits. // Send one 100 kB header with limit 8192 kB (8 MB) and 100 headers. testLargeRequestHeaders(100, 1, 8192, 100); } TEST_P(DownstreamProtocolIntegrationTest, ManyLargeRequestHeadersAccepted) { - EXCLUDE_DOWNSTREAM_HTTP3 - EXCLUDE_UPSTREAM_HTTP3; + // Fail under TSAN. Quic blackhole detection fired and closed the connection with + // QUIC_TOO_MANY_RTOS while waiting for upstream finishing transferring the large header. Observed + // long event loop. + EXCLUDE_DOWNSTREAM_HTTP3; // Send 70 headers each of size 100 kB with limit 8192 kB (8 MB) and 100 headers. testLargeRequestHeaders(100, 70, 8192, 100); } @@ -1609,8 +1607,7 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersAccepted) { TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersRejected) { // QUICHE doesn't limit number of headers. EXCLUDE_DOWNSTREAM_HTTP3 - EXCLUDE_UPSTREAM_HTTP3; // CI asan use-after-free - // Default header (and trailer) count limit is 100. + // The default configured header (and trailer) count limit is 100. config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); config_helper_.addConfigModifier(setEnableUpstreamTrailersHttp1()); Http::TestRequestTrailerMapImpl request_trailers; @@ -1637,7 +1634,6 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersRejected) { } TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersAccepted) { - EXCLUDE_UPSTREAM_HTTP3; // assert failure: validHeaderString // Set header (and trailer) count limit to 200. uint32_t max_count = 200; config_helper_.addConfigModifier( @@ -1670,25 +1666,16 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersAccepted) { // This test uses an Http::HeaderMapImpl instead of an Http::TestHeaderMapImpl to avoid // time-consuming byte size validations that will cause this test to timeout. TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersTimeout) { - // TODO(danzh) re-enable this test after quic headers size become configurable. - EXCLUDE_DOWNSTREAM_HTTP3 - EXCLUDE_UPSTREAM_HTTP3; // Set timeout for 5 seconds, and ensure that a request with 10k+ headers can be sent. testManyRequestHeaders(std::chrono::milliseconds(5000)); } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersAccepted) { - // TODO(danzh) re-enable this test after quic headers size become configurable. - EXCLUDE_DOWNSTREAM_HTTP3 - EXCLUDE_UPSTREAM_HTTP3; config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); testLargeRequestTrailers(60, 96); } TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersRejected) { - // TODO(danzh) investigate why it failed for H3. - EXCLUDE_DOWNSTREAM_HTTP3 - EXCLUDE_UPSTREAM_HTTP3; config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); testLargeRequestTrailers(66, 60); } @@ -1696,10 +1683,6 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersRejected) { // This test uses an Http::HeaderMapImpl instead of an Http::TestHeaderMapImpl to avoid // time-consuming byte size verification that will cause this test to timeout. TEST_P(DownstreamProtocolIntegrationTest, ManyTrailerHeaders) { - // Enable after setting QUICHE max_inbound_header_list_size_ from HCM - // config. - EXCLUDE_DOWNSTREAM_HTTP3 - EXCLUDE_UPSTREAM_HTTP3; setMaxRequestHeadersKb(96); setMaxRequestHeadersCount(20005); diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc index d1eb590cd64e..5aeef19daf49 100644 --- a/test/integration/quic_http_integration_test.cc +++ b/test/integration/quic_http_integration_test.cc @@ -123,7 +123,6 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers quic_config_, supported_versions_, std::move(connection), persistent_info.server_id_, persistent_info.crypto_config_.get(), &push_promise_index_, *dispatcher_, /*send_buffer_limit=*/2 * Http2::Utility::OptionsLimits::MIN_INITIAL_STREAM_WINDOW_SIZE); - session->Initialize(); return session; } diff --git a/test/integration/quic_protocol_integration_test.cc b/test/integration/quic_protocol_integration_test.cc index 4d94b6783cad..197eb3761c7f 100644 --- a/test/integration/quic_protocol_integration_test.cc +++ b/test/integration/quic_protocol_integration_test.cc @@ -2,13 +2,13 @@ namespace Envoy { -// These will run with HTTP/3 downstream, and Http and HTTP/2 upstream. -INSTANTIATE_TEST_SUITE_P(Protocols, DownstreamProtocolIntegrationTest, +// These will run with HTTP/3 downstream, and Http upstream. +INSTANTIATE_TEST_SUITE_P(DownstreamProtocols, DownstreamProtocolIntegrationTest, testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( - {Http::CodecClient::Type::HTTP3}, - {FakeHttpConnection::Type::HTTP1, FakeHttpConnection::Type::HTTP2})), + {Http::CodecClient::Type::HTTP3}, {FakeHttpConnection::Type::HTTP1})), HttpProtocolIntegrationTest::protocolTestParamsToString); +// These will run with HTTP/3 downstream, and Http and HTTP/2 upstream. INSTANTIATE_TEST_SUITE_P(DownstreamProtocols, ProtocolIntegrationTest, testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( {Http::CodecClient::Type::HTTP3}, diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 52b0f16228c8..16b5fac73870 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -273,6 +273,7 @@ RPC RSA RST RTDS +RTOS RTTI RUNDIR RW