hcm: add match_upstream to SchemeHeaderTransformation (#34099)

Signed-off-by: William <wtzhang23@gmail.com>

api/envoy/config/core/v3/protocol.proto

@@ -651,6 +651,13 @@ message Http3ProtocolOptions {
 message SchemeHeaderTransformation {
   oneof transformation {
     // Overwrite any Scheme header with the contents of this string.
+    // If set, takes precedence over match_upstream.
     string scheme_to_overwrite = 1 [(validate.rules).string = {in: "http" in: "https"}];
   }
+
+  // Set the Scheme header to match the upstream transport protocol. For example, should a
+  // request be sent to the upstream over TLS, the scheme header will be set to "https". Should the
+  // request be sent over plaintext, the scheme header will be set to "http".
+  // If scheme_to_overwrite is set, this field is not used.
+  bool match_upstream = 2;
 }

changelogs/current.yaml

@@ -193,6 +193,10 @@ new_features:
     <envoy_v3_api_field_config.route.v3.RouteAction.RequestMirrorPolicy.disable_shadow_host_suffix_append>`
     in :ref:`request_mirror_policies <envoy_v3_api_field_config.route.v3.RouteAction.request_mirror_policies>`
     for disabling appending of the ``-shadow`` suffix to the shadowed host/authority header.
+- area: http
+  change: |
+    Added field :ref:`match_upstream <envoy_v3_api_field_config.core.v3.SchemeHeaderTransformation.match_upstream>`,
+    which, when set to true, will set the downstream request ``:scheme`` to match the upstream transport protocol.
 - area: redis
   change: |
     Added support for `inline commands <https://redis.io/docs/reference/protocol-spec/#inline-commands>`_.

envoy/stream_info/stream_info.h

@@ -957,6 +957,20 @@ class StreamInfo {
    */
   virtual void setDownstreamTransportFailureReason(absl::string_view failure_reason) PURE;
 
+  /**
+   * Checked by routing filters before forwarding a request upstream.
+   * @return to override the scheme header to match the upstream transport
+   * protocol at routing filters.
+   */
+  virtual bool shouldSchemeMatchUpstream() const PURE;
+
+  /**
+   * Called if a filter decides that the scheme should match the upstream transport protocol
+   * @param should_match_upstream true to hint to routing filters to override the scheme header
+   * to match the upstream transport protocol.
+   */
+  virtual void setShouldSchemeMatchUpstream(bool should_match_upstream) PURE;
+
   /**
    * Checked by streams after finishing serving the request.
    * @return bool true if the connection should be drained once this stream has

source/common/http/conn_manager_config.h

@@ -365,6 +365,11 @@ class ConnectionManagerConfig {
    */
   virtual const absl::optional<std::string>& schemeToSet() const PURE;
 
+  /**
+   * @return bool whether the scheme should be overwritten to match the upstream transport protocol.
+   */
+  virtual bool shouldSchemeMatchUpstream() const PURE;
+
   /**
    * @return ConnectionManagerStats& the stats to write to.
    */

source/common/http/conn_manager_impl.cc

@@ -823,6 +823,9 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect
   filter_manager_.streamInfo().setStreamIdProvider(
       std::make_shared<HttpStreamIdProviderImpl>(*this));
 
+  filter_manager_.streamInfo().setShouldSchemeMatchUpstream(
+      connection_manager.config_->shouldSchemeMatchUpstream());
+
   // TODO(chaoqin-li1123): can this be moved to the on demand filter?
   static const std::string route_factory = "envoy.route_config_update_requester.default";
   auto factory =

source/common/router/router.cc

@@ -124,7 +124,17 @@ uint64_t FilterUtility::percentageOfTimeout(const std::chrono::milliseconds resp
   return static_cast<uint64_t>(response_time.count() * TimeoutPrecisionFactor / timeout.count());
 }
 
-void FilterUtility::setUpstreamScheme(Http::RequestHeaderMap& headers, bool downstream_secure) {
+void FilterUtility::setUpstreamScheme(Http::RequestHeaderMap& headers, bool downstream_ssl,
+                                      bool upstream_ssl, bool use_upstream) {
+  if (use_upstream) {
+    if (upstream_ssl) {
+      headers.setReferenceScheme(Http::Headers::get().SchemeValues.Https);
+    } else {
+      headers.setReferenceScheme(Http::Headers::get().SchemeValues.Http);
+    }
+    return;
+  }
+
   if (Http::Utility::schemeIsValid(headers.getSchemeValue())) {
     return;
   }
@@ -137,7 +147,7 @@ void FilterUtility::setUpstreamScheme(Http::RequestHeaderMap& headers, bool down
     return;
   }
 
-  if (downstream_secure) {
+  if (downstream_ssl) {
     headers.setReferenceScheme(Http::Headers::get().SchemeValues.Https);
   } else {
     headers.setReferenceScheme(Http::Headers::get().SchemeValues.Http);
@@ -713,7 +723,9 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers,
   route_entry_->finalizeRequestHeaders(headers, callbacks_->streamInfo(),
                                        !config_->suppress_envoy_headers_);
   FilterUtility::setUpstreamScheme(
-      headers, callbacks_->streamInfo().downstreamAddressProvider().sslConnection() != nullptr);
+      headers, callbacks_->streamInfo().downstreamAddressProvider().sslConnection() != nullptr,
+      host->transportSocketFactory().sslCtx() != nullptr,
+      callbacks_->streamInfo().shouldSchemeMatchUpstream());
 
   // Ensure an http transport scheme is selected before continuing with decoding.
   ASSERT(headers.Scheme());

source/common/router/router.h

@@ -119,11 +119,13 @@ class FilterUtility {
 
   /**
    * Set the :scheme header using the best information available. In order this is
+   * - whether the upstream connection is using TLS if use_upstream is true
    * - existing scheme header if valid
    * - x-forwarded-proto header if valid
-   * - security of downstream connection
+   * - whether the downstream connection is using TLS
    */
-  static void setUpstreamScheme(Http::RequestHeaderMap& headers, bool downstream_secure);
+  static void setUpstreamScheme(Http::RequestHeaderMap& headers, bool downstream_ssl,
+                                bool upstream_ssl, bool use_upstream);
 
   /**
    * Determine whether a request should be shadowed.

source/common/stream_info/stream_info_impl.h

@@ -431,6 +431,12 @@ struct StreamInfoImpl : public StreamInfo {
     return downstream_transport_failure_reason_;
   }
 
+  bool shouldSchemeMatchUpstream() const override { return should_scheme_match_upstream_; }
+
+  void setShouldSchemeMatchUpstream(bool should_match_upstream) override {
+    should_scheme_match_upstream_ = should_match_upstream;
+  }
+
   bool shouldDrainConnectionUponCompletion() const override { return should_drain_connection_; }
 
   void setShouldDrainConnectionUponCompletion(bool should_drain) override {
@@ -483,6 +489,7 @@ struct StreamInfoImpl : public StreamInfo {
   BytesMeterSharedPtr downstream_bytes_meter_;
   bool is_shadow_{false};
   std::string downstream_transport_failure_reason_;
+  bool should_scheme_match_upstream_{false};
   bool should_drain_connection_{false};
 };
 

source/extensions/filters/network/http_connection_manager/config.cc

@@ -654,6 +654,13 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
 
   if (!config.scheme_header_transformation().scheme_to_overwrite().empty()) {
     scheme_to_set_ = config.scheme_header_transformation().scheme_to_overwrite();
+
+    if (config.scheme_header_transformation().match_upstream()) {
+      ENVOY_LOG(warn, "match_upstream and scheme_to_overwrite both set, using scheme_to_overwrite");
+    }
+    should_scheme_match_upstream_ = false;
+  } else {
+    should_scheme_match_upstream_ = config.scheme_header_transformation().match_upstream();
   }
 
   if (!config.server_name().empty()) {

source/extensions/filters/network/http_connection_manager/config.h

@@ -202,6 +202,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
     return server_transformation_;
   }
   const absl::optional<std::string>& schemeToSet() const override { return scheme_to_set_; }
+  bool shouldSchemeMatchUpstream() const override { return should_scheme_match_upstream_; }
   Http::ConnectionManagerStats& stats() override { return stats_; }
   Http::ConnectionManagerTracingStats& tracingStats() override { return tracing_stats_; }
   bool useRemoteAddress() const override { return use_remote_address_; }
@@ -314,6 +315,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
       HttpConnectionManagerProto::OVERWRITE};
   std::string server_name_;
   absl::optional<std::string> scheme_to_set_;
+  bool should_scheme_match_upstream_;
   Tracing::TracerSharedPtr tracer_{std::make_shared<Tracing::NullTracer>()};
   Http::TracingConnectionManagerConfigPtr tracing_config_;
   absl::optional<std::string> user_agent_;

source/extensions/health_checkers/grpc/health_checker_impl.cc

@@ -224,7 +224,7 @@ void GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::onInterval() {
       headers_message->headers(),
       // Here there is no downstream connection so scheme will be based on
       // upstream crypto
-      host_->transportSocketFactory().implementsSecureTransport());
+      false, host_->transportSocketFactory().implementsSecureTransport(), true);
 
   auto status = request_encoder_->encodeHeaders(headers_message->headers(), false);
   // Encoding will only fail if required headers are missing.

source/extensions/health_checkers/http/health_checker_impl.cc

@@ -268,7 +268,7 @@ void HttpHealthCheckerImpl::HttpActiveHealthCheckSession::onInterval() {
       *request_headers,
       // Here there is no downstream connection so scheme will be based on
       // upstream crypto
-      host_->transportSocketFactory().implementsSecureTransport());
+      false, host_->transportSocketFactory().implementsSecureTransport(), true);
   StreamInfo::StreamInfoImpl stream_info(protocol_, parent_.dispatcher_.timeSource(),
                                          local_connection_info_provider_,
                                          StreamInfo::FilterState::LifeSpan::FilterChain);

source/server/admin/admin.h

@@ -163,6 +163,7 @@ class AdminImpl : public Admin,
   OptRef<const Router::ScopeKeyBuilder> scopeKeyBuilder() override { return scope_key_builder_; }
   const std::string& serverName() const override { return Http::DefaultServerString::get(); }
   const absl::optional<std::string>& schemeToSet() const override { return scheme_; }
+  bool shouldSchemeMatchUpstream() const override { return scheme_match_upstream_; }
   HttpConnectionManagerProto::ServerHeaderTransformation
   serverHeaderTransformation() const override {
     return HttpConnectionManagerProto::OVERWRITE;
@@ -495,6 +496,7 @@ class AdminImpl : public Admin,
   const std::vector<Http::OriginalIPDetectionSharedPtr> detection_extensions_{};
   const std::vector<Http::EarlyHeaderMutationPtr> early_header_mutations_{};
   const absl::optional<std::string> scheme_{};
+  const bool scheme_match_upstream_ = false;
   const bool ignore_global_conn_limit_;
   std::unique_ptr<HttpConnectionManagerProto::ProxyStatusConfig> proxy_status_config_;
   const Http::HeaderValidatorFactoryPtr header_validator_factory_;

test/common/http/conn_manager_impl_fuzz_test.cc

@@ -185,6 +185,7 @@ class FuzzConfig : public ConnectionManagerConfig {
     return server_transformation_;
   }
   const absl::optional<std::string>& schemeToSet() const override { return scheme_; }
+  bool shouldSchemeMatchUpstream() const override { return scheme_match_upstream_; }
   ConnectionManagerStats& stats() override { return stats_; }
   ConnectionManagerTracingStats& tracingStats() override { return tracing_stats_; }
   bool useRemoteAddress() const override { return use_remote_address_; }
@@ -265,6 +266,7 @@ class FuzzConfig : public ConnectionManagerConfig {
   HttpConnectionManagerProto::ServerHeaderTransformation server_transformation_{
       HttpConnectionManagerProto::OVERWRITE};
   absl::optional<std::string> scheme_;
+  bool scheme_match_upstream_{};
   Stats::IsolatedStoreImpl fake_stats_;
   ConnectionManagerStats stats_;
   ConnectionManagerTracingStats tracing_stats_;

test/common/http/conn_manager_impl_test_2.cc

@@ -4210,5 +4210,48 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamTimingsRecordWhenRequestHeaderPr
   filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose);
 }
 
+TEST_F(HttpConnectionManagerImplTest, PassMatchUpstreamSchemeHintToStreamInfo) {
+  setup(/*ssl=*/false, /*server_name=*/"", /*tracing=*/false);
+  scheme_match_upstream_ = true;
+
+  // Store the basic request encoder during filter chain setup.
+  std::shared_ptr<MockStreamDecoderFilter> filter(new NiceMock<MockStreamDecoderFilter>());
+
+  EXPECT_CALL(*filter, decodeHeaders(_, true))
+      .WillOnce(Invoke([&](RequestHeaderMap&, bool) -> FilterHeadersStatus {
+        EXPECT_TRUE(filter->callbacks_->streamInfo().shouldSchemeMatchUpstream());
+        return FilterHeadersStatus::StopIteration;
+      }));
+
+  EXPECT_CALL(*filter, setDecoderFilterCallbacks(_));
+
+  EXPECT_CALL(filter_factory_, createFilterChain(_))
+      .WillOnce(Invoke([&](FilterChainManager& manager) -> bool {
+        auto factory = createDecoderFilterFactoryCb(filter);
+        manager.applyFilterFactoryCb({}, factory);
+        return true;
+      }));
+
+  EXPECT_CALL(filter_callbacks_.connection_.dispatcher_, deferredDelete_(_));
+
+  EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status {
+    decoder_ = &conn_manager_->newStream(response_encoder_);
+
+    // Send request to be passed on through filter chain to check that
+    // filters have the correct hint passed through the callbacks
+    RequestHeaderMapPtr headers{
+        new TestRequestHeaderMapImpl{{":authority", "host"}, {":path", "/"}, {":method", "GET"}}};
+    decoder_->decodeHeaders(std::move(headers), true);
+    return Http::okStatus();
+  }));
+
+  // Kick off the incoming data.
+  Buffer::OwnedImpl fake_input{};
+  conn_manager_->onData(fake_input, false);
+
+  // Clean up.
+  filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose);
+}
+
 } // namespace Http
 } // namespace Envoy

