[
https://issues.apache.org/jira/browse/HDDS-17196?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17196:
------------------------------
Attachment: CR-6-is-tenant-admin-skip-missing-access-id.patch
> Tenant admin request is rejected with PERMISSION_DENIED while an admin
> revokes another accessId of the same user
> ----------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17196
> URL: https://issues.apache.org/jira/browse/HDDS-17196
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: CR-6-is-tenant-admin-skip-missing-access-id.patch
>
>
> h3. Mechanism
> {{OMMultiTenantManagerImpl.isTenantAdmin(String, String, boolean)}} reads the
> user's {{principalToAccessIdsTable}} row and then looks up each listed
> accessId in {{tenantAccessIdTable}}, without a lock. When one of those
> lookups returns null it returns {{false}} right away instead of checking the
> remaining accessIds. The comment on that branch already says the row "could
> be null since we may not have a lock on the tenant", and
> {{OzoneManager.tenantGetUserInfo}} skips such an accessId with a warning, so
> skipping was the intent.
> {{OMTenantRevokeUserAccessIdRequest.validateAndUpdateCache}} first updates
> the user's principal row and then deletes the accessId row. A reader that
> reads the principal row before the revoke and the accessId row after it gets
> null for the revoked accessId. If that accessId is looked at before the one
> that makes the user a tenant admin (the accessIds are a {{HashSet}}, so the
> order depends on the names), the check fails.
> h3. Trigger
> A tenant admin check of a user reads the user's {{principalToAccessIdsTable}}
> row while another accessId of the same user is being revoked: the check reads
> the principal row before the revoke updates it and the
> {{tenantAccessIdTable}} row after the revoke deletes it.
> h3. Impact
> * The user's request fails with {{PERMISSION_DENIED}}, although the user's
> admin row of that tenant was never touched. Affected: preExecute of tenant
> user assign and revoke and of tenant admin assign and revoke
> ({{checkTenantAdmin}}), and {{listUsersInTenant}}.
> * Stored state stays correct and a retry succeeds. Nothing in OM retries the
> request.
> * If a principal row permanently lists an accessId without a row (a dangling
> reference, which a failed tenant user assign can leave behind, filed
> separately), the same early return makes the denial permanent for that user
> whenever the dangling accessId is looked at first. Argued from the code; the
> regression test uses such a state, the full sequence was not run.
> h3. Reproduction
> The issue has no standalone reproduction. The regression test in the attached
> patch covers it (see below).
> h3. Patch
> [^CR-6-is-tenant-admin-skip-missing-access-id.patch], against
> a6b7bdb937109ba2893688b41a89470bc95c88cf. It also applies to master at
> 1753288f333.
> A missing {{tenantAccessIdTable}} row now continues the loop instead of
> returning {{false}}. This cannot make a user a tenant admin without an admin
> row: the result is {{true}} only from an existing {{tenantAccessIdTable}} row
> that names the requested tenant with the admin flag, and a user cannot hold
> two accessIds in the same tenant. Covered by
> {{testIsTenantAdminSkipsMissingAccessId}} in the existing
> {{TestOMMultiTenantManagerImpl}}, which fails without the change and also
> checks that, past the missing row, an admin without the delegated flag still
> fails the delegated admin check. With it the tenant manager, tenant request
> and {{TestS3GetSecretRequest}} unit suites pass and checkstyle is clean.
> Found by code review of the OM multi-tenancy and Ranger sync paths, as part
> of the TLA+ verification effort under HDDS-15926, on commit
> a6b7bdb937109ba2893688b41a89470bc95c88cf. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. The attached patch
> is a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
