Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Bug Fixes
---------
*Changes expected to improve the state of the world and are unlikely to have negative effects*

* ext_authz: fix the ext_authz network filter to correctly set response flag and code details to ``UAEX`` when a connection is denied.
* listener: fixed the crash when updating listeners that do not bind to port.
* thrift_proxy: fix the thrift_proxy connection manager to correctly report success/error response metrics when performing :ref:`payload passthrough <envoy_v3_api_field_extensions.filters.network.thrift_proxy.v3.ThriftProxy.payload_passthrough>`.

Expand Down
11 changes: 11 additions & 0 deletions source/extensions/filters/common/ext_authz/ext_authz.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ struct TracingConstantValues {

using TracingConstants = ConstSingleton<TracingConstantValues>;

/**
* Possible constant response code details values for a check call.
*/
struct ResponseCodeDetailsValues {
// The ext_authz filter denied the downstream request/connection.
const std::string AuthzDenied = "ext_authz_denied";
// The ext_authz filter encountered a failure, and was configured to fail-closed.
const std::string AuthzError = "ext_authz_error";
};
using ResponseCodeDetails = ConstSingleton<ResponseCodeDetailsValues>;

/**
* Constant auth related HTTP headers. All lower case. This group of headers can
* contain prefix override headers.
Expand Down
20 changes: 7 additions & 13 deletions source/extensions/filters/http/ext_authz/ext_authz.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,6 @@ namespace Extensions {
namespace HttpFilters {
namespace ExtAuthz {

struct RcDetailsValues {
// The ext_authz filter denied the downstream request.
const std::string AuthzDenied = "ext_authz_denied";
// The ext_authz filter encountered a failure, and was configured to fail-closed.
const std::string AuthzError = "ext_authz_error";
};
using RcDetails = ConstSingleton<RcDetailsValues>;

void FilterConfigPerRoute::merge(const FilterConfigPerRoute& other) {
// We only merge context extensions here, and leave boolean flags untouched since those flags are
// not used from the merged config.
Expand Down Expand Up @@ -91,8 +83,9 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers,
*decoder_callbacks_);
decoder_callbacks_->streamInfo().setResponseFlag(
StreamInfo::ResponseFlag::UnauthorizedExternalService);
decoder_callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr,
absl::nullopt, RcDetails::get().AuthzError);
decoder_callbacks_->sendLocalReply(
config_->statusOnError(), EMPTY_STRING, nullptr, absl::nullopt,
Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzError);
return Http::FilterHeadersStatus::StopIteration;
}
return Http::FilterHeadersStatus::Continue;
Expand Down Expand Up @@ -371,7 +364,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) {
response_headers.addCopy(header.first, header.second);
}
},
absl::nullopt, RcDetails::get().AuthzDenied);
absl::nullopt, Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied);
decoder_callbacks_->streamInfo().setResponseFlag(
StreamInfo::ResponseFlag::UnauthorizedExternalService);
break;
Expand All @@ -396,8 +389,9 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) {
*decoder_callbacks_, enumToInt(config_->statusOnError()));
decoder_callbacks_->streamInfo().setResponseFlag(
StreamInfo::ResponseFlag::UnauthorizedExternalService);
decoder_callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr,
absl::nullopt, RcDetails::get().AuthzError);
decoder_callbacks_->sendLocalReply(
config_->statusOnError(), EMPTY_STRING, nullptr, absl::nullopt,
Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzError);
}
break;
}
Expand Down
6 changes: 6 additions & 0 deletions source/extensions/filters/network/ext_authz/ext_authz.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) {
!config_->failureModeAllow())) {
config_->stats().cx_closed_.inc();
filter_callbacks_->connection().close(Network::ConnectionCloseType::NoFlush);
filter_callbacks_->connection().streamInfo().setResponseFlag(
StreamInfo::ResponseFlag::UnauthorizedExternalService);
filter_callbacks_->connection().streamInfo().setResponseCodeDetails(
response->status == Filters::Common::ExtAuthz::CheckStatus::Denied
? Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied
: Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzError);
} else {
// Let the filter chain continue.
filter_return_ = FilterReturn::Continue;
Expand Down
16 changes: 15 additions & 1 deletion test/extensions/filters/network/ext_authz/ext_authz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,11 @@ TEST_F(ExtAuthzFilterTest, DeniedWithOnData) {
stats_store_.gauge("ext_authz.name.active", Stats::Gauge::ImportMode::Accumulate).value());

EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush));
EXPECT_CALL(filter_callbacks_.connection_.stream_info_,
setResponseFlag(StreamInfo::ResponseFlag::UnauthorizedExternalService));
EXPECT_CALL(
filter_callbacks_.connection_.stream_info_,
setResponseCodeDetails(Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied));
EXPECT_CALL(*client_, cancel()).Times(0);
request_callbacks_->onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::Denied));

Expand Down Expand Up @@ -276,6 +281,11 @@ TEST_F(ExtAuthzFilterTest, FailClose) {

EXPECT_CALL(filter_callbacks_.connection_, close(_));
EXPECT_CALL(filter_callbacks_, continueReading()).Times(0);
EXPECT_CALL(filter_callbacks_.connection_.stream_info_,
setResponseFlag(StreamInfo::ResponseFlag::UnauthorizedExternalService));
EXPECT_CALL(
filter_callbacks_.connection_.stream_info_,
setResponseCodeDetails(Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzError));
request_callbacks_->onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::Error));

EXPECT_EQ(0U, stats_store_.counter("ext_authz.name.disabled").value());
Expand Down Expand Up @@ -429,7 +439,11 @@ TEST_F(ExtAuthzFilterTest, ImmediateNOK) {
EXPECT_EQ(ns, NetworkFilterNames::get().ExtAuthorization);
EXPECT_TRUE(TestUtility::protoEqual(returned_dynamic_metadata, dynamic_metadata));
}));

EXPECT_CALL(filter_callbacks_.connection_.stream_info_,
setResponseFlag(StreamInfo::ResponseFlag::UnauthorizedExternalService));
EXPECT_CALL(
filter_callbacks_.connection_.stream_info_,
setResponseCodeDetails(Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied));
EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection());
Buffer::OwnedImpl data("hello");
EXPECT_EQ(Network::FilterStatus::StopIteration, filter_->onData(data, false));
Expand Down