test/common/http/conn_manager_impl_test_base.cc

@@ -83,6 +83,7 @@ class ConnectionManagerConfigProxyObject : public ConnectionManagerConfig {
     return parent_.serverHeaderTransformation();
   }
   const absl::optional<std::string>& schemeToSet() const override { return parent_.schemeToSet(); }
+  bool shouldSchemeMatchUpstream() const override { return parent_.shouldSchemeMatchUpstream(); }
   ConnectionManagerStats& stats() override { return parent_.stats(); }
   ConnectionManagerTracingStats& tracingStats() override { return parent_.tracingStats(); }
   bool useRemoteAddress() const override { return parent_.useRemoteAddress(); }

test/common/http/conn_manager_impl_test_base.h

@@ -120,6 +120,7 @@ class HttpConnectionManagerImplMixin : public ConnectionManagerConfig {
     return server_transformation_;
   }
   const absl::optional<std::string>& schemeToSet() const override { return scheme_; }
+  bool shouldSchemeMatchUpstream() const override { return scheme_match_upstream_; }
   ConnectionManagerStats& stats() override { return stats_; }
   ConnectionManagerTracingStats& tracingStats() override { return tracing_stats_; }
   bool useRemoteAddress() const override { return use_remote_address_; }
@@ -235,6 +236,7 @@ class HttpConnectionManagerImplMixin : public ConnectionManagerConfig {
   HttpConnectionManagerProto::ServerHeaderTransformation server_transformation_{
       HttpConnectionManagerProto::OVERWRITE};
   absl::optional<std::string> scheme_;
+  bool scheme_match_upstream_{false};
   Network::Address::Ipv4Instance local_address_{"127.0.0.1"};
   bool use_remote_address_{true};
   Http::DefaultInternalAddressConfig internal_address_config_;

test/common/router/router_test.cc

@@ -5806,31 +5806,44 @@ TEST(RouterFilterUtilityTest, FinalTimeoutSupressEnvoyHeaders) {
 TEST(RouterFilterUtilityTest, SetUpstreamScheme) {
   TestScopedRuntime scoped_runtime;
 
-  // With no scheme and x-forwarded-proto, set scheme based on encryption level
+  // With upstream scheme, set scheme based on upstream encryption level
   {
     Http::TestRequestHeaderMapImpl headers;
-    FilterUtility::setUpstreamScheme(headers, false);
+    FilterUtility::setUpstreamScheme(headers, false, false, true);
     EXPECT_EQ("http", headers.get_(":scheme"));
   }
   {
     Http::TestRequestHeaderMapImpl headers;
-    FilterUtility::setUpstreamScheme(headers, true);
+    FilterUtility::setUpstreamScheme(headers, false, true, true);
+    EXPECT_EQ("https", headers.get_(":scheme"));
+  }
+
+  // With no scheme and x-forwarded-proto, set scheme based on downstream
+  // encryption level
+  {
+    Http::TestRequestHeaderMapImpl headers;
+    FilterUtility::setUpstreamScheme(headers, false, false, false);
+    EXPECT_EQ("http", headers.get_(":scheme"));
+  }
+  {
+    Http::TestRequestHeaderMapImpl headers;
+    FilterUtility::setUpstreamScheme(headers, true, false, false);
     EXPECT_EQ("https", headers.get_(":scheme"));
   }
 
   // With invalid x-forwarded-proto, still use scheme.
   {
     Http::TestRequestHeaderMapImpl headers;
     headers.setForwardedProto("foo");
-    FilterUtility::setUpstreamScheme(headers, true);
+    FilterUtility::setUpstreamScheme(headers, true, false, false);
     EXPECT_EQ("https", headers.get_(":scheme"));
   }
 
   // Use valid x-forwarded-proto.
   {
     Http::TestRequestHeaderMapImpl headers;
     headers.setForwardedProto(Http::Headers::get().SchemeValues.Http);
-    FilterUtility::setUpstreamScheme(headers, true);
+    FilterUtility::setUpstreamScheme(headers, true, false, false);
     EXPECT_EQ("http", headers.get_(":scheme"));
   }
 
@@ -5839,7 +5852,7 @@ TEST(RouterFilterUtilityTest, SetUpstreamScheme) {
     Http::TestRequestHeaderMapImpl headers;
     headers.setScheme(Http::Headers::get().SchemeValues.Https);
     headers.setForwardedProto(Http::Headers::get().SchemeValues.Http);
-    FilterUtility::setUpstreamScheme(headers, false);
+    FilterUtility::setUpstreamScheme(headers, false, false, false);
     EXPECT_EQ("https", headers.get_(":scheme"));
   }
 }
@@ -6639,5 +6652,26 @@ TEST_F(RouterTest, RequestWithUpstreamOverrideHost) {
   router_->onDestroy();
 }
 
+TEST_F(RouterTest, OverwriteSchemeWithUpstreamTransportProtocol) {
+  EXPECT_CALL(callbacks_.stream_info_, shouldSchemeMatchUpstream()).WillRepeatedly(Return(true));
+  EXPECT_CALL(cm_.thread_local_cluster_, httpConnPool(_, absl::optional<Http::Protocol>(), _));
+  EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_, newStream(_, _, _))
+      .WillOnce(Return(&cancellable_));
+  expectResponseTimerCreate();
+
+  Http::TestRequestHeaderMapImpl headers;
+  HttpTestUtility::addDefaultHeaders(headers);
+  headers.setScheme("https");
+  router_->decodeHeaders(headers, true);
+  EXPECT_EQ(headers.getSchemeValue(), "http");
+
+  // When the router filter gets reset we should cancel the pool request.
+  EXPECT_CALL(cancellable_, cancel(_));
+  router_->onDestroy();
+  EXPECT_TRUE(verifyHostUpstreamStats(0, 0));
+  EXPECT_EQ(0U,
+            callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value());
+}
+
 } // namespace Router
 } // namespace Envoy