diff --git a/api/envoy/config/filter/http/ext_authz/v2/BUILD b/api/envoy/config/filter/http/ext_authz/v2/BUILD index a22408bb1be7e..b1d02437df044 100644 --- a/api/envoy/config/filter/http/ext_authz/v2/BUILD +++ b/api/envoy/config/filter/http/ext_authz/v2/BUILD @@ -9,6 +9,7 @@ api_proto_library_internal( "//envoy/api/v2/core:base", "//envoy/api/v2/core:grpc_service", "//envoy/api/v2/core:http_uri", + "//envoy/type:http_status", "//envoy/type/matcher:string", ], ) diff --git a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto index 4c9b4d9c82897..8e2ea7661e1ff 100644 --- a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto +++ b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto @@ -11,6 +11,7 @@ import "envoy/api/v2/core/base.proto"; import "envoy/api/v2/core/grpc_service.proto"; import "envoy/api/v2/core/http_uri.proto"; +import "envoy/type/http_status.proto"; import "envoy/type/matcher/string.proto"; import "validate/validate.proto"; @@ -67,6 +68,10 @@ message ExtAuthz { // altering another client request header. // bool clear_route_cache = 6; + + // Sets the HTTP status that is returned to the client when there is a network error between the + // filter and the authorization server. The default status is HTTP 403 Forbidden. + envoy.type.HttpStatus status_on_error = 7; } // Configuration for buffering the request data. diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 8fca2bdf3b54f..24afc0b19ee71 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -13,6 +13,7 @@ Version history * eds: added support to specify max time for which endpoints can be used :ref:`gRPC filter `. * event: added :ref:`loop duration and poll delay statistics `. * ext_authz: added a `x-envoy-auth-partial-body` metadata header set to `false|true` indicating if there is a partial body sent in the authorization request message. +* ext_authz: added configurable status code that allows customizing HTTP responses on filter check status errors. * ext_authz: added option to `ext_authz` that allows the filter clearing route cache. * grpc-json: added support for :ref:`auto mapping `. diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 04cd842b966f2..a7a5773084601 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -2,7 +2,6 @@ #include "common/common/assert.h" #include "common/common/enum_to_int.h" -#include "common/http/codes.h" #include "common/http/utility.h" #include "common/router/config_impl.h" @@ -147,13 +146,37 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { Stats::StatName empty_stat_name; switch (response->status) { - case CheckStatus::OK: + case CheckStatus::OK: { + ENVOY_STREAM_LOG(trace, "ext_authz filter added header(s) to the request:", *callbacks_); + if (config_->clearRouteCache() && + (!response->headers_to_add.empty() || !response->headers_to_append.empty())) { + ENVOY_STREAM_LOG(debug, "ext_authz is clearing route cache", *callbacks_); + callbacks_->clearRouteCache(); + } + for (const auto& header : response->headers_to_add) { + ENVOY_STREAM_LOG(trace, " '{}':'{}'", *callbacks_, header.first.get(), header.second); + Http::HeaderEntry* header_to_modify = request_headers_->get(header.first); + if (header_to_modify) { + header_to_modify->value(header.second.c_str(), header.second.size()); + } else { + request_headers_->addCopy(header.first, header.second); + } + } + for (const auto& header : response->headers_to_append) { + Http::HeaderEntry* header_to_modify = request_headers_->get(header.first); + if (header_to_modify) { + ENVOY_STREAM_LOG(trace, " '{}':'{}'", *callbacks_, header.first.get(), header.second); + Http::HeaderMapImpl::appendToHeader(header_to_modify->value(), header.second); + } + } cluster_->statsScope().counter("ext_authz.ok").inc(); + continueDecoding(); break; - case CheckStatus::Error: - cluster_->statsScope().counter("ext_authz.error").inc(); - break; - case CheckStatus::Denied: + } + + case CheckStatus::Denied: { + ENVOY_STREAM_LOG(trace, "ext_authz filter rejected the request. Response status code: '{}", + *callbacks_, enumToInt(response->status_code)); cluster_->statsScope().counter("ext_authz.denied").inc(); Http::CodeStats::ResponseStatInfo info{config_->scope(), cluster_->statsScope(), @@ -166,20 +189,6 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { empty_stat_name, false}; config_->httpContext().codeStats().chargeResponseStat(info); - break; - } - - ENVOY_STREAM_LOG(trace, "ext_authz filter received status code {}", *callbacks_, - enumToInt(response->status_code)); - - // We fail open/fail close based of filter config - // if there is an error contacting the service. - if (response->status == CheckStatus::Denied || - (response->status == CheckStatus::Error && !config_->failureModeAllow())) { - ENVOY_STREAM_LOG(debug, "ext_authz filter rejected the request", *callbacks_); - const std::string& details = response->status == CheckStatus::Denied - ? RcDetails::get().AuthzDenied - : RcDetails::get().AuthzError; callbacks_->sendLocalReply( response->status_code, response->body, [& headers = response->headers_to_add, @@ -187,52 +196,37 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { ENVOY_STREAM_LOG(trace, "ext_authz filter added header(s) to the local response:", callbacks); for (const auto& header : headers) { + ENVOY_STREAM_LOG(trace, " '{}':'{}'", callbacks, header.first.get(), header.second); response_headers.remove(header.first); response_headers.addCopy(header.first, header.second); - ENVOY_STREAM_LOG(trace, " '{}':'{}'", callbacks, header.first.get(), header.second); } }, - absl::nullopt, details); + absl::nullopt, RcDetails::get().AuthzDenied); callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::UnauthorizedExternalService); - } else { - ENVOY_STREAM_LOG(debug, "ext_authz filter accepted the request", *callbacks_); - // Let the filter chain continue. - filter_return_ = FilterReturn::ContinueDecoding; - if (config_->failureModeAllow() && response->status == CheckStatus::Error) { - // Status is Error and yet we are allowing the request. Click a counter. - cluster_->statsScope().counter("ext_authz.failure_mode_allowed").inc(); - } - // Only send headers if the response is ok. - if (response->status == CheckStatus::OK) { - ENVOY_STREAM_LOG(trace, "ext_authz filter added header(s) to the request:", *callbacks_); - if (config_->clearRouteCache() && - (!response->headers_to_add.empty() || !response->headers_to_append.empty())) { - ENVOY_STREAM_LOG(debug, "ext_authz is clearing route cache", *callbacks_); - callbacks_->clearRouteCache(); - } + break; + } - for (const auto& header : response->headers_to_add) { - Http::HeaderEntry* header_to_modify = request_headers_->get(header.first); - if (header_to_modify) { - header_to_modify->value(header.second.c_str(), header.second.size()); - } else { - request_headers_->addCopy(header.first, header.second); - } - ENVOY_STREAM_LOG(trace, " '{}':'{}'", *callbacks_, header.first.get(), header.second); - } - for (const auto& header : response->headers_to_append) { - Http::HeaderEntry* header_to_modify = request_headers_->get(header.first); - if (header_to_modify) { - Http::HeaderMapImpl::appendToHeader(header_to_modify->value(), header.second); - ENVOY_STREAM_LOG(trace, " '{}':'{}'", *callbacks_, header.first.get(), header.second); - } - } + case CheckStatus::Error: { + cluster_->statsScope().counter("ext_authz.error").inc(); + if (config_->failureModeAllow()) { + ENVOY_STREAM_LOG(trace, "ext_authz filter allowed the request with error", *callbacks_); + cluster_->statsScope().counter("ext_authz.failure_mode_allowed").inc(); + continueDecoding(); + } else { + ENVOY_STREAM_LOG( + trace, "ext_authz filter rejected the request with an error. Response status code: {}", + *callbacks_, enumToInt(config_->statusOnError())); + callbacks_->streamInfo().setResponseFlag( + StreamInfo::ResponseFlag::UnauthorizedExternalService); + callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr, absl::nullopt, + RcDetails::get().AuthzError); } + break; + } - if (!initiating_call_) { - // We got completion async. Let the filter chain continue. - callbacks_->continueDecoding(); - } + default: + NOT_REACHED_GCOVR_EXCL_LINE; + break; } } @@ -244,6 +238,13 @@ bool Filter::isBufferFull() { return false; } +void Filter::continueDecoding() { + filter_return_ = FilterReturn::ContinueDecoding; + if (!initiating_call_) { + callbacks_->continueDecoding(); + } +} + } // namespace ExtAuthz } // namespace HttpFilters } // namespace Extensions diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h index e32bea814b3c6..427616f6b8ab3 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/source/extensions/filters/http/ext_authz/ext_authz.h @@ -10,11 +10,13 @@ #include "envoy/local_info/local_info.h" #include "envoy/runtime/runtime.h" #include "envoy/stats/scope.h" +#include "envoy/type/http_status.pb.h" #include "envoy/upstream/cluster_manager.h" #include "common/common/assert.h" #include "common/common/logger.h" #include "common/common/matchers.h" +#include "common/http/codes.h" #include "common/http/header_map_impl.h" #include "extensions/filters/common/ext_authz/ext_authz.h" @@ -42,7 +44,8 @@ class FilterConfig { : allow_partial_message_(config.with_request_body().allow_partial_message()), failure_mode_allow_(config.failure_mode_allow()), clear_route_cache_(config.clear_route_cache()), - max_request_bytes_(config.with_request_body().max_request_bytes()), local_info_(local_info), + max_request_bytes_(config.with_request_body().max_request_bytes()), + status_on_error_(toErrorCode(config.status_on_error().code())), local_info_(local_info), scope_(scope), runtime_(runtime), http_context_(http_context) {} bool allowPartialMessage() const { return allow_partial_message_; } @@ -57,6 +60,8 @@ class FilterConfig { const LocalInfo::LocalInfo& localInfo() const { return local_info_; } + Http::Code statusOnError() const { return status_on_error_; } + Runtime::Loader& runtime() { return runtime_; } Stats::Scope& scope() { return scope_; } @@ -64,10 +69,19 @@ class FilterConfig { Http::Context& httpContext() { return http_context_; } private: + static Http::Code toErrorCode(uint64_t status) { + const auto code = static_cast(status); + if (code >= Http::Code::Continue && code <= Http::Code::NetworkAuthenticationRequired) { + return code; + } + return Http::Code::Forbidden; + } + const bool allow_partial_message_; const bool failure_mode_allow_; const bool clear_route_cache_; const uint32_t max_request_bytes_; + const Http::Code status_on_error_; const LocalInfo::LocalInfo& local_info_; Stats::Scope& scope_; Runtime::Loader& runtime_; @@ -134,6 +148,7 @@ class Filter : public Logger::Loggable, private: void addResponseHeaders(Http::HeaderMap& header_map, const Http::HeaderVector& headers); void initiateCall(const Http::HeaderMap& headers); + void continueDecoding(); bool isBufferFull(); // State of this filter's communication with the external authorization service. 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 16233d571eb10..10894e030fd7f 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc @@ -182,6 +182,47 @@ TEST_F(HttpFilterTest, ErrorFailClose) { EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter_->decodeHeaders(request_headers_, false)); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); + EXPECT_CALL(filter_callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&](const Http::HeaderMap& headers, bool) -> void { + EXPECT_EQ(headers.Status()->value().getStringView(), + std::to_string(enumToInt(Http::Code::Forbidden))); + })); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::Error; + request_callbacks_->onComplete(std::make_unique(response)); + EXPECT_EQ(1U, filter_callbacks_.clusterInfo()->statsScope().counter("ext_authz.error").value()); +} + +// Verifies that the filter responds with a configurable HTTP status when an network error occurs. +TEST_F(HttpFilterTest, ErrorCustomStatusCode) { + InSequence s; + + initialize(R"EOF( + grpc_service: + envoy_grpc: + cluster_name: "ext_authz_server" + failure_mode_allow: false + status_on_error: + code: 503 + )EOF"); + + ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); + EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(*client_, check(_, _, _)) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, + filter_->decodeHeaders(request_headers_, false)); + EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); + EXPECT_CALL(filter_callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&](const Http::HeaderMap& headers, bool) -> void { + EXPECT_EQ(headers.Status()->value().getStringView(), + std::to_string(enumToInt(Http::Code::ServiceUnavailable))); + })); Filters::Common::ExtAuthz::Response response{}; response.status = Filters::Common::ExtAuthz::CheckStatus::Error;