Skip to content

doc/dev/rgw: add design document for iam user accounts#54045

Closed
cbodley wants to merge 3 commits intoceph:mainfrom
cbodley:wip-rgw-doc-account
Closed

doc/dev/rgw: add design document for iam user accounts#54045
cbodley wants to merge 3 commits intoceph:mainfrom
cbodley:wip-rgw-doc-account

Conversation

@cbodley
Copy link
Contributor

@cbodley cbodley commented Oct 16, 2023

formalizes an initial design by Abhishek Lekshmanan on the dev list in https://lists.ceph.io/hyperkitty/list/dev@ceph.io/message/SX37W3TBBLJCKUOBDEYDXCJYQK4JAHFQ/

also proposes a special format for account ids to preserve backward-compatibility with tenant names in policy principals

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

formalizes an initial design by Abhishek Lekshmanan on the dev list in
https://lists.ceph.io/hyperkitty/list/dev@ceph.io/message/SX37W3TBBLJCKUOBDEYDXCJYQK4JAHFQ/

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley cbodley requested a review from a team October 16, 2023 16:39
@cbodley cbodley added the rgw label Oct 16, 2023

there exists no 'tenant' metadata that we can attach policy to, index users or buckets on, or accumulate usage stats for reporting or quota enforcement. features like this will instead be attached to the proposed account metadata

to preserve this namespacing functionality, each account can optionally be associated with a tenant name. all users and roles in that account must have that same tenant name
Copy link
Contributor

Choose a reason for hiding this comment

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

Can multiple accounts have the same tenant? Or is tenant a 1-to-1 with an account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's fine for several accounts to share a tenant name, the same way accounts can share the global (empty tenant) namespace

Copy link
Contributor

Choose a reason for hiding this comment

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

