HDDS-16110. [STS] Update key in sts revocation table by fmorg-git · Pull Request #11095 · apache/ozone

fmorg-git · 2026-08-23T21:11:53Z

Please describe your PR in detail:

Previously, the key for the STS revocation table was the sessionToken. It was noted that sessionToken is not audited, so it's difficult to determine what it is from logs without turning on trace logs to log all headers. Therefore, the revocation key is being updated to being just the originalAccessKeyId (which will be added to the audit log as part of the PR and indicated as unverified). Each sessionToken will include a new creationTime field. the revocation logic is updated such that only the tokens that were created by the originalAccessKeyId before the entry in rocksDB is added are revoked - all tokens created by the originalAccessKeyId after that cutoff point will not be revoked.
As a side effect of testing this change, it was noticed certain inputs could cause the token parsing to fail that were not already covered, so this PR addresses that as well (in STSSecurityUtil).
The PR is divided into reviewable commits

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16110

How was this patch tested?

smoke tests and unit tests
manually checking keys in revocation table and then lowering the cleanup threshold to ensure they were still being cleaned up:

bash-5.1$ OZONE_OPTS= ozone debug ldb scan --db=/data/metadata/om.db --column-family=s3RevokedStsTokenTable --with-keys --limit=100
{ "svc-iceberg-rest-catalog/s3g@EXAMPLE.COM": 1787519189024
 }
bash-5.1$ OZONE_OPTS= ozone debug ldb scan --db=/data/metadata/om.db --column-family=s3RevokedStsTokenTable --with-keys --limit=100
{  }

manually checking S3g audit log to ensure originalAccessKeyId was added and it contains (unverified):

2026-08-23 22:23:01,901 | INFO  | S3GAudit | user=ASIAU0XSQY82QPHDVT2FO71W | ip=172.25.0.120 | op=LIST_S3_BUCKETS {"originalAccessKeyId":"svc-iceberg-rest-catalog/s3g@EXAMPLE.COM (unverified)","x-amz-request-id":"7374e5d8-b4ac-4cc3-9ddb-e51b0163079e","x-amz-id-2":"29puQAR4or"} | ret=SUCCESS |

… log

fmorg-git · 2026-08-23T21:17:50Z

hi @ChenSammi @chihsuan - please review this one instead. Thanks!

…to audit log" This reverts commit d3c9ea5.

ChenSammi · 2026-08-24T05:50:58Z


      // Add tempAccessKeyId to the log so it can be determined which permanent user created the tempAccessKeyId
-      auditMap.put("tempAccessKeyId", tempAccessKeyId);
+      auditMap.put(OzoneConsts.S3_STS_TEMP_ACCESS_KEY_ID, tempAccessKeyId);


Can we log the original access key too in audit log?

So the user field in the om audit log is essentially the originalAccessKeyId. A sample om audit log entry is below:

2026-08-23 22:23:09,651 | INFO | OMAudit | user=svc-iceberg-rest-catalog/s3g@EXAMPLE.COM | ip=172.25.0.120 | op=S3_ASSUME_ROLE {"duration":"900","Transaction":"283","isPolicyIncluded":"Y","roleArn":"arn:aws:iam::123456789012:role/sts-temp-bucket-access","requestId":"c2ebe586-df7b-41d9-a964-811312c4a199","roleSessionName":"sts-session-name","action":"AssumeRole","tempAccessKeyId":"ASIA3X2X4JL0GBX0595DU2MU"} | ret=SUCCESS |

Just confirming if we still need to add a separate originalAccessKeyId parameter?

We can use user field. Not must to add a separate originalAccessKeyId, unless we want to make the op=S3_ASSUME_ROLE completely include all the important fields as a whole.

ChenSammi · 2026-08-24T05:53:57Z


