Skip to content

Commit

Permalink
Stringmatcher: factory context for rbac, dubbo_proxy (#32920)
Browse files Browse the repository at this point in the history 
Signed-off-by: Greg Greenway <[email protected]>
  • Loading branch information
@ggreenway
ggreenway authored Mar 15, 2024
1 parent 9e65865 commit 52fa026
Show file tree
Hide file tree
Showing 12 changed files with 208 additions and 131 deletions.
5 changes: 3 additions & 2 deletions source/extensions/filters/common/rbac/engine_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ void generateLog(StreamInfo::StreamInfo& info, EnforcementMode mode, bool log) {

RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl(
const envoy::config::rbac::v3::RBAC& rules,
ProtobufMessage::ValidationVisitor& validation_visitor, const EnforcementMode mode)
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context, const EnforcementMode mode)
: action_(rules.action()), mode_(mode) {
// guard expression builder by presence of a condition in policies
for (const auto& policy : rules.policies()) {
Expand All @@ -54,7 +55,7 @@ RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl(

for (const auto& policy : rules.policies()) {
policies_.emplace(policy.first, std::make_unique<PolicyMatcher>(policy.second, builder_.get(),
validation_visitor));
validation_visitor, context));
}
}

Expand Down
1 change: 1 addition & 0 deletions source/extensions/filters/common/rbac/engine_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class RoleBasedAccessControlEngineImpl : public RoleBasedAccessControlEngine, No
public:
RoleBasedAccessControlEngineImpl(const envoy::config::rbac::v3::RBAC& rules,
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context,
const EnforcementMode mode = EnforcementMode::Enforced);

bool handleAction(const Network::Connection& connection,
Expand Down
43 changes: 25 additions & 18 deletions source/extensions/filters/common/rbac/matchers.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ namespace Common {
namespace RBAC {

MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Permission& permission,
ProtobufMessage::ValidationVisitor& validation_visitor) {
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context) {
switch (permission.rule_case()) {
case envoy::config::rbac::v3::Permission::RuleCase::kAndRules:
return std::make_shared<const AndMatcher>(permission.and_rules(), validation_visitor);
return std::make_shared<const AndMatcher>(permission.and_rules(), validation_visitor, context);
case envoy::config::rbac::v3::Permission::RuleCase::kOrRules:
return std::make_shared<const OrMatcher>(permission.or_rules(), validation_visitor);
return std::make_shared<const OrMatcher>(permission.or_rules(), validation_visitor, context);
case envoy::config::rbac::v3::Permission::RuleCase::kHeader:
return std::make_shared<const HeaderMatcher>(permission.header());
case envoy::config::rbac::v3::Permission::RuleCase::kDestinationIp:
Expand All @@ -33,9 +34,10 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Permission&
case envoy::config::rbac::v3::Permission::RuleCase::kMetadata:
return std::make_shared<const MetadataMatcher>(permission.metadata());
case envoy::config::rbac::v3::Permission::RuleCase::kNotRule:
return std::make_shared<const NotMatcher>(permission.not_rule(), validation_visitor);
return std::make_shared<const NotMatcher>(permission.not_rule(), validation_visitor, context);
case envoy::config::rbac::v3::Permission::RuleCase::kRequestedServerName:
return std::make_shared<const RequestedServerNameMatcher>(permission.requested_server_name());
return std::make_shared<const RequestedServerNameMatcher>(permission.requested_server_name(),
context);
case envoy::config::rbac::v3::Permission::RuleCase::kUrlPath:
return std::make_shared<const PathMatcher>(permission.url_path());
case envoy::config::rbac::v3::Permission::RuleCase::kUriTemplate: {
Expand All @@ -56,14 +58,15 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Permission&
PANIC_DUE_TO_CORRUPT_ENUM;
}

MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Principal& principal) {
MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Principal& principal,
Server::Configuration::CommonFactoryContext& context) {
switch (principal.identifier_case()) {
case envoy::config::rbac::v3::Principal::IdentifierCase::kAndIds:
return std::make_shared<const AndMatcher>(principal.and_ids());
return std::make_shared<const AndMatcher>(principal.and_ids(), context);
case envoy::config::rbac::v3::Principal::IdentifierCase::kOrIds:
return std::make_shared<const OrMatcher>(principal.or_ids());
return std::make_shared<const OrMatcher>(principal.or_ids(), context);
case envoy::config::rbac::v3::Principal::IdentifierCase::kAuthenticated:
return std::make_shared<const AuthenticatedMatcher>(principal.authenticated());
return std::make_shared<const AuthenticatedMatcher>(principal.authenticated(), context);
case envoy::config::rbac::v3::Principal::IdentifierCase::kSourceIp:
return std::make_shared<const IPMatcher>(principal.source_ip(),
IPMatcher::Type::ConnectionRemote);
Expand All @@ -80,7 +83,7 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Principal&
case envoy::config::rbac::v3::Principal::IdentifierCase::kMetadata:
return std::make_shared<const MetadataMatcher>(principal.metadata());
case envoy::config::rbac::v3::Principal::IdentifierCase::kNotId:
return std::make_shared<const NotMatcher>(principal.not_id());
return std::make_shared<const NotMatcher>(principal.not_id(), context);
case envoy::config::rbac::v3::Principal::IdentifierCase::kUrlPath:
return std::make_shared<const PathMatcher>(principal.url_path());
case envoy::config::rbac::v3::Principal::IdentifierCase::kFilterState:
Expand All @@ -92,15 +95,17 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Principal&
}

AndMatcher::AndMatcher(const envoy::config::rbac::v3::Permission::Set& set,
ProtobufMessage::ValidationVisitor& validation_visitor) {
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context) {
for (const auto& rule : set.rules()) {
matchers_.push_back(Matcher::create(rule, validation_visitor));
matchers_.push_back(Matcher::create(rule, validation_visitor, context));
}
}

AndMatcher::AndMatcher(const envoy::config::rbac::v3::Principal::Set& set) {
AndMatcher::AndMatcher(const envoy::config::rbac::v3::Principal::Set& set,
Server::Configuration::CommonFactoryContext& context) {
for (const auto& id : set.ids()) {
matchers_.push_back(Matcher::create(id));
matchers_.push_back(Matcher::create(id, context));
}
}

Expand All @@ -117,15 +122,17 @@ bool AndMatcher::matches(const Network::Connection& connection,
}

OrMatcher::OrMatcher(const Protobuf::RepeatedPtrField<envoy::config::rbac::v3::Permission>& rules,
ProtobufMessage::ValidationVisitor& validation_visitor) {
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context) {
for (const auto& rule : rules) {
matchers_.push_back(Matcher::create(rule, validation_visitor));
matchers_.push_back(Matcher::create(rule, validation_visitor, context));
}
}

OrMatcher::OrMatcher(const Protobuf::RepeatedPtrField<envoy::config::rbac::v3::Principal>& ids) {
OrMatcher::OrMatcher(const Protobuf::RepeatedPtrField<envoy::config::rbac::v3::Principal>& ids,
Server::Configuration::CommonFactoryContext& context) {
for (const auto& id : ids) {
matchers_.push_back(Matcher::create(id));
matchers_.push_back(Matcher::create(id, context));
}
}

Expand Down
64 changes: 39 additions & 25 deletions source/extensions/filters/common/rbac/matchers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,15 @@ class Matcher {
* proto message.
*/
static MatcherConstSharedPtr create(const envoy::config::rbac::v3::Permission& permission,
ProtobufMessage::ValidationVisitor& validation_visitor);
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context);

/**
* Creates a shared instance of a matcher based off the rules defined in the Principal config
* proto message.
*/
static MatcherConstSharedPtr create(const envoy::config::rbac::v3::Principal& principal);
static MatcherConstSharedPtr create(const envoy::config::rbac::v3::Principal& principal,
Server::Configuration::CommonFactoryContext& context);
};

/**
Expand All @@ -76,8 +78,10 @@ class AlwaysMatcher : public Matcher {
class AndMatcher : public Matcher {
public:
AndMatcher(const envoy::config::rbac::v3::Permission::Set& rules,
ProtobufMessage::ValidationVisitor& validation_visitor);
AndMatcher(const envoy::config::rbac::v3::Principal::Set& ids);
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context);
AndMatcher(const envoy::config::rbac::v3::Principal::Set& ids,
Server::Configuration::CommonFactoryContext& context);

bool matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers,
const StreamInfo::StreamInfo&) const override;
Expand All @@ -93,12 +97,17 @@ class AndMatcher : public Matcher {
class OrMatcher : public Matcher {
public:
OrMatcher(const envoy::config::rbac::v3::Permission::Set& set,
ProtobufMessage::ValidationVisitor& validation_visitor)
: OrMatcher(set.rules(), validation_visitor) {}
OrMatcher(const envoy::config::rbac::v3::Principal::Set& set) : OrMatcher(set.ids()) {}
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context)
: OrMatcher(set.rules(), validation_visitor, context) {}
OrMatcher(const envoy::config::rbac::v3::Principal::Set& set,
Server::Configuration::CommonFactoryContext& context)
: OrMatcher(set.ids(), context) {}
OrMatcher(const Protobuf::RepeatedPtrField<envoy::config::rbac::v3::Permission>& rules,
ProtobufMessage::ValidationVisitor& validation_visitor);
OrMatcher(const Protobuf::RepeatedPtrField<envoy::config::rbac::v3::Principal>& ids);
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context);
OrMatcher(const Protobuf::RepeatedPtrField<envoy::config::rbac::v3::Principal>& ids,
Server::Configuration::CommonFactoryContext& context);

bool matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers,
const StreamInfo::StreamInfo&) const override;
Expand All @@ -110,10 +119,12 @@ class OrMatcher : public Matcher {
class NotMatcher : public Matcher {
public:
NotMatcher(const envoy::config::rbac::v3::Permission& permission,
ProtobufMessage::ValidationVisitor& validation_visitor)
: matcher_(Matcher::create(permission, validation_visitor)) {}
NotMatcher(const envoy::config::rbac::v3::Principal& principal)
: matcher_(Matcher::create(principal)) {}
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context)
: matcher_(Matcher::create(permission, validation_visitor, context)) {}
NotMatcher(const envoy::config::rbac::v3::Principal& principal,
Server::Configuration::CommonFactoryContext& context)
: matcher_(Matcher::create(principal, context)) {}

bool matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers,
const StreamInfo::StreamInfo&) const override;
Expand Down Expand Up @@ -188,18 +199,19 @@ class PortRangeMatcher : public Matcher {
*/
class AuthenticatedMatcher : public Matcher {
public:
AuthenticatedMatcher(const envoy::config::rbac::v3::Principal::Authenticated& auth)
AuthenticatedMatcher(const envoy::config::rbac::v3::Principal::Authenticated& auth,
Server::Configuration::CommonFactoryContext& context)
: matcher_(auth.has_principal_name()
? absl::make_optional<
Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher>>(
auth.principal_name())
? absl::make_optional<Matchers::StringMatcherImplWithContext<
envoy::type::matcher::v3::StringMatcher>>(auth.principal_name(), context)
: absl::nullopt) {}

bool matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers,
const StreamInfo::StreamInfo&) const override;

private:
const absl::optional<Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher>>
const absl::optional<
Matchers::StringMatcherImplWithContext<envoy::type::matcher::v3::StringMatcher>>
matcher_;
};

Expand All @@ -211,9 +223,10 @@ class AuthenticatedMatcher : public Matcher {
class PolicyMatcher : public Matcher, NonCopyable {
public:
PolicyMatcher(const envoy::config::rbac::v3::Policy& policy, Expr::Builder* builder,
ProtobufMessage::ValidationVisitor& validation_visitor)
: permissions_(policy.permissions(), validation_visitor), principals_(policy.principals()),
condition_(policy.condition()) {
ProtobufMessage::ValidationVisitor& validation_visitor,
Server::Configuration::CommonFactoryContext& context)
: permissions_(policy.permissions(), validation_visitor, context),
principals_(policy.principals(), context), condition_(policy.condition()) {
if (policy.has_condition()) {
expr_ = Expr::createExpression(*builder, condition_);
}
Expand Down Expand Up @@ -258,11 +271,12 @@ class FilterStateMatcher : public Matcher {
*/
class RequestedServerNameMatcher
: public Matcher,
Envoy::Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher> {
Envoy::Matchers::StringMatcherImplWithContext<envoy::type::matcher::v3::StringMatcher> {
public:
RequestedServerNameMatcher(const envoy::type::matcher::v3::StringMatcher& requested_server_name)
: Envoy::Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher>(
requested_server_name) {}
RequestedServerNameMatcher(const envoy::type::matcher::v3::StringMatcher& requested_server_name,
Server::Configuration::CommonFactoryContext& context)
: Envoy::Matchers::StringMatcherImplWithContext<envoy::type::matcher::v3::StringMatcher>(
requested_server_name, context) {}

bool matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers,
const StreamInfo::StreamInfo&) const override;
Expand Down
4 changes: 2 additions & 2 deletions source/extensions/filters/common/rbac/utility.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ createEngine(const ConfigType& config, Server::Configuration::ServerFactoryConte
}
if (config.has_rules()) {
return std::make_unique<RoleBasedAccessControlEngineImpl>(config.rules(), validation_visitor,
EnforcementMode::Enforced);
context, EnforcementMode::Enforced);
}

return nullptr;
Expand All @@ -71,7 +71,7 @@ createShadowEngine(const ConfigType& config, Server::Configuration::ServerFactor
}
if (config.has_shadow_rules()) {
return std::make_unique<RoleBasedAccessControlEngineImpl>(
config.shadow_rules(), validation_visitor, EnforcementMode::Shadow);
config.shadow_rules(), validation_visitor, context, EnforcementMode::Shadow);
}

return nullptr;
Expand Down
9 changes: 5 additions & 4 deletions source/extensions/filters/network/dubbo_proxy/router/route_matcher.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,9 @@ ParameterRouteEntryImpl::ParameterData::ParameterData(uint32_t index,
}

MethodRouteEntryImpl::MethodRouteEntryImpl(
const envoy::extensions::filters::network::dubbo_proxy::v3::Route& route)
: RouteEntryImplBase(route), method_name_(route.match().method().name()) {
const envoy::extensions::filters::network::dubbo_proxy::v3::Route& route,
Server::Configuration::CommonFactoryContext& context)
: RouteEntryImplBase(route), method_name_(route.match().method().name(), context) {
if (route.match().method().params_match_size() != 0) {
parameter_route_ = std::make_shared<ParameterRouteEntryImpl>(route);
}
Expand Down Expand Up @@ -206,12 +207,12 @@ RouteConstSharedPtr MethodRouteEntryImpl::matches(const MessageMetadata& metadat
}

SingleRouteMatcherImpl::SingleRouteMatcherImpl(const RouteConfig& config,
Server::Configuration::ServerFactoryContext&)
Server::Configuration::ServerFactoryContext& context)
: interface_matcher_(config.interface()), group_(config.group()), version_(config.version()) {
using envoy::extensions::filters::network::dubbo_proxy::v3::RouteMatch;

for (const auto& route : config.routes()) {
routes_.emplace_back(std::make_shared<MethodRouteEntryImpl>(route));
routes_.emplace_back(std::make_shared<MethodRouteEntryImpl>(route, context));
}
ENVOY_LOG(debug, "dubbo route matcher: routes list size {}", routes_.size());
}
Expand Down
6 changes: 4 additions & 2 deletions source/extensions/filters/network/dubbo_proxy/router/route_matcher.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,17 @@ class ParameterRouteEntryImpl : public RouteEntryImplBase {

class MethodRouteEntryImpl : public RouteEntryImplBase {
public:
MethodRouteEntryImpl(const envoy::extensions::filters::network::dubbo_proxy::v3::Route& route);
MethodRouteEntryImpl(const envoy::extensions::filters::network::dubbo_proxy::v3::Route& route,
Server::Configuration::CommonFactoryContext& context);
~MethodRouteEntryImpl() override;

// RoutEntryImplBase
RouteConstSharedPtr matches(const MessageMetadata& metadata,
uint64_t random_value) const override;

private:
const Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher> method_name_;
const Matchers::StringMatcherImplWithContext<envoy::type::matcher::v3::StringMatcher>
method_name_;
std::shared_ptr<ParameterRouteEntryImpl> parameter_route_;
};

Expand Down
1 change: 1 addition & 0 deletions test/extensions/filters/common/rbac/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ envoy_extension_cc_test(
"//source/extensions/filters/common/expr:evaluator_lib",
"//source/extensions/filters/common/rbac:matchers_lib",
"//test/mocks/network:network_mocks",
"//test/mocks/server:server_factory_context_mocks",
"//test/mocks/ssl:ssl_mocks",
"//test/test_common:utility_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
Expand Down
Loading

0 comments on commit 52fa026

Please sign in to comment.