ext_authz: add knob enforce_response_header_limits

api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto

@@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // External Authorization :ref:`configuration overview <config_http_filters_ext_authz>`.
 // [#extension: envoy.filters.http.ext_authz]
 
-// [#next-free-field: 31]
+// [#next-free-field: 32]
 message ExtAuthz {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.http.ext_authz.v3.ExtAuthz";
@@ -322,6 +322,19 @@ message ExtAuthz {
   // If this field is not set or is set to 0, no truncation will occur, and the entire
   // denied response body will be forwarded.
   uint32 max_denied_response_body_bytes = 30;
+
+  // When set to ``true``, the filter will enforce the response header map's count and size limits
+  // by dropping headers added by the external auth service. Enabling this may break applications
+  // relying on those headers!
+  //
+  // When set to ``false``, the filter will ignore the response header map's limits and add / set
+  // all response headers as specified by the external auth service.
+  //
+  // Recommendation: enable if the external authorization service is not trusted. Otherwise, leave
+  // false.
+  //
+  // Defaults to false.
+  bool enforce_response_header_limits = 31;
 }
 
 // Configuration for buffering the request data.

changelogs/current.yaml

@@ -150,5 +150,11 @@ new_features:
     Added QUIC protocol option :ref:`max_sessions_per_event_loop
     <envoy_v3_api_field_config.listener.v3.QuicProtocolOptions.max_sessions_per_event_loop>` to limit the maximum
     number of new QUIC sessions created per event loop. The default is 16, preserving the previous hardcoded limit.
+- area: ext_authz
+  change: |
+    Add a new configuration field to the http ext authz filter
+    :ref:`enforce_response_header_limits <envoy_v3_api_field_extensions.filters.http.ext_authz.v3.ExtAuthz.enforce_response_header_limits>`
+    that allows admins to enable / disable the behavior of dropping response headers once the header
+    map count / size constraints have been reached.
 
 deprecated:

source/extensions/filters/http/ext_authz/ext_authz.cc

@@ -100,6 +100,7 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::ext_authz::v3
       filter_metadata_(config.has_filter_metadata() ? absl::optional(config.filter_metadata())
                                                     : absl::nullopt),
       emit_filter_state_stats_(config.emit_filter_state_stats()),
+      enforce_response_header_limits_(config.enforce_response_header_limits()),
       filter_enabled_(config.has_filter_enabled()
                           ? absl::optional<Runtime::FractionalPercent>(
                                 Runtime::FractionalPercent(config.filter_enabled(), runtime_))
@@ -487,7 +488,7 @@ Http::FilterHeadersStatus Filter::encodeHeaders(Http::ResponseHeaderMap& headers
     ENVOY_STREAM_LOG(
         trace, "ext_authz filter added header(s) to the encoded response:", *encoder_callbacks_);
     for (const auto& [key, value] : response_headers_to_add_) {
-      if (headers.size() >= headers.maxHeadersCount()) {
+      if (config_->enforceResponseHeaderLimits() && headers.size() >= headers.maxHeadersCount()) {
         omitted_response_headers = true;
         break;
       }
@@ -500,7 +501,8 @@ Http::FilterHeadersStatus Filter::encodeHeaders(Http::ResponseHeaderMap& headers
     ENVOY_STREAM_LOG(
         trace, "ext_authz filter set header(s) to the encoded response:", *encoder_callbacks_);
     for (const auto& [key, value] : response_headers_to_set_) {
-      if (headers.get(key).empty() && headers.size() >= headers.maxHeadersCount()) {
+      if (config_->enforceResponseHeaderLimits() && headers.get(key).empty() &&
+          headers.size() >= headers.maxHeadersCount()) {
         omitted_response_headers = true;
         // Continue because there could be other existing headers that can be set without increasing
         // the number of headers.
@@ -515,7 +517,7 @@ Http::FilterHeadersStatus Filter::encodeHeaders(Http::ResponseHeaderMap& headers
     for (const auto& [key, value] : response_headers_to_add_if_absent_) {
       ENVOY_STREAM_LOG(trace, "'{}':'{}'", *encoder_callbacks_, key.get(), value);
       if (auto header_entry = headers.get(key); header_entry.empty()) {
-        if (headers.size() >= headers.maxHeadersCount()) {
+        if (config_->enforceResponseHeaderLimits() && headers.size() >= headers.maxHeadersCount()) {
           omitted_response_headers = true;
           break;
         }
@@ -978,7 +980,8 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) {
           // Then set all of the requested headers, allowing the same header to be set multiple
           // times, e.g. `Set-Cookie`.
           for (const auto& [key, value] : headers) {
-            if (response_headers.size() >= response_headers.maxHeadersCount()) {
+            if (config_->enforceResponseHeaderLimits() &&
+                response_headers.size() >= response_headers.maxHeadersCount()) {
               stats_.omitted_response_headers_.inc();
               ENVOY_LOG_EVERY_POW_2(
                   warn,

source/extensions/filters/http/ext_authz/ext_authz.h

@@ -208,6 +208,8 @@ class FilterConfig {
 
   bool emitFilterStateStats() const { return emit_filter_state_stats_; }
 
+  bool enforceResponseHeaderLimits() const { return enforce_response_header_limits_; }
+
   bool chargeClusterResponseStats() const { return charge_cluster_response_stats_; }
 
   const Filters::Common::ExtAuthz::MatcherSharedPtr& allowedHeadersMatcher() const {
@@ -261,6 +263,7 @@ class FilterConfig {
   LabelsMap destination_labels_;
   const absl::optional<Protobuf::Struct> filter_metadata_;
   const bool emit_filter_state_stats_;
+  const bool enforce_response_header_limits_;
 
   const absl::optional<Runtime::FractionalPercent> filter_enabled_;
   const absl::optional<Matchers::MetadataMatcher> filter_enabled_metadata_;

test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc

@@ -43,9 +43,10 @@ struct GrpcInitializeConfigOpts {
   uint32_t max_denied_response_body_bytes = 0;
   // In some tests a request is never sent. If a request is never sent, stats are not set. In those
   // tests, we need to be able to override this to false.
-  absl::optional<bool> expect_stats_override;
+  absl::optional<bool> expect_stats_override = absl::nullopt;
   // In timeout tests we expect zero response bytes.
   bool stats_expect_response_bytes = true;
+  bool enforce_response_header_limits = false;
 };
 
 struct WaitForSuccessfulUpstreamResponseOpts {
@@ -143,6 +144,10 @@ class ExtAuthzGrpcIntegrationTest
              .mutable_string_value() = "bar";
       }
 
+      if (opts.enforce_response_header_limits) {
+        proto_config_.set_enforce_response_header_limits(true);
+      }
+
       // Add labels and verify they are passed.
       std::map<std::string, std::string> labels;
       labels["label_1"] = "value_1";
@@ -2100,7 +2105,7 @@ TEST_P(ExtAuthzGrpcIntegrationTest, EncodeHeadersToAddExceedsLimit) {
         hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(100);
       });
 
-  initializeConfig();
+  initializeConfig({.enforce_response_header_limits = true});
   setDownstreamProtocol(Http::CodecType::HTTP2);
   HttpIntegrationTest::initialize();
 
@@ -2144,7 +2149,7 @@ TEST_P(ExtAuthzGrpcIntegrationTest, EncodeHeadersToSetExceedsLimit) {
         hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(100);
       });
 
-  initializeConfig();
+  initializeConfig({.enforce_response_header_limits = true});
   setDownstreamProtocol(Http::CodecType::HTTP2);
   HttpIntegrationTest::initialize();
 
@@ -2195,7 +2200,7 @@ TEST_P(ExtAuthzGrpcIntegrationTest, EncodeHeadersToAppendIfAbsentExceedsLimit) {
         hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(100);
       });
 
-  initializeConfig();
+  initializeConfig({.enforce_response_header_limits = true});
   setDownstreamProtocol(Http::CodecType::HTTP2);
   HttpIntegrationTest::initialize();
 

test/extensions/filters/http/ext_authz/ext_authz_test.cc

@@ -5281,6 +5281,7 @@ TEST_F(HttpFilterTest, EncodeHeadersToAddExceedsLimit) {
   grpc_service:
     envoy_grpc:
       cluster_name: "ext_authz_server"
+  enforce_response_header_limits: true
   )EOF");
 
   Filters::Common::ExtAuthz::Response response{};
@@ -5313,6 +5314,45 @@ TEST_F(HttpFilterTest, EncodeHeadersToAddExceedsLimit) {
   EXPECT_EQ(1U, config_->stats().omitted_response_headers_.value());
 }
 
+TEST_F(HttpFilterTest, EncodeHeadersToAddExceedsLimitDisabled) {
+  InSequence s;
+
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_authz_server"
+  )EOF");
+
+  Filters::Common::ExtAuthz::Response response{};
+  response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
+  response.response_headers_to_add.push_back({"key1", "value1"});
+  response.response_headers_to_add.push_back({"key2", "value2"});
+  response.response_headers_to_add.push_back({"key3", "value3"});
+
+  prepareCheck();
+
+  EXPECT_CALL(*client_, check(_, _, _, _))
+      .WillOnce(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks,
+                           const envoy::service::auth::v3::CheckRequest&, Tracing::Span&,
+                           const StreamInfo::StreamInfo&) -> void {
+        callbacks.onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
+      }));
+
+  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, true));
+
+  Http::TestResponseHeaderMapImpl response_headers(
+      {{":status", "200"}, {"existing-header", "value"}}, /*max_headers_kb=*/99999,
+      /*max_headers_count=*/3);
+
+  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers, false));
+
+  EXPECT_EQ(response_headers.size(), 5);
+  EXPECT_TRUE(response_headers.has("key1"));
+  EXPECT_TRUE(response_headers.has("key2"));
+  EXPECT_TRUE(response_headers.has("key3"));
+  EXPECT_EQ(0U, config_->stats().omitted_response_headers_.value());
+}
+
 // Verifies that the filter stops adding new headers from `response_headers_to_set` once the header
 // limit is reached, but still allows overwriting existing ones.
 TEST_F(HttpFilterTest, EncodeHeadersToSetExceedsLimit) {
@@ -5322,6 +5362,7 @@ TEST_F(HttpFilterTest, EncodeHeadersToSetExceedsLimit) {
   grpc_service:
     envoy_grpc:
       cluster_name: "ext_authz_server"
+  enforce_response_header_limits: true
   )EOF");
 
   Filters::Common::ExtAuthz::Response response{};
@@ -5357,6 +5398,49 @@ TEST_F(HttpFilterTest, EncodeHeadersToSetExceedsLimit) {
   EXPECT_EQ(1U, config_->stats().omitted_response_headers_.value());
 }
 
+TEST_F(HttpFilterTest, EncodeHeadersToSetExceedsLimitDisabled) {
+  InSequence s;
+
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_authz_server"
+  enforce_response_header_limits: false
+  )EOF");
+
+  Filters::Common::ExtAuthz::Response response{};
+  response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
+  response.response_headers_to_set.push_back({"existing-header-to-overwrite", "new-value"});
+  response.response_headers_to_set.push_back({"new-header-to-add", "value"});
+  response.response_headers_to_set.push_back({"another-new-header", "value"});
+
+  prepareCheck();
+
+  EXPECT_CALL(*client_, check(_, _, _, _))
+      .WillOnce(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks,
+                           const envoy::service::auth::v3::CheckRequest&, Tracing::Span&,
+                           const StreamInfo::StreamInfo&) -> void {
+        callbacks.onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
+      }));
+
+  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, true));
+
+  Http::TestResponseHeaderMapImpl response_headers(
+      {{":status", "200"}, {"existing-header-to-overwrite", "old-value"}}, /*max_headers_kb=*/99999,
+      /*max_headers_count=*/2);
+
+  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers, false));
+
+  EXPECT_EQ(response_headers.size(), 4);
+  EXPECT_EQ(response_headers.get(LowerCaseString("existing-header-to-overwrite"))[0]
+                ->value()
+                .getStringView(),
+            "new-value");
+  EXPECT_TRUE(response_headers.has("new-header-to-add"));
+  EXPECT_TRUE(response_headers.has("another-new-header"));
+  EXPECT_EQ(0U, config_->stats().omitted_response_headers_.value());
+}
+
 // Verifies that the filter stops adding headers from `response_headers_to_add_if_absent` once the
 // header limit is reached.
 TEST_F(HttpFilterTest, EncodeHeadersToAddIfAbsentExceedsLimit) {
@@ -5366,6 +5450,7 @@ TEST_F(HttpFilterTest, EncodeHeadersToAddIfAbsentExceedsLimit) {
   grpc_service:
     envoy_grpc:
       cluster_name: "ext_authz_server"
+  enforce_response_header_limits: true
   )EOF");
 
   Filters::Common::ExtAuthz::Response response{};
@@ -5397,6 +5482,45 @@ TEST_F(HttpFilterTest, EncodeHeadersToAddIfAbsentExceedsLimit) {
   EXPECT_EQ(1U, config_->stats().omitted_response_headers_.value());
 }
 
+TEST_F(HttpFilterTest, EncodeHeadersToAddIfAbsentExceedsLimitDisabled) {
+  InSequence s;
+
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_authz_server"
+  enforce_response_header_limits: false
+  )EOF");
+
+  Filters::Common::ExtAuthz::Response response{};
+  response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
+  response.response_headers_to_add_if_absent.push_back({"key1", "value1"});
+  response.response_headers_to_add_if_absent.push_back({"key2", "value2"});
+  response.response_headers_to_add_if_absent.push_back({"existing-header", "value"});
+
+  prepareCheck();
+
+  EXPECT_CALL(*client_, check(_, _, _, _))
+      .WillOnce(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks,
+                           const envoy::service::auth::v3::CheckRequest&, Tracing::Span&,
+                           const StreamInfo::StreamInfo&) -> void {
+        callbacks.onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
+      }));
+
+  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, true));
+
+  Http::TestResponseHeaderMapImpl response_headers(
+      {{":status", "200"}, {"existing-header", "value"}}, /*max_headers_kb=*/99999,
+      /*max_headers_count=*/3);
+
+  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers, false));
+
+  EXPECT_EQ(response_headers.size(), 4);
+  EXPECT_TRUE(response_headers.has("key1"));
+  EXPECT_TRUE(response_headers.has("key2"));
+  EXPECT_EQ(0U, config_->stats().omitted_response_headers_.value());
+}
+
 // Verifies that `response_headers_to_overwrite_if_exists` can still modify headers even when the
 // limit is reached, as it does not add new headers.
 TEST_F(HttpFilterTest, EncodeHeadersToOverwriteIfExistsNotLimited) {
@@ -5406,6 +5530,7 @@ TEST_F(HttpFilterTest, EncodeHeadersToOverwriteIfExistsNotLimited) {
   grpc_service:
     envoy_grpc:
       cluster_name: "ext_authz_server"
+  enforce_response_header_limits: true
   )EOF");
 
   Filters::Common::ExtAuthz::Response response{};
@@ -5451,6 +5576,7 @@ TEST_F(HttpFilterTest, DeniedResponseLocalReplyExceedsLimit) {
   grpc_service:
     envoy_grpc:
       cluster_name: "ext_authz_server"
+  enforce_response_header_limits: true
   )EOF");
 
   Filters::Common::ExtAuthz::Response response{};
@@ -5489,6 +5615,52 @@ TEST_F(HttpFilterTest, DeniedResponseLocalReplyExceedsLimit) {
   EXPECT_EQ(1U, config_->stats().omitted_response_headers_.value());
 }
 
+TEST_F(HttpFilterTest, DeniedResponseLocalReplyExceedsLimitDisabled) {
+  InSequence s;
+
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_authz_server"
+  enforce_response_header_limits: false
+  )EOF");
+
+  Filters::Common::ExtAuthz::Response response{};
+  response.status = Filters::Common::ExtAuthz::CheckStatus::Denied;
+  response.status_code = Http::Code::Unauthorized;
+  response.headers_to_set.push_back({"key1", "value1"});
+  response.headers_to_set.push_back({"key2", "value2"});
+  response.headers_to_set.push_back({"key3", "value3"});
+
+  prepareCheck();
+
+  EXPECT_CALL(*client_, check(_, _, _, _))
+      .WillOnce(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks,
+                           const envoy::service::auth::v3::CheckRequest&, Tracing::Span&,
+                           const StreamInfo::StreamInfo&) -> void {
+        callbacks.onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
+      }));
+
+  EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _))
+      .WillOnce(
+          Invoke([&](Http::Code, absl::string_view,
+                     std::function<void(Http::ResponseHeaderMap & headers)> modify_headers,
+                     const absl::optional<Grpc::Status::GrpcStatus>, absl::string_view) -> void {
+            Http::TestResponseHeaderMapImpl response_headers({}, 99999, /*max_headers_count=*/2);
+            if (modify_headers) {
+              modify_headers(response_headers);
+            }
+            EXPECT_EQ(response_headers.size(), 3);
+            EXPECT_TRUE(response_headers.has("key1"));
+            EXPECT_TRUE(response_headers.has("key2"));
+            EXPECT_TRUE(response_headers.has("key3"));
+          }));
+
+  EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark,
+            filter_->decodeHeaders(request_headers_, true));
+  EXPECT_EQ(0U, config_->stats().omitted_response_headers_.value());
+}
+
 } // namespace
 } // namespace ExtAuthz
 } // namespace HttpFilters