Skip to content

Commit

Permalink
quiche: use max header size configured in HCM (envoyproxy#15912)
Browse files Browse the repository at this point in the history
Commit Message: use max header size in HCM config to initialize quic session.
Additional Description:
change QuicSession::Initialize() and filter chain creation order to set max header list size for each session before Initialize(). Move Initialize() of Http3 connection from CodecClient to CodecClientProd.
Added CodecClient::connect() interface.
Change EnvoyQuicConnection not to directly inherit from QuicConnection. And renamed it.
Fix a use-after-free bug in FakeUpstream which causes ASAN failure.

Risk Level: low
Testing: more protocol H3 tests

Part of envoyproxy#2557 envoyproxy#12930 envoyproxy#14829

Signed-off-by: Dan Zhang <[email protected]>
Signed-off-by: Gokul Nair <[email protected]>
  • Loading branch information
danzh2010 authored and Gokul Nair committed May 6, 2021
1 parent 14b1705 commit f68f942
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 53 deletions.
2 changes: 1 addition & 1 deletion source/common/http/codec_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne
auto& quic_session = dynamic_cast<Quic::EnvoyQuicClientSession&>(*connection_);
codec_ = std::make_unique<Quic::QuicHttpClientConnectionImpl>(
quic_session, *this, host->cluster().http3CodecStats(), host->cluster().http3Options(),
Http::DEFAULT_MAX_REQUEST_HEADERS_KB, host->cluster().maxResponseHeadersCount());
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();
Expand Down
1 change: 0 additions & 1 deletion source/common/quic/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,6 @@ envoy_cc_library(
"//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_config_lib",
"@com_googlesource_quiche//:quic_core_http_header_list_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
"@envoy_api//envoy/config/listener/v3:pkg_cc_proto",
Expand Down
9 changes: 4 additions & 5 deletions source/common/quic/client_connection_factory_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@ PersistentQuicInfoImpl::PersistentQuicInfoImpl(
: conn_helper_(dispatcher), alarm_factory_(dispatcher, *conn_helper_.GetClock()),
server_id_{getConfig(transport_socket_factory).serverNameIndication(),
static_cast<uint16_t>(server_addr->ip()->port()), false},
crypto_config_(std::make_unique<quic::QuicCryptoClientConfig>(
std::make_unique<EnvoyQuicProofVerifier>(stats_scope, getConfig(transport_socket_factory),
time_source),
std::make_unique<EnvoyQuicSessionCache>(time_source))) {
crypto_config_(
std::make_unique<quic::QuicCryptoClientConfig>(std::make_unique<EnvoyQuicProofVerifier>(
stats_scope, getConfig(transport_socket_factory), time_source))) {
quiche::FlagRegistry::getInstance();
}

Expand Down Expand Up @@ -60,7 +59,7 @@ createQuicNetworkConnection(Http::PersistentQuicInfo& info, Event::Dispatcher& d
auto ret = std::make_unique<EnvoyQuicClientSession>(
info_impl->quic_config_, info_impl->supported_versions_, std::move(connection),
info_impl->server_id_, info_impl->crypto_config_.get(), &static_info.push_promise_index_,
dispatcher, /*send_buffer_limit=*/0);
dispatcher, 0);
return ret;
}

Expand Down
6 changes: 2 additions & 4 deletions source/common/quic/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,14 @@ 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_count,
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) {
quic_session.setCodecStats(stats);
quic_session.setHttp3Options(http3_options);
quic_session.setHeadersWithUnderscoreAction(headers_with_underscores_action);
quic_session.setHttpConnectionCallbacks(callbacks);
quic_session.setMaxIncomingHeadersCount(max_request_headers_count);
quic_session.set_max_inbound_header_list_size(max_request_headers_kb * 1024u);
}

Expand Down Expand Up @@ -70,12 +69,11 @@ 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_response_headers_count)
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.setMaxIncomingHeadersCount(max_response_headers_count);
session.set_max_inbound_header_list_size(max_request_headers_kb * 1024);
}

