rbac: add support for matching on route metadata

Signed-off-by: Rohit Agrawal <[email protected]>

api/envoy/config/rbac/v3/rbac.proto

@@ -28,6 +28,14 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 
 // [#protodoc-title: Role Based Access Control (RBAC)]
 
+enum MetadataSource {
+  // Query :ref:`dynamic metadata <well_known_dynamic_metadata>`
+  DYNAMIC = 0;
+
+  // Query :ref:`route entry metadata <envoy_v3_api_field_config.route.v3.Route.metadata>`
+  ROUTE_ENTRY = 1;
+}
+
 // Role Based Access Control (RBAC) provides service-level and method-level access control for a
 // service. Requests are allowed or denied based on the ``action`` and whether a matching policy is
 // found. For instance, if the action is ALLOW and a matching policy is found the request should be
@@ -193,8 +201,26 @@ message Policy {
       [(udpa.annotations.field_migrate).oneof_promotion = "expression_specifier"];
 }
 
+// SourcedMetadata enables matching against metadata from different sources in the request processing
+// pipeline. It extends the base MetadataMatcher functionality by allowing specification of where the
+// metadata should be sourced from, rather than only matching against dynamic metadata.
+//
+// The matcher can be configured to look up metadata from:
+// * Dynamic metadata: Runtime metadata added by filters during request processing
+// * Route metadata: Static metadata configured on the route entry
+message SourcedMetadata {
+  // Metadata matcher configuration that defines what metadata to match against. This includes the filter name,
+  // metadata key path, and expected value.
+  type.matcher.v3.MetadataMatcher metadata_matcher = 1;
+
+  // Specifies which metadata source should be used for matching. If not set,
+  // defaults to DYNAMIC (dynamic metadata). Set to ROUTE_ENTRY to match against
+  // static metadata configured on the route entry.
+  MetadataSource metadata_source = 2 [(validate.rules).enum = {defined_only: true}];
+}
+
 // Permission defines an action (or actions) that a principal can take.
-// [#next-free-field: 14]
+// [#next-free-field: 15]
 message Permission {
   option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Permission";
 
@@ -274,12 +300,16 @@ message Permission {
     // URI template path matching.
     // [#extension-category: envoy.path.match]
     core.v3.TypedExtensionConfig uri_template = 13;
+
+    // Matches against metadata from either dynamic state or route configuration. Preferred over the
+    // ``metadata`` field as it provides more flexibility in metadata source selection.
+    SourcedMetadata sourced_metadata = 14;
   }
 }
 
 // Principal defines an identity or a group of identities for a downstream
 // subject.
-// [#next-free-field: 13]
+// [#next-free-field: 14]
 message Principal {
   option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Principal";
 
@@ -366,6 +396,10 @@ message Principal {
     // ``not_id`` would match, this principal would not match. Conversely, if the
     // value of ``not_id`` would not match, this principal would match.
     Principal not_id = 8;
+
+    // Matches against metadata from either dynamic state or route configuration. Preferred over the
+    // ``metadata`` field as it provides more flexibility in metadata source selection.
+    SourcedMetadata sourced_metadata = 13;
   }
 }
 

changelogs/current.yaml

@@ -139,5 +139,9 @@ new_features:
   change: |
     Added support for dynamic cluster selection in UDP proxy. The cluster can be set by one of the session filters
     by setting a per-session state object under the key ``envoy.udp_proxy.cluster``.
+- area: rbac
+  change: |
+    added :ref:`sourced_metadata <envoy_v3_api_field_config.rbac.v3.Permission.sourced_metadata>` which allows
+    specifying an optional source for the metadata to be matched in addition to the metadata matcher.
 
 deprecated:

source/extensions/filters/common/rbac/matchers.cc

