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 e79e59865c06a..abe1638b858e6 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 @@ -50,6 +50,23 @@ message ExtAuthz { // semantically compatible. Deprecation note: This field is deprecated and should only be used for // version upgrade. See release notes for more details. bool use_alpha = 4 [deprecated = true]; + + // Enables filter to buffer the client request body and send it within the authorization request. + BufferSettings with_request_body = 5; +} + +// Configuration for buffering the request data. +message BufferSettings { + // Sets the maximum size of a message body that the filter will hold in memory. Envoy will return + // *HTTP 413* and will *not* initiate the authorization process when buffer reaches the number + // set in this field. Note that this setting will have precedence over :ref:`failure_mode_allow + // `. + uint32 max_request_bytes = 1 [(validate.rules).uint32.gt = 0]; + + // When this field is true, Envoy will buffer the message until *max_request_bytes* is reached. + // The authorization request will be dispatched and no 413 HTTP error will be returned by the + // filter. + bool allow_partial_message = 2; } // HttpService is used for raw HTTP communication between the filter and the authorization service. diff --git a/api/envoy/service/auth/v2/BUILD b/api/envoy/service/auth/v2/BUILD index 5cf93deb777cf..57041668ddc8e 100644 --- a/api/envoy/service/auth/v2/BUILD +++ b/api/envoy/service/auth/v2/BUILD @@ -9,6 +9,7 @@ api_proto_library_internal( ], deps = [ "//envoy/api/v2/core:address", + "//envoy/api/v2/core:base", ], ) diff --git a/api/envoy/service/auth/v2/attribute_context.proto b/api/envoy/service/auth/v2/attribute_context.proto index 7cf7e18eae4c0..b110cec50ed20 100644 --- a/api/envoy/service/auth/v2/attribute_context.proto +++ b/api/envoy/service/auth/v2/attribute_context.proto @@ -6,6 +6,7 @@ option java_outer_classname = "AttributeContextProto"; option java_multiple_files = true; option java_package = "io.envoyproxy.envoy.service.auth.v2"; +import "envoy/api/v2/core/base.proto"; import "envoy/api/v2/core/address.proto"; import "google/protobuf/timestamp.proto"; @@ -112,6 +113,9 @@ message AttributeContext { // See :repo:`headers.h:ProtocolStrings ` for a list of all // possible values. string protocol = 10; + + // The HTTP request body. + string body = 11; } // The source of a network activity, such as starting a TCP connection. diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index b601e6f1dad62..970b481855828 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -24,8 +24,10 @@ Version history * config: use Envoy cpuset size to set the default number or worker threads if :option:`--cpuset-threads` is enabled. * config: added support for :ref:`initial_fetch_timeout `. The timeout is disabled by default. * cors: added :ref:`filter_enabled & shadow_enabled RuntimeFractionalPercent flags ` to filter. -* ext_authz: added an configurable option to make the gRPC service cross-compatible with V2Alpha. Note that this feature is already deprecated. It should be used for a short time, and only when transitioning from alpha to V2 release version. -* ext_authz: migrated from V2alpha to V2 and improved the documentation. +* ext_authz: added support for buffering request body. +* ext_authz: migrated from v2alpha to v2 and improved docs. +* ext_authz: added a configurable option to make the gRPC service cross-compatible with V2Alpha. Note that this feature is already deprecated. It should be used for a short time, and only when transitioning from alpha to V2 release version. +* ext_authz: migrated from v2alpha to v2 and improved the documentation. * ext_authz: authorization request and response configuration has been separated into two distinct objects: :ref:`authorization request ` and :ref:`authorization response `. In addition, :ref:`client headers diff --git a/source/common/http/headers.h b/source/common/http/headers.h index 0f90238da3d70..79c0b1cd153c7 100644 --- a/source/common/http/headers.h +++ b/source/common/http/headers.h @@ -172,8 +172,9 @@ class HeaderValues { const std::string Connect{"CONNECT"}; const std::string Get{"GET"}; const std::string Head{"HEAD"}; - const std::string Post{"POST"}; const std::string Options{"OPTIONS"}; + const std::string Post{"POST"}; + const std::string Trace{"TRACE"}; } MethodValues; struct { diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.cc b/source/extensions/filters/common/ext_authz/check_request_utils.cc index c887b4954eda1..44b87f00b8bd8 100644 --- a/source/extensions/filters/common/ext_authz/check_request_utils.cc +++ b/source/extensions/filters/common/ext_authz/check_request_utils.cc @@ -76,7 +76,7 @@ std::string CheckRequestUtils::getHeaderStr(const Envoy::Http::HeaderEntry* entr void CheckRequestUtils::setHttpRequest( ::envoy::service::auth::v2::AttributeContext_HttpRequest& httpreq, const Envoy::Http::StreamDecoderFilterCallbacks* callbacks, - const Envoy::Http::HeaderMap& headers) { + const Envoy::Http::HeaderMap& headers, uint64_t max_request_bytes) { // Set id // The streamId is not qualified as a const. Although it is as it does not modify the object. @@ -116,20 +116,29 @@ void CheckRequestUtils::setHttpRequest( return Envoy::Http::HeaderMap::Iterate::Continue; }, mutable_headers); + + // Set request body. + const Buffer::Instance* buffer = sdfc->decodingBuffer(); + if (max_request_bytes > 0 && buffer != nullptr) { + const uint64_t length = std::min(buffer->length(), max_request_bytes); + std::string data(length, 0); + buffer->copyOut(0, length, &data[0]); + httpreq.set_body(std::move(data)); + } } void CheckRequestUtils::setAttrContextRequest( ::envoy::service::auth::v2::AttributeContext_Request& req, const Envoy::Http::StreamDecoderFilterCallbacks* callbacks, - const Envoy::Http::HeaderMap& headers) { - setHttpRequest(*req.mutable_http(), callbacks, headers); + const Envoy::Http::HeaderMap& headers, uint64_t max_request_bytes) { + setHttpRequest(*req.mutable_http(), callbacks, headers, max_request_bytes); } void CheckRequestUtils::createHttpCheck( const Envoy::Http::StreamDecoderFilterCallbacks* callbacks, const Envoy::Http::HeaderMap& headers, Protobuf::Map&& context_extensions, - envoy::service::auth::v2::CheckRequest& request) { + envoy::service::auth::v2::CheckRequest& request, uint64_t max_request_bytes) { auto attrs = request.mutable_attributes(); @@ -140,7 +149,7 @@ void CheckRequestUtils::createHttpCheck( setAttrContextPeer(*attrs->mutable_source(), *cb->connection(), service, false); setAttrContextPeer(*attrs->mutable_destination(), *cb->connection(), "", true); - setAttrContextRequest(*attrs->mutable_request(), callbacks, headers); + setAttrContextRequest(*attrs->mutable_request(), callbacks, headers, max_request_bytes); // Fill in the context extensions: (*attrs->mutable_context_extensions()) = std::move(context_extensions); diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.h b/source/extensions/filters/common/ext_authz/check_request_utils.h index a3214d17b50d9..da89fe72fe4e7 100644 --- a/source/extensions/filters/common/ext_authz/check_request_utils.h +++ b/source/extensions/filters/common/ext_authz/check_request_utils.h @@ -43,20 +43,19 @@ class CheckRequestUtils { * @param headers supplies the header map with http headers that will be used to create the * check request. * @param request is the reference to the check request that will be filled up. - * + * @param with_request_body when true, will add the request body to the check request. */ static void createHttpCheck(const Envoy::Http::StreamDecoderFilterCallbacks* callbacks, const Envoy::Http::HeaderMap& headers, Protobuf::Map&& context_extensions, - envoy::service::auth::v2::CheckRequest& request); + envoy::service::auth::v2::CheckRequest& request, uint64_t max_request_bytes); /** * createTcpCheck is used to extract the attributes from the network layer and fill them up * in the CheckRequest proto message. * @param callbacks supplies the network layer context from which data can be extracted. * @param request is the reference to the check request that will be filled up. - * */ static void createTcpCheck(const Network::ReadFilterCallbacks* callbacks, envoy::service::auth::v2::CheckRequest& request); @@ -67,10 +66,11 @@ class CheckRequestUtils { const bool local); static void setHttpRequest(::envoy::service::auth::v2::AttributeContext_HttpRequest& httpreq, const Envoy::Http::StreamDecoderFilterCallbacks* callbacks, - const Envoy::Http::HeaderMap& headers); + const Envoy::Http::HeaderMap& headers, uint64_t max_request_bytes); static void setAttrContextRequest(::envoy::service::auth::v2::AttributeContext_Request& req, const Envoy::Http::StreamDecoderFilterCallbacks* callbacks, - const Envoy::Http::HeaderMap& headers); + const Envoy::Http::HeaderMap& headers, + uint64_t max_request_bytes); static std::string getHeaderStr(const Envoy::Http::HeaderEntry* entry); static Envoy::Http::HeaderMap::Iterate fillHttpHeaders(const Envoy::Http::HeaderEntry&, void*); }; diff --git a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc index f288ca594736b..3d7760ece43d6 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc +++ b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc @@ -161,7 +161,17 @@ void RawHttpClientImpl::check(RequestCallbacks& callbacks, ASSERT(callbacks_ == nullptr); callbacks_ = &callbacks; - Http::HeaderMapPtr headers = std::make_unique(lengthZeroHeader()); + Http::HeaderMapPtr headers; + const uint64_t request_length = request.attributes().request().http().body().size(); + if (request_length > 0) { + headers = + std::make_unique>>( + {{Http::Headers::get().ContentLength, std::to_string(request_length)}}); + } else { + headers = std::make_unique(lengthZeroHeader()); + } + for (const auto& header : request.attributes().request().http().headers()) { const Http::LowerCaseString key{header.first}; if (config_->requestHeaderMatchers()->matches(key.get())) { @@ -179,8 +189,14 @@ void RawHttpClientImpl::check(RequestCallbacks& callbacks, headers->setReference(header_to_add.first, header_to_add.second); } + Http::MessagePtr message = std::make_unique(std::move(headers)); + if (request_length > 0) { + message->body() = + std::make_unique(request.attributes().request().http().body()); + } + request_ = cm_.httpAsyncClientForCluster(config_->cluster()) - .send(std::make_unique(std::move(headers)), *this, + .send(std::move(message), *this, Http::AsyncClient::RequestOptions().setTimeout(config_->timeout())); } diff --git a/source/extensions/filters/http/ext_authz/BUILD b/source/extensions/filters/http/ext_authz/BUILD index 423a8b51b338d..f5b4e7eacea12 100644 --- a/source/extensions/filters/http/ext_authz/BUILD +++ b/source/extensions/filters/http/ext_authz/BUILD @@ -24,6 +24,7 @@ envoy_cc_library( "//source/common/common:matchers_lib", "//source/common/common:minimal_logger_lib", "//source/common/http:codes_lib", + "//source/common/http:utility_lib", "//source/common/router:config_lib", "//source/extensions/filters/common/ext_authz:ext_authz_grpc_lib", "//source/extensions/filters/common/ext_authz:ext_authz_http_lib", diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index a4dac53b8f2ad..6a719bac889c3 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -3,6 +3,7 @@ #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" #include "extensions/filters/http/well_known_names.h" @@ -54,31 +55,57 @@ void Filter::initiateCall(const Http::HeaderMap& headers) { context_extensions = maybe_merged_per_route_config.value().takeContextExtensions(); } Filters::Common::ExtAuthz::CheckRequestUtils::createHttpCheck( - callbacks_, headers, std::move(context_extensions), check_request_); + callbacks_, headers, std::move(context_extensions), check_request_, + config_->maxRequestBytes()); + ENVOY_STREAM_LOG(trace, "ext_authz filter calling authorization server", *callbacks_); state_ = State::Calling; - // Don't let the filter chain continue as we are going to invoke check call. - filter_return_ = FilterReturn::StopDecoding; + filter_return_ = FilterReturn::StopDecoding; // Don't let the filter chain continue as we are + // going to invoke check call. initiating_call_ = true; - ENVOY_STREAM_LOG(trace, "ext_authz filter calling authorization server", *callbacks_); client_->check(*this, check_request_, callbacks_->activeSpan()); initiating_call_ = false; } -Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool) { +Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool end_stream) { request_headers_ = &headers; + buffer_data_ = config_->withRequestBody() && + !(end_stream || Http::Utility::isWebSocketUpgradeRequest(headers) || + Http::Utility::isH2UpgradeRequest(headers)); + if (buffer_data_) { + ENVOY_STREAM_LOG(debug, "ext_authz filter is buffering the request", *callbacks_); + if (!config_->allowPartialMessage()) { + callbacks_->setDecoderBufferLimit(config_->maxRequestBytes()); + } + return Http::FilterHeadersStatus::StopIteration; + } + initiateCall(headers); return filter_return_ == FilterReturn::StopDecoding ? Http::FilterHeadersStatus::StopIteration : Http::FilterHeadersStatus::Continue; } -Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool) { +Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) { + if (buffer_data_) { + if (end_stream || isBufferFull()) { + ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request", *callbacks_); + initiateCall(*request_headers_); + } else { + return Http::FilterDataStatus::StopIterationAndBuffer; + } + } + return filter_return_ == FilterReturn::StopDecoding ? Http::FilterDataStatus::StopIterationAndWatermark : Http::FilterDataStatus::Continue; } Http::FilterTrailersStatus Filter::decodeTrailers(Http::HeaderMap&) { + if (buffer_data_ && filter_return_ != FilterReturn::StopDecoding) { + ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request", *callbacks_); + initiateCall(*request_headers_); + } + return filter_return_ == FilterReturn::StopDecoding ? Http::FilterTrailersStatus::StopIteration : Http::FilterTrailersStatus::Continue; } @@ -122,29 +149,30 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { break; } - ENVOY_STREAM_LOG(trace, "ext_authz received status code {}", *callbacks_, + 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 rejected the request", *callbacks_); - ENVOY_STREAM_LOG(trace, "ext_authz downstream header(s):", *callbacks_); - callbacks_->sendLocalReply(response->status_code, response->body, - [& headers = response->headers_to_add, &callbacks = *callbacks_]( - Http::HeaderMap& response_headers) -> void { - for (const auto& header : headers) { - response_headers.remove(header.first); - response_headers.addCopy(header.first, header.second); - ENVOY_STREAM_LOG(trace, " '{}':'{}'", callbacks, - header.first.get(), header.second); - } - }, - absl::nullopt); + ENVOY_STREAM_LOG(debug, "ext_authz filter rejected the request", *callbacks_); + callbacks_->sendLocalReply( + response->status_code, response->body, + [& headers = response->headers_to_add, + &callbacks = *callbacks_](Http::HeaderMap& response_headers) -> void { + ENVOY_STREAM_LOG(trace, + "ext_authz filter added header(s) to the local response:", callbacks); + for (const auto& header : headers) { + response_headers.remove(header.first); + response_headers.addCopy(header.first, header.second); + ENVOY_STREAM_LOG(trace, " '{}':'{}'", callbacks, header.first.get(), header.second); + } + }, + absl::nullopt); callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::UnauthorizedExternalService); } else { - ENVOY_STREAM_LOG(debug, "ext_authz accepted the request", *callbacks_); + 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) { @@ -153,7 +181,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { } // Only send headers if the response is ok. if (response->status == CheckStatus::OK) { - ENVOY_STREAM_LOG(trace, "ext_authz upstream header(s):", *callbacks_); + ENVOY_STREAM_LOG(trace, "ext_authz filter added header(s) to the request:", *callbacks_); for (const auto& header : response->headers_to_add) { Http::HeaderEntry* header_to_modify = request_headers_->get(header.first); if (header_to_modify) { @@ -179,6 +207,14 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { } } +bool Filter::isBufferFull() { + const auto* buffer = callbacks_->decodingBuffer(); + if (config_->allowPartialMessage() && buffer != nullptr) { + return buffer->length() >= config_->maxRequestBytes(); + } + return false; +} + } // 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 1993ec641c18e..3cc8821509767 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/source/extensions/filters/http/ext_authz/ext_authz.h @@ -39,18 +39,31 @@ class FilterConfig { FilterConfig(const envoy::config::filter::http::ext_authz::v2::ExtAuthz& config, const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, Runtime::Loader& runtime, Http::Context& http_context) - : failure_mode_allow_(config.failure_mode_allow()), local_info_(local_info), scope_(scope), - runtime_(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), + scope_(scope), runtime_(runtime), http_context_(http_context) {} + + bool allowPartialMessage() const { return allow_partial_message_; } + + bool withRequestBody() const { return max_request_bytes_ > 0; } bool failureModeAllow() const { return failure_mode_allow_; } + + uint32_t maxRequestBytes() const { return max_request_bytes_; } + const LocalInfo::LocalInfo& localInfo() const { return local_info_; } + Runtime::Loader& runtime() { return runtime_; } + Stats::Scope& scope() { return scope_; } Http::Context& httpContext() { return http_context_; } private: - bool failure_mode_allow_{}; + const bool allow_partial_message_; + const bool failure_mode_allow_; + const uint32_t max_request_bytes_; const LocalInfo::LocalInfo& local_info_; Stats::Scope& scope_; Runtime::Loader& runtime_; @@ -116,6 +129,8 @@ class Filter : public Logger::Loggable, private: void addResponseHeaders(Http::HeaderMap& header_map, const Http::HeaderVector& headers); + void initiateCall(const Http::HeaderMap& headers); + bool isBufferFull(); // State of this filter's communication with the external authorization service. // The filter has either not started calling the external service, in the middle of calling @@ -127,7 +142,6 @@ class Filter : public Logger::Loggable, // the filter chain should stop. Otherwise the filter chain can continue to the next filter. enum class FilterReturn { ContinueDecoding, StopDecoding }; - void initiateCall(const Http::HeaderMap& headers); Http::HeaderMapPtr getHeaderMap(const Filters::Common::ExtAuthz::ResponsePtr& response); FilterConfigSharedPtr config_; Filters::Common::ExtAuthz::ClientPtr client_; @@ -139,6 +153,7 @@ class Filter : public Logger::Loggable, // Used to identify if the callback to onComplete() is synchronous (on the stack) or asynchronous. bool initiating_call_{}; + bool buffer_data_{}; envoy::service::auth::v2::CheckRequest check_request_{}; }; diff --git a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc index 338ea03c7278e..4adcde6276a33 100644 --- a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc +++ b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc @@ -27,8 +27,30 @@ class CheckRequestUtilsTest : public testing::Test { CheckRequestUtilsTest() { addr_ = std::make_shared("1.2.3.4", 1111); protocol_ = Envoy::Http::Protocol::Http10; + buffer_ = CheckRequestUtilsTest::newTestBuffer(8192); }; + void ExpectBasicHttp() { + EXPECT_CALL(callbacks_, connection()).Times(2).WillRepeatedly(Return(&connection_)); + EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(Const(connection_), ssl()).Times(2).WillRepeatedly(Return(&ssl_)); + EXPECT_CALL(callbacks_, streamId()).Times(1).WillOnce(Return(0)); + EXPECT_CALL(callbacks_, decodingBuffer()).WillOnce(Return(buffer_.get())); + EXPECT_CALL(callbacks_, streamInfo()).Times(3).WillRepeatedly(ReturnRef(req_info_)); + EXPECT_CALL(req_info_, protocol()).Times(2).WillRepeatedly(ReturnPointee(&protocol_)); + } + + static Buffer::InstancePtr newTestBuffer(uint64_t size) { + auto buffer = std::make_unique(); + while (buffer->length() < size) { + auto new_buffer = + Buffer::OwnedImpl("Lorem ipsum dolor sit amet, consectetuer adipiscing elit."); + buffer->add(new_buffer); + } + return std::move(buffer); + } + Network::Address::InstanceConstSharedPtr addr_; absl::optional protocol_; CheckRequestUtils check_request_generator_; @@ -37,6 +59,7 @@ class CheckRequestUtilsTest : public testing::Test { NiceMock connection_; NiceMock ssl_; NiceMock req_info_; + Buffer::InstancePtr buffer_; }; // Verify that createTcpCheck's dependencies are invoked when it's called. @@ -51,19 +74,46 @@ TEST_F(CheckRequestUtilsTest, BasicTcp) { } // Verify that createHttpCheck's dependencies are invoked when it's called. +// Verify that check request object has no request data. TEST_F(CheckRequestUtilsTest, BasicHttp) { - Http::HeaderMapImpl headers; - envoy::service::auth::v2::CheckRequest request; - EXPECT_CALL(callbacks_, connection()).Times(2).WillRepeatedly(Return(&connection_)); - EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); - EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); - EXPECT_CALL(Const(connection_), ssl()).Times(2).WillRepeatedly(Return(&ssl_)); - EXPECT_CALL(callbacks_, streamId()).WillOnce(Return(0)); - EXPECT_CALL(callbacks_, streamInfo()).Times(3).WillRepeatedly(ReturnRef(req_info_)); - EXPECT_CALL(req_info_, protocol()).Times(2).WillRepeatedly(ReturnPointee(&protocol_)); - Protobuf::Map empty; + const uint64_t size = 0; + Http::HeaderMapImpl headers_; + envoy::service::auth::v2::CheckRequest request_; + + ExpectBasicHttp(); + CheckRequestUtils::createHttpCheck(&callbacks_, headers_, + Protobuf::Map(), + request_, size); + ASSERT_EQ(size, request_.attributes().request().http().body().size()); + EXPECT_EQ(buffer_->toString().substr(0, size), request_.attributes().request().http().body()); +} + +// Verify that check request object has only a portion of the request data. +TEST_F(CheckRequestUtilsTest, BasicHttpWithPartialBody) { + const uint64_t size = 4049; + Http::HeaderMapImpl headers_; + envoy::service::auth::v2::CheckRequest request_; + + ExpectBasicHttp(); + CheckRequestUtils::createHttpCheck(&callbacks_, headers_, + Protobuf::Map(), + request_, size); + ASSERT_EQ(size, request_.attributes().request().http().body().size()); + EXPECT_EQ(buffer_->toString().substr(0, size), request_.attributes().request().http().body()); +} - CheckRequestUtils::createHttpCheck(&callbacks_, headers, std::move(empty), request); +// Verify that check request object has all the request data. +TEST_F(CheckRequestUtilsTest, BasicHttpWithFullBody) { + Http::HeaderMapImpl headers_; + envoy::service::auth::v2::CheckRequest request_; + + ExpectBasicHttp(); + CheckRequestUtils::createHttpCheck(&callbacks_, headers_, + Protobuf::Map(), + request_, buffer_->length()); + ASSERT_EQ(buffer_->length(), request_.attributes().request().http().body().size()); + EXPECT_EQ(buffer_->toString().substr(0, buffer_->length()), + request_.attributes().request().http().body()); } // Verify that createHttpCheck extract the proper attributes from the http request into CheckRequest @@ -78,6 +128,7 @@ TEST_F(CheckRequestUtilsTest, CheckAttrContextPeer) { EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_)); EXPECT_CALL(callbacks_, streamId()).WillRepeatedly(Return(0)); EXPECT_CALL(callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_)); + EXPECT_CALL(callbacks_, decodingBuffer()).Times(1); EXPECT_CALL(req_info_, protocol()).WillRepeatedly(ReturnPointee(&protocol_)); EXPECT_CALL(ssl_, uriSanPeerCertificate()).WillOnce(Return(std::vector{"source"})); EXPECT_CALL(ssl_, uriSanLocalCertificate()) @@ -87,7 +138,7 @@ TEST_F(CheckRequestUtilsTest, CheckAttrContextPeer) { context_extensions["key"] = "value"; CheckRequestUtils::createHttpCheck(&callbacks_, request_headers, std::move(context_extensions), - request); + request, false); EXPECT_EQ("source", request.attributes().source().principal()); EXPECT_EQ("destination", request.attributes().destination().principal()); diff --git a/test/extensions/filters/http/ext_authz/config_test.cc b/test/extensions/filters/http/ext_authz/config_test.cc index 887e50ba776a4..2766bb70b71c3 100644 --- a/test/extensions/filters/http/ext_authz/config_test.cc +++ b/test/extensions/filters/http/ext_authz/config_test.cc @@ -77,6 +77,8 @@ TEST(HttpExtAuthzConfigTest, CorrectProtoHttp) { path_prefix: /extauth failure_mode_allow: true + with_request_body: + max_request_bytes: 100 )EOF"; ExtAuthzFilterConfig factory; 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 edb4f12d6e18a..64a2fbeffda0d 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc @@ -21,6 +21,7 @@ #include "test/mocks/http/mocks.h" #include "test/mocks/local_info/mocks.h" #include "test/mocks/network/mocks.h" +#include "test/mocks/router/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/tracing/mocks.h" #include "test/mocks/upstream/mocks.h" @@ -64,7 +65,7 @@ template class HttpFilterTestBase : public T { Filters::Common::ExtAuthz::MockClient* client_; std::unique_ptr filter_; NiceMock filter_callbacks_; - Filters::Common::ExtAuthz::RequestCallbacks* request_callbacks_{}; + Filters::Common::ExtAuthz::RequestCallbacks* request_callbacks_; Http::TestHeaderMapImpl request_headers_; Buffer::OwnedImpl data_; Stats::IsolatedStoreImpl stats_store_; @@ -263,6 +264,242 @@ TEST_F(HttpFilterTest, BadConfig) { ProtoValidationException); } +// Checks that filter does not initiate the authorization request when the buffer reaches the max +// request bytes. +TEST_F(HttpFilterTest, RequestDataIsTooLarge) { + InSequence s; + + initialize(R"EOF( + grpc_service: + envoy_grpc: + cluster_name: "ext_authz_server" + failure_mode_allow: false + with_request_body: + max_request_bytes: 10 + )EOF"); + + ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); + EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(1); + EXPECT_CALL(connection_, remoteAddress()).Times(0); + EXPECT_CALL(connection_, localAddress()).Times(0); + EXPECT_CALL(*client_, check(_, _, _)).Times(0); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + + Buffer::OwnedImpl buffer1("foo"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(buffer1, false)); + + Buffer::OwnedImpl buffer2("foobarbaz"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(buffer2, false)); +} + +// Checks that the filter initiates an authorization request when the buffer reaches max +// request bytes and allow_partial_message is set to true. +TEST_F(HttpFilterTest, RequestDataWithPartialMessage) { + InSequence s; + + initialize(R"EOF( + grpc_service: + envoy_grpc: + cluster_name: "ext_authz_server" + failure_mode_allow: false + with_request_body: + max_request_bytes: 10 + allow_partial_message: true + )EOF"); + + ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); + ON_CALL(filter_callbacks_, decodingBuffer()).WillByDefault(Return(&data_)); + ; + EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(0); + EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(*client_, check(_, _, _)).Times(1); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + + data_.add("foo"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false)); + + data_.add("bar"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false)); + + data_.add("barfoo"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_)); +} + +// Checks that the filter initiates the authorization process only when the filter decode trailers +// is called. +TEST_F(HttpFilterTest, RequestDataWithSmallBuffer) { + InSequence s; + + initialize(R"EOF( + grpc_service: + envoy_grpc: + cluster_name: "ext_authz_server" + failure_mode_allow: false + with_request_body: + max_request_bytes: 10 + allow_partial_message: true + )EOF"); + + ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); + ON_CALL(filter_callbacks_, decodingBuffer()).WillByDefault(Return(&data_)); + EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(0); + EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(*client_, check(_, _, _)).Times(1); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + + data_.add("foo"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_)); +} + +// Checks that the filter buffers the data and initiates the authorization request. +TEST_F(HttpFilterTest, AuthWithRequestData) { + InSequence s; + + initialize(R"EOF( + grpc_service: + envoy_grpc: + cluster_name: "ext_authz_server" + with_request_body: + max_request_bytes: 10 + )EOF"); + + prepareCheck(); + + EXPECT_CALL(*client_, check(_, _, testing::A())) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + data_.add("foo"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false)); + data_.add("bar"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_)); +} + +// Checks that filter does not buffer data on header-only request. +TEST_F(HttpFilterTest, HeaderOnlyRequest) { + InSequence s; + + initialize(R"EOF( + grpc_service: + envoy_grpc: + cluster_name: "ext_authz_server" + with_request_body: + max_request_bytes: 10 + )EOF"); + + prepareCheck(); + + EXPECT_CALL(*client_, check(_, _, testing::A())) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, true)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_)); +} + +// Checks that filter does not buffer data on upgrade WebSocket request. +TEST_F(HttpFilterTest, UpgradeWebsocketRequest) { + InSequence s; + + initialize(R"EOF( + grpc_service: + envoy_grpc: + cluster_name: "ext_authz_server" + with_request_body: + max_request_bytes: 10 + )EOF"); + + prepareCheck(); + + request_headers_.addCopy(Http::Headers::get().Connection, + Http::Headers::get().ConnectionValues.Upgrade); + request_headers_.addCopy(Http::Headers::get().Upgrade, + Http::Headers::get().UpgradeValues.WebSocket); + + EXPECT_CALL(*client_, check(_, _, testing::A())) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_)); +} + +// Checks that filter does not buffer data on upgrade H2 WebSocket request. +TEST_F(HttpFilterTest, H2UpgradeRequest) { + InSequence s; + + initialize(R"EOF( + grpc_service: + envoy_grpc: + cluster_name: "ext_authz_server" + with_request_body: + max_request_bytes: 10 + )EOF"); + + prepareCheck(); + + request_headers_.addCopy(Http::Headers::get().Method, Http::Headers::get().MethodValues.Connect); + request_headers_.addCopy(Http::Headers::get().Protocol, + Http::Headers::get().ProtocolStrings.Http2String); + + EXPECT_CALL(*client_, check(_, _, testing::A())) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_)); +} + +// Checks that filter does not buffer data when is not the end of the stream, but header-only +// request has been received. +TEST_F(HttpFilterTest, HeaderOnlyRequestWithStream) { + InSequence s; + + initialize(R"EOF( + grpc_service: + envoy_grpc: + cluster_name: "ext_authz_server" + with_request_body: + max_request_bytes: 10 + )EOF"); + + prepareCheck(); + + EXPECT_CALL(*client_, check(_, _, testing::A())) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_)); +} + // ------------------- // Parameterized Tests // -------------------