Siyao Meng created HDDS-17410:
---------------------------------
Summary: 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
Reporter: Siyao Meng
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]
