diff --git a/.bazelrc b/.bazelrc index 116a06899ad..6ae4da8d619 100644 --- a/.bazelrc +++ b/.bazelrc @@ -611,6 +611,12 @@ common:debug --config=debug-sandbox common:debug --config=debug-coverage common:debug --config=debug-tests +############################################################################# +# compat: Compatibility with main branch repo settings +############################################################################# +common:bes --config=bes-envoy-engflow +common:rbe --config=remote-envoy-engflow + try-import %workspace%/repo.bazelrc try-import %workspace%/clang.bazelrc try-import %workspace%/user.bazelrc diff --git a/.bazelversion b/.bazelversion index e81e85b8104..5942a0d3a0e 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -7.6.2 +7.7.1 diff --git a/.github/workflows/request.yml b/.github/workflows/request.yml index 5e3b0f10ad3..dc8d4999adc 100644 --- a/.github/workflows/request.yml +++ b/.github/workflows/request.yml @@ -25,7 +25,7 @@ concurrency: jobs: request: permissions: - actions: read + actions: write contents: read packages: read # required to fetch merge commit @@ -36,9 +36,6 @@ jobs: app-id: ${{ secrets.ENVOY_CI_APP_ID }} lock-app-key: ${{ secrets.ENVOY_CI_MUTEX_APP_KEY }} lock-app-id: ${{ secrets.ENVOY_CI_MUTEX_APP_ID }} - gcs-cache-key: ${{ secrets.GCS_CACHE_WRITE_KEY }} - with: - gcs-cache-bucket: ${{ vars.ENVOY_CACHE_BUCKET }} # For branches this can be pinned to a specific version if required # NB: `uses` cannot be dynamic so it _must_ be hardcoded anywhere it is read uses: envoyproxy/envoy/.github/workflows/_request.yml@main diff --git a/VERSION.txt b/VERSION.txt index 1255cd3bea8..027d0e5f402 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -1.34.11-dev +1.34.12-dev diff --git a/changelogs/1.33.13.yaml b/changelogs/1.33.13.yaml new file mode 100644 index 00000000000..d363d83d3ed --- /dev/null +++ b/changelogs/1.33.13.yaml @@ -0,0 +1,17 @@ +date: December 3, 2025 + +behavior_changes: +- area: http + change: | + Added runtime flag ``envoy.reloadable_features.reject_early_connect_data`` to reject ``CONNECT`` requests + that receive data before Envoy sent a ``200`` response to the client. While this is not a strictly compliant behavior + it is very common as a latency reducing measure. As such the option is disabled by default. + +bug_fixes: +- area: tls + change: | + Fixed an issue where SANs of type ``OTHERNAME`` in a TLS cert were truncated if there was + an embedded null octet, leading to incorrect SAN validation. +- area: http + change: | + Fixed a remote ``jwt_auth`` token fetch crash with two or more auth headers when ``allow_missing_or_failed`` is set. diff --git a/changelogs/1.34.11.yaml b/changelogs/1.34.11.yaml new file mode 100644 index 00000000000..2cb66c8264d --- /dev/null +++ b/changelogs/1.34.11.yaml @@ -0,0 +1,28 @@ +date: December 3, 2025 + +behavior_changes: +- area: dynamic modules + change: | + The dynamic module ABI has been updated to support streaming body manipulation. This change also + fixed potential incorrect behavior when access or modify the request or response body. See + https://github.com/envoyproxy/envoy/issues/40918 for more details. +- area: http + change: | + Added runtime flag ``envoy.reloadable_features.reject_early_connect_data`` to reject ``CONNECT`` requests + that receive data before Envoy sent a ``200`` response to the client. While this is not a strictly compliant behavior + it is very common as a latency reducing measure. As such the option is disabled by default. + +bug_fixes: +- area: tcp_proxy + change: | + Fixed a connection leak in the TCP proxy when the ``receive_before_connect`` feature is enabled and the + downstream connection closes before the upstream connection is established. + +deprecated: +- area: tls + change: | + Fixed an issue where SANs of type ``OTHERNAME`` in a TLS cert were truncated if there was + an embedded null octet, leading to incorrect SAN validation. +- area: http + change: | + Fixed a remote ``jwt_auth`` token fetch crash with two or more auth headers when ``allow_missing_or_failed`` is set. diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 8f7c5784ed5..9ecf0d6e48c 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -2,11 +2,6 @@ date: Pending behavior_changes: # *Changes that are expected to cause an incompatibility if applicable; deployment changes are likely required* -- area: dynamic modules - change: | - The dynamic module ABI has been updated to support streaming body manipulation. This change also - fixed potential incorrect behavior when access or modify the request or response body. See - https://github.com/envoyproxy/envoy/issues/40918 for more details. minor_behavior_changes: # *Changes that may cause incompatibilities for some users, but should not for most* diff --git a/distribution/docker/Dockerfile-envoy b/distribution/docker/Dockerfile-envoy index cef84f010f9..c0e6b853717 100644 --- a/distribution/docker/Dockerfile-envoy +++ b/distribution/docker/Dockerfile-envoy @@ -1,6 +1,6 @@ ARG BUILD_OS=ubuntu ARG BUILD_TAG=22.04 -ARG BUILD_SHA=09506232a8004baa32c47d68f1e5c307d648fdd59f5e7eaa42aaf87914100db3 +ARG BUILD_SHA=104ae83764a5119017b8e8d6218fa0832b09df65aae7d5a6de29a85d813da2fb ARG ENVOY_VRP_BASE_IMAGE=envoy-base @@ -29,7 +29,7 @@ RUN --mount=type=tmpfs,target=/var/cache/apt \ --mount=type=tmpfs,target=/var/lib/apt/lists \ apt-get -qq update \ && apt-get -qq upgrade -y \ - && apt-get -qq install --no-install-recommends -y ca-certificates \ + && apt-get -qq install --no-install-recommends -y ca-certificates tzdata \ && apt-get -qq autoremove -y diff --git a/docs/inventories/v1.33/objects.inv b/docs/inventories/v1.33/objects.inv index 33a769d5e23..108c439ad92 100644 Binary files a/docs/inventories/v1.33/objects.inv and b/docs/inventories/v1.33/objects.inv differ diff --git a/docs/inventories/v1.34/objects.inv b/docs/inventories/v1.34/objects.inv index 032e5279c4e..9a4ab7e0354 100644 Binary files a/docs/inventories/v1.34/objects.inv and b/docs/inventories/v1.34/objects.inv differ diff --git a/docs/root/configuration/http/http_conn_man/response_code_details.rst b/docs/root/configuration/http/http_conn_man/response_code_details.rst index dafd4056d4c..ac2840da3f8 100644 --- a/docs/root/configuration/http/http_conn_man/response_code_details.rst +++ b/docs/root/configuration/http/http_conn_man/response_code_details.rst @@ -25,6 +25,7 @@ Below are the list of reasons the HttpConnectionManager or Router filter may sen downstream_remote_disconnect, The client disconnected unexpectedly. duration_timeout, The max connection duration was exceeded. direct_response, A direct response was generated by the router filter. + early_connect_data, Data was received for a CONNECT request before 200 response headers were sent. filter_added_invalid_request_data, A filter added request data at the wrong stage in the filter chain. filter_added_invalid_response_data, A filter added response data at the wrong stage in the filter chain. filter_chain_not_found, The request was rejected due to no matching filter chain. diff --git a/docs/versions.yaml b/docs/versions.yaml index 4f12eefa1a5..918dd11b7c3 100644 --- a/docs/versions.yaml +++ b/docs/versions.yaml @@ -26,5 +26,5 @@ "1.30": 1.30.11 "1.31": 1.31.10 "1.32": 1.32.13 -"1.33": 1.33.12 -"1.34": 1.34.9 +"1.33": 1.33.13 +"1.34": 1.34.10 diff --git a/envoy/stream_info/stream_info.h b/envoy/stream_info/stream_info.h index 6cb68573d23..0f3c6a83e93 100644 --- a/envoy/stream_info/stream_info.h +++ b/envoy/stream_info/stream_info.h @@ -246,6 +246,8 @@ struct ResponseCodeDetailValues { const std::string FilterAddedInvalidRequestData = "filter_added_invalid_request_data"; // A filter called addDecodedData at the wrong point in the filter chain. const std::string FilterAddedInvalidResponseData = "filter_added_invalid_response_data"; + // Data was received for a CONNECT request before 200 response headers were sent. + const std::string EarlyConnectData = "early_connect_data"; // Changes or additions to details should be reflected in // docs/root/configuration/http/http_conn_man/response_code_details.rst }; diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 46b4aac1329..1a4b5e66df8 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -957,7 +957,19 @@ void Filter::sendNoHealthyUpstreamResponse(absl::optional optional_ absl::nullopt, details); } +bool Filter::isEarlyConnectData() { + return downstream_headers_ != nullptr && Http::HeaderUtility::isConnect(*downstream_headers_) && + !downstream_response_started_ && + Runtime::runtimeFeatureEnabled("envoy.reloadable_features.reject_early_connect_data"); +} + Http::FilterDataStatus Filter::decodeData(Buffer::Instance& data, bool end_stream) { + ENVOY_STREAM_LOG(debug, "router decoding data: {}", *callbacks_, data.length()); + if (data.length() > 0 && isEarlyConnectData()) { + callbacks_->sendLocalReply(Http::Code::BadRequest, "", nullptr, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().EarlyConnectData); + return Http::FilterDataStatus::StopIterationNoBuffer; + } // upstream_requests_.size() cannot be > 1 because that only happens when a per // try timeout occurs with hedge_on_per_try_timeout enabled but the per // try timeout timer is not started until onRequestComplete(). It could be zero diff --git a/source/common/router/router.h b/source/common/router/router.h index 98042a1a7c0..83aa68f62ad 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -601,6 +601,7 @@ class Filter : Logger::Loggable, // Process Orca Load Report if necessary (e.g. cluster has lrsReportMetricNames). void maybeProcessOrcaLoadReport(const Envoy::Http::HeaderMap& headers_or_trailers, UpstreamRequest& upstream_request); + bool isEarlyConnectData(); RetryStatePtr retry_state_; const FilterConfigSharedPtr config_; diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 6a2a507097e..bfc721c4921 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -183,6 +183,10 @@ FALSE_RUNTIME_GUARD(envoy_reloadable_features_quic_disable_client_early_data); FALSE_RUNTIME_GUARD(envoy_reloadable_features_ext_proc_graceful_grpc_close); +// TODO(yavlasov): Enabling by default will be hugely disruptive to existing traffic. +// Replace with a config option (default off) post CVE release. +FALSE_RUNTIME_GUARD(envoy_reloadable_features_reject_early_connect_data); + // Block of non-boolean flags. Use of int flags is deprecated. Do not add more. ABSL_FLAG(uint64_t, re2_max_program_size_error_level, 100, ""); // NOLINT ABSL_FLAG(uint64_t, re2_max_program_size_warn_level, // NOLINT diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index c0bf591cabb..dedad3cc252 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -1002,9 +1002,9 @@ void Filter::onConnectMaxAttempts() { void Filter::onUpstreamConnection() { connecting_ = false; - // If we have received any data before upstream connection is established, send it to - // the upstream connection. - if (early_data_buffer_.length() > 0) { + // If we have received any data before upstream connection is established, or if the downstream + // has indicated end of stream, send the data and/or end_stream to the upstream connection. + if (early_data_buffer_.length() > 0 || early_data_end_stream_) { // Early data should only happen when receive_before_connect is enabled. ASSERT(receive_before_connect_); @@ -1017,7 +1017,7 @@ void Filter::onUpstreamConnection() { // Re-enable downstream reads now that the early data buffer is flushed. read_callbacks_->connection().readDisable(false); } else if (!receive_before_connect_) { - // Re-enable downstream reads now that the upstream connection is established + // Re-enable downstream reads now that the upstream connection is established. read_callbacks_->connection().readDisable(false); } diff --git a/source/common/tls/utility.cc b/source/common/tls/utility.cc index 762ba487658..49ee770cc80 100644 --- a/source/common/tls/utility.cc +++ b/source/common/tls/utility.cc @@ -336,22 +336,24 @@ std::string Utility::generalNameAsString(const GENERAL_NAME* general_name) { break; } case V_ASN1_BMPSTRING: { - // `ASN1_BMPSTRING` is encoded using `UCS-4`, which needs conversion to UTF-8. + // `ASN1_BMPSTRING` is encoded using `UTF-16`, which needs conversion to UTF-8. unsigned char* tmp = nullptr; - if (ASN1_STRING_to_UTF8(&tmp, value->value.bmpstring) < 0) { + int length = ASN1_STRING_to_UTF8(&tmp, value->value.bmpstring); + if (length < 0) { break; } - san.assign(reinterpret_cast(tmp)); + san.assign(reinterpret_cast(tmp), length); OPENSSL_free(tmp); break; } case V_ASN1_UNIVERSALSTRING: { // `ASN1_UNIVERSALSTRING` is encoded using `UCS-4`, which needs conversion to UTF-8. unsigned char* tmp = nullptr; - if (ASN1_STRING_to_UTF8(&tmp, value->value.universalstring) < 0) { + int length = ASN1_STRING_to_UTF8(&tmp, value->value.universalstring); + if (length < 0) { break; } - san.assign(reinterpret_cast(tmp)); + san.assign(reinterpret_cast(tmp), length); OPENSSL_free(tmp); break; } diff --git a/source/extensions/filters/http/common/jwks_fetcher.cc b/source/extensions/filters/http/common/jwks_fetcher.cc index f881c5031e1..aba110cc501 100644 --- a/source/extensions/filters/http/common/jwks_fetcher.cc +++ b/source/extensions/filters/http/common/jwks_fetcher.cc @@ -35,6 +35,7 @@ class JwksFetcherImpl : public JwksFetcher, request_->cancel(); ENVOY_LOG(debug, "fetch pubkey [uri = {}]: canceled", remote_jwks_.http_uri().uri()); } + complete_ = true; reset(); } @@ -129,8 +130,10 @@ class JwksFetcherImpl : public JwksFetcher, Http::AsyncClient::Request* request_{}; void reset() { - request_ = nullptr; - receiver_ = nullptr; + if (complete_) { + request_ = nullptr; + receiver_ = nullptr; + } } }; } // namespace diff --git a/source/extensions/filters/http/jwt_authn/authenticator.cc b/source/extensions/filters/http/jwt_authn/authenticator.cc index 79c3146f2f8..6821d8ccb14 100644 --- a/source/extensions/filters/http/jwt_authn/authenticator.cc +++ b/source/extensions/filters/http/jwt_authn/authenticator.cc @@ -288,6 +288,12 @@ void AuthenticatorImpl::startVerify() { if (jwks_data_->getJwtProvider().has_remote_jwks()) { if (!fetcher_) { fetcher_ = create_jwks_fetcher_cb_(cm_, jwks_data_->getJwtProvider().remote_jwks()); + } else { + // Cancel the previous fetch to reset if it is pending or not completed. + // At most one outstanding request may be in-flight, and it is possible that + // a new call is from the callback itself, which in-turn will reset the + // fetcher afterwards. + fetcher_->cancel(); } fetcher_->fetch(*parent_span_, *this); return; diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 8659ac4c11f..69a0f8a6c9b 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -797,6 +797,43 @@ TEST_P(TcpProxyTest, ReceiveBeforeConnectEarlyDataWithEndStream) { upstream_callbacks_->onUpstreamData(response, false); } +// Test that when downstream closes without sending any data before upstream connection is +// established, the end_stream signal is properly propagated to upstream. +// This prevents upstream connection leaks. +TEST_P(TcpProxyTest, ReceiveBeforeConnectDownstreamClosesWithoutData) { + setup(/*connections=*/1, /*set_redirect_records=*/false, /*receive_before_connect=*/true); + + // Downstream closes without sending any data. + Buffer::OwnedImpl empty_buffer; + EXPECT_CALL(*upstream_connections_.at(0), write(_, _)).Times(0); + filter_->onData(empty_buffer, /*end_stream=*/true); + + // When upstream connection is established, the end_stream signal should be sent even though + // the buffer is empty. This ensures the upstream connection is properly closed. + EXPECT_CALL(*upstream_connections_.at(0), write(BufferStringEqual(""), /*end_stream*/ true)); + raiseEventUpstreamConnected(/*conn_index=*/0); +} + +// Test that when downstream sends empty buffer with end_stream before upstream is connected, +// the end_stream is properly handled. +TEST_P(TcpProxyTest, ReceiveBeforeConnectEmptyBufferWithEndStream) { + setup(/*connections=*/1, /*set_redirect_records=*/false, /*receive_before_connect=*/true); + + // Downstream sends empty data with end_stream set. + Buffer::OwnedImpl empty_buffer; + EXPECT_CALL(*upstream_connections_.at(0), write(_, _)).Times(0); + filter_->onData(empty_buffer, /*end_stream=*/true); + + // When upstream connection is established, end_stream should be propagated. + EXPECT_CALL(*upstream_connections_.at(0), write(BufferStringEqual(""), /*end_stream*/ true)); + raiseEventUpstreamConnected(/*conn_index=*/0); + + // Upstream can still send data back. + Buffer::OwnedImpl response("response data"); + EXPECT_CALL(filter_callbacks_.connection_, write(BufferEqual(&response), _)); + upstream_callbacks_->onUpstreamData(response, false); +} + TEST_P(TcpProxyTest, ReceiveBeforeConnectNoEarlyData) { setup(1, /*set_redirect_records=*/false, /*receive_before_connect=*/true); raiseEventUpstreamConnected(/*conn_index=*/0, /*expect_read_enable=*/false); diff --git a/test/common/tls/utility_test.cc b/test/common/tls/utility_test.cc index 9164892c7c8..f466506f213 100644 --- a/test/common/tls/utility_test.cc +++ b/test/common/tls/utility_test.cc @@ -60,6 +60,52 @@ TEST(UtilityTest, TestDnsNameMatching) { EXPECT_FALSE(Utility::dnsNameMatch("lyft.com", "")); } +TEST(UtilityTest, TestOtherNameUniversalWithEmbeddedNull) { + // Universal strings are utf-32. + uint32_t utf32_data[] = { + htonl('t'), htonl('e'), htonl('s'), htonl('t'), 0 /* embedded null */, htonl('s'), htonl('t'), + htonl('r'), htonl('i'), htonl('n'), htonl('g'), + }; + ASN1_STRING* asn1_str = ASN1_UNIVERSALSTRING_new(); + ASN1_STRING_set(asn1_str, utf32_data, sizeof(utf32_data)); + GENERAL_NAME* name = GENERAL_NAME_new(); + ASN1_OBJECT* oid = OBJ_txt2obj("1.2.3.4.5", 1); + ASN1_TYPE* type = ASN1_TYPE_new(); + ASN1_TYPE_set(type, V_ASN1_UNIVERSALSTRING, asn1_str); + GENERAL_NAME_set0_othername(name, oid, type); + + std::string expected = "test"; + expected += '\0'; + expected += "string"; + + EXPECT_EQ(Utility::generalNameAsString(name), expected); + + GENERAL_NAME_free(name); +} + +TEST(UtilityTest, TestOtherNameBmpWithEmbeddedNull) { + // `BMP` strings are utf-16. + uint16_t utf16_data[] = { + htons('t'), htons('e'), htons('s'), htons('t'), 0 /* embedded null */, htons('s'), htons('t'), + htons('r'), htons('i'), htons('n'), htons('g'), + }; + ASN1_STRING* asn1_str = ASN1_BMPSTRING_new(); + ASN1_STRING_set(asn1_str, utf16_data, sizeof(utf16_data)); + GENERAL_NAME* name = GENERAL_NAME_new(); + ASN1_OBJECT* oid = OBJ_txt2obj("1.2.3.4.5", 1); + ASN1_TYPE* type = ASN1_TYPE_new(); + ASN1_TYPE_set(type, V_ASN1_BMPSTRING, asn1_str); + GENERAL_NAME_set0_othername(name, oid, type); + + std::string expected = "test"; + expected += '\0'; + expected += "string"; + + EXPECT_EQ(Utility::generalNameAsString(name), expected); + + GENERAL_NAME_free(name); +} + TEST(UtilityTest, TestGetSubjectAlternateNamesWithDNS) { bssl::UniquePtr cert = readCertFromFile( TestEnvironment::substitute("{{ test_rundir }}/test/common/tls/test_data/san_dns_cert.pem")); diff --git a/test/extensions/filters/http/jwt_authn/authenticator_test.cc b/test/extensions/filters/http/jwt_authn/authenticator_test.cc index 0202d588358..56880739783 100644 --- a/test/extensions/filters/http/jwt_authn/authenticator_test.cc +++ b/test/extensions/filters/http/jwt_authn/authenticator_test.cc @@ -1031,6 +1031,7 @@ TEST_F(AuthenticatorTest, TestAllowFailedMultipleIssuers) { header->set_value_prefix("Bearer "); createAuthenticator(nullptr, absl::nullopt, /*allow_failed=*/true); + EXPECT_CALL(*raw_fetcher_, cancel()); EXPECT_CALL(*raw_fetcher_, fetch(_, _)) .Times(2) .WillRepeatedly(Invoke([](Tracing::Span&, JwksFetcher::JwksReceiver& receiver) { diff --git a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc index 4973cbf9607..94f46dfc72d 100644 --- a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc @@ -412,6 +412,19 @@ class RemoteJwksIntegrationTest : public HttpProtocolIntegrationTest { initialize(); } + void initializeFilterWithAllowMissingOrFailed() { + config_helper_.prependFilter( + getAuthFilterConfig(AllowMissingExampleConfig, false, false, false)); + + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto* jwks_cluster = bootstrap.mutable_static_resources()->add_clusters(); + jwks_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); + jwks_cluster->set_name("pubkey_cluster"); + }); + + initialize(); + } + void initializeAsyncFetchFilter(bool fast_listener) { config_helper_.prependFilter(getAsyncFetchFilterConfig(ExampleConfig, fast_listener, false)); @@ -572,6 +585,47 @@ TEST_P(RemoteJwksIntegrationTest, FetchFailedMissingCluster) { cleanup(); } +// With remote Jwks, this test verifies a request is passed with two good JWTs +// and allow_missing_or_failed but the jwks fetching fails. +TEST_P(RemoteJwksIntegrationTest, WithTwoGoodTokensAllowMissing) { + initializeFilterWithAllowMissingOrFailed(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"Authorization", "Bearer " + std::string(GoodToken)}, + {"Authorization", "Bearer " + std::string(GoodToken)}, + }); + + // Fails the jwks fetching. + waitForJwksResponse("500", ""); + + // Wait for the second fetching. + auto result = fake_jwks_connection_->waitForNewStream(*dispatcher_, jwks_request_); + RELEASE_ASSERT(result, result.message()); + result = jwks_request_->waitForEndStream(*dispatcher_); + RELEASE_ASSERT(result, result.message()); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "500"}}; + jwks_request_->encodeHeaders(response_headers, false); + Buffer::OwnedImpl response_data1(""); + jwks_request_->encodeData(response_data1, true); + + waitForNextUpstreamRequest(); + + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); + + cleanup(); +} + TEST_P(RemoteJwksIntegrationTest, WithGoodTokenAsyncFetch) { on_server_init_function_ = [this]() { waitForJwksResponse("200", PublicKey); }; initializeAsyncFetchFilter(false); diff --git a/test/extensions/filters/http/jwt_authn/test_common.h b/test/extensions/filters/http/jwt_authn/test_common.h index aa9eb099a53..775c916b287 100644 --- a/test/extensions/filters/http/jwt_authn/test_common.h +++ b/test/extensions/filters/http/jwt_authn/test_common.h @@ -186,6 +186,32 @@ const char ExampleConfig[] = R"( bypass_cors_preflight: true )"; +// A good config with allow_missing_or_failed. +const char AllowMissingExampleConfig[] = R"( +providers: + example_provider: + issuer: https://example.com + audiences: + - example_service + - http://example_service1 + - https://example_service2/ + remote_jwks: + http_uri: + uri: https://www.pubkey-server.com/pubkey-path + cluster: pubkey_cluster + timeout: + seconds: 5 + cache_duration: + seconds: 600 + forward_payload_header: sec-istio-auth-userinfo +rules: +- match: + path: "/" + requires: + allow_missing_or_failed: {} +bypass_cors_preflight: true +)"; + // Config with claim_to_headers and clear_route_cache. const char ClaimToHeadersConfig[] = R"( providers: diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index 82baf3ecc31..46152cc1103 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -665,6 +665,11 @@ class FakeRawConnection : public FakeConnectionBase { data_.clear(); } + bool hasData() { + absl::MutexLock lock(&lock_); + return !data_.empty(); + } + private: struct ReadFilter : public Network::ReadFilterBaseImpl { ReadFilter(FakeRawConnection& parent) : parent_(parent) {} diff --git a/test/integration/tcp_tunneling_integration_test.cc b/test/integration/tcp_tunneling_integration_test.cc index e6f50281b9f..5cde56f717f 100644 --- a/test/integration/tcp_tunneling_integration_test.cc +++ b/test/integration/tcp_tunneling_integration_test.cc @@ -447,6 +447,69 @@ TEST_P(ConnectTerminationIntegrationTest, IgnoreH11HostField) { sendRawHttpAndWaitForResponse(lookupPort("http"), full_request.c_str(), &response, true);); } +TEST_P(ConnectTerminationIntegrationTest, EarlyConnectDataRejectedWithOverride) { + config_helper_.addRuntimeOverride("envoy.reloadable_features.reject_early_connect_data", "true"); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + // Send CONNECT request and immediately send some data without waiting for 200 + // response from Envoy. + auto encoder_decoder = codec_client_->startRequest(connect_headers_); + request_encoder_ = &encoder_decoder.first; + codec_client_->sendData(*request_encoder_, "premature data", false); + response_ = std::move(encoder_decoder.second); + + // Envoy will try top open upstream connection before the premature CONNECT data is detected. + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_raw_upstream_connection_)); + + response_->waitForHeaders(); + EXPECT_EQ(response_->headers().getStatusValue(), "400"); + EXPECT_TRUE(response_->waitForEndStream()); + + // Because the downstream connection is closed by Envoy without sending any data the + // upstream connection will remain in the pool and will not be closed. + // However it should not have any data in it. + EXPECT_FALSE(fake_raw_upstream_connection_->hasData()); + cleanupUpstreamAndDownstream(); +} + +TEST_P(ConnectTerminationIntegrationTest, EarlyConnectDataAllowedByDefault) { + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + // Send CONNECT request and immediately send some data without waiting for 200 + // response from Envoy. + auto encoder_decoder = codec_client_->startRequest(connect_headers_); + request_encoder_ = &encoder_decoder.first; + codec_client_->sendData(*request_encoder_, "premature data", false); + response_ = std::move(encoder_decoder.second); + + // Wait for the data to arrive upstream. + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_raw_upstream_connection_)); + ASSERT_TRUE(fake_raw_upstream_connection_->waitForData( + FakeRawConnection::waitForInexactMatch("premature data"))); + + // Send some data downstream. + ASSERT_TRUE(fake_raw_upstream_connection_->write("upstream_send_data")); + + // Wait for the headers and data to arrive downstream. + response_->waitForHeaders(); + response_->waitForBodyData(strlen("upstream_send_data")); + EXPECT_EQ("upstream_send_data", response_->body()); + + codec_client_->sendData(*request_encoder_, "", true); + ASSERT_TRUE(fake_raw_upstream_connection_->waitForHalfClose()); + + ASSERT_TRUE(fake_raw_upstream_connection_->close()); + if (downstream_protocol_ == Http::CodecType::HTTP1) { + ASSERT_TRUE(codec_client_->waitForDisconnect()); + } else { + ASSERT_TRUE(response_->waitForEndStream()); + ASSERT_FALSE(response_->reset()); + } + cleanupUpstreamAndDownstream(); +} + INSTANTIATE_TEST_SUITE_P(HttpAndIpVersions, ConnectTerminationIntegrationTest, testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( {Http::CodecType::HTTP1, Http::CodecType::HTTP2,