conn_manager: allow to remove port from host header

api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto

@@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // HTTP connection manager :ref:`configuration overview <config_http_conn_man>`.
 // [#extension: envoy.filters.network.http_connection_manager]
 
-// [#next-free-field: 39]
+// [#next-free-field: 40]
 message HttpConnectionManager {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager";
@@ -511,6 +511,14 @@ message HttpConnectionManager {
   // host will not be overwritten by the HTTP Connection Manager. The default behaviour is
   // to overwrite the `date` header unconditionally.
   bool preserve_upstream_date = 38;
+
+  // Determines if the port part should be removed from host/authority header before any processing
+  // of request by HTTP filters or routing. The port would be removed only if it is equal to the :ref:`listener's<envoy_api_field_config.listener.v3.Listener.address>`
+  // local port and request method is not CONNECT. This affects the upstream host header as well.
+  // Without setting this option, incoming requests with host `example:443` will not match against
+  // route with :ref:`domains<envoy_api_field_config.route.v3.VirtualHost.domains>` match set to `example`. Defaults to `false`. Note that port removal is not part
+  // of `HTTP spec <https://tools.ietf.org/html/rfc3986>` and is provided for convenience.
+  bool strip_matching_host_port = 39;
 }
 
 message Rds {

api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto

@@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO
 // HTTP connection manager :ref:`configuration overview <config_http_conn_man>`.
 // [#extension: envoy.filters.network.http_connection_manager]
 
-// [#next-free-field: 39]
+// [#next-free-field: 40]
 message HttpConnectionManager {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager";
@@ -511,6 +511,14 @@ message HttpConnectionManager {
   // host will not be overwritten by the HTTP Connection Manager. The default behaviour is
   // to overwrite the `date` header unconditionally.
   bool preserve_upstream_date = 38;
+
+  // Determines if the port part should be removed from host/authority header before any processing
+  // of request by HTTP filters or routing. The port would be removed only if it is equal to the :ref:`listener's<envoy_api_field_config.listener.v4alpha.Listener.address>`
+  // local port and request method is not CONNECT. This affects the upstream host header as well.
+  // Without setting this option, incoming requests with host `example:443` will not match against
+  // route with :ref:`domains<envoy_api_field_config.route.v4alpha.VirtualHost.domains>` match set to `example`. Defaults to `false`. Note that port removal is not part
+  // of `HTTP spec <https://tools.ietf.org/html/rfc3986>` and is provided for convenience.
+  bool strip_matching_host_port = 39;
 }
 
 message Rds {

docs/root/version_history/current.rst

@@ -19,6 +19,7 @@ Changes
   `google.api.HttpBody <https://github.com/googleapis/googleapis/blob/master/google/api/httpbody.proto>`_.
 * gzip filter: added option to set zlib's next output buffer size.
 * health checks: allow configuring health check transport sockets by specifying :ref:`transport socket match criteria <envoy_v3_api_field_config.core.v3.HealthCheck.transport_socket_match_criteria>`.
+* http: added :ref:`stripping port from host header <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.strip_matching_host_port>` support.
 * http: fixed a bug where in some cases slash was moved from path to query string when :ref:`merging of adjacent slashes<envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.merge_slashes>` is enabled.
 * http: fixed a bug where the upgrade header was not cleared on responses to non-upgrade requests.
   Can be reverted temporarily by setting runtime feature `envoy.reloadable_features.fix_upgrade_response` to false.

source/common/http/conn_manager_config.h

@@ -424,6 +424,11 @@ class ConnectionManagerConfig {
    */
   virtual bool shouldMergeSlashes() const PURE;
 
+  /**
+   * @return if the HttpConnectionManager should remove the port from host/authority header
+   */
+  virtual bool shouldStripMatchingPort() const PURE;
+
   /**
    * @return the action HttpConnectionManager should take when receiving client request
    * headers containing underscore characters.

source/common/http/conn_manager_impl.cc

@@ -747,6 +747,14 @@ const Network::Connection* ConnectionManagerImpl::ActiveStream::connection() {
   return &connection_manager_.read_callbacks_->connection();
 }
 
+uint32_t ConnectionManagerImpl::ActiveStream::localPort() {
+  auto ip = connection()->localAddress()->ip();
+  if (ip == nullptr) {
+    return 0;
+  }
+  return ip->port();
+}
+
 // Ordering in this function is complicated, but important.
 //
 // We want to do minimal work before selecting route and creating a filter
@@ -896,6 +904,9 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he
     return;
   }
 
+  ConnectionManagerUtility::maybeNormalizeHost(*request_headers_, connection_manager_.config_,
+                                               localPort());
+
   if (protocol == Protocol::Http11 && request_headers_->Connection() &&
       absl::EqualsIgnoreCase(request_headers_->Connection()->value().getStringView(),
                              Http::Headers::get().ConnectionValues.Close)) {

source/common/http/conn_manager_impl.h

@@ -667,12 +667,15 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
     void onIdleTimeout();
     // Reset per-stream idle timer.
     void resetIdleTimer();
-    // Per-stream request timeout callback
+    // Per-stream request timeout callback.
     void onRequestTimeout();
     // Per-stream alive duration reached.
     void onStreamMaxDurationReached();
     bool hasCachedRoute() { return cached_route_.has_value() && cached_route_.value(); }
 
+    // Return local port of the connection.
+    uint32_t localPort();
+
     friend std::ostream& operator<<(std::ostream& os, const ActiveStream& s) {
       s.dumpState(os);
       return os;

source/common/http/conn_manager_utility.cc

@@ -9,6 +9,7 @@
 #include "common/access_log/access_log_formatter.h"
 #include "common/common/empty_string.h"
 #include "common/common/utility.h"
+#include "common/http/header_utility.h"
 #include "common/http/headers.h"
 #include "common/http/http1/codec_impl.h"
 #include "common/http/http2/codec_impl.h"
@@ -420,5 +421,13 @@ bool ConnectionManagerUtility::maybeNormalizePath(RequestHeaderMap& request_head
   return is_valid_path;
 }
 
+void ConnectionManagerUtility::maybeNormalizeHost(RequestHeaderMap& request_headers,
+                                                  const ConnectionManagerConfig& config,
+                                                  uint32_t port) {
+  if (config.shouldStripMatchingPort()) {
+    HeaderUtility::stripPortFromHost(request_headers, port);
+  }
+}
+
 } // namespace Http
 } // namespace Envoy

source/common/http/conn_manager_utility.h

@@ -69,6 +69,9 @@ class ConnectionManagerUtility {
   static bool maybeNormalizePath(RequestHeaderMap& request_headers,
                                  const ConnectionManagerConfig& config);
 
+  static void maybeNormalizeHost(RequestHeaderMap& request_headers,
+                                 const ConnectionManagerConfig& config, uint32_t port);
+
   /**
    * Mutate request headers if request needs to be traced.
    */

source/common/http/header_utility.cc

@@ -180,6 +180,39 @@ bool HeaderUtility::isEnvoyInternalRequest(const RequestHeaderMap& headers) {
          internal_request_header->value() == Headers::get().EnvoyInternalRequestValues.True;
 }
 
+void HeaderUtility::stripPortFromHost(RequestHeaderMap& headers, uint32_t listener_port) {
+
+  if (headers.Method() &&
+      headers.Method()->value().getStringView() == Http::Headers::get().MethodValues.Connect) {
+    // According to RFC 2817 Connect method should have port part in host header.
+    // In this case we won't strip it even if configured to do so.
+    return;
+  }
+  const auto original_host = headers.Host()->value().getStringView();
+  const absl::string_view::size_type port_start = original_host.rfind(':');
+  if (port_start == absl::string_view::npos) {
+    return;
+  }
+  // According to RFC3986 v6 address is always enclosed in "[]". section 3.2.2.
+  const auto v6_end_index = original_host.rfind("]");
+  if (v6_end_index == absl::string_view::npos || v6_end_index < port_start) {
+    if ((port_start + 1) > original_host.size()) {
+      return;
+    }
+    const absl::string_view port_str = original_host.substr(port_start + 1);
+    uint32_t port = 0;
+    if (!absl::SimpleAtoi(port_str, &port)) {
+      return;
+    }
+    if (port != listener_port) {
+      // We would strip ports only if they are the same, as local port of the listener.
+      return;
+    }
+    const absl::string_view host = original_host.substr(0, port_start);
+    headers.setHost(host);
+  }
+}
+
 absl::optional<std::reference_wrapper<const absl::string_view>>
 HeaderUtility::requestHeadersValid(const RequestHeaderMap& headers) {
   // Make sure the host is valid.

source/common/http/header_utility.h

@@ -135,6 +135,11 @@ class HeaderUtility {
    */
   static absl::optional<std::reference_wrapper<const absl::string_view>>
   requestHeadersValid(const RequestHeaderMap& headers);
+
+  /**
+   * @brief Remove the port part from host/authority header if it is equal to provided port
+   */
+  static void stripPortFromHost(RequestHeaderMap& headers, uint32_t listener_port);
 };
 } // namespace Http
 } // namespace Envoy

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

@@ -216,6 +216,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
                                                       0))),
 #endif
       merge_slashes_(config.merge_slashes()),
+      strip_matching_port_(config.strip_matching_host_port()),
       headers_with_underscores_action_(
           config.common_http_protocol_options().headers_with_underscores_action()),
       preserve_upstream_date_(config.preserve_upstream_date()) {

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

@@ -157,6 +157,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   const Http::Http1Settings& http1Settings() const override { return http1_settings_; }
   bool shouldNormalizePath() const override { return normalize_path_; }
   bool shouldMergeSlashes() const override { return merge_slashes_; }
+  bool shouldStripMatchingPort() const override { return strip_matching_port_; }
   envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
   headersWithUnderscoresAction() const override {
     return headers_with_underscores_action_;
@@ -225,6 +226,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   std::chrono::milliseconds delayed_close_timeout_;
   const bool normalize_path_;
   const bool merge_slashes_;
+  const bool strip_matching_port_;
   const envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
       headers_with_underscores_action_;
   const bool preserve_upstream_date_;

source/server/http/admin.h

@@ -164,6 +164,7 @@ class AdminImpl : public Admin,
   const Http::Http1Settings& http1Settings() const override { return http1_settings_; }
   bool shouldNormalizePath() const override { return true; }
   bool shouldMergeSlashes() const override { return true; }
+  bool shouldStripMatchingPort() const override { return false; }
   envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
   headersWithUnderscoresAction() const override {
     return envoy::config::core::v3::HttpProtocolOptions::ALLOW;

test/common/http/conn_manager_impl_fuzz_test.cc

@@ -156,6 +156,7 @@ class FuzzConfig : public ConnectionManagerConfig {
   const Http::Http1Settings& http1Settings() const override { return http1_settings_; }
   bool shouldNormalizePath() const override { return false; }
   bool shouldMergeSlashes() const override { return false; }
+  bool shouldStripMatchingPort() const override { return false; }
   envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
   headersWithUnderscoresAction() const override {
     return envoy::config::core::v3::HttpProtocolOptions::ALLOW;

test/common/http/conn_manager_impl_test.cc

@@ -126,7 +126,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan
     ON_CALL(filter_callbacks_.connection_, ssl()).WillByDefault(Return(ssl_connection_));
     ON_CALL(Const(filter_callbacks_.connection_), ssl()).WillByDefault(Return(ssl_connection_));
     filter_callbacks_.connection_.local_address_ =
-        std::make_shared<Network::Address::Ipv4Instance>("127.0.0.1");
+        std::make_shared<Network::Address::Ipv4Instance>("127.0.0.1", 443);
     filter_callbacks_.connection_.remote_address_ =
         std::make_shared<Network::Address::Ipv4Instance>("0.0.0.0");
     conn_manager_ = std::make_unique<ConnectionManagerImpl>(
@@ -348,6 +348,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan
   const Http::Http1Settings& http1Settings() const override { return http1_settings_; }
   bool shouldNormalizePath() const override { return normalize_path_; }
   bool shouldMergeSlashes() const override { return merge_slashes_; }
+  bool shouldStripMatchingPort() const override { return strip_matching_port_; }
   RequestIDExtensionSharedPtr requestIDExtension() override { return request_id_extension_; }
   envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
   headersWithUnderscoresAction() const override {
@@ -410,6 +411,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan
   Http::Http1Settings http1_settings_;
   bool normalize_path_ = false;
   bool merge_slashes_ = false;
+  bool strip_matching_port_ = false;
   envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
       headers_with_underscores_action_ = envoy::config::core::v3::HttpProtocolOptions::ALLOW;
   NiceMock<Network::MockClientConnection> upstream_conn_; // for websocket tests
@@ -899,6 +901,80 @@ TEST_F(HttpConnectionManagerImplTest, RouteShouldUseSantizedPath) {
   conn_manager_->onData(fake_input, false);
 }
 
+// Filters observe host header w/o port's part when port's removal is configured
+TEST_F(HttpConnectionManagerImplTest, FilterShouldUseNormalizedHost) {
+  setup(false, "");
+  // Enable port removal
+  strip_matching_port_ = true;
+  const std::string original_host = "host:443";
+  const std::string normalized_host = "host";
+
+  auto* filter = new MockStreamFilter();
+
+  EXPECT_CALL(filter_factory_, createFilterChain(_))
+      .WillOnce(Invoke([&](FilterChainFactoryCallbacks& callbacks) -> void {
+        callbacks.addStreamDecoderFilter(StreamDecoderFilterSharedPtr{filter});
+      }));
+
+  EXPECT_CALL(*filter, decodeHeaders(_, true))
+      .WillRepeatedly(Invoke([&](RequestHeaderMap& header_map, bool) -> FilterHeadersStatus {
+        EXPECT_EQ(normalized_host, header_map.Host()->value().getStringView());
+        return FilterHeadersStatus::StopIteration;
+      }));
+
+  EXPECT_CALL(*filter, setDecoderFilterCallbacks(_));
+
+  EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status {
+    RequestDecoder* decoder = &conn_manager_->newStream(response_encoder_);
+    RequestHeaderMapPtr headers{new TestRequestHeaderMapImpl{
+        {":authority", original_host}, {":path", "/"}, {":method", "GET"}}};
+    decoder->decodeHeaders(std::move(headers), true);
+    return Http::okStatus();
+  }));
+
+  // Kick off the incoming data.
+  Buffer::OwnedImpl fake_input("1234");
+  conn_manager_->onData(fake_input, false);
+}
+
+// The router observes host header w/o port, not the original host, when
+// remove_port is configured
+TEST_F(HttpConnectionManagerImplTest, RouteShouldUseNormalizedHost) {
+  setup(false, "");
+  // Enable port removal
+  strip_matching_port_ = true;
+  const std::string original_host = "host:443";
+  const std::string normalized_host = "host";
+
+  EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status {
+    RequestDecoder* decoder = &conn_manager_->newStream(response_encoder_);
+    RequestHeaderMapPtr headers{new TestRequestHeaderMapImpl{
+        {":authority", original_host}, {":path", "/"}, {":method", "GET"}}};
+    decoder->decodeHeaders(std::move(headers), true);
+    return Http::okStatus();
+  }));
+
+  const std::string fake_cluster_name = "fake_cluster";
+
+  std::shared_ptr<Upstream::MockThreadLocalCluster> fake_cluster =
+      std::make_shared<NiceMock<Upstream::MockThreadLocalCluster>>();
+  std::shared_ptr<Router::MockRoute> route = std::make_shared<NiceMock<Router::MockRoute>>();
+  EXPECT_CALL(route->route_entry_, clusterName()).WillRepeatedly(ReturnRef(fake_cluster_name));
+
+  EXPECT_CALL(*route_config_provider_.route_config_, route(_, _, _))
+      .WillOnce(Invoke(
+          [&](const Http::RequestHeaderMap& header_map, const StreamInfo::StreamInfo&, uint64_t) {
+            EXPECT_EQ(normalized_host, header_map.Host()->value().getStringView());
+            return route;
+          }));
+  EXPECT_CALL(filter_factory_, createFilterChain(_))
+      .WillOnce(Invoke([&](FilterChainFactoryCallbacks&) -> void {}));
+
+  // Kick off the incoming data.
+  Buffer::OwnedImpl fake_input("1234");
+  conn_manager_->onData(fake_input, false);
+}
+
 TEST_F(HttpConnectionManagerImplTest, PreserveUpstreamDateDisabledDateNotSet) {
   setup(false, "");
   setUpEncoderAndDecoder(false, false);