@@ -33,7 +33,12 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Permission&
     return std::make_shared<const AlwaysMatcher>();
   case envoy::config::rbac::v3::Permission::RuleCase::kMetadata:
     return std::make_shared<const MetadataMatcher>(
-        Matchers::MetadataMatcher(permission.metadata(), context));
+        Matchers::MetadataMatcher(permission.metadata(), context),
+        envoy::config::rbac::v3::MetadataSource::DYNAMIC);
+  case envoy::config::rbac::v3::Permission::RuleCase::kSourcedMetadata:
+    return std::make_shared<const MetadataMatcher>(
+        Matchers::MetadataMatcher(permission.sourced_metadata().metadata_matcher(), context),
+        permission.sourced_metadata().metadata_source());
   case envoy::config::rbac::v3::Permission::RuleCase::kNotRule:
     return std::make_shared<const NotMatcher>(permission.not_rule(), validation_visitor, context);
   case envoy::config::rbac::v3::Permission::RuleCase::kRequestedServerName:
@@ -83,7 +88,12 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Principal&
     return std::make_shared<const AlwaysMatcher>();
   case envoy::config::rbac::v3::Principal::IdentifierCase::kMetadata:
     return std::make_shared<const MetadataMatcher>(
-        Matchers::MetadataMatcher(principal.metadata(), context));
+        Matchers::MetadataMatcher(principal.metadata(), context),
+        envoy::config::rbac::v3::MetadataSource::DYNAMIC);
+  case envoy::config::rbac::v3::Principal::IdentifierCase::kSourcedMetadata:
+    return std::make_shared<const MetadataMatcher>(
+        Matchers::MetadataMatcher(principal.sourced_metadata().metadata_matcher(), context),
+        principal.sourced_metadata().metadata_source());
   case envoy::config::rbac::v3::Principal::IdentifierCase::kNotId:
     return std::make_shared<const NotMatcher>(principal.not_id(), context);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kUrlPath:
@@ -248,7 +258,11 @@ bool AuthenticatedMatcher::matches(const Network::Connection& connection,
 
 bool MetadataMatcher::matches(const Network::Connection&, const Envoy::Http::RequestHeaderMap&,
                               const StreamInfo::StreamInfo& info) const {
-  return matcher_.match(info.dynamicMetadata());
+  if (metadata_source_ == envoy::config::rbac::v3::MetadataSource::DYNAMIC) {
+    return matcher_.match(info.dynamicMetadata());
+  }
+
+  return matcher_.match(info.route()->metadata());
 }
 
 bool FilterStateMatcher::matches(const Network::Connection&, const Envoy::Http::RequestHeaderMap&,

source/extensions/filters/common/rbac/matchers.h

@@ -248,13 +248,16 @@ class PolicyMatcher : public Matcher, NonCopyable {
 
 class MetadataMatcher : public Matcher {
 public:
-  MetadataMatcher(const Envoy::Matchers::MetadataMatcher& matcher) : matcher_(matcher) {}
+  MetadataMatcher(const Envoy::Matchers::MetadataMatcher& matcher,
+                  const envoy::config::rbac::v3::MetadataSource& metadata_source)
+      : matcher_(matcher), metadata_source_(metadata_source) {}
 
   bool matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers,
                const StreamInfo::StreamInfo& info) const override;
 
 private:
   const Envoy::Matchers::MetadataMatcher matcher_;
+  const envoy::config::rbac::v3::MetadataSource metadata_source_;
 };
 
 class FilterStateMatcher : public Matcher {

test/extensions/filters/common/rbac/matchers_test.cc

@@ -430,11 +430,13 @@ TEST(MetadataMatcher, MetadataMatcher) {
   matcher.add_path()->set_key("label");
 
   matcher.mutable_value()->mutable_string_match()->set_exact("test");
-  checkMatcher(MetadataMatcher(Matchers::MetadataMatcher(matcher, context)), false, conn, header,
-               info);
+  checkMatcher(MetadataMatcher(Matchers::MetadataMatcher(matcher, context),
+                               envoy::config::rbac::v3::MetadataSource::DYNAMIC),
+               false, conn, header, info);
   matcher.mutable_value()->mutable_string_match()->set_exact("prod");
-  checkMatcher(MetadataMatcher(Matchers::MetadataMatcher(matcher, context)), true, conn, header,
-               info);
+  checkMatcher(MetadataMatcher(Matchers::MetadataMatcher(matcher, context),
+                               envoy::config::rbac::v3::MetadataSource::DYNAMIC),
+               true, conn, header, info);
 }
 
 TEST(PolicyMatcher, PolicyMatcher) {

test/extensions/filters/http/rbac/rbac_filter_integration_test.cc

@@ -41,6 +41,29 @@ name: rbac
           - any: true
 )EOF";
 
+const std::string RBAC_CONFIG_WITH_SOURCED_METADATA = R"EOF(
+name: rbac
+typed_config:
+  "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC
+  rules:
+    action: DENY
+    policies:
+      "deny policy":
+        permissions:
+          - sourced_metadata:
+              metadata_matcher:
+                filter: "envoy.filters.http.lua"
+                path:
+                  - key: "hello.world"
+                  - key: "foo"
+                value:
+                  string_match:
+                    exact: "baz"
+              metadata_source: "ROUTE_ENTRY"
+        principals:
+          - any: true
+)EOF";
+
 const std::string RBAC_CONFIG_WITH_PREFIX_MATCH = R"EOF(
 name: rbac
 typed_config:
@@ -562,6 +585,91 @@ TEST_P(RBACIntegrationTest, DeniedWithDenyAction) {
               testing::HasSubstr("rbac_access_denied_matched_policy[deny_policy]"));
 }
 
+TEST_P(RBACIntegrationTest, RouteMetadataMatcherAllow) {
+  config_helper_.prependFilter(RBAC_CONFIG_WITH_SOURCED_METADATA);
+  // Set route metadata
+  config_helper_.addConfigModifier(
+      [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+             hcm) {
+        const std::string key = "envoy.filters.http.lua";
+        const std::string yaml =
+            R"EOF(
+            foo.bar:
+              baz: bat
+          )EOF";
+
+        ProtobufWkt::Struct value;
+        TestUtility::loadFromYaml(yaml, value);
+        auto default_route =
+            hcm.mutable_route_config()->mutable_virtual_hosts(0)->mutable_routes(0);
+        default_route->mutable_metadata()->mutable_filter_metadata()->insert(
+            Protobuf::MapPair<std::string, ProtobufWkt::Struct>(key, value));
+      });
+  initialize();
+
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+
+  auto response = codec_client_->makeRequestWithBody(
+      Http::TestRequestHeaderMapImpl{
+          {":method", "POST"},
+          {":path", "/foo/../bar"},
+          {":scheme", "http"},
+          {":authority", "sni.lyft.com"},
+          {"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, RouteMetadataMatcherDeny) {
+  useAccessLog("%RESPONSE_CODE_DETAILS%");
+  config_helper_.prependFilter(RBAC_CONFIG_WITH_SOURCED_METADATA);
+  // Set route metadata
+  config_helper_.addConfigModifier(
+      [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+             hcm) {
+        const std::string key = "envoy.filters.http.lua";
+        const std::string yaml =
+            R"EOF(
+            foo.bar:
+              baz: bat
+            hello.world:
+              foo: baz
+          )EOF";
+
+        ProtobufWkt::Struct value;
+        TestUtility::loadFromYaml(yaml, value);
+        auto default_route =
+            hcm.mutable_route_config()->mutable_virtual_hosts(0)->mutable_routes(0);
+        default_route->mutable_metadata()->mutable_filter_metadata()->insert(
+            Protobuf::MapPair<std::string, ProtobufWkt::Struct>(key, value));
+      });
+  initialize();
+
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+
+  auto response = codec_client_->makeRequestWithBody(
+      Http::TestRequestHeaderMapImpl{
+          {":method", "GET"},
+          {":path", "/"},
+          {":scheme", "http"},
+          {":authority", "sni.lyft.com"},
+          {"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, DeniedWithPrefixRule) {
   config_helper_.addConfigModifier(
       [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&