Skip to content

Comments

tentacle: rgw/sts: correcting authentication in case s3 ops are directed to a primary from secondary after assumerole.#63043

Merged
cbodley merged 32 commits intoceph:tentaclefrom
cbodley:wip-71115-tentacle
May 1, 2025
Merged

tentacle: rgw/sts: correcting authentication in case s3 ops are directed to a primary from secondary after assumerole.#63043
cbodley merged 32 commits intoceph:tentaclefrom
cbodley:wip-71115-tentacle

Conversation

in case the request is forwarded from secondary in
a multi-site setup. authenticating with the system
user creds of which are used to sign the request.
Permissions are still derived from the role.

Signed-off-by: Pritha Srivastava <[email protected]>
(cherry picked from commit 63bc738)
syncs, and then creating a bucket on both primary and secondary.
The test name is test_assume_role_after_sync.

Signed-off-by: Pritha Srivastava <[email protected]>
(cherry picked from commit 855db87)
@cbodley cbodley requested a review from a team as a code owner April 29, 2025 14:21
@cbodley cbodley added this to the tentacle milestone Apr 29, 2025
@cbodley cbodley added the rgw label Apr 29, 2025
@github-actions github-actions bot added the tests label Apr 29, 2025
clwluvw added 24 commits April 29, 2025 10:22
So it can be imported by headers like rgw_cr_rados.h that already
has dependency to rgw_data_sync.h.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit e742295)
So it can be called by RGWAsyncRadosRequest classes not holding
sync_env.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 77c9304)
So it can be used by others.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 857f7bd)
Check for s3:ReplicateDelete for replicating object deletes and
delete markers when pipe is set to user mode.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit d7fe791)
…cation

Instead of s3:PutObject rely on s3:s3ReplicateObject permission to
check whether the user can replicate to the destination bucket.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 97ee328)
Check for tag replication permission on destination bucket, so if
there was an explicit deny, donot include tags in the replicated
object.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 3fb1671)
Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 3f2514f)
SysReqApplier now returns true for is_admin_of() when the requester
was a system user and was not impersonating any user/role using
rgwx-perm-check-uid or rgwx-perm-check-role.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 0e650ea)
Since multisite now delegates permission checks for source objects
to the source zone (a3f40b4), we need to avoid allowing system-level
overrides when the request is impersonating another identity.

SysReqApplier should only grant override permission if the request
is truly system-authenticated and not acting on behalf of another
user or role (i.e., no rgwx-perm-check-uid or rgwx-perm-check-role
in the request).

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 2a0cb65)
Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit f2ba4db)
Check for permissions of `s3:GetObjectVersionForReplication` in
addition to `s3:GetObject` and `s3:GetObjectVersion` when fetching
the object for multisite.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 89d92de)
Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 4fde9dd)
Check whether the policies are honored on source object in source
zone when replicating.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit e4f4485)
To replicate objects encrypted via sse-kms objects,
s3:GetObjectVersionForReplication is required.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 3024b70)
Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 86aa6d3)
Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 0a93e74)
Return `Rgwx-Perm-Checked` header as a hint for the destination zone
to know whether the perms where considered or not.
This is just a backward compatibility for upgrade and can be dropped
in T+2 release.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 84a8d1b)
Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit b0200c6)
As of a3f40b4 we no longer evaluate perms locally for source bucket,
this could cause broken permission evaluation dusring upgrade as one
zone is not respecting the perm evaluation based on the `rgwx-perm-check-uid`
arg.

This can be dropped in T+2 release.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 926ed16)
When copying object from remote source (bucket from another zonegroup)
the perms of the source is not evaluated resulting in reading from
unauthorized buckets.
passing `rgwx-perm-check-uid` will let the source zone evaluates the
perm and close this bug.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 3c83520)
Make sure perms are evaluated properly for the source object.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 9523e15)
clwluvw added 6 commits April 29, 2025 10:22
In rgw_sync_pipe_params, the mode can be either system or user.
When in system mode, no user is involved, but the current
implementation holds an empty rgw_user, which can cause confusion
in pipe_rules::find_basic_info_without_tags().

With this change, rgw_user is now optional, ensuring that when no
user is involved, it is explicitly nullopt rather than an empty object.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit c8aca21)
As admin propery of a user is something global and nothing related
to any other owner, we don't need any comparision.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 1a253ea)
If pipe is in user mode and the user is admin, don't check for perms
and let it go.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 97b4b60)
Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit cc033cb)
rely on s->system_request to skip rate limiting on forwarded requests
as well as normal system user requests.

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 004ccc7)
With the introduction of is_impersonating in SysReqApplier,
RoleApplier can now use the same mechanism to mark when a request
has been forwarded by a system user on behalf of another role (e.g.,
through STS) to mark it as a system request (s->system_request).

Signed-off-by: Seena Fallah <[email protected]>
(cherry picked from commit 995dc62)
Copy link
Contributor

@pritha-srivastava pritha-srivastava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clwluvw @cbodley : just to understand the change here, the code in modify_request_state if (is_system_request) { s->system_request = true; } (commit: 3111561) will be addressed by cedcb37

@cbodley
Copy link
Contributor Author

cbodley commented Apr 30, 2025

@clwluvw @cbodley : just to understand the change here, the code in modify_request_state if (is_system_request) { s->system_request = true; } (commit: 3111561) will be addressed by cedcb37

@pritha-srivastava correct. the difference with bool is_impersonating is that rgw will still perform permission checks against the wrapped RoleApplier. before we had always bypassed them for system requests

@cbodley cbodley merged commit 8be7d01 into ceph:tentacle May 1, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants