[bp/1.36] Security patches 25q4 (#42374)

phlax · yanavlasov · botengyao · web-flow · commit daefd2f7b3fc · 2025-12-03T21:37:51.000-05:00
* Security fixes: - [CVE-2025-64527](GHSA-mp85-7mrq-r866): Envoy crashes when JWT authentication is configured with the remote JWKS fetching - [CVE-2025-66220](GHSA-rwjg-c3h2-f57p): TLS certificate matcher for `match_typed_subject_alt_names` may incorrectly treat certificates containing an embedded null byte - [CVE-2025-64763](GHSA-rj35-4m94-77jh): Potential request smuggling from early data after the CONNECT up|grade --------- Signed-off-by: Greg Greenway <ggreenway@apple.com> Signed-off-by: Yan Avlasov <yavlasov@google.com> Signed-off-by: Boteng Yao <boteng@google.com> Signed-off-by: Ryan Northey <ryan@synca.io> Co-authored-by: Yan Avlasov <yavlasov@google.com> Co-authored-by: Boteng Yao <boteng@google.com>
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -7,6 +7,11 @@ behavior_changes:
     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.
 
 minor_behavior_changes:
 # *Changes that may cause incompatibilities for some users, but should not for most*
@@ -22,6 +27,13 @@ bug_fixes:
   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.
+- 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.
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
diff --git a/changelogs/summary.md b/changelogs/summary.md
@@ -0,0 +1,5 @@
+
+* Security fixes:
+  - CVE-2025-64527: Envoy crashes when JWT authentication is configured with the remote JWKS fetching
+  - CVE-2025-66220: TLS certificate matcher for `match_typed_subject_alt_names` may incorrectly treat certificates containing an embedded null byte
+  - CVE-2025-64763: Potential request smuggling from early data after the CONNECT upgrade
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
@@ -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/envoy/stream_info/stream_info.h b/envoy/stream_info/stream_info.h
@@ -248,6 +248,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
@@ -956,7 +956,19 @@ uint64_t Filter::calculateEffectiveBufferLimit() const {
   return std::numeric_limits<uint64_t>::max();
 }
 
+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
@@ -625,6 +625,7 @@ class Filter : Logger::Loggable<Logger::Id::router>,
   // 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
@@ -192,6 +192,9 @@ FALSE_RUNTIME_GUARD(envoy_restart_features_use_cached_grpc_client_for_xds);
 FALSE_RUNTIME_GUARD(envoy_reloadable_features_http_11_proxy_connect_legacy_format);
 // TODO(tsaarni): Flip to true after prod testing or remove.
 FALSE_RUNTIME_GUARD(envoy_reloadable_features_fixed_heap_use_allocated);
+// 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
diff --git a/source/common/tls/utility.cc b/source/common/tls/utility.cc
@@ -339,22 +339,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<const char*>(tmp));
+      san.assign(reinterpret_cast<const char*>(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<const char*>(tmp));
+      san.assign(reinterpret_cast<const char*>(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
@@ -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
@@ -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/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<X509> 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
@@ -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
@@ -410,6 +410,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));
 
@@ -570,6 +583,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
@@ -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
@@ -675,6 +675,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
@@ -449,6 +449,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,

Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,7 @@ class JwksFetcherImpl : public JwksFetcher,`
`35`	`35`	`request_->cancel();`
`36`	`36`	`ENVOY_LOG(debug, "fetch pubkey [uri = {}]: canceled", remote_jwks_.http_uri().uri());`
`37`	`37`	`}`
	`38`	`+ complete_ = true;`
`38`	`39`	`reset();`
`39`	`40`	`}`
`40`	`41`
`@@ -129,8 +130,10 @@ class JwksFetcherImpl : public JwksFetcher,`
`129`	`130`	`Http::AsyncClient::Request* request_{};`
`130`	`131`
`131`	`132`	`void reset() {`
`132`		`- request_ = nullptr;`
`133`		`- receiver_ = nullptr;`
	`133`	`+ if (complete_) {`
	`134`	`+ request_ = nullptr;`
	`135`	`+ receiver_ = nullptr;`
	`136`	`+ }`
`134`	`137`	`}`
`135`	`138`	`};`
`136`	`139`	`} // namespace`