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
5 changes: 4 additions & 1 deletion source/extensions/filters/common/rbac/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@ class RoleBasedAccessControlEngine {
* @param headers the headers of the incoming request used to identify the action/principal. An
* empty map should be used if there are no headers available.
* @param metadata the metadata with additional information about the action/principal.
* @param effective_policy_id it will be filled by the matching policy's ID,
* which is used to identity the source of the allow/deny.
*/
virtual bool allowed(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers,
const envoy::api::v2::core::Metadata& metadata) const PURE;
const envoy::api::v2::core::Metadata& metadata,
std::string* effective_policy_id) const PURE;
};

} // namespace RBAC
Expand Down
17 changes: 11 additions & 6 deletions source/extensions/filters/common/rbac/engine_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,22 @@ RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl(
: allowed_if_matched_(rules.action() ==
envoy::config::rbac::v2alpha::RBAC_Action::RBAC_Action_ALLOW) {
for (const auto& policy : rules.policies()) {
policies_.emplace_back(policy.second);
policies_.insert(std::make_pair(policy.first, policy.second));
}
}

bool RoleBasedAccessControlEngineImpl::allowed(
const Network::Connection& connection, const Envoy::Http::HeaderMap& headers,
const envoy::api::v2::core::Metadata& metadata) const {
bool RoleBasedAccessControlEngineImpl::allowed(const Network::Connection& connection,
const Envoy::Http::HeaderMap& headers,
const envoy::api::v2::core::Metadata& metadata,
std::string* effective_policy_id) const {
bool matched = false;
for (const auto& policy : policies_) {
if (policy.matches(connection, headers, metadata)) {

for (auto it = policies_.begin(); it != policies_.end(); it++) {
if (it->second.matches(connection, headers, metadata)) {
matched = true;
if (effective_policy_id != nullptr) {
*effective_policy_id = it->first;
}
break;
}
}
Expand Down
5 changes: 3 additions & 2 deletions source/extensions/filters/common/rbac/engine_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ class RoleBasedAccessControlEngineImpl : public RoleBasedAccessControlEngine {
RoleBasedAccessControlEngineImpl(const envoy::config::rbac::v2alpha::RBAC& rules);

bool allowed(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers,
const envoy::api::v2::core::Metadata& metadata) const override;
const envoy::api::v2::core::Metadata& metadata,
std::string* effective_policy_id) const override;

private:
const bool allowed_if_matched_;

std::vector<PolicyMatcher> policies_;
std::map<std::string, PolicyMatcher> policies_;
};

} // namespace RBAC
Expand Down
31 changes: 29 additions & 2 deletions source/extensions/filters/http/rbac/rbac_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ namespace Extensions {
namespace HttpFilters {
namespace RBACFilter {

static const std::string resp_code_200 = "200";
static const std::string resp_code_403 = "403";
static const std::string shadow_policy_id_field = "shadow_effective_policyID";
static const std::string shadow_resp_code_field = "shadow_response_code";

RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig(
const envoy::config::filter::http::rbac::v2::RBAC& proto_config,
const std::string& stats_prefix, Stats::Scope& scope)
Expand Down Expand Up @@ -77,24 +82,46 @@ Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::Head
callbacks_->connection()->ssl()->subjectPeerCertificate()
: "none",
headers, callbacks_->requestInfo().dynamicMetadata().DebugString());
std::string effective_policy_id;
const absl::optional<Filters::Common::RBAC::RoleBasedAccessControlEngineImpl>& shadow_engine =
config_->engine(callbacks_->route(), EnforcementMode::Shadow);
if (shadow_engine.has_value()) {
std::string shadow_resp_code = resp_code_200;
if (shadow_engine->allowed(*callbacks_->connection(), headers,
callbacks_->requestInfo().dynamicMetadata())) {
callbacks_->requestInfo().dynamicMetadata(), &effective_policy_id)) {
ENVOY_LOG(debug, "shadow allowed");
config_->stats().shadow_allowed_.inc();
} else {
ENVOY_LOG(debug, "shadow denied");
config_->stats().shadow_denied_.inc();
shadow_resp_code = resp_code_403;
}

const auto& filter_metadata = callbacks_->requestInfo().dynamicMetadata().filter_metadata();
const auto filter_it = filter_metadata.find(HttpFilterNames::get().Rbac);
if (filter_it != filter_metadata.end()) {
ProtobufWkt::Struct metrics;

if (!effective_policy_id.empty()) {
ProtobufWkt::Value policy_id;
policy_id.set_string_value(effective_policy_id);
(*metrics.mutable_fields())[shadow_policy_id_field] = policy_id;
}

ProtobufWkt::Value resp_code;
resp_code.set_string_value(shadow_resp_code);
(*metrics.mutable_fields())[shadow_resp_code_field] = resp_code;

auto filter_meta = filter_metadata.at(HttpFilterNames::get().Rbac);
filter_meta.MergeFrom(metrics);
}
}

const absl::optional<Filters::Common::RBAC::RoleBasedAccessControlEngineImpl>& engine =
config_->engine(callbacks_->route(), EnforcementMode::Enforced);
if (engine.has_value()) {
if (engine->allowed(*callbacks_->connection(), headers,
callbacks_->requestInfo().dynamicMetadata())) {
callbacks_->requestInfo().dynamicMetadata(), nullptr)) {
ENVOY_LOG(debug, "enforced allowed");
config_->stats().allowed_.inc();
return Http::FilterHeadersStatus::Continue;
Expand Down
12 changes: 6 additions & 6 deletions test/extensions/filters/common/rbac/engine_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ namespace Common {
namespace RBAC {
namespace {

void checkEngine(
const RBAC::RoleBasedAccessControlEngineImpl& engine, bool expected,
const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(),
const Envoy::Http::HeaderMap& headers = Envoy::Http::HeaderMapImpl(),
const envoy::api::v2::core::Metadata& metadata = envoy::api::v2::core::Metadata()) {
EXPECT_EQ(expected, engine.allowed(connection, headers, metadata));
void checkEngine(const RBAC::RoleBasedAccessControlEngineImpl& engine, bool expected,
const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(),
const Envoy::Http::HeaderMap& headers = Envoy::Http::HeaderMapImpl(),
const envoy::api::v2::core::Metadata& metadata = envoy::api::v2::core::Metadata(),
std::string* policy_id = nullptr) {
EXPECT_EQ(expected, engine.allowed(connection, headers, metadata, policy_id));
}

TEST(RoleBasedAccessControlEngineImpl, Disabled) {
Expand Down
5 changes: 3 additions & 2 deletions test/extensions/filters/common/rbac/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ class MockEngine : public RoleBasedAccessControlEngineImpl {
MockEngine(const envoy::config::rbac::v2alpha::RBAC& rules)
: RoleBasedAccessControlEngineImpl(rules){};

MOCK_CONST_METHOD3(allowed, bool(const Envoy::Network::Connection&, const Envoy::Http::HeaderMap&,
const envoy::api::v2::core::Metadata&));
MOCK_CONST_METHOD4(allowed,
bool(const Envoy::Network::Connection&, const Envoy::Http::HeaderMap&,
const envoy::api::v2::core::Metadata&, std::string* effective_policy_id));
};

} // namespace RBAC
Expand Down
6 changes: 5 additions & 1 deletion test/extensions/filters/http/rbac/rbac_filter_test.cc
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "common/config/metadata.h"
#include "common/network/utility.h"

#include "extensions/filters/http/rbac/rbac_filter.h"
Expand Down Expand Up @@ -43,6 +44,7 @@ class RoleBasedAccessControlFilterTest : public testing::Test {

void SetUp() {
EXPECT_CALL(callbacks_, connection()).WillRepeatedly(Return(&connection_));
EXPECT_CALL(callbacks_, requestInfo()).WillRepeatedly(ReturnRef(req_info_));
filter_.setDecoderFilterCallbacks(callbacks_);
}

Expand All @@ -53,9 +55,11 @@ class RoleBasedAccessControlFilterTest : public testing::Test {

NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks_;
NiceMock<Network::MockConnection> connection_{};
NiceMock<Envoy::RequestInfo::MockRequestInfo> req_info_;
Stats::IsolatedStoreImpl store_;
RoleBasedAccessControlFilterConfigSharedPtr config_;

envoy::api::v2::core::Metadata metadata_;
RoleBasedAccessControlFilter filter_;
Network::Address::InstanceConstSharedPtr address_;
Http::TestHeaderMapImpl headers_;
Expand Down Expand Up @@ -98,7 +102,7 @@ TEST_F(RoleBasedAccessControlFilterTest, RouteLocalOverride) {
NiceMock<Filters::Common::RBAC::MockEngine> engine{route_config.rbac().rules()};
NiceMock<MockRoleBasedAccessControlRouteSpecificFilterConfig> per_route_config_{route_config};

EXPECT_CALL(engine, allowed(_, _, _)).WillRepeatedly(Return(true));
EXPECT_CALL(engine, allowed(_, _, _, _)).WillRepeatedly(Return(true));
EXPECT_CALL(per_route_config_, engine()).WillRepeatedly(ReturnRef(engine));

EXPECT_CALL(callbacks_.route_->route_entry_, perFilterConfig(HttpFilterNames::get().Rbac))
Expand Down