Expand Down
6 changes: 6 additions & 0 deletions source/common/quic/envoy_quic_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ std::unique_ptr<quic::QuicSession> EnvoyQuicDispatcher::CreateQuicSession(
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 =
Expand Down
26 changes: 0 additions & 26 deletions source/common/quic/envoy_quic_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,31 +244,5 @@ createServerConnectionSocket(Network::IoHandle& io_handle,
return connection_socket;
}

void configQuicInitialFlowControlWindow(const envoy::config::core::v3::QuicProtocolOptions& config,
quic::QuicConfig& quic_config) {
size_t stream_flow_control_window_to_send = PROTOBUF_GET_WRAPPED_OR_DEFAULT(
config, initial_stream_window_size,
Http3::Utility::OptionsLimits::DEFAULT_INITIAL_STREAM_WINDOW_SIZE);
if (stream_flow_control_window_to_send < quic::kMinimumFlowControlSendWindow) {
// If the configured value is smaller than 16kB, only use it for IETF QUIC, because Google QUIC
// requires minimum 16kB stream flow control window. The QUICHE default 16kB will be used for
// Google QUIC connections.
quic_config.SetInitialMaxStreamDataBytesIncomingBidirectionalToSend(
stream_flow_control_window_to_send);
} else {
// Both Google QUIC and IETF Quic can be configured from this.
quic_config.SetInitialStreamFlowControlWindowToSend(stream_flow_control_window_to_send);
}

uint32_t session_flow_control_window_to_send = PROTOBUF_GET_WRAPPED_OR_DEFAULT(
config, initial_connection_window_size,
Http3::Utility::OptionsLimits::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE);
// Config connection level flow control window shouldn't be smaller than the minimum flow control
// window supported in QUICHE which is 16kB.
quic_config.SetInitialSessionFlowControlWindowToSend(
std::max(quic::kMinimumFlowControlSendWindow,
static_cast<quic::QuicByteCount>(session_flow_control_window_to_send)));
}

} // namespace Quic
} // namespace Envoy
4 changes: 0 additions & 4 deletions source/common/quic/envoy_quic_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,5 @@ createServerConnectionSocket(Network::IoHandle& io_handle,
const quic::QuicSocketAddress& peer_address,
const std::string& hostname, absl::string_view alpn);

// Set initial flow control windows in quic_config according to the given Envoy config.
void configQuicInitialFlowControlWindow(const envoy::config::core::v3::QuicProtocolOptions& config,
quic::QuicConfig& quic_config);

} // namespace Quic
} // namespace Envoy
19 changes: 7 additions & 12 deletions test/integration/protocol_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,7 @@ 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) {
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});
NiceMock<Upstream::MockRetryPriority> retry_priority(healthy_priority_load,
Expand Down Expand Up @@ -1321,14 +1322,10 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) {
// 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);
hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(8000);
});
hcm) -> void { hcm.mutable_max_request_headers_kb()->set_value(96); });
}
if (upstreamProtocol() == FakeHttpConnection::Type::HTTP3) {
setMaxRequestHeadersKb(96);
setMaxRequestHeadersCount(8000);
}
initialize();

Expand Down Expand Up @@ -1570,12 +1567,8 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersRejected) {
}

TEST_P(DownstreamProtocolIntegrationTest, VeryLargeRequestHeadersRejected) {
#ifdef WIN32
// TODO(alyssawilk, wrowe) debug.
if (upstreamProtocol() == FakeHttpConnection::Type::HTTP3) {
return;
}
#endif
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);
}
Expand Down Expand Up @@ -1605,7 +1598,9 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersAccepted) {
}

TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersRejected) {
// Default header (and trailer) count limit is 100.
// QUICHE doesn't limit number of headers.
EXCLUDE_DOWNSTREAM_HTTP3
// The default configured header (and trailer) count limit is 100.
config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1());
config_helper_.addConfigModifier(setEnableUpstreamTrailersHttp1());
Http::TestRequestTrailerMapImpl request_trailers;
Expand Down

0 comments on commit f68f942

Please sign in to comment.