From 9d38e5b62565278f3e1b9c4f6888e6f7ba333fb0 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sat, 13 Apr 2019 19:20:26 -0700 Subject: [PATCH 1/7] wip: saving the work Signed-off-by: Gabriel --- .../filter/http/ext_authz/v2/ext_authz.proto | 5 +++++ .../filters/http/ext_authz/ext_authz.cc | 7 ++++--- .../extensions/filters/http/ext_authz/ext_authz.h | 15 ++++++++++++++- 3 files changed, 23 insertions(+), 4 deletions(-) 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 abe1638b858e6..688ddcb9b19c2 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"; @@ -53,6 +54,10 @@ message ExtAuthz { // Enables filter to buffer the client request body and send it within the authorization request. BufferSettings with_request_body = 5; + + // Sets the HTTP status that is returned to the client when a network error compromises the comunication 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/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 6a719bac889c3..85d39dd6695df 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -152,13 +152,14 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { ENVOY_STREAM_LOG(trace, "ext_authz filter received status code {}", *callbacks_, enumToInt(response->status_code)); + const bool is_error = response->status == CheckStatus::Error; + // 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())) { + if (response->status == CheckStatus::Denied || (is_error && !config_->failureModeAllow())) { ENVOY_STREAM_LOG(debug, "ext_authz filter rejected the request", *callbacks_); callbacks_->sendLocalReply( - response->status_code, response->body, + is_error ? config_->statusOnError() : response->status_code, response->body, [& headers = response->headers_to_add, &callbacks = *callbacks_](Http::HeaderMap& response_headers) -> void { ENVOY_STREAM_LOG(trace, diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h index 3cc8821509767..6c99277e76cc0 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/source/extensions/filters/http/ext_authz/ext_authz.h @@ -12,6 +12,8 @@ #include "envoy/stats/scope.h" #include "envoy/upstream/cluster_manager.h" +#include "envoy/type/http_status.pb.h" + #include "common/common/assert.h" #include "common/common/logger.h" #include "common/common/matchers.h" @@ -41,7 +43,8 @@ class FilterConfig { Runtime::Loader& runtime, Http::Context& http_context) : allow_partial_message_(config.with_request_body().allow_partial_message()), failure_mode_allow_(config.failure_mode_allow()), - 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())), local_info_(local_info), scope_(scope), runtime_(runtime), http_context_(http_context) {} bool allowPartialMessage() const { return allow_partial_message_; } @@ -50,6 +53,8 @@ class FilterConfig { bool failureModeAllow() const { return failure_mode_allow_; } + Http::Code statusOnError() const { return status_on_error_; } + uint32_t maxRequestBytes() const { return max_request_bytes_; } const LocalInfo::LocalInfo& localInfo() const { return local_info_; } @@ -61,9 +66,17 @@ class FilterConfig { Http::Context& httpContext() { return http_context_; } private: + Http::Code toOnError(const envoy::type::HttpStatus status) const { + if (status.code() != 0) { + return static_cast(status.code()); + } + return Http::Code::Forbidden; + } + const bool allow_partial_message_; const bool failure_mode_allow_; const uint32_t max_request_bytes_; + const Http::Code status_on_error_; const LocalInfo::LocalInfo& local_info_; Stats::Scope& scope_; Runtime::Loader& runtime_; From 48e0993ddc1e9a65ea55412b2c30c4d1affd027c Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 21 Apr 2019 18:05:45 -0700 Subject: [PATCH 2/7] fixed format and added release notes Signed-off-by: Gabriel --- docs/root/intro/version_history.rst | 1 + source/extensions/filters/http/ext_authz/ext_authz.cc | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index e4f05582044d0..ea54fe80b9dfc 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -6,6 +6,7 @@ Version history * access log: added a new field for response code details in :ref:`file access logger` and :ref:`gRPC access logger`. * dubbo_proxy: support the :ref:`Dubbo proxy filter `. * ext_authz: added option to `ext_authz` that allows the filter clearing route cache. +* ext_authz: added configurable status code that allows customizing HTTP responses on filter errors. * eds: added support to specify max time for which endpoints can be used :ref:`gRPC filter `. * http: mitigated a race condition with the :ref:`delayed_close_timeout` where it could trigger while actively flushing a pending write buffer for a downstream connection. * redis: added :ref:`prefix routing ` to enable routing commands based on their key's prefix to different upstream. diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 9aedffa6c6f26..53ceb5db00dd0 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -139,7 +139,6 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { 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); @@ -149,7 +148,6 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { 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) { @@ -157,7 +155,6 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { Http::HeaderMapImpl::appendToHeader(header_to_modify->value(), header.second); } } - cluster_->statsScope().counter("ext_authz.ok").inc(); continueDecoding(); break; From 706b249225e1969be1b45172ab71924bcc3a32c9 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 21 Apr 2019 18:19:49 -0700 Subject: [PATCH 3/7] fixed typo Signed-off-by: Gabriel --- api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 08900bc360c82..fccc00cbc8c8a 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 @@ -68,7 +68,7 @@ message ExtAuthz { bool clear_route_cache = 6; // Sets the HTTP status that is returned to the client when a network error compromises the - // comunication between the filter and the authorization server. The default status is HTTP 403 + // communication between the filter and the authorization server. The default status is HTTP 403 // Forbidden. envoy.type.HttpStatus status_on_error = 7; } From 8c8640e962ef4abffddde023cec40575e3197017 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Wed, 24 Apr 2019 15:54:03 -0700 Subject: [PATCH 4/7] code review changes Signed-off-by: Gabriel --- docs/root/intro/version_history.rst | 2 +- source/extensions/filters/http/ext_authz/ext_authz.cc | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index ea54fe80b9dfc..527549b7c1011 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -6,7 +6,7 @@ Version history * access log: added a new field for response code details in :ref:`file access logger` and :ref:`gRPC access logger`. * dubbo_proxy: support the :ref:`Dubbo proxy filter `. * ext_authz: added option to `ext_authz` that allows the filter clearing route cache. -* ext_authz: added configurable status code that allows customizing HTTP responses on filter errors. +* ext_authz: added configurable status code that allows customizing HTTP responses on filter check status errors. * eds: added support to specify max time for which endpoints can be used :ref:`gRPC filter `. * http: mitigated a race condition with the :ref:`delayed_close_timeout` where it could trigger while actively flushing a pending write buffer for a downstream connection. * redis: added :ref:`prefix routing ` to enable routing commands based on their key's prefix to different upstream. diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 53ceb5db00dd0..c4e3ed34fb729 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -161,7 +161,8 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { } case CheckStatus::Denied: { - ENVOY_STREAM_LOG(debug, "ext_authz filter rejected the request", *callbacks_); + ENVOY_STREAM_LOG(trace, "ext_authz filter rejected the request. Response status code: {}", + response->status_code, *callbacks_); cluster_->statsScope().counter("ext_authz.denied").inc(); Http::CodeStats::ResponseStatInfo info{config_->scope(), cluster_->statsScope(), @@ -198,7 +199,9 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { cluster_->statsScope().counter("ext_authz.failure_mode_allowed").inc(); continueDecoding(); } else { - ENVOY_STREAM_LOG(debug, "ext_authz filter rejected the request with an error", *callbacks_); + ENVOY_STREAM_LOG( + trace, "ext_authz filter rejected the request with an error. Response status code: {}", + config_->statusOnError(), *callbacks_); callbacks_->streamInfo().setResponseFlag( StreamInfo::ResponseFlag::UnauthorizedExternalService); callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr, absl::nullopt); From b881d69a38ed2e581e4a7547f32c408264e7a21f Mon Sep 17 00:00:00 2001 From: Gabriel Date: Wed, 24 Apr 2019 17:29:14 -0700 Subject: [PATCH 5/7] fixed logger message Signed-off-by: Gabriel --- source/extensions/filters/http/ext_authz/ext_authz.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index c4e3ed34fb729..5848c9224e8b3 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -161,8 +161,8 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { } case CheckStatus::Denied: { - ENVOY_STREAM_LOG(trace, "ext_authz filter rejected the request. Response status code: {}", - response->status_code, *callbacks_); + 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(), @@ -201,7 +201,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { } else { ENVOY_STREAM_LOG( trace, "ext_authz filter rejected the request with an error. Response status code: {}", - config_->statusOnError(), *callbacks_); + *callbacks_, enumToInt(config_->statusOnError())); callbacks_->streamInfo().setResponseFlag( StreamInfo::ResponseFlag::UnauthorizedExternalService); callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr, absl::nullopt); From 5487d9f93f1d04fc354b3410627cfadb97dae4d9 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 26 Apr 2019 10:26:15 -0700 Subject: [PATCH 6/7] edited version history Signed-off-by: Gabriel --- docs/root/intro/version_history.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 859d1ec411d04..71390e307fe14 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -5,12 +5,11 @@ Version history ================ * access log: added a new field for response code details in :ref:`file access logger` and :ref:`gRPC access logger`. * dubbo_proxy: support the :ref:`Dubbo proxy filter `. -* ext_authz: added option to `ext_authz` that allows the filter clearing route cache. +* 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. * 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 option to `ext_authz` that allows the filter clearing route cache. * http: mitigated a race condition with the :ref:`delayed_close_timeout` where it could trigger while actively flushing a pending write buffer for a downstream connection. * jwt_authn: make filter's parsing of JWT more flexible, allowing syntax like ``jwt=eyJhbGciOiJS...ZFnFIw,extra=7,realm=123`` * redis: added :ref:`prefix routing ` to enable routing commands based on their key's prefix to different upstream. From 77b190f2759ef1cf87792e50e2943d01e11f6cc4 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 26 Apr 2019 15:03:29 -0700 Subject: [PATCH 7/7] fixed version alpha order Signed-off-by: Gabriel --- docs/root/intro/version_history.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 71390e307fe14..00489f8acde60 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -5,11 +5,11 @@ Version history ================ * access log: added a new field for response code details in :ref:`file access logger` and :ref:`gRPC access logger`. * dubbo_proxy: support the :ref:`Dubbo proxy filter `. +* 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. -* 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 `. * http: mitigated a race condition with the :ref:`delayed_close_timeout` where it could trigger while actively flushing a pending write buffer for a downstream connection. * jwt_authn: make filter's parsing of JWT more flexible, allowing syntax like ``jwt=eyJhbGciOiJS...ZFnFIw,extra=7,realm=123`` * redis: added :ref:`prefix routing ` to enable routing commands based on their key's prefix to different upstream.