Browse Source

Add IMDSv2 support to `repository-s3` (#117748) (#117817)

The version of the AWS Java SDK we use already magically switches to
IMDSv2 if available, but today we cannot claim to support IMDSv2 in
Elasticsearch since we have no tests demonstrating that the magic really
works for us. In particular, this sort of thing often risks falling foul
of some restrictions imposed by the security manager (if not now then
maybe in some future release).

This commit adds proper support for IMDSv2 by enhancing the test suite
to add the missing coverage to avoid any risk of breaking this magical
SDK behaviour in future.

Closes #105135 Closes ES-9984
David Turner 10 months ago
parent
commit
4b7c3b6e43

+ 6 - 0
docs/changelog/117748.yaml

@@ -0,0 +1,6 @@
+pr: 117748
+summary: Add IMDSv2 support to `repository-s3`
+area: Snapshot/Restore
+type: enhancement
+issues:
+ - 105135

+ 23 - 19
docs/reference/snapshot-restore/repository-s3.asciidoc

@@ -38,7 +38,8 @@ PUT _snapshot/my_s3_repository
 The client that you use to connect to S3 has a number of settings available.
 The settings have the form `s3.client.CLIENT_NAME.SETTING_NAME`. By default,
 `s3` repositories use a client named `default`, but this can be modified using
-the <<repository-s3-repository,repository setting>> `client`. For example:
+the <<repository-s3-repository,repository setting>> `client`. For example, to
+use a client named `my-alternate-client`, register the repository as follows:
 
 [source,console]
 ----
@@ -69,10 +70,19 @@ bin/elasticsearch-keystore add s3.client.default.secret_key
 bin/elasticsearch-keystore add s3.client.default.session_token
 ----
 
-If instead you want to use the instance role or container role to access S3
-then you should leave these settings unset. You can switch from using specific
-credentials back to the default of using the instance role or container role by
-removing these settings from the keystore as follows:
+If you do not configure these settings then {es} will attempt to automatically
+obtain credentials from the environment in which it is running:
+
+* Nodes running on an instance in AWS EC2 will attempt to use the EC2 Instance
+  Metadata Service (IMDS) to obtain instance role credentials. {es} supports
+  both IMDS version 1 and IMDS version 2.
+
+* Nodes running in a container in AWS ECS and AWS EKS will attempt to obtain
+  container role credentials similarly.
+
+You can switch from using specific credentials back to the default of using the
+instance role or container role by removing these settings from the keystore as
+follows:
 
 [source,sh]
 ----
@@ -82,20 +92,14 @@ bin/elasticsearch-keystore remove s3.client.default.secret_key
 bin/elasticsearch-keystore remove s3.client.default.session_token
 ----
 
-*All* client secure settings of this repository type are
-{ref}/secure-settings.html#reloadable-secure-settings[reloadable].
-You can define these settings before the node is started,
-or call the <<cluster-nodes-reload-secure-settings,Nodes reload secure settings API>>
-after the settings are defined to apply them to a running node.
-
-After you reload the settings, the internal `s3` clients, used to transfer the snapshot
-contents, will utilize the latest settings from the keystore. Any existing `s3`
-repositories, as well as any newly created ones, will pick up the new values
-stored in the keystore.
-
-NOTE: In-progress snapshot/restore tasks will not be preempted by a *reload* of
-the client's secure settings. The task will complete using the client as it was
-built when the operation started.
+Define the relevant secure settings in each node's keystore before starting the
+node. The secure settings described here are all
+{ref}/secure-settings.html#reloadable-secure-settings[reloadable] so you may
+update the keystore contents on each node while the node is running and then
+call the <<cluster-nodes-reload-secure-settings,Nodes reload secure settings
+API>> to apply the updated settings to the nodes in the cluster. After this API
+completes, {es} will use the updated setting values for all future snapshot
+operations, but ongoing operations may continue to use older setting values.
 
 The following list contains the available client settings. Those that must be
 stored in the keystore are marked as "secure" and are *reloadable*; the other

+ 2 - 0
modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsCredentialsRestIT.java

@@ -10,6 +10,7 @@
 package org.elasticsearch.repositories.s3;
 
 import fixture.aws.imds.Ec2ImdsHttpFixture;
+import fixture.aws.imds.Ec2ImdsVersion;
 import fixture.s3.DynamicS3Credentials;
 import fixture.s3.S3HttpFixture;
 
@@ -36,6 +37,7 @@ public class RepositoryS3EcsCredentialsRestIT extends AbstractRepositoryS3RestTe
     private static final DynamicS3Credentials dynamicS3Credentials = new DynamicS3Credentials();
 
     private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture(
+        Ec2ImdsVersion.V1,
         dynamicS3Credentials::addValidCredentials,
         Set.of("/ecs_credentials_endpoint")
     );

+ 2 - 0
modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV1CredentialsRestIT.java

@@ -10,6 +10,7 @@
 package org.elasticsearch.repositories.s3;
 
 import fixture.aws.imds.Ec2ImdsHttpFixture;
+import fixture.aws.imds.Ec2ImdsVersion;
 import fixture.s3.DynamicS3Credentials;
 import fixture.s3.S3HttpFixture;
 
@@ -36,6 +37,7 @@ public class RepositoryS3ImdsV1CredentialsRestIT extends AbstractRepositoryS3Res
     private static final DynamicS3Credentials dynamicS3Credentials = new DynamicS3Credentials();
 
     private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture(
+        Ec2ImdsVersion.V1,
         dynamicS3Credentials::addValidCredentials,
         Set.of()
     );

+ 75 - 0
modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.repositories.s3;
+
+import fixture.aws.imds.Ec2ImdsHttpFixture;
+import fixture.aws.imds.Ec2ImdsVersion;
+import fixture.s3.DynamicS3Credentials;
+import fixture.s3.S3HttpFixture;
+
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+
+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.Set;
+
+@ThreadLeakFilters(filters = { TestContainersThreadFilter.class })
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE) // https://github.com/elastic/elasticsearch/issues/102482
+public class RepositoryS3ImdsV2CredentialsRestIT extends AbstractRepositoryS3RestTestCase {
+
+    private static final String PREFIX = getIdentifierPrefix("RepositoryS3ImdsV2CredentialsRestIT");
+    private static final String BUCKET = PREFIX + "bucket";
+    private static final String BASE_PATH = PREFIX + "base_path";
+    private static final String CLIENT = "imdsv2_credentials_client";
+
+    private static final DynamicS3Credentials dynamicS3Credentials = new DynamicS3Credentials();
+
+    private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture(
+        Ec2ImdsVersion.V2,
+        dynamicS3Credentials::addValidCredentials,
+        Set.of()
+    );
+
+    private static final S3HttpFixture s3Fixture = new S3HttpFixture(true, BUCKET, BASE_PATH, dynamicS3Credentials::isAuthorized);
+
+    public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
+        .module("repository-s3")
+        .setting("s3.client." + CLIENT + ".endpoint", s3Fixture::getAddress)
+        .systemProperty("com.amazonaws.sdk.ec2MetadataServiceEndpointOverride", ec2ImdsHttpFixture::getAddress)
+        .build();
+
+    @ClassRule
+    public static TestRule ruleChain = RuleChain.outerRule(ec2ImdsHttpFixture).around(s3Fixture).around(cluster);
+
+    @Override
+    protected String getTestRestCluster() {
+        return cluster.getHttpAddresses();
+    }
+
+    @Override
+    protected String getBucketName() {
+        return BUCKET;
+    }
+
+    @Override
+    protected String getBasePath() {
+        return BASE_PATH;
+    }
+
+    @Override
+    protected String getClientName() {
+        return CLIENT;
+    }
+}

