diff --git a/api/bazel/external_proto_deps.bzl b/api/bazel/external_proto_deps.bzl index 6b11495d3c0dc..916f0e1f7fcf9 100644 --- a/api/bazel/external_proto_deps.bzl +++ b/api/bazel/external_proto_deps.bzl @@ -19,8 +19,17 @@ EXTERNAL_PROTO_IMPORT_BAZEL_DEP_MAP = { # This maps from the Bazel proto_library target to the Go language binding target for external dependencies. EXTERNAL_PROTO_GO_BAZEL_DEP_MAP = { - "@com_google_googleapis//google/api/expr/v1alpha1:checked_proto": "@com_google_googleapis//google/api/expr/v1alpha1:expr_go_proto", - "@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto": "@com_google_googleapis//google/api/expr/v1alpha1:expr_go_proto", + # Note @com_google_googleapis are point to @go_googleapis. + # + # It is aligned to xDS dependency to suppress the conflicting package heights error between + # @com_github_cncf_udpa//xds/type/matcher/v3:pkg_go_proto + # @envoy_api//envoy/config/rbac/v3:pkg_go_proto + # + # TODO(https://github.com/bazelbuild/rules_go/issues/1986): update to + # @com_google_googleapis when the bug is resolved. Also see the note to + # go_googleapis in https://github.com/bazelbuild/rules_go/blob/master/go/dependencies.rst#overriding-dependencies + "@com_google_googleapis//google/api/expr/v1alpha1:checked_proto": "@go_googleapis//google/api/expr/v1alpha1:expr_go_proto", + "@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto": "@go_googleapis//google/api/expr/v1alpha1:expr_go_proto", "@opencensus_proto//opencensus/proto/trace/v1:trace_proto": "@opencensus_proto//opencensus/proto/trace/v1:trace_proto_go", "@opencensus_proto//opencensus/proto/trace/v1:trace_config_proto": "@opencensus_proto//opencensus/proto/trace/v1:trace_and_config_proto_go", "@opentelemetry_proto//:logs": "@opentelemetry_proto//:logs_go_proto", diff --git a/api/envoy/config/rbac/v3/rbac.proto b/api/envoy/config/rbac/v3/rbac.proto index 8abde899d7ed0..630ef418d0aa0 100644 --- a/api/envoy/config/rbac/v3/rbac.proto +++ b/api/envoy/config/rbac/v3/rbac.proto @@ -310,3 +310,29 @@ message Principal { Principal not_id = 8; } } + +// Action defines the result of allowance or denial when a request matches the matcher. +message Action { + // The name indicates the policy name. + string name = 1 [(validate.rules).string = {min_len: 1}]; + + // The action to take if the matcher matches. Every action either allows or denies a request, + // and can also carry out action-specific operations. + // + // Actions: + // + // * ALLOW: If the request gets matched on ALLOW, it is permitted. + // * DENY: If the request gets matched on DENY, it is not permitted. + // * LOG: If the request gets matched on LOG, it is permitted. Besides, the + // dynamic metadata key `access_log_hint` under the shared key namespace + // 'envoy.common' will be set to the value `true`. + // * If the request cannot get matched, it will fallback to DENY. + // + // Log behavior: + // + // If the RBAC matcher contains at least one LOG action, the dynamic + // metadata key `access_log_hint` will be set based on if the request + // get matched on the LOG action. + // + RBAC.Action action = 2; +} diff --git a/api/envoy/extensions/filters/http/rbac/v3/BUILD b/api/envoy/extensions/filters/http/rbac/v3/BUILD index fd183569e5a1e..49cb2ccac4f7d 100644 --- a/api/envoy/extensions/filters/http/rbac/v3/BUILD +++ b/api/envoy/extensions/filters/http/rbac/v3/BUILD @@ -8,5 +8,7 @@ api_proto_package( deps = [ "//envoy/config/rbac/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", + "@com_github_cncf_udpa//xds/annotations/v3:pkg", + "@com_github_cncf_udpa//xds/type/matcher/v3:pkg", ], ) diff --git a/api/envoy/extensions/filters/http/rbac/v3/rbac.proto b/api/envoy/extensions/filters/http/rbac/v3/rbac.proto index 008818456e2fd..eeb505a17fb7b 100644 --- a/api/envoy/extensions/filters/http/rbac/v3/rbac.proto +++ b/api/envoy/extensions/filters/http/rbac/v3/rbac.proto @@ -4,6 +4,10 @@ package envoy.extensions.filters.http.rbac.v3; import "envoy/config/rbac/v3/rbac.proto"; +import "xds/annotations/v3/status.proto"; +import "xds/type/matcher/v3/matcher.proto"; + +import "udpa/annotations/migrate.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; @@ -18,6 +22,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#extension: envoy.filters.http.rbac] // RBAC filter config. +// [#next-free-field: 6] message RBAC { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.rbac.v2.RBAC"; @@ -25,12 +30,33 @@ message RBAC { // Specify the RBAC rules to be applied globally. // If absent, no enforcing RBAC policy will be applied. // If present and empty, DENY. - config.rbac.v3.RBAC rules = 1; + // If both rules and matcher are configured, rules will be ignored. + config.rbac.v3.RBAC rules = 1 + [(udpa.annotations.field_migrate).oneof_promotion = "rules_specifier"]; + + // The match tree to use when resolving RBAC action for incoming requests. Requests do not + // match any matcher will be denied. + // If absent, no enforcing RBAC matcher will be applied. + // If present and empty, deny all requests. + xds.type.matcher.v3.Matcher matcher = 4 [ + (udpa.annotations.field_migrate).oneof_promotion = "rules_specifier", + (xds.annotations.v3.field_status).work_in_progress = true + ]; // Shadow rules are not enforced by the filter (i.e., returning a 403) // but will emit stats and logs and can be used for rule testing. // If absent, no shadow RBAC policy will be applied. - config.rbac.v3.RBAC shadow_rules = 2; + // If both shadow rules and shadow matcher are configured, shadow rules will be ignored. + config.rbac.v3.RBAC shadow_rules = 2 + [(udpa.annotations.field_migrate).oneof_promotion = "shadow_rules_specifier"]; + + // The match tree to use for emitting stats and logs which can be used for rule testing for + // incoming requests. + // If absent, no shadow matcher will be applied. + xds.type.matcher.v3.Matcher shadow_matcher = 5 [ + (udpa.annotations.field_migrate).oneof_promotion = "shadow_rules_specifier", + (xds.annotations.v3.field_status).work_in_progress = true + ]; // If specified, shadow rules will emit stats with the given prefix. // This is useful to distinguish the stat when there are more than 1 RBAC filter configured with diff --git a/api/envoy/extensions/filters/network/rbac/v3/BUILD b/api/envoy/extensions/filters/network/rbac/v3/BUILD index fd183569e5a1e..49cb2ccac4f7d 100644 --- a/api/envoy/extensions/filters/network/rbac/v3/BUILD +++ b/api/envoy/extensions/filters/network/rbac/v3/BUILD @@ -8,5 +8,7 @@ api_proto_package( deps = [ "//envoy/config/rbac/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", + "@com_github_cncf_udpa//xds/annotations/v3:pkg", + "@com_github_cncf_udpa//xds/type/matcher/v3:pkg", ], ) diff --git a/api/envoy/extensions/filters/network/rbac/v3/rbac.proto b/api/envoy/extensions/filters/network/rbac/v3/rbac.proto index 44141f167068a..823e18277d1fd 100644 --- a/api/envoy/extensions/filters/network/rbac/v3/rbac.proto +++ b/api/envoy/extensions/filters/network/rbac/v3/rbac.proto @@ -4,6 +4,10 @@ package envoy.extensions.filters.network.rbac.v3; import "envoy/config/rbac/v3/rbac.proto"; +import "xds/annotations/v3/status.proto"; +import "xds/type/matcher/v3/matcher.proto"; + +import "udpa/annotations/migrate.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -22,7 +26,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // // Header should not be used in rules/shadow_rules in RBAC network filter as // this information is only available in :ref:`RBAC http filter `. -// [#next-free-field: 6] +// [#next-free-field: 8] message RBAC { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.rbac.v2.RBAC"; @@ -41,12 +45,33 @@ message RBAC { // Specify the RBAC rules to be applied globally. // If absent, no enforcing RBAC policy will be applied. // If present and empty, DENY. - config.rbac.v3.RBAC rules = 1; + // If both rules and matcher are configured, rules will be ignored. + config.rbac.v3.RBAC rules = 1 + [(udpa.annotations.field_migrate).oneof_promotion = "rules_specifier"]; + + // The match tree to use when resolving RBAC action for incoming connections. Connections do + // not match any matcher will be denied. + // If absent, no enforcing RBAC matcher will be applied. + // If present and empty, deny all connections. + xds.type.matcher.v3.Matcher matcher = 6 [ + (udpa.annotations.field_migrate).oneof_promotion = "rules_specifier", + (xds.annotations.v3.field_status).work_in_progress = true + ]; // Shadow rules are not enforced by the filter but will emit stats and logs // and can be used for rule testing. // If absent, no shadow RBAC policy will be applied. - config.rbac.v3.RBAC shadow_rules = 2; + // If both shadow rules and shadow matcher are configured, shadow rules will be ignored. + config.rbac.v3.RBAC shadow_rules = 2 + [(udpa.annotations.field_migrate).oneof_promotion = "shadow_rules_specifier"]; + + // The match tree to use for emitting stats and logs which can be used for rule testing for + // incoming connections. + // If absent, no shadow matcher will be applied. + xds.type.matcher.v3.Matcher shadow_matcher = 7 [ + (udpa.annotations.field_migrate).oneof_promotion = "shadow_rules_specifier", + (xds.annotations.v3.field_status).work_in_progress = true + ]; // If specified, shadow rules will emit stats with the given prefix. // This is useful to distinguish the stat when there are more than 1 RBAC filter configured with diff --git a/changelogs/current.yaml b/changelogs/current.yaml index a7a8669451b2a..48bab1ca5d86c 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -231,6 +231,9 @@ new_features: - area: matching change : | added support for matching authenticated inputs in network and HTTP matching data. +- area: rbac + change: | + added :ref:`matcher ` for selecting connections and requests to different actions. deprecated: - area: dubbo_proxy diff --git a/docs/root/api-v3/config/rbac/matchers.rst b/docs/root/api-v3/config/rbac/matchers.rst index d32ce66750b86..c51c825834ff7 100644 --- a/docs/root/api-v3/config/rbac/matchers.rst +++ b/docs/root/api-v3/config/rbac/matchers.rst @@ -1,3 +1,5 @@ +.. _api-v3_config_rbac_matchers: + RBAC Matchers ============= diff --git a/docs/root/configuration/http/http_filters/rbac_filter.rst b/docs/root/configuration/http/http_filters/rbac_filter.rst index 819964e6211ed..775985e7b750a 100644 --- a/docs/root/configuration/http/http_filters/rbac_filter.rst +++ b/docs/root/configuration/http/http_filters/rbac_filter.rst @@ -3,10 +3,10 @@ Role Based Access Control (RBAC) Filter ======================================= -The RBAC filter is used to authorize actions (permissions) by identified downstream clients -(principals). This is useful to explicitly manage callers to an application and protect it from -unexpected or forbidden agents. The filter supports configuration with either a safe-list (ALLOW) or -block-list (DENY) set of policies based off properties of the connection (IPs, ports, SSL subject) +The RBAC filter is used to authorize actions by identified downstream clients. This is useful to +explicitly manage callers to an application and protect it from unexpected or forbidden agents. The +filter supports configuration with either a safe-list (ALLOW) or block-list (DENY) set of policies, +or a matcher with different actions, based off properties of the connection (IPs, ports, SSL subject) as well as the incoming request's HTTP headers. This filter also supports policy in both enforcement and shadow mode, shadow mode won't effect real users, it is used to test that a new set of policies work before rolling out to production. diff --git a/docs/root/configuration/listeners/network_filters/rbac_filter.rst b/docs/root/configuration/listeners/network_filters/rbac_filter.rst index ee8338c06041b..0fa3e82f9ed87 100644 --- a/docs/root/configuration/listeners/network_filters/rbac_filter.rst +++ b/docs/root/configuration/listeners/network_filters/rbac_filter.rst @@ -3,10 +3,10 @@ Role Based Access Control (RBAC) Network Filter =============================================== -The RBAC network filter is used to authorize actions (permissions) by identified downstream clients -(principals). This is useful to explicitly manage callers to an application and protect it from -unexpected or forbidden agents. The filter supports configuration with either a safe-list (ALLOW) or -block-list (DENY) set of policies based on properties of the connection (IPs, ports, SSL subject). +The RBAC network filter is used to authorize actions by identified downstream clients. This is useful +to explicitly manage callers to an application and protect it from unexpected or forbidden agents. +The filter supports configuration with either a safe-list (ALLOW) or block-list (DENY) set of policies, +or a matcher with different actions, based on properties of the connection (IPs, ports, SSL subject). This filter also supports policy in both enforcement and shadow modes. Shadow mode won't effect real users, it is used to test that a new set of policies work before rolling out to production. diff --git a/docs/root/intro/arch_overview/security/rbac_filter.rst b/docs/root/intro/arch_overview/security/rbac_filter.rst index a5ba2b8e2488a..0672d28323d7c 100644 --- a/docs/root/intro/arch_overview/security/rbac_filter.rst +++ b/docs/root/intro/arch_overview/security/rbac_filter.rst @@ -15,6 +15,10 @@ or as a :ref:`HTTP filter ` or both. If the request is by the network filter then the connection will be closed. If the request is deemed unauthorized by the HTTP filter the request will be denied with 403 (Forbidden) response. +The RBAC filter's rules can be either configured with a list of +:ref:`policies ` or the +:ref:`matching API `. + Policy ------ @@ -26,13 +30,27 @@ the request, for example, the method and path of a HTTP request. The principal s downstream client identities of the request, for example, the URI SAN of the downstream client certificate. A policy is matched if its permissions and principals are matched at the same time. -Shadow Policy -------------- +.. _arch_overview_rbac_matcher: + +Matcher +------- +Instead of specifying :ref:`policies `, the RBAC +filter can also be configured with the :ref:`matching API `. +:ref:`Network inputs ` are available for both RBAC +network filter and HTTP filter, and :ref:`HTTP inputs ` +are only available in HTTP filter. + +:ref:`RBAC matcher extensions ` are not compatible with the +:ref:`matching API `. + +Shadow Policy and Shadow Matcher +-------------------------------- The filter can be configured with a -:ref:`shadow policy ` that doesn't -have any effect (i.e. not deny the request) but only emit stats and log the result. This is useful -for testing a rule before applying in production. +:ref:`shadow policy ` or a +:ref:`shadow matcher ` that +doesn't have any effect (i.e. not deny the request) but only emit stats and log the result. This is +useful for testing a rule before applying in production. .. _arch_overview_condition: diff --git a/source/extensions/filters/common/rbac/BUILD b/source/extensions/filters/common/rbac/BUILD index a49234a5111ac..80a3891048098 100644 --- a/source/extensions/filters/common/rbac/BUILD +++ b/source/extensions/filters/common/rbac/BUILD @@ -58,6 +58,11 @@ envoy_cc_library( srcs = ["engine_impl.cc"], hdrs = ["engine_impl.h"], deps = [ + "//source/common/http/matching:data_impl_lib", + "//source/common/http/matching:inputs_lib", + "//source/common/matcher:matcher_lib", + "//source/common/network/matching:inputs_lib", + "//source/common/ssl/matching:inputs_lib", "//source/extensions/filters/common/rbac:engine_interface", "//source/extensions/filters/common/rbac:matchers_lib", "@envoy_api//envoy/config/rbac/v3:pkg_cc_proto", diff --git a/source/extensions/filters/common/rbac/engine_impl.cc b/source/extensions/filters/common/rbac/engine_impl.cc index 531a7ea63f5b7..33ef236939947 100644 --- a/source/extensions/filters/common/rbac/engine_impl.cc +++ b/source/extensions/filters/common/rbac/engine_impl.cc @@ -1,6 +1,7 @@ #include "source/extensions/filters/common/rbac/engine_impl.h" #include "envoy/config/rbac/v3/rbac.pb.h" +#include "envoy/config/rbac/v3/rbac.pb.validate.h" #include "source/common/http/header_map_impl.h" @@ -10,6 +11,35 @@ namespace Filters { namespace Common { namespace RBAC { +Envoy::Matcher::ActionFactoryCb +ActionFactory::createActionFactoryCb(const Protobuf::Message& config, ActionContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor) { + const auto& action_config = + MessageUtil::downcastAndValidate(config, + validation_visitor); + const auto& name = action_config.name(); + const auto action = action_config.action(); + + // If there is at least an action is LOG, we have to go through LOG procedure when handle action. + if (action == envoy::config::rbac::v3::RBAC::LOG) { + context.has_log_ = true; + } + + return [name, action]() { return std::make_unique(name, action); }; +} + +REGISTER_FACTORY(ActionFactory, Envoy::Matcher::ActionFactory); + +void generateLog(StreamInfo::StreamInfo& info, EnforcementMode mode, bool log) { + // If not shadow enforcement, set shared log metadata. + if (mode != EnforcementMode::Shadow) { + ProtobufWkt::Struct log_metadata; + auto& log_fields = *log_metadata.mutable_fields(); + log_fields[DynamicMetadataKeysSingleton::get().AccessLogKey].set_bool_value(log); + info.setDynamicMetadata(DynamicMetadataKeysSingleton::get().CommonNamespace, log_metadata); + } +} + RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl( const envoy::config::rbac::v3::RBAC& rules, ProtobufMessage::ValidationVisitor& validation_visitor, const EnforcementMode mode) @@ -48,13 +78,7 @@ bool RoleBasedAccessControlEngineImpl::handleAction(const Network::Connection& c case envoy::config::rbac::v3::RBAC::DENY: return !matched; case envoy::config::rbac::v3::RBAC::LOG: { - // If not shadow enforcement, set shared log metadata - if (mode_ != EnforcementMode::Shadow) { - ProtobufWkt::Struct log_metadata; - auto& log_fields = *log_metadata.mutable_fields(); - log_fields[DynamicMetadataKeysSingleton::get().AccessLogKey].set_bool_value(matched); - info.setDynamicMetadata(DynamicMetadataKeysSingleton::get().CommonNamespace, log_metadata); - } + generateLog(info, mode_, matched); return true; } @@ -80,6 +104,64 @@ bool RoleBasedAccessControlEngineImpl::checkPolicyMatch( return matched; } +RoleBasedAccessControlMatcherEngineImpl::RoleBasedAccessControlMatcherEngineImpl( + const xds::type::matcher::v3::Matcher& matcher, + Server::Configuration::ServerFactoryContext& factory_context, + ActionValidationVisitor& validation_visitor, const EnforcementMode mode) + : mode_(mode) { + ActionContext context{false}; + Envoy::Matcher::MatchTreeFactory factory( + context, factory_context, validation_visitor); + matcher_ = factory.create(matcher)(); + has_log_ = context.has_log_; + + if (!validation_visitor.errors().empty()) { + throw EnvoyException(fmt::format("requirement violation while creating RBAC match tree: {}", + validation_visitor.errors()[0])); + } +} + +bool RoleBasedAccessControlMatcherEngineImpl::handleAction(const Network::Connection& connection, + StreamInfo::StreamInfo& info, + std::string* effective_policy_id) const { + return handleAction(connection, *Http::StaticEmptyHeaders::get().request_headers, info, + effective_policy_id); +} + +bool RoleBasedAccessControlMatcherEngineImpl::handleAction( + const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers, + StreamInfo::StreamInfo& info, std::string* effective_policy_id) const { + Http::Matching::HttpMatchingDataImpl data(connection.connectionInfoProvider()); + data.onRequestHeaders(headers); + const auto& result = Envoy::Matcher::evaluateMatch(*matcher_, data); + ASSERT(result.match_state_ == Envoy::Matcher::MatchState::MatchComplete); + if (result.result_) { + auto action = result.result_()->getTyped(); + if (effective_policy_id != nullptr) { + *effective_policy_id = action.name(); + } + + // If there is at least an LOG action in matchers, we have to turn on and off for shared log + // metadata every time when there is a connection or request. + auto rbac_action = action.action(); + if (has_log_) { + generateLog(info, mode_, rbac_action == envoy::config::rbac::v3::RBAC::LOG); + } + + switch (rbac_action) { + PANIC_ON_PROTO_ENUM_SENTINEL_VALUES; + case envoy::config::rbac::v3::RBAC::ALLOW: + case envoy::config::rbac::v3::RBAC::LOG: + return true; + case envoy::config::rbac::v3::RBAC::DENY: + return false; + } + } + + // Default to DENY. + return false; +} + } // namespace RBAC } // namespace Common } // namespace Filters diff --git a/source/extensions/filters/common/rbac/engine_impl.h b/source/extensions/filters/common/rbac/engine_impl.h index 431763919f7ce..c748142c8a97e 100644 --- a/source/extensions/filters/common/rbac/engine_impl.h +++ b/source/extensions/filters/common/rbac/engine_impl.h @@ -2,9 +2,13 @@ #include "envoy/config/rbac/v3/rbac.pb.h" +#include "source/common/http/matching/data_impl.h" +#include "source/common/matcher/matcher.h" #include "source/extensions/filters/common/rbac/engine.h" #include "source/extensions/filters/common/rbac/matchers.h" +#include "xds/type/matcher/v3/matcher.pb.h" + namespace Envoy { namespace Extensions { namespace Filters { @@ -25,6 +29,38 @@ using DynamicMetadataKeysSingleton = ConstSingleton; enum class EnforcementMode { Enforced, Shadow }; +struct ActionContext { + bool has_log_; +}; + +class Action : public Envoy::Matcher::ActionBase { +public: + Action(const std::string& name, const envoy::config::rbac::v3::RBAC::Action action) + : name_(name), action_(action) {} + + const std::string& name() const { return name_; } + envoy::config::rbac::v3::RBAC::Action action() const { return action_; } + +private: + const std::string name_; + const envoy::config::rbac::v3::RBAC::Action action_; +}; + +class ActionFactory : public Envoy::Matcher::ActionFactory { +public: + Envoy::Matcher::ActionFactoryCb + createActionFactoryCb(const Protobuf::Message& config, ActionContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor) override; + std::string name() const override { return "envoy.filters.rbac.action"; } + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique(); + } +}; + +using ActionValidationVisitor = Envoy::Matcher::MatchTreeValidationVisitor; + +void generateLog(StreamInfo::StreamInfo& info, EnforcementMode mode, bool log); + class RoleBasedAccessControlEngineImpl : public RoleBasedAccessControlEngine, NonCopyable { public: RoleBasedAccessControlEngineImpl(const envoy::config::rbac::v3::RBAC& rules, @@ -53,6 +89,27 @@ class RoleBasedAccessControlEngineImpl : public RoleBasedAccessControlEngine, No Expr::BuilderPtr builder_; }; +class RoleBasedAccessControlMatcherEngineImpl : public RoleBasedAccessControlEngine, NonCopyable { +public: + RoleBasedAccessControlMatcherEngineImpl( + const xds::type::matcher::v3::Matcher& matcher, + Server::Configuration::ServerFactoryContext& factory_context, + ActionValidationVisitor& validation_visitor, + const EnforcementMode mode = EnforcementMode::Enforced); + + bool handleAction(const Network::Connection& connection, + const Envoy::Http::RequestHeaderMap& headers, StreamInfo::StreamInfo& info, + std::string* effective_policy_id) const override; + + bool handleAction(const Network::Connection& connection, StreamInfo::StreamInfo& info, + std::string* effective_policy_id) const override; + +private: + const EnforcementMode mode_; + Envoy::Matcher::MatchTreeSharedPtr matcher_; + bool has_log_; +}; + } // namespace RBAC } // namespace Common } // namespace Filters diff --git a/source/extensions/filters/common/rbac/utility.h b/source/extensions/filters/common/rbac/utility.h index b96205648d290..ac2a339226cae 100644 --- a/source/extensions/filters/common/rbac/utility.h +++ b/source/extensions/filters/common/rbac/utility.h @@ -38,21 +38,43 @@ RoleBasedAccessControlFilterStats generateStats(const std::string& prefix, const std::string& shadow_prefix, Stats::Scope& scope); template -std::unique_ptr -createEngine(const ConfigType& config, ProtobufMessage::ValidationVisitor& validation_visitor) { - return config.has_rules() ? std::make_unique( - config.rules(), validation_visitor, EnforcementMode::Enforced) - : nullptr; +std::unique_ptr +createEngine(const ConfigType& config, Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor, + ActionValidationVisitor& action_validation_visitor) { + if (config.has_matcher()) { + if (config.has_rules()) { + ENVOY_LOG_MISC(warn, "RBAC rules are ignored when matcher is configured"); + } + return std::make_unique( + config.matcher(), context, action_validation_visitor, EnforcementMode::Enforced); + } + if (config.has_rules()) { + return std::make_unique(config.rules(), validation_visitor, + EnforcementMode::Enforced); + } + + return nullptr; } template -std::unique_ptr -createShadowEngine(const ConfigType& config, - ProtobufMessage::ValidationVisitor& validation_visitor) { - return config.has_shadow_rules() - ? std::make_unique( - config.shadow_rules(), validation_visitor, EnforcementMode::Shadow) - : nullptr; +std::unique_ptr +createShadowEngine(const ConfigType& config, Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor, + ActionValidationVisitor& action_validation_visitor) { + if (config.has_shadow_matcher()) { + if (config.has_shadow_rules()) { + ENVOY_LOG_MISC(warn, "RBAC shadow rules are ignored when shadow matcher is configured"); + } + return std::make_unique( + config.shadow_matcher(), context, action_validation_visitor, EnforcementMode::Shadow); + } + if (config.has_shadow_rules()) { + return std::make_unique( + config.shadow_rules(), validation_visitor, EnforcementMode::Shadow); + } + + return nullptr; } std::string responseDetail(const std::string& policy_id); diff --git a/source/extensions/filters/http/rbac/config.cc b/source/extensions/filters/http/rbac/config.cc index 1559328594fa0..fefe21e1ba1cf 100644 --- a/source/extensions/filters/http/rbac/config.cc +++ b/source/extensions/filters/http/rbac/config.cc @@ -16,7 +16,8 @@ Http::FilterFactoryCb RoleBasedAccessControlFilterConfigFactory::createFilterFac const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { auto config = std::make_shared( - proto_config, stats_prefix, context.scope(), context.messageValidationVisitor()); + proto_config, stats_prefix, context.scope(), context.getServerFactoryContext(), + context.messageValidationVisitor()); return [config](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamDecoderFilter(std::make_shared(config)); @@ -26,9 +27,10 @@ Http::FilterFactoryCb RoleBasedAccessControlFilterConfigFactory::createFilterFac Router::RouteSpecificFilterConfigConstSharedPtr RoleBasedAccessControlFilterConfigFactory::createRouteSpecificFilterConfigTyped( const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& proto_config, - Server::Configuration::ServerFactoryContext&, ProtobufMessage::ValidationVisitor& validator) { - return std::make_shared(proto_config, - validator); + Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validator) { + return std::make_shared( + proto_config, context, validator); } /** diff --git a/source/extensions/filters/http/rbac/rbac_filter.cc b/source/extensions/filters/http/rbac/rbac_filter.cc index 85669c2e3a9c8..2fa6f0aef7dd2 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.cc +++ b/source/extensions/filters/http/rbac/rbac_filter.cc @@ -1,9 +1,11 @@ #include "source/extensions/filters/http/rbac/rbac_filter.h" -#include "envoy/extensions/filters/http/rbac/v3/rbac.pb.h" #include "envoy/stats/scope.h" +#include "source/common/http/matching/inputs.h" #include "source/common/http/utility.h" +#include "source/common/network/matching/inputs.h" +#include "source/common/ssl/matching/inputs.h" #include "absl/strings/str_join.h" @@ -12,17 +14,59 @@ namespace Extensions { namespace HttpFilters { namespace RBACFilter { +absl::Status ActionValidationVisitor::performDataInputValidation( + const Envoy::Matcher::DataInputFactory&, absl::string_view type_url) { + static absl::flat_hash_set allowed_inputs_set{ + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::network::v3::DestinationIPInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl(envoy::extensions::matching::common_inputs::network:: + v3::DestinationPortInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::network::v3::SourceIPInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::network::v3::SourcePortInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::network::v3::DirectSourceIPInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::network::v3::ServerNameInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::type::matcher::v3::HttpRequestHeaderMatchInput::descriptor()->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::ssl::v3::UriSanInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::ssl::v3::DnsSanInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::ssl::v3::SubjectInput::descriptor() + ->full_name())}}; + if (allowed_inputs_set.contains(type_url)) { + return absl::OkStatus(); + } + + return absl::InvalidArgumentError(fmt::format("RBAC HTTP filter cannot match on '{}'", type_url)); +} + RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig( const envoy::extensions::filters::http::rbac::v3::RBAC& proto_config, const std::string& stats_prefix, Stats::Scope& scope, + Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor) : stats_(Filters::Common::RBAC::generateStats(stats_prefix, proto_config.shadow_rules_stat_prefix(), scope)), shadow_rules_stat_prefix_(proto_config.shadow_rules_stat_prefix()), - engine_(Filters::Common::RBAC::createEngine(proto_config, validation_visitor)), - shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config, validation_visitor)) {} + engine_(Filters::Common::RBAC::createEngine(proto_config, context, validation_visitor, + action_validation_visitor_)), + shadow_engine_(Filters::Common::RBAC::createShadowEngine( + proto_config, context, validation_visitor, action_validation_visitor_)) {} -const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl* +const Filters::Common::RBAC::RoleBasedAccessControlEngine* RoleBasedAccessControlFilterConfig::engine(const Router::RouteConstSharedPtr route, Filters::Common::RBAC::EnforcementMode mode) const { const auto* route_local = Http::Utility::resolveMostSpecificPerFilterConfig< @@ -37,13 +81,15 @@ RoleBasedAccessControlFilterConfig::engine(const Router::RouteConstSharedPtr rou RoleBasedAccessControlRouteSpecificFilterConfig::RoleBasedAccessControlRouteSpecificFilterConfig( const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& per_route_config, + Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor) { // Moved from member initializer to ctor body to overcome clang false warning about memory // leak (clang-analyzer-cplusplus.NewDeleteLeaks,-warnings-as-errors). // Potentially https://lists.llvm.org/pipermail/llvm-bugs/2018-July/066769.html - engine_ = Filters::Common::RBAC::createEngine(per_route_config.rbac(), validation_visitor); - shadow_engine_ = - Filters::Common::RBAC::createShadowEngine(per_route_config.rbac(), validation_visitor); + engine_ = Filters::Common::RBAC::createEngine(per_route_config.rbac(), context, + validation_visitor, action_validation_visitor_); + shadow_engine_ = Filters::Common::RBAC::createShadowEngine( + per_route_config.rbac(), context, validation_visitor, action_validation_visitor_); } Http::FilterHeadersStatus diff --git a/source/extensions/filters/http/rbac/rbac_filter.h b/source/extensions/filters/http/rbac/rbac_filter.h index a29fa532dc073..d26c3807fb5f0 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.h +++ b/source/extensions/filters/http/rbac/rbac_filter.h @@ -16,21 +16,30 @@ namespace Extensions { namespace HttpFilters { namespace RBACFilter { +class ActionValidationVisitor : public Filters::Common::RBAC::ActionValidationVisitor { +public: + absl::Status performDataInputValidation( + const Envoy::Matcher::DataInputFactory& data_input, + absl::string_view type_url) override; +}; + class RoleBasedAccessControlRouteSpecificFilterConfig : public Router::RouteSpecificFilterConfig { public: RoleBasedAccessControlRouteSpecificFilterConfig( const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& per_route_config, + Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor); - const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl* + const Filters::Common::RBAC::RoleBasedAccessControlEngine* engine(Filters::Common::RBAC::EnforcementMode mode) const { return mode == Filters::Common::RBAC::EnforcementMode::Enforced ? engine_.get() : shadow_engine_.get(); } private: - std::unique_ptr engine_; - std::unique_ptr shadow_engine_; + ActionValidationVisitor action_validation_visitor_; + std::unique_ptr engine_; + std::unique_ptr shadow_engine_; }; /** @@ -41,6 +50,7 @@ class RoleBasedAccessControlFilterConfig { RoleBasedAccessControlFilterConfig( const envoy::extensions::filters::http::rbac::v3::RBAC& proto_config, const std::string& stats_prefix, Stats::Scope& scope, + Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor); Filters::Common::RBAC::RoleBasedAccessControlFilterStats& stats() { return stats_; } @@ -53,12 +63,12 @@ class RoleBasedAccessControlFilterConfig { Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().ShadowEngineResultField; } - const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl* + const Filters::Common::RBAC::RoleBasedAccessControlEngine* engine(const Router::RouteConstSharedPtr route, Filters::Common::RBAC::EnforcementMode mode) const; private: - const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl* + const Filters::Common::RBAC::RoleBasedAccessControlEngine* engine(Filters::Common::RBAC::EnforcementMode mode) const { return mode == Filters::Common::RBAC::EnforcementMode::Enforced ? engine_.get() : shadow_engine_.get(); @@ -67,8 +77,9 @@ class RoleBasedAccessControlFilterConfig { Filters::Common::RBAC::RoleBasedAccessControlFilterStats stats_; const std::string shadow_rules_stat_prefix_; - std::unique_ptr engine_; - std::unique_ptr shadow_engine_; + ActionValidationVisitor action_validation_visitor_; + std::unique_ptr engine_; + std::unique_ptr shadow_engine_; }; using RoleBasedAccessControlFilterConfigSharedPtr = diff --git a/source/extensions/filters/network/rbac/config.cc b/source/extensions/filters/network/rbac/config.cc index 3c95a5987956b..e7071c87d4ad8 100644 --- a/source/extensions/filters/network/rbac/config.cc +++ b/source/extensions/filters/network/rbac/config.cc @@ -9,6 +9,8 @@ #include "source/extensions/filters/network/rbac/rbac_filter.h" #include "source/extensions/filters/network/well_known_names.h" +#include "xds/type/matcher/v3/matcher.pb.h" + namespace Envoy { namespace Extensions { namespace NetworkFilters { @@ -76,10 +78,15 @@ Network::FilterFactoryCb RoleBasedAccessControlNetworkFilterConfigFactory::createFilterFactoryFromProtoTyped( const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Server::Configuration::FactoryContext& context) { - validateRbacRules(proto_config.rules()); - validateRbacRules(proto_config.shadow_rules()); + if (proto_config.has_rules()) { + validateRbacRules(proto_config.rules()); + } + if (proto_config.has_shadow_rules()) { + validateRbacRules(proto_config.shadow_rules()); + } RoleBasedAccessControlFilterConfigSharedPtr config( std::make_shared(proto_config, context.scope(), + context.getServerFactoryContext(), context.messageValidationVisitor())); return [config](Network::FilterManager& filter_manager) -> void { filter_manager.addReadFilter(std::make_shared(config)); diff --git a/source/extensions/filters/network/rbac/rbac_filter.cc b/source/extensions/filters/network/rbac/rbac_filter.cc index 6eb21b13495e2..86698dd2398c7 100644 --- a/source/extensions/filters/network/rbac/rbac_filter.cc +++ b/source/extensions/filters/network/rbac/rbac_filter.cc @@ -4,6 +4,8 @@ #include "envoy/extensions/filters/network/rbac/v3/rbac.pb.h" #include "envoy/network/connection.h" +#include "source/common/network/matching/inputs.h" +#include "source/common/ssl/matching/inputs.h" #include "source/extensions/filters/network/well_known_names.h" #include "absl/strings/str_join.h" @@ -13,14 +15,54 @@ namespace Extensions { namespace NetworkFilters { namespace RBACFilter { +absl::Status ActionValidationVisitor::performDataInputValidation( + const Envoy::Matcher::DataInputFactory&, absl::string_view type_url) { + static absl::flat_hash_set allowed_inputs_set{ + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::network::v3::DestinationIPInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl(envoy::extensions::matching::common_inputs::network:: + v3::DestinationPortInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::network::v3::SourceIPInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::network::v3::SourcePortInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::network::v3::DirectSourceIPInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::network::v3::ServerNameInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::ssl::v3::UriSanInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::ssl::v3::DnsSanInput::descriptor() + ->full_name())}, + {TypeUtil::descriptorFullNameToTypeUrl( + envoy::extensions::matching::common_inputs::ssl::v3::SubjectInput::descriptor() + ->full_name())}}; + if (allowed_inputs_set.contains(type_url)) { + return absl::OkStatus(); + } + + return absl::InvalidArgumentError(fmt::format("RBAC network filter cannot match '{}'", type_url)); +} + RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig( const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Stats::Scope& scope, + Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor) : stats_(Filters::Common::RBAC::generateStats(proto_config.stat_prefix(), proto_config.shadow_rules_stat_prefix(), scope)), shadow_rules_stat_prefix_(proto_config.shadow_rules_stat_prefix()), - engine_(Filters::Common::RBAC::createEngine(proto_config, validation_visitor)), - shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config, validation_visitor)), + engine_(Filters::Common::RBAC::createEngine(proto_config, context, validation_visitor, + action_validation_visitor_)), + shadow_engine_(Filters::Common::RBAC::createShadowEngine( + proto_config, context, validation_visitor, action_validation_visitor_)), enforcement_type_(proto_config.enforcement_type()) {} Network::FilterStatus RoleBasedAccessControlFilter::onData(Buffer::Instance&, bool) { diff --git a/source/extensions/filters/network/rbac/rbac_filter.h b/source/extensions/filters/network/rbac/rbac_filter.h index f43b3853b8884..a534f7c88010e 100644 --- a/source/extensions/filters/network/rbac/rbac_filter.h +++ b/source/extensions/filters/network/rbac/rbac_filter.h @@ -21,6 +21,13 @@ struct Result { std::string connection_termination_details_; }; +class ActionValidationVisitor : public Filters::Common::RBAC::ActionValidationVisitor { +public: + absl::Status performDataInputValidation( + const Envoy::Matcher::DataInputFactory& data_input, + absl::string_view type_url) override; +}; + /** * Configuration for the RBAC network filter. */ @@ -28,6 +35,7 @@ class RoleBasedAccessControlFilterConfig { public: RoleBasedAccessControlFilterConfig( const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Stats::Scope& scope, + Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor); Filters::Common::RBAC::RoleBasedAccessControlFilterStats& stats() { return stats_; } @@ -40,7 +48,7 @@ class RoleBasedAccessControlFilterConfig { Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().ShadowEngineResultField; } - const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl* + const Filters::Common::RBAC::RoleBasedAccessControlEngine* engine(Filters::Common::RBAC::EnforcementMode mode) const { return mode == Filters::Common::RBAC::EnforcementMode::Enforced ? engine_.get() : shadow_engine_.get(); @@ -54,8 +62,9 @@ class RoleBasedAccessControlFilterConfig { Filters::Common::RBAC::RoleBasedAccessControlFilterStats stats_; const std::string shadow_rules_stat_prefix_; - std::unique_ptr engine_; - std::unique_ptr shadow_engine_; + ActionValidationVisitor action_validation_visitor_; + std::unique_ptr engine_; + std::unique_ptr shadow_engine_; const envoy::extensions::filters::network::rbac::v3::RBAC::EnforcementType enforcement_type_; }; diff --git a/test/extensions/filters/common/rbac/BUILD b/test/extensions/filters/common/rbac/BUILD index e1576d00d05e2..dc8ec26940fe3 100644 --- a/test/extensions/filters/common/rbac/BUILD +++ b/test/extensions/filters/common/rbac/BUILD @@ -35,12 +35,15 @@ envoy_extension_cc_test( extension_names = ["envoy.filters.http.rbac"], deps = [ "//source/extensions/filters/common/rbac:engine_lib", + "//source/extensions/filters/http/rbac:rbac_filter_lib", "//test/mocks/network:network_mocks", + "//test/mocks/server:factory_context_mocks", "//test/mocks/ssl:ssl_mocks", "//test/mocks/stream_info:stream_info_mocks", "//test/test_common:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/config/rbac/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/matching/common_inputs/network/v3:pkg_cc_proto", ], ) @@ -50,6 +53,9 @@ envoy_extension_cc_test( extension_names = ["envoy.filters.http.rbac"], deps = [ "//source/extensions/filters/common/rbac:utility_lib", + "//source/extensions/filters/http/rbac:rbac_filter_lib", + "//test/mocks/server:factory_context_mocks", + "//test/test_common:utility_lib", ], ) diff --git a/test/extensions/filters/common/rbac/engine_impl_test.cc b/test/extensions/filters/common/rbac/engine_impl_test.cc index 24c727245f5dc..8b2c6d975a7db 100644 --- a/test/extensions/filters/common/rbac/engine_impl_test.cc +++ b/test/extensions/filters/common/rbac/engine_impl_test.cc @@ -1,11 +1,15 @@ #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/rbac/v3/rbac.pb.h" #include "envoy/config/rbac/v3/rbac.pb.validate.h" +#include "envoy/extensions/matching/common_inputs/network/v3/network_inputs.pb.h" +#include "source/common/network/address_impl.h" #include "source/common/network/utility.h" #include "source/extensions/filters/common/rbac/engine_impl.h" +#include "source/extensions/filters/http/rbac/rbac_filter.h" #include "test/mocks/network/mocks.h" +#include "test/mocks/server/factory_context.h" #include "test/mocks/ssl/mocks.h" #include "test/mocks/stream_info/mocks.h" #include "test/test_common/utility.h" @@ -57,6 +61,38 @@ void checkEngine( checkEngine(engine, expected, expected_log, empty_info, connection, headers); } +void checkMatcherEngine( + RBAC::RoleBasedAccessControlMatcherEngineImpl& engine, bool expected, LogResult expected_log, + StreamInfo::StreamInfo& info, + const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(), + const Envoy::Http::RequestHeaderMap& headers = Envoy::Http::TestRequestHeaderMapImpl()) { + + bool engineRes = engine.handleAction(connection, headers, info, nullptr); + EXPECT_EQ(expected, engineRes); + + if (expected_log != LogResult::Undecided) { + auto filter_meta = info.dynamicMetadata().filter_metadata().at( + RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace); + EXPECT_EQ(expected_log == LogResult::Yes, + filter_meta.fields() + .at(RBAC::DynamicMetadataKeysSingleton::get().AccessLogKey) + .bool_value()); + } else { + EXPECT_EQ(info.dynamicMetadata().filter_metadata().end(), + info.dynamicMetadata().filter_metadata().find( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace)); + } +} + +void checkMatcherEngine( + RBAC::RoleBasedAccessControlMatcherEngineImpl& engine, bool expected, LogResult expected_log, + const Envoy::Network::Connection& connection, + const Envoy::Http::RequestHeaderMap& headers = Envoy::Http::TestRequestHeaderMapImpl()) { + + NiceMock empty_info; + checkMatcherEngine(engine, expected, expected_log, empty_info, connection, headers); +} + void onMetadata(NiceMock& info) { ON_CALL(info, setDynamicMetadata("envoy.common", _)) .WillByDefault(Invoke([&info](const std::string&, const ProtobufWkt::Struct& obj) { @@ -429,6 +465,112 @@ TEST(RoleBasedAccessControlEngineImpl, ConjunctiveCondition) { checkEngine(engine, false, LogResult::Undecided, info, conn, headers); } +TEST(RoleBasedAccessControlMatcherEngineImpl, Disabled) { + xds::type::matcher::v3::Matcher matcher; + + NiceMock factory_context; + HttpFilters::RBACFilter::ActionValidationVisitor validation_visitor; + RBAC::RoleBasedAccessControlMatcherEngineImpl engine(matcher, factory_context, + validation_visitor); + + Envoy::Network::MockConnection conn; + Network::ConnectionInfoSetterImpl provider(std::make_shared(80), + std::make_shared(80)); + EXPECT_CALL(conn, connectionInfoProvider()).WillRepeatedly(ReturnRef(provider)); + + checkMatcherEngine(engine, false, LogResult::Undecided, conn); +} + +TEST(RoleBasedAccessControlMatcherEngineImpl, AllowedAllowlist) { + envoy::extensions::matching::common_inputs::network::v3::DestinationPortInput input; + envoy::config::rbac::v3::Action allow_action; + allow_action.set_name("allow"); + allow_action.set_action(envoy::config::rbac::v3::RBAC::ALLOW); + envoy::config::rbac::v3::Action deny_action; + deny_action.set_name("deny"); + deny_action.set_action(envoy::config::rbac::v3::RBAC::DENY); + + xds::type::matcher::v3::Matcher matcher; + auto matcher_matcher = matcher.mutable_matcher_list()->mutable_matchers()->Add(); + auto matcher_action = matcher_matcher->mutable_on_match()->mutable_action(); + matcher_action->set_name("action"); + matcher_action->mutable_typed_config()->PackFrom(allow_action); + auto matcher_predicate = matcher_matcher->mutable_predicate()->mutable_single_predicate(); + auto matcher_input = matcher_predicate->mutable_input(); + matcher_input->set_name("envoy.matching.inputs.destination_port"); + matcher_input->mutable_typed_config()->PackFrom(input); + matcher_predicate->mutable_value_match()->set_exact("123"); + + auto matcher_on_no_match_action = matcher.mutable_on_no_match()->mutable_action(); + matcher_on_no_match_action->set_name("action"); + matcher_on_no_match_action->mutable_typed_config()->PackFrom(deny_action); + + NiceMock factory_context; + HttpFilters::RBACFilter::ActionValidationVisitor validation_visitor; + RBAC::RoleBasedAccessControlMatcherEngineImpl engine(matcher, factory_context, + validation_visitor); + + Envoy::Network::MockConnection conn; + Envoy::Http::TestRequestHeaderMapImpl headers; + NiceMock info; + Network::ConnectionInfoSetterImpl provider(std::make_shared(80), + std::make_shared(80)); + EXPECT_CALL(conn, connectionInfoProvider()).WillRepeatedly(ReturnRef(provider)); + Envoy::Network::Address::InstanceConstSharedPtr addr = + Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 123, false); + provider.setLocalAddress(addr); + checkMatcherEngine(engine, true, LogResult::Undecided, info, conn, headers); + + addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 456, false); + provider.setLocalAddress(addr); + checkMatcherEngine(engine, false, LogResult::Undecided, info, conn, headers); +} + +TEST(RoleBasedAccessControlMatcherEngineImpl, DeniedDenylist) { + envoy::extensions::matching::common_inputs::network::v3::DestinationPortInput input; + envoy::config::rbac::v3::Action allow_action; + allow_action.set_name("allow"); + allow_action.set_action(envoy::config::rbac::v3::RBAC::ALLOW); + envoy::config::rbac::v3::Action deny_action; + deny_action.set_name("deny"); + deny_action.set_action(envoy::config::rbac::v3::RBAC::DENY); + + xds::type::matcher::v3::Matcher matcher; + auto matcher_matcher = matcher.mutable_matcher_list()->mutable_matchers()->Add(); + auto matcher_action = matcher_matcher->mutable_on_match()->mutable_action(); + matcher_action->set_name("action"); + matcher_action->mutable_typed_config()->PackFrom(deny_action); + auto matcher_predicate = matcher_matcher->mutable_predicate()->mutable_single_predicate(); + auto matcher_input = matcher_predicate->mutable_input(); + matcher_input->set_name("envoy.matching.inputs.destination_port"); + matcher_input->mutable_typed_config()->PackFrom(input); + matcher_predicate->mutable_value_match()->set_exact("123"); + + auto matcher_on_no_match_action = matcher.mutable_on_no_match()->mutable_action(); + matcher_on_no_match_action->set_name("action"); + matcher_on_no_match_action->mutable_typed_config()->PackFrom(allow_action); + + NiceMock factory_context; + HttpFilters::RBACFilter::ActionValidationVisitor validation_visitor; + RBAC::RoleBasedAccessControlMatcherEngineImpl engine(matcher, factory_context, + validation_visitor); + + Envoy::Network::MockConnection conn; + Envoy::Http::TestRequestHeaderMapImpl headers; + NiceMock info; + Network::ConnectionInfoSetterImpl provider(std::make_shared(80), + std::make_shared(80)); + EXPECT_CALL(conn, connectionInfoProvider()).WillRepeatedly(ReturnRef(provider)); + Envoy::Network::Address::InstanceConstSharedPtr addr = + Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 123, false); + provider.setLocalAddress(addr); + checkMatcherEngine(engine, false, LogResult::Undecided, info, conn, headers); + + addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 456, false); + provider.setLocalAddress(addr); + checkMatcherEngine(engine, true, LogResult::Undecided, info, conn, headers); +} + // Log tests TEST(RoleBasedAccessControlEngineImpl, DisabledLog) { NiceMock info; @@ -467,6 +609,53 @@ TEST(RoleBasedAccessControlEngineImpl, LogIfMatched) { checkEngine(engine, true, RBAC::LogResult::No, info, conn, headers); } +TEST(RoleBasedAccessControlMatcherEngineImpl, LogIfMatched) { + envoy::extensions::matching::common_inputs::network::v3::DestinationPortInput input; + envoy::config::rbac::v3::Action log_action; + log_action.set_name("log"); + log_action.set_action(envoy::config::rbac::v3::RBAC::LOG); + envoy::config::rbac::v3::Action allow_action; + allow_action.set_name("allow"); + allow_action.set_action(envoy::config::rbac::v3::RBAC::ALLOW); + + xds::type::matcher::v3::Matcher matcher; + auto matcher_matcher = matcher.mutable_matcher_list()->mutable_matchers()->Add(); + auto matcher_action = matcher_matcher->mutable_on_match()->mutable_action(); + matcher_action->set_name("action"); + matcher_action->mutable_typed_config()->PackFrom(log_action); + auto matcher_predicate = matcher_matcher->mutable_predicate()->mutable_single_predicate(); + auto matcher_input = matcher_predicate->mutable_input(); + matcher_input->set_name("envoy.matching.inputs.destination_port"); + matcher_input->mutable_typed_config()->PackFrom(input); + matcher_predicate->mutable_value_match()->set_exact("123"); + + auto matcher_on_no_match_action = matcher.mutable_on_no_match()->mutable_action(); + matcher_on_no_match_action->set_name("action"); + matcher_on_no_match_action->mutable_typed_config()->PackFrom(allow_action); + + NiceMock factory_context; + HttpFilters::RBACFilter::ActionValidationVisitor validation_visitor; + RBAC::RoleBasedAccessControlMatcherEngineImpl engine(matcher, factory_context, + validation_visitor); + + Envoy::Network::MockConnection conn; + Envoy::Http::TestRequestHeaderMapImpl headers; + NiceMock info; + Network::ConnectionInfoSetterImpl provider(std::make_shared(80), + std::make_shared(80)); + EXPECT_CALL(conn, connectionInfoProvider()).WillRepeatedly(ReturnRef(provider)); + onMetadata(info); + + Envoy::Network::Address::InstanceConstSharedPtr addr = + Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 123, false); + provider.setLocalAddress(addr); + checkMatcherEngine(engine, true, RBAC::LogResult::Yes, info, conn, headers); + + addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 456, false); + provider.setLocalAddress(addr); + checkMatcherEngine(engine, true, RBAC::LogResult::No, info, conn, headers); +} + } // namespace } // namespace RBAC } // namespace Common diff --git a/test/extensions/filters/common/rbac/mocks.h b/test/extensions/filters/common/rbac/mocks.h index 99503af52f4e4..83088d8ab6efe 100644 --- a/test/extensions/filters/common/rbac/mocks.h +++ b/test/extensions/filters/common/rbac/mocks.h @@ -6,6 +6,7 @@ #include "source/extensions/filters/common/rbac/engine_impl.h" #include "gmock/gmock.h" +#include "xds/type/matcher/v3/matcher.pb.h" namespace Envoy { namespace Extensions { @@ -31,6 +32,26 @@ class MockEngine : public RoleBasedAccessControlEngineImpl { (const)); }; +class MockMatcherEngine : public RoleBasedAccessControlMatcherEngineImpl { +public: + MockMatcherEngine(const xds::type::matcher::v3::Matcher& matcher, + Server::Configuration::ServerFactoryContext& factory_context, + ActionValidationVisitor& validation_visitor, + const EnforcementMode mode = EnforcementMode::Enforced) + : RoleBasedAccessControlMatcherEngineImpl(matcher, factory_context, validation_visitor, + mode){}; + + MOCK_METHOD(bool, handleAction, + (const Envoy::Network::Connection&, const Envoy::Http::RequestHeaderMap&, + StreamInfo::StreamInfo&, std::string* effective_policy_id), + (const)); + + MOCK_METHOD(bool, handleAction, + (const Envoy::Network::Connection&, StreamInfo::StreamInfo&, + std::string* effective_policy_id), + (const)); +}; + } // namespace RBAC } // namespace Common } // namespace Filters diff --git a/test/extensions/filters/common/rbac/utility_test.cc b/test/extensions/filters/common/rbac/utility_test.cc index 1bc4f7de17547..37525eb004aa1 100644 --- a/test/extensions/filters/common/rbac/utility_test.cc +++ b/test/extensions/filters/common/rbac/utility_test.cc @@ -1,6 +1,8 @@ #include "source/extensions/filters/common/rbac/utility.h" +#include "source/extensions/filters/http/rbac/rbac_filter.h" -#include "gtest/gtest.h" +#include "test/mocks/server/factory_context.h" +#include "test/test_common/utility.h" namespace Envoy { namespace Extensions { @@ -15,6 +17,54 @@ TEST(ResponseDetail, ResponseDetail) { EXPECT_EQ(RBAC::responseDetail("a \t\f\v\n\ry"), "rbac_access_denied_matched_policy[a______y]"); } +TEST(CreateEngine, IgnoreRules) { + envoy::extensions::filters::http::rbac::v3::RBAC config; + std::string yaml = R"EOF( +rules: + action: ALLOW +matcher: + on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: none + action: ALLOW +)EOF"; + TestUtility::loadFromYaml(yaml, config); + + NiceMock factory_context; + HttpFilters::RBACFilter::ActionValidationVisitor validation_visitor; + EXPECT_LOG_CONTAINS("warn", "RBAC rules are ignored when matcher is configured", + createEngine(config, factory_context, + ProtobufMessage::getStrictValidationVisitor(), + validation_visitor)); +} + +TEST(CreateShadowEngine, IgnoreShadowRules) { + envoy::extensions::filters::http::rbac::v3::RBAC config; + std::string yaml = R"EOF( +shadow_rules: + action: ALLOW +shadow_matcher: + on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: none + action: ALLOW +)EOF"; + TestUtility::loadFromYaml(yaml, config); + + NiceMock factory_context; + HttpFilters::RBACFilter::ActionValidationVisitor validation_visitor; + EXPECT_LOG_CONTAINS("warn", "RBAC shadow rules are ignored when shadow matcher is configured", + createShadowEngine(config, factory_context, + ProtobufMessage::getStrictValidationVisitor(), + validation_visitor)); +} + } // namespace } // namespace RBAC } // namespace Common diff --git a/test/extensions/filters/http/rbac/BUILD b/test/extensions/filters/http/rbac/BUILD index 1e90100c982f4..185e1346c605f 100644 --- a/test/extensions/filters/http/rbac/BUILD +++ b/test/extensions/filters/http/rbac/BUILD @@ -36,9 +36,12 @@ envoy_extension_cc_test( "//test/extensions/filters/http/rbac:route_config_mocks", "//test/mocks/http:http_mocks", "//test/mocks/network:network_mocks", + "//test/mocks/server:factory_context_mocks", "@envoy_api//envoy/config/rbac/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/http/rbac/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/matching/common_inputs/network/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/rbac/matchers/upstream_ip_port/v3:pkg_cc_proto", + "@envoy_api//envoy/type/matcher/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/filters/http/rbac/config_test.cc b/test/extensions/filters/http/rbac/config_test.cc index 8871521627f5c..388ce2b650e79 100644 --- a/test/extensions/filters/http/rbac/config_test.cc +++ b/test/extensions/filters/http/rbac/config_test.cc @@ -4,6 +4,7 @@ #include "source/extensions/filters/common/rbac/engine.h" #include "source/extensions/filters/http/rbac/config.h" +#include "source/extensions/filters/http/rbac/rbac_filter.h" #include "test/mocks/server/factory_context.h" #include "test/mocks/server/instance.h" @@ -35,6 +36,26 @@ TEST(RoleBasedAccessControlFilterConfigFactoryTest, ValidProto) { cb(filter_callbacks); } +TEST(RoleBasedAccessControlFilterConfigFactoryTest, ValidMatcherProto) { + envoy::config::rbac::v3::Action action; + action.set_name("foo"); + action.set_action(envoy::config::rbac::v3::RBAC::ALLOW); + + xds::type::matcher::v3::Matcher matcher; + auto matcher_action = matcher.mutable_on_no_match()->mutable_action(); + matcher_action->set_name("action"); + matcher_action->mutable_typed_config()->PackFrom(action); + envoy::extensions::filters::http::rbac::v3::RBAC config; + *config.mutable_matcher() = matcher; + + NiceMock context; + RoleBasedAccessControlFilterConfigFactory factory; + Http::FilterFactoryCb cb = factory.createFilterFactoryFromProto(config, "stats", context); + Http::MockFilterChainFactoryCallbacks filter_callbacks; + EXPECT_CALL(filter_callbacks, addStreamDecoderFilter(_)); + cb(filter_callbacks); +} + TEST(RoleBasedAccessControlFilterConfigFactoryTest, EmptyProto) { RoleBasedAccessControlFilterConfigFactory factory; EXPECT_NE(nullptr, dynamic_cast( @@ -47,6 +68,42 @@ TEST(RoleBasedAccessControlFilterConfigFactoryTest, EmptyRouteProto) { factory.createEmptyRouteConfigProto().get())); } +TEST(RoleBasedAccessControlFilterConfigFactoryTest, InvalidMatcherProto) { + xds::type::matcher::v3::Matcher matcher_proto{}; + TestUtility::loadFromYaml(R"EOF( +matcher_tree: + input: + name: source-ip + typed_config: + '@type': type.googleapis.com/envoy.type.matcher.v3.HttpResponseHeaderMatchInput + header_name: foo + exact_match_map: + map: + "bar": + action: + name: action + typed_config: + '@type': type.googleapis.com/envoy.config.rbac.v3.Action + name: deny +)EOF", + matcher_proto); + + envoy::extensions::filters::http::rbac::v3::RBAC config{}; + *config.mutable_matcher() = matcher_proto; + + Stats::IsolatedStoreImpl store; + NiceMock context; + EXPECT_THROW(std::make_shared( + config, "test", store, context, ProtobufMessage::getStrictValidationVisitor()), + Envoy::EnvoyException); + + config.clear_matcher(); + *config.mutable_shadow_matcher() = matcher_proto; + EXPECT_THROW(std::make_shared( + config, "test", store, context, ProtobufMessage::getStrictValidationVisitor()), + Envoy::EnvoyException); +} + TEST(RoleBasedAccessControlFilterConfigFactoryTest, RouteSpecificConfig) { RoleBasedAccessControlFilterConfigFactory factory; NiceMock context; diff --git a/test/extensions/filters/http/rbac/mocks.h b/test/extensions/filters/http/rbac/mocks.h index 3a079fc5758d6..4e0f5852da1b1 100644 --- a/test/extensions/filters/http/rbac/mocks.h +++ b/test/extensions/filters/http/rbac/mocks.h @@ -18,11 +18,12 @@ class MockRoleBasedAccessControlRouteSpecificFilterConfig : public RoleBasedAccessControlRouteSpecificFilterConfig { public: MockRoleBasedAccessControlRouteSpecificFilterConfig( - const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& r) + const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& r, + Server::Configuration::ServerFactoryContext& context) : RoleBasedAccessControlRouteSpecificFilterConfig( - r, ProtobufMessage::getStrictValidationVisitor()){}; + r, context, ProtobufMessage::getStrictValidationVisitor()){}; - MOCK_METHOD(Filters::Common::RBAC::RoleBasedAccessControlEngineImpl&, engine, (), (const)); + MOCK_METHOD(Filters::Common::RBAC::RoleBasedAccessControlEngine&, engine, (), (const)); }; } // namespace diff --git a/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc b/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc index 254bb8b467fa8..9d6aa54d25af5 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc @@ -144,6 +144,201 @@ name: rbac string_value: {} )EOF"; +const std::string RBAC_MATCHER_CONFIG = R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC + matcher: + matcher_list: + matchers: + - predicate: + single_predicate: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: :method + value_match: + exact: GET + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: foo + action: ALLOW +)EOF"; + +const std::string RBAC_MATCHER_CONFIG_WITH_DENY_ACTION = R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC + matcher: + matcher_list: + matchers: + - predicate: + single_predicate: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: :method + value_match: + exact: GET + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: "deny policy" + action: DENY + on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: none + action: ALLOW +)EOF"; + +const std::string RBAC_MATCHER_CONFIG_WITH_PREFIX_MATCH = R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC + matcher: + matcher_list: + matchers: + - predicate: + single_predicate: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: :path + value_match: + prefix: "/foo" + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: foo + action: ALLOW +)EOF"; + +// TODO(zhxie): it is not equivalent with the URL path rule in RBAC_CONFIG_WITH_PATH_EXACT_MATCH. +// There will be a replacement when the URL path custom matcher is ready. +const std::string RBAC_MATCHER_CONFIG_WITH_PATH_EXACT_MATCH = R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC + matcher: + matcher_list: + matchers: + - predicate: + single_predicate: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: :path + value_match: + prefix: "/allow" + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: foo + action: ALLOW +)EOF"; + +// TODO(zhxie): it is not equivalent with the URL path rule in +// RBAC_CONFIG_DENY_WITH_PATH_EXACT_MATCH. There will be a replacement when the URL path custom +// matcher is ready. +const std::string RBAC_MATCHER_CONFIG_DENY_WITH_PATH_EXACT_MATCH = R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC + matcher: + matcher_list: + matchers: + - predicate: + single_predicate: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: :path + value_match: + prefix: "/deny" + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: foo + action: DENY +)EOF"; + +const std::string RBAC_MATCHER_CONFIG_WITH_PATH_IGNORE_CASE_MATCH = R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC + matcher: + matcher_list: + matchers: + - predicate: + single_predicate: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: :path + value_match: + exact: "/ignore_case" + ignore_case: true + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: foo + action: ALLOW +)EOF"; + +const std::string RBAC_MATCHER_CONFIG_WITH_LOG_ACTION = R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC + matcher: + matcher_list: + matchers: + - predicate: + single_predicate: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: :method + value_match: + exact: GET + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: foo + action: LOG + on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: none + action: ALLOW +)EOF"; + using RBACIntegrationTest = HttpProtocolIntegrationTest; INSTANTIATE_TEST_SUITE_P(Protocols, RBACIntegrationTest, @@ -540,6 +735,323 @@ TEST_P(RBACIntegrationTest, HeaderMatchConditionDuplicateHeaderMatch) { EXPECT_EQ("200", response->headers().getStatusValue()); } +TEST_P(RBACIntegrationTest, MatcherAllowed) { + useAccessLog("%RESPONSE_CODE_DETAILS%"); + config_helper_.prependFilter(RBAC_MATCHER_CONFIG); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); + EXPECT_THAT(waitForAccessLog(access_log_name_), testing::HasSubstr("via_upstream")); +} + +TEST_P(RBACIntegrationTest, MatcherDenied) { + useAccessLog("%RESPONSE_CODE_DETAILS%"); + config_helper_.prependFilter(RBAC_MATCHER_CONFIG); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("403", response->headers().getStatusValue()); + EXPECT_THAT(waitForAccessLog(access_log_name_), + testing::HasSubstr("rbac_access_denied_matched_policy[none]")); +} + +TEST_P(RBACIntegrationTest, MatcherDeniedWithDenyAction) { + useAccessLog("%RESPONSE_CODE_DETAILS%"); + config_helper_.prependFilter(RBAC_MATCHER_CONFIG_WITH_DENY_ACTION); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("403", response->headers().getStatusValue()); + // Note the whitespace in the policy id is replaced by '_'. + EXPECT_THAT(waitForAccessLog(access_log_name_), + testing::HasSubstr("rbac_access_denied_matched_policy[deny_policy]")); +} + +TEST_P(RBACIntegrationTest, MatcherDeniedWithPrefixRule) { + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + cfg) { cfg.mutable_normalize_path()->set_value(false); }); + config_helper_.prependFilter(RBAC_MATCHER_CONFIG_WITH_PREFIX_MATCH); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, + {":path", "/foo/../bar"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); +} + +TEST_P(RBACIntegrationTest, RbacPrefixRuleUseNormalizePathMatcher) { + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + cfg) { cfg.mutable_normalize_path()->set_value(true); }); + config_helper_.prependFilter(RBAC_MATCHER_CONFIG_WITH_PREFIX_MATCH); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, + {":path", "/foo/../bar"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("403", response->headers().getStatusValue()); +} + +TEST_P(RBACIntegrationTest, MatcherDeniedHeadReply) { + config_helper_.prependFilter(RBAC_MATCHER_CONFIG); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "HEAD"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("403", response->headers().getStatusValue()); + ASSERT_TRUE(response->headers().ContentLength()); + EXPECT_NE("0", response->headers().getContentLengthValue()); + EXPECT_THAT(response->body(), ::testing::IsEmpty()); +} + +TEST_P(RBACIntegrationTest, MatcherRouteOverride) { + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + cfg) { + envoy::extensions::filters::http::rbac::v3::RBACPerRoute per_route_config; + TestUtility::loadFromJson("{}", per_route_config); + + auto* config = cfg.mutable_route_config() + ->mutable_virtual_hosts() + ->Mutable(0) + ->mutable_typed_per_filter_config(); + + (*config)["envoy.filters.http.rbac"].PackFrom(per_route_config); + }); + config_helper_.prependFilter(RBAC_MATCHER_CONFIG); + + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); +} + +TEST_P(RBACIntegrationTest, PathMatcherWithQueryAndFragmentWithOverride) { + config_helper_.prependFilter(RBAC_MATCHER_CONFIG_WITH_PATH_EXACT_MATCH); + config_helper_.addRuntimeOverride("envoy.reloadable_features.http_reject_path_with_fragment", + "false"); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + const std::vector paths{"/allow", "/allow?p1=v1&p2=v2", "/allow?p1=v1#seg"}; + + for (const auto& path : paths) { + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, + {":path", path}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); + } +} + +TEST_P(RBACIntegrationTest, PathMatcherWithFragmentRejectedByDefault) { + config_helper_.prependFilter(RBAC_MATCHER_CONFIG_WITH_PATH_EXACT_MATCH); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, + {":path", "/allow?p1=v1#seg"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + // Request should not hit the upstream + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("400", response->headers().getStatusValue()); +} + +// This test ensures that the exact match deny rule is not affected by fragment and query +// when Envoy is configured to strip both fragment and query. +TEST_P(RBACIntegrationTest, MatcherDenyExactMatchIgnoresQueryAndFragment) { + config_helper_.prependFilter(RBAC_MATCHER_CONFIG_DENY_WITH_PATH_EXACT_MATCH); + config_helper_.addRuntimeOverride("envoy.reloadable_features.http_reject_path_with_fragment", + "false"); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + const std::vector paths{"/deny#", "/deny#fragment", "/deny?p1=v1&p2=v2", + "/deny?p1=v1#seg"}; + + for (const auto& path : paths) { + printf("Testing: %s\n", path.c_str()); + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, + {":path", path}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("403", response->headers().getStatusValue()); + if (downstreamProtocol() == Http::CodecClient::Type::HTTP1) { + ASSERT_TRUE(codec_client_->waitForDisconnect()); + codec_client_ = makeHttpConnection(lookupPort("http")); + } + } +} + +TEST_P(RBACIntegrationTest, PathIgnoreCaseMatcher) { + config_helper_.prependFilter(RBAC_MATCHER_CONFIG_WITH_PATH_IGNORE_CASE_MATCH); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + const std::vector paths{"/ignore_case", "/IGNORE_CASE", "/ignore_CASE"}; + + for (const auto& path : paths) { + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, + {":path", path}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); + } +} + +TEST_P(RBACIntegrationTest, MatcherLogConnectionAllow) { + config_helper_.prependFilter(RBAC_MATCHER_CONFIG_WITH_LOG_ACTION); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeRequestWithBody( + Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + }, + 1024); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); +} + // Helper for integration testing of RBAC filter with dynamic forward proxy. class RbacDynamicForwardProxyIntegrationHelper : public testing::TestWithParam, diff --git a/test/extensions/filters/http/rbac/rbac_filter_test.cc b/test/extensions/filters/http/rbac/rbac_filter_test.cc index 9b2dc930d71a6..651ad1c38ad5d 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_test.cc @@ -1,8 +1,11 @@ #include "envoy/config/rbac/v3/rbac.pb.h" #include "envoy/extensions/filters/http/rbac/v3/rbac.pb.h" +#include "envoy/extensions/matching/common_inputs/network/v3/network_inputs.pb.h" #include "envoy/extensions/rbac/matchers/upstream_ip_port/v3/upstream_ip_port_matcher.pb.h" +#include "envoy/type/matcher/v3/http_inputs.pb.h" #include "source/common/config/metadata.h" +#include "source/common/network/address_impl.h" #include "source/common/network/utility.h" #include "source/common/stream_info/upstream_address.h" #include "source/extensions/filters/common/rbac/utility.h" @@ -12,6 +15,9 @@ #include "test/extensions/filters/http/rbac/mocks.h" #include "test/mocks/http/mocks.h" #include "test/mocks/network/mocks.h" +#include "test/mocks/server/factory_context.h" + +#include "xds/type/matcher/v3/matcher.pb.h" using testing::_; using testing::NiceMock; @@ -28,8 +34,7 @@ enum class LogResult { Yes, No, Undecided }; class RoleBasedAccessControlFilterTest : public testing::Test { public: - RoleBasedAccessControlFilterConfigSharedPtr - setupConfig(envoy::config::rbac::v3::RBAC::Action action) { + void setupPolicy(envoy::config::rbac::v3::RBAC::Action action) { envoy::extensions::filters::http::rbac::v3::RBAC config; envoy::config::rbac::v3::Policy policy; @@ -51,38 +56,137 @@ class RoleBasedAccessControlFilterTest : public testing::Test { (*config.mutable_shadow_rules()->mutable_policies())["bar"] = shadow_policy; config.set_shadow_rules_stat_prefix("prefix_"); - return std::make_shared( - config, "test", store_, ProtobufMessage::getStrictValidationVisitor()); + setupConfig(std::make_shared( + config, "test", store_, context_, ProtobufMessage::getStrictValidationVisitor())); } - RoleBasedAccessControlFilterTest() - : config_(setupConfig(envoy::config::rbac::v3::RBAC::ALLOW)), filter_(config_) {} + void setupMatcher(std::string action, std::string on_no_match_action) { + envoy::extensions::filters::http::rbac::v3::RBAC config; - void SetUp() override { - config_ = setupConfig(envoy::config::rbac::v3::RBAC::ALLOW); - filter_ = RoleBasedAccessControlFilter(config_); + const std::string matcher_yaml = R"EOF( +matcher_list: + matchers: + - predicate: + or_matcher: + predicate: + - single_predicate: + input: + name: envoy.matching.inputs.server_name + typed_config: + "@type": type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.ServerNameInput + value_match: + safe_regex: + google_re2: {} + regex: .*cncf.io + - single_predicate: + input: + name: envoy.matching.inputs.destination_port + typed_config: + "@type": type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DestinationPortInput + value_match: + exact: "123" + - single_predicate: + input: + name: envoy.matching.inputs.request_headers + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: ":path" + value_match: + # TODO(zhxie): it is not equivalent with the URL path rule in setupPolicy(). There + # will be a replacement when the URL path custom matcher is ready. + prefix: "/suffix" + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: foo + action: {} +on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: none + action: {} +)EOF"; + const std::string shadow_matcher_yaml = R"EOF( +matcher_list: + matchers: + - predicate: + or_matcher: + predicate: + - single_predicate: + input: + name: envoy.matching.inputs.server_name + typed_config: + "@type": type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.ServerNameInput + value_match: + exact: xyz.cncf.io + - single_predicate: + input: + name: envoy.matching.inputs.destination_port + typed_config: + "@type": type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DestinationPortInput + value_match: + exact: "456" + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: bar + action: {} +on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: none + action: {} +)EOF"; + + xds::type::matcher::v3::Matcher matcher; + TestUtility::loadFromYaml(fmt::format(matcher_yaml, "{}", action, on_no_match_action), matcher); + *config.mutable_matcher() = matcher; + + xds::type::matcher::v3::Matcher shadow_matcher; + TestUtility::loadFromYaml(fmt::format(shadow_matcher_yaml, action, on_no_match_action), + shadow_matcher); + *config.mutable_shadow_matcher() = shadow_matcher; + config.set_shadow_rules_stat_prefix("prefix_"); - EXPECT_CALL(callbacks_, connection()).WillRepeatedly(Return(&connection_)); - EXPECT_CALL(callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_)); - filter_.setDecoderFilterCallbacks(callbacks_); + setupConfig(std::make_shared( + config, "test", store_, context_, ProtobufMessage::getStrictValidationVisitor())); } - void SetUp(RoleBasedAccessControlFilterConfigSharedPtr config) { + void setupConfig(RoleBasedAccessControlFilterConfigSharedPtr config) { config_ = config; - filter_ = RoleBasedAccessControlFilter(config_); + filter_ = std::make_unique(config_); EXPECT_CALL(callbacks_, connection()).WillRepeatedly(Return(&connection_)); EXPECT_CALL(callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_)); - filter_.setDecoderFilterCallbacks(callbacks_); + filter_->setDecoderFilterCallbacks(callbacks_); } + + RoleBasedAccessControlFilterTest() + : provider_(std::make_shared(80), + std::make_shared(80)){}; + void setDestinationPort(uint16_t port) { address_ = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", port, false); req_info_.downstream_connection_info_provider_->setLocalAddress(address_); + + provider_.setLocalAddress(address_); + ON_CALL(connection_, connectionInfoProvider()).WillByDefault(ReturnRef(provider_)); } void setRequestedServerName(std::string server_name) { requested_server_name_ = server_name; ON_CALL(connection_, requestedServerName()).WillByDefault(Return(requested_server_name_)); + + provider_.setRequestedServerName(server_name); + ON_CALL(connection_, connectionInfoProvider()).WillByDefault(ReturnRef(provider_)); } void checkAccessLogMetadata(LogResult expected) { @@ -121,22 +225,26 @@ class RoleBasedAccessControlFilterTest : public testing::Test { NiceMock connection_{}; NiceMock req_info_; Stats::IsolatedStoreImpl store_; + NiceMock context_; RoleBasedAccessControlFilterConfigSharedPtr config_; - RoleBasedAccessControlFilter filter_; + std::unique_ptr filter_; Network::Address::InstanceConstSharedPtr address_; + Network::ConnectionInfoSetterImpl provider_; std::string requested_server_name_; Http::TestRequestHeaderMapImpl headers_; Http::TestRequestTrailerMapImpl trailers_; }; TEST_F(RoleBasedAccessControlFilterTest, Allowed) { + setupPolicy(envoy::config::rbac::v3::RBAC::ALLOW); + setDestinationPort(123); setMetadata(); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); Http::MetadataMap metadata_map{{"metadata", "metadata"}}; - EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_.decodeMetadata(metadata_map)); + EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->decodeMetadata(metadata_map)); EXPECT_EQ(1U, config_->stats().allowed_.value()); EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name()); @@ -145,18 +253,20 @@ TEST_F(RoleBasedAccessControlFilterTest, Allowed) { EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); - EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false)); - EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.decodeTrailers(trailers_)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(trailers_)); checkAccessLogMetadata(LogResult::Undecided); } TEST_F(RoleBasedAccessControlFilterTest, RequestedServerName) { + setupPolicy(envoy::config::rbac::v3::RBAC::ALLOW); + setDestinationPort(999); setRequestedServerName("www.cncf.io"); setMetadata(); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); EXPECT_EQ(1U, config_->stats().allowed_.value()); EXPECT_EQ(0U, config_->stats().denied_.value()); EXPECT_EQ(0U, config_->stats().shadow_allowed_.value()); @@ -167,13 +277,15 @@ TEST_F(RoleBasedAccessControlFilterTest, RequestedServerName) { EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); - EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false)); - EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.decodeTrailers(trailers_)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(trailers_)); checkAccessLogMetadata(LogResult::Undecided); } TEST_F(RoleBasedAccessControlFilterTest, Path) { + setupPolicy(envoy::config::rbac::v3::RBAC::ALLOW); + setDestinationPort(999); setMetadata(); @@ -183,11 +295,13 @@ TEST_F(RoleBasedAccessControlFilterTest, Path) { {":scheme", "http"}, {":authority", "host"}, }; - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers, false)); checkAccessLogMetadata(LogResult::Undecided); } TEST_F(RoleBasedAccessControlFilterTest, Denied) { + setupPolicy(envoy::config::rbac::v3::RBAC::ALLOW); + setDestinationPort(456); setMetadata(); @@ -199,7 +313,7 @@ TEST_F(RoleBasedAccessControlFilterTest, Denied) { EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), false)); EXPECT_CALL(callbacks_, encodeData(_, true)); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, true)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(headers_, true)); EXPECT_EQ(1U, config_->stats().denied_.value()); EXPECT_EQ(1U, config_->stats().shadow_allowed_.value()); EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name()); @@ -215,13 +329,139 @@ TEST_F(RoleBasedAccessControlFilterTest, Denied) { } TEST_F(RoleBasedAccessControlFilterTest, RouteLocalOverride) { + setupPolicy(envoy::config::rbac::v3::RBAC::ALLOW); + setDestinationPort(456); setMetadata(); 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 per_route_config_{route_config}; + NiceMock per_route_config_{route_config, + context_}; + + EXPECT_CALL(engine, handleAction(_, _, _, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(per_route_config_, engine()).WillRepeatedly(ReturnRef(engine)); + + EXPECT_CALL(*callbacks_.route_, mostSpecificPerFilterConfig("envoy.filters.http.rbac")) + .WillRepeatedly(Return(&per_route_config_)); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, true)); + checkAccessLogMetadata(LogResult::Undecided); +} + +TEST_F(RoleBasedAccessControlFilterTest, MatcherAllowed) { + setupMatcher("ALLOW", "DENY"); + + setDestinationPort(123); + setMetadata(); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); + Http::MetadataMap metadata_map{{"metadata", "metadata"}}; + EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->decodeMetadata(metadata_map)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("testrbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("testrbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + + Buffer::OwnedImpl data(""); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(trailers_)); + + checkAccessLogMetadata(LogResult::Undecided); +} + +TEST_F(RoleBasedAccessControlFilterTest, RequestedServerNameMatcher) { + setupMatcher("ALLOW", "DENY"); + + setDestinationPort(999); + setRequestedServerName("www.cncf.io"); + setMetadata(); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().denied_.value()); + EXPECT_EQ(0U, config_->stats().shadow_allowed_.value()); + EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("testrbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("testrbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + + Buffer::OwnedImpl data(""); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(trailers_)); + + checkAccessLogMetadata(LogResult::Undecided); +} + +TEST_F(RoleBasedAccessControlFilterTest, PathMatcher) { + setupMatcher("ALLOW", "DENY"); + + setDestinationPort(999); + setMetadata(); + + auto headers = Http::TestRequestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/suffix#seg?param=value"}, + {":scheme", "http"}, + {":authority", "host"}, + }; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers, false)); + checkAccessLogMetadata(LogResult::Undecided); +} + +TEST_F(RoleBasedAccessControlFilterTest, MatcherDenied) { + setupMatcher("ALLOW", "DENY"); + + setDestinationPort(456); + setMetadata(); + + Http::TestResponseHeaderMapImpl response_headers{ + {":status", "403"}, + {"content-length", "19"}, + {"content-type", "text/plain"}, + }; + EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), false)); + EXPECT_CALL(callbacks_, encodeData(_, true)); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(headers_, true)); + EXPECT_EQ(1U, config_->stats().denied_.value()); + EXPECT_EQ(1U, config_->stats().shadow_allowed_.value()); + EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("testrbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("testrbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + + auto filter_meta = req_info_.dynamicMetadata().filter_metadata().at("envoy.filters.http.rbac"); + EXPECT_EQ("allowed", filter_meta.fields().at("prefix_shadow_engine_result").string_value()); + EXPECT_EQ("bar", filter_meta.fields().at("prefix_shadow_effective_policy_id").string_value()); + EXPECT_EQ("rbac_access_denied_matched_policy[none]", callbacks_.details()); + checkAccessLogMetadata(LogResult::Undecided); +} + +TEST_F(RoleBasedAccessControlFilterTest, MatcherRouteLocalOverride) { + setupMatcher("ALLOW", "DENY"); + + setDestinationPort(456); + setMetadata(); + + envoy::extensions::filters::http::rbac::v3::RBACPerRoute route_config; + envoy::config::rbac::v3::Action action; + action.set_name("none"); + action.set_action(envoy::config::rbac::v3::RBAC::ALLOW); + xds::type::matcher::v3::Matcher matcher; + auto matcher_on_no_match_action = matcher.mutable_on_no_match()->mutable_action(); + matcher_on_no_match_action->set_name("action"); + matcher_on_no_match_action->mutable_typed_config()->PackFrom(action); + *route_config.mutable_rbac()->mutable_matcher() = matcher; + ActionValidationVisitor validation_visitor; + NiceMock engine{route_config.rbac().matcher(), context_, + validation_visitor}; + NiceMock per_route_config_{route_config, + context_}; EXPECT_CALL(engine, handleAction(_, _, _, _)).WillRepeatedly(Return(true)); EXPECT_CALL(per_route_config_, engine()).WillRepeatedly(ReturnRef(engine)); @@ -229,20 +469,18 @@ TEST_F(RoleBasedAccessControlFilterTest, RouteLocalOverride) { EXPECT_CALL(*callbacks_.route_, mostSpecificPerFilterConfig("envoy.filters.http.rbac")) .WillRepeatedly(Return(&per_route_config_)); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, true)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, true)); checkAccessLogMetadata(LogResult::Undecided); } // Log Tests TEST_F(RoleBasedAccessControlFilterTest, ShouldLog) { - config_ = setupConfig(envoy::config::rbac::v3::RBAC::LOG); - filter_ = RoleBasedAccessControlFilter(config_); - filter_.setDecoderFilterCallbacks(callbacks_); + setupPolicy(envoy::config::rbac::v3::RBAC::LOG); setDestinationPort(123); setMetadata(); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); EXPECT_EQ(1U, config_->stats().allowed_.value()); EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name()); @@ -251,21 +489,19 @@ TEST_F(RoleBasedAccessControlFilterTest, ShouldLog) { EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); - EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false)); - EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.decodeTrailers(trailers_)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(trailers_)); checkAccessLogMetadata(LogResult::Yes); } TEST_F(RoleBasedAccessControlFilterTest, ShouldNotLog) { - config_ = setupConfig(envoy::config::rbac::v3::RBAC::LOG); - filter_ = RoleBasedAccessControlFilter(config_); - filter_.setDecoderFilterCallbacks(callbacks_); + setupPolicy(envoy::config::rbac::v3::RBAC::LOG); setDestinationPort(456); setMetadata(); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); EXPECT_EQ(1U, config_->stats().allowed_.value()); EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name()); @@ -274,8 +510,50 @@ TEST_F(RoleBasedAccessControlFilterTest, ShouldNotLog) { EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); - EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false)); - EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.decodeTrailers(trailers_)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(trailers_)); + + checkAccessLogMetadata(LogResult::No); +} + +TEST_F(RoleBasedAccessControlFilterTest, MatcherShouldLog) { + setupMatcher("LOG", "ALLOW"); + + setDestinationPort(123); + setMetadata(); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("testrbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("testrbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + + Buffer::OwnedImpl data(""); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(trailers_)); + + checkAccessLogMetadata(LogResult::Yes); +} + +TEST_F(RoleBasedAccessControlFilterTest, MatcherShouldNotLog) { + setupMatcher("LOG", "ALLOW"); + + setDestinationPort(456); + setMetadata(); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("testrbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("testrbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + + Buffer::OwnedImpl data(""); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(trailers_)); checkAccessLogMetadata(LogResult::No); } @@ -340,10 +618,10 @@ class UpstreamIpPortMatcherTests : public RoleBasedAccessControlFilterTest { (*config.mutable_rules()->mutable_policies())["foo"] = policy; auto config_ptr = std::make_shared( - config, "test", store_, ProtobufMessage::getStrictValidationVisitor()); + config, "test", store_, context_, ProtobufMessage::getStrictValidationVisitor()); // Setup test with the policy config. - SetUp(config_ptr); + setupConfig(config_ptr); } void upstreamIpTestsFilterStateSetup(NiceMock& callback, @@ -373,7 +651,7 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamIpNoFilterStateMetadata) { upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::ALLOW); // Filter iteration should be stopped as there is no filter state metadata. - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(headers_, false)); // Expect `denied` stats to be incremented. EXPECT_EQ(1U, config_->stats().denied_.value()); @@ -382,17 +660,17 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamIpNoFilterStateMetadata) { // Tests simple upstream_ip ALLOW permission policy with ONLY upstream ip metadata in the filter // state. TEST_F(UpstreamIpPortMatcherTests, UpstreamIpWithFilterStateAllow) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); - // Setup policy config. const std::vector configs = { {"1.2.3.4"}, }; upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::ALLOW); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + // Filter iteration should continue since the policy is ALLOW. - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); // Expect `allowed` stats to be incremented. EXPECT_EQ(1U, config_->stats().allowed_.value()); @@ -401,9 +679,6 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamIpWithFilterStateAllow) { // Tests simple upstream_ip DENY permission policy with ONLY upstream ip metadata in the filter // state. TEST_F(UpstreamIpPortMatcherTests, UpstreamIpWithFilterStateDeny) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); - // Setup policy config. const std::vector configs = { {"1.2.3.4"}, @@ -411,8 +686,11 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamIpWithFilterStateDeny) { upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + // Filter iteration should stop since the policy is DENY. - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(headers_, false)); // Expect `denied` stats to be incremented. EXPECT_EQ(1U, config_->stats().denied_.value()); @@ -421,9 +699,6 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamIpWithFilterStateDeny) { // Tests simple upstream_ip DENY permission policy with BOTH upstream ip and port matching the // policy. TEST_F(UpstreamIpPortMatcherTests, UpstreamIpPortMatchDeny) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); - const std::vector configs = { {"1.2.3.4", 120, 123}, }; @@ -431,17 +706,17 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamIpPortMatchDeny) { // Setup policy config. upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + // Filter iteration should stop since the policy is DENY. - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(headers_, false)); // Expect `denied` stats to be incremented. EXPECT_EQ(1U, config_->stats().denied_.value()); } TEST_F(UpstreamIpPortMatcherTests, UpstreamIpPortMatchAllow) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); - const std::vector configs = { {"1.2.3.4", 120, 123}, }; @@ -449,16 +724,16 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamIpPortMatchAllow) { // Setup policy config. upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::ALLOW); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); // Expect `allowed` stats to be incremented. EXPECT_EQ(1U, config_->stats().allowed_.value()); } TEST_F(UpstreamIpPortMatcherTests, UpstreamPortMatchDeny) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); - const std::vector configs = { {120, 123}, }; @@ -466,17 +741,17 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamPortMatchDeny) { // Setup policy config. upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + // Filter iteration should stop since the policy is DENY. - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(headers_, false)); // Expect `denied` stats to be incremented. EXPECT_EQ(1U, config_->stats().denied_.value()); } TEST_F(UpstreamIpPortMatcherTests, UpstreamPortMatchAllow) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); - const std::vector configs = { {120, 123}, }; @@ -484,7 +759,10 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamPortMatchAllow) { // Setup policy config. upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::ALLOW); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); // Expect `allowed` stats to be incremented. EXPECT_EQ(1U, config_->stats().allowed_.value()); @@ -493,17 +771,17 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamPortMatchAllow) { // Tests upstream_ip DENY permission policy with multiple upstream ips to match in the policy. // If any of the configured upstream ip addresses match the metadata, the policy is enforced (DENY). TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsAnyPolicyDeny) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); - // Setup policy config. const std::vector configs = { {"1.1.1.2"}, {"1.2.3.4", 120, 123}, {"1.2.3.5"}}; upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + // Filter iteration should stop since the policy is DENY. - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(headers_, false)); // Expect `denied` stats to be incremented. EXPECT_EQ(1U, config_->stats().denied_.value()); @@ -512,16 +790,16 @@ TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsAnyPolicyDeny) { // Tests upstream_ip DENY permission policy with multiple upstream ips to match in the policy. // If ONLY port is configured in the policy, a match should enforce the policy. TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsNoIpMatchPortMatchDeny) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"2.2.3.4:123"}); - // Setup policy config. const std::vector configs = {{"1.1.1.2"}, {120, 123}, {"1.2.3.5"}}; upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"2.2.3.4:123"}); + // Filter iteration should stop since the policy is DENY. - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(headers_, false)); // Expect `denied` stats to be incremented. EXPECT_EQ(1U, config_->stats().denied_.value()); @@ -531,15 +809,15 @@ TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsNoIpMatchPortMatchDeny) { // If NONE of the configured upstream ip addresses or port match the metadata, the policy is NOT // enforced. TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsNoIpMatchNoPortMatchDeny) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"2.2.3.4:123"}); - // Setup policy config. const std::vector configs = {{"1.1.1.2"}, {124, 125}, {"1.2.3.5"}}; upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"2.2.3.4:123"}); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); // Expect `allowed` stats to be incremented. EXPECT_EQ(1U, config_->stats().allowed_.value()); @@ -549,16 +827,16 @@ TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsNoIpMatchNoPortMatchDeny) { // If NONE of the configured upstream ip addresses or port match the metadata, the policy is NOT // enforced. TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsAnyPolicyNoMatchDeny) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); - // Setup policy config. const std::vector configs = { {"1.1.1.2"}, {"1.2.3.4", 124, 125}, {"1.2.3.5"}}; upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); // Expect `allowed` stats to be incremented. EXPECT_EQ(1U, config_->stats().allowed_.value()); @@ -566,9 +844,6 @@ TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsAnyPolicyNoMatchDeny) { // Tests simple DENY permission policy with misconfigured port range. TEST_F(UpstreamIpPortMatcherTests, UpstreamPortBadRangeDeny) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:8080"}); - const std::vector configs = { {8080, 0}, }; @@ -576,19 +851,22 @@ TEST_F(UpstreamIpPortMatcherTests, UpstreamPortBadRangeDeny) { // Setup policy config. upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:8080"}); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers_, false)); EXPECT_EQ(0, config_->stats().denied_.value()); } // Verifies that if no IP or port is configured, EnvoyException is thrown. TEST_F(UpstreamIpPortMatcherTests, EmptyUpstreamConfigPolicyDeny) { - // Setup filter state with the upstream address. - upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); - // Setup policy config. const std::vector configs = {{}}; + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + EXPECT_THROW_WITH_MESSAGE( upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY), EnvoyException, "Invalid UpstreamIpPortMatcher configuration - missing `upstream_ip` " diff --git a/test/extensions/filters/network/rbac/BUILD b/test/extensions/filters/network/rbac/BUILD index 380281297340d..15f6c511dc9af 100644 --- a/test/extensions/filters/network/rbac/BUILD +++ b/test/extensions/filters/network/rbac/BUILD @@ -32,8 +32,10 @@ envoy_extension_cc_test( "//source/extensions/filters/network:well_known_names", "//source/extensions/filters/network/rbac:rbac_filter", "//test/mocks/network:network_mocks", + "//test/mocks/server:factory_context_mocks", "@envoy_api//envoy/config/rbac/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/rbac/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/matching/common_inputs/network/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/filters/network/rbac/config_test.cc b/test/extensions/filters/network/rbac/config_test.cc index 134392d492652..6dc2c96600d97 100644 --- a/test/extensions/filters/network/rbac/config_test.cc +++ b/test/extensions/filters/network/rbac/config_test.cc @@ -3,12 +3,14 @@ #include "envoy/extensions/filters/network/rbac/v3/rbac.pb.validate.h" #include "source/extensions/filters/network/rbac/config.h" +#include "source/extensions/filters/network/rbac/rbac_filter.h" #include "test/mocks/server/factory_context.h" #include "fmt/printf.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "xds/type/matcher/v3/matcher.pb.h" using testing::_; using testing::NiceMock; @@ -23,6 +25,9 @@ const std::string header = R"EOF( { "header": {"name": "key", "exact_match": "value"} } )EOF"; +const absl::string_view header_key = "key"; +const absl::string_view header_value = "value"; + } // namespace class RoleBasedAccessControlNetworkFilterConfigFactoryTest : public testing::Test { @@ -31,6 +36,10 @@ class RoleBasedAccessControlNetworkFilterConfigFactoryTest : public testing::Tes checkRule(fmt::sprintf(policy_json, header)); } + void validateMatcher(const std::string& matcher_yaml) { + checkMatcher(fmt::format(matcher_yaml, header_key, header_value)); + } + private: void checkRule(const std::string& policy_json) { envoy::config::rbac::v3::Policy policy_proto{}; @@ -48,6 +57,27 @@ class RoleBasedAccessControlNetworkFilterConfigFactoryTest : public testing::Tes (*config.mutable_shadow_rules()->mutable_policies())["foo"] = policy_proto; EXPECT_THROW(factory.createFilterFactoryFromProto(config, context), Envoy::EnvoyException); } + + void checkMatcher(const std::string& matcher_yaml) { + xds::type::matcher::v3::Matcher matcher_proto{}; + TestUtility::loadFromYaml(matcher_yaml, matcher_proto); + + envoy::extensions::filters::network::rbac::v3::RBAC config{}; + config.set_stat_prefix("test"); + *config.mutable_matcher() = matcher_proto; + + Stats::IsolatedStoreImpl store; + NiceMock context; + EXPECT_THROW(std::make_shared( + config, store, context, ProtobufMessage::getStrictValidationVisitor()), + Envoy::EnvoyException); + + config.clear_matcher(); + *config.mutable_shadow_matcher() = matcher_proto; + EXPECT_THROW(std::make_shared( + config, store, context, ProtobufMessage::getStrictValidationVisitor()), + Envoy::EnvoyException); + } }; TEST_F(RoleBasedAccessControlNetworkFilterConfigFactoryTest, ValidProto) { @@ -66,6 +96,27 @@ TEST_F(RoleBasedAccessControlNetworkFilterConfigFactoryTest, ValidProto) { cb(connection); } +TEST_F(RoleBasedAccessControlNetworkFilterConfigFactoryTest, ValidMatcherProto) { + envoy::config::rbac::v3::Action action; + action.set_name("foo"); + action.set_action(envoy::config::rbac::v3::RBAC::ALLOW); + + xds::type::matcher::v3::Matcher matcher; + auto matcher_action = matcher.mutable_on_no_match()->mutable_action(); + matcher_action->set_name("action"); + matcher_action->mutable_typed_config()->PackFrom(action); + envoy::extensions::filters::network::rbac::v3::RBAC config; + config.set_stat_prefix("stats"); + *config.mutable_matcher() = matcher; + + NiceMock context; + RoleBasedAccessControlNetworkFilterConfigFactory factory; + Network::FilterFactoryCb cb = factory.createFilterFactoryFromProto(config, context); + Network::MockConnection connection; + EXPECT_CALL(connection, addReadFilter(_)); + cb(connection); +} + TEST_F(RoleBasedAccessControlNetworkFilterConfigFactoryTest, EmptyProto) { RoleBasedAccessControlNetworkFilterConfigFactory factory; EXPECT_NE(nullptr, dynamic_cast( @@ -132,6 +183,42 @@ TEST_F(RoleBasedAccessControlNetworkFilterConfigFactoryTest, InvalidPrincipal) { )EOF"); } +TEST_F(RoleBasedAccessControlNetworkFilterConfigFactoryTest, InvalidMatcher) { + validateMatcher(R"EOF( +matcher_tree: + input: + name: source-ip + typed_config: + '@type': type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: {} + exact_match_map: + map: + "{}": + action: + name: action + typed_config: + '@type': type.googleapis.com/envoy.config.rbac.v3.Action + name: deny +)EOF"); + + validateMatcher(R"EOF( +matcher_tree: + input: + name: source-ip + typed_config: + '@type': type.googleapis.com/envoy.type.matcher.v3.HttpResponseHeaderMatchInput + header_name: {} + exact_match_map: + map: + "{}": + action: + name: action + typed_config: + '@type': type.googleapis.com/envoy.config.rbac.v3.Action + name: deny +)EOF"); +} + } // namespace RBACFilter } // namespace NetworkFilters } // namespace Extensions diff --git a/test/extensions/filters/network/rbac/filter_test.cc b/test/extensions/filters/network/rbac/filter_test.cc index 7d2bcf4dc3c42..56f29811728fc 100644 --- a/test/extensions/filters/network/rbac/filter_test.cc +++ b/test/extensions/filters/network/rbac/filter_test.cc @@ -2,13 +2,18 @@ #include "envoy/config/rbac/v3/rbac.pb.h" #include "envoy/extensions/filters/network/rbac/v3/rbac.pb.h" +#include "envoy/extensions/matching/common_inputs/network/v3/network_inputs.pb.h" +#include "source/common/network/address_impl.h" #include "source/common/network/utility.h" #include "source/extensions/filters/common/rbac/utility.h" #include "source/extensions/filters/network/rbac/rbac_filter.h" #include "source/extensions/filters/network/well_known_names.h" #include "test/mocks/network/mocks.h" +#include "test/mocks/server/factory_context.h" + +#include "xds/type/matcher/v3/matcher.pb.h" using testing::NiceMock; using testing::Return; @@ -21,8 +26,8 @@ namespace RBACFilter { class RoleBasedAccessControlNetworkFilterTest : public testing::Test { public: - RoleBasedAccessControlFilterConfigSharedPtr - setupConfig(bool with_policy = true, bool continuous = false, + void + setupPolicy(bool with_policy = true, bool continuous = false, envoy::config::rbac::v3::RBAC::Action action = envoy::config::rbac::v3::RBAC::ALLOW) { envoy::extensions::filters::network::rbac::v3::RBAC config; @@ -52,11 +57,119 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { config.set_enforcement_type(envoy::extensions::filters::network::rbac::v3::RBAC::CONTINUOUS); } - return std::make_shared( - config, store_, ProtobufMessage::getStrictValidationVisitor()); + config_ = std::make_shared( + config, store_, context_, ProtobufMessage::getStrictValidationVisitor()); + + filter_ = std::make_unique(config_); + filter_->initializeReadFilterCallbacks(callbacks_); } - RoleBasedAccessControlNetworkFilterTest() : config_(setupConfig()) { + void setupMatcher(bool with_matcher = true, bool continuous = false, std::string action = "ALLOW", + std::string on_no_match_action = "DENY") { + envoy::extensions::filters::network::rbac::v3::RBAC config; + config.set_stat_prefix("tcp."); + config.set_shadow_rules_stat_prefix("prefix_"); + + if (with_matcher) { + const std::string matcher_yaml = R"EOF( +matcher_list: + matchers: + - predicate: + or_matcher: + predicate: + - single_predicate: + input: + name: envoy.matching.inputs.server_name + typed_config: + "@type": type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.ServerNameInput + value_match: + safe_regex: + google_re2: {} + regex: .*cncf.io + - single_predicate: + input: + name: envoy.matching.inputs.destination_port + typed_config: + "@type": type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DestinationPortInput + value_match: + exact: "123" + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: foo + action: {} +on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: none + action: {} +)EOF"; + const std::string shadow_matcher_yaml = R"EOF( +matcher_list: + matchers: + - predicate: + or_matcher: + predicate: + - single_predicate: + input: + name: envoy.matching.inputs.server_name + typed_config: + "@type": type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.ServerNameInput + value_match: + exact: xyz.cncf.io + - single_predicate: + input: + name: envoy.matching.inputs.destination_port + typed_config: + "@type": type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DestinationPortInput + value_match: + exact: "456" + on_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: bar + action: {} +on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: none + action: {} +)EOF"; + + xds::type::matcher::v3::Matcher matcher; + // Escape the first {} for safe_regex. + TestUtility::loadFromYaml(fmt::format(matcher_yaml, "{}", action, on_no_match_action), + matcher); + *config.mutable_matcher() = matcher; + + xds::type::matcher::v3::Matcher shadow_matcher; + TestUtility::loadFromYaml(fmt::format(shadow_matcher_yaml, action, on_no_match_action), + shadow_matcher); + *config.mutable_shadow_matcher() = shadow_matcher; + } + + if (continuous) { + config.set_enforcement_type(envoy::extensions::filters::network::rbac::v3::RBAC::CONTINUOUS); + } + + config_ = std::make_shared( + config, store_, context_, ProtobufMessage::getStrictValidationVisitor()); + + filter_ = std::make_unique(config_); + filter_->initializeReadFilterCallbacks(callbacks_); + } + + RoleBasedAccessControlNetworkFilterTest() + : provider_(std::make_shared(80), + std::make_shared(80)) { EXPECT_CALL(callbacks_, connection()).WillRepeatedly(ReturnRef(callbacks_.connection_)); EXPECT_CALL(callbacks_.connection_, streamInfo()).WillRepeatedly(ReturnRef(stream_info_)); @@ -67,12 +180,18 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { void setDestinationPort(uint16_t port) { address_ = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", port, false); stream_info_.downstream_connection_info_provider_->setLocalAddress(address_); + + provider_.setLocalAddress(address_); + ON_CALL(callbacks_.connection_, connectionInfoProvider()).WillByDefault(ReturnRef(provider_)); } void setRequestedServerName(std::string server_name) { requested_server_name_ = server_name; ON_CALL(callbacks_.connection_, requestedServerName()) .WillByDefault(Return(requested_server_name_)); + + provider_.setRequestedServerName(requested_server_name_); + ON_CALL(callbacks_.connection_, connectionInfoProvider()).WillByDefault(ReturnRef(provider_)); } void checkAccessLogMetadata(bool expected) { @@ -105,15 +224,19 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { NiceMock callbacks_; NiceMock stream_info_; Stats::IsolatedStoreImpl store_; + NiceMock context_; Buffer::OwnedImpl data_; RoleBasedAccessControlFilterConfigSharedPtr config_; std::unique_ptr filter_; Network::Address::InstanceConstSharedPtr address_; + Network::ConnectionInfoSetterImpl provider_; std::string requested_server_name_; }; TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithOneTimeEnforcement) { + setupPolicy(); + setDestinationPort(123); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); @@ -132,9 +255,8 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithOneTimeEnforcement) { } TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithContinuousEnforcement) { - config_ = setupConfig(true, true /* continuous enforcement */); - filter_ = std::make_unique(config_); - filter_->initializeReadFilterCallbacks(callbacks_); + setupPolicy(true, true /* continuous enforcement */); + setDestinationPort(123); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); @@ -153,6 +275,8 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithContinuousEnforcement } TEST_F(RoleBasedAccessControlNetworkFilterTest, RequestedServerName) { + setupPolicy(); + setDestinationPort(999); setRequestedServerName("www.cncf.io"); @@ -172,9 +296,8 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, RequestedServerName) { } TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithNoPolicy) { - config_ = setupConfig(false /* with_policy */); - filter_ = std::make_unique(config_); - filter_->initializeReadFilterCallbacks(callbacks_); + setupPolicy(false /* with_policy */); + setDestinationPort(0); // Allow access and no metric change when there is no policy. @@ -191,6 +314,113 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithNoPolicy) { } TEST_F(RoleBasedAccessControlNetworkFilterTest, Denied) { + setupPolicy(); + + setDestinationPort(456); + setMetadata(); + + EXPECT_CALL(callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush)).Times(2); + + // Call onData() twice, should only increase stats once. + EXPECT_EQ(Network::FilterStatus::StopIteration, filter_->onData(data_, false)); + EXPECT_EQ(Network::FilterStatus::StopIteration, filter_->onData(data_, false)); + EXPECT_EQ(0U, config_->stats().allowed_.value()); + EXPECT_EQ(1U, config_->stats().denied_.value()); + EXPECT_EQ(1U, config_->stats().shadow_allowed_.value()); + EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + + auto filter_meta = + stream_info_.dynamicMetadata().filter_metadata().at(NetworkFilterNames::get().Rbac); + EXPECT_EQ("bar", filter_meta.fields().at("prefix_shadow_effective_policy_id").string_value()); + EXPECT_EQ("allowed", filter_meta.fields().at("prefix_shadow_engine_result").string_value()); +} + +TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherAllowedWithOneTimeEnforcement) { + setupMatcher(); + + setDestinationPort(123); + + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); + + // Call onData() twice, should only increase stats once. + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().denied_.value()); + EXPECT_EQ(0U, config_->stats().shadow_allowed_.value()); + EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); +} + +TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherAllowedWithContinuousEnforcement) { + setupMatcher(true, true /* continuous enforcement */); + + setDestinationPort(123); + + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); + + // Call onData() twice, should increase stats twice. + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(2U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().denied_.value()); + EXPECT_EQ(0U, config_->stats().shadow_allowed_.value()); + EXPECT_EQ(2U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); +} + +TEST_F(RoleBasedAccessControlNetworkFilterTest, RequestedServerNameMatcher) { + setupMatcher(); + + setDestinationPort(999); + setRequestedServerName("www.cncf.io"); + + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); + + // Call onData() twice, should only increase stats once. + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().denied_.value()); + EXPECT_EQ(0U, config_->stats().shadow_allowed_.value()); + EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); +} + +TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithNoMatcher) { + setupMatcher(false); + + setDestinationPort(0); + + // Allow access and no metric change when there is no policy. + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(0U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().denied_.value()); + EXPECT_EQ(0U, config_->stats().shadow_allowed_.value()); + EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); +} + +TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherDenied) { + setupMatcher(); + setDestinationPort(456); setMetadata(); @@ -216,9 +446,7 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, Denied) { // Log Tests TEST_F(RoleBasedAccessControlNetworkFilterTest, ShouldLog) { - config_ = setupConfig(true, false, envoy::config::rbac::v3::RBAC::LOG); - filter_ = std::make_unique(config_); - filter_->initializeReadFilterCallbacks(callbacks_); + setupPolicy(true, false, envoy::config::rbac::v3::RBAC::LOG); setDestinationPort(123); setMetadata(); @@ -235,9 +463,7 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, ShouldLog) { } TEST_F(RoleBasedAccessControlNetworkFilterTest, ShouldNotLog) { - config_ = setupConfig(true, false, envoy::config::rbac::v3::RBAC::LOG); - filter_ = std::make_unique(config_); - filter_->initializeReadFilterCallbacks(callbacks_); + setupPolicy(true, false, envoy::config::rbac::v3::RBAC::LOG); setDestinationPort(456); setMetadata(); @@ -254,6 +480,56 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, ShouldNotLog) { } TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowNoChangeLog) { + setupPolicy(); + + setDestinationPort(123); + setMetadata(); + + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + + // Check that Allow action does not set access log metadata + EXPECT_EQ(stream_info_.dynamicMetadata().filter_metadata().end(), + stream_info_.dynamicMetadata().filter_metadata().find( + Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().CommonNamespace)); +} + +TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherShouldLog) { + setupMatcher(true, false, "LOG", "ALLOW"); + + setDestinationPort(123); + setMetadata(); + + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + + checkAccessLogMetadata(true); +} + +TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherShouldNotLog) { + setupMatcher(true, false, "LOG", "ALLOW"); + + setDestinationPort(456); + setMetadata(); + + EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false)); + EXPECT_EQ(1U, config_->stats().allowed_.value()); + EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); + EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + + checkAccessLogMetadata(false); +} + +TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherAllowNoChangeLog) { + setupMatcher(); + setDestinationPort(123); setMetadata(); diff --git a/test/extensions/filters/network/rbac/integration_test.cc b/test/extensions/filters/network/rbac/integration_test.cc index 7552f8e00c255..388dc28507820 100644 --- a/test/extensions/filters/network/rbac/integration_test.cc +++ b/test/extensions/filters/network/rbac/integration_test.cc @@ -162,6 +162,100 @@ name: rbac testing::HasSubstr("rbac_access_denied_matched_policy[deny_all]")); } +TEST_P(RoleBasedAccessControlNetworkFilterIntegrationTest, MatcherAllowed) { + initializeFilter(R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC + stat_prefix: tcp. + matcher: + on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: allow_all + action: ALLOW + shadow_matcher: + on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: deny_all + action: DENY +)EOF"); + IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("listener_0")); + ASSERT_TRUE(tcp_client->write("hello")); + ASSERT_TRUE(tcp_client->connected()); + tcp_client->close(); + + test_server_->waitForCounterGe("tcp.rbac.allowed", 1); + EXPECT_EQ(0U, test_server_->counter("tcp.rbac.denied")->value()); + EXPECT_EQ(0U, test_server_->counter("tcp.rbac.shadow_allowed")->value()); + test_server_->waitForCounterGe("tcp.rbac.shadow_denied", 1); +} + +TEST_P(RoleBasedAccessControlNetworkFilterIntegrationTest, MatcherDenied) { + initializeFilter(R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC + stat_prefix: tcp. + matcher: + on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: deny_all + action: DENY + shadow_matcher: + on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: allow_all + action: ALLOW +)EOF"); + IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("listener_0")); + ASSERT_TRUE(tcp_client->write("hello", false, false)); + tcp_client->waitForDisconnect(); + + EXPECT_EQ(0U, test_server_->counter("tcp.rbac.allowed")->value()); + EXPECT_EQ(1U, test_server_->counter("tcp.rbac.denied")->value()); + EXPECT_EQ(1U, test_server_->counter("tcp.rbac.shadow_allowed")->value()); + EXPECT_EQ(0U, test_server_->counter("tcp.rbac.shadow_denied")->value()); +} + +TEST_P(RoleBasedAccessControlNetworkFilterIntegrationTest, MatcherDeniedWithDenyAction) { + useListenerAccessLog("%CONNECTION_TERMINATION_DETAILS%"); + initializeFilter(R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC + stat_prefix: tcp. + matcher: + on_no_match: + action: + name: action + typed_config: + "@type": type.googleapis.com/envoy.config.rbac.v3.Action + name: "deny all" + action: DENY +)EOF"); + IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("listener_0")); + ASSERT_TRUE(tcp_client->write("hello", false, false)); + tcp_client->waitForDisconnect(); + + EXPECT_EQ(0U, test_server_->counter("tcp.rbac.allowed")->value()); + EXPECT_EQ(1U, test_server_->counter("tcp.rbac.denied")->value()); + // Note the whitespace in the policy id is replaced by '_'. + EXPECT_THAT(waitForAccessLog(listener_access_log_name_), + testing::HasSubstr("rbac_access_denied_matched_policy[deny_all]")); +} + } // namespace RBAC } // namespace NetworkFilters } // namespace Extensions