Enable metadata for Network::RBAC

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 <config_http_filters_rbac>`.
 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;
 }

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,

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()) {

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) {

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<Filters::Common::RBAC::RoleBasedAccessControlEngineImpl> engine_;
   const absl::optional<Filters::Common::RBAC::RoleBasedAccessControlEngineImpl> shadow_engine_;
+  const envoy::config::filter::network::rbac::v2::RBAC::EnforcementType enforcement_type_;
 };
 
 typedef std::shared_ptr<RoleBasedAccessControlFilterConfig>

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:

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<RoleBasedAccessControlFilterConfig>(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<RoleBasedAccessControlFilter>(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");