Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rbac: add support for matching on route metadata #36957

Merged
merged 21 commits into from
Nov 9, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 51 additions & 6 deletions api/envoy/config/rbac/v3/rbac.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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 metadata <envoy_v3_api_field_config.route.v3.Route.metadata>`
ROUTE = 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
Expand Down Expand Up @@ -193,8 +201,27 @@ 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
[(validate.rules).message = {required: true}];

// Specifies which metadata source should be used for matching. If not set,
// defaults to DYNAMIC (dynamic metadata). Set to ROUTE 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";

Expand Down Expand Up @@ -237,8 +264,13 @@ message Permission {
// A port number range that describes a range of destination ports connecting to.
type.v3.Int32Range destination_port_range = 11;

// Metadata that describes additional information about the action.
type.matcher.v3.MetadataMatcher metadata = 7;
// Metadata that describes additional information about the action. This field is deprecated; please use
// :ref:`sourced_metadata<envoy_v3_api_field_config.rbac.v3.Permission.sourced_metadata>` instead.
type.matcher.v3.MetadataMatcher metadata = 7 [
deprecated = true,
(envoy.annotations.deprecated_at_minor_version) = "3.0",
(envoy.annotations.disallowed_by_default) = true
agrawroh marked this conversation as resolved.
Show resolved Hide resolved
];

// Negates matching the provided permission. For instance, if the value of
// ``not_rule`` would match, this permission would not match. Conversely, if
Expand Down Expand Up @@ -274,12 +306,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;
agrawroh marked this conversation as resolved.
Show resolved Hide resolved
}
}

// 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";

Expand Down Expand Up @@ -356,8 +392,13 @@ message Principal {
// A URL path on the incoming HTTP request. Only available for HTTP.
type.matcher.v3.PathMatcher url_path = 9;

// Metadata that describes additional information about the principal.
type.matcher.v3.MetadataMatcher metadata = 7;
// Metadata that describes additional information about the principal. This field is deprecated; please use
// :ref:`sourced_metadata<envoy_v3_api_field_config.rbac.v3.Principal.sourced_metadata>` instead.
type.matcher.v3.MetadataMatcher metadata = 7 [
deprecated = true,
(envoy.annotations.deprecated_at_minor_version) = "3.0",
(envoy.annotations.disallowed_by_default) = true
agrawroh marked this conversation as resolved.
Show resolved Hide resolved
];

// Identifies the principal using a filter state object.
type.matcher.v3.FilterStateMatcher filter_state = 12;
Expand All @@ -366,6 +407,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;
agrawroh marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
8 changes: 8 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -183,5 +183,13 @@ new_features:
<envoy_v3_api_field_extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig.query_timeout_seconds>` and
custom tries could be configured by specifying :ref:`query_tries
<envoy_v3_api_field_extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig.query_tries>`.
- 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:
- area: rbac
change: |
metadata :ref:`sourced_metadata <envoy_v3_api_field_config.rbac.v3.Permission.metadata>` is now deprecated in the
favor of :ref:`sourced_metadata <envoy_v3_api_field_config.rbac.v3.Permission.sourced_metadata>`.
18 changes: 16 additions & 2 deletions source/extensions/filters/common/rbac/matchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -248,6 +258,10 @@ bool AuthenticatedMatcher::matches(const Network::Connection& connection,

bool MetadataMatcher::matches(const Network::Connection&, const Envoy::Http::RequestHeaderMap&,
const StreamInfo::StreamInfo& info) const {
if (metadata_source_ == envoy::config::rbac::v3::MetadataSource::ROUTE) {
// Return false if there's no route since we can't match its metadata
return info.route() ? matcher_.match(info.route()->metadata()) : false;
}
return matcher_.match(info.dynamicMetadata());
}

Expand Down
5 changes: 4 additions & 1 deletion source/extensions/filters/common/rbac/matchers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
83 changes: 79 additions & 4 deletions test/extensions/filters/common/rbac/matchers_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
agrawroh marked this conversation as resolved.
Show resolved Hide resolved
}

