diff --git a/doc/radosgw/bucketpolicy.rst b/doc/radosgw/bucketpolicy.rst index 23356624a46a..0ade1b080f1c 100644 --- a/doc/radosgw/bucketpolicy.rst +++ b/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 diff --git a/doc/radosgw/keystone.rst b/doc/radosgw/keystone.rst index 068eac24ff2c..21f79e490599 100644 --- a/doc/radosgw/keystone.rst +++ b/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 --------------------- diff --git a/release-management/backports/01a16411-8df8-7e1f-baaf-8aa40a09ecdd.md b/release-management/backports/01a16411-8df8-7e1f-baaf-8aa40a09ecdd.md new file mode 100644 index 000000000000..602aaf1cd331 --- /dev/null +++ b/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. diff --git a/src/rgw/rgw_auth.cc b/src/rgw/rgw_auth.cc index 74805fbb186b..ab7cc143a030 100644 --- a/src/rgw/rgw_auth.cc +++ b/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 */ diff --git a/src/rgw/rgw_auth.h b/src/rgw/rgw_auth.h index b29f863d030f..10009c5d8f6e 100644 --- a/src/rgw/rgw_auth.h +++ b/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 keystone_scope; + const std::vector keystone_roles; + const std::string keystone_user_id; public: enum class acct_privilege_t { @@ -609,7 +611,10 @@ class RemoteApplier : public IdentityApplier { const std::string access_key_id, const std::string subuser, const uint32_t acct_type=TYPE_NONE, - std::optional keystone_scope=std::nullopt) + std::optional keystone_scope=std::nullopt, + std::vector keystone_roles = {}, + const std::string keystone_user_id = {} + ) : acct_user(acct_user), acct_name(acct_name), perm_mask(perm_mask), @@ -617,7 +622,9 @@ class RemoteApplier : public IdentityApplier { 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) { } }; diff --git a/src/rgw/rgw_auth_keystone.cc b/src/rgw/rgw_auth_keystone.cc index e0678e6fb9f5..2185efd81082 100644 --- a/src/rgw/rgw_auth_keystone.cc +++ b/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 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 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() }; } diff --git a/src/test/rgw/test_rgw_iam_policy.cc b/src/test/rgw/test_rgw_iam_policy.cc index b7c9eb229d4c..ec6228df7c47 100644 --- a/src/test/rgw/test_rgw_iam_policy.cc +++ b/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 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"}})); +} \ No newline at end of file