Skip to content

Commit 3674e92

Browse files
authored
Merge pull request ceph#57629 from clwluvw/policy-remove-self-access
rgw: implement ConfirmRemoveSelfBucketAccess header for bucket policy Reviewed-by: Adam Emerson <[email protected]>
2 parents 8890c0f + 753fcff commit 3674e92

File tree

8 files changed

+94
-8
lines changed

8 files changed

+94
-8
lines changed

PendingReleaseNotes

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@
9797
redistribution. In contrast, a stretch rule with a single-take configuration
9898
will not cause any data movement during the upgrade process.
9999

100+
* RGW: The `x-amz-confirm-remove-self-bucket-access` header is now supported by
101+
`PutBucketPolicy`. Additionally, the root user will always have access to modify
102+
the bucket policy, even if the current policy explicitly denies access.
103+
100104
>=19.2.1
101105

102106
* CephFS: Command `fs subvolume create` now allows tagging subvolumes through option

src/rgw/rgw_auth.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,13 @@ static auto transform_old_authinfo(const RGWUserInfo& user,
246246
return match_owner(o, id, account);
247247
}
248248

249+
bool is_root() const override {
250+
if (account)
251+
return get_identity_type() == TYPE_ROOT;
252+
253+
return get_perm_mask() == RGW_PERM_FULL_CONTROL;
254+
}
255+
249256
bool is_identity(const Principal& p) const override {
250257
if (p.is_wildcard()) {
251258
return true;
@@ -840,6 +847,11 @@ bool rgw::auth::RemoteApplier::is_owner_of(const rgw_owner& o) const
840847
return info.acct_user == *uid;
841848
}
842849

850+
bool rgw::auth::RemoteApplier::is_root() const
851+
{
852+
return get_perm_mask() == RGW_PERM_FULL_CONTROL;
853+
}
854+
843855
bool rgw::auth::RemoteApplier::is_identity(const Principal& p) const {
844856
// We also need to cover cases where rgw_keystone_implicit_tenants
845857
// was enabled.
@@ -1062,6 +1074,14 @@ bool rgw::auth::LocalApplier::is_owner_of(const rgw_owner& o) const
10621074
return match_owner(o, user_info.user_id, account);
10631075
}
10641076

1077+
bool rgw::auth::LocalApplier::is_root() const
1078+
{
1079+
if (account)
1080+
return get_identity_type() == TYPE_ROOT;
1081+
1082+
return get_perm_mask() == RGW_PERM_FULL_CONTROL;
1083+
}
1084+
10651085
bool rgw::auth::LocalApplier::is_identity(const Principal& p) const {
10661086
if (p.is_wildcard()) {
10671087
return true;

src/rgw/rgw_auth.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,15 @@ class Identity {
5656
* On internal error throws rgw::auth::Exception storing the reason. */
5757
virtual bool is_owner_of(const rgw_owner& o) const = 0;
5858

59+
/* Verify whether a given identity is the root user. */
60+
virtual bool is_root() const = 0;
61+
62+
/* Verify whether a given identity is the root user and the owner of the
63+
* rgw_owner specified in @o. */
64+
virtual bool is_root_of(const rgw_owner& o) const {
65+
return is_root() && is_owner_of(o);
66+
}
67+
5968
/* Return the permission mask that is used to narrow down the set of
6069
* operations allowed for a given identity. This method reflects the idea
6170
* of subuser tied to RGWUserInfo. On error throws rgw::auth::Exception
@@ -477,6 +486,10 @@ class WebIdentityApplier : public IdentityApplier {
477486

478487
bool is_owner_of(const rgw_owner& o) const override;
479488

489+
bool is_root() const override {
490+
return false;
491+
}
492+
480493
uint32_t get_perm_mask() const override {
481494
return RGW_PERM_NONE;
482495
}
@@ -653,6 +666,7 @@ class RemoteApplier : public IdentityApplier {
653666
uint32_t get_perms_from_aclspec(const DoutPrefixProvider* dpp, const aclspec_t& aclspec) const override;
654667
bool is_admin_of(const rgw_owner& o) const override;
655668
bool is_owner_of(const rgw_owner& o) const override;
669+
bool is_root() const override;
656670
bool is_identity(const Principal& p) const override;
657671

658672
uint32_t get_perm_mask() const override { return info.perm_mask; }
@@ -718,6 +732,7 @@ class LocalApplier : public IdentityApplier {
718732
uint32_t get_perms_from_aclspec(const DoutPrefixProvider* dpp, const aclspec_t& aclspec) const override;
719733
bool is_admin_of(const rgw_owner& o) const override;
720734
bool is_owner_of(const rgw_owner& o) const override;
735+
bool is_root() const override;
721736
bool is_identity(const Principal& p) const override;
722737
uint32_t get_perm_mask() const override {
723738
if (this->perm_mask == RGW_PERM_INVALID) {
@@ -798,6 +813,9 @@ class RoleApplier : public IdentityApplier {
798813
return false;
799814
}
800815
bool is_owner_of(const rgw_owner& o) const override;
816+
bool is_root() const override {
817+
return false;
818+
}
801819
bool is_identity(const Principal& p) const override;
802820
uint32_t get_perm_mask() const override {
803821
return RGW_PERM_NONE;

src/rgw/rgw_auth_filters.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ class DecoratedApplier : public rgw::auth::IdentityApplier {
8181
return get_decoratee().is_owner_of(o);
8282
}
8383

84+
bool is_root() const override {
85+
return get_decoratee().is_root();
86+
}
87+
8488
bool is_anonymous() const override {
8589
return get_decoratee().is_anonymous();
8690
}

src/rgw/rgw_common.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,11 @@ using ceph::crypto::MD5;
165165
#define RGW_ATTR_OBJ_REPLICATION_TIMESTAMP RGW_ATTR_PREFIX "replicated-at"
166166

167167
/* IAM Policy */
168-
#define RGW_ATTR_IAM_POLICY RGW_ATTR_PREFIX "iam-policy"
169-
#define RGW_ATTR_USER_POLICY RGW_ATTR_PREFIX "user-policy"
170-
#define RGW_ATTR_MANAGED_POLICY RGW_ATTR_PREFIX "managed-policy"
171-
#define RGW_ATTR_PUBLIC_ACCESS RGW_ATTR_PREFIX "public-access"
168+
#define RGW_ATTR_IAM_POLICY RGW_ATTR_PREFIX "iam-policy"
169+
#define RGW_ATTR_USER_POLICY RGW_ATTR_PREFIX "user-policy"
170+
#define RGW_ATTR_MANAGED_POLICY RGW_ATTR_PREFIX "managed-policy"
171+
#define RGW_ATTR_PUBLIC_ACCESS RGW_ATTR_PREFIX "public-access"
172+
#define RGW_ATTR_IAM_POLICY_REMOVE_SELF_ACCESS RGW_ATTR_PREFIX "iam-policy-remove-self-access"
172173

173174
/* RGW File Attributes */
174175
#define RGW_ATTR_UNIX_KEY1 RGW_ATTR_PREFIX "unix-key1"

src/rgw/rgw_op.cc

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8500,6 +8500,14 @@ void RGWPutBucketPolicy::send_response()
85008500

85018501
int RGWPutBucketPolicy::verify_permission(optional_yield y)
85028502
{
8503+
// If the user is the root account of the bucket owner,
8504+
// and x-amz-confirm-remove-self-bucket-access was not set,
8505+
// then the user can put bucket policy.
8506+
if (s->auth.identity->is_root_of(s->bucket_owner.id) &&
8507+
s->bucket_attrs.find(RGW_ATTR_IAM_POLICY_REMOVE_SELF_ACCESS) == s->bucket_attrs.end()) {
8508+
return 0;
8509+
}
8510+
85038511
auto [has_s3_existing_tag, has_s3_resource_tag] = rgw_check_policy_condition(this, s, false);
85048512
if (has_s3_resource_tag)
85058513
rgw_iam_add_buckettags(this, s);
@@ -8549,10 +8557,15 @@ void RGWPutBucketPolicy::execute(optional_yield y)
85498557
}
85508558

85518559
op_ret = retry_raced_bucket_write(this, s->bucket.get(), [&p, this, &attrs] {
8552-
attrs[RGW_ATTR_IAM_POLICY].clear();
8553-
attrs[RGW_ATTR_IAM_POLICY].append(p.text);
8554-
op_ret = s->bucket->merge_and_store_attrs(this, attrs, s->yield);
8555-
return op_ret;
8560+
attrs[RGW_ATTR_IAM_POLICY].clear();
8561+
attrs[RGW_ATTR_IAM_POLICY].append(p.text);
8562+
if (s->info.env->exists("HTTP_X_AMZ_CONFIRM_REMOVE_SELF_BUCKET_ACCESS")) {
8563+
attrs[RGW_ATTR_IAM_POLICY_REMOVE_SELF_ACCESS].clear();
8564+
} else {
8565+
attrs.erase(RGW_ATTR_IAM_POLICY_REMOVE_SELF_ACCESS);
8566+
}
8567+
op_ret = s->bucket->merge_and_store_attrs(this, attrs, s->yield);
8568+
return op_ret;
85568569
}, y);
85578570
} catch (rgw::IAM::PolicyParseException& e) {
85588571
ldpp_dout(this, 5) << "failed to parse policy: " << e.what() << dendl;
@@ -8573,6 +8586,14 @@ void RGWGetBucketPolicy::send_response()
85738586

85748587
int RGWGetBucketPolicy::verify_permission(optional_yield y)
85758588
{
8589+
// If the user is the root account of the bucket owner,
8590+
// and x-amz-confirm-remove-self-bucket-access was not set,
8591+
// then the user can put bucket policy.
8592+
if (s->auth.identity->is_root_of(s->bucket_owner.id) &&
8593+
s->bucket_attrs.find(RGW_ATTR_IAM_POLICY_REMOVE_SELF_ACCESS) == s->bucket_attrs.end()) {
8594+
return 0;
8595+
}
8596+
85768597
auto [has_s3_existing_tag, has_s3_resource_tag] = rgw_check_policy_condition(this, s, false);
85778598
if (has_s3_resource_tag)
85788599
rgw_iam_add_buckettags(this, s);
@@ -8622,6 +8643,14 @@ void RGWDeleteBucketPolicy::send_response()
86228643

86238644
int RGWDeleteBucketPolicy::verify_permission(optional_yield y)
86248645
{
8646+
// If the user is the root account of the bucket owner,
8647+
// and x-amz-confirm-remove-self-bucket-access was not set,
8648+
// then the user can put bucket policy.
8649+
if (s->auth.identity->is_root_of(s->bucket_owner.id) &&
8650+
s->bucket_attrs.find(RGW_ATTR_IAM_POLICY_REMOVE_SELF_ACCESS) == s->bucket_attrs.end()) {
8651+
return 0;
8652+
}
8653+
86258654
auto [has_s3_existing_tag, has_s3_resource_tag] = rgw_check_policy_condition(this, s, false);
86268655
if (has_s3_resource_tag)
86278656
rgw_iam_add_buckettags(this, s);
@@ -8645,6 +8674,7 @@ void RGWDeleteBucketPolicy::execute(optional_yield y)
86458674
op_ret = retry_raced_bucket_write(this, s->bucket.get(), [this] {
86468675
rgw::sal::Attrs& attrs = s->bucket->get_attrs();
86478676
attrs.erase(RGW_ATTR_IAM_POLICY);
8677+
attrs.erase(RGW_ATTR_IAM_POLICY_REMOVE_SELF_ACCESS);
86488678
op_ret = s->bucket->put_info(this, false, real_time(), s->yield);
86498679
return op_ret;
86508680
}, y);

src/test/rgw/test_rgw_iam_policy.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,11 @@ class FakeIdentity : public Identity {
168168
return false;
169169
}
170170

171+
bool is_root() const override {
172+
ceph_abort();
173+
return false;
174+
}
175+
171176
virtual uint32_t get_perm_mask() const override {
172177
ceph_abort();
173178
return 0;

src/test/rgw/test_rgw_lua.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ class FakeIdentity : public Identity {
5050
return false;
5151
}
5252

53+
bool is_root() const override {
54+
return false;
55+
}
56+
5357
virtual uint32_t get_perm_mask() const override {
5458
return 0;
5559
}

0 commit comments

Comments
 (0)