From c90fae35cbc22b8009dbf6b5ef4616cd1113fa46 Mon Sep 17 00:00:00 2001 From: Ashitha Santhosh <55257063+ashithasantosh@users.noreply.github.com> Date: Tue, 11 May 2021 09:56:36 -0700 Subject: [PATCH] IPMatcher implementation. (#26132) --- .../authorization/cel_authorization_engine.cc | 5 +- .../security/authorization/evaluate_args.cc | 79 +++++++------ .../security/authorization/evaluate_args.h | 23 +++- .../lib/security/authorization/matchers.cc | 102 +++++++++++------ .../lib/security/authorization/matchers.h | 21 ++-- .../lib/security/authorization/rbac_policy.h | 26 ++--- .../lib/security/credentials/tls/tls_utils.cc | 32 ++++++ .../lib/security/credentials/tls/tls_utils.h | 13 +++ .../security/authorization_matchers_test.cc | 106 ++++++++++++++++-- test/core/security/evaluate_args_test.cc | 41 +++---- 10 files changed, 324 insertions(+), 124 deletions(-) diff --git a/src/core/lib/security/authorization/cel_authorization_engine.cc b/src/core/lib/security/authorization/cel_authorization_engine.cc index 844d9fb512c..fae5cc456d0 100644 --- a/src/core/lib/security/authorization/cel_authorization_engine.cc +++ b/src/core/lib/security/authorization/cel_authorization_engine.cc @@ -16,6 +16,7 @@ #include "absl/memory/memory.h" +#include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/security/authorization/cel_authorization_engine.h" namespace grpc_core { @@ -132,7 +133,7 @@ std::unique_ptr CelAuthorizationEngine::CreateActivation( activation->InsertValue(kHeaders, mock_cel::CelValue::CreateMap(headers_.get())); } else if (elem == kSourceAddress) { - absl::string_view source_address(args.GetPeerAddress()); + absl::string_view source_address(args.GetPeerAddressString()); if (!source_address.empty()) { activation->InsertValue( kSourceAddress, @@ -142,7 +143,7 @@ std::unique_ptr CelAuthorizationEngine::CreateActivation( activation->InsertValue( kSourcePort, mock_cel::CelValue::CreateInt64(args.GetPeerPort())); } else if (elem == kDestinationAddress) { - absl::string_view destination_address(args.GetLocalAddress()); + absl::string_view destination_address(args.GetLocalAddressString()); if (!destination_address.empty()) { activation->InsertValue( kDestinationAddress, diff --git a/src/core/lib/security/authorization/evaluate_args.cc b/src/core/lib/security/authorization/evaluate_args.cc index 12144dfbde3..98dc371bf7e 100644 --- a/src/core/lib/security/authorization/evaluate_args.cc +++ b/src/core/lib/security/authorization/evaluate_args.cc @@ -17,48 +17,43 @@ #include "src/core/lib/security/authorization/evaluate_args.h" #include "src/core/lib/address_utils/parse_address.h" +#include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/gprpp/host_port.h" +#include "src/core/lib/security/credentials/tls/tls_utils.h" #include "src/core/lib/slice/slice_utils.h" namespace grpc_core { namespace { -absl::string_view GetAuthPropertyValue(grpc_auth_context* context, - const char* property_name) { - grpc_auth_property_iterator it = - grpc_auth_context_find_properties_by_name(context, property_name); - const grpc_auth_property* prop = grpc_auth_property_iterator_next(&it); - if (prop == nullptr) { - gpr_log(GPR_DEBUG, "No value found for %s property.", property_name); - return ""; - } - if (grpc_auth_property_iterator_next(&it) != nullptr) { - gpr_log(GPR_DEBUG, "Multiple values found for %s property.", property_name); - return ""; - } - return absl::string_view(prop->value, prop->value_length); -} - -void ParseEndpointUri(absl::string_view uri_text, std::string* address, - int* port) { +EvaluateArgs::PerChannelArgs::Address ParseEndpointUri( + absl::string_view uri_text) { + EvaluateArgs::PerChannelArgs::Address address; absl::StatusOr uri = URI::Parse(uri_text); if (!uri.ok()) { gpr_log(GPR_DEBUG, "Failed to parse uri."); - return; + return address; } absl::string_view host_view; absl::string_view port_view; if (!SplitHostPort(uri->path(), &host_view, &port_view)) { gpr_log(GPR_DEBUG, "Failed to split %s into host and port.", uri->path().c_str()); - return; + return address; } - *address = std::string(host_view); - if (!absl::SimpleAtoi(port_view, port)) { + if (!absl::SimpleAtoi(port_view, &address.port)) { gpr_log(GPR_DEBUG, "Port %s is out of range or null.", std::string(port_view).c_str()); } + address.address_str = std::string(host_view); + grpc_error_handle error = grpc_string_to_sockaddr( + &address.address, address.address_str.c_str(), address.port); + if (error != GRPC_ERROR_NONE) { + gpr_log(GPR_DEBUG, "Address %s is not IPv4/IPv6. Error: %s", + address.address_str.c_str(), grpc_error_std_string(error).c_str()); + } + GRPC_ERROR_UNREF(error); + return address; } } // namespace @@ -70,14 +65,13 @@ EvaluateArgs::PerChannelArgs::PerChannelArgs(grpc_auth_context* auth_context, auth_context, GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME); spiffe_id = GetAuthPropertyValue(auth_context, GRPC_PEER_SPIFFE_ID_PROPERTY_NAME); + dns_sans = GetAuthPropertyArray(auth_context, GRPC_PEER_DNS_PROPERTY_NAME); common_name = GetAuthPropertyValue(auth_context, GRPC_X509_CN_PROPERTY_NAME); } if (endpoint != nullptr) { - ParseEndpointUri(grpc_endpoint_get_local_address(endpoint), &local_address, - &local_port); - ParseEndpointUri(grpc_endpoint_get_peer(endpoint), &peer_address, - &peer_port); + local_address = ParseEndpointUri(grpc_endpoint_get_local_address(endpoint)); + peer_address = ParseEndpointUri(grpc_endpoint_get_peer(endpoint)); } } @@ -134,32 +128,46 @@ absl::optional EvaluateArgs::GetHeaderValue( return grpc_metadata_batch_get_value(metadata_, key, concatenated_value); } -absl::string_view EvaluateArgs::GetLocalAddress() const { +grpc_resolved_address EvaluateArgs::GetLocalAddress() const { + if (channel_args_ == nullptr) { + return {}; + } + return channel_args_->local_address.address; +} + +absl::string_view EvaluateArgs::GetLocalAddressString() const { if (channel_args_ == nullptr) { return ""; } - return channel_args_->local_address; + return channel_args_->local_address.address_str; } int EvaluateArgs::GetLocalPort() const { if (channel_args_ == nullptr) { return 0; } - return channel_args_->local_port; + return channel_args_->local_address.port; +} + +grpc_resolved_address EvaluateArgs::GetPeerAddress() const { + if (channel_args_ == nullptr) { + return {}; + } + return channel_args_->peer_address.address; } -absl::string_view EvaluateArgs::GetPeerAddress() const { +absl::string_view EvaluateArgs::GetPeerAddressString() const { if (channel_args_ == nullptr) { return ""; } - return channel_args_->peer_address; + return channel_args_->peer_address.address_str; } int EvaluateArgs::GetPeerPort() const { if (channel_args_ == nullptr) { return 0; } - return channel_args_->peer_port; + return channel_args_->peer_address.port; } absl::string_view EvaluateArgs::GetTransportSecurityType() const { @@ -176,6 +184,13 @@ absl::string_view EvaluateArgs::GetSpiffeId() const { return channel_args_->spiffe_id; } +std::vector EvaluateArgs::GetDnsSans() const { + if (channel_args_ == nullptr) { + return {}; + } + return channel_args_->dns_sans; +} + absl::string_view EvaluateArgs::GetCommonName() const { if (channel_args_ == nullptr) { return ""; diff --git a/src/core/lib/security/authorization/evaluate_args.h b/src/core/lib/security/authorization/evaluate_args.h index b3ac93caa4e..166d52d9e60 100644 --- a/src/core/lib/security/authorization/evaluate_args.h +++ b/src/core/lib/security/authorization/evaluate_args.h @@ -22,6 +22,7 @@ #include "absl/types/optional.h" #include "src/core/lib/iomgr/endpoint.h" +#include "src/core/lib/iomgr/resolve_address.h" #include "src/core/lib/security/context/security_context.h" #include "src/core/lib/transport/metadata_batch.h" @@ -32,15 +33,22 @@ class EvaluateArgs { // Caller is responsible for ensuring auth_context outlives PerChannelArgs // struct. struct PerChannelArgs { + struct Address { + // The address in sockaddr form. + grpc_resolved_address address; + // The same address with only the host part. + std::string address_str; + int port = 0; + }; + PerChannelArgs(grpc_auth_context* auth_context, grpc_endpoint* endpoint); absl::string_view transport_security_type; absl::string_view spiffe_id; + std::vector dns_sans; absl::string_view common_name; - std::string local_address; - int local_port = 0; - std::string peer_address; - int peer_port = 0; + Address local_address; + Address peer_address; }; EvaluateArgs(grpc_metadata_batch* metadata, PerChannelArgs* channel_args) @@ -60,12 +68,15 @@ class EvaluateArgs { absl::optional GetHeaderValue( absl::string_view key, std::string* concatenated_value) const; - absl::string_view GetLocalAddress() const; + grpc_resolved_address GetLocalAddress() const; + absl::string_view GetLocalAddressString() const; int GetLocalPort() const; - absl::string_view GetPeerAddress() const; + grpc_resolved_address GetPeerAddress() const; + absl::string_view GetPeerAddressString() const; int GetPeerPort() const; absl::string_view GetTransportSecurityType() const; absl::string_view GetSpiffeId() const; + std::vector GetDnsSans() const; absl::string_view GetCommonName() const; private: diff --git a/src/core/lib/security/authorization/matchers.cc b/src/core/lib/security/authorization/matchers.cc index 5613b748ebd..478054210a5 100644 --- a/src/core/lib/security/authorization/matchers.cc +++ b/src/core/lib/security/authorization/matchers.cc @@ -14,34 +14,11 @@ #include +#include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/security/authorization/matchers.h" namespace grpc_core { -namespace { - -bool AuthenticatedMatchesHelper(const EvaluateArgs& args, - const StringMatcher& matcher) { - if (args.GetTransportSecurityType() != GRPC_SSL_TRANSPORT_SECURITY_TYPE) { - // Connection is not authenticated. - return false; - } - if (matcher.string_matcher().empty()) { - // Allows any authenticated user. - return true; - } - absl::string_view spiffe_id = args.GetSpiffeId(); - if (!spiffe_id.empty()) { - return matcher.Match(spiffe_id); - } - // TODO(ashithasantosh): Check principal matches DNS SAN, followed by Subject - // field from certificate. This requires updating tsi_peer to expose these - // fields. - return false; -} - -} // namespace - std::unique_ptr AuthorizationMatcher::Create( Rbac::Permission permission) { switch (permission.type) { @@ -60,8 +37,9 @@ std::unique_ptr AuthorizationMatcher::Create( return absl::make_unique( std::move(permission.string_matcher), permission.not_rule); case Rbac::Permission::RuleType::kDestIp: - return absl::make_unique(std::move(permission.ip), - permission.not_rule); + return absl::make_unique( + IpAuthorizationMatcher::Type::kDestIp, std::move(permission.ip), + permission.not_rule); case Rbac::Permission::RuleType::kDestPort: return absl::make_unique(permission.port, permission.not_rule); @@ -87,10 +65,17 @@ std::unique_ptr AuthorizationMatcher::Create( return absl::make_unique( std::move(principal.string_matcher), principal.not_rule); case Rbac::Principal::RuleType::kSourceIp: + return absl::make_unique( + IpAuthorizationMatcher::Type::kSourceIp, std::move(principal.ip), + principal.not_rule); case Rbac::Principal::RuleType::kDirectRemoteIp: + return absl::make_unique( + IpAuthorizationMatcher::Type::kDirectRemoteIp, + std::move(principal.ip), principal.not_rule); case Rbac::Principal::RuleType::kRemoteIp: - return absl::make_unique(std::move(principal.ip), - principal.not_rule); + return absl::make_unique( + IpAuthorizationMatcher::Type::kRemoteIp, std::move(principal.ip), + principal.not_rule); case Rbac::Principal::RuleType::kHeader: return absl::make_unique( std::move(principal.header_matcher), principal.not_rule); @@ -162,9 +147,40 @@ bool HeaderAuthorizationMatcher::Matches(const EvaluateArgs& args) const { return matches != not_rule_; } -// TODO(ashithasantosh): Implement IpAuthorizationMatcher::Matches. -bool IpAuthorizationMatcher::Matches(const EvaluateArgs&) const { - bool matches = false; +IpAuthorizationMatcher::IpAuthorizationMatcher(Type type, Rbac::CidrRange range, + bool not_rule) + : type_(type), prefix_len_(range.prefix_len), not_rule_(not_rule) { + grpc_error_handle error = + grpc_string_to_sockaddr(&subnet_address_, range.address_prefix.c_str(), + /*port does not matter here*/ 0); + if (error == GRPC_ERROR_NONE) { + grpc_sockaddr_mask_bits(&subnet_address_, prefix_len_); + } else { + gpr_log(GPR_DEBUG, "CidrRange address %s is not IPv4/IPv6. Error: %s", + range.address_prefix.c_str(), grpc_error_std_string(error).c_str()); + } + GRPC_ERROR_UNREF(error); +} + +bool IpAuthorizationMatcher::Matches(const EvaluateArgs& args) const { + grpc_resolved_address address; + switch (type_) { + case Type::kDestIp: { + address = args.GetLocalAddress(); + break; + } + case Type::kSourceIp: + case Type::kDirectRemoteIp: { + address = args.GetPeerAddress(); + break; + } + default: { + // Currently we do not support matching rules containing "remote_ip". + return not_rule_; + } + } + bool matches = + grpc_sockaddr_match_subnet(&address, &subnet_address_, prefix_len_); return matches != not_rule_; } @@ -175,8 +191,28 @@ bool PortAuthorizationMatcher::Matches(const EvaluateArgs& args) const { bool AuthenticatedAuthorizationMatcher::Matches( const EvaluateArgs& args) const { - bool matches = AuthenticatedMatchesHelper(args, matcher_); - return matches != not_rule_; + if (args.GetTransportSecurityType() != GRPC_SSL_TRANSPORT_SECURITY_TYPE) { + // Connection is not authenticated. + return not_rule_; + } + if (matcher_.string_matcher().empty()) { + // Allows any authenticated user. + return !not_rule_; + } + absl::string_view spiffe_id = args.GetSpiffeId(); + if (!spiffe_id.empty()) { + return matcher_.Match(spiffe_id) != not_rule_; + } + std::vector dns_sans = args.GetDnsSans(); + if (!dns_sans.empty()) { + for (const auto& dns : dns_sans) { + if (matcher_.Match(dns)) { + return !not_rule_; + } + } + } + // TODO(ashithasantosh): Check Subject field from certificate. + return not_rule_; } bool ReqServerNameAuthorizationMatcher::Matches(const EvaluateArgs&) const { diff --git a/src/core/lib/security/authorization/matchers.h b/src/core/lib/security/authorization/matchers.h index 86b40ee75b5..b74bd7541ed 100644 --- a/src/core/lib/security/authorization/matchers.h +++ b/src/core/lib/security/authorization/matchers.h @@ -107,18 +107,25 @@ class HeaderAuthorizationMatcher : public AuthorizationMatcher { }; // Perform a match against IP Cidr Range. -// TODO(ashithasantosh): Handle type of Ip or use seperate matchers for each -// type. Implement Match functionality, this would require updating EvaluateArgs -// getters, to return format of IP as well. class IpAuthorizationMatcher : public AuthorizationMatcher { public: - explicit IpAuthorizationMatcher(Rbac::CidrRange range, bool not_rule = false) - : range_(std::move(range)), not_rule_(not_rule) {} + enum class Type { + kDestIp, + kSourceIp, + kDirectRemoteIp, + kRemoteIp, + }; - bool Matches(const EvaluateArgs&) const override; + IpAuthorizationMatcher(Type type, Rbac::CidrRange range, + bool not_rule = false); + + bool Matches(const EvaluateArgs& args) const override; private: - const Rbac::CidrRange range_; + const Type type_; + // Subnet masked address. + grpc_resolved_address subnet_address_; + const uint32_t prefix_len_; // Negates matching the provided permission/principal. const bool not_rule_; }; diff --git a/src/core/lib/security/authorization/rbac_policy.h b/src/core/lib/security/authorization/rbac_policy.h index 027bf92eb26..f13232ba403 100644 --- a/src/core/lib/security/authorization/rbac_policy.h +++ b/src/core/lib/security/authorization/rbac_policy.h @@ -58,21 +58,21 @@ struct Rbac { }; Permission() = default; - // For AND/OR RuleType. + // For kAnd/kOr RuleType. Permission(Permission::RuleType type, std::vector> permissions, bool not_rule = false); - // For ANY RuleType. + // For kAny RuleType. explicit Permission(Permission::RuleType type, bool not_rule = false); - // For HEADER RuleType. + // For kHeader RuleType. Permission(Permission::RuleType type, HeaderMatcher header_matcher, bool not_rule = false); - // For PATH/REQ_SERVER_NAME RuleType. + // For kPath/kReqServerName RuleType. Permission(Permission::RuleType type, StringMatcher string_matcher, bool not_rule = false); - // For DEST_IP RuleType. + // For kDestIp RuleType. Permission(Permission::RuleType type, CidrRange ip, bool not_rule = false); - // For DEST_PORT RuleType. + // For kDestPort RuleType. Permission(Permission::RuleType type, int port, bool not_rule = false); Permission(Permission&& other) noexcept; @@ -85,7 +85,7 @@ struct Rbac { StringMatcher string_matcher; CidrRange ip; int port; - // For type AND/OR. + // For type kAnd/kOr. std::vector> permissions; bool not_rule = false; }; @@ -104,18 +104,18 @@ struct Rbac { }; Principal() = default; - // For AND/OR RuleType. + // For kAnd/kOr RuleType. Principal(Principal::RuleType type, std::vector> principals, bool not_rule = false); - // For ANY RuleType. + // For kAny RuleType. explicit Principal(Principal::RuleType type, bool not_rule = false); - // For PRINCIPAL_NAME/PATH RuleType. + // For kPrincipalName/kPath RuleType. Principal(Principal::RuleType type, StringMatcher string_matcher, bool not_rule = false); - // For SOURCE_IP/DIRECT_REMOTE_IP/REMOTE_IP RuleType. + // For kSourceIp/kDirectRemoteIp/kRemoteIp RuleType. Principal(Principal::RuleType type, CidrRange ip, bool not_rule = false); - // For HEADER RuleType. + // For kHeader RuleType. Principal(Principal::RuleType type, HeaderMatcher header_matcher, bool not_rule = false); @@ -128,7 +128,7 @@ struct Rbac { HeaderMatcher header_matcher; StringMatcher string_matcher; CidrRange ip; - // For type AND/OR. + // For type kAnd/kOr. std::vector> principals; bool not_rule = false; }; diff --git a/src/core/lib/security/credentials/tls/tls_utils.cc b/src/core/lib/security/credentials/tls/tls_utils.cc index b94c2eed5c9..2dcc19741cb 100644 --- a/src/core/lib/security/credentials/tls/tls_utils.cc +++ b/src/core/lib/security/credentials/tls/tls_utils.cc @@ -88,4 +88,36 @@ bool VerifySubjectAlternativeName(absl::string_view subject_alternative_name, std::string::npos; } +absl::string_view GetAuthPropertyValue(grpc_auth_context* context, + const char* property_name) { + grpc_auth_property_iterator it = + grpc_auth_context_find_properties_by_name(context, property_name); + const grpc_auth_property* prop = grpc_auth_property_iterator_next(&it); + if (prop == nullptr) { + gpr_log(GPR_DEBUG, "No value found for %s property.", property_name); + return ""; + } + if (grpc_auth_property_iterator_next(&it) != nullptr) { + gpr_log(GPR_DEBUG, "Multiple values found for %s property.", property_name); + return ""; + } + return absl::string_view(prop->value, prop->value_length); +} + +std::vector GetAuthPropertyArray(grpc_auth_context* context, + const char* property_name) { + std::vector values; + grpc_auth_property_iterator it = + grpc_auth_context_find_properties_by_name(context, property_name); + const grpc_auth_property* prop = grpc_auth_property_iterator_next(&it); + while (prop != nullptr) { + values.emplace_back(prop->value, prop->value_length); + prop = grpc_auth_property_iterator_next(&it); + } + if (values.empty()) { + gpr_log(GPR_DEBUG, "No value found for %s property.", property_name); + } + return values; +} + } // namespace grpc_core diff --git a/src/core/lib/security/credentials/tls/tls_utils.h b/src/core/lib/security/credentials/tls/tls_utils.h index 6fe9bb46673..31a7f32e082 100644 --- a/src/core/lib/security/credentials/tls/tls_utils.h +++ b/src/core/lib/security/credentials/tls/tls_utils.h @@ -26,6 +26,8 @@ #include "absl/strings/string_view.h" +#include "src/core/lib/security/context/security_context.h" + namespace grpc_core { // Matches \a subject_alternative_name with \a matcher. Returns true if there @@ -33,6 +35,17 @@ namespace grpc_core { bool VerifySubjectAlternativeName(absl::string_view subject_alternative_name, const std::string& matcher); +// Returns value for the specified property_name from auth context. Here the +// property is expected to have a single value. Returns empty if multiple values +// are found. +absl::string_view GetAuthPropertyValue(grpc_auth_context* context, + const char* property_name); + +// Returns values for the specified property_name from auth context. Here the +// property can have any number of values. +std::vector GetAuthPropertyArray(grpc_auth_context* context, + const char* property_name); + } // namespace grpc_core #endif // GRPC_CORE_LIB_SECURITY_CREDENTIALS_TLS_TLS_UTILS_H diff --git a/test/core/security/authorization_matchers_test.cc b/test/core/security/authorization_matchers_test.cc index 36b38621d18..10aa9e707ab 100644 --- a/test/core/security/authorization_matchers_test.cc +++ b/test/core/security/authorization_matchers_test.cc @@ -78,14 +78,13 @@ TEST_F(AuthorizationMatchersTest, AndAuthorizationMatcherFailedMatch) { TEST_F(AuthorizationMatchersTest, NotAndAuthorizationMatcher) { args_.AddPairToMetadata(":path", "/expected/foo"); EvaluateArgs args = args_.MakeEvaluateArgs(); - StringMatcher string_matcher = + std::vector> ids; + ids.push_back(absl::make_unique( + Rbac::Permission::RuleType::kPath, StringMatcher::Create(StringMatcher::Type::kExact, /*matcher=*/"/expected/foo", /*case_sensitive=*/false) - .value(); - std::vector> ids; - ids.push_back(absl::make_unique( - Rbac::Permission::RuleType::kPath, std::move(string_matcher))); + .value())); AndAuthorizationMatcher matcher(std::move(ids), /*not_rule=*/true); EXPECT_FALSE(matcher.Matches(args)); } @@ -94,13 +93,12 @@ TEST_F(AuthorizationMatchersTest, OrAuthorizationMatcherSuccessfulMatch) { args_.AddPairToMetadata("foo", "bar"); args_.SetLocalEndpoint("ipv4:255.255.255.255:123"); EvaluateArgs args = args_.MakeEvaluateArgs(); - HeaderMatcher header_matcher = - HeaderMatcher::Create(/*name=*/"foo", HeaderMatcher::Type::kExact, - /*matcher=*/"bar") - .value(); std::vector> rules; rules.push_back(absl::make_unique( - Rbac::Permission::RuleType::kHeader, header_matcher)); + Rbac::Permission::RuleType::kHeader, + HeaderMatcher::Create(/*name=*/"foo", HeaderMatcher::Type::kExact, + /*matcher=*/"bar") + .value())); rules.push_back(absl::make_unique( Rbac::Permission::RuleType::kDestPort, /*port=*/456)); OrAuthorizationMatcher matcher(std::move(rules)); @@ -211,7 +209,9 @@ TEST_F(AuthorizationMatchersTest, NotPathAuthorizationMatcher) { args_.AddPairToMetadata(":path", "expected/path"); EvaluateArgs args = args_.MakeEvaluateArgs(); PathAuthorizationMatcher matcher( - StringMatcher::Create(StringMatcher::Type::kExact, "expected/path", false) + StringMatcher::Create(StringMatcher::Type::kExact, + /*matcher=*/"expected/path", + /*case_sensitive=*/false) .value(), /*not_rule=*/true); EXPECT_FALSE(matcher.Matches(args)); @@ -281,6 +281,60 @@ TEST_F(AuthorizationMatchersTest, NotHeaderAuthorizationMatcher) { EXPECT_TRUE(matcher.Matches(args)); } +TEST_F(AuthorizationMatchersTest, + IpAuthorizationMatcherLocalIpSuccessfulMatch) { + args_.SetLocalEndpoint("ipv4:1.2.3.4:123"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + IpAuthorizationMatcher matcher( + IpAuthorizationMatcher::Type::kDestIp, + Rbac::CidrRange(/*address_prefix=*/"1.7.8.9", /*prefix_len=*/8)); + EXPECT_TRUE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, IpAuthorizationMatcherLocalIpFailedMatch) { + args_.SetLocalEndpoint("ipv4:1.2.3.4:123"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + IpAuthorizationMatcher matcher( + IpAuthorizationMatcher::Type::kDestIp, + Rbac::CidrRange(/*address_prefix=*/"1.2.3.9", /*prefix_len=*/32)); + EXPECT_FALSE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, IpAuthorizationMatcherPeerIpSuccessfulMatch) { + args_.SetPeerEndpoint("ipv6:[1:2:3::]:456"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + IpAuthorizationMatcher matcher( + IpAuthorizationMatcher::Type::kSourceIp, + Rbac::CidrRange(/*address_prefix=*/"1:2:4::", /*prefix_len=*/32)); + EXPECT_TRUE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, IpAuthorizationMatcherPeerIpFailedMatch) { + args_.SetPeerEndpoint("ipv6:[1:2::]:456"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + IpAuthorizationMatcher matcher( + IpAuthorizationMatcher::Type::kSourceIp, + Rbac::CidrRange(/*address_prefix=*/"1:3::", /*prefix_len=*/32)); + EXPECT_FALSE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, + IpAuthorizationMatcherUnsupportedIpFailedMatch) { + EvaluateArgs args = args_.MakeEvaluateArgs(); + IpAuthorizationMatcher matcher(IpAuthorizationMatcher::Type::kRemoteIp, {}); + EXPECT_FALSE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, NotIpAuthorizationMatcherSuccessfulMatch) { + args_.SetLocalEndpoint("ipv4:1.2.3.4:123"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + IpAuthorizationMatcher matcher( + IpAuthorizationMatcher::Type::kDestIp, + Rbac::CidrRange(/*address_prefix=*/"1.7.8.9", /*prefix_len=*/8), + /*not_rule=*/true); + EXPECT_FALSE(matcher.Matches(args)); +} + TEST_F(AuthorizationMatchersTest, PortAuthorizationMatcherSuccessfulMatch) { args_.SetLocalEndpoint("ipv4:255.255.255.255:123"); EvaluateArgs args = args_.MakeEvaluateArgs(); @@ -355,6 +409,36 @@ TEST_F(AuthorizationMatchersTest, AuthenticatedMatcherFailedSpiffeIdMatches) { EXPECT_FALSE(matcher.Matches(args)); } +TEST_F(AuthorizationMatchersTest, AuthenticatedMatcherSuccessfulDnsSanMatches) { + args_.AddPropertyToAuthContext(GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME, + GRPC_SSL_TRANSPORT_SECURITY_TYPE); + args_.AddPropertyToAuthContext(GRPC_PEER_DNS_PROPERTY_NAME, + "foo.test.domain.com"); + args_.AddPropertyToAuthContext(GRPC_PEER_DNS_PROPERTY_NAME, + "bar.test.domain.com"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + AuthenticatedAuthorizationMatcher matcher( + StringMatcher::Create(StringMatcher::Type::kExact, + /*matcher=*/"bar.test.domain.com", + /*case_sensitive=*/false) + .value()); + EXPECT_TRUE(matcher.Matches(args)); +} + +TEST_F(AuthorizationMatchersTest, AuthenticatedMatcherFailedDnsSanMatches) { + args_.AddPropertyToAuthContext(GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME, + GRPC_SSL_TRANSPORT_SECURITY_TYPE); + args_.AddPropertyToAuthContext(GRPC_PEER_DNS_PROPERTY_NAME, + "foo.test.domain.com"); + EvaluateArgs args = args_.MakeEvaluateArgs(); + AuthenticatedAuthorizationMatcher matcher( + StringMatcher::Create(StringMatcher::Type::kExact, + /*matcher=*/"bar.test.domain.com", + /*case_sensitive=*/false) + .value()); + EXPECT_FALSE(matcher.Matches(args)); +} + TEST_F(AuthorizationMatchersTest, AuthenticatedMatcherFailedNothingMatches) { args_.AddPropertyToAuthContext(GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME, GRPC_SSL_TRANSPORT_SECURITY_TYPE); diff --git a/test/core/security/evaluate_args_test.cc b/test/core/security/evaluate_args_test.cc index de98537d5f3..36ddfce7369 100644 --- a/test/core/security/evaluate_args_test.cc +++ b/test/core/security/evaluate_args_test.cc @@ -17,6 +17,7 @@ #include #include +#include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/security/authorization/evaluate_args.h" #include "test/core/util/evaluate_args_test_util.h" #include "test/core/util/test_config.h" @@ -75,38 +76,31 @@ TEST_F(EvaluateArgsTest, GetHeaderValueSuccess) { EXPECT_EQ(value.value(), "value123"); } -TEST_F(EvaluateArgsTest, TestIpv4LocalAddressAndPort) { - util_.SetLocalEndpoint("ipv4:255.255.255.255:123"); - EvaluateArgs args = util_.MakeEvaluateArgs(); - EXPECT_EQ(args.GetLocalAddress(), "255.255.255.255"); - EXPECT_EQ(args.GetLocalPort(), 123); -} - -TEST_F(EvaluateArgsTest, TestIpv4PeerAddressAndPort) { - util_.SetPeerEndpoint("ipv4:128.128.128.128:321"); - EvaluateArgs args = util_.MakeEvaluateArgs(); - EXPECT_EQ(args.GetPeerAddress(), "128.128.128.128"); - EXPECT_EQ(args.GetPeerPort(), 321); -} - -TEST_F(EvaluateArgsTest, TestIpv6LocalAddressAndPort) { +TEST_F(EvaluateArgsTest, TestLocalAddressAndPort) { util_.SetLocalEndpoint("ipv6:[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:456"); EvaluateArgs args = util_.MakeEvaluateArgs(); - EXPECT_EQ(args.GetLocalAddress(), "2001:0db8:85a3:0000:0000:8a2e:0370:7334"); + grpc_resolved_address local_address = args.GetLocalAddress(); + EXPECT_EQ(grpc_sockaddr_to_uri(&local_address), + "ipv6:[2001:db8:85a3::8a2e:370:7334]:456"); + EXPECT_EQ(args.GetLocalAddressString(), + "2001:0db8:85a3:0000:0000:8a2e:0370:7334"); EXPECT_EQ(args.GetLocalPort(), 456); } -TEST_F(EvaluateArgsTest, TestIpv6PeerAddressAndPort) { - util_.SetPeerEndpoint("ipv6:[2001:db8::1]:654"); +TEST_F(EvaluateArgsTest, TestPeerAddressAndPort) { + util_.SetPeerEndpoint("ipv4:255.255.255.255:123"); EvaluateArgs args = util_.MakeEvaluateArgs(); - EXPECT_EQ(args.GetPeerAddress(), "2001:db8::1"); - EXPECT_EQ(args.GetPeerPort(), 654); + grpc_resolved_address peer_address = args.GetPeerAddress(); + EXPECT_EQ(grpc_sockaddr_to_uri(&peer_address), "ipv4:255.255.255.255:123"); + EXPECT_EQ(args.GetPeerAddressString(), "255.255.255.255"); + EXPECT_EQ(args.GetPeerPort(), 123); } TEST_F(EvaluateArgsTest, EmptyAuthContext) { EvaluateArgs args = util_.MakeEvaluateArgs(); EXPECT_TRUE(args.GetTransportSecurityType().empty()); EXPECT_TRUE(args.GetSpiffeId().empty()); + EXPECT_TRUE(args.GetDnsSans().empty()); EXPECT_TRUE(args.GetCommonName().empty()); } @@ -139,6 +133,13 @@ TEST_F(EvaluateArgsTest, GetSpiffeIdFailDuplicateProperty) { EXPECT_TRUE(args.GetSpiffeId().empty()); } +TEST_F(EvaluateArgsTest, GetDnsSanSuccessMultipleProperties) { + util_.AddPropertyToAuthContext(GRPC_PEER_DNS_PROPERTY_NAME, "foo"); + util_.AddPropertyToAuthContext(GRPC_PEER_DNS_PROPERTY_NAME, "bar"); + EvaluateArgs args = util_.MakeEvaluateArgs(); + EXPECT_THAT(args.GetDnsSans(), ::testing::ElementsAre("foo", "bar")); +} + TEST_F(EvaluateArgsTest, GetCommonNameSuccessOneProperty) { util_.AddPropertyToAuthContext(GRPC_X509_CN_PROPERTY_NAME, "server123"); EvaluateArgs args = util_.MakeEvaluateArgs();