Backport/tentacle 20.2.4 cobaltcore storage/keystone iam policy

doc/radosgw/bucketpolicy.rst

@@ -122,6 +122,11 @@ For all requests, condition keys we support are:
 - aws:UserAgent
 - aws:username
 
+Request that authenticate with Keystone also include:
+
+- keystone:role
+- keystone:userid
+
 We support certain s3 condition keys for bucket and object requests.
 
 *Support for the following bucket-related operations was added in the Mimic

doc/radosgw/keystone.rst

@@ -159,6 +159,19 @@ S3 API (with AWS-like access and secret keys), if the ``rgw s3 auth
 use keystone`` option is set. For details, see
 :doc:`s3/authentication`.
 
+Requests authenticated via Keystone (Swift tokens or S3 with Keystone-managed
+credentials) expose the Keystone idenitity in the IAM policy environment
+as the condition keys: ``keystone:role`` (role names) and
+``keystone:userid`` (user UUID). These conditions can be used in bucket
+policies and idenitity policies with ``StringEquals``, ``StringNotEquals`` etc.
+
+- **keystone:role** - Allow or deny by *role* (e.g only users with role
+  ``reader`` get read access). Use for RBAC.
+- **keystone:userid** - Restrict to specific *user*. Use when policy
+  depends on a specific user, not just their role.
+
+See :doc:`bucketpolicy` for list of supported condition keys.
+
 Service token support
 ---------------------
 

release-management/backports/01a16411-8df8-7e1f-baaf-8aa40a09ecdd.md

@@ -0,0 +1,80 @@
+---
+id: 01a16411-8df8-7e1f-baaf-8aa40a09ecdd
+title: 'IAM / Bucket Policy: Keystone role and user ID support'
+component: rgw
+category: feature
+provenance:
+  type: upstream_pr
+  upstream_prs:
+  - https://github.com/ceph/ceph/pull/67752
+  upstream_pr_state: merged-main
+  upstream_commits:
+  - b06b7a82ed1a5c57dce8b9ce2fa0931fe0f55ff7
+  previous_backport: 01a0cdb3-3f84-74ab-9403-c7638a039697
+justification:
+  type: customer
+  refs:
+  - cobaltcore-dev/cloud-storage#566
+  - cobaltcore-dev/cloud-storage#565
+  - cobaltcore-dev/cloud-storage#380
+  - cobaltcore-dev/cloud-storage#372
+risk:
+  blast: cosmetic
+  conflict: trivial
+  coverage: strong
+---
+
+## Justification (public)
+
+Allows Keystone role names and user IDs to be injected as `keystone:role` and
+`keystone:userid` condition keys in S3 bucket policies, enabling role-based and
+per-user access control via standard IAM policy conditions.
+
+## Justification (internal)
+
+Improves keystone authorization features.
+
+## Risk notes
+
+**blast: cosmetic** — purely additive. Two new condition keys are populated for
+Keystone-authenticated requests. No existing condition key, policy or auth path
+changes meaning, and non-Keystone requests are untouched.
+
+**conflict: trivial**
+
+**coverage: strong** — 
+47/47 unit tests pass in `unittest_rgw_iam_policy`.
+14/14 live checks pass against vstart RGW and a real Keystone, with two
+multi-role users in one project: `keystone:role` matching (single value,
+multi-value, shared role, absent role), case handling (`StringEquals` is
+case-sensitive, `StringEqualsIgnoreCase` is not), `keystone:userid` matching per
+user, `StringLike` wildcards, and three `StringNotEquals` cases that confirm the
+AND semantics with multiple roles.
+7/7 live grant checks pass with non-owner users from a second project: `Allow`
+on `keystone:role` or `keystone:userid` grants exactly the listed read, list,
+write and delete actions, users without the role or ID get nothing, and `Deny`
+overrides `Allow`.
+
+## Behavior change
+
+Requests authenticated via Keystone now expose two additional IAM policy
+condition keys: `keystone:role` (one entry per role, lowercased) and
+`keystone:userid` (the Keystone user UUID). Requests not authenticated via
+Keystone are unchanged, and no existing policy changes behaviour.
+
+### Two points for policy authors
+
+Role names in policies are case-sensitive. Roles are lowercased on the way into
+the policy environment, so how the role is spelled in Keystone does not matter,
+but the value typed in the policy is compared exactly. A policy containing
+`"TestRole"` never matches and gives no warning. Write role names in lower case,
+or use `StringEqualsIgnoreCase`.
+
+Rules that distinguish between users of the same project must be written as
+`Deny`. All users in an OpenStack project share a single RGW account, so each is
+an owner of buckets created there, and owners are not restricted by `Allow`
+conditions. `Deny` statements do apply to owners and discriminate correctly.
+
+## Upgrade notes
+
+None.

