diff --git a/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/s3/RevokeSTSTokenHandler.java b/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/s3/RevokeSTSTokenHandler.java index 2f63d4f2a5ad..274304217f86 100644 --- a/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/s3/RevokeSTSTokenHandler.java +++ b/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/s3/RevokeSTSTokenHandler.java @@ -29,20 +29,14 @@ /** * Executes revocation of STS tokens. * - *

This command marks the specified STS temporary access key id as revoked - * by adding it to the OM's revoked STS token table. Subsequent S3 requests - * using the same temporary access key id will be rejected once the revocation + *

This command marks the specified STS token as revoked by adding it to the OM's revoked STS token table. + * Subsequent S3 requests using the same session token will be rejected once the revocation * state has propagated.

*/ @Command(name = "revokeststoken", - description = "Revoke S3 STS token for the given access key id") + description = "Revoke S3 STS token for the given session token") public class RevokeSTSTokenHandler extends S3Handler { - @Option(names = "-k", - required = true, - description = "STS temporary access key id (for example, ASIA...)") - private String accessKeyId; - @Option(names = "-t", required = true, description = "STS session token") @@ -62,8 +56,8 @@ protected void execute(OzoneClient client, OzoneAddress address) throws IOException { if (!yes) { - out().print("Enter 'y' to confirm STS token revocation for accessKeyId '" + - accessKeyId + "': "); + out().print("Enter 'y' to confirm STS token revocation for sessionToken '" + + sessionToken + "': "); out().flush(); final Scanner scanner = new Scanner(new InputStreamReader(System.in, StandardCharsets.UTF_8)); final String confirmation = scanner.next().trim().toLowerCase(); @@ -73,7 +67,7 @@ protected void execute(OzoneClient client, OzoneAddress address) } } - client.getObjectStore().revokeSTSToken(accessKeyId, sessionToken); - out().println("STS token revoked for accessKeyId '" + accessKeyId + "'."); + client.getObjectStore().revokeSTSToken(sessionToken); + out().println("STS token revoked for sessionToken '" + sessionToken + "'."); } } diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java index 226ebbfb0349..4e6e1b0ae9de 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java @@ -768,12 +768,11 @@ public AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName, /** * Revokes an STS token. - * @param accessKeyId The STS accessKeyId (starting with ASIA...) - * @param sessionToken The STS session token + * @param sessionToken The STS sessionToken * @throws IOException if an error occurs while revoking the STS token */ - public void revokeSTSToken(String accessKeyId, String sessionToken) throws IOException { - proxy.revokeSTSToken(accessKeyId, sessionToken); + public void revokeSTSToken(String sessionToken) throws IOException { + proxy.revokeSTSToken(sessionToken); } /** diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java index 0067407aff33..88c282856164 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java @@ -1375,9 +1375,8 @@ AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName, int du /** * Revokes an STS token. - * @param accessKeyId The STS accessKeyId (starting with ASIA...) - * @param sessionToken The STS session token + * @param sessionToken The STS sessionToken * @throws IOException if an error occurs while revoking the STS token */ - void revokeSTSToken(String accessKeyId, String sessionToken) throws IOException; + void revokeSTSToken(String sessionToken) throws IOException; } diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 791a159f01be..67e2ac203bfd 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -2798,8 +2798,8 @@ public AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName, } @Override - public void revokeSTSToken(String accessKeyId, String sessionToken) throws IOException { - ozoneManagerClient.revokeSTSToken(accessKeyId, sessionToken); + public void revokeSTSToken(String sessionToken) throws IOException { + ozoneManagerClient.revokeSTSToken(sessionToken); } private static ExecutorService createThreadPoolExecutor( diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java index f98196d7276e..2661bc82366e 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java @@ -1194,11 +1194,10 @@ default AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName /** * Revokes an STS token. - * @param accessKeyId The STS accessKeyId (starting with ASIA...) - * @param sessionToken The STS session token + * @param sessionToken The STS sessionToken * @throws IOException if an error occurs while revoking the STS token */ - default void revokeSTSToken(String accessKeyId, String sessionToken) throws IOException { + default void revokeSTSToken(String sessionToken) throws IOException { throw new UnsupportedOperationException("OzoneManager does not require this to be implemented"); } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index 105d353a4637..8bbc3320dc1a 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -2676,10 +2676,9 @@ public AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName, } @Override - public void revokeSTSToken(String accessKeyId, String sessionToken) throws IOException { + public void revokeSTSToken(String sessionToken) throws IOException { final OzoneManagerProtocolProtos.RevokeSTSTokenRequest request = OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setAccessKeyId(accessKeyId) .setSessionToken(sessionToken) .build(); diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index b24aff586cea..08eb79fbd9ea 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -2387,8 +2387,7 @@ message AssumeRoleResponse { } message RevokeSTSTokenRequest { - required string accessKeyId = 1; - required string sessionToken = 2; + required string sessionToken = 1; } message RevokeSTSTokenResponse { diff --git a/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java b/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java index 7afe2c6249a9..82e04b3ff10e 100644 --- a/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java +++ b/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java @@ -489,7 +489,7 @@ String getMultipartKeyFSO(String volume, String bucket, String key, String * * @return Table. */ - Table getS3RevokedStsTokenTable(); + Table getS3RevokedStsTokenTable(); /** diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index 3439e04c0636..6b051611ee5c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -181,7 +181,7 @@ public class OmMetadataManagerImpl implements OMMetadataManager, private TypedTable snapshotRenamedTable; private TypedTable compactionLogTable; - private TypedTable s3RevokedStsTokenTable; + private TypedTable s3RevokedStsTokenTable; private OzoneManager ozoneManager; @@ -489,8 +489,8 @@ protected void initializeOmTables(CacheType cacheType, compactionLogTable = initializer.get(OMDBDefinition.COMPACTION_LOG_TABLE_DEF); - // temporaryAccessKeyId -> sessionToken - // FULL_CACHE keeps revocations in memory as there are not expected to be many revoked tokens + // sessionToken -> insertionTimeMillis + // FULL_CACHE keeps revocations in memory as there are not expected to be many s3RevokedStsTokenTable = initializer.get( OMDBDefinition.S3_REVOKED_STS_TOKEN_TABLE_DEF, CacheType.FULL_CACHE); } @@ -1691,7 +1691,7 @@ public Table getCompactionLogTable() { } @Override - public Table getS3RevokedStsTokenTable() { + public Table getS3RevokedStsTokenTable() { return s3RevokedStsTokenTable; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java index 8b4632ef45bf..c8a6ac239810 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java @@ -56,7 +56,7 @@ * | userTable | /user :- UserVolumeInfo | * | dTokenTable | OzoneTokenID :- renew_time | * | s3SecretTable | s3g_access_key_id :- s3Secret | - * | s3RevokedStsTokenTable | sts_access_key_id :- sessionToken | + * | s3RevokedStsTokenTable | sts_session_token :- insertionTimeMillis | * |------------------------------------------------------------------------| * } * @@ -163,11 +163,11 @@ public final class OMDBDefinition extends DBDefinition.WithMap { S3SecretValue.getCodec()); public static final String S3_REVOKED_STS_TOKEN_TABLE = "s3RevokedStsTokenTable"; - /** s3RevokedStsTokenTable: sts_access_key_id :- sessionToken.*/ - public static final DBColumnFamilyDefinition S3_REVOKED_STS_TOKEN_TABLE_DEF + /** s3RevokedStsTokenTable: sts_session_token :- insertionTimeMillis.*/ + public static final DBColumnFamilyDefinition S3_REVOKED_STS_TOKEN_TABLE_DEF = new DBColumnFamilyDefinition<>(S3_REVOKED_STS_TOKEN_TABLE, StringCodec.get(), - StringCodec.get()); + LongCodec.get()); //--------------------------------------------------------------------------- // Volume, Bucket, Prefix and Transaction Tables: diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3RevokeSTSTokenRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3RevokeSTSTokenRequest.java index ff7a3831d0d6..369fc8bc14ad 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3RevokeSTSTokenRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3RevokeSTSTokenRequest.java @@ -25,7 +25,6 @@ import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OzoneManager; -import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext; import org.apache.hadoop.ozone.om.request.OMClientRequest; import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; @@ -43,9 +42,9 @@ /** * Handles S3RevokeSTSTokenRequest request. * - *

This request marks an STS temporary access key id as revoked by inserting + *

This request marks an STS session token as revoked by inserting * it into the {@code s3RevokedStsTokenTable}. Subsequent S3 requests - * authenticated with the same STS access key id will be rejected when the + * authenticated with the same STS session token will be rejected when the * revocation state has propagated.

*/ public class S3RevokeSTSTokenRequest extends OMClientRequest { @@ -53,8 +52,6 @@ public class S3RevokeSTSTokenRequest extends OMClientRequest { private static final Logger LOG = LoggerFactory.getLogger(S3RevokeSTSTokenRequest.class); private static final Clock CLOCK = Clock.system(ZoneOffset.UTC); - private String originalAccessKeyId; - public S3RevokeSTSTokenRequest(OMRequest omRequest) { super(omRequest); } @@ -67,21 +64,14 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { // Get the original (long-lived) access key id from the session token // and enforce the same permission model that is used for S3 secret // operations (get/set/revoke). Only the owner of the original access - // key (or an S3 / tenant admin) is allowed to revoke its temporary - // STS credentials. + // key (i.e. the creator of the STS token) or an S3 / tenant admin is allowed + // to revoke its temporary STS credentials. final String sessionToken = revokeReq.getSessionToken(); - final String tempAccessKeyId = revokeReq.getAccessKeyId(); final STSTokenIdentifier stsTokenIdentifier = STSSecurityUtil.constructValidateAndDecryptSTSToken( sessionToken, ozoneManager.getSecretKeyClient(), CLOCK); - originalAccessKeyId = stsTokenIdentifier.getOriginalAccessKeyId(); - - // Validate that the Access Key ID in the request matches the one in the token - // to prevent users from revoking arbitrary keys using a valid token. - if (!stsTokenIdentifier.getTempAccessKeyId().equals(tempAccessKeyId)) { - throw new OMException("Access Key ID in request does not match the session token", - OMException.ResultCodes.INVALID_REQUEST); - } + final String originalAccessKeyId = stsTokenIdentifier.getOriginalAccessKeyId(); + final OzoneManagerProtocolProtos.UserInfo userInfo = getUserInfo(); final UserGroupInformation ugi = S3SecretRequestHelper.getOrCreateUgi(originalAccessKeyId); S3SecretRequestHelper.checkAccessIdSecretOpPermission(ozoneManager, ugi, originalAccessKeyId); @@ -89,7 +79,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { .setRevokeSTSTokenRequest(revokeReq) .setCmdType(getOmRequest().getCmdType()) .setClientId(getOmRequest().getClientId()) - .setUserInfo(getUserInfo()); + .setUserInfo(userInfo); if (getOmRequest().hasTraceID()) { omRequest.setTraceID(getOmRequest().getTraceID()); @@ -103,20 +93,20 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut final OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(getOmRequest()); final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeReq = getOmRequest().getRevokeSTSTokenRequest(); - final String accessKeyId = revokeReq.getAccessKeyId(); final String sessionToken = revokeReq.getSessionToken(); // All actual DB mutations are done in the response's addToDBBatch(). final OMClientResponse omClientResponse = new S3RevokeSTSTokenResponse( - accessKeyId, sessionToken, omResponse.build()); + sessionToken, omResponse.build()); // Audit log final Map auditMap = new HashMap<>(); - auditMap.put(OzoneConsts.S3_REVOKESTSTOKEN_USER, originalAccessKeyId); + final OzoneManagerProtocolProtos.UserInfo userInfo = getOmRequest().getUserInfo(); + auditMap.put(OzoneConsts.S3_REVOKESTSTOKEN_USER, userInfo.getUserName()); markForAudit(ozoneManager.getAuditLogger(), buildAuditMessage( - OMAction.REVOKE_STS_TOKEN, auditMap, null, getOmRequest().getUserInfo())); + OMAction.REVOKE_STS_TOKEN, auditMap, null, userInfo)); - LOG.info("Marked STS temporary access key '{}' as revoked.", accessKeyId); + LOG.info("Marked STS session token '{}' as revoked.", sessionToken); return omClientResponse; } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3RevokeSTSTokenResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3RevokeSTSTokenResponse.java index 523311bbadb8..5b1a8cf3b019 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3RevokeSTSTokenResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3RevokeSTSTokenResponse.java @@ -22,6 +22,8 @@ import jakarta.annotation.Nonnull; import java.io.IOException; +import java.time.Clock; +import java.time.ZoneOffset; import org.apache.hadoop.hdds.utils.db.BatchOperation; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.ozone.om.OMMetadataManager; @@ -35,22 +37,22 @@ @CleanupTableInfo(cleanupTables = {S3_REVOKED_STS_TOKEN_TABLE}) public class S3RevokeSTSTokenResponse extends OMClientResponse { - private final String accessKeyId; + private static final Clock CLOCK = Clock.system(ZoneOffset.UTC); + private final String sessionToken; - public S3RevokeSTSTokenResponse(String accessKeyId, String sessionToken, @Nonnull OMResponse omResponse) { + public S3RevokeSTSTokenResponse(String sessionToken, @Nonnull OMResponse omResponse) { super(omResponse); - this.accessKeyId = accessKeyId; this.sessionToken = sessionToken; } @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { - if (accessKeyId != null && getOMResponse().hasStatus() && getOMResponse().getStatus() == OK) { - final Table table = omMetadataManager.getS3RevokedStsTokenTable(); + if (sessionToken != null && getOMResponse().hasStatus() && getOMResponse().getStatus() == OK) { + final Table table = omMetadataManager.getS3RevokedStsTokenTable(); if (table != null) { - // Store sessionToken as value - table.putWithBatch(batchOperation, accessKeyId, sessionToken); + // Store insertionTimeMillis as value + table.putWithBatch(batchOperation, sessionToken, CLOCK.millis()); } } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/S3SecurityUtil.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/S3SecurityUtil.java index aeb4a2e189a3..792b64e8697c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/S3SecurityUtil.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/S3SecurityUtil.java @@ -73,7 +73,7 @@ public static void validateS3Credential(OMRequest omRequest, token, ozoneManager.getSecretKeyClient(), CLOCK); // Ensure the token is not revoked - if (isRevokedStsTempAccessKeyId(stsTokenIdentifier, ozoneManager)) { + if (isRevokedStsToken(token, ozoneManager)) { LOG.info("Session token has been revoked: {}, {}", stsTokenIdentifier.getTempAccessKeyId(), token); throw new OMException("STS token has been revoked", REVOKED_TOKEN); } @@ -140,9 +140,9 @@ private static void validateSTSTokenAwsSignature(STSTokenIdentifier stsTokenIden } /** - * Returns true if the STS token's temporary access key ID is present in the revoked STS token table. + * Returns true if the STS session token is present in the revoked STS token table. */ - private static boolean isRevokedStsTempAccessKeyId(STSTokenIdentifier stsTokenIdentifier, OzoneManager ozoneManager) + private static boolean isRevokedStsToken(String sessionToken, OzoneManager ozoneManager) throws OMException { try { final OMMetadataManager metadataManager = ozoneManager.getMetadataManager(); @@ -152,17 +152,14 @@ private static boolean isRevokedStsTempAccessKeyId(STSTokenIdentifier stsTokenId throw new OMException(msg, INTERNAL_ERROR); } - final Table revokedStsTokenTable = metadataManager.getS3RevokedStsTokenTable(); + final Table revokedStsTokenTable = metadataManager.getS3RevokedStsTokenTable(); if (revokedStsTokenTable == null) { final String msg = "Could not determine STS revocation: revokedStsTokenTable is null"; LOG.warn(msg); throw new OMException(msg, INTERNAL_ERROR); } - // When the STSTokenIdentifier is validated, it ensures the temp access key id is not null/empty - final String tempAccessKeyId = stsTokenIdentifier.getTempAccessKeyId(); - - return revokedStsTokenTable.getIfExist(tempAccessKeyId) != null; + return revokedStsTokenTable.getIfExist(sessionToken) != null; } catch (Exception e) { final String msg = "Could not determine STS revocation because of Exception: " + e.getMessage(); LOG.warn(msg, e); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java index 3541d303b246..bd5f2b56dd75 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java @@ -79,7 +79,7 @@ import org.apache.hadoop.hdds.protocol.StorageType; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.utils.TransactionInfo; -import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.hdds.utils.db.TypedTable; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.om.codec.OMDBDefinition; @@ -105,6 +105,7 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.PartKeyInfo; import org.apache.hadoop.ozone.snapshot.ListSnapshotResponse; import org.apache.hadoop.util.Time; +import org.apache.ozone.test.TestClock; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -1299,34 +1300,36 @@ public void testS3RevokedStsTokenTablePutAndGet() throws Exception { // Ensure the table is initialized assertNotNull(omMetadataManager.getS3RevokedStsTokenTable(), "s3RevokedStsTokenTable should be initialized"); - final String tempAccessKeyId1 = "ASIA7VUS1EOBCW8RRJVR"; + final TestClock clock = TestClock.newInstance(); final String sessionToken1 = "test-session-token-1"; - final String tempAccessKeyId2 = "ASIA904E65QIGL9ON305"; + final long insertionTime1 = clock.millis(); final String sessionToken2 = "test-session-token-2"; - - final Table table = omMetadataManager.getS3RevokedStsTokenTable(); + final long insertionTime2 = insertionTime1 + 1234L; // This table is configured as FULL_CACHE in OmMetadataManagerImpl. // A put() writes to RocksDB but does not update the table cache, so get() and getIfExist() will return null unless // the cache is updated with addCacheEntry(). getSkipCache() will read the DB instead of the cache. - table.put(tempAccessKeyId1, sessionToken1); - table.put(tempAccessKeyId2, sessionToken2); + final TypedTable revokedTable = + (TypedTable) omMetadataManager.getS3RevokedStsTokenTable(); + + revokedTable.put(sessionToken1, insertionTime1); + revokedTable.put(sessionToken2, insertionTime2); // Verify the values are persisted in RocksDB. - assertEquals(sessionToken1, table.getSkipCache(tempAccessKeyId1)); - assertEquals(sessionToken2, table.getSkipCache(tempAccessKeyId2)); + assertEquals(insertionTime1, revokedTable.getSkipCache(sessionToken1)); + assertEquals(insertionTime2, revokedTable.getSkipCache(sessionToken2)); // Update cache to make get/getIfExist reflect the write for FULL_CACHE tables. - table.addCacheEntry(tempAccessKeyId1, sessionToken1, 1L); - table.addCacheEntry(tempAccessKeyId2, sessionToken2, 1L); + revokedTable.addCacheEntry(sessionToken1, insertionTime1, 1L); + revokedTable.addCacheEntry(sessionToken2, insertionTime2, 1L); // Verify get and getIfExist return the stored value - assertEquals(sessionToken1, table.get(tempAccessKeyId1)); - assertEquals(sessionToken1, table.getIfExist(tempAccessKeyId1)); - assertEquals(sessionToken2, table.get(tempAccessKeyId2)); - assertEquals(sessionToken2, table.getIfExist(tempAccessKeyId2)); + assertEquals(insertionTime1, revokedTable.get(sessionToken1)); + assertEquals(insertionTime1, revokedTable.getIfExist(sessionToken1)); + assertEquals(insertionTime2, revokedTable.get(sessionToken2)); + assertEquals(insertionTime2, revokedTable.getIfExist(sessionToken2)); - // Unknown key should return null for getIfExist - assertNull(table.getIfExist("ASIA_UNKNOWN_ACCESS_KEY")); + // Invalid sessionToken should return null for getIfExist + assertNull(revokedTable.getIfExist("INVALID_SESSION_TOKEN")); } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3RevokeSTSTokenRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3RevokeSTSTokenRequest.java index 9a68c047f008..d4460ad83e6e 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3RevokeSTSTokenRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3RevokeSTSTokenRequest.java @@ -64,7 +64,7 @@ public void setUp() throws Exception { KerberosName.setRuleMechanism(DEFAULT_MECHANISM); KerberosName.setRules( "RULE:[2:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" + "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" + "DEFAULT"); - + secretKeyClient = new SecretKeyTestClient(); stsTokenSecretManager = new STSTokenSecretManager(secretKeyClient); // Multi-tenant manager mock used for tests that exercise the S3 multi-tenancy permission branch. @@ -98,7 +98,6 @@ public void testPreExecuteFailsForNonOwnerOfOriginalAccessKey() throws Exception final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setAccessKeyId(tempAccessKeyId) .setSessionToken(sessionToken) .build(); @@ -135,7 +134,6 @@ public void testPreExecuteSucceedsForOriginalAccessKeyOwner() throws Exception { final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setAccessKeyId(tempAccessKeyId) .setSessionToken(sessionToken) .build(); @@ -179,7 +177,6 @@ public void testPreExecuteSucceedsForTenantAccessIdOwner() throws Exception { final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setAccessKeyId(tempAccessKeyId) .setSessionToken(sessionToken) .build(); @@ -224,7 +221,6 @@ public void testPreExecuteSucceedsForTenantAdmin() throws Exception { final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setAccessKeyId(tempAccessKeyId) .setSessionToken(sessionToken) .build(); @@ -271,7 +267,6 @@ public void testPreExecuteFailsForNonOwnerNonAdminInTenant() throws Exception { final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setAccessKeyId(tempAccessKeyId) .setSessionToken(sessionToken) .build(); @@ -288,46 +283,6 @@ public void testPreExecuteFailsForNonOwnerNonAdminInTenant() throws Exception { assertEquals(OMException.ResultCodes.USER_MISMATCH, ex.getResult()); } - @Test - public void testPreExecuteFailsForMismatchedAccessKeyId() throws Exception { - // Verify that if the request access key id does not match the one inside the session token, the request is - // rejected. This prevents a user with a valid session token from revoking arbitrary STS credentials. - final String tempAccessKeyId = "ASIA123456789"; - final String otherAccessKeyId = "ASI987654321"; - final String originalAccessKeyId = "original-access-key-id"; - final String sessionToken = createSessionToken(tempAccessKeyId, originalAccessKeyId); - - // Caller is the owner of the session token, so permissions should pass - final UserGroupInformation originalUgi = UserGroupInformation.createRemoteUser(originalAccessKeyId); - Server.getCurCall().set(new StubCall(originalUgi)); - - final OMException ex; - try (OzoneManager ozoneManager = mock(OzoneManager.class)) { - when(ozoneManager.isS3MultiTenancyEnabled()).thenReturn(false); - when(ozoneManager.isS3Admin(any(UserGroupInformation.class))) - .thenReturn(false); - when(ozoneManager.getSecretKeyClient()).thenReturn(secretKeyClient); - - // Request tries to revoke otherAccessKeyId using a token for tempAccessKeyId - final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = - OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setAccessKeyId(otherAccessKeyId) - .setSessionToken(sessionToken) - .build(); - - final OMRequest omRequest = OMRequest.newBuilder() - .setClientId(UUID.randomUUID().toString()) - .setCmdType(Type.RevokeSTSToken) - .setRevokeSTSTokenRequest(revokeRequest) - .build(); - - final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(omRequest); - - ex = assertThrows(OMException.class, () -> omClientRequest.preExecute(ozoneManager)); - } - assertEquals(OMException.ResultCodes.INVALID_REQUEST, ex.getResult()); - } - /** * Stub used to inject a remote user into the ProtobufRpcEngine.Server.getRemoteUser() thread-local. */ diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/package-info.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/package-info.java new file mode 100644 index 000000000000..7727afbfc68c --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/package-info.java @@ -0,0 +1,21 @@ +/* + * 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 contains test classes for S3 Security requests. + */ +package org.apache.hadoop.ozone.om.request.s3.security; diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestS3SecurityUtil.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestS3SecurityUtil.java index c5cce385071b..20b3f3fb28b7 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestS3SecurityUtil.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestS3SecurityUtil.java @@ -65,7 +65,7 @@ public void testValidateS3CredentialFailsWhenTokenRevoked() throws Exception { // If the revoked STS token table contains an entry for the temporary access key id extracted from the session // token, validateS3Credential should reject the request with REVOKED_TOKEN final OMMetadataManager metadataManager = mock(OMMetadataManager.class); - final Table revokedSTSTokenTable = new InMemoryTestTable<>(); + final Table revokedSTSTokenTable = new InMemoryTestTable<>(); validateS3CredentialHelper( "session-token-a", metadataManager, revokedSTSTokenTable, true, createSTSTokenIdentifier(), REVOKED_TOKEN, "STS token has been revoked"); @@ -83,7 +83,7 @@ public void testValidateS3CredentialWhenMetadataUnavailable() throws Exception { public void testValidateS3CredentialSuccessWhenNotRevoked() throws Exception { // Normal case: token is NOT revoked and request is accepted final OMMetadataManager metadataManager = mock(OMMetadataManager.class); - final Table revokedSTSTokenTable = new InMemoryTestTable<>(); + final Table revokedSTSTokenTable = new InMemoryTestTable<>(); validateS3CredentialHelper( "session-token-c", metadataManager, revokedSTSTokenTable, false, createSTSTokenIdentifier(), null, null); @@ -102,7 +102,7 @@ public void testValidateS3CredentialWhenMetadataManagerAvailableButRevokedTableN public void testValidateS3CredentialWhenTableThrowsException() throws Exception { // If the revoked STS token table lookup throws, throws INTERNAL_ERROR (wrapped) final OMMetadataManager metadataManager = mock(OMMetadataManager.class); - final Table revokedSTSTokenTable = spy(new InMemoryTestTable<>()); + final Table revokedSTSTokenTable = spy(new InMemoryTestTable<>()); doThrow(new RuntimeException("lookup failed")).when(revokedSTSTokenTable).getIfExist(anyString()); validateS3CredentialHelper( "session-token-g", metadataManager, revokedSTSTokenTable, false, createSTSTokenIdentifier(), @@ -110,7 +110,7 @@ public void testValidateS3CredentialWhenTableThrowsException() throws Exception } private void validateS3CredentialHelper(String sessionToken, OMMetadataManager metadataManager, - Table revokedSTSTokenTable, boolean isRevoked, STSTokenIdentifier stsTokenIdentifier, + Table revokedSTSTokenTable, boolean isRevoked, STSTokenIdentifier stsTokenIdentifier, OMException.ResultCodes expectedResult, String expectedMessageContents) throws Exception { try (OzoneManager ozoneManager = mock(OzoneManager.class)) { @@ -124,10 +124,8 @@ private void validateS3CredentialHelper(String sessionToken, OMMetadataManager m final String tempAccessKeyId = "temp-access-key-id"; if (isRevoked) { - if (revokedSTSTokenTable == null) { - throw new IllegalArgumentException("revokedSTSTokenTable must not be null when isRevoked=true"); - } - revokedSTSTokenTable.put(tempAccessKeyId, sessionToken); + final long insertionTimeMillis = CLOCK.millis(); + revokedSTSTokenTable.put(sessionToken, insertionTimeMillis); } try (MockedStatic stsSecurityUtilMock = mockStatic(STSSecurityUtil.class, CALLS_REAL_METHODS); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java index b56c6ca3fe8a..477b6876af85 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java @@ -815,6 +815,6 @@ public AssumeRoleResponseInfo assumeRole( } @Override - public void revokeSTSToken(String accessKeyId, String sessionToken) throws IOException { + public void revokeSTSToken(String sessionToken) throws IOException { } }