diff --git a/source/extensions/filters/common/rbac/engine_impl.cc b/source/extensions/filters/common/rbac/engine_impl.cc index 76d010c562c7..f4b1f9e11bdd 100644 --- a/source/extensions/filters/common/rbac/engine_impl.cc +++ b/source/extensions/filters/common/rbac/engine_impl.cc @@ -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()) { @@ -54,7 +55,7 @@ RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl( for (const auto& policy : rules.policies()) { policies_.emplace(policy.first, std::make_unique(policy.second, builder_.get(), - validation_visitor)); + validation_visitor, context)); } } diff --git a/source/extensions/filters/common/rbac/engine_impl.h b/source/extensions/filters/common/rbac/engine_impl.h index c748142c8a97..7f51648caa47 100644 --- a/source/extensions/filters/common/rbac/engine_impl.h +++ b/source/extensions/filters/common/rbac/engine_impl.h @@ -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, diff --git a/source/extensions/filters/common/rbac/matchers.cc b/source/extensions/filters/common/rbac/matchers.cc index 5df6db1bfe37..47ffb5ec57b7 100644 --- a/source/extensions/filters/common/rbac/matchers.cc +++ b/source/extensions/filters/common/rbac/matchers.cc @@ -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(permission.and_rules(), validation_visitor); + return std::make_shared(permission.and_rules(), validation_visitor, context); case envoy::config::rbac::v3::Permission::RuleCase::kOrRules: - return std::make_shared(permission.or_rules(), validation_visitor); + return std::make_shared(permission.or_rules(), validation_visitor, context); case envoy::config::rbac::v3::Permission::RuleCase::kHeader: return std::make_shared(permission.header()); case envoy::config::rbac::v3::Permission::RuleCase::kDestinationIp: @@ -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(permission.metadata()); case envoy::config::rbac::v3::Permission::RuleCase::kNotRule: - return std::make_shared(permission.not_rule(), validation_visitor); + return std::make_shared(permission.not_rule(), validation_visitor, context); case envoy::config::rbac::v3::Permission::RuleCase::kRequestedServerName: - return std::make_shared(permission.requested_server_name()); + return std::make_shared(permission.requested_server_name(), + context); case envoy::config::rbac::v3::Permission::RuleCase::kUrlPath: return std::make_shared(permission.url_path()); case envoy::config::rbac::v3::Permission::RuleCase::kUriTemplate: { @@ -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(principal.and_ids()); + return std::make_shared(principal.and_ids(), context); case envoy::config::rbac::v3::Principal::IdentifierCase::kOrIds: - return std::make_shared(principal.or_ids()); + return std::make_shared(principal.or_ids(), context); case envoy::config::rbac::v3::Principal::IdentifierCase::kAuthenticated: - return std::make_shared(principal.authenticated()); + return std::make_shared(principal.authenticated(), context); case envoy::config::rbac::v3::Principal::IdentifierCase::kSourceIp: return std::make_shared(principal.source_ip(), IPMatcher::Type::ConnectionRemote); @@ -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(principal.metadata()); case envoy::config::rbac::v3::Principal::IdentifierCase::kNotId: - return std::make_shared(principal.not_id()); + return std::make_shared(principal.not_id(), context); case envoy::config::rbac::v3::Principal::IdentifierCase::kUrlPath: return std::make_shared(principal.url_path()); case envoy::config::rbac::v3::Principal::IdentifierCase::kFilterState: @@ -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)); } } @@ -117,15 +122,17 @@ bool AndMatcher::matches(const Network::Connection& connection, } OrMatcher::OrMatcher(const Protobuf::RepeatedPtrField& 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& ids) { +OrMatcher::OrMatcher(const Protobuf::RepeatedPtrField& ids, + Server::Configuration::CommonFactoryContext& context) { for (const auto& id : ids) { - matchers_.push_back(Matcher::create(id)); + matchers_.push_back(Matcher::create(id, context)); } } diff --git a/source/extensions/filters/common/rbac/matchers.h b/source/extensions/filters/common/rbac/matchers.h index b5914f397f3f..bc39bc94c97e 100644 --- a/source/extensions/filters/common/rbac/matchers.h +++ b/source/extensions/filters/common/rbac/matchers.h @@ -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); }; /** @@ -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; @@ -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& rules, - ProtobufMessage::ValidationVisitor& validation_visitor); - OrMatcher(const Protobuf::RepeatedPtrField& ids); + ProtobufMessage::ValidationVisitor& validation_visitor, + Server::Configuration::CommonFactoryContext& context); + OrMatcher(const Protobuf::RepeatedPtrField& ids, + Server::Configuration::CommonFactoryContext& context); bool matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo&) const override; @@ -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; @@ -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>( - auth.principal_name()) + ? absl::make_optional>(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> + const absl::optional< + Matchers::StringMatcherImplWithContext> matcher_; }; @@ -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_); } @@ -258,11 +271,12 @@ class FilterStateMatcher : public Matcher { */ class RequestedServerNameMatcher : public Matcher, - Envoy::Matchers::StringMatcherImpl { + Envoy::Matchers::StringMatcherImplWithContext { public: - RequestedServerNameMatcher(const envoy::type::matcher::v3::StringMatcher& requested_server_name) - : Envoy::Matchers::StringMatcherImpl( - requested_server_name) {} + RequestedServerNameMatcher(const envoy::type::matcher::v3::StringMatcher& requested_server_name, + Server::Configuration::CommonFactoryContext& context) + : Envoy::Matchers::StringMatcherImplWithContext( + requested_server_name, context) {} bool matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo&) const override; diff --git a/source/extensions/filters/common/rbac/utility.h b/source/extensions/filters/common/rbac/utility.h index ac2a339226ca..79cd8be7d2bf 100644 --- a/source/extensions/filters/common/rbac/utility.h +++ b/source/extensions/filters/common/rbac/utility.h @@ -51,7 +51,7 @@ createEngine(const ConfigType& config, Server::Configuration::ServerFactoryConte } if (config.has_rules()) { return std::make_unique(config.rules(), validation_visitor, - EnforcementMode::Enforced); + context, EnforcementMode::Enforced); } return nullptr; @@ -71,7 +71,7 @@ createShadowEngine(const ConfigType& config, Server::Configuration::ServerFactor } if (config.has_shadow_rules()) { return std::make_unique( - config.shadow_rules(), validation_visitor, EnforcementMode::Shadow); + config.shadow_rules(), validation_visitor, context, EnforcementMode::Shadow); } return nullptr; diff --git a/source/extensions/filters/network/dubbo_proxy/router/route_matcher.cc b/source/extensions/filters/network/dubbo_proxy/router/route_matcher.cc index 334236048f21..b58abc50dfc1 100644 --- a/source/extensions/filters/network/dubbo_proxy/router/route_matcher.cc +++ b/source/extensions/filters/network/dubbo_proxy/router/route_matcher.cc @@ -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(route); } @@ -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(route)); + routes_.emplace_back(std::make_shared(route, context)); } ENVOY_LOG(debug, "dubbo route matcher: routes list size {}", routes_.size()); } diff --git a/source/extensions/filters/network/dubbo_proxy/router/route_matcher.h b/source/extensions/filters/network/dubbo_proxy/router/route_matcher.h index 8b37eb2a2f23..b5fbcb2c7004 100644 --- a/source/extensions/filters/network/dubbo_proxy/router/route_matcher.h +++ b/source/extensions/filters/network/dubbo_proxy/router/route_matcher.h @@ -118,7 +118,8 @@ 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 @@ -126,7 +127,8 @@ class MethodRouteEntryImpl : public RouteEntryImplBase { uint64_t random_value) const override; private: - const Matchers::StringMatcherImpl method_name_; + const Matchers::StringMatcherImplWithContext + method_name_; std::shared_ptr parameter_route_; }; diff --git a/test/extensions/filters/common/rbac/BUILD b/test/extensions/filters/common/rbac/BUILD index 3cdc7f4eb9ff..ca59ee1f59d8 100644 --- a/test/extensions/filters/common/rbac/BUILD +++ b/test/extensions/filters/common/rbac/BUILD @@ -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", diff --git a/test/extensions/filters/common/rbac/engine_impl_test.cc b/test/extensions/filters/common/rbac/engine_impl_test.cc index 872336d6744f..957229877617 100644 --- a/test/extensions/filters/common/rbac/engine_impl_test.cc +++ b/test/extensions/filters/common/rbac/engine_impl_test.cc @@ -100,15 +100,16 @@ void onMetadata(NiceMock& info) { } TEST(RoleBasedAccessControlEngineImpl, Disabled) { + Server::Configuration::MockServerFactoryContext factory_context; envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); RBAC::RoleBasedAccessControlEngineImpl engine_allow( - rbac, ProtobufMessage::getStrictValidationVisitor()); + rbac, ProtobufMessage::getStrictValidationVisitor(), factory_context); checkEngine(engine_allow, false, LogResult::Undecided); rbac.set_action(envoy::config::rbac::v3::RBAC::DENY); - RBAC::RoleBasedAccessControlEngineImpl engine_deny(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine_deny( + rbac, ProtobufMessage::getStrictValidationVisitor(), factory_context); checkEngine(engine_deny, true, LogResult::Undecided); } @@ -199,6 +200,7 @@ TEST(RoleBasedAccessControlEngineImpl, InvalidConfig) { } TEST(RoleBasedAccessControlEngineImpl, AllowedAllowlist) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_destination_port(123); policy.add_principals()->set_any(true); @@ -206,8 +208,8 @@ TEST(RoleBasedAccessControlEngineImpl, AllowedAllowlist) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; @@ -223,6 +225,7 @@ TEST(RoleBasedAccessControlEngineImpl, AllowedAllowlist) { } TEST(RoleBasedAccessControlEngineImpl, DeniedDenylist) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_destination_port(123); policy.add_principals()->set_any(true); @@ -230,8 +233,8 @@ TEST(RoleBasedAccessControlEngineImpl, DeniedDenylist) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::DENY); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; @@ -247,6 +250,7 @@ TEST(RoleBasedAccessControlEngineImpl, DeniedDenylist) { } TEST(RoleBasedAccessControlEngineImpl, BasicCondition) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_any(true); policy.add_principals()->set_any(true); @@ -259,12 +263,13 @@ TEST(RoleBasedAccessControlEngineImpl, BasicCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); checkEngine(engine, false, LogResult::Undecided); } TEST(RoleBasedAccessControlEngineImpl, MalformedCondition) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_any(true); policy.add_principals()->set_any(true); @@ -282,16 +287,17 @@ TEST(RoleBasedAccessControlEngineImpl, MalformedCondition) { (*rbac.mutable_policies())["foo"] = policy; EXPECT_THROW_WITH_REGEX(RBAC::RoleBasedAccessControlEngineImpl engine( - rbac, ProtobufMessage::getStrictValidationVisitor()), + rbac, ProtobufMessage::getStrictValidationVisitor(), factory_context), EnvoyException, "failed to create an expression: .*"); rbac.set_action(envoy::config::rbac::v3::RBAC::LOG); EXPECT_THROW_WITH_REGEX(RBAC::RoleBasedAccessControlEngineImpl engine_log( - rbac, ProtobufMessage::getStrictValidationVisitor()), + rbac, ProtobufMessage::getStrictValidationVisitor(), factory_context), EnvoyException, "failed to create an expression: .*"); } TEST(RoleBasedAccessControlEngineImpl, MistypedCondition) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_any(true); policy.add_principals()->set_any(true); @@ -304,12 +310,13 @@ TEST(RoleBasedAccessControlEngineImpl, MistypedCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); checkEngine(engine, false, LogResult::Undecided); } TEST(RoleBasedAccessControlEngineImpl, EvaluationFailure) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_any(true); policy.add_principals()->set_any(true); @@ -325,12 +332,13 @@ TEST(RoleBasedAccessControlEngineImpl, EvaluationFailure) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); checkEngine(engine, false, LogResult::Undecided); } TEST(RoleBasedAccessControlEngineImpl, ErrorCondition) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_any(true); policy.add_principals()->set_any(true); @@ -351,12 +359,13 @@ TEST(RoleBasedAccessControlEngineImpl, ErrorCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); checkEngine(engine, false, LogResult::Undecided, Envoy::Network::MockConnection()); } TEST(RoleBasedAccessControlEngineImpl, HeaderCondition) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_any(true); policy.add_principals()->set_any(true); @@ -382,8 +391,8 @@ TEST(RoleBasedAccessControlEngineImpl, HeaderCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); Envoy::Http::TestRequestHeaderMapImpl headers; Envoy::Http::LowerCaseString key("foo"); @@ -394,6 +403,7 @@ TEST(RoleBasedAccessControlEngineImpl, HeaderCondition) { } TEST(RoleBasedAccessControlEngineImpl, MetadataCondition) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_any(true); policy.add_principals()->set_any(true); @@ -424,8 +434,8 @@ TEST(RoleBasedAccessControlEngineImpl, MetadataCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); Envoy::Http::TestRequestHeaderMapImpl headers; NiceMock info; @@ -440,6 +450,7 @@ TEST(RoleBasedAccessControlEngineImpl, MetadataCondition) { } TEST(RoleBasedAccessControlEngineImpl, ConjunctiveCondition) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_destination_port(123); policy.add_principals()->set_any(true); @@ -452,8 +463,8 @@ TEST(RoleBasedAccessControlEngineImpl, ConjunctiveCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; @@ -572,17 +583,19 @@ TEST(RoleBasedAccessControlMatcherEngineImpl, DeniedDenylist) { // Log tests TEST(RoleBasedAccessControlEngineImpl, DisabledLog) { + NiceMock factory_context; NiceMock info; onMetadata(info); envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::LOG); - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); checkEngine(engine, true, RBAC::LogResult::No, info); } TEST(RoleBasedAccessControlEngineImpl, LogIfMatched) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_destination_port(123); policy.add_principals()->set_any(true); @@ -590,8 +603,8 @@ TEST(RoleBasedAccessControlEngineImpl, LogIfMatched) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::LOG); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac, - ProtobufMessage::getStrictValidationVisitor()); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, ProtobufMessage::getStrictValidationVisitor(), + factory_context); Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 2ef2b577fcea..c64d6ec9818f 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -11,6 +11,7 @@ #include "source/extensions/filters/common/rbac/matchers.h" #include "test/mocks/network/mocks.h" +#include "test/mocks/server/server_factory_context.h" #include "test/mocks/ssl/mocks.h" #include "gmock/gmock.h" @@ -40,11 +41,13 @@ PortRangeMatcher createPortRangeMatcher(envoy::type::v3::Int32Range range) { ret TEST(AlwaysMatcher, AlwaysMatches) { checkMatcher(RBAC::AlwaysMatcher(), true); } TEST(AndMatcher, Permission_Set) { + NiceMock factory_context; envoy::config::rbac::v3::Permission::Set set; envoy::config::rbac::v3::Permission* perm = set.add_rules(); perm->set_any(true); - checkMatcher(RBAC::AndMatcher(set, ProtobufMessage::getStrictValidationVisitor()), true); + checkMatcher( + RBAC::AndMatcher(set, ProtobufMessage::getStrictValidationVisitor(), factory_context), true); perm = set.add_rules(); perm->set_destination_port(123); @@ -56,22 +59,25 @@ TEST(AndMatcher, Permission_Set) { Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 123, false); info.downstream_connection_info_provider_->setLocalAddress(addr); - checkMatcher(RBAC::AndMatcher(set, ProtobufMessage::getStrictValidationVisitor()), true, conn, - headers, info); + checkMatcher( + RBAC::AndMatcher(set, ProtobufMessage::getStrictValidationVisitor(), factory_context), true, + conn, headers, info); addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 8080, false); info.downstream_connection_info_provider_->setLocalAddress(addr); - checkMatcher(RBAC::AndMatcher(set, ProtobufMessage::getStrictValidationVisitor()), false, conn, - headers, info); + checkMatcher( + RBAC::AndMatcher(set, ProtobufMessage::getStrictValidationVisitor(), factory_context), false, + conn, headers, info); } TEST(AndMatcher, Principal_Set) { + NiceMock factory_context; envoy::config::rbac::v3::Principal::Set set; envoy::config::rbac::v3::Principal* principal = set.add_ids(); principal->set_any(true); - checkMatcher(RBAC::AndMatcher(set), true); + checkMatcher(RBAC::AndMatcher(set, factory_context), true); principal = set.add_ids(); auto* cidr = principal->mutable_direct_remote_ip(); @@ -85,15 +91,16 @@ TEST(AndMatcher, Principal_Set) { Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 123, false); info.downstream_connection_info_provider_->setDirectRemoteAddressForTest(addr); - checkMatcher(RBAC::AndMatcher(set), true, conn, headers, info); + checkMatcher(RBAC::AndMatcher(set, factory_context), true, conn, headers, info); addr = Envoy::Network::Utility::parseInternetAddress("1.2.4.6", 123, false); info.downstream_connection_info_provider_->setDirectRemoteAddressForTest(addr); - checkMatcher(RBAC::AndMatcher(set), false, conn, headers, info); + checkMatcher(RBAC::AndMatcher(set, factory_context), false, conn, headers, info); } TEST(OrMatcher, Permission_Set) { + NiceMock factory_context; envoy::config::rbac::v3::Permission::Set set; envoy::config::rbac::v3::Permission* perm = set.add_rules(); perm->set_destination_port(123); @@ -105,21 +112,21 @@ TEST(OrMatcher, Permission_Set) { Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 456, false); info.downstream_connection_info_provider_->setLocalAddress(addr); - checkMatcher(RBAC::OrMatcher(set, ProtobufMessage::getStrictValidationVisitor()), false, conn, - headers, info); + checkMatcher(RBAC::OrMatcher(set, ProtobufMessage::getStrictValidationVisitor(), factory_context), + false, conn, headers, info); perm = set.add_rules(); perm->mutable_destination_port_range()->set_start(123); perm->mutable_destination_port_range()->set_end(456); - checkMatcher(RBAC::OrMatcher(set, ProtobufMessage::getStrictValidationVisitor()), false, conn, - headers, info); + checkMatcher(RBAC::OrMatcher(set, ProtobufMessage::getStrictValidationVisitor(), factory_context), + false, conn, headers, info); perm = set.add_rules(); perm->set_any(true); - checkMatcher(RBAC::OrMatcher(set, ProtobufMessage::getStrictValidationVisitor()), true, conn, - headers, info); + checkMatcher(RBAC::OrMatcher(set, ProtobufMessage::getStrictValidationVisitor(), factory_context), + true, conn, headers, info); } TEST(OrMatcher, Principal_Set) { @@ -129,6 +136,7 @@ TEST(OrMatcher, Principal_Set) { cidr->set_address_prefix("1.2.3.0"); cidr->mutable_prefix_len()->set_value(24); + NiceMock factory_context; Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; NiceMock info; @@ -136,27 +144,31 @@ TEST(OrMatcher, Principal_Set) { Envoy::Network::Utility::parseInternetAddress("1.2.4.6", 456, false); info.downstream_connection_info_provider_->setDirectRemoteAddressForTest(addr); - checkMatcher(RBAC::OrMatcher(set), false, conn, headers, info); + checkMatcher(RBAC::OrMatcher(set, factory_context), false, conn, headers, info); id = set.add_ids(); id->set_any(true); - checkMatcher(RBAC::OrMatcher(set), true, conn, headers, info); + checkMatcher(RBAC::OrMatcher(set, factory_context), true, conn, headers, info); } TEST(NotMatcher, Permission) { + NiceMock factory_context; envoy::config::rbac::v3::Permission perm; perm.set_any(true); - checkMatcher(RBAC::NotMatcher(perm, ProtobufMessage::getStrictValidationVisitor()), false, - Envoy::Network::MockConnection()); + checkMatcher( + RBAC::NotMatcher(perm, ProtobufMessage::getStrictValidationVisitor(), factory_context), false, + Envoy::Network::MockConnection()); } TEST(NotMatcher, Principal) { + NiceMock factory_context; envoy::config::rbac::v3::Principal principal; principal.set_any(true); - checkMatcher(RBAC::NotMatcher(principal), false, Envoy::Network::MockConnection()); + checkMatcher(RBAC::NotMatcher(principal, factory_context), false, + Envoy::Network::MockConnection()); } TEST(HeaderMatcher, HeaderMatcher) { @@ -315,15 +327,16 @@ TEST(AuthenticatedMatcher, uriSanPeerCertificate) { EXPECT_CALL(Const(conn), ssl()).WillRepeatedly(Return(ssl)); // We should check if any URI SAN matches. + NiceMock factory_context; envoy::config::rbac::v3::Principal::Authenticated auth; auth.mutable_principal_name()->set_exact("foo"); - checkMatcher(AuthenticatedMatcher(auth), true, conn); + checkMatcher(AuthenticatedMatcher(auth, factory_context), true, conn); auth.mutable_principal_name()->set_exact("baz"); - checkMatcher(AuthenticatedMatcher(auth), true, conn); + checkMatcher(AuthenticatedMatcher(auth, factory_context), true, conn); auth.mutable_principal_name()->set_exact("bar"); - checkMatcher(AuthenticatedMatcher(auth), false, conn); + checkMatcher(AuthenticatedMatcher(auth, factory_context), false, conn); } TEST(AuthenticatedMatcher, dnsSanPeerCertificate) { @@ -344,14 +357,15 @@ TEST(AuthenticatedMatcher, dnsSanPeerCertificate) { // We should get check if any DNS SAN matches as URI SAN is not available. envoy::config::rbac::v3::Principal::Authenticated auth; + NiceMock factory_context; auth.mutable_principal_name()->set_exact("foo"); - checkMatcher(AuthenticatedMatcher(auth), true, conn); + checkMatcher(AuthenticatedMatcher(auth, factory_context), true, conn); auth.mutable_principal_name()->set_exact("baz"); - checkMatcher(AuthenticatedMatcher(auth), true, conn); + checkMatcher(AuthenticatedMatcher(auth, factory_context), true, conn); auth.mutable_principal_name()->set_exact("bar"); - checkMatcher(AuthenticatedMatcher(auth), false, conn); + checkMatcher(AuthenticatedMatcher(auth, factory_context), false, conn); } TEST(AuthenticatedMatcher, subjectPeerCertificate) { @@ -366,11 +380,12 @@ TEST(AuthenticatedMatcher, subjectPeerCertificate) { EXPECT_CALL(Const(conn), ssl()).WillRepeatedly(Return(ssl)); envoy::config::rbac::v3::Principal::Authenticated auth; + NiceMock factory_context; auth.mutable_principal_name()->set_exact("bar"); - checkMatcher(AuthenticatedMatcher(auth), true, conn); + checkMatcher(AuthenticatedMatcher(auth, factory_context), true, conn); auth.mutable_principal_name()->set_exact("foo"); - checkMatcher(AuthenticatedMatcher(auth), false, conn); + checkMatcher(AuthenticatedMatcher(auth, factory_context), false, conn); } TEST(AuthenticatedMatcher, AnySSLSubject) { @@ -381,16 +396,18 @@ TEST(AuthenticatedMatcher, AnySSLSubject) { EXPECT_CALL(Const(conn), ssl()).WillRepeatedly(Return(ssl)); envoy::config::rbac::v3::Principal::Authenticated auth; - checkMatcher(AuthenticatedMatcher(auth), true, conn); + NiceMock factory_context; + checkMatcher(AuthenticatedMatcher(auth, factory_context), true, conn); auth.mutable_principal_name()->MergeFrom(TestUtility::createRegexMatcher(".*")); - checkMatcher(AuthenticatedMatcher(auth), true, conn); + checkMatcher(AuthenticatedMatcher(auth, factory_context), true, conn); } TEST(AuthenticatedMatcher, NoSSL) { + NiceMock factory_context; Envoy::Network::MockConnection conn; EXPECT_CALL(Const(conn), ssl()).WillOnce(Return(nullptr)); - checkMatcher(AuthenticatedMatcher({}), false, conn); + checkMatcher(AuthenticatedMatcher({}, factory_context), false, conn); } TEST(MetadataMatcher, MetadataMatcher) { @@ -417,6 +434,7 @@ TEST(MetadataMatcher, MetadataMatcher) { } TEST(PolicyMatcher, PolicyMatcher) { + NiceMock factory_context; envoy::config::rbac::v3::Policy policy; policy.add_permissions()->set_destination_port(123); policy.add_permissions()->set_destination_port(456); @@ -424,7 +442,8 @@ TEST(PolicyMatcher, PolicyMatcher) { policy.add_principals()->mutable_authenticated()->mutable_principal_name()->set_exact("bar"); Expr::BuilderPtr builder = Expr::createBuilder(nullptr); - RBAC::PolicyMatcher matcher(policy, builder.get(), ProtobufMessage::getStrictValidationVisitor()); + RBAC::PolicyMatcher matcher(policy, builder.get(), ProtobufMessage::getStrictValidationVisitor(), + factory_context); Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; @@ -457,36 +476,52 @@ TEST(PolicyMatcher, PolicyMatcher) { } TEST(RequestedServerNameMatcher, ValidRequestedServerName) { + NiceMock factory_context; Envoy::Network::MockConnection conn; EXPECT_CALL(conn, requestedServerName()) .Times(9) .WillRepeatedly(Return(absl::string_view("www.cncf.io"))); - checkMatcher(RequestedServerNameMatcher(TestUtility::createRegexMatcher(".*cncf.io")), true, - conn); - checkMatcher(RequestedServerNameMatcher(TestUtility::createRegexMatcher(".*cncf.*")), true, conn); - checkMatcher(RequestedServerNameMatcher(TestUtility::createRegexMatcher("www.*")), true, conn); - checkMatcher(RequestedServerNameMatcher(TestUtility::createRegexMatcher(".*io")), true, conn); - checkMatcher(RequestedServerNameMatcher(TestUtility::createRegexMatcher(".*")), true, conn); - - checkMatcher(RequestedServerNameMatcher(TestUtility::createExactMatcher("")), false, conn); - checkMatcher(RequestedServerNameMatcher(TestUtility::createExactMatcher("www.cncf.io")), true, - conn); - checkMatcher(RequestedServerNameMatcher(TestUtility::createExactMatcher("xyz.cncf.io")), false, - conn); - checkMatcher(RequestedServerNameMatcher(TestUtility::createExactMatcher("example.com")), false, - conn); + checkMatcher( + RequestedServerNameMatcher(TestUtility::createRegexMatcher(".*cncf.io"), factory_context), + true, conn); + checkMatcher( + RequestedServerNameMatcher(TestUtility::createRegexMatcher(".*cncf.*"), factory_context), + true, conn); + checkMatcher( + RequestedServerNameMatcher(TestUtility::createRegexMatcher("www.*"), factory_context), true, + conn); + checkMatcher(RequestedServerNameMatcher(TestUtility::createRegexMatcher(".*io"), factory_context), + true, conn); + checkMatcher(RequestedServerNameMatcher(TestUtility::createRegexMatcher(".*"), factory_context), + true, conn); + + checkMatcher(RequestedServerNameMatcher(TestUtility::createExactMatcher(""), factory_context), + false, conn); + checkMatcher( + RequestedServerNameMatcher(TestUtility::createExactMatcher("www.cncf.io"), factory_context), + true, conn); + checkMatcher( + RequestedServerNameMatcher(TestUtility::createExactMatcher("xyz.cncf.io"), factory_context), + false, conn); + checkMatcher( + RequestedServerNameMatcher(TestUtility::createExactMatcher("example.com"), factory_context), + false, conn); } TEST(RequestedServerNameMatcher, EmptyRequestedServerName) { + NiceMock factory_context; Envoy::Network::MockConnection conn; EXPECT_CALL(conn, requestedServerName()).Times(3).WillRepeatedly(Return(absl::string_view(""))); - checkMatcher(RequestedServerNameMatcher(TestUtility::createRegexMatcher(".*")), true, conn); + checkMatcher(RequestedServerNameMatcher(TestUtility::createRegexMatcher(".*"), factory_context), + true, conn); - checkMatcher(RequestedServerNameMatcher(TestUtility::createExactMatcher("")), true, conn); - checkMatcher(RequestedServerNameMatcher(TestUtility::createExactMatcher("example.com")), false, - conn); + checkMatcher(RequestedServerNameMatcher(TestUtility::createExactMatcher(""), factory_context), + true, conn); + checkMatcher( + RequestedServerNameMatcher(TestUtility::createExactMatcher("example.com"), factory_context), + false, conn); } TEST(PathMatcher, NoPathInHeader) { diff --git a/test/extensions/filters/common/rbac/mocks.h b/test/extensions/filters/common/rbac/mocks.h index 83088d8ab6ef..c1f9ac0916e9 100644 --- a/test/extensions/filters/common/rbac/mocks.h +++ b/test/extensions/filters/common/rbac/mocks.h @@ -17,9 +17,10 @@ namespace RBAC { class MockEngine : public RoleBasedAccessControlEngineImpl { public: MockEngine(const envoy::config::rbac::v3::RBAC& rules, + Server::Configuration::CommonFactoryContext& context, const EnforcementMode mode = EnforcementMode::Enforced) : RoleBasedAccessControlEngineImpl(rules, ProtobufMessage::getStrictValidationVisitor(), - mode){}; + context, mode){}; MOCK_METHOD(bool, handleAction, (const Envoy::Network::Connection&, const Envoy::Http::RequestHeaderMap&, diff --git a/test/extensions/filters/http/rbac/rbac_filter_test.cc b/test/extensions/filters/http/rbac/rbac_filter_test.cc index 29e638179176..c8578f2e2622 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_test.cc @@ -422,7 +422,8 @@ TEST_F(RoleBasedAccessControlFilterTest, RouteLocalOverride) { envoy::extensions::filters::http::rbac::v3::RBACPerRoute route_config; route_config.mutable_rbac()->mutable_rules()->set_action(envoy::config::rbac::v3::RBAC::DENY); - NiceMock engine{route_config.rbac().rules()}; + NiceMock factory_context; + NiceMock engine{route_config.rbac().rules(), factory_context}; NiceMock per_route_config_{route_config, context_};