diff --git a/bazel/external/quiche.patch b/bazel/external/quiche.patch new file mode 100644 index 0000000000..83ed96bb74 --- /dev/null +++ b/bazel/external/quiche.patch @@ -0,0 +1,226 @@ +commit 6c726cca04b68c5593fbded31fadb001771844ef +Author: danzh +Date: Wed Jul 9 08:28:21 2025 -0700 + + Patch a fix from https://github.com/villainb-dg for the issue of leaking connection flow control window when a HTTP2 stream receives a DATA frame after being closed (reset, etc). + + When a stream receives a DATA frame while it is already closed, the data is counted against the connection flow control window, but is never marked as consumed. + Fix: Mark the data as consumed when the received DATA frame is on a reset or invalid stream. + + Reported QUICHE issue https://github.com/google/quiche/issues/91 + Reported Envoy issue https://github.com/envoyproxy/envoy/issues/40085 + Proposed external fix: https://github.com/google/quiche/pull/92 + + PiperOrigin-RevId: 781064590 + +diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc +index c25b2917b..9b1afb22b 100644 +--- a/quiche/http2/adapter/oghttp2_session.cc ++++ b/quiche/http2/adapter/oghttp2_session.cc +@@ -1152,11 +1152,15 @@ void OgHttp2Session::OnDataFrameHeader(spdy::SpdyStreamId stream_id, + + void OgHttp2Session::OnStreamFrameData(spdy::SpdyStreamId stream_id, + const char* data, size_t len) { +- // Count the data against flow control, even if the stream is unknown. ++ // Count the data against flow control, even if the stream is unknown, so that ++ // the connection flow control window is in sync with peer's. + MarkDataBuffered(stream_id, len); + + auto iter = stream_map_.find(stream_id); + if (iter == stream_map_.end()) { ++ // Mark the data consumed immediately as we are dropping them. This will ++ // allow the connection flow control window to shift. ++ Consume(stream_id, len); + return; + } + // Validate against the content-length if it exists. +@@ -1171,6 +1175,9 @@ void OgHttp2Session::OnStreamFrameData(spdy::SpdyStreamId stream_id, + if (streams_reset_.contains(stream_id)) { + // If the stream was unknown due to a protocol error, the visitor was + // informed in OnDataFrameHeader(). ++ // Mark the data consumed immediately as we are dropping them. This will ++ // allow the connection flow control window to shift. ++ Consume(stream_id, len); + return; + } + +diff --git a/quiche/http2/adapter/oghttp2_session_test.cc b/quiche/http2/adapter/oghttp2_session_test.cc +index 34a387bec..4afcf1a3d 100644 +--- a/quiche/http2/adapter/oghttp2_session_test.cc ++++ b/quiche/http2/adapter/oghttp2_session_test.cc +@@ -1217,6 +1217,175 @@ TEST(OgHttp2SessionTest, ServerClosesStreamDuringOnEndStream) { + EXPECT_EQ(result, frames.size()); + } + ++TEST(OgHttp2SessionTest, ResetStreamRaceWithIncomingData) { ++ TestVisitor visitor; ++ OgHttp2Session::Options options; ++ options.perspective = Perspective::kServer; ++ OgHttp2Session session(visitor, options); ++ ++ const std::string frames = TestFrameSequence() ++ .ClientPreface() ++ .Headers(1, ++ {{":method", "POST"}, ++ {":scheme", "https"}, ++ {":authority", "example.com"}, ++ {":path", "/"}}, ++ /*fin=*/false) ++ .Data(1, "Request body", false) ++ .Serialize(); ++ testing::InSequence s; ++ ++ // Client preface (empty SETTINGS) ++ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); ++ EXPECT_CALL(visitor, OnSettingsStart()); ++ EXPECT_CALL(visitor, OnSettingsEnd()); ++ // Stream 1 ++ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 0x4)); ++ EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); ++ EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "POST")); ++ EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https")); ++ EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com")); ++ EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/")); ++ EXPECT_CALL(visitor, OnEndHeadersForStream(1)); ++ ++ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0x0)); ++ EXPECT_CALL(visitor, OnBeginDataForStream(1, _)); ++ EXPECT_CALL(visitor, OnDataForStream(1, "Request body")) ++ .WillOnce(testing::InvokeWithoutArgs([&session]() { ++ session.Consume(1, 12); ++ return true; ++ })); ++ ++ session.ProcessBytes(frames); ++ ++ EXPECT_TRUE(session.want_write()); ++ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); ++ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); ++ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1)); ++ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0)); ++ int result1 = session.Send(); ++ EXPECT_EQ(0, result1); ++ absl::string_view serialized1 = visitor.data(); ++ EXPECT_THAT(serialized1, ++ EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS})); ++ EXPECT_FALSE(session.want_write()); ++ ++ EXPECT_LT(session.GetReceiveWindowSize(), kInitialFlowControlWindowSize); ++ ++ // Reset the stream and receive more data on this stream. ++ session.EnqueueFrame(std::make_unique( ++ 1, spdy::ERROR_CODE_PROTOCOL_ERROR)); ++ const std::string more_frames = ++ TestFrameSequence() ++ .Data(1, std::string(16 * 1024, 'x'), false) ++ .Data(1, std::string(16 * 1024, 'y'), false) ++ .Serialize(); ++ // These bytes are counted against the connection flow control window but ++ // should be dropped right away and considerred as consumed. ++ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, _)).Times(0); ++ EXPECT_CALL(visitor, OnBeginDataForStream(1, _)).Times(0); ++ EXPECT_CALL(visitor, OnDataForStream(1, _)).Times(0); ++ ++ session.ProcessBytes(more_frames); ++ EXPECT_TRUE(session.want_write()); ++ ++ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, _, 0x0)); ++ EXPECT_CALL(visitor, OnFrameSent(RST_STREAM, 1, _, 0x0, 1)); ++ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR)); ++ EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 0, _, 0x0)); ++ EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 0, _, 0x0, 0)); ++ int result2 = session.Send(); ++ EXPECT_EQ(0, result2); ++ absl::string_view serialized2 = visitor.data(); ++ serialized2.remove_prefix(serialized1.size()); ++ EXPECT_THAT(serialized2, EqualsFrames({SpdyFrameType::RST_STREAM, ++ SpdyFrameType::WINDOW_UPDATE})); ++ EXPECT_EQ(session.GetReceiveWindowSize(), kInitialFlowControlWindowSize); ++} ++ ++TEST(OgHttp2SessionTest, ResetAndCloseStreamRaceWithIncomingData) { ++ TestVisitor visitor; ++ OgHttp2Session::Options options; ++ options.perspective = Perspective::kServer; ++ OgHttp2Session session(visitor, options); ++ ++ const std::string frames = TestFrameSequence() ++ .ClientPreface() ++ .Headers(1, ++ {{":method", "POST"}, ++ {":scheme", "https"}, ++ {":authority", "example.com"}, ++ {":path", "/"}}, ++ /*fin=*/false) ++ .Data(1, "Request body", false) ++ .Serialize(); ++ testing::InSequence s; ++ ++ // Client preface (empty SETTINGS) ++ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); ++ EXPECT_CALL(visitor, OnSettingsStart()); ++ EXPECT_CALL(visitor, OnSettingsEnd()); ++ // Stream 1 ++ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 0x4)); ++ EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); ++ EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "POST")); ++ EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https")); ++ EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com")); ++ EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/")); ++ EXPECT_CALL(visitor, OnEndHeadersForStream(1)); ++ ++ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0x0)); ++ EXPECT_CALL(visitor, OnBeginDataForStream(1, _)); ++ EXPECT_CALL(visitor, OnDataForStream(1, "Request body")) ++ .WillOnce(testing::InvokeWithoutArgs([&session]() { ++ session.Consume(1, 12); ++ return true; ++ })); ++ ++ session.ProcessBytes(frames); ++ ++ EXPECT_TRUE(session.want_write()); ++ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); ++ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); ++ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1)); ++ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0)); ++ int result1 = session.Send(); ++ EXPECT_EQ(0, result1); ++ absl::string_view serialized1 = visitor.data(); ++ EXPECT_THAT(serialized1, ++ EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS})); ++ EXPECT_FALSE(session.want_write()); ++ ++ EXPECT_LT(session.GetReceiveWindowSize(), kInitialFlowControlWindowSize); ++ ++ // Reset the stream and receive more data on this stream. ++ session.EnqueueFrame(std::make_unique( ++ 1, spdy::ERROR_CODE_PROTOCOL_ERROR)); ++ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, _, 0x0)); ++ EXPECT_CALL(visitor, OnFrameSent(RST_STREAM, 1, _, 0x0, 1)); ++ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR)); ++ EXPECT_EQ(0, session.Send()); ++ ++ const std::string more_frames = ++ TestFrameSequence() ++ .Data(1, std::string(16 * 1024, 'x'), false) ++ .Data(1, std::string(16 * 1024, 'y'), false) ++ .Serialize(); ++ // These bytes are counted against the connection flow control window but ++ // should be dropped right away and considered as consumed. ++ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, _)).Times(2); ++ EXPECT_CALL(visitor, OnBeginDataForStream(1, _)).Times(0); ++ EXPECT_CALL(visitor, OnDataForStream(1, _)).Times(0); ++ ++ session.ProcessBytes(more_frames); ++ EXPECT_TRUE(session.want_write()); ++ ++ EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 0, _, 0x0)); ++ EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 0, _, 0x0, 0)); ++ EXPECT_EQ(0, session.Send()); ++ EXPECT_EQ(session.GetReceiveWindowSize(), kInitialFlowControlWindowSize); ++} ++ + } // namespace test + } // namespace adapter + } // namespace http2 diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 856679d07b..0445a0eb03 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -8,19 +8,16 @@ minor_behavior_changes: bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* -- area: conn_pool +- area: tls change: | - Fixed an issue that could lead to too many connections when using - :ref:`AutoHttpConfig ` if the - established connection is ``http/2`` and Envoy predicted it would have lower concurrent capacity. -- area: conn_pool + Fixed an issue with incorrectly cached connection properties on TLS connections. + If TLS connection data was queried before it was available, an empty value was being incorrectly cached, preventing later calls from + getting the correct value. This could be triggered with a ``tcp_proxy`` access log configured to emit a log upon connection + establishment if the log contains fields of the the TLS peer certificate. Then a later use of the data, such as the network RBAC + filter validating a peer certificate SAN, may incorrectly fail due to the empty cached value. +- area: http2 change: | - Fixed an issue that could lead to insufficient connections for current pending requests. If a connection starts draining while it - has negative unused capacity (which happens if an HTTP/2 ``SETTINGS`` frame reduces allowed concurrency to below the current number - of requests), that connection's unused capacity will be included in total pool capacity even though it is unusable because it is - draining. This can result in not enough connections being established for current pending requests. This is most problematic for - long-lived requests (such as streaming gRPC requests or long-poll requests) because a connection could be in the draining state - for a long time. + Fixed an issue where http/2 connections using the default codec of ``oghttp2`` could get stuck due to a window buffer leak. removed_config_or_runtime: # *Normally occurs at the end of the* :ref:`deprecation period ` diff --git a/source/common/quic/envoy_quic_server_stream.cc b/source/common/quic/envoy_quic_server_stream.cc index f4f8f9b161..4cbf43a12c 100644 --- a/source/common/quic/envoy_quic_server_stream.cc +++ b/source/common/quic/envoy_quic_server_stream.cc @@ -44,6 +44,10 @@ EnvoyQuicServerStream::EnvoyQuicServerStream( stats_gatherer_ = new QuicStatsGatherer(&filterManagerConnection()->dispatcher().timeSource()); set_ack_listener(stats_gatherer_); RegisterMetadataVisitor(this); + if (Runtime::runtimeFeatureEnabled("envoy.restart_features.validate_http3_pseudo_headers") && + session->allow_extended_connect()) { + header_validator().SetAllowExtendedConnect(); + } } void EnvoyQuicServerStream::encode1xxHeaders(const Http::ResponseHeaderMap& headers) { diff --git a/source/common/quic/envoy_quic_stream.h b/source/common/quic/envoy_quic_stream.h index 34deca6a28..6b54da2c8f 100644 --- a/source/common/quic/envoy_quic_stream.h +++ b/source/common/quic/envoy_quic_stream.h @@ -143,6 +143,29 @@ class EnvoyQuicStream : public virtual Http::StreamEncoder, return Http::HeaderUtility::HeaderValidationResult::ACCEPT; } + void startHeaderBlock() override { + if (!Runtime::runtimeFeatureEnabled("envoy.restart_features.validate_http3_pseudo_headers")) { + return; + } + header_validator_.StartHeaderBlock(); + } + + bool finishHeaderBlock(bool is_trailing_headers) override { + if (!Runtime::runtimeFeatureEnabled("envoy.restart_features.validate_http3_pseudo_headers")) { + return true; + } + if (is_trailing_headers) { + return header_validator_.FinishHeaderBlock(quic_session_.perspective() == + quic::Perspective::IS_CLIENT + ? http2::adapter::HeaderType::RESPONSE_TRAILER + : http2::adapter::HeaderType::REQUEST_TRAILER); + } + return header_validator_.FinishHeaderBlock(quic_session_.perspective() == + quic::Perspective::IS_CLIENT + ? http2::adapter::HeaderType::RESPONSE + : http2::adapter::HeaderType::REQUEST); + } + absl::string_view responseDetails() override { return details_; } const StreamInfo::BytesMeterSharedPtr& bytesMeter() override { return bytes_meter_; } @@ -191,6 +214,8 @@ class EnvoyQuicStream : public virtual Http::StreamEncoder, return received_metadata_bytes_ > 1 << 20; } + http2::adapter::HeaderValidator& header_validator() { return header_validator_; } + #ifdef ENVOY_ENABLE_HTTP_DATAGRAMS // Setting |http_datagram_handler_| enables HTTP Datagram support. std::unique_ptr http_datagram_handler_; diff --git a/source/common/quic/envoy_quic_utils.h b/source/common/quic/envoy_quic_utils.h index 7ded20c854..9431d89143 100644 --- a/source/common/quic/envoy_quic_utils.h +++ b/source/common/quic/envoy_quic_utils.h @@ -57,8 +57,12 @@ quic::QuicSocketAddress envoyIpAddressToQuicSocketAddress(const Network::Address class HeaderValidator { public: virtual ~HeaderValidator() = default; + virtual void startHeaderBlock() = 0; virtual Http::HeaderUtility::HeaderValidationResult validateHeader(absl::string_view name, absl::string_view header_value) = 0; + // Returns true if all required pseudo-headers and no extra pseudo-headers are + // present for the given header type. + virtual bool finishHeaderBlock(bool is_trailing_headers) = 0; }; // The returned header map has all keys in lower case. @@ -67,6 +71,7 @@ std::unique_ptr quicHeadersToEnvoyHeaders(const quic::QuicHeaderList& header_list, HeaderValidator& validator, uint32_t max_headers_kb, uint32_t max_headers_allowed, absl::string_view& details, quic::QuicRstStreamErrorCode& rst) { + validator.startHeaderBlock(); auto headers = T::create(max_headers_kb, max_headers_allowed); for (const auto& entry : header_list) { if (max_headers_allowed == 0) { @@ -96,6 +101,9 @@ quicHeadersToEnvoyHeaders(const quic::QuicHeaderList& header_list, HeaderValidat } } } + if (!validator.finishHeaderBlock(/*is_trailing_headers=*/false)) { + return nullptr; + } return headers; } @@ -111,6 +119,7 @@ http2HeaderBlockToEnvoyTrailers(const quiche::HttpHeaderBlock& header_block, rst = quic::QUIC_STREAM_EXCESSIVE_LOAD; return nullptr; } + validator.startHeaderBlock(); for (auto entry : header_block) { // TODO(danzh): Avoid temporary strings and addCopy() with string_view. std::string key(entry.first); @@ -136,6 +145,9 @@ http2HeaderBlockToEnvoyTrailers(const quiche::HttpHeaderBlock& header_block, } } } + if (!validator.finishHeaderBlock(/*is_trailing_headers=*/true)) { + return nullptr; + } return headers; } diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index f20fb9b25a..8b4084ead7 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -117,6 +117,7 @@ RUNTIME_GUARD(envoy_restart_features_skip_backing_cluster_check_for_sds); RUNTIME_GUARD(envoy_restart_features_use_eds_cache_for_ads); RUNTIME_GUARD(envoy_restart_features_use_fast_protobuf_hash); RUNTIME_GUARD(envoy_reloadable_features_enable_intermediate_ca); +RUNTIME_GUARD(envoy_restart_features_validate_http3_pseudo_headers); // Begin false flags. Most of them should come with a TODO to flip true. diff --git a/source/common/tls/connection_info_impl_base.cc b/source/common/tls/connection_info_impl_base.cc index e46a313e05..244f01b7fe 100644 --- a/source/common/tls/connection_info_impl_base.cc +++ b/source/common/tls/connection_info_impl_base.cc @@ -17,6 +17,16 @@ namespace Extensions { namespace TransportSockets { namespace Tls { +namespace { +// There must be an version of this function for each type possible in variant `CachedValue`. +bool shouldRecalculateCachedEntry(const std::string& str) { return str.empty(); } +bool shouldRecalculateCachedEntry(const std::vector& vec) { return vec.empty(); } +bool shouldRecalculateCachedEntry(const Ssl::ParsedX509NamePtr& ptr) { return ptr == nullptr; } +bool shouldRecalculateCachedEntry(const bssl::UniquePtr& ptr) { + return ptr == nullptr; +} +} // namespace + template const ValueType& ConnectionInfoImplBase::getCachedValueOrCreate(CachedValueTag tag, @@ -26,6 +36,16 @@ ConnectionInfoImplBase::getCachedValueOrCreate(CachedValueTag tag, const ValueType* val = absl::get_if(&it->second); ASSERT(val != nullptr, "Incorrect type in variant"); if (val != nullptr) { + + // Some values are retrieved too early, for example if properties of a peer certificate are + // retrieved before the handshake is complete, an empty value is cached. The value must be + // in the cache, so that we can return a valid reference, but in those cases if another caller + // later retrieves the same value, we must recalculate the value. + if (shouldRecalculateCachedEntry(*val)) { + it->second = create(ssl()); + val = &absl::get(it->second); + } + return *val; } } diff --git a/source/extensions/network/dns_resolver/apple/apple_dns_impl.cc b/source/extensions/network/dns_resolver/apple/apple_dns_impl.cc index b7a877db83..36f70e778a 100644 --- a/source/extensions/network/dns_resolver/apple/apple_dns_impl.cc +++ b/source/extensions/network/dns_resolver/apple/apple_dns_impl.cc @@ -60,7 +60,7 @@ AppleDnsResolverStats AppleDnsResolverImpl::generateAppleDnsResolverStats(Stats: AppleDnsResolverImpl::StartResolutionResult AppleDnsResolverImpl::startResolution(const std::string& dns_name, DnsLookupFamily dns_lookup_family, ResolveCb callback) { - ENVOY_LOG_EVENT(debug, "apple_dns_start", "DNS resolution for {} started", dns_name); + ENVOY_LOG_EVENT(trace, "apple_dns_start", "DNS resolution for {} started", dns_name); // When an IP address is submitted to c-ares in DnsResolverImpl, c-ares synchronously returns // the IP without submitting a DNS query. Because Envoy has come to rely on this behavior, this @@ -69,7 +69,7 @@ AppleDnsResolverImpl::startResolution(const std::string& dns_name, auto address = Utility::parseInternetAddressNoThrow(dns_name); if (address != nullptr) { - ENVOY_LOG_EVENT(debug, "apple_dns_immediate_resolution", + ENVOY_LOG_EVENT(trace, "apple_dns_immediate_resolution", "DNS resolver resolved ({}) to ({}) without issuing call to Apple API", dns_name, address->asString()); callback(DnsResolver::ResolutionStatus::Completed, "apple_dns_immediate_success", @@ -148,7 +148,7 @@ AppleDnsResolverImpl::PendingResolution::PendingResolution(AppleDnsResolverImpl& pending_response_(PendingResponse()), dns_lookup_family_(dns_lookup_family) {} AppleDnsResolverImpl::PendingResolution::~PendingResolution() { - ENVOY_LOG(debug, "Destroying PendingResolution for {}", dns_name_); + ENVOY_LOG(trace, "Destroying PendingResolution for {}", dns_name_); // dns_sd.h says: // If the reference's underlying socket is used in a run loop or select() call, it should @@ -162,7 +162,7 @@ AppleDnsResolverImpl::PendingResolution::~PendingResolution() { // thus the DNSServiceRef is null. // Therefore, only deallocate if the ref is not null. if (sd_ref_) { - ENVOY_LOG(debug, "DNSServiceRefDeallocate individual sd ref"); + ENVOY_LOG(trace, "DNSServiceRefDeallocate individual sd ref"); DnsServiceSingleton::get().dnsServiceRefDeallocate(sd_ref_); } } @@ -191,7 +191,7 @@ std::string AppleDnsResolverImpl::PendingResolution::getTraces() { } void AppleDnsResolverImpl::PendingResolution::onEventCallback(uint32_t events) { - ENVOY_LOG(debug, "DNS resolver file event ({})", events); + ENVOY_LOG(trace, "DNS resolver file event ({})", events); RELEASE_ASSERT(events & Event::FileReadyType::Read, fmt::format("invalid FileReadyType event={}", events)); DNSServiceErrorType error = DnsServiceSingleton::get().dnsServiceProcessResult(sd_ref_); @@ -246,7 +246,7 @@ std::list& AppleDnsResolverImpl::PendingResolution::finalAddressLis } void AppleDnsResolverImpl::PendingResolution::finishResolve(AppleDnsTrace trace) { - ENVOY_LOG_EVENT(debug, "apple_dns_resolution_complete", + ENVOY_LOG_EVENT(trace, "apple_dns_resolution_complete", "dns resolution for {} completed with status {}", dns_name_, static_cast(pending_response_.status_)); addTrace(static_cast(trace)); @@ -254,10 +254,10 @@ void AppleDnsResolverImpl::PendingResolution::finishResolve(AppleDnsTrace trace) std::move(finalAddressList())); if (owned_) { - ENVOY_LOG(debug, "Resolution for {} completed (async)", dns_name_); + ENVOY_LOG(trace, "Resolution for {} completed (async)", dns_name_); delete this; } else { - ENVOY_LOG(debug, "Resolution for {} completed (synchronously)", dns_name_); + ENVOY_LOG(trace, "Resolution for {} completed (synchronously)", dns_name_); synchronously_completed_ = true; } } @@ -337,7 +337,7 @@ void AppleDnsResolverImpl::PendingResolution::onDNSServiceGetAddrInfoReply( // still be non-null and its `sa_family` will be the address family of the query (even if the // address itself isn't a meaningful IP address). - ENVOY_LOG(debug, + ENVOY_LOG(trace, "DNS for {} resolved with: flags={}[MoreComing={}, Add={}], interface_index={}, " "error_code={}, hostname={}", dns_name_, flags, flags & kDNSServiceFlagsMoreComing ? "yes" : "no", @@ -380,7 +380,7 @@ void AppleDnsResolverImpl::PendingResolution::onDNSServiceGetAddrInfoReply( // Therefore, only add this address to the list if kDNSServiceFlagsAdd is set. if (error_code == kDNSServiceErr_NoError && (flags & kDNSServiceFlagsAdd)) { auto dns_response = buildDnsResponse(address, ttl); - ENVOY_LOG(debug, "Address to add address={}, ttl={}", + ENVOY_LOG(trace, "Address to add address={}, ttl={}", dns_response.addrInfo().address_->ip()->addressAsString(), ttl); if (dns_response.addrInfo().address_->ip()->ipv4()) { pending_response_.v4_responses_.push_back(dns_response); @@ -392,7 +392,7 @@ void AppleDnsResolverImpl::PendingResolution::onDNSServiceGetAddrInfoReply( if (!(flags & kDNSServiceFlagsMoreComing) && isAddressFamilyProcessed(kDNSServiceProtocol_IPv4) && isAddressFamilyProcessed(kDNSServiceProtocol_IPv6)) { - ENVOY_LOG(debug, "DNS Resolver flushing queries pending callback"); + ENVOY_LOG(trace, "DNS Resolver flushing queries pending callback"); pending_response_.status_ = ResolutionStatus::Completed; pending_response_.details_ = absl::StrCat("apple_dns_completed_", error_code); AppleDnsTrace trace = (error_code == kDNSServiceErr_NoSuchRecord) ? AppleDnsTrace::NoResult diff --git a/source/extensions/network/dns_resolver/cares/dns_impl.cc b/source/extensions/network/dns_resolver/cares/dns_impl.cc index 0896963c7d..69588e9897 100644 --- a/source/extensions/network/dns_resolver/cares/dns_impl.cc +++ b/source/extensions/network/dns_resolver/cares/dns_impl.cc @@ -212,7 +212,7 @@ void DnsResolverImpl::AddrInfoPendingResolution::onAresGetAddrInfoCallback( // ARES_ECONNREFUSED. If the PendingResolution has not been cancelled that means that the // callback_ target _should_ still be around. In that case, raise the callback_ so the target // can be done with this query and initiate a new one. - ENVOY_LOG_EVENT(debug, "cares_dns_resolution_destroyed", "dns resolution for {} destroyed", + ENVOY_LOG_EVENT(trace, "cares_dns_resolution_destroyed", "dns resolution for {} destroyed", dns_name_); // Nothing can follow a call to finishResolve due to the deletion of this object upon @@ -322,7 +322,7 @@ void DnsResolverImpl::AddrInfoPendingResolution::onAresGetAddrInfoCallback( } void DnsResolverImpl::PendingResolution::finishResolve() { - ENVOY_LOG_EVENT(debug, "cares_dns_resolution_complete", + ENVOY_LOG_EVENT(trace, "cares_dns_resolution_complete", "dns resolution for {} completed with status {:#06x}: \"{}\"", dns_name_, static_cast(pending_response_.status_), pending_response_.details_); @@ -419,7 +419,7 @@ void DnsResolverImpl::reinitializeChannel() { int result = ares_reinit(channel_); RELEASE_ASSERT(result == ARES_SUCCESS, "c-ares channel re-initialization failed"); stats_.reinits_.inc(); - ENVOY_LOG_EVENT(debug, "cares_channel_reinitialized", + ENVOY_LOG_EVENT(trace, "cares_channel_reinitialized", "Reinitialized cares channel via ares_reinit"); if (resolvers_csv_.has_value()) { @@ -439,7 +439,7 @@ void DnsResolverImpl::reinitializeChannel() { ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, DnsLookupFamily dns_lookup_family, ResolveCb callback) { - ENVOY_LOG_EVENT(debug, "cares_dns_resolution_start", "dns resolution for {} started", dns_name); + ENVOY_LOG_EVENT(trace, "cares_dns_resolution_start", "dns resolution for {} started", dns_name); // TODO(hennna): Add DNS caching which will allow testing the edge case of a // failed initial call to getAddrInfo followed by a synchronous IPv4 @@ -451,7 +451,7 @@ ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, if (pending_resolution->completed_) { // Resolution does not need asynchronous behavior or network events. For // example, localhost lookup. - ENVOY_LOG_EVENT(debug, "cares_resolution_completed", + ENVOY_LOG_EVENT(trace, "cares_resolution_completed", "dns resolution for {} completed with no async or network events", dns_name); return nullptr; } else { @@ -520,14 +520,14 @@ void DnsResolverImpl::AddrInfoPendingResolution::startResolutionImpl(int family) switch (family) { case AF_INET: if (!available_interfaces_.v4_available_) { - ENVOY_LOG_EVENT(debug, "cares_resolution_filtered", "filtered v4 lookup"); + ENVOY_LOG_EVENT(trace, "cares_resolution_filtered", "filtered v4 lookup"); onAresGetAddrInfoCallback(ARES_EBADFAMILY, 0, nullptr); return; } break; case AF_INET6: if (!available_interfaces_.v6_available_) { - ENVOY_LOG_EVENT(debug, "cares_resolution_filtered", "filtered v6 lookup"); + ENVOY_LOG_EVENT(trace, "cares_resolution_filtered", "filtered v6 lookup"); onAresGetAddrInfoCallback(ARES_EBADFAMILY, 0, nullptr); return; } @@ -644,7 +644,7 @@ class CaresDnsResolverFactory : public DnsResolverFactory, absl::MutexLock lock(&mutex_); if (!ares_library_initialized_) { ares_library_initialized_ = true; - ENVOY_LOG(debug, "c-ares library initialized."); + ENVOY_LOG(trace, "c-ares library initialized."); ares_library_init(ARES_LIB_INIT_ALL); } } @@ -653,7 +653,7 @@ class CaresDnsResolverFactory : public DnsResolverFactory, absl::MutexLock lock(&mutex_); if (ares_library_initialized_) { ares_library_initialized_ = false; - ENVOY_LOG(debug, "c-ares library cleaned up."); + ENVOY_LOG(trace, "c-ares library cleaned up."); ares_library_cleanup(); } } diff --git a/source/extensions/network/dns_resolver/getaddrinfo/getaddrinfo.cc b/source/extensions/network/dns_resolver/getaddrinfo/getaddrinfo.cc index 0a1bed228a..82bbb2964a 100644 --- a/source/extensions/network/dns_resolver/getaddrinfo/getaddrinfo.cc +++ b/source/extensions/network/dns_resolver/getaddrinfo/getaddrinfo.cc @@ -34,8 +34,8 @@ GetAddrInfoDnsResolver::~GetAddrInfoDnsResolver() { ActiveDnsQuery* GetAddrInfoDnsResolver::resolve(const std::string& dns_name, DnsLookupFamily dns_lookup_family, ResolveCb callback) { - ENVOY_LOG(debug, "adding new query [{}] to pending queries", dns_name); - auto new_query = std::make_unique(dns_name, dns_lookup_family, callback); + ENVOY_LOG(trace, "adding new query [{}] to pending queries", dns_name); + auto new_query = std::make_unique(dns_name, dns_lookup_family, std::move(callback)); new_query->addTrace(static_cast(GetAddrInfoTrace::NotStarted)); ActiveDnsQuery* active_query; { @@ -109,7 +109,7 @@ GetAddrInfoDnsResolver::processResponse(const PendingQuery& query, break; } - ENVOY_LOG(debug, "getaddrinfo resolution complete for host '{}': {}", query.dns_name_, + ENVOY_LOG(trace, "getaddrinfo resolution complete for host '{}': {}", query.dns_name_, accumulateToString(final_results, [](const auto& dns_response) { return dns_response.addrInfo().address_->asString(); })); @@ -119,7 +119,7 @@ GetAddrInfoDnsResolver::processResponse(const PendingQuery& query, // Background thread which wakes up and does resolutions. void GetAddrInfoDnsResolver::resolveThreadRoutine() { - ENVOY_LOG(debug, "starting getaddrinfo resolver thread"); + ENVOY_LOG(trace, "starting getaddrinfo resolver thread"); while (true) { std::unique_ptr next_query; @@ -143,7 +143,7 @@ void GetAddrInfoDnsResolver::resolveThreadRoutine() { } } - ENVOY_LOG(debug, "popped pending query [{}]", next_query->dns_name_); + ENVOY_LOG(trace, "popped pending query [{}]", next_query->dns_name_); // For mock testing make sure the getaddrinfo() response is freed prior to the post. std::pair> response; @@ -170,7 +170,7 @@ void GetAddrInfoDnsResolver::resolveThreadRoutine() { (*num_retries)--; } if (!num_retries.has_value()) { - ENVOY_LOG(debug, "retrying query [{}]", next_query->dns_name_); + ENVOY_LOG(trace, "retrying query [{}]", next_query->dns_name_); next_query->addTrace(static_cast(GetAddrInfoTrace::Retrying)); { absl::MutexLock guard(&mutex_); @@ -179,7 +179,7 @@ void GetAddrInfoDnsResolver::resolveThreadRoutine() { continue; } if (*num_retries > 0) { - ENVOY_LOG(debug, "retrying query [{}], num_retries: {}", next_query->dns_name_, + ENVOY_LOG(trace, "retrying query [{}], num_retries: {}", next_query->dns_name_, *num_retries); next_query->addTrace(static_cast(GetAddrInfoTrace::Retrying)); { @@ -215,7 +215,7 @@ void GetAddrInfoDnsResolver::resolveThreadRoutine() { details = std::string(details)]() mutable { if (finished_query->isCancelled()) { finished_query->addTrace(static_cast(GetAddrInfoTrace::Cancelled)); - ENVOY_LOG(debug, "dropping cancelled query [{}]", finished_query->dns_name_); + ENVOY_LOG(trace, "dropping cancelled query [{}]", finished_query->dns_name_); } else { finished_query->addTrace(static_cast(GetAddrInfoTrace::Callback)); finished_query->callback_(response.first, std::move(details), std::move(response.second)); @@ -223,7 +223,7 @@ void GetAddrInfoDnsResolver::resolveThreadRoutine() { }); } - ENVOY_LOG(debug, "getaddrinfo resolver thread exiting"); + ENVOY_LOG(trace, "getaddrinfo resolver thread exiting"); } // Register the CaresDnsResolverFactory diff --git a/source/extensions/network/dns_resolver/getaddrinfo/getaddrinfo.h b/source/extensions/network/dns_resolver/getaddrinfo/getaddrinfo.h index 2357fdb4ac..17c7818116 100644 --- a/source/extensions/network/dns_resolver/getaddrinfo/getaddrinfo.h +++ b/source/extensions/network/dns_resolver/getaddrinfo/getaddrinfo.h @@ -51,7 +51,7 @@ class GetAddrInfoDnsResolver : public DnsResolver, public Logger::Loggable, Filesystem::Instance& file_system, const ProcessContextOptRef& process_context = absl::nullopt); + ~ValidationInstance() override; + // Server::Instance void run() override { PANIC("not implemented"); } OptRef admin() override { diff --git a/test/common/quic/envoy_quic_utils_test.cc b/test/common/quic/envoy_quic_utils_test.cc index 7ebbc84657..299a2cbdcb 100644 --- a/test/common/quic/envoy_quic_utils_test.cc +++ b/test/common/quic/envoy_quic_utils_test.cc @@ -40,6 +40,8 @@ TEST(EnvoyQuicUtilsTest, ConversionBetweenQuicAddressAndEnvoyAddress) { class MockServerHeaderValidator : public HeaderValidator { public: ~MockServerHeaderValidator() override = default; + MOCK_METHOD(void, startHeaderBlock, ()); + MOCK_METHOD(bool, finishHeaderBlock, (bool is_trailing_headers)); MOCK_METHOD(Http::HeaderUtility::HeaderValidationResult, validateHeader, (absl::string_view header_name, absl::string_view header_value)); }; @@ -59,6 +61,8 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) { NiceMock validator; absl::string_view details; quic::QuicRstStreamErrorCode rst = quic::QUIC_REFUSED_STREAM; + EXPECT_CALL(validator, startHeaderBlock()); + EXPECT_CALL(validator, finishHeaderBlock(true)).WillOnce(Return(true)); auto envoy_headers = http2HeaderBlockToEnvoyTrailers( headers_block, 60, 100, validator, details, rst); // Envoy header block is 3 headers larger because QUICHE header block does coalescing. @@ -87,6 +91,7 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) { quic_headers.OnHeader("key1", "value2"); quic_headers.OnHeader("key-to-drop", ""); quic_headers.OnHeaderBlockEnd(0, 0); + EXPECT_CALL(validator, startHeaderBlock()); EXPECT_CALL(validator, validateHeader(_, _)) .WillRepeatedly([](absl::string_view header_name, absl::string_view) { if (header_name == "key-to-drop") { @@ -94,6 +99,7 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) { } return Http::HeaderUtility::HeaderValidationResult::ACCEPT; }); + EXPECT_CALL(validator, finishHeaderBlock(false)).WillOnce(Return(true)); auto envoy_headers2 = quicHeadersToEnvoyHeaders( quic_headers, validator, 60, 100, details, rst); EXPECT_EQ(*envoy_headers, *envoy_headers2); @@ -105,6 +111,7 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) { quic_headers2.OnHeader(":scheme", "https"); quic_headers2.OnHeader("invalid_key", ""); quic_headers2.OnHeaderBlockEnd(0, 0); + EXPECT_CALL(validator, startHeaderBlock()); EXPECT_CALL(validator, validateHeader(_, _)) .WillRepeatedly([](absl::string_view header_name, absl::string_view) { if (header_name == "invalid_key") { @@ -118,23 +125,29 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) { } TEST(EnvoyQuicUtilsTest, HeadersSizeBounds) { - quiche::HttpHeaderBlock headers_block; - headers_block[":authority"] = "www.google.com"; - headers_block[":path"] = "/index.hml"; - headers_block[":scheme"] = "https"; - headers_block["foo"] = std::string("bar\0eep\0baz", 11); + quic::QuicHeaderList quic_headers; + quic_headers.OnHeader(":authority", "www.google.com"); + quic_headers.OnHeader(":path", "/index.hml"); + quic_headers.OnHeader(":scheme", "https"); + quic_headers.OnHeader("foo1", "bar"); + quic_headers.OnHeader("foo2", "bar"); + quic_headers.OnHeader("foo3", "bar"); + quic_headers.OnHeaderBlockEnd(0, 0); absl::string_view details; - // 6 headers are allowed. NiceMock validator; quic::QuicRstStreamErrorCode rst = quic::QUIC_REFUSED_STREAM; - EXPECT_NE(nullptr, http2HeaderBlockToEnvoyTrailers( - headers_block, 60, 6, validator, details, rst)); + EXPECT_CALL(validator, finishHeaderBlock(false)).WillOnce(Return(true)); + // 6 headers are allowed. + EXPECT_NE(nullptr, quicHeadersToEnvoyHeaders(quic_headers, validator, + 60, 6, details, rst)); // Given the cap is 6, make sure anything lower, exact or otherwise, is rejected. - EXPECT_EQ(nullptr, http2HeaderBlockToEnvoyTrailers( - headers_block, 60, 5, validator, details, rst)); - EXPECT_EQ("http3.too_many_trailers", details); - EXPECT_EQ(nullptr, http2HeaderBlockToEnvoyTrailers( - headers_block, 60, 4, validator, details, rst)); + EXPECT_EQ(nullptr, quicHeadersToEnvoyHeaders(quic_headers, validator, + 60, 5, details, rst)); + EXPECT_EQ("http3.too_many_headers", details); + EXPECT_EQ(rst, quic::QUIC_STREAM_EXCESSIVE_LOAD); + EXPECT_EQ(nullptr, quicHeadersToEnvoyHeaders(quic_headers, validator, + 60, 4, details, rst)); + EXPECT_EQ("http3.too_many_headers", details); EXPECT_EQ(rst, quic::QUIC_STREAM_EXCESSIVE_LOAD); } @@ -147,13 +160,18 @@ TEST(EnvoyQuicUtilsTest, TrailersSizeBounds) { absl::string_view details; NiceMock validator; quic::QuicRstStreamErrorCode rst = quic::QUIC_REFUSED_STREAM; + EXPECT_CALL(validator, finishHeaderBlock(true)).WillOnce(Return(true)); + // 6 headers are allowed. EXPECT_NE(nullptr, http2HeaderBlockToEnvoyTrailers( headers_block, 60, 6, validator, details, rst)); + // Given the cap is 6, make sure anything lower, exact or otherwise, is rejected. EXPECT_EQ(nullptr, http2HeaderBlockToEnvoyTrailers( - headers_block, 60, 2, validator, details, rst)); + headers_block, 60, 5, validator, details, rst)); EXPECT_EQ("http3.too_many_trailers", details); + EXPECT_EQ(rst, quic::QUIC_STREAM_EXCESSIVE_LOAD); EXPECT_EQ(nullptr, http2HeaderBlockToEnvoyTrailers( - headers_block, 60, 2, validator, details, rst)); + headers_block, 60, 4, validator, details, rst)); + EXPECT_EQ("http3.too_many_trailers", details); EXPECT_EQ(rst, quic::QUIC_STREAM_EXCESSIVE_LOAD); } @@ -164,6 +182,7 @@ TEST(EnvoyQuicUtilsTest, TrailerCharacters) { headers_block[":scheme"] = "https"; absl::string_view details; NiceMock validator; + EXPECT_CALL(validator, startHeaderBlock()); EXPECT_CALL(validator, validateHeader(_, _)) .WillRepeatedly(Return(Http::HeaderUtility::HeaderValidationResult::REJECT)); quic::QuicRstStreamErrorCode rst = quic::QUIC_REFUSED_STREAM; @@ -228,10 +247,12 @@ TEST(EnvoyQuicUtilsTest, HeaderMapMaxSizeLimit) { quic_headers.OnHeader(":path", "/index.hml"); quic_headers.OnHeader(":scheme", "https"); quic_headers.OnHeaderBlockEnd(0, 0); + EXPECT_CALL(validator, startHeaderBlock()); EXPECT_CALL(validator, validateHeader(_, _)) .WillRepeatedly([](absl::string_view, absl::string_view) { return Http::HeaderUtility::HeaderValidationResult::ACCEPT; }); + EXPECT_CALL(validator, finishHeaderBlock(false)).WillOnce(Return(true)); // Request header map test. auto request_header = quicHeadersToEnvoyHeaders( quic_headers, validator, 60, 100, details, rst); @@ -239,6 +260,8 @@ TEST(EnvoyQuicUtilsTest, HeaderMapMaxSizeLimit) { EXPECT_EQ(request_header->maxHeadersKb(), 60); // Response header map test. + EXPECT_CALL(validator, startHeaderBlock()); + EXPECT_CALL(validator, finishHeaderBlock(false)).WillOnce(Return(true)); auto response_header = quicHeadersToEnvoyHeaders( quic_headers, validator, 60, 100, details, rst); EXPECT_EQ(response_header->maxHeadersCount(), 100); @@ -250,12 +273,16 @@ TEST(EnvoyQuicUtilsTest, HeaderMapMaxSizeLimit) { headers_block[":scheme"] = "https"; // Request trailer map test. + EXPECT_CALL(validator, startHeaderBlock()); + EXPECT_CALL(validator, finishHeaderBlock(true)).WillOnce(Return(true)); auto request_trailer = http2HeaderBlockToEnvoyTrailers( headers_block, 60, 100, validator, details, rst); EXPECT_EQ(request_trailer->maxHeadersCount(), 100); EXPECT_EQ(request_trailer->maxHeadersKb(), 60); // Response trailer map test. + EXPECT_CALL(validator, startHeaderBlock()); + EXPECT_CALL(validator, finishHeaderBlock(true)).WillOnce(Return(true)); auto response_trailer = http2HeaderBlockToEnvoyTrailers( headers_block, 60, 100, validator, details, rst); EXPECT_EQ(response_trailer->maxHeadersCount(), 100); diff --git a/test/config/utility.cc b/test/config/utility.cc index c752048d46..d80781f839 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -672,7 +672,8 @@ envoy::config::listener::v3::Listener ConfigHelper::buildListener(const std::str } envoy::config::route::v3::RouteConfiguration -ConfigHelper::buildRouteConfig(const std::string& name, const std::string& cluster) { +ConfigHelper::buildRouteConfig(const std::string& name, const std::string& cluster, + bool header_mutations) { API_NO_BOOST(envoy::config::route::v3::RouteConfiguration) route; #ifdef ENVOY_ENABLE_YAML TestUtility::loadFromYaml(fmt::format(R"EOF( @@ -686,10 +687,31 @@ ConfigHelper::buildRouteConfig(const std::string& name, const std::string& clust )EOF", name, cluster), route); + + if (header_mutations) { + auto* route_entry = route.mutable_virtual_hosts(0)->mutable_routes(0); + auto* header1 = route_entry->add_request_headers_to_add(); + *header1->mutable_header()->mutable_key() = "test-metadata"; + *header1->mutable_header()->mutable_value() = "%METADATA(ROUTE:com.test.my_filter)%"; + + auto* header2 = route_entry->add_response_headers_to_add(); + *header2->mutable_header()->mutable_key() = "test-cel"; + *header2->mutable_header()->mutable_value() = "%CEL(request.headers['some-header'])%"; + + auto* header3 = route_entry->add_response_headers_to_add(); + *header3->mutable_header()->mutable_key() = "test-other-command"; + *header3->mutable_header()->mutable_value() = "%START_TIME%"; + + auto* header4 = route_entry->add_response_headers_to_add(); + *header4->mutable_header()->mutable_key() = "test-plain"; + *header4->mutable_header()->mutable_value() = "plain"; + } + return route; #else UNREFERENCED_PARAMETER(name); UNREFERENCED_PARAMETER(cluster); + UNREFERENCED_PARAMETER(header_mutations); PANIC("YAML support compiled out"); #endif } diff --git a/test/config/utility.h b/test/config/utility.h index b9f530fe49..fcb6cc6575 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -164,8 +164,7 @@ class ConfigHelper { bool keylog_multiple_ips_{false}; std::string keylog_path_; Network::Address::IpVersion ip_version_{Network::Address::IpVersion::v4}; - std::vector - san_matchers_{}; + std::vector san_matchers_; std::string tls_cert_selector_yaml_{""}; bool client_with_intermediate_cert_{false}; bool trust_root_only_{false}; @@ -280,8 +279,9 @@ class ConfigHelper { const std::string& address, const std::string& stat_prefix); - static envoy::config::route::v3::RouteConfiguration buildRouteConfig(const std::string& name, - const std::string& cluster); + static envoy::config::route::v3::RouteConfiguration + buildRouteConfig(const std::string& name, const std::string& cluster, + bool header_mutations = false); // Builds a standard Endpoint suitable for population by finalize(). static envoy::config::endpoint::v3::Endpoint buildEndpoint(const std::string& address); diff --git a/test/extensions/resource_monitors/cpu_utilization/linux_cpu_stats_reader_test.cc b/test/extensions/resource_monitors/cpu_utilization/linux_cpu_stats_reader_test.cc index ea268a8e0c..43d70819ff 100644 --- a/test/extensions/resource_monitors/cpu_utilization/linux_cpu_stats_reader_test.cc +++ b/test/extensions/resource_monitors/cpu_utilization/linux_cpu_stats_reader_test.cc @@ -89,29 +89,50 @@ cpu1 1883161 620 375962 1448133 5963 0 85914 10 0 0 EXPECT_EQ(cpu_times.total_time, 0); } -TEST(LinuxContainerCpuStatsReader, ReadsCgroupContainerStats) { - Event::MockDispatcher dispatcher; - Api::ApiPtr api = Api::createApiForTest(); - Server::MockOptions options; - Server::Configuration::ResourceMonitorFactoryContextImpl context( - dispatcher, options, *api, ProtobufMessage::getStrictValidationVisitor()); - TimeSource& test_time_source = context.api().timeSource(); - - const std::string temp_path_cpu_allocated = - TestEnvironment::temporaryPath("cgroup_cpu_allocated_stats"); - AtomicFileUpdater file_updater_cpu_allocated(temp_path_cpu_allocated); - const std::string mock_contents_cpu_allocated = R"EOF(2000 -)EOF"; - file_updater_cpu_allocated.update(mock_contents_cpu_allocated); - - const std::string temp_path_cpu_times = TestEnvironment::temporaryPath("cgroup_cpu_times_stats"); - AtomicFileUpdater file_updater_cpu_times(temp_path_cpu_times); - const std::string mock_contents_cpu_times = R"EOF(1000 -)EOF"; - file_updater_cpu_times.update(mock_contents_cpu_times); - - LinuxContainerCpuStatsReader container_stats_reader(test_time_source, temp_path_cpu_allocated, - temp_path_cpu_times); +class LinuxContainerCpuStatsReaderTest : public testing::Test { +public: + LinuxContainerCpuStatsReaderTest() + : api_(Api::createApiForTest()), + context_(dispatcher_, options_, *api_, ProtobufMessage::getStrictValidationVisitor()), + cpu_allocated_path_(TestEnvironment::temporaryPath("cgroup_cpu_allocated_stats")), + cpu_times_path_(TestEnvironment::temporaryPath("cgroup_cpu_times_stats")) { + // We populate the files that LinuxContainerStatsReader tries to read with some default + // sane values, so the tests don't need to populate the files they don't actually care + // about keeping the test cases focused on what they actually want to test. + setCpuAllocated("2000\n"); + setCpuTimes("1000\n"); + } + + TimeSource& timeSource() { return context_.api().timeSource(); } + + const std::string& cpuAllocatedPath() const { return cpu_allocated_path_; } + void setCpuAllocated(const std::string& contents) { + AtomicFileUpdater cpu_allocated(cpuAllocatedPath()); + cpu_allocated.update(contents); + } + + const std::string& cpuTimesPath() const { return cpu_times_path_; } + void setCpuTimes(const std::string& contents) { + AtomicFileUpdater cpu_times(cpuTimesPath()); + cpu_times.update(contents); + } + +private: + Event::MockDispatcher dispatcher_; + Api::ApiPtr api_; + Server::MockOptions options_; + Server::Configuration::ResourceMonitorFactoryContextImpl context_; + std::string cpu_allocated_path_; + std::string cpu_times_path_; +}; + +TEST_F(LinuxContainerCpuStatsReaderTest, ReadsCgroupContainerStats) { + TimeSource& test_time_source = timeSource(); + setCpuAllocated("2000\n"); + setCpuTimes("1000\n"); + + LinuxContainerCpuStatsReader container_stats_reader(test_time_source, cpuAllocatedPath(), + cpuTimesPath()); CpuTimes envoy_container_stats = container_stats_reader.getCpuTimes(); const uint64_t current_monotonic_time = std::chrono::duration_cast( @@ -123,50 +144,25 @@ TEST(LinuxContainerCpuStatsReader, ReadsCgroupContainerStats) { EXPECT_GT(time_diff_ns, 0); } -TEST(LinuxContainerCpuStatsReader, CannotReadFileCpuAllocated) { - Event::MockDispatcher dispatcher; - Api::ApiPtr api = Api::createApiForTest(); - Server::MockOptions options; - Server::Configuration::ResourceMonitorFactoryContextImpl context( - dispatcher, options, *api, ProtobufMessage::getStrictValidationVisitor()); - TimeSource& test_time_source = context.api().timeSource(); - +TEST_F(LinuxContainerCpuStatsReaderTest, CannotReadFileCpuAllocated) { + TimeSource& test_time_source = timeSource(); const std::string temp_path_cpu_allocated = - TestEnvironment::temporaryPath("container_cpu_allocated_not_exists"); - - const std::string temp_path_cpu_times = TestEnvironment::temporaryPath("container_cpu_times"); - AtomicFileUpdater file_updater_cpu_times(temp_path_cpu_times); - const std::string cpu_times_contents = R"EOF(100000 -)EOF"; - file_updater_cpu_times.update(cpu_times_contents); + TestEnvironment::temporaryPath("container_cpu_times_not_exists"); LinuxContainerCpuStatsReader container_stats_reader(test_time_source, temp_path_cpu_allocated, - temp_path_cpu_times); + cpuTimesPath()); CpuTimes envoy_container_stats = container_stats_reader.getCpuTimes(); EXPECT_FALSE(envoy_container_stats.is_valid); EXPECT_EQ(envoy_container_stats.work_time, 0); EXPECT_EQ(envoy_container_stats.total_time, 0); } -TEST(LinuxContainerCpuStatsReader, CannotReadFileCpuTimes) { - Event::MockDispatcher dispatcher; - Api::ApiPtr api = Api::createApiForTest(); - Server::MockOptions options; - Server::Configuration::ResourceMonitorFactoryContextImpl context( - dispatcher, options, *api, ProtobufMessage::getStrictValidationVisitor()); - TimeSource& test_time_source = context.api().timeSource(); - - const std::string temp_path_cpu_allocated = - TestEnvironment::temporaryPath("container_cpu_allocated"); - AtomicFileUpdater file_updater_cpu_allocated(temp_path_cpu_allocated); - const std::string cpu_allocated_contents = R"EOF(1000101 -)EOF"; - file_updater_cpu_allocated.update(cpu_allocated_contents); - +TEST_F(LinuxContainerCpuStatsReaderTest, CannotReadFileCpuTimes) { + TimeSource& test_time_source = timeSource(); const std::string temp_path_cpu_times = TestEnvironment::temporaryPath("container_cpu_times_not_exists"); - LinuxContainerCpuStatsReader container_stats_reader(test_time_source, temp_path_cpu_allocated, + LinuxContainerCpuStatsReader container_stats_reader(test_time_source, cpuAllocatedPath(), temp_path_cpu_times); CpuTimes envoy_container_stats = container_stats_reader.getCpuTimes(); EXPECT_FALSE(envoy_container_stats.is_valid); @@ -174,52 +170,24 @@ TEST(LinuxContainerCpuStatsReader, CannotReadFileCpuTimes) { EXPECT_EQ(envoy_container_stats.total_time, 0); } -TEST(LinuxContainerCpuStatsReader, UnexpectedFormatCpuAllocatedLine) { - Event::MockDispatcher dispatcher; - Api::ApiPtr api = Api::createApiForTest(); - Server::MockOptions options; - Server::Configuration::ResourceMonitorFactoryContextImpl context( - dispatcher, options, *api, ProtobufMessage::getStrictValidationVisitor()); - TimeSource& test_time_source = context.api().timeSource(); +TEST_F(LinuxContainerCpuStatsReaderTest, UnexpectedFormatCpuAllocatedLine) { + TimeSource& test_time_source = timeSource(); + setCpuAllocated("notanumb3r\n"); - const std::string temp_path_cpu_allocated = - TestEnvironment::temporaryPath("container_cpu_allocated_unexpected_format"); - AtomicFileUpdater file_updater_cpu_allocated(temp_path_cpu_allocated); - const std::string cpu_allocated_contents = R"EOF(notanumb3r -)EOF"; - file_updater_cpu_allocated.update(cpu_allocated_contents); - - LinuxContainerCpuStatsReader container_stats_reader(test_time_source, temp_path_cpu_allocated); + LinuxContainerCpuStatsReader container_stats_reader(test_time_source, cpuAllocatedPath(), + cpuTimesPath()); CpuTimes envoy_container_stats = container_stats_reader.getCpuTimes(); EXPECT_FALSE(envoy_container_stats.is_valid); EXPECT_EQ(envoy_container_stats.work_time, 0); EXPECT_EQ(envoy_container_stats.total_time, 0); } -TEST(LinuxContainerCpuStatsReader, UnexpectedFormatCpuTimesLine) { - Event::MockDispatcher dispatcher; - Api::ApiPtr api = Api::createApiForTest(); - Server::MockOptions options; - Server::Configuration::ResourceMonitorFactoryContextImpl context( - dispatcher, options, *api, ProtobufMessage::getStrictValidationVisitor()); - TimeSource& test_time_source = context.api().timeSource(); - - const std::string temp_path_cpu_allocated = - TestEnvironment::temporaryPath("container_cpu_allocated"); - AtomicFileUpdater file_updater_cpu_allocated(temp_path_cpu_allocated); - const std::string cpu_allocated_contents = R"EOF(1000101 -)EOF"; - file_updater_cpu_allocated.update(cpu_allocated_contents); +TEST_F(LinuxContainerCpuStatsReaderTest, UnexpectedFormatCpuTimesLine) { + TimeSource& test_time_source = timeSource(); + setCpuTimes("notanumb3r\n"); - const std::string temp_path_cpu_times = - TestEnvironment::temporaryPath("container_cpu_times_unexpected_format"); - AtomicFileUpdater file_update_cpu_times(temp_path_cpu_times); - const std::string cpu_times_contents = R"EOF(notanumb3r -)EOF"; - file_update_cpu_times.update(cpu_times_contents); - - LinuxContainerCpuStatsReader container_stats_reader(test_time_source, temp_path_cpu_allocated, - temp_path_cpu_times); + LinuxContainerCpuStatsReader container_stats_reader(test_time_source, cpuAllocatedPath(), + cpuTimesPath()); CpuTimes envoy_container_stats = container_stats_reader.getCpuTimes(); EXPECT_FALSE(envoy_container_stats.is_valid); EXPECT_EQ(envoy_container_stats.work_time, 0); diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/sampling_controller_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/sampling_controller_test.cc index 17da6ac7cb..f449af1226 100644 --- a/test/extensions/tracers/opentelemetry/samplers/dynatrace/sampling_controller_test.cc +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/sampling_controller_test.cc @@ -89,6 +89,24 @@ TEST(SamplingControllerTest, TestWithOneAllowedSpan) { EXPECT_EQ(sc.getSamplingState("1").getMultiplicity(), 1); } +// Test with 1 root span per minute and more offered entries +TEST(SamplingControllerTest, TestWithOneAllowedSpanMoreEntries) { + auto scf = std::make_unique(1); + SamplingController sc(std::move(scf)); + sc.update(); + EXPECT_EQ(sc.getSamplingState("1").getExponent(), SamplingController::MAX_SAMPLING_EXPONENT); + offerEntry(sc, "1", 1); + offerEntry(sc, "2", 1); + offerEntry(sc, "3", 1); + EXPECT_EQ(sc.getSamplingState("1").getExponent(), SamplingController::MAX_SAMPLING_EXPONENT); + EXPECT_EQ(sc.getSamplingState("2").getExponent(), SamplingController::MAX_SAMPLING_EXPONENT); + EXPECT_EQ(sc.getSamplingState("3").getExponent(), SamplingController::MAX_SAMPLING_EXPONENT); + sc.update(); + EXPECT_EQ(sc.getSamplingState("1").getMultiplicity(), 2); + EXPECT_EQ(sc.getSamplingState("2").getMultiplicity(), 2); + EXPECT_EQ(sc.getSamplingState("3").getMultiplicity(), 1); +} + // Test with StreamSummary size not exceeded TEST(SamplingControllerTest, TestStreamSummarySizeNotExceeded) { auto scf = std::make_unique(); diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc index d0f6a7ec6c..12b63d3510 100644 --- a/test/integration/quic_http_integration_test.cc +++ b/test/integration/quic_http_integration_test.cc @@ -560,6 +560,24 @@ TEST_P(QuicHttpIntegrationTest, ResetRequestWithoutAuthorityHeader) { codec_client_->close(); } +// Test to ensure code coverage of the flag codepath. +TEST_P(QuicHttpIntegrationTest, DoNotValidatePseudoHeaders) { + config_helper_.addRuntimeOverride("envoy.restart_features.validate_http3_pseudo_headers", + "false"); + + initialize(); + + codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); + auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); + + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(default_response_headers_, true); + + EXPECT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + codec_client_->close(); +} + TEST_P(QuicHttpIntegrationTest, ResetRequestWithInvalidCharacter) { config_helper_.addRuntimeOverride("envoy.reloadable_features.validate_upstream_headers", "false"); @@ -1017,7 +1035,8 @@ TEST_P(QuicHttpIntegrationTest, DeferredLoggingWithQuicReset) { EXPECT_EQ(/* request headers */ metrics.at(19), metrics.at(20)); } -TEST_P(QuicHttpIntegrationTest, DeferredLoggingWithEnvoyReset) { +// TODO(RyanTheOptimist): Re-enable after figuring out how to cause this reset. +TEST_P(QuicHttpIntegrationTest, DISABLED_DeferredLoggingWithEnvoyReset) { config_helper_.addRuntimeOverride( "envoy.reloadable_features.FLAGS_envoy_quiche_reloadable_flag_quic_act_upon_invalid_header", "false"); diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index 22e3edf615..a8cfd0790a 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -1521,6 +1521,50 @@ TEST_P(TcpProxySslIntegrationTest, LargeBidirectionalTlsWrites) { sendAndReceiveTlsData(large_data, large_data); } +// Test that if SSL connection data, such as peer certificate data, is read before it is +// available, it is not cached when it is read again later when available. +TEST_P(TcpProxySslIntegrationTest, SslConnectionDataEarlyReadNotCached) { + std::string access_log_path = TestEnvironment::temporaryPath( + fmt::format("access_log{}{}.txt", version_ == Network::Address::IpVersion::v4 ? "v4" : "v6", + TestUtility::uniqueFilename())); + config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { + auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); + auto* filter_chain = listener->mutable_filter_chains(0); + auto* config_blob = filter_chain->mutable_filters(0)->mutable_typed_config(); + + ASSERT_TRUE(config_blob->Is()); + auto tcp_proxy_config = + MessageUtil::anyConvert( + *config_blob); + + auto* access_log = tcp_proxy_config.add_access_log(); + access_log->set_name("accesslog"); + envoy::extensions::access_loggers::file::v3::FileAccessLog access_log_config; + access_log_config.set_path(access_log_path); + access_log_config.mutable_log_format()->mutable_text_format_source()->set_inline_string( + "san=%DOWNSTREAM_PEER_URI_SAN% fingerprint=%DOWNSTREAM_PEER_FINGERPRINT_256%\n"); + access_log->mutable_typed_config()->PackFrom(access_log_config); + tcp_proxy_config.mutable_access_log_options()->set_flush_access_log_on_connected(true); + config_blob->PackFrom(tcp_proxy_config); + }); + + setupConnections(); + std::string large_data(1024 * 8, 'a'); + sendAndReceiveTlsData(large_data, large_data); + + // The test set `flush_access_log_on_connected`, so the first access log is emitted before the + // handshake has completed. + auto log_result = waitForAccessLog(access_log_path, 0, true); + EXPECT_EQ(log_result, "san=- fingerprint=-"); + + // The second access log is when the connection closes, so the handshake is complete and + // a valid peer cert is now available. + log_result = waitForAccessLog(access_log_path, 1, false); + EXPECT_EQ(log_result, + "san=spiffe://lyft.com/frontend-team,http://frontend.lyft.com " + "fingerprint=7346b3836cfc41385351191b5e6163f1a69704cfdf0a03634ed2019128e6fdc4"); +} + // Test that a half-close on the downstream side is proxied correctly. TEST_P(TcpProxySslIntegrationTest, DownstreamHalfClose) { setupConnections(); diff --git a/test/server/config_validation/xds_verifier_test.cc b/test/server/config_validation/xds_verifier_test.cc index f0cbef7fdc..d5f73e9bec 100644 --- a/test/server/config_validation/xds_verifier_test.cc +++ b/test/server/config_validation/xds_verifier_test.cc @@ -11,7 +11,7 @@ envoy::config::listener::v3::Listener buildListener(const std::string& listener_ } envoy::config::route::v3::RouteConfiguration buildRoute(const std::string& route_name) { - return ConfigHelper::buildRouteConfig(route_name, "cluster_0"); + return ConfigHelper::buildRouteConfig(route_name, "cluster_0", true); } // Add, warm, drain and remove a listener.