+ 8 - 2
test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsHttpFixture.java

@@ -24,16 +24,22 @@ public class Ec2ImdsHttpFixture extends ExternalResource {
 
     private HttpServer server;
 
+    private final Ec2ImdsVersion ec2ImdsVersion;
     private final BiConsumer<String, String> newCredentialsConsumer;
     private final Set<String> alternativeCredentialsEndpoints;
 
-    public Ec2ImdsHttpFixture(BiConsumer<String, String> newCredentialsConsumer, Set<String> alternativeCredentialsEndpoints) {
+    public Ec2ImdsHttpFixture(
+        Ec2ImdsVersion ec2ImdsVersion,
+        BiConsumer<String, String> newCredentialsConsumer,
+        Set<String> alternativeCredentialsEndpoints
+    ) {
+        this.ec2ImdsVersion = Objects.requireNonNull(ec2ImdsVersion);
         this.newCredentialsConsumer = Objects.requireNonNull(newCredentialsConsumer);
         this.alternativeCredentialsEndpoints = Objects.requireNonNull(alternativeCredentialsEndpoints);
     }
 
     protected HttpHandler createHandler() {
-        return new Ec2ImdsHttpHandler(newCredentialsConsumer, alternativeCredentialsEndpoints);
+        return new Ec2ImdsHttpHandler(ec2ImdsVersion, newCredentialsConsumer, alternativeCredentialsEndpoints);
     }
 
     public String getAddress() {

+ 32 - 3
test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsHttpHandler.java

@@ -38,10 +38,18 @@ public class Ec2ImdsHttpHandler implements HttpHandler {
 
     private static final String IMDS_SECURITY_CREDENTIALS_PATH = "/latest/meta-data/iam/security-credentials/";
 
+    private final Ec2ImdsVersion ec2ImdsVersion;
+    private final Set<String> validImdsTokens = ConcurrentCollections.newConcurrentSet();
+
     private final BiConsumer<String, String> newCredentialsConsumer;
     private final Set<String> validCredentialsEndpoints = ConcurrentCollections.newConcurrentSet();
 
-    public Ec2ImdsHttpHandler(BiConsumer<String, String> newCredentialsConsumer, Collection<String> alternativeCredentialsEndpoints) {
+    public Ec2ImdsHttpHandler(
+        Ec2ImdsVersion ec2ImdsVersion,
+        BiConsumer<String, String> newCredentialsConsumer,
+        Collection<String> alternativeCredentialsEndpoints
+    ) {
+        this.ec2ImdsVersion = Objects.requireNonNull(ec2ImdsVersion);
         this.newCredentialsConsumer = Objects.requireNonNull(newCredentialsConsumer);
         this.validCredentialsEndpoints.addAll(alternativeCredentialsEndpoints);
     }
@@ -55,11 +63,32 @@ public class Ec2ImdsHttpHandler implements HttpHandler {
             final var requestMethod = exchange.getRequestMethod();
 
             if ("PUT".equals(requestMethod) && "/latest/api/token".equals(path)) {
-                // Reject IMDSv2 probe
-                exchange.sendResponseHeaders(RestStatus.METHOD_NOT_ALLOWED.getStatus(), -1);
+                switch (ec2ImdsVersion) {
+                    case V1 -> exchange.sendResponseHeaders(RestStatus.METHOD_NOT_ALLOWED.getStatus(), -1);
+                    case V2 -> {
+                        final var token = randomSecretKey();
+                        validImdsTokens.add(token);
+                        final var responseBody = token.getBytes(StandardCharsets.UTF_8);
+                        exchange.getResponseHeaders().add("Content-Type", "text/plain");
+                        exchange.sendResponseHeaders(RestStatus.OK.getStatus(), responseBody.length);
+                        exchange.getResponseBody().write(responseBody);
+                    }
+                }
                 return;
             }
 
+            if (ec2ImdsVersion == Ec2ImdsVersion.V2) {
+                final var token = exchange.getRequestHeaders().getFirst("X-aws-ec2-metadata-token");
+                if (token == null) {
+                    exchange.sendResponseHeaders(RestStatus.UNAUTHORIZED.getStatus(), -1);
+                    return;
+                }
+                if (validImdsTokens.contains(token) == false) {
+                    exchange.sendResponseHeaders(RestStatus.FORBIDDEN.getStatus(), -1);
+                    return;
+                }
+            }
+
             if ("GET".equals(requestMethod)) {
                 if (path.equals(IMDS_SECURITY_CREDENTIALS_PATH)) {
                     final var profileName = randomIdentifier();

+ 26 - 0
test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsVersion.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package fixture.aws.imds;
+
+/**
+ * Represents the IMDS protocol version simulated by the {@link Ec2ImdsHttpHandler}.
+ */
+public enum Ec2ImdsVersion {
+    /**
+     * Classic V1 behavior: plain {@code GET} requests, no tokens.
+     */
+    V1,
+
+    /**
+     * Newer V2 behavior: {@code GET} requests must include a {@code X-aws-ec2-metadata-token} header providing a token previously obtained
+     * by calling {@code PUT /latest/api/token}.
+     */
+    V2
+}

+ 62 - 5
test/fixtures/ec2-imds-fixture/src/test/java/fixture/aws/imds/Ec2ImdsHttpHandlerTests.java

@@ -19,6 +19,7 @@ import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.xcontent.XContentHelper;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xcontent.XContentType;
@@ -29,6 +30,7 @@ import java.io.OutputStream;
 import java.net.InetSocketAddress;
 import java.net.URI;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -36,17 +38,19 @@ import static org.hamcrest.Matchers.aMapWithSize;
 
 public class Ec2ImdsHttpHandlerTests extends ESTestCase {
 
+    private static final String SECURITY_CREDENTIALS_URI = "/latest/meta-data/iam/security-credentials/";
+
     public void testImdsV1() throws IOException {
         final Map<String, String> generatedCredentials = new HashMap<>();
 
-        final var handler = new Ec2ImdsHttpHandler(generatedCredentials::put, Set.of());
+        final var handler = new Ec2ImdsHttpHandler(Ec2ImdsVersion.V1, generatedCredentials::put, Set.of());
 
-        final var roleResponse = handleRequest(handler, "GET", "/latest/meta-data/iam/security-credentials/");
+        final var roleResponse = handleRequest(handler, "GET", SECURITY_CREDENTIALS_URI);
         assertEquals(RestStatus.OK, roleResponse.status());
         final var profileName = roleResponse.body().utf8ToString();
         assertTrue(Strings.hasText(profileName));
 
-        final var credentialsResponse = handleRequest(handler, "GET", "/latest/meta-data/iam/security-credentials/" + profileName);
+        final var credentialsResponse = handleRequest(handler, "GET", SECURITY_CREDENTIALS_URI + profileName);
         assertEquals(RestStatus.OK, credentialsResponse.status());
 
         assertThat(generatedCredentials, aMapWithSize(1));
@@ -62,14 +66,67 @@ public class Ec2ImdsHttpHandlerTests extends ESTestCase {
     public void testImdsV2Disabled() {
         assertEquals(
             RestStatus.METHOD_NOT_ALLOWED,
-            handleRequest(new Ec2ImdsHttpHandler((accessKey, sessionToken) -> fail(), Set.of()), "PUT", "/latest/api/token").status()
+            handleRequest(
+                new Ec2ImdsHttpHandler(Ec2ImdsVersion.V1, (accessKey, sessionToken) -> fail(), Set.of()),
+                "PUT",
+                "/latest/api/token"
+            ).status()
         );
     }
 
+    public void testImdsV2() throws IOException {
+        final Map<String, String> generatedCredentials = new HashMap<>();
+
+        final var handler = new Ec2ImdsHttpHandler(Ec2ImdsVersion.V2, generatedCredentials::put, Set.of());
+
+        final var tokenResponse = handleRequest(handler, "PUT", "/latest/api/token");
+        assertEquals(RestStatus.OK, tokenResponse.status());
+        final var token = tokenResponse.body().utf8ToString();
+
+        final var roleResponse = checkImdsV2GetRequest(handler, SECURITY_CREDENTIALS_URI, token);
+        assertEquals(RestStatus.OK, roleResponse.status());
+        final var profileName = roleResponse.body().utf8ToString();
+        assertTrue(Strings.hasText(profileName));
+
+        final var credentialsResponse = checkImdsV2GetRequest(handler, SECURITY_CREDENTIALS_URI + profileName, token);
+        assertEquals(RestStatus.OK, credentialsResponse.status());
+
+        assertThat(generatedCredentials, aMapWithSize(1));
+        final var accessKey = generatedCredentials.keySet().iterator().next();
+        final var sessionToken = generatedCredentials.values().iterator().next();
+
+        final var responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), credentialsResponse.body().streamInput(), false);
+        assertEquals(Set.of("AccessKeyId", "Expiration", "RoleArn", "SecretAccessKey", "Token"), responseMap.keySet());
+        assertEquals(accessKey, responseMap.get("AccessKeyId"));
+        assertEquals(sessionToken, responseMap.get("Token"));
+    }
+
     private record TestHttpResponse(RestStatus status, BytesReference body) {}
 
+    private static TestHttpResponse checkImdsV2GetRequest(Ec2ImdsHttpHandler handler, String uri, String token) {
+        final var unauthorizedResponse = handleRequest(handler, "GET", uri, null);
+        assertEquals(RestStatus.UNAUTHORIZED, unauthorizedResponse.status());
+
+        final var forbiddenResponse = handleRequest(handler, "GET", uri, randomValueOtherThan(token, ESTestCase::randomSecretKey));
+        assertEquals(RestStatus.FORBIDDEN, forbiddenResponse.status());
+
+        return handleRequest(handler, "GET", uri, token);
+    }
+
     private static TestHttpResponse handleRequest(Ec2ImdsHttpHandler handler, String method, String uri) {
-        final var httpExchange = new TestHttpExchange(method, uri, BytesArray.EMPTY, TestHttpExchange.EMPTY_HEADERS);
+        return handleRequest(handler, method, uri, null);
+    }
+
+    private static TestHttpResponse handleRequest(Ec2ImdsHttpHandler handler, String method, String uri, @Nullable String token) {
+        final Headers headers;
+        if (token == null) {
+            headers = TestHttpExchange.EMPTY_HEADERS;
+        } else {
+            headers = new Headers();
+            headers.put("X-aws-ec2-metadata-token", List.of(token));
+        }
+
+        final var httpExchange = new TestHttpExchange(method, uri, BytesArray.EMPTY, headers);
         try {
             handler.handle(httpExchange);
         } catch (IOException e) {