Skip to content

Commit 4b7534b

Browse files
DaveCTurneromricohenn
authored andcommitted
Validate AWS signer region and service in tests (elastic#125559)
Extends the predicate in `AwsCredentialsUtils` to verify that we are using a proper AWS v4 signature complete with the correct region and service, rather than just looking for the access key as a substring.
1 parent 933aa1b commit 4b7534b

File tree

12 files changed

+60
-21
lines changed

12 files changed

+60
-21
lines changed

modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3BasicCredentialsRestIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public class RepositoryS3BasicCredentialsRestIT extends AbstractRepositoryS3Rest
3333
private static final String SECRET_KEY = PREFIX + "secret-key";
3434
private static final String CLIENT = "basic_credentials_client";
3535

36-
private static final S3HttpFixture s3Fixture = new S3HttpFixture(true, BUCKET, BASE_PATH, fixedAccessKey(ACCESS_KEY));
36+
private static final S3HttpFixture s3Fixture = new S3HttpFixture(true, BUCKET, BASE_PATH, fixedAccessKey(ACCESS_KEY, "*", "s3"));
3737

3838
public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
3939
.module("repository-s3")

modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3RestReloadCredentialsIT.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,12 @@ public class RepositoryS3RestReloadCredentialsIT extends ESRestTestCase {
3939

4040
private static volatile String repositoryAccessKey;
4141

42-
public static final S3HttpFixture s3Fixture = new S3HttpFixture(true, BUCKET, BASE_PATH, mutableAccessKey(() -> repositoryAccessKey));
42+
public static final S3HttpFixture s3Fixture = new S3HttpFixture(
43+
true,
44+
BUCKET,
45+
BASE_PATH,
46+
mutableAccessKey(() -> repositoryAccessKey, "*", "s3")
47+
);
4348

4449
private static final MutableSettingsProvider keystoreSettings = new MutableSettingsProvider();
4550

modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3SessionCredentialsRestIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public class RepositoryS3SessionCredentialsRestIT extends AbstractRepositoryS3Re
3838
true,
3939
BUCKET,
4040
BASE_PATH,
41-
fixedAccessKeyAndToken(ACCESS_KEY, SESSION_TOKEN)
41+
fixedAccessKeyAndToken(ACCESS_KEY, SESSION_TOKEN, "*", "s3")
4242
);
4343

4444
public static ElasticsearchCluster cluster = ElasticsearchCluster.local()

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ void executeSingleUpload(
469469
S3BlobStore.configureRequestForMetrics(putRequest, blobStore, Operation.PUT_OBJECT, purpose);
470470

471471
try (AmazonS3Reference clientReference = s3BlobStore.clientReference()) {
472-
SocketAccess.doPrivilegedVoid(() -> { clientReference.client().putObject(putRequest); });
472+
SocketAccess.doPrivilegedVoid(() -> clientReference.client().putObject(putRequest));
473473
} catch (final AmazonClientException e) {
474474
throw new IOException("Unable to upload object [" + blobName + "] using a single upload", e);
475475
}

modules/repository-s3/src/yamlRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ClientYamlTestSuiteIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public class RepositoryS3ClientYamlTestSuiteIT extends AbstractRepositoryS3Clien
3636
true,
3737
"bucket",
3838
"base_path_integration_tests",
39-
fixedAccessKey(ACCESS_KEY)
39+
fixedAccessKey(ACCESS_KEY, "*", "s3")
4040
);
4141

4242
public static ElasticsearchCluster cluster = ElasticsearchCluster.local()

plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2EnvironmentVariableCredentialsIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public class DiscoveryEc2EnvironmentVariableCredentialsIT extends DiscoveryEc2Cl
2929
private static final String ACCESS_KEY = PREFIX + "-access-key";
3030

3131
private static final AwsEc2HttpFixture ec2ApiFixture = new AwsEc2HttpFixture(
32-
fixedAccessKey(ACCESS_KEY),
32+
fixedAccessKey(ACCESS_KEY, REGION, "ec2"),
3333
DiscoveryEc2EnvironmentVariableCredentialsIT::getAvailableTransportEndpoints
3434
);
3535

plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2KeystoreCredentialsIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public class DiscoveryEc2KeystoreCredentialsIT extends DiscoveryEc2ClusterFormat
2929
private static final String ACCESS_KEY = PREFIX + "-access-key";
3030

3131
private static final AwsEc2HttpFixture ec2ApiFixture = new AwsEc2HttpFixture(
32-
fixedAccessKey(ACCESS_KEY),
32+
fixedAccessKey(ACCESS_KEY, REGION, "ec2"),
3333
DiscoveryEc2KeystoreCredentialsIT::getAvailableTransportEndpoints
3434
);
3535

plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2KeystoreSessionCredentialsIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public class DiscoveryEc2KeystoreSessionCredentialsIT extends DiscoveryEc2Cluste
3030
private static final String SESSION_TOKEN = PREFIX + "-session-token";
3131

3232
private static final AwsEc2HttpFixture ec2ApiFixture = new AwsEc2HttpFixture(
33-
fixedAccessKeyAndToken(ACCESS_KEY, SESSION_TOKEN),
33+
fixedAccessKeyAndToken(ACCESS_KEY, SESSION_TOKEN, REGION, "ec2"),
3434
DiscoveryEc2KeystoreSessionCredentialsIT::getAvailableTransportEndpoints
3535
);
3636

plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2SystemPropertyCredentialsIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public class DiscoveryEc2SystemPropertyCredentialsIT extends DiscoveryEc2Cluster
2929
private static final String ACCESS_KEY = PREFIX + "-access-key";
3030

3131
private static final AwsEc2HttpFixture ec2ApiFixture = new AwsEc2HttpFixture(
32-
fixedAccessKey(ACCESS_KEY),
32+
fixedAccessKey(ACCESS_KEY, REGION, "ec2"),
3333
DiscoveryEc2SystemPropertyCredentialsIT::getAvailableTransportEndpoints
3434
);
3535

test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/AwsCredentialsUtils.java

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,26 +24,55 @@ public enum AwsCredentialsUtils {
2424
;
2525

2626
/**
27-
* @return an authorization predicate that ensures the access key matches the given values.
27+
* @return an authorization predicate that ensures the authorization header matches the given access key, region and service name.
28+
* @see <a href="https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-auth-using-authorization-header.html">AWS v4 Signatures</a>
29+
* @param region the name of the AWS region used to sign the request, or {@code *} to skip validation of the region parameter
2830
*/
29-
public static BiPredicate<String, String> fixedAccessKey(String accessKey) {
30-
return mutableAccessKey(() -> accessKey);
31+
public static BiPredicate<String, String> fixedAccessKey(String accessKey, String region, String serviceName) {
32+
return mutableAccessKey(() -> accessKey, region, serviceName);
3133
}
3234

3335
/**
34-
* @return an authorization predicate that ensures the access key matches one supplied by the given supplier.
36+
* @return an authorization predicate that ensures the authorization header matches the access key supplied by the given supplier,
37+
* and also matches the given region and service name.
38+
* @see <a href="https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-auth-using-authorization-header.html">AWS v4 Signatures</a>
39+
* @param region the name of the AWS region used to sign the request, or {@code *} to skip validation of the region parameter
3540
*/
36-
public static BiPredicate<String, String> mutableAccessKey(Supplier<String> accessKeySupplier) {
37-
return (authorizationHeader, sessionTokenHeader) -> authorizationHeader != null
38-
&& authorizationHeader.contains(accessKeySupplier.get());
41+
public static BiPredicate<String, String> mutableAccessKey(Supplier<String> accessKeySupplier, String region, String serviceName) {
42+
return (authorizationHeader, sessionTokenHeader) -> {
43+
if (authorizationHeader == null) {
44+
return false;
45+
}
46+
47+
final var accessKey = accessKeySupplier.get();
48+
final var expectedPrefix = "AWS4-HMAC-SHA256 Credential=" + accessKey + "/";
49+
if (authorizationHeader.startsWith(expectedPrefix) == false) {
50+
return false;
51+
}
52+
53+
if (region.equals("*")) {
54+
// skip region validation; TODO eliminate this when region is fixed in all tests
55+
return authorizationHeader.contains("/" + serviceName + "/aws4_request, ");
56+
}
57+
58+
final var remainder = authorizationHeader.substring(expectedPrefix.length() + "YYYYMMDD".length() /* skip over date field */);
59+
return remainder.startsWith("/" + region + "/" + serviceName + "/aws4_request, ");
60+
};
3961
}
4062

4163
/**
42-
* @return an authorization predicate that ensures the access key and session token both match the given values.
64+
* @return an authorization predicate that ensures the access key, session token, region and service name all match the given values.
65+
* @see <a href="https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-auth-using-authorization-header.html">AWS v4 Signatures</a>
66+
* @param region the name of the AWS region used to sign the request, or {@code *} to skip validation of the region parameter
4367
*/
44-
public static BiPredicate<String, String> fixedAccessKeyAndToken(String accessKey, String sessionToken) {
68+
public static BiPredicate<String, String> fixedAccessKeyAndToken(
69+
String accessKey,
70+
String sessionToken,
71+
String region,
72+
String serviceName
73+
) {
4574
Objects.requireNonNull(sessionToken);
46-
final var accessKeyPredicate = fixedAccessKey(accessKey);
75+
final var accessKeyPredicate = fixedAccessKey(accessKey, region, serviceName);
4776
return (authorizationHeader, sessionTokenHeader) -> accessKeyPredicate.test(authorizationHeader, sessionTokenHeader)
4877
&& sessionToken.equals(sessionTokenHeader);
4978
}

0 commit comments

Comments
 (0)