[
https://issues.apache.org/jira/browse/HDDS-17410?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17410:
------------------------------
Affects Version/s: 2.3.0
> Revoking a tenant access id should record an STS revocation cutoff for the
> access id
> ------------------------------------------------------------------------------------
>
> Key: HDDS-17410
> URL: https://issues.apache.org/jira/browse/HDDS-17410
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Priority: Major
> Attachments: CR-6-record-sts-revocation-cutoff-on-tenant-revoke.patch
>
>
> h3. Mechanism
> {{S3SecurityUtil.validateS3Credential}} runs two revocation checks for an STS
> session token. {{isRevokedStsToken}} compares the signed creation time of the
> token with the cutoff stored for its {{originalAccessKeyId}} in
> {{s3RevokedStsTokenTable}}. {{isOriginalAccessKeyIdRevoked}} treats the
> creator as revoked when no S3 secret exists for that access id
> ({{!hasS3Secret(originalAccessKeyId)}}).
> A tenant access id can be the {{originalAccessKeyId}} of an STS token:
> {{S3AssumeRoleRequest}} accepts it like any other access id and
> {{getSessionPolicy}} resolves the tenant volume for it.
> {{OMTenantRevokeUserAccessIdRequest}} (behind {{ozone tenant user revoke}})
> removes the access id together with its S3 secret
> ({{OMTenantRevokeUserAccessIdResponse.addToDBBatch}}) and writes nothing to
> {{s3RevokedStsTokenTable}}. The revocation is therefore represented only by
> the absence of a secret, and no revocation time is kept. HDDS-17409 is the
> same gap in {{S3RevokeSecretRequest}}; {{OMTenantRevokeUserAccessIdRequest}}
> is the only other request that removes an S3 secret
> ({{OMTenantDeleteRequest}} requires a tenant without access ids and removes
> no secret).
> h3. Trigger
> The secret of a tenant access id is removed by
> {{OMTenantRevokeUserAccessIdRequest}} and a secret is later stored for the
> same access id, while STS tokens issued under the first secret are unexpired
> (an STS token lives up to 12 hours). A tenant access id is derived from the
> tenant and user names ({{OMMultiTenantManager.getDefaultAccessId}},
> {{tenant$user}}), so the same string can have a secret again later. From that
> point neither check reflects the earlier revocation: a secret exists, and no
> cutoff was recorded.
> While the access id has no secret, {{S3RevokeSTSTokenRequest.preExecute}}
> refuses to record a cutoff for it ({{ACCESS_ID_NOT_FOUND}}).
> h3. Impact
> * For STS tokens issued before the access id was revoked, the revocation
> lasts only until a secret is stored for the same access id. From then on
> {{validateS3Credential}} handles them like tokens of an access id that was
> never revoked, until they expire.
> * No data is lost or changed. STS is not part of a released Ozone version.
> h3. Reproduction
> The regression tests in the attached patch cover it, in
> {{TestS3GetSecretRequest}} ({{hadoop-ozone/ozone-manager}}). They drive the
> real tenant request classes on a real {{OmMetadataManagerImpl}} and commit
> each response to the DB:
> * {{testTenantRevokeRecordsStsRevocationCutoff}}: revoking a tenant access id
> records the cutoff stamped in {{preExecute}} in {{s3RevokedStsTokenTable}},
> and the cutoff is still there after a new secret is stored for the access id.
> Without the change it fails because {{preExecute}} attaches no cutoff
> ({{Expecting actual: 0L to be greater than: 0L}}).
> * {{testTenantRevokeWithoutAttachedCutoffRecordsNone}}: a log entry without
> the attached cutoff removes the secret and records nothing, as before. It
> passes with and without the change.
> No standalone reproduction is attached. Not run: a multi OM cluster, or the
> RPC path end to end.
> h3. Patch
> [^CR-6-record-sts-revocation-cutoff-on-tenant-revoke.patch], against
> 5f7111a27ea07ce6b7bf5284a4213ebacb65f2ae with the HDDS-17409 patch applied:
> it applies only on top of that patch, because it moves the helper that patch
> adds into {{S3SecretRequestHelper}}. With that patch applied first, it also
> applies to master at 65320362775.
> {{OMTenantRevokeUserAccessIdRequest.preExecute}} attaches a
> {{RevokeSTSTokenRequest}} to the {{OMRequest}} with the leader's current time
> as {{revocationTimeMillis}}, as HDDS-17409 does for
> {{S3RevokeSecretRequest}}. This is an existing field of {{OMRequest}}, so
> there is no proto change. {{validateAndUpdateCache}} records that time for
> the access id in the {{s3RevokedStsTokenTable}} cache, keeping the larger
> value if a cutoff already exists, and {{OMTenantRevokeUserAccessIdResponse}}
> writes it in the same batch as the secret deletion. The cache and max logic
> moves unchanged from {{S3RevokeSecretRequest}} to
> {{S3SecretRequestHelper.updateStsRevocationCutoff}}, which both requests now
> call. A log entry without the attached cutoff is applied as before.
> With the cutoff in place, {{isRevokedStsToken}} rejects every token of the
> access id created before the revocation, whether or not a secret is stored
> later. Tokens created after the revocation are unaffected.
> {{RevokedSTSTokenCleanupService}} removes the DB row after the usual
> threshold; the cached cutoff stays until the table cache is rebuilt, unless
> HDDS-17406 lands. Each successful tenant access id revoke now keeps one row
> in {{s3RevokedStsTokenTable}} for that period, also when STS is not used.
> With the patch, {{TestS3GetSecretRequest}}, {{TestS3RevokeSTSTokenRequest}},
> {{TestS3SecurityUtil}}, {{TestS3AssumeRoleRequest}},
> {{TestS3SecretRequestHelper}}, {{TestRevokedSTSTokenCleanupService}},
> {{TestOmMetadataManager}}, {{TestOMMultiTenantManager}},
> {{TestOMMultiTenantManagerImpl}}, {{TestOMTenantCreateRequest}},
> {{TestOMTenantDeleteRequest}} and {{TestMultiTenantVolume}} pass, and
> checkstyle on {{hadoop-ozone/ozone-manager}} is clean.
> Found by code review of the OM STS token revocation and cleanup paths, as
> part of the TLA+ verification effort under HDDS-15926, on commit
> 5f7111a27ea07ce6b7bf5284a4213ebacb65f2ae. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not
> duplicates: HDDS-17409 (the same gap in {{S3RevokeSecretRequest}}),
> HDDS-17408 (adds the same keep the larger step to
> {{S3RevokeSTSTokenRequest}}; whichever of it and this patch lands later
> should call {{S3SecretRequestHelper.updateStsRevocationCutoff}}), HDDS-17406
> (the purge and the table cache), HDDS-14364 (added the secret presence
> check), HDDS-14066 and HDDS-14067 (the revocation cutoff table and
> {{revokeststoken}}), HDDS-14094 (the cleanup service). The attached patch is
> a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