src/rgw/rgw_auth.cc

@@ -1033,6 +1033,18 @@ void rgw::auth::RemoteApplier::modify_request_state(const DoutPrefixProvider* dp
   // copy our identity policies into req_state
   s->iam_identity_policies.insert(s->iam_identity_policies.end(),
                                   policies.begin(), policies.end());
+
+  for (auto role : this->info.keystone_roles) {
+    // Keystone roles are case-insensitive. Normalize the roles to
+    // lowercase before placing them into the environment.
+    std::transform(role.begin(), role.end(), role.begin(),
+      [](unsigned char c){ return std::tolower(c); });
+    s->env.emplace("keystone:role", std::move(role));
+  }
+
+  if (!info.keystone_user_id.empty()) {
+    s->env.emplace("keystone:userid", info.keystone_user_id);
+  }
 }
 
 /* rgw::auth::LocalApplier */

src/rgw/rgw_auth.h

@@ -592,6 +592,8 @@ class RemoteApplier : public IdentityApplier {
     const std::string access_key_id;
     const std::string subuser;
     const std::optional<rgw::keystone::ScopeInfo> keystone_scope;
+    const std::vector<std::string> keystone_roles;
+    const std::string keystone_user_id;
 
   public:
     enum class acct_privilege_t {
@@ -609,15 +611,20 @@ class RemoteApplier : public IdentityApplier {
              const std::string access_key_id,
              const std::string subuser,
              const uint32_t acct_type=TYPE_NONE,
-             std::optional<rgw::keystone::ScopeInfo> keystone_scope=std::nullopt)
+             std::optional<rgw::keystone::ScopeInfo> keystone_scope=std::nullopt,
+             std::vector<std::string> keystone_roles = {},
+             const std::string keystone_user_id = {}
+            )
     : acct_user(acct_user),
       acct_name(acct_name),
       perm_mask(perm_mask),
       is_admin(acct_privilege_t::IS_ADMIN_ACCT == level),
       acct_type(acct_type),
       access_key_id(access_key_id),
       subuser(subuser),
-      keystone_scope(std::move(keystone_scope)) {
+      keystone_scope(std::move(keystone_scope)),
+      keystone_roles(std::move(keystone_roles)),
+      keystone_user_id(keystone_user_id) {
     }
   };
 

src/rgw/rgw_auth_keystone.cc

@@ -144,13 +144,14 @@ TokenEngine::get_creds_info(const TokenEngine::token_envelope_t& token
                            ) const noexcept
 {
   using acct_privilege_t = rgw::auth::RemoteApplier::AuthInfo::acct_privilege_t;
+  std::vector<std::string> role_names;
 
   /* Check whether the user has an admin status. */
   acct_privilege_t level = acct_privilege_t::IS_PLAIN_ACCT;
   for (const auto& role : token.roles) {
+    role_names.push_back(role.name);
     if (role.is_admin && !role.is_reader) {
       level = acct_privilege_t::IS_ADMIN_ACCT;
-      break;
     }
   }
 
@@ -169,8 +170,10 @@ TokenEngine::get_creds_info(const TokenEngine::token_envelope_t& token
     rgw::auth::RemoteApplier::AuthInfo::NO_ACCESS_KEY,
     rgw::auth::RemoteApplier::AuthInfo::NO_SUBUSER,
     TYPE_KEYSTONE,
-    std::move(keystone_scope)
-};
+    std::move(keystone_scope),
+    std::move(role_names),
+    token.get_user_id()
+  };
 }
 
 static inline const std::string
@@ -673,6 +676,11 @@ EC2Engine::get_creds_info(const EC2Engine::token_envelope_t& token,
   /* Build keystone scope info if ops logging is enabled */
   auto keystone_scope = rgw::keystone::build_scope_info(cct, token);
 
+  std::vector<std::string> role_names;
+  for (const auto& role : token.roles) {
+    role_names.push_back(role.name);
+  }
+
   return auth_info_t {
     /* Suggested account name for the authenticated user. */
     rgw_user(token.get_project_id()),
@@ -685,7 +693,9 @@ EC2Engine::get_creds_info(const EC2Engine::token_envelope_t& token,
     access_key_id,
     rgw::auth::RemoteApplier::AuthInfo::NO_SUBUSER,
     TYPE_KEYSTONE,
-    std::move(keystone_scope)
+    std::move(keystone_scope),
+    std::move(role_names),
+    token.get_user_id()
   };
 }
 

src/test/rgw/test_rgw_iam_policy.cc

@@ -1779,3 +1779,130 @@ TEST_F(ConditionTest, Null)
     EXPECT_TRUE(notNull.eval({{key, "admin/config.txt"}}));
   }
 }
