From e7885b921a20f9505d7f8a3282433468c0842d98 Mon Sep 17 00:00:00 2001 From: David Turner Date: Wed, 26 Mar 2025 12:59:05 +0000 Subject: [PATCH 1/4] Validate region/service in `DynamicAwsCredentials` Following on from #125559, we can validate the region and service name in tests that use `DynamicAwsCredentials` too. --- .../s3/RepositoryS3EcsCredentialsRestIT.java | 2 +- .../RepositoryS3ImdsV1CredentialsRestIT.java | 2 +- .../RepositoryS3ImdsV2CredentialsRestIT.java | 9 +++- .../s3/RepositoryS3StsCredentialsRestIT.java | 2 +- .../ec2/DiscoveryEc2EcsCredentialsIT.java | 4 +- .../ec2/DiscoveryEc2InstanceProfileIT.java | 12 ++++-- .../java/fixture/aws/AwsCredentialsUtils.java | 41 +++++++++++-------- .../fixture/aws/DynamicAwsCredentials.java | 33 ++++++++++++++- 8 files changed, 78 insertions(+), 27 deletions(-) diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsCredentialsRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsCredentialsRestIT.java index c525c2ea42f58..66fcdc4ececf4 100644 --- a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsCredentialsRestIT.java +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsCredentialsRestIT.java @@ -35,7 +35,7 @@ public class RepositoryS3EcsCredentialsRestIT extends AbstractRepositoryS3RestTe private static final String BASE_PATH = PREFIX + "base_path"; private static final String CLIENT = "ecs_credentials_client"; - private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(); + private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials("*", "s3"); private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture( new Ec2ImdsServiceBuilder(Ec2ImdsVersion.V1).newCredentialsConsumer(dynamicCredentials::addValidCredentials) diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV1CredentialsRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV1CredentialsRestIT.java index bc689ea52ca32..29031da10665d 100644 --- a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV1CredentialsRestIT.java +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV1CredentialsRestIT.java @@ -33,7 +33,7 @@ public class RepositoryS3ImdsV1CredentialsRestIT extends AbstractRepositoryS3Res private static final String BASE_PATH = PREFIX + "base_path"; private static final String CLIENT = "imdsv1_credentials_client"; - private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(); + private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials("*", "s3"); private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture( new Ec2ImdsServiceBuilder(Ec2ImdsVersion.V1).newCredentialsConsumer(dynamicCredentials::addValidCredentials) diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java index dedf205d3def2..3683da207f0db 100644 --- a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java @@ -18,12 +18,16 @@ import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.elasticsearch.common.util.LazyInitializable; +import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.fixtures.testcontainers.TestContainersThreadFilter; import org.junit.ClassRule; import org.junit.rules.RuleChain; import org.junit.rules.TestRule; +import java.util.function.Supplier; + @ThreadLeakFilters(filters = { TestContainersThreadFilter.class }) @ThreadLeakScope(ThreadLeakScope.Scope.NONE) // https://github.com/elastic/elasticsearch/issues/102482 public class RepositoryS3ImdsV2CredentialsRestIT extends AbstractRepositoryS3RestTestCase { @@ -33,10 +37,13 @@ public class RepositoryS3ImdsV2CredentialsRestIT extends AbstractRepositoryS3Res private static final String BASE_PATH = PREFIX + "base_path"; private static final String CLIENT = "imdsv2_credentials_client"; - private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(); + private static final Supplier regionSupplier = new LazyInitializable<>(ESTestCase::randomIdentifier)::getOrCompute; + + private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(regionSupplier, "s3"); private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture( new Ec2ImdsServiceBuilder(Ec2ImdsVersion.V2).newCredentialsConsumer(dynamicCredentials::addValidCredentials) + .instanceIdentityDocument((b, p) -> b.field("region", regionSupplier.get())) ); private static final S3HttpFixture s3Fixture = new S3HttpFixture(true, BUCKET, BASE_PATH, dynamicCredentials::isAuthorized); diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsRestIT.java index 61f0acdb16154..a58645363b0e9 100644 --- a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsRestIT.java +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsRestIT.java @@ -32,7 +32,7 @@ public class RepositoryS3StsCredentialsRestIT extends AbstractRepositoryS3RestTe private static final String BASE_PATH = PREFIX + "base_path"; private static final String CLIENT = "sts_credentials_client"; - private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(); + private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials("*", "s3"); private static final S3HttpFixture s3HttpFixture = new S3HttpFixture(true, BUCKET, BASE_PATH, dynamicCredentials::isAuthorized); diff --git a/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2EcsCredentialsIT.java b/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2EcsCredentialsIT.java index 5c6368fe0db67..38d4dfac28ae4 100644 --- a/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2EcsCredentialsIT.java +++ b/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2EcsCredentialsIT.java @@ -26,12 +26,12 @@ public class DiscoveryEc2EcsCredentialsIT extends DiscoveryEc2ClusterFormationTestCase { - private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(); - private static final String PREFIX = getIdentifierPrefix("DiscoveryEc2EcsCredentialsIT"); private static final String REGION = PREFIX + "-region"; private static final String CREDENTIALS_ENDPOINT = "/ecs_credentials_endpoint_" + PREFIX; + private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(REGION, "ec2"); + private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture( new Ec2ImdsServiceBuilder(Ec2ImdsVersion.V1).newCredentialsConsumer(dynamicCredentials::addValidCredentials) .alternativeCredentialsEndpoints(Set.of(CREDENTIALS_ENDPOINT)) diff --git a/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java b/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java index 01eadac9364a8..95361e0d62724 100644 --- a/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java +++ b/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java @@ -15,22 +15,26 @@ import fixture.aws.imds.Ec2ImdsServiceBuilder; import fixture.aws.imds.Ec2ImdsVersion; +import org.elasticsearch.common.util.LazyInitializable; import org.elasticsearch.discovery.DiscoveryModule; +import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.junit.ClassRule; import org.junit.rules.RuleChain; import org.junit.rules.TestRule; import java.util.List; +import java.util.function.Supplier; public class DiscoveryEc2InstanceProfileIT extends DiscoveryEc2ClusterFormationTestCase { - private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(); + private static final Supplier regionSupplier = new LazyInitializable<>(ESTestCase::randomIdentifier)::getOrCompute; + + private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(regionSupplier, "ec2"); private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture( - new Ec2ImdsServiceBuilder(Ec2ImdsVersion.V2).instanceIdentityDocument( - (builder, params) -> builder.field("region", randomIdentifier()) - ).newCredentialsConsumer(dynamicCredentials::addValidCredentials) + new Ec2ImdsServiceBuilder(Ec2ImdsVersion.V2).instanceIdentityDocument((builder, params) -> builder.field("region", regionSupplier)) + .newCredentialsConsumer(dynamicCredentials::addValidCredentials) ); private static final AwsEc2HttpFixture ec2ApiFixture = new AwsEc2HttpFixture( diff --git a/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/AwsCredentialsUtils.java b/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/AwsCredentialsUtils.java index aab6504f2d8c3..b48398d0719b1 100644 --- a/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/AwsCredentialsUtils.java +++ b/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/AwsCredentialsUtils.java @@ -39,25 +39,34 @@ public static BiPredicate fixedAccessKey(String accessKey, Strin * @param region the name of the AWS region used to sign the request, or {@code *} to skip validation of the region parameter */ public static BiPredicate mutableAccessKey(Supplier accessKeySupplier, String region, String serviceName) { - return (authorizationHeader, sessionTokenHeader) -> { - if (authorizationHeader == null) { - return false; - } + return (authorizationHeader, sessionTokenHeader) -> authorizationHeader != null + && isValidAwsV4SignedAuthorizationHeader(accessKeySupplier.get(), region, serviceName, authorizationHeader); + } - final var accessKey = accessKeySupplier.get(); - final var expectedPrefix = "AWS4-HMAC-SHA256 Credential=" + accessKey + "/"; - if (authorizationHeader.startsWith(expectedPrefix) == false) { - return false; - } + /** + * @return whether the given value is a valid AWS-v4-signed authorization header that matches the given access key, region, and service + * name. + * @see AWS v4 Signatures + * @param region the name of the AWS region used to sign the request, or {@code *} to skip validation of the region parameter + */ + public static boolean isValidAwsV4SignedAuthorizationHeader( + String accessKey, + String region, + String serviceName, + String authorizationHeader + ) { + final var expectedPrefix = "AWS4-HMAC-SHA256 Credential=" + accessKey + "/"; + if (authorizationHeader.startsWith(expectedPrefix) == false) { + return false; + } - if (region.equals("*")) { - // skip region validation; TODO eliminate this when region is fixed in all tests - return authorizationHeader.contains("/" + serviceName + "/aws4_request, "); - } + if (region.equals("*")) { + // skip region validation; TODO eliminate this when region is fixed in all tests + return authorizationHeader.contains("/" + serviceName + "/aws4_request, "); + } - final var remainder = authorizationHeader.substring(expectedPrefix.length() + "YYYYMMDD".length() /* skip over date field */); - return remainder.startsWith("/" + region + "/" + serviceName + "/aws4_request, "); - }; + final var remainder = authorizationHeader.substring(expectedPrefix.length() + "YYYYMMDD".length() /* skip over date field */); + return remainder.startsWith("/" + region + "/" + serviceName + "/aws4_request, "); } /** diff --git a/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java b/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java index 24a5b4ea018e3..22f7a4c2d6779 100644 --- a/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java +++ b/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java @@ -14,6 +14,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Supplier; /** * Allows dynamic creation of access-key/session-token credentials for accessing AWS services such as S3. Typically there's one service @@ -21,12 +22,42 @@ * fixture uses {@link #isAuthorized} to validate the credentials it receives corresponds with some previously-generated credentials. */ public class DynamicAwsCredentials { + + private final Supplier expectedRegionSupplier; + private final String expectedServiceName; private final Map> validCredentialsMap = ConcurrentCollections.newConcurrentMap(); + /** + * @param expectedRegion The region to use for validating the authorization header, or {@code *} to skip this validation. + * @param expectedServiceName The service name that should appear in the authorization header. + */ + public DynamicAwsCredentials(String expectedRegion, String expectedServiceName) { + this(() -> expectedRegion, expectedServiceName); + } + + /** + * @param expectedRegionSupplier Supplies the region to use for validating the authorization header, or {@code *} to skip this + * validation. + * @param expectedServiceName The service name that should appear in the authorization header. + */ + public DynamicAwsCredentials(Supplier expectedRegionSupplier, String expectedServiceName) { + this.expectedRegionSupplier = expectedRegionSupplier; + this.expectedServiceName = expectedServiceName; + } + public boolean isAuthorized(String authorizationHeader, String sessionTokenHeader) { return authorizationHeader != null && sessionTokenHeader != null - && validCredentialsMap.getOrDefault(sessionTokenHeader, Set.of()).stream().anyMatch(authorizationHeader::contains); + && validCredentialsMap.getOrDefault(sessionTokenHeader, Set.of()) + .stream() + .anyMatch( + validAccessKey -> AwsCredentialsUtils.isValidAwsV4SignedAuthorizationHeader( + validAccessKey, + expectedRegionSupplier.get(), + expectedServiceName, + authorizationHeader + ) + ); } public void addValidCredentials(String accessKey, String sessionToken) { From 054161788b6e76f031f0a76ee0326de9913537c8 Mon Sep 17 00:00:00 2001 From: David Turner Date: Wed, 26 Mar 2025 14:57:34 +0000 Subject: [PATCH 2/4] V1 SDK doesn't use the region from IMDS --- .../s3/RepositoryS3ImdsV2CredentialsRestIT.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java index 3683da207f0db..d6fc86a0afe34 100644 --- a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java @@ -18,16 +18,12 @@ import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.elasticsearch.common.util.LazyInitializable; -import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.fixtures.testcontainers.TestContainersThreadFilter; import org.junit.ClassRule; import org.junit.rules.RuleChain; import org.junit.rules.TestRule; -import java.util.function.Supplier; - @ThreadLeakFilters(filters = { TestContainersThreadFilter.class }) @ThreadLeakScope(ThreadLeakScope.Scope.NONE) // https://github.com/elastic/elasticsearch/issues/102482 public class RepositoryS3ImdsV2CredentialsRestIT extends AbstractRepositoryS3RestTestCase { @@ -37,13 +33,10 @@ public class RepositoryS3ImdsV2CredentialsRestIT extends AbstractRepositoryS3Res private static final String BASE_PATH = PREFIX + "base_path"; private static final String CLIENT = "imdsv2_credentials_client"; - private static final Supplier regionSupplier = new LazyInitializable<>(ESTestCase::randomIdentifier)::getOrCompute; - - private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(regionSupplier, "s3"); + private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials("*", "s3"); private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture( new Ec2ImdsServiceBuilder(Ec2ImdsVersion.V2).newCredentialsConsumer(dynamicCredentials::addValidCredentials) - .instanceIdentityDocument((b, p) -> b.field("region", regionSupplier.get())) ); private static final S3HttpFixture s3Fixture = new S3HttpFixture(true, BUCKET, BASE_PATH, dynamicCredentials::isAuthorized); From 73cc0af9db456b65d73bd67e684742471fa681d6 Mon Sep 17 00:00:00 2001 From: David Turner Date: Wed, 26 Mar 2025 15:12:50 +0000 Subject: [PATCH 3/4] regionSupplier.get() --- .../discovery/ec2/DiscoveryEc2InstanceProfileIT.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java b/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java index 95361e0d62724..d6b7a5c5e31f1 100644 --- a/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java +++ b/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java @@ -33,8 +33,9 @@ public class DiscoveryEc2InstanceProfileIT extends DiscoveryEc2ClusterFormationT private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(regionSupplier, "ec2"); private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture( - new Ec2ImdsServiceBuilder(Ec2ImdsVersion.V2).instanceIdentityDocument((builder, params) -> builder.field("region", regionSupplier)) - .newCredentialsConsumer(dynamicCredentials::addValidCredentials) + new Ec2ImdsServiceBuilder(Ec2ImdsVersion.V2).instanceIdentityDocument( + (builder, params) -> builder.field("region", regionSupplier.get()) + ).newCredentialsConsumer(dynamicCredentials::addValidCredentials) ); private static final AwsEc2HttpFixture ec2ApiFixture = new AwsEc2HttpFixture( From d8892acb1a53315c9febaa67a32f87d45fb0cf10 Mon Sep 17 00:00:00 2001 From: David Turner Date: Wed, 26 Mar 2025 21:59:36 +0000 Subject: [PATCH 4/4] Comments --- .../ec2/DiscoveryEc2InstanceProfileIT.java | 1 + .../java/fixture/aws/DynamicAwsCredentials.java | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java b/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java index d6b7a5c5e31f1..78c3ca01e359c 100644 --- a/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java +++ b/plugins/discovery-ec2/src/javaRestTest/java/org/elasticsearch/discovery/ec2/DiscoveryEc2InstanceProfileIT.java @@ -28,6 +28,7 @@ public class DiscoveryEc2InstanceProfileIT extends DiscoveryEc2ClusterFormationTestCase { + // Lazy-initialized so we can generate it randomly, which is not possible in static context. private static final Supplier regionSupplier = new LazyInitializable<>(ESTestCase::randomIdentifier)::getOrCompute; private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(regionSupplier, "ec2"); diff --git a/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java b/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java index 22f7a4c2d6779..04c50607681a6 100644 --- a/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java +++ b/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java @@ -23,8 +23,21 @@ */ public class DynamicAwsCredentials { + /** + * Extra validation that requests are signed using the correct region. Lazy so it can be randomly generated after initialization, since + * randomness is not available in static context. + */ private final Supplier expectedRegionSupplier; + + /** + * Extra validation that requests are directed to the correct service. + */ private final String expectedServiceName; + + /** + * The set of access keys for each session token registered with {@link #addValidCredentials}. It's this way round because the session + * token is a separate header so it's easier to extract. + */ private final Map> validCredentialsMap = ConcurrentCollections.newConcurrentMap(); /**