[
https://issues.apache.org/jira/browse/HDDS-17409?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17409:
------------------------------
Affects Version/s: 2.3.0
> 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
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Priority: Major
> 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]
