Siyao Meng created HDDS-17409:
---------------------------------
Summary: Revoking an S3 secret should record an STS revocation
cutoff for the access id
Key: HDDS-17409
URL: https://issues.apache.org/jira/browse/HDDS-17409
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-3-record-sts-revocation-cutoff-on-secret-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)}}).
{{S3RevokeSecretRequest}} (behind {{ozone s3 revokesecret}}) removes the secret
and writes nothing to {{s3RevokedStsTokenTable}}. The revocation of the secret
is therefore represented only by the absence of a secret, and no revocation
time is kept. The STS design doc ({{ozone-sts.md}}, section 3.5) says that when
the secret of the user that created an STS token is revoked with {{ozone s3
revokesecret}}, all existing unexpired STS tokens that user created are
revoked, and HDDS-14364 states the same requirement. With only the presence
check, that holds as long as the access id has no secret.
h3. Trigger
The secret for an access id is removed by {{S3RevokeSecretRequest}} 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). 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}}). That check is
intended (it keeps entries for unknown ids out of the table), but it means a
cutoff can be recorded with {{ozone s3 revokeststoken}} only once a secret
exists for the access id.
h3. Impact
* For STS tokens issued before the secret was revoked, the revocation described
in the design doc 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 request classes on a real {{OmMetadataManagerImpl}} and commit each
response to the DB:
* {{testRevokeSecretRecordsStsRevocationCutoff}}: revoking a secret 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}}).
* {{testRevokeSecretKeepsLaterStsRevocationCutoff}}: an existing later cutoff
is kept, not lowered. Without the change it fails with {{expected: <60000> but
was: <null>}}, since nothing is written to the DB.
* {{testRevokeSecretWithoutAttachedCutoffRecordsNone}}: a revoke entry applied
without {{preExecute}}, as from a leader without the change, removes the secret
and records no cutoff, in the cache or in the DB. It passes with and without
the change.
* {{testRevokeMissingSecretRecordsNoStsRevocationCutoff}}: revoking an access
id that has no secret ({{S3_SECRET_NOT_FOUND}}) records no cutoff. 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
[^MC-3-record-sts-revocation-cutoff-on-secret-revoke.patch], against
5f7111a27ea07ce6b7bf5284a4213ebacb65f2ae. It also applies to master at
65320362775.
{{S3RevokeSecretRequest.preExecute}} attaches a {{RevokeSTSTokenRequest}} to
the {{OMRequest}} with the leader's current time as {{revocationTimeMillis}},
as {{S3RevokeSTSTokenRequest}} stamps its cutoff and as {{S3GetSecretRequest}}
attaches the leader generated {{UpdateGetS3SecretRequest}}. This is an existing
field of {{OMRequest}}, so there is no proto change. When
{{validateAndUpdateCache}} removes the secret, it records that time for the
access id in the {{s3RevokedStsTokenTable}} cache, keeping the larger value if
a cutoff already exists, and {{S3RevokeSecretResponse}} writes it in the same
batch as the secret deletion. A log entry without the attached cutoff is
applied as before, and a revoke of an access id with no secret records nothing.
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}} purges the DB row after the usual threshold,
by which time all tokens it covers have expired; the copy in the table cache
stays in memory until the cache is rebuilt, for example on OM restart (unless
HDDS-17406 lands). Each successful {{revokesecret}} now adds one such row, also
when STS is not used.
The "keep the larger cutoff" logic is the same as in the change proposed in
HDDS-17408 for {{S3RevokeSTSTokenRequest}}; whichever lands second should move
it into one shared helper. Rows written by this path are subject to the purge
behaviour addressed in HDDS-17406. {{OMTenantRevokeUserAccessIdRequest}}
removes the secret of a tenant access id the same way, without recording a
cutoff; that path is handled in a separate issue.
With the patch, {{TestS3GetSecretRequest}}, {{TestS3RevokeSTSTokenRequest}},
{{TestS3SecurityUtil}}, {{TestS3AssumeRoleRequest}},
{{TestS3SecretRequestHelper}}, {{TestRevokedSTSTokenCleanupService}},
{{TestOmMetadataManager}}, {{TestSecureOzoneCluster.testGetSetRevokeS3Secret}}
and {{TestMultiTenantVolume}} pass, and checkstyle on
{{hadoop-ozone/ozone-manager}} is clean.
Found by TLA+ model checking and code review of the OM STS token revocation and
cleanup paths under HDDS-15926, on commit
5f7111a27ea07ce6b7bf5284a4213ebacb65f2ae. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. Related but not
duplicates: 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]