-    return omRequest;
+    if (!ozoneManager.getS3SecretManager().hasS3Secret(originalAccessKeyId)) {
+      throw new OMException("originalAccessKeyId does not exist: " + originalAccessKeyId, INVALID_REQUEST);


We can return ACCESS_ID_NOT_FOUND here.

updated - 43f7d23

ChenSammi · 2026-08-24T06:04:44Z

+  This request will be used internally by OM to replicate the revocation cutoff captured by the leader
+  across the OMs in HA mode.
+*/
+message UpdateRevokeSTSTokenRequest {


Can we reuse RevokeSTSTokenRequest? @ptlrs has a proposal to use the oneof to rearrange OM request and response.

updated - 734a613

ChenSammi · 2026-08-24T06:07:05Z

-    final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeReq =
-        omRequest.getRevokeSTSTokenRequest();
+    final RevokeSTSTokenRequest revokeReq = omRequest.getRevokeSTSTokenRequest();
+    validateRevokeRequestFields(revokeReq);


Maybe we don't need to valid the access key ID format anymore here, given that exact DB record match search is performed later.

updated - 4e50ca5

However, because we are now reusing RevokeSTSTokenRequest for the update, we need to validate that the client did not send revocationTimeMillis

ChenSammi · 2026-08-24T06:14:00Z

    // Audit log
    final Map<String, String> auditMap = new HashMap<>();
    final OzoneManagerProtocolProtos.UserInfo userInfo = getOmRequest().getUserInfo();
    auditMap.put(OzoneConsts.S3_REVOKESTSTOKEN_USER, userInfo.getUserName());


In the s3 secret case, S3RevokeSecretUser is the user which is been revoked. In STS case, it's
the S3_STS_ORIGINAL_ACCESS_KEY_ID.

So maybe we can combine to show

auditMap.put(OzoneConsts.S3_REVOKESTSTOKEN_USER, originalAccessKeyId);

, and move the statement ahead to the try scope, so no originalAccessKeyId null check.

Besides, we don't need to log the userInfo explicitly, it's covered in buildAuditMessage().

updated - 3bcbe03

ChenSammi · 2026-08-24T06:20:55Z

      token.decodeFromUrlString(encodedToken);
      return token;
-    } catch (IOException e) {
+    } catch (IOException | RuntimeException e) {


This RuntimeException seems not required.

so actually, this is very subtle and required. The catching of this RuntimeException is what I was referring to for this fix in the PR description As a side effect of testing this change, it was noticed certain inputs could cause the token parsing to fail that were not already covered, so this PR addresses that as well.

Here is explanation: 
Token.decodeFromUrlString() only declares IOException, but Hadoop’s deserialization can throw unchecked exceptions for malformed tokens — e.g. NegativeArraySizeException when a decoded length is negative.
We want all decode failures mapped to SecretManager.InvalidToken → OMException(INVALID_TOKEN), so that’s why RuntimeException is caught here. testConstructValidateAndDecryptSTSTokenRuntimeDecodeFailure in unit tests and the not-a-valid-token case in Tampered STS Token Service, Policy, or Signature Must Fail smoke test cover this.

ChenSammi · 2026-08-24T07:00:15Z

    // serialized there.
-    final STSTokenIdentifier identifier = new STSTokenIdentifier(
-        tempAccessKeyId, originalAccessKeyId, roleArn, expiration, secretAccessKey, sessionPolicy, encryptionKey);
+    final STSTokenIdentifier identifier = new STSTokenIdentifier(STSTokenIdentifier.Params.newBuilder()


There is a problem here. The encryptionKey passed into STSTokenIdentifier which encrypts the secretAccessKey, can be different than the currentKey in generateToken, which signs the whole token, if the key secret is rotated during the small time window.

thanks so much for catching this. Updated - 10ef46a

ChenSammi · 2026-08-24T07:09:11Z

    auditMap.put("x-amz-request-id", requestIdentifier.getRequestId());
    auditMap.put("x-amz-id-2", requestIdentifier.getAmzId());
+    if (s3Auth != null) {
+      // For STS temporary credentials, record the originalAccessKeyId (the permanent principal that


Why do we need to log the originalAccessKeyId for every request which using sessionToken? Since the covert seems expensive to me in getStsOriginalAccessKeyId().

yeah I was wondering why log originalAccessKeyId as well, since for every valid token, we have om audit log to tell us who the creator is, and previously we ensured that all om followers would have the same information for created tokens. updated to remove this audit entry - 99a1064

ChenSammi · 2026-08-24T07:12:26Z

Thanks @fmorg-git , overall looks good.

Copilot

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR updates STS token revocation to key the revocation table by originalAccessKeyId (with a replicated revocation cutoff time) instead of sessionToken, and augments auditing to include the original access key ID extracted from STS session tokens.

Changes:

Switch revocation storage/cleanup and revoke RPC/CLI to use originalAccessKeyId -> revocationTimeMillis and revoke only tokens created strictly before the cutoff.
Add creationTime to STS tokens and enforce it in token validation.
Enhance S3 Gateway audit logging to include originalAccessKeyId (unverified) decoded from the presented session token.

Reviewed changes

Copilot reviewed 38 out of 39 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/util/TestAuditUtils.java	Adds unit tests for extracting `originalAccessKeyId` from STS session tokens for auditing.
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestEndpointBase.java	Adds audit tests ensuring STS original access key ID is included/omitted appropriately.
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java	Updates revoke API stub signature to accept `originalAccessKeyId`.
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/util/AuditUtils.java	Implements decoding of `originalAccessKeyId` from a session token for audit logging.
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java	Records `(unverified)` `originalAccessKeyId` in the S3G audit map for STS requests.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenSecretManager.java	Extends tests to assert STS tokens include `creationTime`.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenIdentifier.java	Migrates tests to Params builder and verifies protobuf includes issue/creation time.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenEncryption.java	Adds creation time assertions for encrypted STS token identifiers.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSSecurityUtil.java	Adds tests for runtime decode failures and creationTime being required.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestS3SecurityUtil.java	Updates revocation tests to use cutoff time keyed by `originalAccessKeyId`.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestRevokedSTSTokenCleanupService.java	Updates cleanup tests for the new key/value semantics and request fields.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3RevokeSTSTokenRequest.java	Refactors revoke request tests to validate access, unknown IDs, and replicated fields.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java	Updates metadata table tests to store/look up revocation entries by `originalAccessKeyId`.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenSecretManager.java	Sets token `creationTime` and passes it via new Params object when issuing STS tokens.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenIdentifier.java	Adds `creationTime`, serializes it to `issueDate`, and updates equality/hash/toString.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSSecurityUtil.java	Treats runtime decode failures as invalid-token and requires `creationTime`.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/S3SecurityUtil.java	Revokes based on per-`originalAccessKeyId` cutoff and token creation time.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/RevokedSTSTokenCleanupService.java	Cleans up revocation cutoffs keyed by `originalAccessKeyId` with max-token-lifetime retention.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3RevokeSTSTokenResponse.java	Stores revocation cutoff time for an `originalAccessKeyId` in the DB batch.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3DeleteRevokedSTSTokensResponse.java	Deletes revocation entries by `originalAccessKeyId`.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3RevokeSTSTokenRequest.java	Validates input, captures replicated cutoff, updates cache, and audits `originalAccessKeyId`.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3DeleteRevokedSTSTokensRequest.java	Updates request handling to delete entries by `originalAccessKeyId`.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3AssumeRoleRequest.java	Centralizes STS temp access key constants and standardizes audit param name.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java	Uses shared STS token prefix constant rather than request-class constant.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java	Rehydrates ThreadLocal STS identifier using Params (setting creation/expiry to `Instant.MAX`).
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java	Updates DB definition docs for new revocation key/value semantics.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java	Updates table comment to reflect `originalAccessKeyId -> revocationTimeMillis`.
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto	Changes revoke request field, adds replicated update request, and updates delete request fields.
hadoop-ozone/dist/src/main/smoketest/security/ozone-secure-sts.robot	Updates smoketests for new revoke CLI, post-revoke assume-role behavior, and malformed token.
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java	Updates revoke RPC to send `originalAccessKeyId`.
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java	Updates revoke API contract to revoke by `originalAccessKeyId`.
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3STSUtils.java	Adds shared STS token/access-key constants used across OM components.
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java	Updates client method to revoke by `originalAccessKeyId`.
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java	Updates client protocol signature/docs for revoke-by-`originalAccessKeyId`.
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java	Updates ObjectStore API/docs for revoke-by-`originalAccessKeyId`.
hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/s3/RevokeSTSTokenHandler.java	Updates CLI option/UX and output to revoke by `originalAccessKeyId`.
hadoop-hdds/docs/content/design/ozone-sts.md	Updates design docs for creationTime and revocation cutoff semantics.
hadoop-hdds/common/src/main/resources/ozone-default.xml	Updates cleanup service description to match new retention semantics.
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java	Adds constants for STS audit param names.

Suppressed comments (1)

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/util/AuditUtils.java:1

This decodes and logs a client-controlled value (marked as unverified), but it does not verify the protobuf type is S3_STS_TOKEN and it does not bound/sanitize the extracted value size/content. A malformed token could inject an arbitrary originalAccessKeyId into audit logs or bloat audit records. Consider (1) checking OMTokenProto.getType() before reading originalAccessKeyId, and (2) enforcing a strict max length / allowed character set (or returning null) before adding it to audit params.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…TSTokenRequest

… of having UpdateRevokeSTSTokenRequest

…RevokeSTSTokenRequest

…t params

…and signing

…o s3g audit log

ChenSammi · 2026-08-25T04:44:40Z

+        .setSessionPolicy(sessionPolicy)
+        .setEncryptionKey(encryptionKey)
+        .build());
+    identifier.setSecretKeyId(secretKey.getId());


How about set the entire ManagedSecretKey to the STSTokenIdentifier builder? So we can avoid check id, and refetch the ManageedSecretKey again?

the ManagedSecretKey is fetched only once and passed directly into generateToken() method used to sign. It doesn't appear to be a good idea to encode the entire ManagedSecretKey into the STS token itself (possible security risk).

Copilot

Pull request overview

Copilot reviewed 34 out of 35 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenIdentifier.java:266

fromProtoBuf() only sets creationTime when issueDate is present. Combined with STSSecurityUtil.ensureEssentialFieldsArePresentInToken() requiring a non-null creationTime, this will cause previously-issued STS tokens (which may not have issueDate) to be rejected after upgrade. Consider defaulting creationTime when issueDate is absent (eg derive it from maxDate and the max STS duration) to preserve compatibility during rolling upgrades.

    if (token.hasIssueDate()) {
      this.creationTime = Instant.ofEpochMilli(token.getIssueDate());
    }
    if (token.hasOriginalAccessKeyId()) {

… on replicated request Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Fabian Morgan added 7 commits August 23, 2026 13:46

redesign: update design doc

64bd6f7

redesign: update proto and related classes

5f7e3f2

redesign: update rocksDb related classes

21e89ab

redesign: update ozone cli

fc939e4

redesign: update main revocation logic

5829263

redesign: update smoke tests

942d82c

redesign: ensure only validated originalAccessKeyId is added to audit…

d3c9ea5

… log

fmorg-git mentioned this pull request Aug 23, 2026

HDDS-16110. [STS] Update key in sts revocation table #11030

Closed

Fabian Morgan added 2 commits August 23, 2026 14:57

Revert "redesign: ensure only validated originalAccessKeyId is added …

28037ce

…to audit log" This reverts commit d3c9ea5.

redesign: indicate that originalAccessKeyId in s3g log is unverified

4418fb9

ChenSammi reviewed Aug 24, 2026

View reviewed changes

ChenSammi requested a lite review from Copilot August 24, 2026 09:24

Copilot AI reviewed Aug 24, 2026

View reviewed changes

Copilot started reviewing on behalf of ChenSammi August 24, 2026 10:04 View session

Fabian Morgan added 6 commits August 24, 2026 14:47

pr review update for ChenSammi - use ACCESS_ID_NOT_FOUND in S3RevokeS…

43f7d23

…TSTokenRequest

pr review updates for ChenSammi - reuse RevokeSTSTokenRequest instead…

734a613

… of having UpdateRevokeSTSTokenRequest

pr review updates for ChenSammi - remove unnecessary validation in S3…

4e50ca5

…RevokeSTSTokenRequest

pr review updates for ChenSammi - update S3RevokeSTSTokenRequest audi…

3bcbe03

…t params

pr review comments for ChenSammi - use same secretKey for encryption …

10ef46a

…and signing

pr review updates for ChenSammi - remove adding originalAccessKeyId t…

99a1064

…o s3g audit log

ChenSammi requested a lite review from Copilot August 25, 2026 04:39

Copilot started reviewing on behalf of ChenSammi August 25, 2026 04:39 View session

ChenSammi reviewed Aug 25, 2026

View reviewed changes

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Comment thread ...er/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3RevokeSTSTokenRequest.java

pr review update for copilot - ensure originalAccessKeyId is not lost…

286a06a

… on replicated request Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

fmorg-git mentioned this pull request Aug 26, 2026

HDDS-16290. [STS] Implement GetCallerIdentity API #11121

Draft

Uh oh!

Conversation

fmorg-git commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

fmorg-git commented Aug 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChenSammi Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmorg-git Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChenSammi Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChenSammi commented Aug 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fmorg-git commented Aug 23, 2026 •

edited

Loading

ChenSammi Aug 24, 2026 •

edited

Loading

fmorg-git Aug 25, 2026 •

edited

Loading

ChenSammi Aug 24, 2026 •

edited

Loading