You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "fmorg-git (via GitHub)" <gi...@apache.org> on 2026/09/23 05:29:35 UTC
[PR] HDDS-16583. STS credential validation updates [ozone]
fmorg-git opened a new pull request, #11305:
URL: https://github.com/apache/ozone/pull/11305
Please describe your PR in detail:
* This ticket addresses several issues:
1) STS credentials should be validated on the leader, similar to S3 credentials
2) The assumedRoleId and assumedRoleUserArn should be added when resolving STS fields
3) Verify STS token before decrypting (change the order)
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16583
## How was this patch tested?
unit tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16583. STS credential validation updates [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi merged PR #11305:
URL: https://github.com/apache/ozone/pull/11305
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16583. STS credential validation updates [ozone]
Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on PR #11305:
URL: https://github.com/apache/ozone/pull/11305#issuecomment-5809358403
Thanks for the review and merge
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16583. STS credential validation updates [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #11305:
URL: https://github.com/apache/ozone/pull/11305#discussion_r4089382880
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSSecurityUtil.java:
##########
@@ -213,23 +217,116 @@ public static void ensureResolvedStsFieldsInvariants(OzoneManagerProtocolProtos.
if (!hasSessionToken) {
// If sessionToken is missing/empty, resolved fields must be empty.
- if (s3Auth.hasResolvedStsSessionPolicy() || s3Auth.hasResolvedStsRoleArn() ||
- s3Auth.hasResolvedStsOriginalAccessKeyId() || s3Auth.hasResolvedStsTempAccessKeyId() ||
- s3Auth.hasResolvedStsSecretKeyId()) {
+ if (hasAnyResolvedStsField(s3Auth)) {
throw new OMException("Resolved STS fields must be empty when sessionToken is not present", INVALID_TOKEN);
}
return;
}
- ensureResolvedFieldsArePresent(s3Auth);
+ if (!hasAllResolvedStsFields(s3Auth)) {
+ throw new OMException("Resolved STS fields must be present when sessionToken is present", INVALID_TOKEN);
+ }
+ }
+
+ private static boolean hasAnyResolvedStsField(S3Authentication s3Auth) {
+ return s3Auth.hasResolvedStsSessionPolicy() || s3Auth.hasResolvedStsRoleArn() ||
+ s3Auth.hasResolvedStsOriginalAccessKeyId() || s3Auth.hasResolvedStsTempAccessKeyId() ||
+ s3Auth.hasResolvedStsSecretKeyId() || s3Auth.hasResolvedStsAssumedRoleId() ||
+ s3Auth.hasResolvedStsAssumedRoleUserArn();
}
- private static void ensureResolvedFieldsArePresent(S3Authentication s3Auth) throws OMException {
- if (!s3Auth.hasResolvedStsSessionPolicy() || !s3Auth.hasResolvedStsRoleArn() ||
- !s3Auth.hasResolvedStsOriginalAccessKeyId() || !s3Auth.hasResolvedStsTempAccessKeyId() ||
- !s3Auth.hasResolvedStsSecretKeyId()) {
- throw new OMException("Resolved STS fields must be present when sessionToken is present", INVALID_TOKEN);
+ private static boolean hasAllResolvedStsFields(S3Authentication s3Auth) {
+ return s3Auth.hasResolvedStsSessionPolicy() && s3Auth.hasResolvedStsRoleArn() &&
+ s3Auth.hasResolvedStsOriginalAccessKeyId() && s3Auth.hasResolvedStsTempAccessKeyId() &&
+ s3Auth.hasResolvedStsSecretKeyId() && s3Auth.hasResolvedStsAssumedRoleId() &&
+ s3Auth.hasResolvedStsAssumedRoleUserArn();
+ }
+
+ /**
+ * Copies the STS state that {@link S3SecurityUtil#validateS3Credential} put in the {@link OzoneManager}
+ * thread local onto {@code s3Auth}, or clears the resolved fields when the request has no session token.
+ *
+ * <p>The resolved fields are the only STS state visible to the Ratis apply thread, because the thread locals
+ * belong to the RPC handler thread and do not follow the request onto a Ratis thread. Resolving here, where
+ * the token has just been verified, is what lets the apply thread rebuild the request context without
+ * repeating any crypto.</p>
+ *
+ * <p>Returns {@code s3Auth} itself when there is nothing to resolve and nothing to clear.</p>
+ *
+ * @throws OMException if a session token is present but the token identifier is not, which means the
+ * request reached this point without passing STS validation
+ */
+ public static S3Authentication resolveS3Authentication(S3Authentication s3Auth, OzoneManager ozoneManager)
+ throws OMException {
+ final boolean hasSessionToken = s3Auth.hasSessionToken() && !s3Auth.getSessionToken().isEmpty();
+ final STSTokenIdentifier stsTokenIdentifier = OzoneManager.getStsTokenIdentifier();
+
+ // This should not happen, so explicitly throw an error. An existing sessionToken
+ // implies prior STS validation must have populated the ThreadLocal.
+ if (ozoneManager.isSecurityEnabled() && hasSessionToken && stsTokenIdentifier == null) {
+ throw new OMException(
+ "S3Authentication has session token but no STS token identifier in OzoneManager ThreadLocal",
+ INVALID_REQUEST);
+ }
+
+ if (!hasSessionToken || stsTokenIdentifier == null) {
Review Comment:
The previous code, "&&" is used. Now it's "||". Is it a change on purpose?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16583. STS credential validation updates [ozone]
Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #11305:
URL: https://github.com/apache/ozone/pull/11305#discussion_r4089617216
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSSecurityUtil.java:
##########
@@ -213,23 +217,116 @@ public static void ensureResolvedStsFieldsInvariants(OzoneManagerProtocolProtos.
if (!hasSessionToken) {
// If sessionToken is missing/empty, resolved fields must be empty.
- if (s3Auth.hasResolvedStsSessionPolicy() || s3Auth.hasResolvedStsRoleArn() ||
- s3Auth.hasResolvedStsOriginalAccessKeyId() || s3Auth.hasResolvedStsTempAccessKeyId() ||
- s3Auth.hasResolvedStsSecretKeyId()) {
+ if (hasAnyResolvedStsField(s3Auth)) {
throw new OMException("Resolved STS fields must be empty when sessionToken is not present", INVALID_TOKEN);
}
return;
}
- ensureResolvedFieldsArePresent(s3Auth);
+ if (!hasAllResolvedStsFields(s3Auth)) {
+ throw new OMException("Resolved STS fields must be present when sessionToken is present", INVALID_TOKEN);
+ }
+ }
+
+ private static boolean hasAnyResolvedStsField(S3Authentication s3Auth) {
+ return s3Auth.hasResolvedStsSessionPolicy() || s3Auth.hasResolvedStsRoleArn() ||
+ s3Auth.hasResolvedStsOriginalAccessKeyId() || s3Auth.hasResolvedStsTempAccessKeyId() ||
+ s3Auth.hasResolvedStsSecretKeyId() || s3Auth.hasResolvedStsAssumedRoleId() ||
+ s3Auth.hasResolvedStsAssumedRoleUserArn();
}
- private static void ensureResolvedFieldsArePresent(S3Authentication s3Auth) throws OMException {
- if (!s3Auth.hasResolvedStsSessionPolicy() || !s3Auth.hasResolvedStsRoleArn() ||
- !s3Auth.hasResolvedStsOriginalAccessKeyId() || !s3Auth.hasResolvedStsTempAccessKeyId() ||
- !s3Auth.hasResolvedStsSecretKeyId()) {
- throw new OMException("Resolved STS fields must be present when sessionToken is present", INVALID_TOKEN);
+ private static boolean hasAllResolvedStsFields(S3Authentication s3Auth) {
Review Comment:
updated - made `resolvedStsAssumedRoleId` and `resolvedStsAssumedRoleUserArn `optional in `hasAllResolvedStsFields()` since they're for GetCallerIdentity, not apply-path ACL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16583. STS credential validation updates [ozone]
Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #11305:
URL: https://github.com/apache/ozone/pull/11305#discussion_r4089541403
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSSecurityUtil.java:
##########
@@ -213,23 +217,116 @@ public static void ensureResolvedStsFieldsInvariants(OzoneManagerProtocolProtos.
if (!hasSessionToken) {
// If sessionToken is missing/empty, resolved fields must be empty.
- if (s3Auth.hasResolvedStsSessionPolicy() || s3Auth.hasResolvedStsRoleArn() ||
- s3Auth.hasResolvedStsOriginalAccessKeyId() || s3Auth.hasResolvedStsTempAccessKeyId() ||
- s3Auth.hasResolvedStsSecretKeyId()) {
+ if (hasAnyResolvedStsField(s3Auth)) {
throw new OMException("Resolved STS fields must be empty when sessionToken is not present", INVALID_TOKEN);
}
return;
}
- ensureResolvedFieldsArePresent(s3Auth);
+ if (!hasAllResolvedStsFields(s3Auth)) {
+ throw new OMException("Resolved STS fields must be present when sessionToken is present", INVALID_TOKEN);
+ }
+ }
+
+ private static boolean hasAnyResolvedStsField(S3Authentication s3Auth) {
+ return s3Auth.hasResolvedStsSessionPolicy() || s3Auth.hasResolvedStsRoleArn() ||
+ s3Auth.hasResolvedStsOriginalAccessKeyId() || s3Auth.hasResolvedStsTempAccessKeyId() ||
+ s3Auth.hasResolvedStsSecretKeyId() || s3Auth.hasResolvedStsAssumedRoleId() ||
+ s3Auth.hasResolvedStsAssumedRoleUserArn();
}
- private static void ensureResolvedFieldsArePresent(S3Authentication s3Auth) throws OMException {
- if (!s3Auth.hasResolvedStsSessionPolicy() || !s3Auth.hasResolvedStsRoleArn() ||
- !s3Auth.hasResolvedStsOriginalAccessKeyId() || !s3Auth.hasResolvedStsTempAccessKeyId() ||
- !s3Auth.hasResolvedStsSecretKeyId()) {
- throw new OMException("Resolved STS fields must be present when sessionToken is present", INVALID_TOKEN);
+ private static boolean hasAllResolvedStsFields(S3Authentication s3Auth) {
+ return s3Auth.hasResolvedStsSessionPolicy() && s3Auth.hasResolvedStsRoleArn() &&
+ s3Auth.hasResolvedStsOriginalAccessKeyId() && s3Auth.hasResolvedStsTempAccessKeyId() &&
+ s3Auth.hasResolvedStsSecretKeyId() && s3Auth.hasResolvedStsAssumedRoleId() &&
+ s3Auth.hasResolvedStsAssumedRoleUserArn();
+ }
+
+ /**
+ * Copies the STS state that {@link S3SecurityUtil#validateS3Credential} put in the {@link OzoneManager}
+ * thread local onto {@code s3Auth}, or clears the resolved fields when the request has no session token.
+ *
+ * <p>The resolved fields are the only STS state visible to the Ratis apply thread, because the thread locals
+ * belong to the RPC handler thread and do not follow the request onto a Ratis thread. Resolving here, where
+ * the token has just been verified, is what lets the apply thread rebuild the request context without
+ * repeating any crypto.</p>
+ *
+ * <p>Returns {@code s3Auth} itself when there is nothing to resolve and nothing to clear.</p>
+ *
+ * @throws OMException if a session token is present but the token identifier is not, which means the
+ * request reached this point without passing STS validation
+ */
+ public static S3Authentication resolveS3Authentication(S3Authentication s3Auth, OzoneManager ozoneManager)
+ throws OMException {
+ final boolean hasSessionToken = s3Auth.hasSessionToken() && !s3Auth.getSessionToken().isEmpty();
+ final STSTokenIdentifier stsTokenIdentifier = OzoneManager.getStsTokenIdentifier();
+
+ // This should not happen, so explicitly throw an error. An existing sessionToken
+ // implies prior STS validation must have populated the ThreadLocal.
+ if (ozoneManager.isSecurityEnabled() && hasSessionToken && stsTokenIdentifier == null) {
+ throw new OMException(
+ "S3Authentication has session token but no STS token identifier in OzoneManager ThreadLocal",
+ INVALID_REQUEST);
+ }
+
+ if (!hasSessionToken || stsTokenIdentifier == null) {
Review Comment:
yes, this is intentional. The condition was inverted when refactoring from `OMClientRequest.resolveS3Authentication` into `STSSecurityUtil.resolveS3Authentication()`. The old positive branch was `hasSessionToken && stsTokenIdentifier != null;` The new early-return is the inverted equivalent `!hasSessionToken || stsTokenIdentifier == null`. The behavior should be unchanged - it populate sresolved fields only when both session token and ThreadLocal identifier are present, otherwise clear any stale resolved fields.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16583. STS credential validation updates [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #11305:
URL: https://github.com/apache/ozone/pull/11305#discussion_r4089468637
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSSecurityUtil.java:
##########
@@ -213,23 +217,116 @@ public static void ensureResolvedStsFieldsInvariants(OzoneManagerProtocolProtos.
if (!hasSessionToken) {
// If sessionToken is missing/empty, resolved fields must be empty.
- if (s3Auth.hasResolvedStsSessionPolicy() || s3Auth.hasResolvedStsRoleArn() ||
- s3Auth.hasResolvedStsOriginalAccessKeyId() || s3Auth.hasResolvedStsTempAccessKeyId() ||
- s3Auth.hasResolvedStsSecretKeyId()) {
+ if (hasAnyResolvedStsField(s3Auth)) {
throw new OMException("Resolved STS fields must be empty when sessionToken is not present", INVALID_TOKEN);
}
return;
}
- ensureResolvedFieldsArePresent(s3Auth);
+ if (!hasAllResolvedStsFields(s3Auth)) {
+ throw new OMException("Resolved STS fields must be present when sessionToken is present", INVALID_TOKEN);
+ }
+ }
+
+ private static boolean hasAnyResolvedStsField(S3Authentication s3Auth) {
+ return s3Auth.hasResolvedStsSessionPolicy() || s3Auth.hasResolvedStsRoleArn() ||
+ s3Auth.hasResolvedStsOriginalAccessKeyId() || s3Auth.hasResolvedStsTempAccessKeyId() ||
+ s3Auth.hasResolvedStsSecretKeyId() || s3Auth.hasResolvedStsAssumedRoleId() ||
+ s3Auth.hasResolvedStsAssumedRoleUserArn();
}
- private static void ensureResolvedFieldsArePresent(S3Authentication s3Auth) throws OMException {
- if (!s3Auth.hasResolvedStsSessionPolicy() || !s3Auth.hasResolvedStsRoleArn() ||
- !s3Auth.hasResolvedStsOriginalAccessKeyId() || !s3Auth.hasResolvedStsTempAccessKeyId() ||
- !s3Auth.hasResolvedStsSecretKeyId()) {
- throw new OMException("Resolved STS fields must be present when sessionToken is present", INVALID_TOKEN);
+ private static boolean hasAllResolvedStsFields(S3Authentication s3Auth) {
Review Comment:
Rolling upgrade compatibility concern from Cursor
hasAllResolvedStsFields() now requires all seven resolved fields to be present, including the two new ones.
And ensureResolvedStsFieldsInvariants() is called on the Ratis apply thread in OzoneManagerStateMachine.runCommand() (line 707). During a rolling upgrade where the leader is still on the old version:
1 Old leader processes an STS write request → Ratis log entry is committed without new fields
2. A follower upgraded to the new version applies the committed entry
3. hasAllResolvedStsFields() returns false → ensureResolvedStsFieldsInvariants() throws OMException(INVALID_TOKEN)
4. Caught by catch (IOException e) in runCommand() → createErrorResponse() is returned
5. handler.handleWriteRequest() is never called → metadata change is NOT applied on this follower
The Ratis log says the entry was committed while the follower silently skips applying it. If this follower later becomes leader (e.g., after the upgrade completes), the cluster has diverged metadata for those STS-authenticated writes.
This makes sense if this patch cannot be part of initial release, and later cluster is upgrade with ZDU(if ZDU is not used, then no issue).
So can we make the s3Auth.hasResolvedStsAssumedRoleId() and s3Auth.hasResolvedStsAssumedRoleUserArn() check optional? I believe these two fields are for GetCallerIdentity only, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org