From: Casey Bodley <cbodley@redhat.com>
Date: Thu, 13 Mar 2025 23:45:22 +0000 (-0400)
Subject: rgw/iam: simplify match_policy() for colon-delimited use only
X-Git-Tag: v19.2.3~58^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=04a52dfe96b43aec10d89521afb56b411b0e2d4b;p=ceph.git

rgw/iam: simplify match_policy() for colon-delimited use only

remove unused MATCH_POLICY_STRING

ARN::match() was the only caller for MATCH_POLICY_ARN, but it was used
to match the 'region' and 'account' components that were already parsed
out of the colon-separated ARN string. for that use, we don't need the
loop-over-colons behavior of match_policy() so can call match_wildcards()
directly

after doing the same for MATCH_POLICY_RESOURCE, we no longer have any
non-looping callers of match_policy() so can treat 'bool colonblocks' as
unconditionally true

Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 4cd40c7f715304519fe91b1d6f296ce06ef6c2ef)
---

diff --git a/src/rgw/rgw_arn.cc b/src/rgw/rgw_arn.cc
index fddc3d769cc2..e0ab8fbbe2af 100644
--- a/src/rgw/rgw_arn.cc
+++ b/src/rgw/rgw_arn.cc
@@ -328,15 +328,15 @@ bool ARN::match(const ARN& candidate) const {
     return false;
   }
 
-  if (!match_policy(region, candidate.region, MATCH_POLICY_ARN)) {
+  if (!match_wildcards(region, candidate.region, MATCH_CASE_INSENSITIVE)) {
     return false;
   }
 
-  if (!match_policy(account, candidate.account, MATCH_POLICY_ARN)) {
+  if (!match_wildcards(account, candidate.account, MATCH_CASE_INSENSITIVE)) {
     return false;
   }
 
-  if (!match_policy(resource, candidate.resource, MATCH_POLICY_RESOURCE)) {
+  if (!match_wildcards(resource, candidate.resource, 0)) {
     return false;
   }
 
diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc
index 0cf3cd3ff6fd..6fa913f05d6d 100644
--- a/src/rgw/rgw_common.cc
+++ b/src/rgw/rgw_common.cc
@@ -2169,15 +2169,11 @@ bool match_policy(const std::string& pattern, const std::string& input,
 {
   const uint32_t flag2 = flag & (MATCH_POLICY_ACTION|MATCH_POLICY_ARN) ?
       MATCH_CASE_INSENSITIVE : 0;
-  const bool colonblocks = !(flag & (MATCH_POLICY_RESOURCE |
-				     MATCH_POLICY_STRING));
 
-  const auto npos = std::string_view::npos;
   std::string_view::size_type last_pos_input = 0, last_pos_pattern = 0;
   while (true) {
-    auto cur_pos_input = colonblocks ? input.find(":", last_pos_input) : npos;
-    auto cur_pos_pattern =
-      colonblocks ? pattern.find(":", last_pos_pattern) : npos;
+    auto cur_pos_input = input.find(":", last_pos_input);
+    auto cur_pos_pattern = pattern.find(":", last_pos_pattern);
 
     auto substr_input = input.substr(last_pos_input, cur_pos_input);
     auto substr_pattern = pattern.substr(last_pos_pattern, cur_pos_pattern);
@@ -2185,9 +2181,9 @@ bool match_policy(const std::string& pattern, const std::string& input,
     if (!match_wildcards(substr_pattern, substr_input, flag2))
       return false;
 
-    if (cur_pos_pattern == npos)
-      return cur_pos_input == npos;
-    if (cur_pos_input == npos)
+    if (cur_pos_pattern == pattern.npos)
+      return cur_pos_input == input.npos;
+    if (cur_pos_input == input.npos)
       return false;
 
     last_pos_pattern = cur_pos_pattern + 1;
diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h
index 9aa8fb9945f7..a94d4adab992 100644
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -1937,9 +1937,7 @@ extern std::string calc_hash_sha256_restart_stream(ceph::crypto::SHA256** phash)
 extern int rgw_parse_op_type_list(const std::string& str, uint32_t *perm);
 
 static constexpr uint32_t MATCH_POLICY_ACTION = 0x01;
-static constexpr uint32_t MATCH_POLICY_RESOURCE = 0x02;
-static constexpr uint32_t MATCH_POLICY_ARN = 0x04;
-static constexpr uint32_t MATCH_POLICY_STRING = 0x08;
+static constexpr uint32_t MATCH_POLICY_ARN = 0x02;
 
 extern bool match_policy(const std::string& pattern, const std::string& input,
                          uint32_t flag);
diff --git a/src/test/rgw/test_rgw_iam_policy.cc b/src/test/rgw/test_rgw_iam_policy.cc
index 3160cf6887ad..e33f15f8223e 100644
--- a/src/test/rgw/test_rgw_iam_policy.cc
+++ b/src/test/rgw/test_rgw_iam_policy.cc
@@ -1454,15 +1454,6 @@ TEST(MatchPolicy, Action)
   EXPECT_FALSE(match_policy("a:*", "a:b:c", flag)); // cannot span segments
 }
 
-TEST(MatchPolicy, Resource)
-{
-  constexpr auto flag = MATCH_POLICY_RESOURCE;
-  EXPECT_TRUE(match_policy("a:b:c", "a:b:c", flag));
-  EXPECT_FALSE(match_policy("a:b:c", "A:B:C", flag)); // case sensitive
-  EXPECT_TRUE(match_policy("a:*:e", "a:bcd:e", flag));
-  EXPECT_TRUE(match_policy("a:*", "a:b:c", flag)); // can span segments
-}
-
 TEST(MatchPolicy, ARN)
 {
   constexpr auto flag = MATCH_POLICY_ARN;
@@ -1472,15 +1463,6 @@ TEST(MatchPolicy, ARN)
   EXPECT_FALSE(match_policy("a:*", "a:b:c", flag)); // cannot span segments
 }
 
-TEST(MatchPolicy, String)
-{
-  constexpr auto flag = MATCH_POLICY_STRING;
-  EXPECT_TRUE(match_policy("a:b:c", "a:b:c", flag));
-  EXPECT_FALSE(match_policy("a:b:c", "A:B:C", flag)); // case sensitive
-  EXPECT_TRUE(match_policy("a:*:e", "a:bcd:e", flag));
-  EXPECT_TRUE(match_policy("a:*", "a:b:c", flag)); // can span segments
-}
-
 Action_t set_range_bits(std::uint64_t start, std::uint64_t end)
 {
   Action_t result;