diff --git a/api/envoy/config/filter/network/rbac/v2/rbac.proto b/api/envoy/config/filter/network/rbac/v2/rbac.proto index c16ac6838e16f..6f4427816b28d 100644 --- a/api/envoy/config/filter/network/rbac/v2/rbac.proto +++ b/api/envoy/config/filter/network/rbac/v2/rbac.proto @@ -13,7 +13,7 @@ import "gogoproto/gogo.proto"; // RBAC network filter config. // -// Header and Metadata should not be used in rules/shadow_rules in RBAC network filter as +// Header should not be used in rules/shadow_rules in RBAC network filter as // this information is only available in :ref:`RBAC http filter `. message RBAC { // Specify the RBAC rules to be applied globally. @@ -27,4 +27,22 @@ message RBAC { // The prefix to use when emitting statistics. string stat_prefix = 3 [(validate.rules).string.min_bytes = 1]; + + enum EnforcementType { + // Apply RBAC policies when the first byte of data arrives on the connection. + ONE_TIME_ON_FIRST_BYTE = 0; + + // Continuously apply RBAC policies as data arrives. Use this mode when + // using RBAC with message oriented protocols such as Mongo, MySQL, Kafka, + // etc. when the protocol decoders emit dynamic metadata such as the + // resources being accessed and the operations on the resources. + CONTINUOUS = 1; + }; + + // RBAC enforcement strategy. By default RBAC will be enforced only once + // when the first byte of data arrives from the downstream. When used in + // conjunction with filters that emit dynamic metadata after decoding + // every payload (e.g., Mongo, MySQL, Kafka) set the enforcement type to + // CONTINUOUS to enforce RBAC policies on every message boundary. + EnforcementType enforcement_type = 4; } diff --git a/api/envoy/config/rbac/v2alpha/rbac.proto b/api/envoy/config/rbac/v2alpha/rbac.proto index 9c1b04c24d15e..0751b67177bb7 100644 --- a/api/envoy/config/rbac/v2alpha/rbac.proto +++ b/api/envoy/config/rbac/v2alpha/rbac.proto @@ -118,8 +118,7 @@ message Permission { // A port number that describes the destination port connecting to. uint32 destination_port = 6 [(validate.rules).uint32.lte = 65535]; - // Metadata that describes additional information about the action. Only available for HTTP - // request. + // Metadata that describes additional information about the action. envoy.type.matcher.MetadataMatcher metadata = 7; // Negates matching the provided permission. For instance, if the value of `not_rule` would @@ -191,8 +190,7 @@ message Principal { // available for HTTP request. envoy.api.v2.route.HeaderMatcher header = 6; - // Metadata that describes additional information about the principal. Only available for HTTP - // request. + // Metadata that describes additional information about the principal. envoy.type.matcher.MetadataMatcher metadata = 7; // Negates matching the provided principal. For instance, if the value of `not_id` would match, diff --git a/source/extensions/filters/network/rbac/config.cc b/source/extensions/filters/network/rbac/config.cc index 35396f6907a1e..7e00874fd0d08 100644 --- a/source/extensions/filters/network/rbac/config.cc +++ b/source/extensions/filters/network/rbac/config.cc @@ -11,15 +11,15 @@ namespace Extensions { namespace NetworkFilters { namespace RBACFilter { -static void validateFail(const std::string& header, const std::string& metadata) { - throw EnvoyException(fmt::format("Found header({}) or metadata({}) rule," +static void validateFail(const std::string& header) { + throw EnvoyException(fmt::format("Found header({}) rule," "not supported by RBAC network filter", - header, metadata)); + header)); } static void validatePermission(const envoy::config::rbac::v2alpha::Permission& permission) { - if (permission.has_header() || permission.has_metadata()) { - validateFail(permission.header().DebugString(), permission.metadata().DebugString()); + if (permission.has_header()) { + validateFail(permission.header().DebugString()); } if (permission.has_and_rules()) { for (const auto& r : permission.and_rules().rules()) { @@ -37,8 +37,8 @@ static void validatePermission(const envoy::config::rbac::v2alpha::Permission& p } static void validatePrincipal(const envoy::config::rbac::v2alpha::Principal& principal) { - if (principal.has_header() || principal.has_metadata()) { - validateFail(principal.header().DebugString(), principal.metadata().DebugString()); + if (principal.has_header()) { + validateFail(principal.header().DebugString()); } if (principal.has_and_ids()) { for (const auto& r : principal.and_ids().ids()) { diff --git a/source/extensions/filters/network/rbac/rbac_filter.cc b/source/extensions/filters/network/rbac/rbac_filter.cc index d62c87388f883..1267e9cac3872 100644 --- a/source/extensions/filters/network/rbac/rbac_filter.cc +++ b/source/extensions/filters/network/rbac/rbac_filter.cc @@ -14,7 +14,8 @@ RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig( const envoy::config::filter::network::rbac::v2::RBAC& proto_config, Stats::Scope& scope) : stats_(Filters::Common::RBAC::generateStats(proto_config.stat_prefix(), scope)), engine_(Filters::Common::RBAC::createEngine(proto_config)), - shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config)) {} + shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config)), + enforcement_type_(proto_config.enforcement_type()) {} Network::FilterStatus RoleBasedAccessControlFilter::onData(Buffer::Instance&, bool) { ENVOY_LOG( @@ -31,15 +32,20 @@ Network::FilterStatus RoleBasedAccessControlFilter::onData(Buffer::Instance&, bo : "none", callbacks_->connection().streamInfo().dynamicMetadata().DebugString()); - if (shadow_engine_result_ == Unknown) { - // TODO(quanlin): Pass the shadow engine results to other filters. - // Only check the engine and increase stats for the first time call to onData(), any following - // calls to onData() could just use the cached result and no need to increase the stats anymore. + // When the enforcement type is continuous always do the RBAC checks. If it is a one time check, + // run the check once and skip it for subsequent onData calls. + if (config_->enforcementType() == envoy::config::filter::network::rbac::v2::RBAC::CONTINUOUS) { shadow_engine_result_ = checkEngine(Filters::Common::RBAC::EnforcementMode::Shadow); - } - - if (engine_result_ == Unknown) { engine_result_ = checkEngine(Filters::Common::RBAC::EnforcementMode::Enforced); + } else { + if (shadow_engine_result_ == Unknown) { + // TODO(quanlin): Pass the shadow engine results to other filters. + shadow_engine_result_ = checkEngine(Filters::Common::RBAC::EnforcementMode::Shadow); + } + + if (engine_result_ == Unknown) { + engine_result_ = checkEngine(Filters::Common::RBAC::EnforcementMode::Enforced); + } } if (engine_result_ == Allow) { diff --git a/source/extensions/filters/network/rbac/rbac_filter.h b/source/extensions/filters/network/rbac/rbac_filter.h index 7c33149affa38..bbb09708b7877 100644 --- a/source/extensions/filters/network/rbac/rbac_filter.h +++ b/source/extensions/filters/network/rbac/rbac_filter.h @@ -31,11 +31,16 @@ class RoleBasedAccessControlFilterConfig { return mode == Filters::Common::RBAC::EnforcementMode::Enforced ? engine_ : shadow_engine_; } + envoy::config::filter::network::rbac::v2::RBAC::EnforcementType enforcementType() const { + return enforcement_type_; + } + private: Filters::Common::RBAC::RoleBasedAccessControlFilterStats stats_; const absl::optional engine_; const absl::optional shadow_engine_; + const envoy::config::filter::network::rbac::v2::RBAC::EnforcementType enforcement_type_; }; typedef std::shared_ptr diff --git a/test/extensions/filters/network/rbac/config_test.cc b/test/extensions/filters/network/rbac/config_test.cc index e7ebabd2adf5d..3479f43d032a4 100644 --- a/test/extensions/filters/network/rbac/config_test.cc +++ b/test/extensions/filters/network/rbac/config_test.cc @@ -21,20 +21,12 @@ const std::string header = R"EOF( { "header": {"name": "key", "exact_match": "value"} } )EOF"; -const std::string metadata = R"EOF( -{ - "metadata": { - "filter": "t", "path": [ { "key": "a" } ], "value": { "string_match": { "exact": "x" } } - } -} -)EOF"; } // namespace class RoleBasedAccessControlNetworkFilterConfigFactoryTest : public testing::Test { public: void validateRule(const std::string& policy_json) { checkRule(fmt::sprintf(policy_json, header)); - checkRule(fmt::sprintf(policy_json, metadata)); } private: diff --git a/test/extensions/filters/network/rbac/filter_test.cc b/test/extensions/filters/network/rbac/filter_test.cc index bc3d88526b139..feb084d5714c6 100644 --- a/test/extensions/filters/network/rbac/filter_test.cc +++ b/test/extensions/filters/network/rbac/filter_test.cc @@ -19,7 +19,8 @@ namespace RBACFilter { class RoleBasedAccessControlNetworkFilterTest : public testing::Test { public: - RoleBasedAccessControlFilterConfigSharedPtr setupConfig(bool with_policy = true) { + RoleBasedAccessControlFilterConfigSharedPtr setupConfig(bool with_policy = true, + bool continuous = false) { envoy::config::filter::network::rbac::v2::RBAC config; config.set_stat_prefix("tcp."); @@ -41,6 +42,10 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { (*config.mutable_shadow_rules()->mutable_policies())["bar"] = shadow_policy; } + if (continuous) { + config.set_enforcement_type(envoy::config::filter::network::rbac::v2::RBAC::CONTINUOUS); + } + return std::make_shared(config, store_); } @@ -83,7 +88,7 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { std::string requested_server_name_; }; -TEST_F(RoleBasedAccessControlNetworkFilterTest, Allowed) { +TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithOneTimeEnforcement) { setDestinationPort(123); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); @@ -97,6 +102,23 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, Allowed) { EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); } +TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithContinuousEnforcement) { + config_ = setupConfig(true, true /* continuous enforcement */); + filter_ = std::make_unique(config_); + filter_->initializeReadFilterCallbacks(callbacks_); + 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()); +} + TEST_F(RoleBasedAccessControlNetworkFilterTest, RequestedServerName) { setDestinationPort(999); setRequestedServerName("www.cncf.io");