TEST(PolicyMatcher, PolicyMatcher) {
Expand Down Expand Up @@ -624,6 +626,79 @@ TEST(UriTemplateMatcher, NoPathInHeader) {
checkMatcher(UriTemplateMatcher(matcher), false, Envoy::Network::MockConnection(), headers);
}

TEST(MetadataMatcher, SourcedMetadataMatcher) {
NiceMock<Server::Configuration::MockServerFactoryContext> context;
Envoy::Network::MockConnection conn;
Envoy::Http::TestRequestHeaderMapImpl header;
NiceMock<StreamInfo::MockStreamInfo> info;
std::shared_ptr<Router::MockRoute> route = std::make_shared<Router::MockRoute>();

// Set up dynamic metadata
auto dynamic_label = MessageUtil::keyValueStruct("dynamic_key", "dynamic_value");
envoy::config::core::v3::Metadata dynamic_metadata;
dynamic_metadata.mutable_filter_metadata()->insert(
Protobuf::MapPair<std::string, ProtobufWkt::Struct>("rbac", dynamic_label));
EXPECT_CALL(Const(info), dynamicMetadata()).WillRepeatedly(ReturnRef(dynamic_metadata));

// Set up route metadata
auto route_label = MessageUtil::keyValueStruct("route_key", "route_value");
envoy::config::core::v3::Metadata route_metadata;
route_metadata.mutable_filter_metadata()->insert(
Protobuf::MapPair<std::string, ProtobufWkt::Struct>("rbac", route_label));
EXPECT_CALL(*route, metadata()).WillRepeatedly(ReturnRef(route_metadata));
EXPECT_CALL(info, route()).WillRepeatedly(Return(route));

// Test DYNAMIC source metadata match
{
envoy::type::matcher::v3::MetadataMatcher matcher;
matcher.set_filter("rbac");
matcher.add_path()->set_key("dynamic_key");
matcher.mutable_value()->mutable_string_match()->set_exact("dynamic_value");

checkMatcher(MetadataMatcher(Matchers::MetadataMatcher(matcher, context),
envoy::config::rbac::v3::MetadataSource::DYNAMIC),
true, conn, header, info);

// Should not match with wrong value
matcher.mutable_value()->mutable_string_match()->set_exact("wrong_value");
checkMatcher(MetadataMatcher(Matchers::MetadataMatcher(matcher, context),
envoy::config::rbac::v3::MetadataSource::DYNAMIC),
false, conn, header, info);
}

// Test ROUTE source metadata match
{
envoy::type::matcher::v3::MetadataMatcher matcher;
matcher.set_filter("rbac");
matcher.add_path()->set_key("route_key");
matcher.mutable_value()->mutable_string_match()->set_exact("route_value");

checkMatcher(MetadataMatcher(Matchers::MetadataMatcher(matcher, context),
envoy::config::rbac::v3::MetadataSource::ROUTE),
true, conn, header, info);

// Should not match with wrong value
matcher.mutable_value()->mutable_string_match()->set_exact("wrong_value");
checkMatcher(MetadataMatcher(Matchers::MetadataMatcher(matcher, context),
envoy::config::rbac::v3::MetadataSource::ROUTE),
false, conn, header, info);
}

// Test ROUTE source with null route
{
EXPECT_CALL(info, route()).WillRepeatedly(Return(nullptr));

envoy::type::matcher::v3::MetadataMatcher matcher;
matcher.set_filter("rbac");
matcher.add_path()->set_key("route_key");
matcher.mutable_value()->mutable_string_match()->set_exact("route_value");

checkMatcher(MetadataMatcher(Matchers::MetadataMatcher(matcher, context),
envoy::config::rbac::v3::MetadataSource::ROUTE),
false, conn, header, info);
}
}

} // namespace
} // namespace RBAC
} // namespace Common
Expand Down
1 change: 1 addition & 0 deletions test/extensions/filters/http/rbac/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ envoy_extension_cc_test(
"//source/extensions/common/matcher:matcher_lib",
"//source/extensions/common/matcher:trie_matcher_lib",
"//source/extensions/filters/http/dynamic_forward_proxy:config",
"//source/extensions/filters/http/header_to_metadata:config",
"//source/extensions/filters/http/rbac:config",
"//source/extensions/key_value/file_based:config_lib",
"//source/extensions/matching/http/cel_input:cel_input_lib",
Expand Down
Loading