for ease of understanding, we should clarify the implications of accounts sharing a tenant and maybe why someone would do it--iirc, @mmgaggle has described the ability to maintain a hierarchy of accounts in aws, and I don't think we're bringing all that detail in (e.g.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should clarify the implications of accounts sharing a tenant and maybe why someone would do it

you'd use multiple tenants if you want namespace isolation, and multiple accounts if you want isolation of user/role/policy management

if you want a global dns namespace like aws, put all accounts in the global empty-tenant namespace

iirc, @mmgaggle has described the ability to maintain a hierarchy of accounts in aws, and I don't think we're bringing all that detail in (e.g.)

interested to hear more about that requirement and how it relates to tenant namespacing. naively, it sounds like we'd just want an account to have an optional 'parent' account that it can inherit some policy from

Copy link
Contributor Author

Choose a reason for hiding this comment

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

iirc, @mmgaggle has described the ability to maintain a hierarchy of accounts in aws, and I don't think we're bringing all that detail in (e.g.)

interested to hear more about that requirement and how it relates to tenant namespacing. naively, it sounds like we'd just want an account to have an optional 'parent' account that it can inherit some policy from

in aws, it's the AWS Organizations API that's responsible for the grouping of related accounts and the management of policy like service control policy:

https://docs.aws.amazon.com/organizations/latest/APIReference/Welcome.html
https://docs.aws.amazon.com/organizations/latest/userguide/orgs_introduction.html
https://docs.aws.amazon.com/organizations/latest/userguide/orgs_manage_policies_scps.html

maybe we could find some subset of that Organizations API that would accomplish our goals here, instead of adding special custom behaviors to the accounts themselves?

Copy link
Contributor

@likid0 likid0 Feb 15, 2024

Choose a reason for hiding this comment

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

I agree @cbodley; investigating, if using a subset of the OUs API could help us introduce a similar concept as to what AWS does with Control Tower, specifically with what they call preventive controls, which looks like it's enforcing SCPs at the OU level , perhaps within organizations, we could introduce templates for different policy/configurations and rules for enforcing them.

@cbodley cbodley requested a review from mmgaggle October 20, 2023 15:56
@mattbenjamin mattbenjamin self-requested a review October 21, 2023 15:11
@@ -55,6 +55,19 @@ this requires the account metadata to also manage the index of all buckets under
we should be able to use the same cls_user calls for `{accountid}.buckets` that we already use for `{userid}.buckets`. so if we solve this scaling problem for users, we could apply the same solution to accounts
Copy link
Contributor

Choose a reason for hiding this comment

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

this scaling discussion drops down to details of the RADOS implementation, but as of today, I think we need to also consider alternate stores; are there any aspects of this design that aren't trivially covered by zipper abstractions and compatible with dbstore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

are there any aspects of this design that aren't trivially covered by zipper abstractions and compatible with dbstore?

nope! sal has interfaces to read/write the quota limits on buckets/users, and a Bucket::check_quota() that's responsible for quota enforcement. backends can implement the quota accounting for that however they choose

this design goes into details of the rados backend because it's the only one that implements quota

@cbodley
Copy link
Contributor Author

cbodley commented Nov 3, 2023

revisiting the ACL docs in https://docs.aws.amazon.com/AmazonS3/latest/userguide/acl-overview.html, i see that their acls refer to "the AWS account's canonical user ID" rather than specific user ids like rgw. it seems that we'll need to add a random 'canonical user id' to the rgw account metadata for use here (unless we want to use the account id directly?)

@cbodley cbodley mentioned this pull request Nov 3, 2023
52 tasks

## iam groups and group policy?

we don't have any support for groups yet. is there interest in groups and group policy? this seems relatively easy to add as future work
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, I do think that IAM groups can help when implementing/operating IAM accounts at-scale, as an operator/admin I can configure, all the required policies in IAM groups, and then add the IAM users to the different groups as needed, instead of applying policy per IAM user, so it could come in handy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the iam rest apis for Group look very similar to User and Role, so we should be able to share lots of code there. then we'd need AddUserToGroup/RemoveUserFromGroup actions to manage group memberships in RGWUserInfo, parse group principals in iam policy documents, and match group principals to the authenticated user. past that, i'm not sure what else is missing

i don't think group support needs to be part of the 'mvp' for accounts, but we can track the design here for followup work

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I agree @cbodley , no need for groups in the MVP

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i did end up implementing the Group and GroupPolicy apis in #54333

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Jan 29, 2024
@cbodley cbodley removed the stale label Jan 30, 2024

## namespacing

rgw supports namespace isolation of users and buckets using the 'tenant' name, such that two tenants A and B can both have buckets named 'example-bucket', or users named 'example-user', that are completely indepedent of one another
Copy link
Contributor

Choose a reason for hiding this comment

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

@cbodley one question here, just to verify if I'm getting this right, we don't need to rely on tenants to be able to have the same user name on different accounts right?,for example: if you have two accounts, you can have users with the same name in each, but there is a single bucket namespace for everyone(both accounts will belong to the default tenant).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

essentially yes, account A can have a UserName=foo and account B can have a different user with UserName=foo. if account A and B belong to the same tenant, their users will share a common bucket namespace

however, the iam User APIs map UserName to RGWUserInfo::display_name instead of RGWUserInfo::user_id. those variables correspond to the --display-name and --uid arguments to radosgw-admin user create

it's the user_ids that we require to be unique within the tenant, and that still holds for account users - we just generate a random uuid like '550e8400-e29b-41d4-a716-446655440000' for unique user_ids

user ARNs also look different for account users: arn:aws:iam::{account_id}:user/{display_name} vs arn:aws:iam::{tenant}:user/{user_id}

so for a user in account RGW12345678901234567, an iam GetUser response would look something like this:

    <User>
      <UserId>550e8400-e29b-41d4-a716-446655440000</UserId>
      <Path>/</Path>
      <UserName>foo</UserName>
      <Arn>arn:aws:iam::RGW12345678901234567:user/foo</Arn>
      <CreateDate>2013-10-02T17:01:44Z</CreateDate>
    </User>

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation, that is clear now.


the account metadata maintains an index of its users and roles to support API operations like ListUsers and ListRoles

the bucket, user, and role metadata have a new 'account_id' field which refers to their account, if any
Copy link
Contributor

Choose a reason for hiding this comment

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

@cbodley could it also be a good idea to add the OIDC Provider as a resource with an ownership link to the account?.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, i hadn't planned for this yet. Topic metadata also needs to be adapted for account ownership. i added these to the TODO list in #54333

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks


add new IAM APIs for User and AccessKey operations. some Role/RolePolicy/UserPolicy operations are already supported, but require special admin capabilities. once a User or Role are added to an account, that account's root user will be granted full access to them

existing users and roles must be added to an account manually, either by a radosgw-admin command or /admin API, before they can be managed by an account root user
Copy link
Contributor

@pritha-srivastava pritha-srivastava Feb 19, 2024

Choose a reason for hiding this comment

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

@cbodley : existing rgw users have certain permissions by default like CreateBucket etc. What happens to those permissions when they are added as an iam user to an account?

Also what happens to policy evaluation logic in case existing users/ roles are grouped into multiple accounts inside a tenant and then if an existing resource policy (bucket policy) has ::user/* set as Principal - will this allow all users of all accounts belonging to that tenant, to access that resource? or are we going to expect the end users to update the policies if they plan to add existing users/roles to accounts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbodley : existing rgw users have certain permissions by default like CreateBucket etc. What happens to those permissions when they are added as an iam user to an account?

they're lost. when you adopt users into an account, you also need to add some identity policy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

existing resource policy (bucket policy) has ::user/* set as Principal

you can find the updated LocalApplier::is_identity() in #54333

currently, account users will match the following principals:

  • wildcard principal *
  • user principals arn:aws:iam::{account_id}:user/{path}{display_name} or arn:aws:iam::{account_id}:user/*
  • account principals arn:aws:iam::{account_id}:root or {account_id}
  • account principals arn:aws:iam::{tenant}:root or {tenant}

it probably makes sense to add matching for user principals like arn:aws:iam::{tenant}:user/{user_id} and arn:aws:iam::{tenant}:user/* for backward compatibility with existing resource policy

Copy link
Contributor

@pritha-srivastava pritha-srivastava Feb 20, 2024

Choose a reason for hiding this comment

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

the question was specifically for tenant-id/user/*, tenant-id was in angle brackets <> and didn't show up in the comment.

@@ -0,0 +1,73 @@
# iam accounts

add a user account primitive for use with IAM REST APIs. this enables end-users to use tools like `aws iam create-user` for self-service management of the users, roles, policies, and access keys in their account without requiring global admin privileges for radosgw-admin commands or radosgw /admin REST APIs
Copy link
Contributor

Choose a reason for hiding this comment

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

s/APIs. t/APIs. T/

s/end-users/end users/

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label May 13, 2024
@github-actions
Copy link

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

@github-actions github-actions bot closed this Jun 12, 2024
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.

6 participants