diff --git a/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto b/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto index 2d2c2a7246b3e..3d1f361eecc51 100644 --- a/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto +++ b/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 `. // [#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. diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 8a58a3fd9a70a..87c641e68ef1c 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -150,5 +150,11 @@ new_features: Added QUIC protocol option :ref:`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 ` + that allows admins to enable / disable the behavior of dropping response headers once the header + map count / size constraints have been reached. deprecated: diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index c1ff352f7b605..c34deadbc2675 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/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(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, diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h index 0bd1a195e6ba6..f912a6c72a825 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/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 filter_metadata_; const bool emit_filter_state_stats_; + const bool enforce_response_header_limits_; const absl::optional filter_enabled_; const absl::optional filter_enabled_metadata_; diff --git a/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc index e092ef44c6941..5d6d2f3ff368d 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc +++ b/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 expect_stats_override; + absl::optional 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 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(); diff --git a/test/extensions/filters/http/ext_authz/ext_authz_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_test.cc index dfc822a964cfa..9ce8ce59b7d8d 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc +++ b/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(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(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(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(response)); + })); + + EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)) + .WillOnce( + Invoke([&](Http::Code, absl::string_view, + std::function modify_headers, + const absl::optional, 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