+
+TEST_F(ConditionTest, KeyStoneRoleStringEquals)
+{
+  std::string key = "keystone:role";
+
+  Condition cond{TokenID::StringEquals, key.data(), key.size(), false};
+  cond.vals.push_back("testrole");
+
+  // No roles
+  EXPECT_FALSE(cond.eval({}));
+
+  // Single matching role
+  EXPECT_TRUE(cond.eval({{key, "testrole"}}));
+
+  // Single non-matching role
+  EXPECT_FALSE(cond.eval({{key, "member"}}));
+
+  //Multiple roles in env,one matches
+  Environment multi_env;
+  multi_env.emplace(key, "member");
+  multi_env.emplace(key, "testrole");
+  multi_env.emplace(key, "reader");
+  EXPECT_TRUE(cond.eval(multi_env));
+
+  //Multiple roles in env, no match
+  Environment no_match_env;
+  no_match_env.emplace(key, "member");
+  no_match_env.emplace(key, "reader");
+  EXPECT_FALSE(cond.eval(no_match_env));
+
+  // Multiple identical roles (redundancy check)
+  Environment duplicate_env;
+  duplicate_env.emplace(key, "testrole");
+  duplicate_env.emplace(key, "testrole");
+  EXPECT_TRUE(cond.eval(duplicate_env));
+}
+
+TEST_F(ConditionTest, KeyStoneRoleNotStringEquals)
+{
+  std::string key = "keystone:role";
+
+  Condition cond{TokenID::StringNotEquals, key.data(), key.size(), false};
+  cond.vals.push_back("admin");
+
+  // No roles
+  EXPECT_FALSE(cond.eval({}));
+
+  // Role matches
+  EXPECT_FALSE(cond.eval({{key, "admin"}}));
+
+  // Role doesn't match
+  EXPECT_TRUE(cond.eval({{key, "member"}}));
+
+  // Multiple roles in env, one matches -> false
+  Environment multi_env;
+  multi_env.emplace(key, "member");
+  multi_env.emplace(key, "admin");
+  EXPECT_FALSE(multi_env.count(key) == 0);
+  EXPECT_FALSE(cond.eval(multi_env));
+
+  // Multiple roles, none match -> true
+  Environment no_match_env;
+  no_match_env.emplace(key, "member");
+  no_match_env.emplace(key, "reader");
+  EXPECT_TRUE(cond.eval(no_match_env));
+}
+
+TEST_F(ConditionTest, KeyStoneRolePolicyParsing)
+{
+  string keystone_role_policy = R"(
+  {
+    "Version": "2012-10-17",
+    "Statement": [{
+      "Effect" : "Allow",
+      "Principal": "*",
+      "Action": "s3:GetObject",
+      "Resource": [
+        "arn:aws:s3:::example_bucket/*"
+      ],
+      "Condition": {
+        "StringEquals": {
+          "keystone:role": "testrole"
+        }
+      }
+    }]
+  }
+  )";
+
+  string tenant = "arbitrary_tenant";
+  boost::optional<Policy> p;
+
+  ASSERT_NO_THROW(
+    p = Policy(cct.get(), &tenant, keystone_role_policy, true));
+  ASSERT_TRUE(p);
+  EXPECT_EQ(p->statements.size(), 1U);
+  EXPECT_EQ(p->statements[0].conditions.size(), 1U);
+  EXPECT_EQ(p->statements[0].conditions[0].key, "keystone:role");
+  EXPECT_EQ(p->statements[0].conditions[0].vals.size(), 1U);
+  EXPECT_EQ(p->statements[0].conditions[0].vals[0], "testrole");
+
+  // Eval with matching role in environment
+  Environment match_env;
+  match_env.emplace("keystone:role", "testrole");
+  EXPECT_TRUE(p->statements[0].conditions[0].eval(match_env));
+
+  // Eval with non-matching role
+  Environment nomatch_env;
+  nomatch_env.emplace("keystone:role", "member");
+  EXPECT_FALSE(p->statements[0].conditions[0].eval(nomatch_env));
+
+  // Eval with multiple roles, one matching
+  Environment multi_env;
+  multi_env.emplace("keystone:role", "member");
+  multi_env.emplace("keystone:role", "testrole");
+  EXPECT_TRUE(p->statements[0].conditions[0].eval(multi_env));
+}
+
+TEST_F(ConditionTest, KeystoneUserIdStringEquals)
+{
+  const std::string key = "keystone:userid";
+  Condition cond{TokenID::StringEquals, key.data(), key.size(), false};
+  cond.vals.push_back("user-123");
+
+  EXPECT_FALSE(cond.eval({}));
+  EXPECT_TRUE(cond.eval({{key, "user-123"}}));
+  EXPECT_FALSE(cond.eval({{key, "user-456"}}));
+}