Copilot commented on code in PR #11220:
URL: https://github.com/apache/ozone/pull/11220#discussion_r3964691078
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -691,7 +694,41 @@ public void close() {
*/
@VisibleForTesting
OMResponse runCommand(OMRequest request, TermIndex termIndex) {
+ boolean isS3AuthThreadLocalSet = false;
+ boolean isStsThreadLocalSet = false;
try {
+ if (ozoneManager.isSecurityEnabled() && request.hasS3Authentication()) {
+ // STS token verification runs on the leader RPC path so we don't need
to recheck here on the apply
+ // after the log is committed
+ STSSecurityUtil.ensureResolvedStsFieldsInvariants(request);
+
+ final OzoneManagerProtocolProtos.S3Authentication s3Auth =
request.getS3Authentication();
+ // ThreadLocal carries S3 action for OmMetadataReader.
+ OzoneManager.setS3Auth(s3Auth);
+ isS3AuthThreadLocalSet = true;
+
+ if (s3Auth.hasSessionToken() && !s3Auth.getSessionToken().isEmpty()) {
+ // ThreadLocal carries session policy for OmMetadataReader
+ // Use Instant.MAX for creationTime so a future revocation check on
this ThreadLocal
+ // identifier never treats the token as issued before a stored
cutoff.
+ final STSTokenIdentifier rehydratedTokenIdentifier = new
STSTokenIdentifier(
+ STSTokenIdentifier.Params.newBuilder()
+ .setTempAccessKeyId(
+ s3Auth.hasResolvedStsTempAccessKeyId() ?
s3Auth.getResolvedStsTempAccessKeyId() : "")
+ .setOriginalAccessKeyId(
+ s3Auth.hasResolvedStsOriginalAccessKeyId() ?
s3Auth.getResolvedStsOriginalAccessKeyId() : "")
+ .setRoleArn(s3Auth.hasResolvedStsRoleArn() ?
s3Auth.getResolvedStsRoleArn() : "")
+ .setCreationTime(Instant.MAX)
+ .setExpiry(Instant.MAX) // ensure it deterministically is
not expired
+ .setSecretAccessKey(null) // no secretAccessKey needed
Review Comment:
Using Instant.MAX for STS ThreadLocal timestamps can throw
ArithmeticException if any downstream code calls toEpochMilli() (Instant.MAX
overflows). Use a safe max Instant that round-trips to epoch millis (e.g.,
Instant.ofEpochMilli(Long.MAX_VALUE)) so revocation/expiry comparisons remain
deterministic without risking overflow.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3sts/S3STSConfigKeys.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.s3sts;
+
+import org.apache.hadoop.ozone.OzoneConfigKeys;
+
+/**
+ * This class contains constants for configuration keys used
+ * in S3 STS endpoint.
+ */
+public final class S3STSConfigKeys {
+ public static final String OZONE_S3G_STS_HTTP_ENABLED_KEY =
+ OzoneConfigKeys.OZONE_S3G_STS_HTTP_ENABLED_KEY;
+ public static final String OZONE_S3G_STS_HTTP_BIND_HOST_KEY =
+ "ozone.s3g.sts.http-bind-host";
+ public static final String OZONE_S3G_STS_HTTPS_BIND_HOST_KEY =
+ "ozone.s3g.sts.https-bind-host";
+ public static final String OZONE_S3G_STS_HTTP_ADDRESS_KEY =
+ "ozone.s3g.sts.http-address";
+ public static final String OZONE_S3G_STS_HTTPS_ADDRESS_KEY =
+ "ozone.s3g.sts.https-address";
+ public static final int OZONE_S3G_STS_HTTP_BIND_PORT_DEFAULT = 9880;
+ public static final int OZONE_S3G_STS_HTTPS_BIND_PORT_DEFAULT = 9881;
+ // Max payload default size for STS AssumeRole API calls (32 KB)
+ // as STS AssumeRole has these parameters required in payload:
+ // Action=AssumeRole&RoleArn=...&RoleSessionName=...&DurationSeconds=...
+ // where RoleArn max length is 2048 and max bytes per character in UTF-8
encoding is 12
+ // (2048 * 12 = 24576) + other parameters and overheads, so setting to 32 KB
+ // this limit can be adjusted via configuration if needed.
Review Comment:
Comment says UTF-8 can use 12 bytes per character, but UTF-8 is at most 4
bytes per code point. This misstates the sizing rationale for the 32KB payload
limit.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/OSTSExceptionMapper.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.s3.exception;
+
+import javax.inject.Inject;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.ext.ExceptionMapper;
+import javax.ws.rs.ext.Provider;
+import org.apache.hadoop.ozone.s3.RequestIdentifier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Class that represents various errors returned by the Ozone STS service.
+ */
+@Provider
+public class OSTSExceptionMapper implements ExceptionMapper<OSTSException> {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(OSTSExceptionMapper.class);
+
+ @Inject
+ private RequestIdentifier requestIdentifier;
+
+ @Override
+ public Response toResponse(OSTSException exception) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Returning exception. ex: {}", exception.toString());
+ }
+ exception.setRequestId(requestIdentifier.getRequestId());
+ return Response.status(exception.getHttpCode())
+ .entity(exception.toXml()).build();
Review Comment:
STS exception responses should set an explicit XML media type; otherwise the
Content-Type may default and AWS clients can misinterpret the payload. Align
with OS3ExceptionMapper/OSTSNotFoundExceptionMapper by setting application/xml.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
