rgw: prevent data sync from replicating to buckets not owned by the user#61828
rgw: prevent data sync from replicating to buckets not owned by the user#61828
Conversation
Issue https://tracker.ceph.com/issues/68884 revealed that because user_acl is initialized by default in RGWUserPermHandler::Init with the same identity, calling verify_bucket_permission_no_policy() would mistakenly allow the request since the user ACL matches the identity. Removing the default creation of user_acl would align the behavior with other S3 operations to prevent unauthorized data replication. Fixes: https://tracker.ceph.com/issues/69972 Signed-off-by: Seena Fallah <[email protected]>
|
what would it take to get test coverage for this stuff? cc @smanjara |
I guess with the current test setup testing sync policies would be tricky as the cluster is running with symmetric replication rules so all buckets are replicated to other zones with system users. I'm not sure how it would react if we apply a policy on a bucket while the global policy is something else. basically, that is what we wanted to clarify by the design doc you wanted to propose so we can make a line here eventually. So my preference is to add all this coverage for sync policies after the design is there and perhaps would be only for zonegroup replication. |
to cover this pr and also #60685 we could keep the zonegroup policy in allowed state, build whatever bucket sync policies you need, including user creation and attaching policies to it to test user mode sync, and apply it to just that testcase and tear it down at the end using |
|
jenkins test api |
|
thanks for adding the testcases. looks great! |
qa/tasks/rgw_multisite_tests.py
Outdated
| arg = ['--account-id', user.account] | ||
| arg += master_zone.zone_args() | ||
| master_zone.cluster.admin(['account', 'create'] + arg) | ||
| user = multisite.User('rgw-multisite-test-user') |
There was a problem hiding this comment.
this removed account support and broke account-related tests:
ERROR: rgw_multi.tests.test_role_sync
ERROR: rgw_multi.tests.test_role_delete_sync
ERROR: rgw_multi.tests.test_account_metadata_sync
each failed with an error like this:
botocore.exceptions.ClientError: An error occurred (AccessDenied) when calling the CreateRole operation: None
There was a problem hiding this comment.
Thanks, I have two possible solutions for this:
- We retain the default user as accounted and create two additional users—let's call them
non_account_userandnon_account_alt_user—to use forPutBucketReplicationtests. - We could utilize the
Rolefield inBucketReplicationConfigurationwith a non-role ARN (though this might not be the preferred approach). When the user is associated with an account, we prompt for a UID from their account, check if the UID belongs to the account, and use it as the UID (not sure if anything would break if suddenly that UID is deleted and/or shifted to another account). This would allow us to bypass the-ERR_NOT_IMPLEMENTEDhere: ad51353, eliminating the need for differentiation at this point.
What are your thoughts on these options?
There was a problem hiding this comment.
I took the first approach - leaving the API sane. let me know if you think otherwise.
Create a user and alt user without account to test PutBucketReplication scenarios as account is not supported to call that API yet. Signed-off-by: Seena Fallah <[email protected]>
Test policies created through PutBucketReplication API, whether they respect the uid used in the policies. Signed-off-by: Seena Fallah <[email protected]>
clwluvw
force-pushed
the
datasync-useracl
branch
from
241f217 to
a3f45c7
Compare
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
3 participants
Issue https://tracker.ceph.com/issues/68884 revealed that because user_acl is initialized by default in RGWUserPermHandler::Init with the same identity, calling verify_bucket_permission_no_policy() would mistakenly allow the request since the user ACL matches the identity. Removing the default creation of user_acl would align the behavior with other S3 operations (leaving user_acl uninitialized) to prevent unauthorized data replication.
Fixes: https://tracker.ceph.com/issues/69972