diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index e7e78b812063..188bd65559ff 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -314,6 +314,7 @@ public final class OzoneConsts { public static final String S3_SETSECRET_USER = "S3SetSecretUser"; public static final String S3_REVOKESECRET_USER = "S3RevokeSecretUser"; public static final String S3_REVOKESTSTOKEN_USER = "S3RevokeSTSTokenUser"; + public static final String S3_STS_TEMP_ACCESS_KEY_ID = "tempAccessKeyId"; public static final String RENAMED_KEYS_MAP = "renamedKeysMap"; public static final String UNRENAMED_KEYS_MAP = "unRenamedKeysMap"; public static final String MULTIPART_UPLOAD_PART_NUMBER = "partNumber"; diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index c72c351402e3..db79aa505f01 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -5254,9 +5254,10 @@ 3h OZONE, OM, PERFORMANCE, SECURITY - A background job that periodically checks revoked STS token entries and - deletes ones that have existed for 12 hours. This entry controls the interval of this - cleanup check. Unit could be defined with postfix (ns,ms,s,m,h,d). + A background service that periodically scans the s3RevokedStsTokenTable and deletes + revocation entries whose cutoff is older than the maximum STS token lifetime (12 hours). + This property controls how often the cleanup service runs. Unit could be defined with + postfix (ns,ms,s,m,h,d). diff --git a/hadoop-hdds/docs/content/design/ozone-sts.md b/hadoop-hdds/docs/content/design/ozone-sts.md index 6cc94eadd4bc..7ee1d9fcd73e 100644 --- a/hadoop-hdds/docs/content/design/ozone-sts.md +++ b/hadoop-hdds/docs/content/design/ozone-sts.md @@ -139,17 +139,24 @@ was included with the AssumeRole request, the String return value will also incl would further limit the scope of the permissions, resources and actions granted by the role in Ranger, such that the temporary credential will have the permissions and actions comprising the intersection of the role permissions and actions and the sessionPolicy permissions and actions. - HMAC-SHA256 signature - used to ensure the sessionToken was created by Ozone and was not altered since it was created. +- creation time of the token (via `OMTokenProto#issueDate`, exposed as `STSTokenIdentifier#getCreationTime()`) - expiration time of the token (via `ShortLivedTokenIdentifier#getExpiry()`) - UUID of the OzoneManager secret key used to sign the sessionToken and encrypt the secretAccessKey (via `ShortLivedTokenIdentifier#getSecretKeyId()`) ## 3.5 STS Token Revocation In the rare event temporary credentials need to be revoked (ex. for security reasons), a table in the OzoneManager RocksDB will be created -to store revoked tokens, and a command-line utility will be created to add tokens to the table. A background cleaner service -will be created to run every 3 hours to delete revoked tokens that have been in the table for more than 12 hours. The -input parameter for the command-line utility will be the sessionToken - this value is returned in plain text as a result -of the AssumeRole call (mentioned above). In this way, specific STS tokens can be revoked as opposed to all tokens. Furthermore, -AWS doesn't have a standard API to revoke tokens therefore we are creating our own system. +to store revocation cutoffs per originalAccessKeyId, and a command-line utility will be created to add entries to the table. +A background cleaner service will be created to run every 3 hours to delete revocation entries whose cutoff is more than 12 hours old. + +The command-line utility accepts only `originalAccessKeyId`. The OM stores revocations by keying the table on +`originalAccessKeyId` and storing the revocation cutoff time in milliseconds as the value. When the command is issued, +all STS tokens created by that `originalAccessKeyId` whose signed `creationTime` is strictly before the cutoff are +revoked. Tokens created at or after the cutoff remain valid. + +Before writing a revocation entry, the OM verifies that `originalAccessKeyId` corresponds to a real Kerberos identity by +checking that an S3 secret exists for it. This prevents bogus entries from filling the table. Non-admins may only +revoke their own `originalAccessKeyId`; S3 and tenant admins may revoke other principals. Additionally, if the Kerberos identity of the user that created the STS token is revoked via the `ozone s3 revokesecret` command, then all the existing and unexpired STS tokens that user created will be revoked. @@ -221,7 +228,8 @@ created in Ranger as per the Prerequisites above. originalAccessKeyId in the session token and perform the following checks: - Ensure that if the accessKeyId starts with "ASIA", that a sessionToken was included in the `x-amz-security-token` header - Ensure the sessionToken is not expired - - Ensure the sessionToken is not revoked via a `keyMayExist` check in OzoneManager RocksDB + - Ensure the STS credentials are not revoked by looking up the revocation cutoff for the token's originalAccessKeyId + and comparing it against the token's signed creationTime - Validate the HMAC-SHA256 signature in the sessionToken - Decrypt the secretAccessKey from the sessionToken and validate the AWS signature - Authorize the call with either RangerOzoneAuthorizer or OzoneNativeAuthorizer 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 274304217f86..9cd715d581a1 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,18 +29,18 @@ /** * Executes revocation of STS tokens. * - *

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.

+ *

This command records a revocation cutoff for the given original access key ID in the OM's + * revoked STS token table. Subsequent S3 requests using STS tokens created before that cutoff + * will be rejected once the revocation state has propagated.

*/ @Command(name = "revokeststoken", - description = "Revoke S3 STS token for the given session token") + description = "Revoke S3 STS tokens for the given original access key ID") public class RevokeSTSTokenHandler extends S3Handler { - @Option(names = "-t", + @Option(names = {"-o", "--original-access-key-id"}, required = true, - description = "STS session token") - private String sessionToken; + description = "Original long-lived access key ID whose STS tokens should be revoked") + private String originalAccessKeyId; @Option(names = "-y", description = "Continue without interactive user confirmation") @@ -56,8 +56,8 @@ protected void execute(OzoneClient client, OzoneAddress address) throws IOException { if (!yes) { - out().print("Enter 'y' to confirm STS token revocation for sessionToken '" + - sessionToken + "': "); + out().print( + "Enter 'y' to confirm STS token revocation for originalAccessKeyId '" + originalAccessKeyId + "': "); out().flush(); final Scanner scanner = new Scanner(new InputStreamReader(System.in, StandardCharsets.UTF_8)); final String confirmation = scanner.next().trim().toLowerCase(); @@ -67,7 +67,7 @@ protected void execute(OzoneClient client, OzoneAddress address) } } - client.getObjectStore().revokeSTSToken(sessionToken); - out().println("STS token revoked for sessionToken '" + sessionToken + "'."); + client.getObjectStore().revokeSTSToken(originalAccessKeyId); + out().println("STS tokens revoked for originalAccessKeyId '" + originalAccessKeyId + "'."); } } 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 bd045ef04e03..ce0f780b72de 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 @@ -813,12 +813,12 @@ public AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName, } /** - * Revokes an STS token. - * @param sessionToken The STS sessionToken + * Revokes STS tokens for the given original access key ID. + * @param originalAccessKeyId The original long-lived access key ID whose STS tokens to revoke * @throws IOException if an error occurs while revoking the STS token */ - public void revokeSTSToken(String sessionToken) throws IOException { - proxy.revokeSTSToken(sessionToken); + public void revokeSTSToken(String originalAccessKeyId) throws IOException { + proxy.revokeSTSToken(originalAccessKeyId); } /** 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 807cd2757cfc..b5f7baa0ef2c 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 @@ -1648,11 +1648,11 @@ AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName, int du String awsIamSessionPolicy, String requestId) throws IOException; /** - * Revokes an STS token. - * @param sessionToken The STS sessionToken + * Revokes STS tokens for the given original access key ID. + * @param originalAccessKeyId The original long-lived access key ID whose STS tokens to revoke * @throws IOException if an error occurs while revoking the STS token */ - void revokeSTSToken(String sessionToken) throws IOException; + void revokeSTSToken(String originalAccessKeyId) throws IOException; /** * Gets the lifecycle configuration information. 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 733c915dcd03..9ca47013462d 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 @@ -3022,8 +3022,8 @@ public AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName, } @Override - public void revokeSTSToken(String sessionToken) throws IOException { - ozoneManagerClient.revokeSTSToken(sessionToken); + public void revokeSTSToken(String originalAccessKeyId) throws IOException { + ozoneManagerClient.revokeSTSToken(originalAccessKeyId); } @Override diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3STSUtils.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3STSUtils.java index 763c8fe9bfa4..642210896559 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3STSUtils.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3STSUtils.java @@ -40,6 +40,12 @@ public final class S3STSUtils { // AWS limit for session policy is 2048 characters public static final int MAX_SESSION_POLICY_LENGTH = 2048; + public static final String STS_TOKEN_PREFIX = "ASIA"; + public static final String STS_ACCESS_KEY_ID_ALLOWED_CHARS = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + public static final int STS_ACCESS_KEY_ID_ALLOWED_CHARS_LENGTH = STS_ACCESS_KEY_ID_ALLOWED_CHARS.length(); + public static final int STS_ACCESS_KEY_ID_RANDOM_LENGTH = 20; + public static final int STS_ACCESS_KEY_ID_LENGTH = STS_TOKEN_PREFIX.length() + STS_ACCESS_KEY_ID_RANDOM_LENGTH; + private S3STSUtils() { } 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 604669487c68..46254e3d6f63 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 @@ -1336,11 +1336,11 @@ default AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName } /** - * Revokes an STS token. - * @param sessionToken The STS sessionToken + * Revokes STS tokens for the given original access key ID. + * @param originalAccessKeyId The original long-lived access key ID whose STS tokens to revoke * @throws IOException if an error occurs while revoking the STS token */ - default void revokeSTSToken(String sessionToken) throws IOException { + default void revokeSTSToken(String originalAccessKeyId) 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 c60bc60db700..7077fd1b02c7 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 @@ -2981,10 +2981,10 @@ public AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName, } @Override - public void revokeSTSToken(String sessionToken) throws IOException { + public void revokeSTSToken(String originalAccessKeyId) throws IOException { final OzoneManagerProtocolProtos.RevokeSTSTokenRequest request = OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setSessionToken(sessionToken) + .setOriginalAccessKeyId(originalAccessKeyId) .build(); final OMRequest omRequest = createOMRequest(Type.RevokeSTSToken) diff --git a/hadoop-ozone/dist/src/main/smoketest/security/ozone-secure-sts.robot b/hadoop-ozone/dist/src/main/smoketest/security/ozone-secure-sts.robot index a1f34e818b76..4d5903261bf7 100644 --- a/hadoop-ozone/dist/src/main/smoketest/security/ozone-secure-sts.robot +++ b/hadoop-ozone/dist/src/main/smoketest/security/ozone-secure-sts.robot @@ -66,6 +66,7 @@ ${ACTION_MATCHES_PUTOBJECT_CREATE_WRITE_ROLE_ARN} arn:aws:iam::123456789012:rol ${ACTION_MATCHES_GETOBJECT_PUTOBJECT_ROLE_ARN} arn:aws:iam::123456789012:role/${ACTION_MATCHES_GETOBJECT_PUTOBJECT_ROLE} ${ACTION_MATCHES_UPLOADPARTCOPY_EXPECTED_OWNER_ROLE_ARN} arn:aws:iam::123456789012:role/${ACTION_MATCHES_UPLOADPARTCOPY_EXPECTED_OWNER_ROLE} ${ACTION_MATCHES_GET_STAR_READ_ROLE_ARN} arn:aws:iam::123456789012:role/${ACTION_MATCHES_GET_STAR_READ_ROLE} +${TEST_USER_ADMIN} testuser ${TEST_USER_NON_ADMIN} testuser2 @{ICEBERG_OBJECT_KEYS} file1.txt file1again.txt folder/pepper.txt folder/salt.txt userA/userA.txt userB/userB.txt userAfile.txt @{ICEBERG_LISTABLE_OBJECT_KEYS_OBS} file1.txt file1again.txt folder/pepper.txt folder/salt.txt userA/userA.txt userB/userB.txt userAfile.txt zeroByteFile zeroByteFolder/ @@ -254,6 +255,17 @@ Configure STS Profile With Bogus Credential Part Configure STS Profile ${STS_ACCESS_KEY_ID} ${STS_SECRET_KEY} bogusSessionToken END +Verify STS Token Revocation And Post Revocation Assume Role + [Arguments] ${bucket} ${role_arn} ${revoker_user} ${revoker_keytab} + Assume Role And Configure STS Profile perm_access_key_id=${PERMANENT_ACCESS_KEY_ID} perm_secret_key=${PERMANENT_SECRET_KEY} role_arn=${role_arn} + Get Object Should Succeed ${bucket} ${ICEBERG_BUCKET_TESTFILE} + Kinit test user ${revoker_user} ${revoker_keytab} + ${output} = Execute ozone s3 revokeststoken -o ${PERMANENT_ACCESS_KEY_ID} -y ${OM_HA_PARAM} + Should Contain ${output} STS tokens revoked for originalAccessKeyId + Get Object Should Fail ${bucket} ${ICEBERG_BUCKET_TESTFILE} AccessDenied + Assume Role And Configure STS Profile perm_access_key_id=${PERMANENT_ACCESS_KEY_ID} perm_secret_key=${PERMANENT_SECRET_KEY} role_arn=${role_arn} + Get Object Should Succeed ${bucket} ${ICEBERG_BUCKET_TESTFILE} + *** Test Cases *** Create User in Ranger ${user_json} = Set Variable { "loginId": "${ICEBERG_SVC_CATALOG_USER}", "name": "${ICEBERG_SVC_CATALOG_USER}", "password": "Password123", "firstName": "Iceberg REST", "lastName": "Catalog", "emailAddress": "${ICEBERG_SVC_CATALOG_USER}@example.com", "userRoleList": ["ROLE_USER"], "userPermList": [ { "moduleId": 1, "isAllowed": 1 }, { "moduleId": 3, "isAllowed": 1 }, { "moduleId": 7, "isAllowed": 1 } ] } @@ -555,27 +567,32 @@ Verify Token Revocation via CLI FOR ${bucket} ${role_arn} IN ... ${ICEBERG_BUCKET_OBS} ${ICEBERG_ALL_ACCESS_ROLE_OBS_ARN} ... ${ICEBERG_BUCKET_FSO} ${ICEBERG_ALL_ACCESS_ROLE_FSO_ARN} - Assume Role And Configure STS Profile perm_access_key_id=${PERMANENT_ACCESS_KEY_ID} perm_secret_key=${PERMANENT_SECRET_KEY} role_arn=${role_arn} - ${output} = Execute ozone s3 revokeststoken -t ${STS_SESSION_TOKEN} -y ${OM_HA_PARAM} - Should Contain ${output} STS token revoked for sessionToken - # Trying to use the token for even get-object should now fail. - Get Object Should Fail ${bucket} ${ICEBERG_BUCKET_TESTFILE} AccessDenied + # Owner of the original access key can revoke the STS token. + Verify STS Token Revocation And Post Revocation Assume Role ${bucket} ${role_arn} ${ICEBERG_SVC_CATALOG_USER} ${ICEBERG_SVC_CATALOG_USER}.keytab + # S3 admin can also revoke an STS token owned by another user. + Verify STS Token Revocation And Post Revocation Assume Role ${bucket} ${role_arn} ${TEST_USER_ADMIN} ${TEST_USER_ADMIN}.keytab END Non-Admin Cannot Revoke STS Token FOR ${role_arn} IN ${ICEBERG_ALL_ACCESS_ROLE_OBS_ARN} ${ICEBERG_ALL_ACCESS_ROLE_FSO_ARN} # Create a token first. Assume Role And Get Temporary Credentials perm_access_key_id=${PERMANENT_ACCESS_KEY_ID} perm_secret_key=${PERMANENT_SECRET_KEY} role_arn=${role_arn} - ${token_to_revoke} = Set Variable ${STS_SESSION_TOKEN} # Kinit as non-admin user. Kinit test user ${TEST_USER_NON_ADMIN} ${TEST_USER_NON_ADMIN}.keytab # Try to revoke - should give USER_MISMATCH error. - ${output} = Execute And Ignore Error ozone s3 revokeststoken -t ${token_to_revoke} -y ${OM_HA_PARAM} + ${output} = Execute And Ignore Error ozone s3 revokeststoken -o ${PERMANENT_ACCESS_KEY_ID} -y ${OM_HA_PARAM} Should Contain ${output} USER_MISMATCH END +Revoke STS Token Should Fail For Unknown Original Access Key Id + # Revoking a bogus originalAccessKeyId must fail before writing to the revocation table. + Kinit test user ${TEST_USER_ADMIN} ${TEST_USER_ADMIN}.keytab + ${output} = Execute And Ignore Error ozone s3 revokeststoken -o bogus-original-access-key-id -y ${OM_HA_PARAM} + Should Contain ${output} ACCESS_ID_NOT_FOUND + Should Contain ${output} does not exist + List Objects V1 and V2 IAM Session Policy Matrix for OBS and FSO Kinit test user ${ICEBERG_SVC_CATALOG_USER} ${ICEBERG_SVC_CATALOG_USER}.keytab @@ -610,6 +627,13 @@ Tampered STS Token Service, Policy, or Signature Must Fail Get Object Should Fail ${ICEBERG_BUCKET_OBS} ${ICEBERG_BUCKET_TESTFILE} AccessDenied Put Object Should Fail ${ICEBERG_BUCKET_OBS} ${ICEBERG_BUCKET_TESTFILE} AccessDenied + # Exercise malformed token decoding with incorrect token structure. Unlike the earlier + # bogusSessionToken credential-part check, this literal decodes to a negative Writable + # length and covers unchecked decoder failures such as NegativeArraySizeException. + Configure STS Profile ${STS_ACCESS_KEY_ID} ${STS_SECRET_KEY} not-a-valid-token + Get Object Should Fail ${ICEBERG_BUCKET_OBS} ${ICEBERG_BUCKET_TESTFILE} AccessDenied + Put Object Should Fail ${ICEBERG_BUCKET_OBS} ${ICEBERG_BUCKET_TESTFILE} AccessDenied + Assume Role Session Policy With Multiple Buckets Should Access All Buckets ${session_policy} = Set Variable {"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":"arn:aws:s3:::${ICEBERG_BUCKET_OBS}/*"},{"Effect":"Allow","Action":"s3:GetObject","Resource":"arn:aws:s3:::${ICEBERG_BUCKET_FSO}/*"}]} Assume Role And Get Temporary Credentials policy_json=${session_policy} perm_access_key_id=${PERMANENT_ACCESS_KEY_ID} perm_secret_key=${PERMANENT_SECRET_KEY} role_arn=${ICEBERG_MULTI_BUCKET_ROLE_ARN} diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index c06d54209a0b..638cf99bd1ce 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -2534,18 +2534,20 @@ message UpdateAssumeRoleRequest { } message RevokeSTSTokenRequest { - required string sessionToken = 1; + required string originalAccessKeyId = 1; + // Leader-generated revocation cutoff, replicated across OMs in HA mode. + optional uint64 revocationTimeMillis = 2; } message RevokeSTSTokenResponse { } /** - This will contain a list of revoked STS session tokens whose entries should be removed from + This will contain a list of originalAccessKeyIds whose revocation entries should be removed from the s3RevokedStsTokenTable. */ message DeleteRevokedSTSTokensRequest { - repeated string sessionToken = 1; + repeated string originalAccessKeyId = 1; } message DeleteRevokedSTSTokensResponse { 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 97f6cf920365..f1a7a51d3e26 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 @@ -540,7 +540,7 @@ protected void initializeOmTables(CacheType cacheType, compactionLogTable = initializer.get(OMDBDefinition.COMPACTION_LOG_TABLE_DEF); - // sessionToken -> insertionTimeMillis + // originalAccessKeyId -> revocationTimeMillis // 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); 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 2e99871e17c4..08600bb99504 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 @@ -60,7 +60,7 @@ * | userTable | /user :- UserVolumeInfo | * | dTokenTable | OzoneTokenID :- renew_time | * | s3SecretTable | s3g_access_key_id :- s3Secret | - * | s3RevokedStsTokenTable | sts_session_token :- insertionTimeMillis | + * | s3RevokedStsTokenTable | originalAccessKeyId :- revocationTimeMillis | * |------------------------------------------------------------------------| * } * @@ -169,7 +169,10 @@ public final class OMDBDefinition extends DBDefinition.WithMap { S3SecretValue.getCodec()); public static final String S3_REVOKED_STS_TOKEN_TABLE = "s3RevokedStsTokenTable"; - /** s3RevokedStsTokenTable: sts_session_token :- insertionTimeMillis.*/ + /** + * s3RevokedStsTokenTable: originalAccessKeyId :- revocationTimeMillis. + * The value is the revocation cutoff in milliseconds. + */ public static final DBColumnFamilyDefinition S3_REVOKED_STS_TOKEN_TABLE_DEF = new DBColumnFamilyDefinition<>(S3_REVOKED_STS_TOKEN_TABLE, StringCodec.get(), diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java index 3db85f508051..5b9c453d8936 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java @@ -25,6 +25,7 @@ import com.google.common.base.Preconditions; import com.google.common.util.concurrent.ThreadFactoryBuilder; import java.io.IOException; +import java.time.Instant; import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; @@ -683,15 +684,22 @@ OMResponse runCommand(OMRequest request, TermIndex termIndex) { 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( - s3Auth.hasResolvedStsTempAccessKeyId() ? s3Auth.getResolvedStsTempAccessKeyId() : "", - s3Auth.hasResolvedStsOriginalAccessKeyId() ? s3Auth.getResolvedStsOriginalAccessKeyId() : "", - s3Auth.hasResolvedStsRoleArn() ? s3Auth.getResolvedStsRoleArn() : "", - java.time.Instant.MAX, // ensure it deterministically is not expired - "", // no secretAccessKey needed - s3Auth.hasResolvedStsSessionPolicy() ? s3Auth.getResolvedStsSessionPolicy() : "", - null // no encryption key needed - ); + 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 + .setSessionPolicy( + s3Auth.hasResolvedStsSessionPolicy() ? s3Auth.getResolvedStsSessionPolicy() : "") + .setManagedSecretKey(null) // no ManagedSecretKey needed + .build()); OzoneManager.setStsTokenIdentifier(rehydratedTokenIdentifier); isStsThreadLocalSet = true; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java index 6b9c6698cf9f..29abe0eb8eec 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java @@ -46,10 +46,10 @@ import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OMAuditLogger; import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; +import org.apache.hadoop.ozone.om.helpers.S3STSUtils; import org.apache.hadoop.ozone.om.lock.OMLockDetails; import org.apache.hadoop.ozone.om.protocolPB.grpc.GrpcClientConstants; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils; -import org.apache.hadoop.ozone.om.request.s3.security.S3AssumeRoleRequest; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.om.upgrade.OMLayoutVersionManager; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; @@ -226,7 +226,7 @@ public OzoneManagerProtocolProtos.UserInfo getUserInfo() throws IOException { // falling back to accessId if session token not present. if (omRequest.hasS3Authentication()) { final String accessKeyId = omRequest.getS3Authentication().getAccessId(); - if (accessKeyId.startsWith(S3AssumeRoleRequest.STS_TOKEN_PREFIX) && + if (accessKeyId.startsWith(S3STSUtils.STS_TOKEN_PREFIX) && !omRequest.getS3Authentication().hasSessionToken()) { throw new IOException("Error with STS token", new AuthenticationException( "Missing session token for accessKeyId: " + accessKeyId)); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3AssumeRoleRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3AssumeRoleRequest.java index 4efd18b4b327..b6d650cc4393 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3AssumeRoleRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3AssumeRoleRequest.java @@ -17,6 +17,10 @@ package org.apache.hadoop.ozone.om.request.s3.security; +import static org.apache.hadoop.ozone.om.helpers.S3STSUtils.STS_ACCESS_KEY_ID_ALLOWED_CHARS; +import static org.apache.hadoop.ozone.om.helpers.S3STSUtils.STS_ACCESS_KEY_ID_ALLOWED_CHARS_LENGTH; +import static org.apache.hadoop.ozone.om.helpers.S3STSUtils.STS_ACCESS_KEY_ID_RANDOM_LENGTH; +import static org.apache.hadoop.ozone.om.helpers.S3STSUtils.STS_TOKEN_PREFIX; import static org.apache.hadoop.ozone.security.acl.AssumeRoleRequest.OzoneGrant; import com.google.common.annotations.VisibleForTesting; @@ -31,6 +35,7 @@ import java.util.Set; import org.apache.hadoop.hdds.scm.client.HddsClientUtils; import org.apache.hadoop.ipc_.ProtobufRpcEngine; +import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OzoneAclUtils; @@ -70,16 +75,12 @@ public class S3AssumeRoleRequest extends OMClientRequest { SECURE_RANDOM = secureRandom; } - private static final int STS_ACCESS_KEY_ID_LENGTH = 20; private static final int STS_SECRET_ACCESS_KEY_LENGTH = 40; private static final int STS_ROLE_ID_LENGTH = 16; private static final String ASSUME_ROLE_ID_PREFIX = "AROA"; - private static final String CHARS_FOR_ACCESS_KEY_IDS = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; - private static final int CHARS_FOR_ACCESS_KEY_IDS_LENGTH = CHARS_FOR_ACCESS_KEY_IDS.length(); - private static final String CHARS_FOR_SECRET_ACCESS_KEYS = CHARS_FOR_ACCESS_KEY_IDS + + private static final String CHARS_FOR_SECRET_ACCESS_KEYS = STS_ACCESS_KEY_ID_ALLOWED_CHARS + "abcdefghijklmnopqrstuvwxyz/+"; private static final int CHARS_FOR_SECRET_ACCESS_KEYS_LENGTH = CHARS_FOR_SECRET_ACCESS_KEYS.length(); - public static final String STS_TOKEN_PREFIX = "ASIA"; private final Clock clock; @@ -103,11 +104,13 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { // Generate temporary AWS credentials using cryptographically strong SecureRandom final String tempAccessKeyId = STS_TOKEN_PREFIX + generateSecureRandomStringUsingChars( - CHARS_FOR_ACCESS_KEY_IDS, CHARS_FOR_ACCESS_KEY_IDS_LENGTH, STS_ACCESS_KEY_ID_LENGTH); + STS_ACCESS_KEY_ID_ALLOWED_CHARS, STS_ACCESS_KEY_ID_ALLOWED_CHARS_LENGTH, + STS_ACCESS_KEY_ID_RANDOM_LENGTH); final String secretAccessKey = generateSecureRandomStringUsingChars( CHARS_FOR_SECRET_ACCESS_KEYS, CHARS_FOR_SECRET_ACCESS_KEYS_LENGTH, STS_SECRET_ACCESS_KEY_LENGTH); final String roleId = ASSUME_ROLE_ID_PREFIX + generateSecureRandomStringUsingChars( - CHARS_FOR_ACCESS_KEY_IDS, CHARS_FOR_ACCESS_KEY_IDS_LENGTH, STS_ROLE_ID_LENGTH); + STS_ACCESS_KEY_ID_ALLOWED_CHARS, STS_ACCESS_KEY_ID_ALLOWED_CHARS_LENGTH, + STS_ROLE_ID_LENGTH); // Build UpdateAssumeRoleRequest with leader-generated credentials final UpdateAssumeRoleRequest.Builder updateAssumeRoleRequestBuilder = @@ -182,7 +185,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut final long expirationEpochSeconds = clock.instant().plusSeconds(durationSeconds).getEpochSecond(); // 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); final AssumeRoleResponse.Builder responseBuilder = AssumeRoleResponse.newBuilder() .setAccessKeyId(tempAccessKeyId) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3DeleteRevokedSTSTokensRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3DeleteRevokedSTSTokensRequest.java index f41b20353a83..81558ec58504 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3DeleteRevokedSTSTokensRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3DeleteRevokedSTSTokensRequest.java @@ -35,6 +35,7 @@ /** * Handles DeleteRevokedSTSTokens requests submitted by {@link RevokedSTSTokenCleanupService}. + * Each request contains originalAccessKeyIds to remove from the revocation table. */ public class S3DeleteRevokedSTSTokensRequest extends OMClientRequest { @@ -62,8 +63,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut final DeleteRevokedSTSTokensRequest request = getOmRequest().getDeleteRevokedSTSTokensRequest(); final OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(getOmRequest()); - final List sessionTokens = request.getSessionTokenList(); - return new S3DeleteRevokedSTSTokensResponse(sessionTokens, omResponse.build()); + final List originalAccessKeyIds = request.getOriginalAccessKeyIdList(); + return new S3DeleteRevokedSTSTokensResponse(originalAccessKeyIds, omResponse.build()); } } 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 52a92d8a5560..02e6cac1b3d4 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 @@ -17,26 +17,30 @@ package org.apache.hadoop.ozone.om.request.s3.security; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.ACCESS_ID_NOT_FOUND; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INTERNAL_ERROR; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST; + import java.io.IOException; import java.time.Clock; import java.time.ZoneOffset; import java.util.HashMap; import java.util.Map; +import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; 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; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.om.response.s3.security.S3RevokeSTSTokenResponse; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; -import org.apache.hadoop.ozone.security.STSSecurityUtil; -import org.apache.hadoop.ozone.security.STSTokenIdentifier; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RevokeSTSTokenRequest; import org.apache.hadoop.security.UserGroupInformation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,10 +48,14 @@ /** * Handles S3RevokeSTSTokenRequest request. * - *

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

+ *

The client submits {@link RevokeSTSTokenRequest} with {@code originalAccessKeyId} only. On the + * leader, {@code preExecute} captures the revocation cutoff in {@code revocationTimeMillis} and + * replicates the updated request through Ratis so every OM applies the same cutoff.

+ * + *

This request records a revocation cutoff for the given {@code originalAccessKeyId} in the + * {@code s3RevokedStsTokenTable}. Subsequent S3 requests authenticated with STS tokens whose + * {@code creationTime} is strictly before the cutoff will be rejected when the revocation state + * has propagated.

*/ public class S3RevokeSTSTokenRequest extends OMClientRequest { @@ -61,48 +69,89 @@ public S3RevokeSTSTokenRequest(OMRequest omRequest) { @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { final OMRequest omRequest = super.preExecute(ozoneManager); - final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeReq = - omRequest.getRevokeSTSTokenRequest(); + final RevokeSTSTokenRequest revokeReq = omRequest.getRevokeSTSTokenRequest(); + validateRevokeRequestFields(revokeReq); - // Get the original (long-lived) access key id from the session token - // and enforce the same permission model that is used for S3 secret + // Use the original (long-lived) access key ID from the request and enforce + // the same permission model that is used for S3 secret // operations (get/set/revoke). Only the owner of the original access // 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 STSTokenIdentifier stsTokenIdentifier = STSSecurityUtil.constructValidateAndDecryptSTSToken( - sessionToken, ozoneManager.getSecretKeyClient(), CLOCK); - final String originalAccessKeyId = stsTokenIdentifier.getOriginalAccessKeyId(); + final String originalAccessKeyId = revokeReq.getOriginalAccessKeyId(); final UserGroupInformation ugi = S3SecretRequestHelper.getOrCreateUgi(originalAccessKeyId); S3SecretRequestHelper.checkAccessIdSecretOpPermission(ozoneManager, ugi, originalAccessKeyId); - return omRequest; + if (!ozoneManager.getS3SecretManager().hasS3Secret(originalAccessKeyId)) { + throw new OMException("originalAccessKeyId does not exist: " + originalAccessKeyId, ACCESS_ID_NOT_FOUND); + } + + final long revocationTimeMillis = CLOCK.millis(); + final RevokeSTSTokenRequest updatedRevokeReq = revokeReq.toBuilder() + .setRevocationTimeMillis(revocationTimeMillis) + .build(); + + return omRequest.toBuilder() + .setRevokeSTSTokenRequest(updatedRevokeReq) + .build(); } @Override public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) { final OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(getOmRequest()); + IOException exception = null; + OMClientResponse omClientResponse; + final Map auditMap = new HashMap<>(); - final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeReq = getOmRequest().getRevokeSTSTokenRequest(); - final String sessionToken = revokeReq.getSessionToken(); + try { + final RevokeSTSTokenRequest revokeReq = validateReplicatedRevokeRequestFields(getOmRequest()); + final String originalAccessKeyId = revokeReq.getOriginalAccessKeyId(); + auditMap.put(OzoneConsts.S3_REVOKESTSTOKEN_USER, originalAccessKeyId); + final long revocationTimeMillis = revokeReq.getRevocationTimeMillis(); - // All actual DB mutations are done in the response's addToDBBatch(). - final OMClientResponse omClientResponse = new S3RevokeSTSTokenResponse( - sessionToken, omResponse.build()); + // All actual DB mutations are done in the response's addToDBBatch(). + omClientResponse = new S3RevokeSTSTokenResponse(originalAccessKeyId, revocationTimeMillis, omResponse.build()); - // Audit log - final Map auditMap = new HashMap<>(); - final OzoneManagerProtocolProtos.UserInfo userInfo = getOmRequest().getUserInfo(); - auditMap.put(OzoneConsts.S3_REVOKESTSTOKEN_USER, userInfo.getUserName()); - markForAudit(ozoneManager.getAuditLogger(), buildAuditMessage( - OMAction.REVOKE_STS_TOKEN, auditMap, null, userInfo)); + // Update the cache immediately so subsequent validation checks see the revocation + ozoneManager.getMetadataManager().getS3RevokedStsTokenTable().addCacheEntry( + new CacheKey<>(originalAccessKeyId), CacheValue.get(context.getIndex(), revocationTimeMillis)); - // Update the cache immediately so subsequent validation checks see the revocation - ozoneManager.getMetadataManager().getS3RevokedStsTokenTable().addCacheEntry( - new CacheKey<>(sessionToken), CacheValue.get(context.getIndex(), CLOCK.millis())); + LOG.info( + "Marked STS tokens as revoked for originalAccessKeyId={} with cutoff time {}.", + originalAccessKeyId, revocationTimeMillis); + } catch (IOException ex) { + exception = ex; + omClientResponse = new S3RevokeSTSTokenResponse(null, 0L, createErrorOMResponse(omResponse, ex)); + } - LOG.info("Marked STS session token '{}' as revoked.", sessionToken); + // Audit log + markForAudit( + ozoneManager.getAuditLogger(), buildAuditMessage( + OMAction.REVOKE_STS_TOKEN, auditMap, exception, getOmRequest().getUserInfo())); return omClientResponse; } + + private static void validateRevokeRequestFields(RevokeSTSTokenRequest revokeReq) throws OMException { + final String originalAccessKeyId = revokeReq.getOriginalAccessKeyId(); + if (StringUtils.isEmpty(originalAccessKeyId)) { + throw new OMException("originalAccessKeyId is required for STS token revocation", INVALID_REQUEST); + } + if (revokeReq.hasRevocationTimeMillis()) { + throw new OMException("revocationTimeMillis must not be set by client", INVALID_REQUEST); + } + } + + private static RevokeSTSTokenRequest validateReplicatedRevokeRequestFields(OMRequest omRequest) throws OMException { + if (!omRequest.hasRevokeSTSTokenRequest()) { + throw new OMException("revokeSTSTokenRequest is required for STS token revocation", INTERNAL_ERROR); + } + final RevokeSTSTokenRequest revokeReq = omRequest.getRevokeSTSTokenRequest(); + if (StringUtils.isEmpty(revokeReq.getOriginalAccessKeyId())) { + throw new OMException("originalAccessKeyId is required for STS token revocation", INTERNAL_ERROR); + } + if (!revokeReq.hasRevocationTimeMillis()) { + throw new OMException("revocationTimeMillis is required for STS token revocation", INTERNAL_ERROR); + } + return revokeReq; + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3DeleteRevokedSTSTokensResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3DeleteRevokedSTSTokensResponse.java index cb44e7f466d9..a1b255689de5 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3DeleteRevokedSTSTokensResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3DeleteRevokedSTSTokensResponse.java @@ -36,16 +36,16 @@ @CleanupTableInfo(cleanupTables = {S3_REVOKED_STS_TOKEN_TABLE}) public class S3DeleteRevokedSTSTokensResponse extends OMClientResponse { - private final List sessionTokens; + private final List originalAccessKeyIds; - public S3DeleteRevokedSTSTokensResponse(List sessionTokens, @Nonnull OMResponse omResponse) { + public S3DeleteRevokedSTSTokensResponse(List originalAccessKeyIds, @Nonnull OMResponse omResponse) { super(omResponse); - this.sessionTokens = sessionTokens; + this.originalAccessKeyIds = originalAccessKeyIds; } @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { - if (sessionTokens == null || sessionTokens.isEmpty()) { + if (originalAccessKeyIds == null || originalAccessKeyIds.isEmpty()) { return; } if (!getOMResponse().hasStatus() || getOMResponse().getStatus() != OK) { @@ -57,8 +57,8 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation bat return; } - for (String sessionToken : sessionTokens) { - table.deleteWithBatch(batchOperation, sessionToken); + for (String originalAccessKeyId : originalAccessKeyIds) { + table.deleteWithBatch(batchOperation, originalAccessKeyId); } } } 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 5b1a8cf3b019..db9233357ed2 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,8 +22,6 @@ 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; @@ -37,22 +35,23 @@ @CleanupTableInfo(cleanupTables = {S3_REVOKED_STS_TOKEN_TABLE}) public class S3RevokeSTSTokenResponse extends OMClientResponse { - private static final Clock CLOCK = Clock.system(ZoneOffset.UTC); + private final String originalAccessKeyId; + private final long revocationTimeMillis; - private final String sessionToken; - - public S3RevokeSTSTokenResponse(String sessionToken, @Nonnull OMResponse omResponse) { + public S3RevokeSTSTokenResponse(String originalAccessKeyId, long revocationTimeMillis, + @Nonnull OMResponse omResponse) { super(omResponse); - this.sessionToken = sessionToken; + this.originalAccessKeyId = originalAccessKeyId; + this.revocationTimeMillis = revocationTimeMillis; } @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { - if (sessionToken != null && getOMResponse().hasStatus() && getOMResponse().getStatus() == OK) { + if (originalAccessKeyId != null && getOMResponse().hasStatus() && getOMResponse().getStatus() == OK) { final Table table = omMetadataManager.getS3RevokedStsTokenTable(); if (table != null) { - // Store insertionTimeMillis as value - table.putWithBatch(batchOperation, sessionToken, CLOCK.millis()); + // Store revocationTimeMillis as value + table.putWithBatch(batchOperation, originalAccessKeyId, revocationTimeMillis); } } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/RevokedSTSTokenCleanupService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/RevokedSTSTokenCleanupService.java index 3d9668d6469c..c627f6a21cb7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/RevokedSTSTokenCleanupService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/RevokedSTSTokenCleanupService.java @@ -37,6 +37,7 @@ import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.hadoop.ozone.om.helpers.S3STSUtils; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteRevokedSTSTokensRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; @@ -57,7 +58,8 @@ public class RevokedSTSTokenCleanupService extends BackgroundService { // Use a single thread private static final int REVOKED_STS_TOKEN_CLEANER_CORE_POOL_SIZE = 1; private static final Clock CLOCK = Clock.system(ZoneOffset.UTC); - private static final long CLEANUP_THRESHOLD = 12 * 60 * 60 * 1000L; // 12 hours in milliseconds + // Keep revocation entries until max STS token lifetime after the cutoff was captured. + private static final long CLEANUP_THRESHOLD = TimeUnit.SECONDS.toMillis(S3STSUtils.MAX_DURATION_SECONDS); // 12 hours private final OzoneManager ozoneManager; private final OMMetadataManager metadataManager; @@ -124,7 +126,7 @@ private boolean shouldRun() { return !suspended.get() && ozoneManager.isLeaderReady(); } - private class RevokedSTSTokenCleanupTask implements BackgroundTask { + private final class RevokedSTSTokenCleanupTask implements BackgroundTask { @Override public BackgroundTaskResult call() throws Exception { @@ -143,17 +145,17 @@ public BackgroundTaskResult call() throws Exception { iterator.seekToFirst(); while (iterator.hasNext()) { final Table.KeyValue entry = iterator.next(); - final String sessionToken = entry.getKey(); - final Long initialCreationTimeMillis = entry.getValue(); + final String originalAccessKeyId = entry.getKey(); + final Long revocationTimeMillis = entry.getValue(); - if (shouldCleanup(initialCreationTimeMillis)) { - // Calculate the size this token would add to the protobuf message. + if (shouldCleanup(revocationTimeMillis)) { + // Calculate the size this originalAccessKeyId would add to the protobuf message. // Make a copy of the batch to do the size check final List batchCopyWithCandidate = new ArrayList<>(batch); - batchCopyWithCandidate.add(sessionToken); + batchCopyWithCandidate.add(originalAccessKeyId); int batchWithCandidateSize = getBatchSerializedSize(batchCopyWithCandidate); - // If adding this token would exceed the limit, submit the current batch + // If adding this originalAccessKeyId would exceed the limit, submit the current batch if (batchWithCandidateSize > ratisByteLimit) { if (!batch.isEmpty()) { if (submitCleanupRequest(batch)) { @@ -163,22 +165,22 @@ public BackgroundTaskResult call() throws Exception { } batch.clear(); - // Re-calculate the size of the candidate token alone in an empty batch + // Re-calculate the size of the candidate key alone in an empty batch // to check if it exceeds the limit by itself. final List singleCandidateBatch = new ArrayList<>(); - singleCandidateBatch.add(sessionToken); + singleCandidateBatch.add(originalAccessKeyId); batchWithCandidateSize = getBatchSerializedSize(singleCandidateBatch); } - // Check if the single token exceeds the limit (either strictly single or after flush) + // Check if the single key exceeds the limit (either strictly single or after flush) if (batchWithCandidateSize > ratisByteLimit) { LOG.error( - "Single revoked STS Token size ({}) would exceed the ratisByteLimit ({}). SessionToken " + - "initialCreationTimeMillis: {}", batchWithCandidateSize, ratisByteLimit, initialCreationTimeMillis); + "Single originalAccessKeyId entry size ({}) would exceed the ratisByteLimit ({}). " + + "revocationTimeMillis: {}", batchWithCandidateSize, ratisByteLimit, revocationTimeMillis); continue; } } - batch.add(sessionToken); + batch.add(originalAccessKeyId); } } } catch (IOException e) { @@ -213,16 +215,16 @@ public BackgroundTaskResult call() throws Exception { } /** - * Returns true if the given STS session token has been in the table past the cleanup threshold. + * Returns true if the revocation cutoff is older than the cleanup threshold. */ - private boolean shouldCleanup(long initialCreationTimeMillis) { + private boolean shouldCleanup(long revocationTimeMillis) { final long now = CLOCK.millis(); - if (now - initialCreationTimeMillis > CLEANUP_THRESHOLD) { + if (now - revocationTimeMillis > CLEANUP_THRESHOLD) { if (LOG.isDebugEnabled()) { LOG.debug( - "Revoked STS token entry created at {} is older than 12 hours, will clean up. Current time: {}", - initialCreationTimeMillis, now); + "Revoked STS token cutoff at {} is older than {} ms, will clean up. Current time: {}", + revocationTimeMillis, CLEANUP_THRESHOLD, now); } return true; } @@ -230,11 +232,11 @@ private boolean shouldCleanup(long initialCreationTimeMillis) { } /** - * Builds and submits an OMRequest to delete the provided revoked STS token(s). + * Builds and submits an OMRequest to delete the provided originalAccessKeyId revocation entries. */ - private boolean submitCleanupRequest(List sessionTokens) { + private boolean submitCleanupRequest(List originalAccessKeyIds) { final DeleteRevokedSTSTokensRequest request = DeleteRevokedSTSTokensRequest.newBuilder() - .addAllSessionToken(sessionTokens) + .addAllOriginalAccessKeyId(originalAccessKeyIds) .build(); final OMRequest omRequest = OMRequest.newBuilder() @@ -254,9 +256,9 @@ private boolean submitCleanupRequest(List sessionTokens) { } } - private int getBatchSerializedSize(List sessionTokenBatch) { + private int getBatchSerializedSize(List originalAccessKeyIdBatch) { final DeleteRevokedSTSTokensRequest request = DeleteRevokedSTSTokensRequest.newBuilder() - .addAllSessionToken(sessionTokenBatch) + .addAllOriginalAccessKeyId(originalAccessKeyIdBatch) .build(); return request.getSerializedSize(); 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 08ac1f2bee11..6612fff2bad8 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 @@ -75,8 +75,10 @@ public static void validateS3Credential(OMRequest omRequest, token, ozoneManager.getSecretKeyClient(), CLOCK); // Ensure the token is not revoked - if (isRevokedStsToken(token, ozoneManager)) { - LOG.info("Session token has been revoked: {}, {}", stsTokenIdentifier.getTempAccessKeyId(), token); + if (isRevokedStsToken(stsTokenIdentifier, ozoneManager)) { + LOG.info( + "STS token has been revoked for originalAccessKeyId={}, tempAccessKeyId={}", + stsTokenIdentifier.getOriginalAccessKeyId(), stsTokenIdentifier.getTempAccessKeyId()); throw new OMException("STS token has been revoked", REVOKED_TOKEN); } @@ -157,11 +159,12 @@ private static void validateSTSTokenAwsSignature(STSTokenIdentifier stsTokenIden } /** - * Returns true if the STS session token is present in the revoked STS token table. + * Returns true if the STS token was created before the revocation cutoff for its originalAccessKeyId. */ - private static boolean isRevokedStsToken(String sessionToken, OzoneManager ozoneManager) + private static boolean isRevokedStsToken(STSTokenIdentifier stsTokenIdentifier, OzoneManager ozoneManager) throws OMException { try { + final String originalAccessKeyId = stsTokenIdentifier.getOriginalAccessKeyId(); final OMMetadataManager metadataManager = ozoneManager.getMetadataManager(); if (metadataManager == null) { final String msg = "Could not determine STS revocation: metadataManager is null"; @@ -176,7 +179,9 @@ private static boolean isRevokedStsToken(String sessionToken, OzoneManager ozone throw new OMException(msg, INTERNAL_ERROR); } - return revokedStsTokenTable.getIfExist(sessionToken) != null; + final Long revocationTimeMillis = revokedStsTokenTable.getIfExist(originalAccessKeyId); + return revocationTimeMillis != null + && stsTokenIdentifier.getCreationTime().toEpochMilli() < revocationTimeMillis; } 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/main/java/org/apache/hadoop/ozone/security/STSSecurityUtil.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSSecurityUtil.java index 2212ad6db797..ead735f12eac 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSSecurityUtil.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSSecurityUtil.java @@ -101,7 +101,7 @@ private static STSTokenIdentifier verifyAndDecryptToken(Token decodeTokenFromString(String encodedToken) throws SecretManager.InvalidToken { final Token token = new Token<>(); + // token.decodeFromUrlString() only declares IOException, but deserialization can throw + // unchecked exceptions (e.g. NegativeArraySizeException) when malformed input decodes to a + // negative byte-array length. Map those to InvalidToken (via catching RuntimeException) + // instead of failing the OM request. try { token.decodeFromUrlString(encodedToken); return token; - } catch (IOException e) { + } catch (IOException | RuntimeException e) { throw new SecretManager.InvalidToken("Failed to decode STS token string: " + e); } } @@ -180,6 +184,9 @@ static void ensureEssentialFieldsArePresentInToken(STSTokenIdentifier stsTokenId if (StringUtils.isEmpty(stsTokenIdentifier.getSecretAccessKey())) { throw new SecretManager.InvalidToken("Invalid STS token - secretAccessKey is null/empty"); } + if (stsTokenIdentifier.getCreationTime() == null) { + throw new SecretManager.InvalidToken("Invalid STS token - creationTime is null"); + } } /** diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenIdentifier.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenIdentifier.java index 8c13aac51905..e5629073c5ad 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenIdentifier.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenIdentifier.java @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.security; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import java.io.ByteArrayInputStream; import java.io.DataInput; @@ -29,6 +30,7 @@ import java.util.UUID; import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.annotation.InterfaceStability; +import org.apache.hadoop.hdds.security.symmetric.ManagedSecretKey; import org.apache.hadoop.hdds.security.token.ShortLivedTokenIdentifier; import org.apache.hadoop.io.Text; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMTokenProto; @@ -46,9 +48,11 @@ public class STSTokenIdentifier extends ShortLivedTokenIdentifier { private String originalAccessKeyId; private String secretAccessKey; private String sessionPolicy; + private Instant creationTime; - // Encryption key derived from ManagedSecretKey for this token - private transient byte[] encryptionKey; + // SCM secret key used for encrypting sensitive fields and signing this token + // It will NOT be encoded in the token. + private transient ManagedSecretKey managedSecretKey; // Service name for STS tokens public static final String STS_SERVICE = "STS"; @@ -63,23 +67,143 @@ public STSTokenIdentifier() { /** * Create a new STS token identifier with encryption support. * - * @param tempAccessKeyId the temporary access key ID (owner) - * @param originalAccessKeyId the original long-lived access key ID that created this token - * @param roleArn the ARN of the assumed role - * @param expiry the token expiration time - * @param secretAccessKey the secret access key associated with the temporary access key ID - * @param sessionPolicy an optional opaque identifier that further limits the scope of - * the permissions granted by the role - * @param encryptionKey the key bytes for encrypting sensitive fields + * @param params the STS token creation parameters */ - public STSTokenIdentifier(String tempAccessKeyId, String originalAccessKeyId, String roleArn, Instant expiry, - String secretAccessKey, String sessionPolicy, byte[] encryptionKey) { - super(tempAccessKeyId, expiry); - this.originalAccessKeyId = originalAccessKeyId; - this.roleArn = roleArn; - this.secretAccessKey = secretAccessKey; - this.sessionPolicy = sessionPolicy; - this.encryptionKey = encryptionKey != null ? encryptionKey.clone() : null; + public STSTokenIdentifier(Params params) { + super(params.getTempAccessKeyId(), params.getExpiry()); + this.originalAccessKeyId = params.getOriginalAccessKeyId(); + this.roleArn = params.getRoleArn(); + this.creationTime = params.getCreationTime(); + this.secretAccessKey = params.getSecretAccessKey(); + this.sessionPolicy = params.getSessionPolicy(); + this.managedSecretKey = params.getManagedSecretKey(); + // In the OzoneManagerStateMachine case, both secretAccessKey and managedSecretKey are set to null + if (this.secretAccessKey != null) { + if (this.managedSecretKey != null) { + setSecretKeyId(managedSecretKey.getId()); + } else { + throw new IllegalArgumentException("ManagedSecretKey is not set"); + } + } + } + + /** + * Parameters for constructing an {@link STSTokenIdentifier}. + */ + public static final class Params { + private final String tempAccessKeyId; + private final String originalAccessKeyId; + private final String roleArn; + private final Instant creationTime; + private final Instant expiry; + private final String secretAccessKey; + private final String sessionPolicy; + private final ManagedSecretKey managedSecretKey; + + private Params(Builder builder) { + this.tempAccessKeyId = builder.tempAccessKeyId; + this.originalAccessKeyId = builder.originalAccessKeyId; + this.roleArn = builder.roleArn; + this.creationTime = builder.creationTime; + this.expiry = builder.expiry; + this.secretAccessKey = builder.secretAccessKey; + this.sessionPolicy = builder.sessionPolicy; + this.managedSecretKey = builder.managedSecretKey; + } + + public static Builder newBuilder() { + return new Builder(); + } + + public String getTempAccessKeyId() { + return tempAccessKeyId; + } + + public String getOriginalAccessKeyId() { + return originalAccessKeyId; + } + + public String getRoleArn() { + return roleArn; + } + + public Instant getCreationTime() { + return creationTime; + } + + public Instant getExpiry() { + return expiry; + } + + public String getSecretAccessKey() { + return secretAccessKey; + } + + public String getSessionPolicy() { + return sessionPolicy; + } + + public ManagedSecretKey getManagedSecretKey() { + return managedSecretKey; + } + + /** + * Builder for {@link Params}. + */ + public static final class Builder { + private String tempAccessKeyId; + private String originalAccessKeyId; + private String roleArn; + private Instant creationTime; + private Instant expiry; + private String secretAccessKey; + private String sessionPolicy; + private ManagedSecretKey managedSecretKey; + + public Builder setTempAccessKeyId(String value) { + this.tempAccessKeyId = value; + return this; + } + + public Builder setOriginalAccessKeyId(String value) { + this.originalAccessKeyId = value; + return this; + } + + public Builder setRoleArn(String value) { + this.roleArn = value; + return this; + } + + public Builder setCreationTime(Instant value) { + this.creationTime = value; + return this; + } + + public Builder setExpiry(Instant value) { + this.expiry = value; + return this; + } + + public Builder setSecretAccessKey(String value) { + this.secretAccessKey = value; + return this; + } + + public Builder setSessionPolicy(String value) { + this.sessionPolicy = value; + return this; + } + + public Builder setManagedSecretKey(ManagedSecretKey value) { + this.managedSecretKey = value; + return this; + } + + public Params build() { + return new Params(this); + } + } } @Override @@ -112,23 +236,19 @@ public void readFields(DataInput in) throws IOException { /** * Convert this identifier to protobuf format. */ - public OMTokenProto toProtoBuf() { - Preconditions.checkArgument(this.encryptionKey != null, "The encryption key must not be null"); - - final OMTokenProto.Builder builder = OMTokenProto.newBuilder(); - // Note: secretKeyId must be set before attempting to decrypt secretAccessKey - if (getSecretKeyId() != null) { - builder.setSecretKeyId(getSecretKeyId().toString()); - } + public OMTokenProto toProtoBuf() throws IOException { + Preconditions.checkArgument(this.managedSecretKey != null, "The ManagedSecretKey must not be null"); - builder + final OMTokenProto.Builder builder = OMTokenProto.newBuilder() .setType(OMTokenProto.Type.S3_STS_TOKEN) + .setIssueDate(creationTime.toEpochMilli()) .setMaxDate(getExpiry().toEpochMilli()) .setOwner(getOwnerId() != null ? getOwnerId() : "") .setAccessKeyId(getOwnerId() != null ? getOwnerId() : "") .setOriginalAccessKeyId(originalAccessKeyId != null ? originalAccessKeyId : "") .setRoleArn(roleArn != null ? roleArn : "") .setSecretAccessKey(secretAccessKey != null ? encryptSensitiveField(secretAccessKey) : "") + .setSecretKeyId(managedSecretKey.getId().toString()) .setSessionPolicy(sessionPolicy != null ? sessionPolicy : ""); return builder.build(); @@ -141,11 +261,14 @@ public void fromProtoBuf(OMTokenProto token) throws IOException { Preconditions.checkArgument( token.getType() == OMTokenProto.Type.S3_STS_TOKEN, "Invalid token type for STSTokenIdentifier: " + token.getType()); - Preconditions.checkArgument(this.encryptionKey != null, "The encryption key must not be null"); + Preconditions.checkArgument(this.managedSecretKey != null, "The ManagedSecretKey must not be null"); setOwnerId(token.getOwner()); setExpiry(Instant.ofEpochMilli(token.getMaxDate())); + if (token.hasIssueDate()) { + this.creationTime = Instant.ofEpochMilli(token.getIssueDate()); + } if (token.hasOriginalAccessKeyId()) { this.originalAccessKeyId = token.getOriginalAccessKeyId(); } @@ -174,32 +297,24 @@ public void fromProtoBuf(OMTokenProto token) throws IOException { /** * Encrypt a sensitive field using the configured encryption key. */ - private String encryptSensitiveField(String value) { - if (encryptionKey == null) { - throw new IllegalStateException("Encryption key must be set before encrypting sensitive fields"); - } - + private String encryptSensitiveField(String value) throws IOException { try { final byte[] aad = computeAadBytes(); - return STSTokenEncryption.encrypt(value, encryptionKey, aad); + return STSTokenEncryption.encrypt(value, getSecretKeyBytes(), aad); } catch (STSTokenEncryption.STSTokenEncryptionException e) { - throw new RuntimeException("Token encryption failed", e); + throw new IOException("Token encryption failed", e); } } /** * Decrypt a sensitive field using the configured encryption key. */ - private String decryptSensitiveField(String encryptedValue) { - if (encryptionKey == null) { - throw new IllegalStateException("Encryption key must be set before decrypting sensitive fields"); - } - + private String decryptSensitiveField(String encryptedValue) throws IOException { try { final byte[] aad = computeAadBytes(); - return STSTokenEncryption.decrypt(encryptedValue, encryptionKey, aad); + return STSTokenEncryption.decrypt(encryptedValue, getSecretKeyBytes(), aad); } catch (STSTokenEncryption.STSTokenEncryptionException e) { - throw new RuntimeException("Token decryption failed", e); + throw new IOException("Token decryption failed", e); } } @@ -244,8 +359,43 @@ public String getSessionPolicy() { return sessionPolicy; } - public void setEncryptionKey(byte[] encryptionKey) { - this.encryptionKey = encryptionKey.clone(); + public Instant getCreationTime() { + return creationTime; + } + + // For test only + @VisibleForTesting + public void setManagedSecretKey(ManagedSecretKey secretKey) { + this.managedSecretKey = secretKey; + } + + public ManagedSecretKey getManagedSecretKey() { + return managedSecretKey; + } + + /** + * Sign serialized identifier bytes using the configured {@link ManagedSecretKey}. + */ + public byte[] sign(byte[] identifierBytes) { + Objects.requireNonNull(managedSecretKey, "ManagedSecretKey must be set before signing"); + return managedSecretKey.sign(identifierBytes); + } + + /** + * Verify a signature against serialized identifier bytes using the configured + * {@link ManagedSecretKey}. + */ + public boolean isValidSignature(byte[] identifierBytes, byte[] signature) { + Objects.requireNonNull(managedSecretKey, "ManagedSecretKey must be set before signature verification"); + return managedSecretKey.isValidSignature(identifierBytes, signature); + } + + private byte[] getSecretKeyBytes() { + if (managedSecretKey == null) { + throw new IllegalStateException("ManagedSecretKey is not set"); + } + + return managedSecretKey.getSecretKey().getEncoded(); } @Override @@ -265,13 +415,13 @@ public boolean equals(Object o) { final STSTokenIdentifier that = (STSTokenIdentifier) o; return Objects.equals(roleArn, that.roleArn) && Objects.equals(secretAccessKey, that.secretAccessKey) && Objects.equals(originalAccessKeyId, that.originalAccessKeyId) && - Objects.equals(sessionPolicy, that.sessionPolicy); + Objects.equals(sessionPolicy, that.sessionPolicy) && Objects.equals(creationTime, that.creationTime); } @Override public int hashCode() { return Objects.hash( - super.hashCode(), roleArn, secretAccessKey, originalAccessKeyId, sessionPolicy); + super.hashCode(), roleArn, secretAccessKey, originalAccessKeyId, sessionPolicy, creationTime); } @Override @@ -279,7 +429,7 @@ public String toString() { // Intentionally left off secretAccessKey return "STSTokenIdentifier{" + "tempAccessKeyId='" + getOwnerId() + "'" + ", originalAccessKeyId='" + originalAccessKeyId + "', roleArn='" + roleArn + "'" + - ", expiry='" + getExpiry() + "', secretKeyId='" + getSecretKeyId() + "'" + - ", sessionPolicy='" + sessionPolicy + "'}"; + ", creationTime='" + creationTime + "', expiry='" + getExpiry() + "', secretKeyId='" + getSecretKeyId() + + "', sessionPolicy='" + sessionPolicy + "'}"; } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenSecretManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenSecretManager.java index f72b1892de85..63c4d8121edf 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenSecretManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenSecretManager.java @@ -20,9 +20,9 @@ import java.io.IOException; import java.time.Clock; import java.time.Instant; +import java.util.Objects; import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.annotation.InterfaceStability; -import org.apache.hadoop.hdds.security.symmetric.ManagedSecretKey; import org.apache.hadoop.hdds.security.symmetric.SecretKeySignerClient; import org.apache.hadoop.hdds.security.token.ShortLivedTokenSecretManager; import org.apache.hadoop.io.Text; @@ -63,10 +63,13 @@ public STSTokenSecretManager(SecretKeySignerClient secretKeyClient) { */ @Override public Token generateToken(STSTokenIdentifier tokenIdentifier) { - final ManagedSecretKey secretKey = secretKeyClient.getCurrentSecretKey(); - tokenIdentifier.setSecretKeyId(secretKey.getId()); + // Note - the ManagedSecretKey will NOT be encoded in the token. When generateToken() is called, + // it eventually calls the write() method in STSTokenIdentifier which calls toProtoBuf(), and the + // ManagedSecretKey is not serialized there. + Objects.requireNonNull( + tokenIdentifier.getManagedSecretKey(), "ManagedSecretKey must be set on the token identifier before signing"); final byte[] identifierBytes = tokenIdentifier.getBytes(); - final byte[] password = secretKey.sign(identifierBytes); + final byte[] password = tokenIdentifier.sign(identifierBytes); return new Token<>(identifierBytes, password, tokenIdentifier.getKind(), new Text(tokenIdentifier.getService())); } @@ -85,17 +88,19 @@ public Token generateToken(STSTokenIdentifier tokenIdentifie */ public String createSTSTokenString(String tempAccessKeyId, String originalAccessKeyId, String roleArn, int durationSeconds, String secretAccessKey, String sessionPolicy, Clock clock) throws IOException { - final Instant expiration = clock.instant().plusSeconds(durationSeconds); + final Instant creationTime = clock.instant(); + final Instant expiration = creationTime.plusSeconds(durationSeconds); - // Get the current secret key for encryption - final ManagedSecretKey currentSecretKey = secretKeyClient.getCurrentSecretKey(); - final byte[] encryptionKey = currentSecretKey.getSecretKey().getEncoded(); - - // Note - the encryptionKey will NOT be encoded in the token. When generateToken() is called, it eventually calls - // the write() method in STSTokenIdentifier which calls toProtoBuf(), and the encryptionKey is not - // serialized there. - final STSTokenIdentifier identifier = new STSTokenIdentifier( - tempAccessKeyId, originalAccessKeyId, roleArn, expiration, secretAccessKey, sessionPolicy, encryptionKey); + final STSTokenIdentifier identifier = new STSTokenIdentifier(STSTokenIdentifier.Params.newBuilder() + .setTempAccessKeyId(tempAccessKeyId) + .setOriginalAccessKeyId(originalAccessKeyId) + .setRoleArn(roleArn) + .setCreationTime(creationTime) + .setExpiry(expiration) + .setSecretAccessKey(secretAccessKey) + .setSessionPolicy(sessionPolicy) + .setManagedSecretKey(secretKeyClient.getCurrentSecretKey()) + .build()); final Token token = generateToken(identifier); return token.encodeToUrlString(); 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 d0d11c5ba94f..c284b460dd68 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 @@ -1535,9 +1535,9 @@ public void testS3RevokedStsTokenTablePutAndGet() throws Exception { assertNotNull(omMetadataManager.getS3RevokedStsTokenTable(), "s3RevokedStsTokenTable should be initialized"); final MockClock clock = MockClock.newInstance(); - final String sessionToken1 = "test-session-token-1"; + final String originalAccessKeyId1 = "orig-1"; final long insertionTime1 = clock.millis(); - final String sessionToken2 = "test-session-token-2"; + final String originalAccessKeyId2 = "orig-2"; final long insertionTime2 = insertionTime1 + 1234L; // This table is configured as FULL_CACHE in OmMetadataManagerImpl. @@ -1546,25 +1546,25 @@ public void testS3RevokedStsTokenTablePutAndGet() throws Exception { final TypedTable revokedTable = (TypedTable) omMetadataManager.getS3RevokedStsTokenTable(); - revokedTable.put(sessionToken1, insertionTime1); - revokedTable.put(sessionToken2, insertionTime2); + revokedTable.put(originalAccessKeyId1, insertionTime1); + revokedTable.put(originalAccessKeyId2, insertionTime2); // Verify the values are persisted in RocksDB. - assertEquals(insertionTime1, revokedTable.getSkipCache(sessionToken1)); - assertEquals(insertionTime2, revokedTable.getSkipCache(sessionToken2)); + assertEquals(insertionTime1, revokedTable.getSkipCache(originalAccessKeyId1)); + assertEquals(insertionTime2, revokedTable.getSkipCache(originalAccessKeyId2)); // Update cache to make get/getIfExist reflect the write for FULL_CACHE tables. - revokedTable.addCacheEntry(sessionToken1, insertionTime1, 1L); - revokedTable.addCacheEntry(sessionToken2, insertionTime2, 1L); + revokedTable.addCacheEntry(originalAccessKeyId1, insertionTime1, 1L); + revokedTable.addCacheEntry(originalAccessKeyId2, insertionTime2, 1L); // Verify get and getIfExist return the stored value - assertEquals(insertionTime1, revokedTable.get(sessionToken1)); - assertEquals(insertionTime1, revokedTable.getIfExist(sessionToken1)); - assertEquals(insertionTime2, revokedTable.get(sessionToken2)); - assertEquals(insertionTime2, revokedTable.getIfExist(sessionToken2)); + assertEquals(insertionTime1, revokedTable.get(originalAccessKeyId1)); + assertEquals(insertionTime1, revokedTable.getIfExist(originalAccessKeyId1)); + assertEquals(insertionTime2, revokedTable.get(originalAccessKeyId2)); + assertEquals(insertionTime2, revokedTable.getIfExist(originalAccessKeyId2)); - // Invalid sessionToken should return null for getIfExist - assertNull(revokedTable.getIfExist("INVALID_SESSION_TOKEN")); + // Invalid originalAccessKeyId should return null for getIfExist. + assertNull(revokedTable.getIfExist("INVALID_ORIGINAL_ACCESS_KEY_ID")); } @Test 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 0f3d2519b30c..1b6caed9cb40 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 @@ -19,7 +19,9 @@ import static org.apache.hadoop.security.authentication.util.KerberosName.DEFAULT_MECHANISM; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; @@ -29,16 +31,16 @@ import java.io.IOException; import java.util.Optional; import java.util.UUID; -import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; -import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ipc_.ExternalCall; import org.apache.hadoop.ipc_.Server; +import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMultiTenantManager; import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.hadoop.ozone.om.S3SecretManager; 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; @@ -46,11 +48,8 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; -import org.apache.hadoop.ozone.security.STSTokenSecretManager; -import org.apache.hadoop.ozone.security.SecretKeyTestClient; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.authentication.util.KerberosName; -import org.apache.ozone.test.MockClock; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -60,22 +59,22 @@ */ public class TestS3RevokeSTSTokenRequest { - private static final MockClock CLOCK = MockClock.newInstance(); + private static final String TEST_KERBEROS_RULES = + "RULE:[2:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" + "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" + "DEFAULT"; - private STSTokenSecretManager stsTokenSecretManager; - private SecretKeyClient secretKeyClient; private OMMultiTenantManager omMultiTenantManager; + private String kerberosMechanismBeforeTest; + private String kerberosRulesBeforeTest; @BeforeEach public void setUp() throws Exception { + kerberosMechanismBeforeTest = KerberosName.getRuleMechanism(); + kerberosRulesBeforeTest = KerberosName.getRules(); + KerberosName.setRuleMechanism(DEFAULT_MECHANISM); // Initialize KerberosName rules so that UGI short names derived from // principals like "alice@EXAMPLE.COM" are computed correctly. - KerberosName.setRuleMechanism(DEFAULT_MECHANISM); - KerberosName.setRules( - "RULE:[2:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" + "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" + "DEFAULT"); + KerberosName.setRules(TEST_KERBEROS_RULES); - secretKeyClient = new SecretKeyTestClient(); - stsTokenSecretManager = new STSTokenSecretManager(secretKeyClient); // Multi-tenant manager mock used for tests that exercise the S3 multi-tenancy permission branch. omMultiTenantManager = mock(OMMultiTenantManager.class); } @@ -83,15 +82,15 @@ public void setUp() throws Exception { @AfterEach public void tearDown() { Server.getCurCall().remove(); + KerberosName.setRuleMechanism(kerberosMechanismBeforeTest); + KerberosName.setRules(kerberosRulesBeforeTest); } @Test public void testPreExecuteFailsForNonOwnerOfOriginalAccessKey() throws Exception { - // Verify that preExecute enforces permissions based on the original access key id encoded in the STS token + // Verify that preExecute enforces permissions based on the request's original access key ID // and rejects revocation attempts from non-owners. - final String tempAccessKeyId = "ASIA12345678"; final String originalAccessKeyId = "original-access-key-id"; - final String sessionToken = createSessionToken(tempAccessKeyId, originalAccessKeyId); // An RPC call running another Kerberos identity should NOT be allowed to revoke the token whose original // access key id is different. @@ -100,24 +99,10 @@ public void testPreExecuteFailsForNonOwnerOfOriginalAccessKey() throws Exception 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); - - final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = - OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setSessionToken(sessionToken) - .build(); - - final OMRequest omRequest = OMRequest.newBuilder() - .setClientId(UUID.randomUUID().toString()) - .setCmdType(Type.RevokeSTSToken) - .setRevokeSTSTokenRequest(revokeRequest) - .build(); - - final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(omRequest); + configureOzoneManagerForPreExecute(ozoneManager, originalAccessKeyId, true); + when(ozoneManager.isS3Admin(any(UserGroupInformation.class))).thenReturn(false); + final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(buildRevokeOmRequest(originalAccessKeyId)); ex = assertThrows(OMException.class, () -> omClientRequest.preExecute(ozoneManager)); } assertEquals(OMException.ResultCodes.USER_MISMATCH, ex.getResult()); @@ -125,36 +110,25 @@ public void testPreExecuteFailsForNonOwnerOfOriginalAccessKey() throws Exception @Test public void testPreExecuteSucceedsForOriginalAccessKeyOwner() throws Exception { - // Verify that preExecute allows the owner of the original access key id (as encoded in the STS token) + // Verify that preExecute allows the owner of the original access key ID from the revoke request // to revoke the temporary credentials. - final String tempAccessKeyId = "ASIA4567891230"; final String originalAccessKeyId = "original-access-key-id"; - final String sessionToken = createSessionToken(tempAccessKeyId, originalAccessKeyId); // Simulate RPC call running as originalAccessKeyId final UserGroupInformation originalUgi = UserGroupInformation.createRemoteUser(originalAccessKeyId); Server.getCurCall().set(new StubCall(originalUgi)); final OzoneManager ozoneManager = mock(OzoneManager.class); - when(ozoneManager.isS3MultiTenancyEnabled()).thenReturn(false); - when(ozoneManager.isS3Admin(any(UserGroupInformation.class))) - .thenReturn(false); - when(ozoneManager.getSecretKeyClient()).thenReturn(secretKeyClient); - - final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = - OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setSessionToken(sessionToken) - .build(); - - final OMRequest omRequest = OMRequest.newBuilder() - .setClientId(UUID.randomUUID().toString()) - .setCmdType(Type.RevokeSTSToken) - .setRevokeSTSTokenRequest(revokeRequest) - .build(); + configureOzoneManagerForPreExecute(ozoneManager, originalAccessKeyId, true); + when(ozoneManager.isS3Admin(any(UserGroupInformation.class))).thenReturn(false); - final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(omRequest); + final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(buildRevokeOmRequest(originalAccessKeyId)); final OMRequest result = omClientRequest.preExecute(ozoneManager); + assertEquals(Type.RevokeSTSToken, result.getCmdType()); + assertTrue(result.getRevokeSTSTokenRequest().hasRevocationTimeMillis()); + assertEquals(originalAccessKeyId, result.getRevokeSTSTokenRequest().getOriginalAccessKeyId()); + assertTrue(result.getRevokeSTSTokenRequest().getRevocationTimeMillis() > 0L); } @Test @@ -163,40 +137,23 @@ public void testPreExecuteSucceedsForTenantAccessIdOwner() throws Exception { // the tenant access ID owner is allowed to revoke the temporary credentials. final String tenantId = "finance"; final String originalAccessKeyId = "alice@EXAMPLE.COM"; - final String tempAccessKeyId = "ASIA123456789"; - final String sessionToken = createSessionToken(tempAccessKeyId, originalAccessKeyId); // Caller short name "alice" should match the owner username returned from the multi-tenant manager. final UserGroupInformation callerUgi = UserGroupInformation.createRemoteUser(originalAccessKeyId); Server.getCurCall().set(new StubCall(callerUgi)); final OzoneManager ozoneManager = mock(OzoneManager.class); + configureOzoneManagerForPreExecute(ozoneManager, originalAccessKeyId, true); when(ozoneManager.isS3MultiTenancyEnabled()).thenReturn(true); when(ozoneManager.getMultiTenantManager()).thenReturn(omMultiTenantManager); - when(ozoneManager.getSecretKeyClient()).thenReturn(secretKeyClient); // Original access key id is assigned to a tenant and owned by "alice". - when(omMultiTenantManager.getTenantForAccessID(originalAccessKeyId)) - .thenReturn(Optional.of(tenantId)); - when(omMultiTenantManager.getUserNameGivenAccessId(originalAccessKeyId)) - .thenReturn("alice"); + when(omMultiTenantManager.getTenantForAccessID(originalAccessKeyId)).thenReturn(Optional.of(tenantId)); + when(omMultiTenantManager.getUserNameGivenAccessId(originalAccessKeyId)).thenReturn("alice"); // Not a tenant admin; ownership should be sufficient. - when(omMultiTenantManager.isTenantAdmin(callerUgi, tenantId, false)) - .thenReturn(false); - - final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = - OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setSessionToken(sessionToken) - .build(); - - final OMRequest omRequest = OMRequest.newBuilder() - .setClientId(UUID.randomUUID().toString()) - .setCmdType(Type.RevokeSTSToken) - .setRevokeSTSTokenRequest(revokeRequest) - .build(); - - final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(omRequest); + when(omMultiTenantManager.isTenantAdmin(callerUgi, tenantId, false)).thenReturn(false); + final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(buildRevokeOmRequest(originalAccessKeyId)); final OMRequest result = omClientRequest.preExecute(ozoneManager); assertEquals(Type.RevokeSTSToken, result.getCmdType()); } @@ -207,40 +164,23 @@ public void testPreExecuteSucceedsForTenantAdmin() throws Exception { // tenant admin (who is not the owner) is allowed to revoke the temporary credentials. final String tenantId = "finance"; final String originalAccessKeyId = "alice@EXAMPLE.COM"; - final String tempAccessKeyId = "ASIA4567890123"; - final String sessionToken = createSessionToken(tempAccessKeyId, originalAccessKeyId); // Caller short name "bob" does not own the access ID but will be configured as tenant admin. final UserGroupInformation callerUgi = UserGroupInformation.createRemoteUser("bob@EXAMPLE.COM"); Server.getCurCall().set(new StubCall(callerUgi)); final OzoneManager ozoneManager = mock(OzoneManager.class); + configureOzoneManagerForPreExecute(ozoneManager, originalAccessKeyId, true); when(ozoneManager.isS3MultiTenancyEnabled()).thenReturn(true); when(ozoneManager.getMultiTenantManager()).thenReturn(omMultiTenantManager); - when(ozoneManager.getSecretKeyClient()).thenReturn(secretKeyClient); // Original access key id is assigned to a tenant and owned by "alice". - when(omMultiTenantManager.getTenantForAccessID(originalAccessKeyId)) - .thenReturn(Optional.of(tenantId)); - when(omMultiTenantManager.getUserNameGivenAccessId(originalAccessKeyId)) - .thenReturn("alice"); + when(omMultiTenantManager.getTenantForAccessID(originalAccessKeyId)).thenReturn(Optional.of(tenantId)); + when(omMultiTenantManager.getUserNameGivenAccessId(originalAccessKeyId)).thenReturn("alice"); // Caller is configured as tenant admin so the check should pass. - when(omMultiTenantManager.isTenantAdmin(callerUgi, tenantId, false)) - .thenReturn(true); - - final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = - OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setSessionToken(sessionToken) - .build(); - - final OMRequest omRequest = OMRequest.newBuilder() - .setClientId(UUID.randomUUID().toString()) - .setCmdType(Type.RevokeSTSToken) - .setRevokeSTSTokenRequest(revokeRequest) - .build(); - - final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(omRequest); + when(omMultiTenantManager.isTenantAdmin(callerUgi, tenantId, false)).thenReturn(true); + final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(buildRevokeOmRequest(originalAccessKeyId)); final OMRequest result = omClientRequest.preExecute(ozoneManager); assertEquals(Type.RevokeSTSToken, result.getCmdType()); } @@ -251,8 +191,6 @@ public void testPreExecuteFailsForNonOwnerNonAdminInTenant() throws Exception { // non-owner, non-admin caller is rejected. final String tenantId = "finance"; final String originalAccessKeyId = "alice@EXAMPLE.COM"; - final String tempAccessKeyId = "ASIA123456789"; - final String sessionToken = createSessionToken(tempAccessKeyId, originalAccessKeyId); // Caller short name "carol" does not own the access ID and is not // configured as tenant admin. @@ -261,42 +199,65 @@ public void testPreExecuteFailsForNonOwnerNonAdminInTenant() throws Exception { final OMException ex; try (OzoneManager ozoneManager = mock(OzoneManager.class)) { + configureOzoneManagerForPreExecute(ozoneManager, originalAccessKeyId, true); when(ozoneManager.isS3MultiTenancyEnabled()).thenReturn(true); when(ozoneManager.getMultiTenantManager()).thenReturn(omMultiTenantManager); - when(ozoneManager.getSecretKeyClient()).thenReturn(secretKeyClient); - // Original access key id is assigned to a tenant and owned by "alice". - when(omMultiTenantManager.getTenantForAccessID(originalAccessKeyId)) - .thenReturn(Optional.of(tenantId)); - when(omMultiTenantManager.getUserNameGivenAccessId(originalAccessKeyId)) - .thenReturn("alice"); + when(omMultiTenantManager.getTenantForAccessID(originalAccessKeyId)).thenReturn(Optional.of(tenantId)); + when(omMultiTenantManager.getUserNameGivenAccessId(originalAccessKeyId)).thenReturn("alice"); // Caller is not a tenant admin. - when(omMultiTenantManager.isTenantAdmin(callerUgi, tenantId, false)) - .thenReturn(false); + when(omMultiTenantManager.isTenantAdmin(callerUgi, tenantId, false)).thenReturn(false); + + final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(buildRevokeOmRequest(originalAccessKeyId)); + ex = assertThrows(OMException.class, () -> omClientRequest.preExecute(ozoneManager)); + } + assertEquals(OMException.ResultCodes.USER_MISMATCH, ex.getResult()); + } - final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = - OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setSessionToken(sessionToken) - .build(); + @Test + public void testPreExecuteRejectsUnknownOriginalAccessKeyId() throws Exception { + // Reject revocation when originalAccessKeyId has no S3 secret in RocksDB. + final String originalAccessKeyId = "unknown-access-key-id"; + final UserGroupInformation callerUgi = UserGroupInformation.createRemoteUser(originalAccessKeyId); + Server.getCurCall().set(new StubCall(callerUgi)); - final OMRequest omRequest = OMRequest.newBuilder() - .setClientId(UUID.randomUUID().toString()) - .setCmdType(Type.RevokeSTSToken) - .setRevokeSTSTokenRequest(revokeRequest) - .build(); + try (OzoneManager ozoneManager = mock(OzoneManager.class)) { + final S3SecretManager s3SecretManager = configureOzoneManagerForPreExecute( + ozoneManager, originalAccessKeyId, false); + final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(buildRevokeOmRequest(originalAccessKeyId)); + final OMException ex = assertThrows(OMException.class, () -> omClientRequest.preExecute(ozoneManager)); + assertEquals(OMException.ResultCodes.ACCESS_ID_NOT_FOUND, ex.getResult()); + assertTrue(ex.getMessage().contains("does not exist")); + assertTrue(ex.getMessage().contains(originalAccessKeyId)); + verify(s3SecretManager).hasS3Secret(originalAccessKeyId); + } + } - final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(omRequest); + @Test + public void testPreExecuteRejectsUnknownOriginalAccessKeyIdForS3Admin() throws Exception { + // S3 admins may revoke other principals' tokens, but not for unknown access key IDs. + final String originalAccessKeyId = "unknown-access-key-id"; + final UserGroupInformation adminUgi = UserGroupInformation.createRemoteUser("om-admin"); + Server.getCurCall().set(new StubCall(adminUgi)); - ex = assertThrows(OMException.class, () -> omClientRequest.preExecute(ozoneManager)); + try (OzoneManager ozoneManager = mock(OzoneManager.class)) { + final S3SecretManager s3SecretManager = configureOzoneManagerForPreExecute( + ozoneManager, originalAccessKeyId, false); + when(ozoneManager.isS3Admin(adminUgi)).thenReturn(true); + + final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(buildRevokeOmRequest(originalAccessKeyId)); + final OMException ex = assertThrows(OMException.class, () -> omClientRequest.preExecute(ozoneManager)); + assertEquals(OMException.ResultCodes.ACCESS_ID_NOT_FOUND, ex.getResult()); + assertTrue(ex.getMessage().contains("does not exist")); + assertTrue(ex.getMessage().contains(originalAccessKeyId)); + verify(s3SecretManager).hasS3Secret(originalAccessKeyId); } - assertEquals(OMException.ResultCodes.USER_MISMATCH, ex.getResult()); } @Test - public void testValidateAndUpdateCacheUpdatesCacheImmediately() throws Exception { - final String tempAccessKeyId = "ASIA4567891230"; + public void testValidateAndUpdateCacheUpdatesCacheImmediately() { final String originalAccessKeyId = "original-access-key-id"; - final String sessionToken = createSessionToken(tempAccessKeyId, originalAccessKeyId); + final long revocationTimeMillis = 1_700_000_000_000L; final OzoneManager ozoneManager = mock(OzoneManager.class); final OMMetadataManager omMetadataManager = mock(OMMetadataManager.class); @@ -311,7 +272,8 @@ public void testValidateAndUpdateCacheUpdatesCacheImmediately() throws Exception final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() - .setSessionToken(sessionToken) + .setOriginalAccessKeyId(originalAccessKeyId) + .setRevocationTimeMillis(revocationTimeMillis) .build(); final OMRequest omRequest = OMRequest.newBuilder() @@ -324,12 +286,91 @@ public void testValidateAndUpdateCacheUpdatesCacheImmediately() throws Exception final OMClientResponse omClientResponse = s3RevokeSTSTokenRequest.validateAndUpdateCache(ozoneManager, context); assertEquals(OzoneManagerProtocolProtos.Status.OK, omClientResponse.getOMResponse().getStatus()); - verify(s3RevokedStsTokenTable).addCacheEntry(eq(new CacheKey<>(sessionToken)), any(CacheValue.class)); + verify(s3RevokedStsTokenTable).addCacheEntry( + eq(new CacheKey<>(originalAccessKeyId)), any()); + assertNotNull(s3RevokeSTSTokenRequest.getAuditBuilder().getAuditMap()); + assertEquals( + originalAccessKeyId, s3RevokeSTSTokenRequest.getAuditBuilder().getAuditMap().get( + OzoneConsts.S3_REVOKESTSTOKEN_USER)); + } + + @Test + public void testValidateAndUpdateCacheRejectsMissingRevocationTimeMillis() { + final String originalAccessKeyId = "original-access-key-id"; + + final OzoneManager ozoneManager = mock(OzoneManager.class); + final OMMetadataManager omMetadataManager = mock(OMMetadataManager.class); + @SuppressWarnings("unchecked") + final Table s3RevokedStsTokenTable = mock(Table.class); + final ExecutionContext context = mock(ExecutionContext.class); + + when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager); + when(omMetadataManager.getS3RevokedStsTokenTable()).thenReturn(s3RevokedStsTokenTable); + + final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = + OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() + .setOriginalAccessKeyId(originalAccessKeyId) + .build(); + + final OMRequest omRequest = OMRequest.newBuilder() + .setClientId(UUID.randomUUID().toString()) + .setCmdType(Type.RevokeSTSToken) + .setRevokeSTSTokenRequest(revokeRequest) + .build(); + + final S3RevokeSTSTokenRequest s3RevokeSTSTokenRequest = new S3RevokeSTSTokenRequest(omRequest); + final OMClientResponse omClientResponse = + s3RevokeSTSTokenRequest.validateAndUpdateCache(ozoneManager, context); + assertEquals(OzoneManagerProtocolProtos.Status.INTERNAL_ERROR, omClientResponse.getOMResponse().getStatus()); + } + + @Test + public void testPreExecuteRejectsClientSuppliedRevocationTimeMillis() throws Exception { + final String originalAccessKeyId = "original-access-key-id"; + final UserGroupInformation callerUgi = UserGroupInformation.createRemoteUser(originalAccessKeyId); + Server.getCurCall().set(new StubCall(callerUgi)); + + final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = + OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() + .setOriginalAccessKeyId(originalAccessKeyId) + .setRevocationTimeMillis(1_700_000_000_000L) + .build(); + final OMRequest omRequest = OMRequest.newBuilder() + .setClientId(UUID.randomUUID().toString()) + .setCmdType(Type.RevokeSTSToken) + .setRevokeSTSTokenRequest(revokeRequest) + .build(); + + try (OzoneManager ozoneManager = mock(OzoneManager.class)) { + configureOzoneManagerForPreExecute(ozoneManager, originalAccessKeyId, true); + final OMClientRequest omClientRequest = new S3RevokeSTSTokenRequest(omRequest); + final OMException ex = assertThrows(OMException.class, () -> omClientRequest.preExecute(ozoneManager)); + assertEquals(OMException.ResultCodes.INVALID_REQUEST, ex.getResult()); + } + } + + private static OMRequest buildRevokeOmRequest(String originalAccessKeyId) { + final OzoneManagerProtocolProtos.RevokeSTSTokenRequest revokeRequest = + OzoneManagerProtocolProtos.RevokeSTSTokenRequest.newBuilder() + .setOriginalAccessKeyId(originalAccessKeyId) + .build(); + + return OMRequest.newBuilder() + .setClientId(UUID.randomUUID().toString()) + .setCmdType(Type.RevokeSTSToken) + .setRevokeSTSTokenRequest(revokeRequest) + .build(); + } + + private static S3SecretManager configureOzoneManagerForPreExecute(OzoneManager ozoneManager, + String originalAccessKeyId, boolean hasSecret) throws IOException { + when(ozoneManager.isS3MultiTenancyEnabled()).thenReturn(false); + final S3SecretManager s3SecretManager = mock(S3SecretManager.class); + when(ozoneManager.getS3SecretManager()).thenReturn(s3SecretManager); + when(s3SecretManager.hasS3Secret(originalAccessKeyId)).thenReturn(hasSecret); + return s3SecretManager; } - /** - * Stub used to inject a remote user into the ProtobufRpcEngine.Server.getRemoteUser() thread-local. - */ private static final class StubCall extends ExternalCall { private final UserGroupInformation ugi; @@ -343,10 +384,4 @@ public UserGroupInformation getRemoteUser() { return ugi; } } - - private String createSessionToken(String tempAccessKeyId, String originalAccessKeyId) throws IOException { - return stsTokenSecretManager.createSTSTokenString( - tempAccessKeyId, originalAccessKeyId, "arn:aws:iam::123456789012:role/test-role", 3600, - "test-secret-access-key", "test-session-policy", CLOCK); - } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestRevokedSTSTokenCleanupService.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestRevokedSTSTokenCleanupService.java index d7cf3630b955..2b734cea2456 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestRevokedSTSTokenCleanupService.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestRevokedSTSTokenCleanupService.java @@ -75,13 +75,13 @@ public void setUp() { @Test public void submitsCleanupRequestForOnlyExpiredTokens() throws Exception { - // If there are two revoked entries, one expired and one not expired, only the expired session token should be - // submitted for cleanup. + // If there are two revoked entries, one expired and one not expired, only the expired + // originalAccessKeyId should be submitted for cleanup. final long nowMillis = testClock.millis(); final long expiredCreationTimeMillis = nowMillis - TimeUnit.HOURS.toMillis(13); // older than 12h threshold final long validCreationTimeMillis = nowMillis - TimeUnit.HOURS.toMillis(1); - revokedStsTokenTable.put("session-token-a", expiredCreationTimeMillis); - revokedStsTokenTable.put("session-token-b", validCreationTimeMillis); + revokedStsTokenTable.put("original-access-key-a", expiredCreationTimeMillis); + revokedStsTokenTable.put("original-access-key-b", validCreationTimeMillis); final AtomicReference capturedRequest = new AtomicReference<>(); @@ -100,7 +100,7 @@ public void submitsCleanupRequestForOnlyExpiredTokens() throws Exception { final DeleteRevokedSTSTokensRequest deleteRevokedSTSTokensRequest = omRequest.getDeleteRevokedSTSTokensRequest(); - assertThat(deleteRevokedSTSTokensRequest.getSessionTokenList()).containsExactly("session-token-a"); + assertThat(deleteRevokedSTSTokensRequest.getOriginalAccessKeyIdList()).containsExactly("original-access-key-a"); } } @@ -109,8 +109,8 @@ public void doesNotSubmitRequestWhenThereAreNoExpiredTokens() throws Exception { // If only non-expired entries exist in the revoked sts token table, no cleanup request should be submitted and // no metrics should be updated. final long nowMillis = testClock.millis(); - revokedStsTokenTable.put("session-token-c", nowMillis - TimeUnit.HOURS.toMillis(1)); - revokedStsTokenTable.put("session-token-d", nowMillis - TimeUnit.HOURS.toMillis(2)); + revokedStsTokenTable.put("original-access-key-c", nowMillis - TimeUnit.HOURS.toMillis(1)); + revokedStsTokenTable.put("original-access-key-d", nowMillis - TimeUnit.HOURS.toMillis(2)); final AtomicReference capturedRequest = new AtomicReference<>(); @@ -149,8 +149,8 @@ public void doesNotUpdateMetricsOnRatisSubmissionServiceExceptionFailure() throw // If there are expired tokens in the table but the OM request submission to clean up the entries fails with a // service exception, the metrics should not be updated final long nowMillis = testClock.millis(); - revokedStsTokenTable.put("session-token-e", nowMillis - TimeUnit.HOURS.toMillis(13)); - revokedStsTokenTable.put("session-token-f", nowMillis - TimeUnit.HOURS.toMillis(14)); + revokedStsTokenTable.put("original-access-key-e", nowMillis - TimeUnit.HOURS.toMillis(13)); + revokedStsTokenTable.put("original-access-key-f", nowMillis - TimeUnit.HOURS.toMillis(14)); final AtomicInteger submitAttempts = new AtomicInteger(0); @@ -172,7 +172,7 @@ public void doesNotUpdateMetricsOnNonSuccessfulResponse() throws Exception { // If there is an expired token in the table but the OM request submission to clean up the entries gets a // non-successful response, the metrics should not be updated final long nowMillis = testClock.millis(); - revokedStsTokenTable.put("session-token-f", nowMillis - TimeUnit.HOURS.toMillis(20)); + revokedStsTokenTable.put("original-access-key-f", nowMillis - TimeUnit.HOURS.toMillis(20)); try (MockedStatic ozoneManagerRatisUtilsMock = mockStatic(OzoneManagerRatisUtils.class)) { // Return a non-successful response @@ -190,9 +190,9 @@ public void doesNotUpdateMetricsOnNonSuccessfulResponse() throws Exception { public void handlesAllExpiredTokens() throws Exception { // If all the tokens in the table are expired on a particular run, ensure the metrics are updated appropriately final long nowMillis = testClock.millis(); - revokedStsTokenTable.put("session-token-g", nowMillis - TimeUnit.HOURS.toMillis(13)); - revokedStsTokenTable.put("session-token-h", nowMillis - TimeUnit.HOURS.toMillis(14)); - revokedStsTokenTable.put("session-token-i", nowMillis - TimeUnit.HOURS.toMillis(15)); + revokedStsTokenTable.put("original-access-key-g", nowMillis - TimeUnit.HOURS.toMillis(13)); + revokedStsTokenTable.put("original-access-key-h", nowMillis - TimeUnit.HOURS.toMillis(14)); + revokedStsTokenTable.put("original-access-key-i", nowMillis - TimeUnit.HOURS.toMillis(15)); final AtomicReference capturedRequest = new AtomicReference<>(); @@ -211,8 +211,8 @@ public void handlesAllExpiredTokens() throws Exception { final DeleteRevokedSTSTokensRequest deleteRevokedSTSTokensRequest = omRequest.getDeleteRevokedSTSTokensRequest(); - assertThat(deleteRevokedSTSTokensRequest.getSessionTokenList()) - .containsExactlyInAnyOrder("session-token-g", "session-token-h", "session-token-i"); + assertThat(deleteRevokedSTSTokensRequest.getOriginalAccessKeyIdList()) + .containsExactlyInAnyOrder("original-access-key-g", "original-access-key-h", "original-access-key-i"); } } @@ -221,9 +221,9 @@ public void submitsMultipleRequestsWhenBatchSizeIsExceeded() throws Exception { // If the tokens exceed the configured batch size, multiple requests should be submitted final long nowMillis = testClock.millis(); - // Create 10 expired tokens + // Create 10 expired originalAccessKeyIds for (int i = 0; i < 10; i++) { - revokedStsTokenTable.put("session-token-" + i, nowMillis - TimeUnit.HOURS.toMillis(13)); + revokedStsTokenTable.put(String.format("AKIA%07d", i), nowMillis - TimeUnit.HOURS.toMillis(13)); } // Set a very small ratisByteLimit (100 bytes) to force batching. A single token request will be small, but 10 @@ -245,7 +245,7 @@ public void submitsMultipleRequestsWhenBatchSizeIsExceeded() throws Exception { // Verify all tokens were included across the requests final int totalTokens = capturedRequests.stream() - .mapToInt(r -> r.getDeleteRevokedSTSTokensRequest().getSessionTokenList().size()) + .mapToInt(r -> r.getDeleteRevokedSTSTokensRequest().getOriginalAccessKeyIdList().size()) .sum(); assertThat(totalTokens).isEqualTo(10); assertThat(revokedSTSTokenCleanupService.getSubmittedDeletedEntryCount()).isEqualTo(10); @@ -254,7 +254,7 @@ public void submitsMultipleRequestsWhenBatchSizeIsExceeded() throws Exception { @Test public void testSingleOversizedExpiredTokenAndItIsTheOnlyExpiredToken() throws Exception { - // One sessionToken is larger than the ratisByteLimit, and it is the only expired token + // One originalAccessKeyId is larger than the ratisByteLimit, and it is the only expired entry final long nowMillis = testClock.millis(); // Serialized size for largeToken is 102 > 90 (the effective ratisByteLimit) . final String largeToken = new String(new char[100]).replace('\0', 'a'); @@ -279,10 +279,10 @@ public void testSingleOversizedExpiredTokenAndItIsTheOnlyExpiredToken() throws E @Test public void testSingleOversizedExpiredTokenAndThereAreMultipleExpiredTokens() throws Exception { - // One sessionToken is larger than the ratisByteLimit, and it is not the only expired token + // One originalAccessKeyId is larger than the ratisByteLimit, and it is not the only expired entry final long nowMillis = testClock.millis(); - final String smallToken = "session-token-j"; - final String largeToken = "session-token-k-" + new String(new char[90]).replace('\0', 'a'); // > 90 bytes + final String smallToken = "AKIASMALL01"; + final String largeToken = "AKIALARGE-" + new String(new char[90]).replace('\0', 'a'); // > 90 bytes revokedStsTokenTable.put(smallToken, nowMillis - TimeUnit.HOURS.toMillis(13)); revokedStsTokenTable.put(largeToken, nowMillis - TimeUnit.HOURS.toMillis(13)); @@ -308,9 +308,9 @@ public void testExpiredAndNonExpiredTokensWithSmallRatisByteLimit() throws Excep // Expired and non-expired entries with ratisByteLimit of 100 final long nowMillis = testClock.millis(); - revokedStsTokenTable.put("session-token-l", nowMillis - TimeUnit.HOURS.toMillis(13)); - revokedStsTokenTable.put("session-token-m", nowMillis - TimeUnit.HOURS.toMillis(1)); // Should be skipped - revokedStsTokenTable.put("session-token-n", nowMillis - TimeUnit.HOURS.toMillis(13)); + revokedStsTokenTable.put("original-access-key-l", nowMillis - TimeUnit.HOURS.toMillis(13)); + revokedStsTokenTable.put("original-access-key-m", nowMillis - TimeUnit.HOURS.toMillis(1)); // Should be skipped + revokedStsTokenTable.put("original-access-key-n", nowMillis - TimeUnit.HOURS.toMillis(13)); ozoneConfiguration.setStorageSize( OMConfigKeys.OZONE_OM_RATIS_LOG_APPENDER_QUEUE_BYTE_LIMIT, 100, StorageUnit.BYTES); @@ -323,10 +323,11 @@ public void testExpiredAndNonExpiredTokensWithSmallRatisByteLimit() throws Excep final RevokedSTSTokenCleanupService revokedSTSTokenCleanupService = createAndRunCleanupService(); assertThat(revokedSTSTokenCleanupService.getRunCount()).isEqualTo(1); - // session-token-l and session-token-n fit in one batch. session-token-m is ignored because it is not expired. + // original-access-key-l and original-access-key-n fit in one batch. + // original-access-key-m is ignored because it is not expired. assertThat(capturedRequests).hasSize(1); - assertThat(capturedRequests.get(0).getDeleteRevokedSTSTokensRequest().getSessionTokenList()) - .containsExactly("session-token-l", "session-token-n"); + assertThat(capturedRequests.get(0).getDeleteRevokedSTSTokensRequest().getOriginalAccessKeyIdList()) + .containsExactly("original-access-key-l", "original-access-key-n"); assertThat(revokedSTSTokenCleanupService.getSubmittedDeletedEntryCount()).isEqualTo(2); } } @@ -359,12 +360,12 @@ public void testExpiredTokenMatchesRatisByteLimitExactly() throws Exception { public void testCallIdCountIncreasesAcrossBatches() throws Exception { // Force small batch of 40 bytes (which should trigger multiple calls to OzoneManagerRatisUtils.submitRequest) // and ensure the callIdCount increases across each batch - // session-token-1 and session-token-2 are in first batch, and session-token-3 is in second batch. + // AKIA0000001 and AKIA0000002 are in first batch, and AKIA0000003 is in second batch. final long nowMillis = testClock.millis(); - revokedStsTokenTable.put("session-token-1", nowMillis - TimeUnit.HOURS.toMillis(13)); - revokedStsTokenTable.put("session-token-2", nowMillis - TimeUnit.HOURS.toMillis(13)); - revokedStsTokenTable.put("session-token-3", nowMillis - TimeUnit.HOURS.toMillis(13)); + revokedStsTokenTable.put("AKIA0000001", nowMillis - TimeUnit.HOURS.toMillis(13)); + revokedStsTokenTable.put("AKIA0000002", nowMillis - TimeUnit.HOURS.toMillis(13)); + revokedStsTokenTable.put("AKIA0000003", nowMillis - TimeUnit.HOURS.toMillis(13)); ozoneConfiguration.setStorageSize(OMConfigKeys.OZONE_OM_RATIS_LOG_APPENDER_QUEUE_BYTE_LIMIT, 40, StorageUnit.BYTES); 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 99c358b929d2..f9d641f60b75 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 @@ -36,8 +36,11 @@ import java.io.IOException; import java.time.Clock; +import java.time.Duration; +import java.time.Instant; import java.util.UUID; -import java.util.concurrent.ThreadLocalRandom; +import javax.crypto.spec.SecretKeySpec; +import org.apache.hadoop.hdds.security.symmetric.ManagedSecretKey; import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; import org.apache.hadoop.hdds.utils.db.InMemoryTestTable; import org.apache.hadoop.hdds.utils.db.Table; @@ -57,21 +60,15 @@ * Tests for STS revocation handling in {@link S3SecurityUtil}. */ public class TestS3SecurityUtil { - private static final byte[] ENCRYPTION_KEY = new byte[5]; + private static final ManagedSecretKey MANAGED_SECRET_KEY = createManagedSecretKey(); private static final MockClock CLOCK = MockClock.newInstance(); private static final String TEMP_ACCESS_KEY_ID = "temp-access-key-id"; - { - ThreadLocalRandom.current().nextBytes(ENCRYPTION_KEY); - } - @Test - public void testValidateS3CredentialFailsWhenTokenRevoked() throws Exception { - // If the revoked STS token table contains an entry for the session token, the request should be rejected with - // REVOKED_TOKEN + public void testValidateS3CredentialFailsWhenTokenCreatedBeforeRevocationCutoff() throws Exception { validateS3CredentialHelper( new TestConfig() - .setTokenRevoked(true) + .setRevocationCutoffOffsetMs(1) .setExpectedResult(REVOKED_TOKEN) .setExpectedMessage("STS token has been revoked")); } @@ -162,6 +159,22 @@ public void testValidateS3CredentialFailsWhenRequestAccessIdEmpty() throws Excep .setExpectedMessage("STS token validation failed - accessKeyId is invalid for session token")); } + @Test + public void testValidateS3CredentialSuccessWhenTokenCreatedAfterRevocationCutoff() throws Exception { + validateS3CredentialHelper( + new TestConfig() + .setRevocationCutoffOffsetMs(-1) + .setExpectedResult(null)); + } + + @Test + public void testValidateS3CredentialSuccessWhenTokenCreatedAtRevocationCutoff() throws Exception { + validateS3CredentialHelper( + new TestConfig() + .setRevocationCutoffOffsetMs(0) + .setExpectedResult(null)); + } + private void validateS3CredentialHelper(TestConfig config) throws Exception { try (OzoneManager ozoneManager = mock(OzoneManager.class)) { when(ozoneManager.isSecurityEnabled()).thenReturn(true); @@ -188,12 +201,13 @@ private void validateS3CredentialHelper(TestConfig config) throws Exception { } final String sessionToken = "session-token"; - if (config.isTokenRevoked && config.revokedSTSTokenTable != null) { - final long insertionTimeMillis = CLOCK.millis(); - config.revokedSTSTokenTable.put(sessionToken, insertionTimeMillis); - } - final STSTokenIdentifier stsTokenIdentifier = createSTSTokenIdentifier(); + final String originalAccessKeyId = stsTokenIdentifier.getOriginalAccessKeyId(); + if (config.revocationCutoffOffsetMs != null && config.revokedSTSTokenTable != null) { + final long revocationTimeMillis = stsTokenIdentifier.getCreationTime().toEpochMilli() + + config.revocationCutoffOffsetMs; + config.revokedSTSTokenTable.put(originalAccessKeyId, revocationTimeMillis); + } try (MockedStatic stsSecurityUtilMock = mockStatic(STSSecurityUtil.class, CALLS_REAL_METHODS); MockedStatic awsV4AuthValidatorMock = mockStatic( @@ -229,10 +243,28 @@ private void validateS3CredentialHelper(TestConfig config) throws Exception { } private STSTokenIdentifier createSTSTokenIdentifier() { - return new STSTokenIdentifier( - TEMP_ACCESS_KEY_ID, "original-access-key-id", "arn:aws:iam::123456789012:role/test-role", - CLOCK.instant().plusSeconds(3600), "secret-access-key", "session-policy", - ENCRYPTION_KEY); + return new STSTokenIdentifier(STSTokenIdentifier.Params.newBuilder() + .setTempAccessKeyId(TEMP_ACCESS_KEY_ID) + .setOriginalAccessKeyId("original-access-key-id") + .setRoleArn("arn:aws:iam::123456789012:role/test-role") + .setCreationTime(CLOCK.instant()) + .setExpiry(CLOCK.instant().plusSeconds(3600)) + .setSecretAccessKey("secret-access-key") + .setSessionPolicy("session-policy") + .setManagedSecretKey(MANAGED_SECRET_KEY) + .build()); + } + + private static ManagedSecretKey createManagedSecretKey() { + final byte[] keyBytes = new byte[32]; + for (int i = 0; i < keyBytes.length; i++) { + keyBytes[i] = (byte) i; + } + return new ManagedSecretKey( + UUID.randomUUID(), + Instant.EPOCH, + Instant.EPOCH.plus(Duration.ofDays(1)), + new SecretKeySpec(keyBytes, "HmacSHA256")); } private static OMRequest createRequestWithSessionToken(String accessId, boolean includeAccessId) { @@ -258,7 +290,7 @@ private static OMRequest createRequestWithSessionToken(String accessId, boolean private static final class TestConfig { private OMMetadataManager metadataManager = mock(OMMetadataManager.class); private Table revokedSTSTokenTable = new InMemoryTestTable<>(); - private boolean isTokenRevoked = false; + private Long revocationCutoffOffsetMs = null; private boolean isOriginalAccessKeyIdRevoked = false; private boolean shouldOriginalAccessKeyIdCheckThrowError = false; private String requestAccessId = TEMP_ACCESS_KEY_ID; @@ -277,9 +309,8 @@ TestConfig setRevokedSTSTokenTable(Table table) { return this; } - @SuppressWarnings("SameParameterValue") - TestConfig setTokenRevoked(boolean isRevoked) { - this.isTokenRevoked = isRevoked; + TestConfig setRevocationCutoffOffsetMs(long offsetMs) { + this.revocationCutoffOffsetMs = offsetMs; return this; } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSSecurityUtil.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSSecurityUtil.java index d2033deabec1..290d848535a8 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSSecurityUtil.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSSecurityUtil.java @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.security; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_TOKEN; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TOKEN_EXPIRED; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -29,7 +30,6 @@ import java.time.Instant; import java.time.ZoneOffset; import java.util.UUID; -import java.util.concurrent.ThreadLocalRandom; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.symmetric.ManagedSecretKey; import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; @@ -54,17 +54,12 @@ public class TestSTSSecurityUtil { private static final String SECRET_ACCESS_KEY = "test-secret-access-key"; private static final String SESSION_POLICY = "test-session-policy"; private static final int DURATION_SECONDS = 3600; - private static final byte[] ENCRYPTION_KEY = new byte[5]; - + private static final ManagedSecretKey MANAGED_SECRET_KEY = new SecretKeyTestClient().getCurrentSecretKey(); private final SecretKeyTestClient secretKeyClient = new SecretKeyTestClient(); private final STSTokenSecretManager tokenSecretManager = new STSTokenSecretManager(secretKeyClient); private final UUID secretKeyId = secretKeyClient.getCurrentSecretKey().getId(); private final MockClock clock = new MockClock(Instant.ofEpochMilli(1764819000), ZoneOffset.UTC); - { - ThreadLocalRandom.current().nextBytes(ENCRYPTION_KEY); - } - @Test public void testConstructValidateAndDecryptSTSTokenInvalidProtobuf() throws IOException { // Create a token whose identifier bytes are not a valid OMTokenProto @@ -98,6 +93,7 @@ public void testConstructValidateAndDecryptSTSTokenSuccess() throws IOException assertThat(result.getRoleArn()).isEqualTo(ROLE_ARN); assertThat(result.getSecretAccessKey()).isEqualTo(SECRET_ACCESS_KEY); assertThat(result.getSessionPolicy()).isEqualTo(SESSION_POLICY); + assertThat(result.getCreationTime()).isEqualTo(clock.instant()); assertThat(result.isExpired(clock.instant())).isFalse(); final long expirationEpochMillis = result.getExpiry().toEpochMilli(); assertThat(expirationEpochMillis).isEqualTo(clock.millis() + (DURATION_SECONDS * 1000)); @@ -126,6 +122,16 @@ public void testConstructValidateAndDecryptSTSTokenInvalidFormat() { .hasMessageContaining("Invalid STS token format: Failed to decode STS token string"); } + @Test + public void testConstructValidateAndDecryptSTSTokenRuntimeDecodeFailure() { + assertThatThrownBy(() -> + STSSecurityUtil.constructValidateAndDecryptSTSToken("not-a-valid-token", secretKeyClient, clock)) + .isInstanceOf(OMException.class) + .satisfies(exception -> assertThat(((OMException) exception).getResult()).isEqualTo(INVALID_TOKEN)) + .hasMessageContaining("Invalid STS token format: Failed to decode STS token string") + .hasMessageContaining("NegativeArraySizeException"); + } + @Test public void testConstructValidateAndDecryptSTSTokenInvalidKind() throws Exception { // Create a valid identifier to use as base @@ -303,7 +309,8 @@ public void testConstructValidateAndDecryptSTSTokenEmptyString() { assertThatThrownBy(() -> STSSecurityUtil.constructValidateAndDecryptSTSToken("", secretKeyClient, clock)) .isInstanceOf(OMException.class) - .hasMessage("Invalid STS token format: Failed to decode STS token string: java.io.EOFException"); + .hasMessage( + "Invalid STS token format: Failed to decode STS token string: java.io.EOFException"); } @Test @@ -330,8 +337,7 @@ public void testConstructValidateAndDecryptMultipleTokens() throws Exception { @Test public void testEnsureEssentialFieldsArePresentInTokenMissingExpiry() { - final STSTokenIdentifier tokenIdentifier = new STSTokenIdentifier( - TEMP_ACCESS_KEY, ORIGINAL_ACCESS_KEY, ROLE_ARN, null, SECRET_ACCESS_KEY, SESSION_POLICY, ENCRYPTION_KEY); + final STSTokenIdentifier tokenIdentifier = new STSTokenIdentifier(paramsBuilder().setExpiry(null).build()); assertThatThrownBy(() -> STSSecurityUtil.ensureEssentialFieldsArePresentInToken(tokenIdentifier)) .isInstanceOf(SecretManager.InvalidToken.class) @@ -340,8 +346,7 @@ public void testEnsureEssentialFieldsArePresentInTokenMissingExpiry() { @Test public void testEnsureEssentialFieldsArePresentInTokenMissingTempAccessKeyId() { - final STSTokenIdentifier tokenIdentifier = new STSTokenIdentifier( - null, ORIGINAL_ACCESS_KEY, ROLE_ARN, clock.instant(), SECRET_ACCESS_KEY, SESSION_POLICY, ENCRYPTION_KEY); + final STSTokenIdentifier tokenIdentifier = new STSTokenIdentifier(paramsBuilder().setTempAccessKeyId(null).build()); assertThatThrownBy(() -> STSSecurityUtil.ensureEssentialFieldsArePresentInToken(tokenIdentifier)) .isInstanceOf(SecretManager.InvalidToken.class) @@ -350,8 +355,7 @@ public void testEnsureEssentialFieldsArePresentInTokenMissingTempAccessKeyId() { @Test public void testEnsureEssentialFieldsArePresentInTokenMissingRoleArn() { - final STSTokenIdentifier tokenIdentifier = new STSTokenIdentifier( - TEMP_ACCESS_KEY, ORIGINAL_ACCESS_KEY, null, clock.instant(), SECRET_ACCESS_KEY, SESSION_POLICY, ENCRYPTION_KEY); + final STSTokenIdentifier tokenIdentifier = new STSTokenIdentifier(paramsBuilder().setRoleArn(null).build()); assertThatThrownBy(() -> STSSecurityUtil.ensureEssentialFieldsArePresentInToken(tokenIdentifier)) .isInstanceOf(SecretManager.InvalidToken.class) @@ -361,7 +365,7 @@ public void testEnsureEssentialFieldsArePresentInTokenMissingRoleArn() { @Test public void testEnsureEssentialFieldsArePresentInTokenMissingOriginalAccessKeyId() { final STSTokenIdentifier tokenIdentifier = new STSTokenIdentifier( - TEMP_ACCESS_KEY, null, ROLE_ARN, clock.instant(), SECRET_ACCESS_KEY, SESSION_POLICY, ENCRYPTION_KEY); + paramsBuilder().setOriginalAccessKeyId(null).build()); assertThatThrownBy(() -> STSSecurityUtil.ensureEssentialFieldsArePresentInToken(tokenIdentifier)) .isInstanceOf(SecretManager.InvalidToken.class) @@ -370,14 +374,22 @@ public void testEnsureEssentialFieldsArePresentInTokenMissingOriginalAccessKeyId @Test public void testEnsureEssentialFieldsArePresentInTokenMissingSecretAccessKey() { - final STSTokenIdentifier tokenIdentifier = new STSTokenIdentifier( - TEMP_ACCESS_KEY, ORIGINAL_ACCESS_KEY, ROLE_ARN, clock.instant(), null, SESSION_POLICY, ENCRYPTION_KEY); + final STSTokenIdentifier tokenIdentifier = new STSTokenIdentifier(paramsBuilder().setSecretAccessKey(null).build()); assertThatThrownBy(() -> STSSecurityUtil.ensureEssentialFieldsArePresentInToken(tokenIdentifier)) .isInstanceOf(SecretManager.InvalidToken.class) .hasMessage("Invalid STS token - secretAccessKey is null/empty"); } + @Test + public void testEnsureEssentialFieldsArePresentInTokenMissingCreationTime() { + final STSTokenIdentifier tokenIdentifier = new STSTokenIdentifier(paramsBuilder().setCreationTime(null).build()); + + assertThatThrownBy(() -> STSSecurityUtil.ensureEssentialFieldsArePresentInToken(tokenIdentifier)) + .isInstanceOf(SecretManager.InvalidToken.class) + .hasMessage("Invalid STS token - creationTime is null"); + } + @Test public void testEnsureResolvedStsFieldsInvariantsSuccess() throws Exception { final String tokenString = tokenSecretManager.createSTSTokenString( @@ -449,4 +461,16 @@ public void testEnsureResolvedStsFieldsInvariantsNoS3Auth() throws Exception { // Should not throw STSSecurityUtil.ensureResolvedStsFieldsInvariants(request); } + + private STSTokenIdentifier.Params.Builder paramsBuilder() { + return STSTokenIdentifier.Params.newBuilder() + .setTempAccessKeyId(TEMP_ACCESS_KEY) + .setOriginalAccessKeyId(ORIGINAL_ACCESS_KEY) + .setRoleArn(ROLE_ARN) + .setCreationTime(clock.instant()) + .setExpiry(clock.instant().plusSeconds(DURATION_SECONDS)) + .setSecretAccessKey(SECRET_ACCESS_KEY) + .setSessionPolicy(SESSION_POLICY) + .setManagedSecretKey(MANAGED_SECRET_KEY); + } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenEncryption.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenEncryption.java index 1eb880f9dd03..268e672a38fb 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenEncryption.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenEncryption.java @@ -23,12 +23,14 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.time.Instant; import java.util.Base64; import java.util.UUID; import javax.crypto.KeyGenerator; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; +import org.apache.hadoop.hdds.security.symmetric.ManagedSecretKey; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.security.STSTokenEncryption.STSTokenEncryptionException; import org.junit.jupiter.api.BeforeAll; @@ -43,11 +45,17 @@ public class TestSTSTokenEncryption { private static final int HKDF_SALT_LENGTH = 16; // 128 bits private static SecretKey sharedSecretKey; + private static ManagedSecretKey managedSecretKey; @BeforeAll public static void setUpClass() { final byte[] keyBytes = "01234567890123456789012345678901".getBytes(StandardCharsets.US_ASCII); sharedSecretKey = new SecretKeySpec(keyBytes, "HmacSHA256"); + managedSecretKey = new ManagedSecretKey( + UUID.randomUUID(), + Instant.EPOCH, + Instant.EPOCH.plus(Duration.ofDays(1)), + sharedSecretKey); } @Test @@ -69,20 +77,26 @@ public void testEncryptDecryptRoundTrip() throws Exception { @Test public void testSTSTokenIdentifierEncryption() throws Exception { - final byte[] keyBytes = sharedSecretKey.getEncoded(); - final String tempAccessKeyId = "ASIA123TEMPKEY"; final String originalAccessKeyId = "AKIA123ORIGINAL"; final String roleArn = "arn:aws:iam::123456789012:role/TestRole"; final String secretAccessKey = "mySecretAccessKey123456"; // Use millisecond precision to match serialization format - final Instant expiry = Instant.ofEpochMilli(Instant.now().plusSeconds(3600).toEpochMilli()); + final Instant creationTime = Instant.ofEpochMilli(1_700_000_000_000L); + final Instant expiry = creationTime.plusSeconds(3600); final String sessionPolicy = "test-session-policy"; - + // Create token identifier with encryption - final STSTokenIdentifier tokenId = new STSTokenIdentifier( - tempAccessKeyId, originalAccessKeyId, roleArn, expiry, secretAccessKey, sessionPolicy, keyBytes); - tokenId.setSecretKeyId(UUID.randomUUID()); + final STSTokenIdentifier tokenId = new STSTokenIdentifier(STSTokenIdentifier.Params.newBuilder() + .setTempAccessKeyId(tempAccessKeyId) + .setOriginalAccessKeyId(originalAccessKeyId) + .setRoleArn(roleArn) + .setCreationTime(creationTime) + .setExpiry(expiry) + .setSecretAccessKey(secretAccessKey) + .setSessionPolicy(sessionPolicy) + .setManagedSecretKey(managedSecretKey) + .build()); // Convert to protobuf final OzoneManagerProtocolProtos.OMTokenProto omTokenProto = tokenId.toProtoBuf(); @@ -91,7 +105,7 @@ public void testSTSTokenIdentifierEncryption() throws Exception { // Create new token identifier from protobuf with decryption key final STSTokenIdentifier decodedTokenId = new STSTokenIdentifier(); - decodedTokenId.setEncryptionKey(keyBytes); + decodedTokenId.setManagedSecretKey(managedSecretKey); decodedTokenId.readFromByteArray(protobufBytes); // Verify all fields are correctly decrypted @@ -100,6 +114,7 @@ public void testSTSTokenIdentifierEncryption() throws Exception { assertEquals(roleArn, decodedTokenId.getRoleArn()); assertEquals(secretAccessKey, decodedTokenId.getSecretAccessKey()); assertEquals(expiry, decodedTokenId.getExpiry()); + assertEquals(creationTime, decodedTokenId.getCreationTime()); } @Test diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenIdentifier.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenIdentifier.java index 09a786faaea3..c2136388e2a0 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenIdentifier.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenIdentifier.java @@ -24,11 +24,13 @@ import java.io.ByteArrayOutputStream; import java.io.DataOutputStream; import java.io.IOException; -import java.security.SecureRandom; +import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.UUID; import java.util.concurrent.ThreadLocalRandom; +import javax.crypto.spec.SecretKeySpec; +import org.apache.hadoop.hdds.security.symmetric.ManagedSecretKey; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMTokenProto; import org.junit.jupiter.api.Test; @@ -37,17 +39,25 @@ */ public class TestSTSTokenIdentifier { - private static final byte[] ENCRYPTION_KEY = new byte[5]; + private static final byte[] SECRET_KEY_BYTES = new byte[5]; + private static final ManagedSecretKey MANAGED_SECRET_KEY; + private static final Instant CREATION_TIME = Instant.ofEpochMilli(1_700_000_000_000L); - { - ThreadLocalRandom.current().nextBytes(ENCRYPTION_KEY); + static { + ThreadLocalRandom.current().nextBytes(SECRET_KEY_BYTES); + MANAGED_SECRET_KEY = createManagedSecretKey(SECRET_KEY_BYTES); } @Test public void testKindAndService() { - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", - Instant.now().plusSeconds(3600), "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(Instant.now().plusSeconds(3600)) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); assertEquals("STSToken", stsTokenIdentifier.getKind().toString()); assertEquals("STS", stsTokenIdentifier.getService()); @@ -59,16 +69,22 @@ public void testProtoBufRoundTrip() throws IOException { // so use a millisecond-precision Instant to avoid nanos-only differences across // platforms/JDKs during round-trips. final Instant expiry = Instant.now().plusSeconds(7200).truncatedTo(ChronoUnit.MILLIS); - final STSTokenIdentifier originalTokenIdentifier = new STSTokenIdentifier( - "tempAccess", "origAccess", "arn:aws:iam::123456789012:role/RoleY", - expiry, "secretKey", "sessionPolicy", ENCRYPTION_KEY); - final UUID secretKeyId = UUID.randomUUID(); - originalTokenIdentifier.setSecretKeyId(secretKeyId); + final STSTokenIdentifier originalTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccess") + .setOriginalAccessKeyId("origAccess") + .setRoleArn("arn:aws:iam::123456789012:role/RoleY") + .setExpiry(expiry) + .setSecretAccessKey("secretKey") + .setSessionPolicy("sessionPolicy") + .setManagedSecretKey(MANAGED_SECRET_KEY) + .build()); + final UUID secretKeyId = MANAGED_SECRET_KEY.getId(); final OMTokenProto proto = originalTokenIdentifier.toProtoBuf(); assertThat(proto.getType()).isEqualTo(OMTokenProto.Type.S3_STS_TOKEN); assertThat(proto.getOwner()).isEqualTo("tempAccess"); assertThat(proto.getMaxDate()).isEqualTo(expiry.toEpochMilli()); + assertThat(proto.getIssueDate()).isEqualTo(CREATION_TIME.toEpochMilli()); assertThat(proto.getOriginalAccessKeyId()).isEqualTo("origAccess"); assertThat(proto.getRoleArn()).isEqualTo("arn:aws:iam::123456789012:role/RoleY"); assertThat(proto.getSecretAccessKey()).isNotEqualTo("secretKey"); // must be encrypted @@ -76,11 +92,12 @@ public void testProtoBufRoundTrip() throws IOException { assertThat(proto.getSecretKeyId()).isEqualTo(secretKeyId.toString()); final STSTokenIdentifier parsedTokenIdentifier = new STSTokenIdentifier(); - parsedTokenIdentifier.setEncryptionKey(ENCRYPTION_KEY); + parsedTokenIdentifier.setManagedSecretKey(MANAGED_SECRET_KEY); parsedTokenIdentifier.fromProtoBuf(proto); assertThat(parsedTokenIdentifier.getOwnerId()).isEqualTo("tempAccess"); assertThat(parsedTokenIdentifier.getExpiry()).isEqualTo(expiry); + assertThat(parsedTokenIdentifier.getCreationTime()).isEqualTo(CREATION_TIME); assertThat(parsedTokenIdentifier.getOriginalAccessKeyId()).isEqualTo("origAccess"); assertThat(parsedTokenIdentifier.getRoleArn()).isEqualTo("arn:aws:iam::123456789012:role/RoleY"); assertThat(parsedTokenIdentifier.getSecretAccessKey()).isEqualTo("secretKey"); @@ -99,9 +116,14 @@ public void testFromProtoBufInvalidSecretKeyId() { .setSecretKeyId("not-a-uuid") .build(); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", Instant.now(), - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(Instant.now()) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); final IOException ex = assertThrows(IOException.class, () -> stsTokenIdentifier.fromProtoBuf(invalid)); assertThat(ex.getMessage()).isEqualTo("Invalid secretKeyId format in STS token: not-a-uuid"); @@ -110,17 +132,20 @@ public void testFromProtoBufInvalidSecretKeyId() { @Test public void testProtobufRoundTripWithNullSessionPolicy() throws IOException { final Instant expiry = Instant.now().plusSeconds(7200); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccess", "origAccess", "arn:aws:iam::123456789012:role/RoleX", - expiry, "secretKey", null, ENCRYPTION_KEY); - final UUID secretKeyId = UUID.randomUUID(); - stsTokenIdentifier.setSecretKeyId(secretKeyId); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccess") + .setOriginalAccessKeyId("origAccess") + .setRoleArn("arn:aws:iam::123456789012:role/RoleX") + .setExpiry(expiry) + .setSecretAccessKey("secretKey") + .setManagedSecretKey(MANAGED_SECRET_KEY) + .build()); final OMTokenProto proto = stsTokenIdentifier.toProtoBuf(); assertThat(proto.getSessionPolicy()).isEmpty(); final STSTokenIdentifier parsedTokenIdentifier = new STSTokenIdentifier(); - parsedTokenIdentifier.setEncryptionKey(ENCRYPTION_KEY); + parsedTokenIdentifier.setManagedSecretKey(MANAGED_SECRET_KEY); parsedTokenIdentifier.fromProtoBuf(proto); assertThat(parsedTokenIdentifier.getSessionPolicy()).isEmpty(); @@ -129,17 +154,21 @@ public void testProtobufRoundTripWithNullSessionPolicy() throws IOException { @Test public void testProtobufRoundTripWithEmptySessionPolicy() throws IOException { final Instant expiry = Instant.now().plusSeconds(4000); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccess", "origAccess", "arn:aws:iam::123456789012:role/RoleZ", - expiry, "secretKey", "", ENCRYPTION_KEY); - final UUID secretKeyId = UUID.randomUUID(); - stsTokenIdentifier.setSecretKeyId(secretKeyId); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccess") + .setOriginalAccessKeyId("origAccess") + .setRoleArn("arn:aws:iam::123456789012:role/RoleZ") + .setExpiry(expiry) + .setSecretAccessKey("secretKey") + .setSessionPolicy("") + .setManagedSecretKey(MANAGED_SECRET_KEY) + .build()); final OMTokenProto proto = stsTokenIdentifier.toProtoBuf(); assertThat(proto.getSessionPolicy()).isEmpty(); final STSTokenIdentifier parsedTokenIdentifier = new STSTokenIdentifier(); - parsedTokenIdentifier.setEncryptionKey(ENCRYPTION_KEY); + parsedTokenIdentifier.setManagedSecretKey(MANAGED_SECRET_KEY); parsedTokenIdentifier.fromProtoBuf(proto); assertThat(parsedTokenIdentifier.getSessionPolicy()).isEmpty(); @@ -153,9 +182,14 @@ public void testFromProtoBufInvalidTokenType() { .setMaxDate(Instant.now().toEpochMilli()) .build(); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "origAccessKeyId", "roleArn", Instant.now(), - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("origAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(Instant.now()) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); final IllegalArgumentException ex = assertThrows( IllegalArgumentException.class, () -> stsTokenIdentifier.fromProtoBuf(invalidType)); @@ -169,10 +203,15 @@ public void testWriteToAndReadFromByteArray() throws Exception { // compared to the original object, which is compared using equals(). final Instant expiry = Instant.now().plusSeconds(1000).truncatedTo(ChronoUnit.MILLIS); - final STSTokenIdentifier originalTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); - originalTokenIdentifier.setSecretKeyId(UUID.randomUUID()); + final STSTokenIdentifier originalTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .setManagedSecretKey(MANAGED_SECRET_KEY) + .build()); final ByteArrayOutputStream baos = new ByteArrayOutputStream(); try (DataOutputStream out = new DataOutputStream(baos)) { @@ -181,35 +220,41 @@ public void testWriteToAndReadFromByteArray() throws Exception { final byte[] bytes = baos.toByteArray(); final STSTokenIdentifier parsedTokenIdentifier = new STSTokenIdentifier(); - parsedTokenIdentifier.setEncryptionKey(ENCRYPTION_KEY); + parsedTokenIdentifier.setManagedSecretKey(MANAGED_SECRET_KEY); parsedTokenIdentifier.readFromByteArray(bytes); assertThat(parsedTokenIdentifier).isEqualTo(originalTokenIdentifier); } @Test - public void testWriteToAndReadFromByteArrayWithDifferentSecretKeyIds() throws Exception { - final UUID uuid1 = UUID.randomUUID(); - UUID uuid2 = UUID.randomUUID(); - if (uuid2.equals(uuid1)) { - uuid2 = UUID.randomUUID(); - } - + public void testWriteToAndReadFromByteArrayWithDifferentSecretKeys() throws Exception { final Instant expiry = Instant.now().plusSeconds(1500); - final STSTokenIdentifier originalTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); - originalTokenIdentifier.setSecretKeyId(uuid1); + final STSTokenIdentifier originalTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .setManagedSecretKey(MANAGED_SECRET_KEY) + .build()); final ByteArrayOutputStream baos1 = new ByteArrayOutputStream(); try (DataOutputStream out = new DataOutputStream(baos1)) { originalTokenIdentifier.write(out); } - final STSTokenIdentifier anotherTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); - anotherTokenIdentifier.setSecretKeyId(uuid2); + byte[] rawBytes = new byte[5]; + ManagedSecretKey managedSecretKey2 = createManagedSecretKey(rawBytes); + final STSTokenIdentifier anotherTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .setManagedSecretKey(managedSecretKey2) + .build()); final ByteArrayOutputStream baos2 = new ByteArrayOutputStream(); try (DataOutputStream out = new DataOutputStream(baos2)) { @@ -223,33 +268,42 @@ public void testWriteToAndReadFromByteArrayWithDifferentSecretKeyIds() throws Ex final byte[] byteArr2 = baos2.toByteArray(); assertThat(byteArr1).isNotEqualTo(byteArr2); final STSTokenIdentifier tokenFromByteArr1 = new STSTokenIdentifier(); - tokenFromByteArr1.setEncryptionKey(ENCRYPTION_KEY); + tokenFromByteArr1.setManagedSecretKey(MANAGED_SECRET_KEY); tokenFromByteArr1.readFromByteArray(byteArr1); final STSTokenIdentifier tokenFromByteArr2 = new STSTokenIdentifier(); - tokenFromByteArr2.setEncryptionKey(ENCRYPTION_KEY); + tokenFromByteArr2.setManagedSecretKey(managedSecretKey2); tokenFromByteArr2.readFromByteArray(byteArr2); assertThat(tokenFromByteArr1).isNotEqualTo(tokenFromByteArr2); } @Test public void testWriteToAndReadFromByteArrayWithSameSecretKeyIds() throws Exception { - final UUID uuid = UUID.randomUUID(); final Instant expiry = Instant.now().plusSeconds(1700); - final STSTokenIdentifier originalTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); - originalTokenIdentifier.setSecretKeyId(uuid); + final STSTokenIdentifier originalTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .setManagedSecretKey(MANAGED_SECRET_KEY) + .build()); final ByteArrayOutputStream baos1 = new ByteArrayOutputStream(); try (DataOutputStream out = new DataOutputStream(baos1)) { originalTokenIdentifier.write(out); } - final STSTokenIdentifier anotherTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); - anotherTokenIdentifier.setSecretKeyId(uuid); + final STSTokenIdentifier anotherTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .setManagedSecretKey(MANAGED_SECRET_KEY) + .build()); final ByteArrayOutputStream baos2 = new ByteArrayOutputStream(); try (DataOutputStream out = new DataOutputStream(baos2)) { @@ -262,10 +316,10 @@ public void testWriteToAndReadFromByteArrayWithSameSecretKeyIds() throws Excepti final byte[] byteArr2 = baos2.toByteArray(); assertThat(byteArr1).isNotEqualTo(byteArr2); final STSTokenIdentifier tokenFromByteArr1 = new STSTokenIdentifier(); - tokenFromByteArr1.setEncryptionKey(ENCRYPTION_KEY); + tokenFromByteArr1.setManagedSecretKey(MANAGED_SECRET_KEY); tokenFromByteArr1.readFromByteArray(byteArr1); final STSTokenIdentifier tokenFromByteArr2 = new STSTokenIdentifier(); - tokenFromByteArr2.setEncryptionKey(ENCRYPTION_KEY); + tokenFromByteArr2.setManagedSecretKey(MANAGED_SECRET_KEY); tokenFromByteArr2.readFromByteArray(byteArr2); assertThat(tokenFromByteArr1).isEqualTo(tokenFromByteArr2); } @@ -279,13 +333,20 @@ public void testGettersReturnCorrectValues() { final String secretAccessKey = "mySecretKey"; final String sessionPolicy = "myPolicy"; - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - tempAccessKeyId, originalAccessKeyId, roleArn, expiry, secretAccessKey, sessionPolicy, ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId(tempAccessKeyId) + .setOriginalAccessKeyId(originalAccessKeyId) + .setRoleArn(roleArn) + .setExpiry(expiry) + .setSecretAccessKey(secretAccessKey) + .setSessionPolicy(sessionPolicy) + .build()); assertThat(stsTokenIdentifier.getOwnerId()).isEqualTo(tempAccessKeyId); assertThat(stsTokenIdentifier.getTempAccessKeyId()).isEqualTo(tempAccessKeyId); assertThat(stsTokenIdentifier.getOriginalAccessKeyId()).isEqualTo(originalAccessKeyId); assertThat(stsTokenIdentifier.getRoleArn()).isEqualTo(roleArn); + assertThat(stsTokenIdentifier.getCreationTime()).isEqualTo(CREATION_TIME); assertThat(stsTokenIdentifier.getExpiry()).isEqualTo(expiry); assertThat(stsTokenIdentifier.getSecretAccessKey()).isEqualTo(secretAccessKey); assertThat(stsTokenIdentifier.getSessionPolicy()).isEqualTo(sessionPolicy); @@ -296,14 +357,24 @@ public void testEqualsAndHashCode() { final Instant expiry = Instant.now().plusSeconds(3600); final UUID uuid = UUID.randomUUID(); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); stsTokenIdentifier.setSecretKeyId(uuid); - final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); stsTokenIdentifier2.setSecretKeyId(uuid); assertThat(stsTokenIdentifier).isEqualTo(stsTokenIdentifier2); @@ -314,13 +385,23 @@ public void testEqualsAndHashCode() { public void testNotEqualsWhenTempAccessKeyIdDiffers() { final Instant expiry = Instant.now().plusSeconds(3600); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId1", "originalAccessKeyId", "roleArn", - expiry, "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); - - final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier( - "tempAccessKeyId2", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId1") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); + + final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId2") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); assertThat(stsTokenIdentifier).isNotEqualTo(stsTokenIdentifier2); } @@ -329,13 +410,23 @@ public void testNotEqualsWhenTempAccessKeyIdDiffers() { public void testNotEqualsWhenOriginalAccessKeyIdDiffers() { final Instant expiry = Instant.now().plusSeconds(3600); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId1", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); - - final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId2", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId1") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); + + final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId2") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); assertThat(stsTokenIdentifier).isNotEqualTo(stsTokenIdentifier2); } @@ -344,26 +435,46 @@ public void testNotEqualsWhenOriginalAccessKeyIdDiffers() { public void testNotEqualsWhenRoleArnDiffers() { final Instant expiry = Instant.now().plusSeconds(3600); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn1", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); - - final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn2", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn1") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); + + final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn2") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); assertThat(stsTokenIdentifier).isNotEqualTo(stsTokenIdentifier2); } @Test public void testNotEqualsWhenExpirationDiffers() { - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", - Instant.now().plusSeconds(3600), "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); - - final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", - Instant.now().plusSeconds(7600), "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(Instant.now().plusSeconds(3600)) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); + + final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(Instant.now().plusSeconds(7600)) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); assertThat(stsTokenIdentifier).isNotEqualTo(stsTokenIdentifier2); } @@ -372,13 +483,23 @@ public void testNotEqualsWhenExpirationDiffers() { public void testNotEqualsWhenSecretAccessKeyDiffers() { final Instant expiry = Instant.now().plusSeconds(3600); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey1", "sessionPolicy", ENCRYPTION_KEY); - - final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey2", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey1") + .setSessionPolicy("sessionPolicy") + .build()); + + final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey2") + .setSessionPolicy("sessionPolicy") + .build()); assertThat(stsTokenIdentifier).isNotEqualTo(stsTokenIdentifier2); } @@ -387,13 +508,23 @@ public void testNotEqualsWhenSecretAccessKeyDiffers() { public void testNotEqualsWhenSessionPolicyDiffers() { final Instant expiry = Instant.now().plusSeconds(3600); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy1", ENCRYPTION_KEY); - - final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy2", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy1") + .build()); + + final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy2") + .build()); assertThat(stsTokenIdentifier).isNotEqualTo(stsTokenIdentifier2); } @@ -403,14 +534,20 @@ public void testToString() { final Instant expiry = Instant.now().plusSeconds(3600); final UUID uuid = UUID.randomUUID(); - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); stsTokenIdentifier.setSecretKeyId(uuid); final String stsTokenIdentifierStr = stsTokenIdentifier.toString(); final String expectedString = "STSTokenIdentifier{" + "tempAccessKeyId='tempAccessKeyId'" + - ", originalAccessKeyId='originalAccessKeyId'" + ", roleArn='roleArn'" + ", expiry='" + expiry + + ", originalAccessKeyId='originalAccessKeyId'" + ", roleArn='roleArn'" + + ", creationTime='" + CREATION_TIME + "', expiry='" + expiry + "', secretKeyId='" + uuid + "', sessionPolicy='sessionPolicy'" + '}'; assertEquals(expectedString, stsTokenIdentifierStr); @@ -418,37 +555,65 @@ public void testToString() { @Test public void testNotEqualsWithNull() { - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", Instant.now(), - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(Instant.now()) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); assertThat(stsTokenIdentifier).isNotEqualTo(null); } @Test - public void testEqualsWithDifferentEncryptionKeys() { + public void testEqualsWithDifferentManagedSecretKeys() { final Instant expiry = Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.MILLIS); final UUID uuid = UUID.randomUUID(); // Create first identifier with the default key - final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", ENCRYPTION_KEY); + final STSTokenIdentifier stsTokenIdentifier = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .build()); stsTokenIdentifier.setSecretKeyId(uuid); - // Create second identifier with a different encryption key but otherwise same parameters - byte[] differentKey = new byte[5]; - new SecureRandom().nextBytes(differentKey); - - final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier( - "tempAccessKeyId", "originalAccessKeyId", "roleArn", expiry, - "secretAccessKey", "sessionPolicy", differentKey); + // Create second identifier with a different ManagedSecretKey but otherwise same parameters + byte[] differentKeyBytes = new byte[5]; + ThreadLocalRandom.current().nextBytes(differentKeyBytes); + + final STSTokenIdentifier stsTokenIdentifier2 = new STSTokenIdentifier(paramsBuilder() + .setTempAccessKeyId("tempAccessKeyId") + .setOriginalAccessKeyId("originalAccessKeyId") + .setRoleArn("roleArn") + .setExpiry(expiry) + .setSecretAccessKey("secretAccessKey") + .setSessionPolicy("sessionPolicy") + .setManagedSecretKey(createManagedSecretKey(differentKeyBytes)) + .build()); stsTokenIdentifier2.setSecretKeyId(uuid); - // They should still be equal because encryptionKey is transient/ignored for identity + // They should still be equal because managedSecretKey is transient/ignored for identity assertThat(stsTokenIdentifier).isEqualTo(stsTokenIdentifier2); assertThat(stsTokenIdentifier.hashCode()).isEqualTo(stsTokenIdentifier2.hashCode()); } -} + private static ManagedSecretKey createManagedSecretKey(byte[] keyBytes) { + return new ManagedSecretKey( + UUID.randomUUID(), + CREATION_TIME, + CREATION_TIME.plus(Duration.ofDays(1)), + new SecretKeySpec(keyBytes, "HmacSHA256")); + } + private static STSTokenIdentifier.Params.Builder paramsBuilder() { + return STSTokenIdentifier.Params.newBuilder() + .setCreationTime(CREATION_TIME) + .setManagedSecretKey(MANAGED_SECRET_KEY); + } +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenSecretManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenSecretManager.java index 800aeabe97c5..4408652dbb9d 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenSecretManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestSTSTokenSecretManager.java @@ -26,12 +26,16 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.time.Instant; import java.time.ZoneOffset; +import java.util.HashMap; +import java.util.Map; import java.util.UUID; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; import org.apache.hadoop.hdds.security.symmetric.ManagedSecretKey; +import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; import org.apache.hadoop.hdds.security.symmetric.SecretKeySignerClient; import org.apache.hadoop.io.Text; import org.apache.hadoop.security.token.Token; @@ -70,7 +74,7 @@ public void setUp() throws Exception { final UUID keyId = UUID.fromString("00000000-0000-0000-0000-000000000000"); when(mockSecretKey.getId()).thenReturn(keyId); when(mockSecretKey.getSecretKey()).thenReturn(sharedSecretKey); - when(mockSecretKey.sign(any(STSTokenIdentifier.class))) + when(mockSecretKey.sign(any(byte[].class))) .thenReturn("mock-signature".getBytes(StandardCharsets.UTF_8)); when(mockSecretKeyClient.getCurrentSecretKey()).thenReturn(mockSecretKey); @@ -89,7 +93,9 @@ public void testCreateSTSTokenStringContainsCorrectFields() throws IOException { // Verify the token identifier fields final STSTokenIdentifier identifier = new STSTokenIdentifier(); - identifier.setEncryptionKey(sharedSecretKey.getEncoded()); + identifier.setManagedSecretKey(createManagedSecretKey( + UUID.fromString("00000000-0000-0000-0000-000000000000"), + sharedSecretKey.getEncoded(), Instant.now())); identifier.readFromByteArray(token.getIdentifier()); final Instant expiration = identifier.getExpiry(); @@ -98,6 +104,7 @@ public void testCreateSTSTokenStringContainsCorrectFields() throws IOException { assertEquals(ROLE_ARN, identifier.getRoleArn()); assertEquals(SECRET_ACCESS_KEY, identifier.getSecretAccessKey()); assertEquals(SESSION_POLICY, identifier.getSessionPolicy()); + assertEquals(clock.instant(), identifier.getCreationTime()); assertNotNull(identifier.getSecretKeyId()); assertEquals(new Text("STSToken"), identifier.getKind()); assertEquals("STS", identifier.getService()); @@ -114,8 +121,78 @@ public void testCreateSTSTokenStringWithNullSessionPolicy() throws IOException { token.decodeFromUrlString(tokenString); final STSTokenIdentifier identifier = new STSTokenIdentifier(); - identifier.setEncryptionKey(sharedSecretKey.getEncoded()); + identifier.setManagedSecretKey(createManagedSecretKey( + UUID.fromString("00000000-0000-0000-0000-000000000000"), + sharedSecretKey.getEncoded(), Instant.now())); identifier.readFromByteArray(token.getIdentifier()); assertTrue(identifier.getSessionPolicy().isEmpty()); } + + /** + * createSTSTokenString() must use a single getCurrentSecretKey() for encryption, secretKeyId, and signing. If a + * second fetch happened during signing, a key rotation between calls would encrypt with the old key but stamp the + * token with the new key id. + */ + @Test + public void testCreateSTSTokenStringValidatesWhenSecretKeyRotatesDuringCreation() throws Exception { + // ManagedSecretKey.isExpired() uses Instant.now(), not the test clock. + final Instant keyCreationTime = Instant.now(); + final ManagedSecretKey encryptionKey = createManagedSecretKey( + UUID.fromString("11111111-1111-1111-1111-111111111111"), + "encryption-key-material-012345678901".getBytes(StandardCharsets.US_ASCII), + keyCreationTime); + final ManagedSecretKey signingKey = createManagedSecretKey( + UUID.fromString("22222222-2222-2222-2222-222222222222"), + "signing-key-material-01234567890123".getBytes(StandardCharsets.US_ASCII), + keyCreationTime); + + final RotatingSecretKeyTestClient rotatingSecretKeyClient = new RotatingSecretKeyTestClient( + encryptionKey, signingKey); + final STSTokenSecretManager rotatingSecretManager = new STSTokenSecretManager(rotatingSecretKeyClient); + + final String tokenString = rotatingSecretManager.createSTSTokenString( + TEMP_ACCESS_KEY, ORIGINAL_ACCESS_KEY, ROLE_ARN, DURATION_SECONDS, SECRET_ACCESS_KEY, SESSION_POLICY, clock); + + final STSTokenIdentifier result = STSSecurityUtil.constructValidateAndDecryptSTSToken( + tokenString, rotatingSecretKeyClient, clock); + assertEquals(SECRET_ACCESS_KEY, result.getSecretAccessKey()); + assertEquals(encryptionKey.getId(), result.getSecretKeyId()); + assertEquals(1, rotatingSecretKeyClient.getCurrentSecretKeyCallCount()); + } + + private static ManagedSecretKey createManagedSecretKey(UUID id, byte[] keyBytes, Instant creationTime) { + final SecretKey secretKey = new SecretKeySpec(keyBytes, "HmacSHA256"); + return new ManagedSecretKey(id, creationTime, creationTime.plus(Duration.ofHours(1)), secretKey); + } + + /** + * Returns different current keys on consecutive getCurrentSecretKey() calls to simulate rotation. + */ + private static final class RotatingSecretKeyTestClient implements SecretKeyClient { + private final ManagedSecretKey firstKey; + private final ManagedSecretKey secondKey; + private final Map keysById = new HashMap<>(); + private int getCurrentSecretKeyCallCount; + + private RotatingSecretKeyTestClient(ManagedSecretKey firstKey, ManagedSecretKey secondKey) { + this.firstKey = firstKey; + this.secondKey = secondKey; + keysById.put(firstKey.getId(), firstKey); + keysById.put(secondKey.getId(), secondKey); + } + + @Override + public synchronized ManagedSecretKey getCurrentSecretKey() { + return getCurrentSecretKeyCallCount++ == 0 ? firstKey : secondKey; + } + + @Override + public ManagedSecretKey getSecretKey(UUID id) { + return keysById.get(id); + } + + private int getCurrentSecretKeyCallCount() { + return getCurrentSecretKeyCallCount; + } + } } 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 b79984fa93e7..abd80cbc1fcf 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 @@ -899,7 +899,7 @@ public AssumeRoleResponseInfo assumeRole(String roleArn, String roleSessionName, } @Override - public void revokeSTSToken(String sessionToken) throws IOException { + public void revokeSTSToken(String originalAccessKeyId) throws IOException { } @Override