Skip to content

Commit 80636b1

Browse files
committed
Validate AWS signer region and service in tests
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 f461731 commit 80636b1

File tree

12 files changed

+50
-19
lines changed

12 files changed

+50
-19
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: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,10 @@ 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(() -> {
473+
// WIP
474+
clientReference.client().putObject(putRequest);
475+
});
473476
} catch (final AmazonClientException e) {
474477
throw new IOException("Unable to upload object [" + blobName + "] using a single upload", e);
475478
}

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: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,24 +26,42 @@ public enum AwsCredentialsUtils {
2626
/**
2727
* @return an authorization predicate that ensures the access key matches the given values.
2828
*/
29-
public static BiPredicate<String, String> fixedAccessKey(String accessKey) {
30-
return mutableAccessKey(() -> accessKey);
29+
public static BiPredicate<String, String> fixedAccessKey(String accessKey, String region, String service) {
30+
return mutableAccessKey(() -> accessKey, region, service);
3131
}
3232

3333
/**
34-
* @return an authorization predicate that ensures the access key matches one supplied by the given supplier.
34+
* @return an authorization predicate that ensures the authorization header matches the access key supplied by the given supplier,
35+
* and the given region and service name.
3536
*/
36-
public static BiPredicate<String, String> mutableAccessKey(Supplier<String> accessKeySupplier) {
37-
return (authorizationHeader, sessionTokenHeader) -> authorizationHeader != null
38-
&& authorizationHeader.contains(accessKeySupplier.get());
37+
public static BiPredicate<String, String> mutableAccessKey(Supplier<String> accessKeySupplier, String region, String service) {
38+
return (authorizationHeader, sessionTokenHeader) -> {
39+
if (authorizationHeader == null) {
40+
return false;
41+
}
42+
43+
final var accessKey = accessKeySupplier.get();
44+
final var expectedPrefix = "AWS4-HMAC-SHA256 Credential=" + accessKey + "/";
45+
if (authorizationHeader.startsWith(expectedPrefix) == false) {
46+
return false;
47+
}
48+
49+
if (region.equals("*")) {
50+
// skip region validation; TODO eliminate this when region is fixed in all tests
51+
return authorizationHeader.contains("/" + service + "/aws4_request, ");
52+
}
53+
54+
final var remainder = authorizationHeader.substring(expectedPrefix.length() + 8 /* YYYYMMDD not validated */);
55+
return remainder.startsWith("/" + region + "/" + service + "/aws4_request, ");
56+
};
3957
}
4058

4159
/**
4260
* @return an authorization predicate that ensures the access key and session token both match the given values.
4361
*/
44-
public static BiPredicate<String, String> fixedAccessKeyAndToken(String accessKey, String sessionToken) {
62+
public static BiPredicate<String, String> fixedAccessKeyAndToken(String accessKey, String sessionToken, String region, String service) {
4563
Objects.requireNonNull(sessionToken);
46-
final var accessKeyPredicate = fixedAccessKey(accessKey);
64+
final var accessKeyPredicate = fixedAccessKey(accessKey, region, service);
4765
return (authorizationHeader, sessionTokenHeader) -> accessKeyPredicate.test(authorizationHeader, sessionTokenHeader)
4866
&& sessionToken.equals(sessionTokenHeader);
4967
}

0 commit comments

Comments
 (0)