Make ES files inside Docker container world readable (#64274)

Running the Elasticsearch Docker image with a different GID is
possible but trappy, since at present all the ES files are only
readable by the user and group. This PR documents a Docker CLI flag
that fixes this situation, by ensuring the container user is added
to the default group (which is `root`, GID 0).

I also added a test for this case, and refactored the Docker tests
to use a builder pattern for constructing the `docker run` command.
The existing code was becoming unwieldy and hard to change.

distribution/docker/src/docker/Dockerfile

@@ -274,10 +274,13 @@ COPY bin/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
 # 2. Sync the user and group permissions of /etc/passwd
 # 3. Set correct permissions of the entrypoint
 # 4. Ensure that there are no files with setuid or setgid, in order to mitigate "stackclash" attacks.
-RUN find /usr/share/elasticsearch/jdk -type d -exec chmod 0755 '{}' \\; && \\
+# 5. Ensure all files are world-readable by default. It should be possible to
+#    examine the contents of the image under any UID:GID
+RUN find /usr/share/elasticsearch/jdk -type d -exec chmod 0755 {} + && \\
     chmod g=u /etc/passwd && \\
     chmod 0775 /usr/local/bin/docker-entrypoint.sh && \\
-    find / -xdev -perm -4000 -exec chmod ug-s {} +
+    find / -xdev -perm -4000 -exec chmod ug-s {} + && \\
+    find /usr/share/elasticsearch -type f -exec chmod o+r {} +
 
 EXPOSE 9200 9300
 

docs/reference/setup/install/docker.asciidoc

@@ -222,7 +222,8 @@ which runs containers using an arbitrarily assigned user ID.
 Openshift presents persistent volumes with the gid set to `0`, which works without any adjustments.
 
 If you are bind-mounting a local directory or file, it must be readable by the `elasticsearch` user.
-In addition, this user must have write access to the <<path-settings,data and log dirs>>.
+In addition, this user must have write access to the <<path-settings,config, data and log dirs>>
+({es} needs write access to the `config` directory so that it can generate a keystore).
 A good strategy is to grant group access to gid `0` for the local directory.
 
 For example, to prepare a local directory for storing data through a bind-mount:
@@ -234,6 +235,12 @@ chmod g+rwx esdatadir
 chgrp 0 esdatadir
 --------------------------------------------
 
+You can also run an {es} container using both a custom UID and GID. Unless you
+bind-mount each of the `config`, data` and `logs` directories, you must pass
+the command line option `--group-add 0` to `docker run`. This ensures that the user
+under which {es} is running is also a member of the `root` (GID 0) group inside the
+container.
+
 ===== Increase ulimits for nofile and nproc
 
 Increased ulimits for <<setting-system-settings,nofile>> and <<max-number-threads-check,nproc>>

qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java

@@ -54,6 +54,7 @@ import static org.elasticsearch.packaging.util.Docker.runContainer;
 import static org.elasticsearch.packaging.util.Docker.runContainerExpectingFailure;
 import static org.elasticsearch.packaging.util.Docker.verifyContainerInstallation;
 import static org.elasticsearch.packaging.util.Docker.waitForElasticsearch;
+import static org.elasticsearch.packaging.util.DockerRun.builder;
 import static org.elasticsearch.packaging.util.FileMatcher.p600;
 import static org.elasticsearch.packaging.util.FileMatcher.p644;
 import static org.elasticsearch.packaging.util.FileMatcher.p660;
@@ -175,7 +176,7 @@ public class DockerTests extends PackagingTestCase {
 
         // Restart the container
         final Map<Path, Path> volumes = Map.of(tempDir, Path.of("/usr/share/elasticsearch/config"));
-        runContainer(distribution(), volumes, Map.of("ES_JAVA_OPTS", "-XX:-UseCompressedOops"));
+        runContainer(distribution(), builder().volumes(volumes).envVars(Map.of("ES_JAVA_OPTS", "-XX:-UseCompressedOops")));
 
         waitForElasticsearch(installation);
 
@@ -203,7 +204,7 @@ public class DockerTests extends PackagingTestCase {
             // Restart the container
             final Map<Path, Path> volumes = Map.of(tempEsDataDir.toAbsolutePath(), installation.data);
 
-            runContainer(distribution(), volumes, null);
+            runContainer(distribution(), builder().volumes(volumes));
 
             waitForElasticsearch(installation);
 
@@ -223,6 +224,9 @@ public class DockerTests extends PackagingTestCase {
 
     /**
      * Check that it is possible to run Elasticsearch under a different user and group to the default.
+     * Note that while the default configuration files are world-readable, when we execute Elasticsearch
+     * it will attempt to create a keystore under the `config` directory. This will fail unless
+     * we also bind-mount the config dir.
      */
     public void test072RunEsAsDifferentUserAndGroup() throws Exception {
         assumeFalse(Platforms.WINDOWS);
@@ -251,7 +255,18 @@ public class DockerTests extends PackagingTestCase {
         volumes.put(tempEsLogsDir.toAbsolutePath(), installation.logs);
 
         // Restart the container
-        runContainer(distribution(), volumes, null, 501, 501);
+        runContainer(distribution(), builder().volumes(volumes).uid(501, 501));
+
+        waitForElasticsearch(installation);
+    }
+
+    /**
+     * Check that it is possible to run Elasticsearch under a different user and group to the default,
+     * without bind-mounting any directories, provided the container user is added to the `root` group.
+     */
+    public void test073RunEsAsDifferentUserAndGroupWithoutBindMounting() throws Exception {
+        // Restart the container
+        runContainer(distribution(), builder().uid(501, 501).extraArgs("--group-add 0"));
 
         waitForElasticsearch(installation);
     }
@@ -286,7 +301,7 @@ public class DockerTests extends PackagingTestCase {
         final Map<Path, Path> volumes = Map.of(tempDir, Path.of("/run/secrets"));
 
         // Restart the container
-        runContainer(distribution(), volumes, envVars);
+        runContainer(distribution(), builder().volumes(volumes).envVars(envVars));
 
         // If we configured security correctly, then this call will only work if we specify the correct credentials.
         try {
@@ -342,7 +357,7 @@ public class DockerTests extends PackagingTestCase {
 
         // Restart the container - this will check that Elasticsearch started correctly,
         // and didn't fail to follow the symlink and check the file permissions
-        runContainer(distribution(), volumes, envVars);
+        runContainer(distribution(), builder().volumes(volumes).envVars(envVars));
     }
 
     /**
@@ -363,7 +378,7 @@ public class DockerTests extends PackagingTestCase {
 
         final Map<Path, Path> volumes = Map.of(tempDir, Path.of("/run/secrets"));
 
-        final Result dockerLogs = runContainerExpectingFailure(distribution, volumes, envVars);
+        final Result dockerLogs = runContainerExpectingFailure(distribution, builder().volumes(volumes).envVars(envVars));
 
         assertThat(
             dockerLogs.stderr,
@@ -388,7 +403,7 @@ public class DockerTests extends PackagingTestCase {
         final Map<Path, Path> volumes = Map.of(tempDir, Path.of("/run/secrets"));
 
         // Restart the container
-        final Result dockerLogs = runContainerExpectingFailure(distribution(), volumes, envVars);
+        final Result dockerLogs = runContainerExpectingFailure(distribution(), builder().volumes(volumes).envVars(envVars));
 
         assertThat(
             dockerLogs.stderr,
@@ -433,7 +448,7 @@ public class DockerTests extends PackagingTestCase {
         final Map<Path, Path> volumes = Map.of(tempDir, Path.of("/run/secrets"));
 
         // Restart the container
-        final Result dockerLogs = runContainerExpectingFailure(distribution(), volumes, envVars);
+        final Result dockerLogs = runContainerExpectingFailure(distribution(), builder().volumes(volumes).envVars(envVars));
 
         assertThat(
             dockerLogs.stderr,
@@ -456,7 +471,7 @@ public class DockerTests extends PackagingTestCase {
         // tool in question is only in the default distribution.
         assumeTrue(distribution.isDefault());
 
-        runContainer(distribution(), null, Map.of("http.host", "this.is.not.valid"));
+        runContainer(distribution(), builder().envVars(Map.of("http.host", "this.is.not.valid")));
 
         // This will fail if the env var above is passed as a -E argument
         final Result result = sh.runIgnoreExitCode("elasticsearch-setup-passwords auto");

qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java

@@ -43,6 +43,7 @@ import static org.elasticsearch.packaging.util.Docker.runContainer;
 import static org.elasticsearch.packaging.util.Docker.runContainerExpectingFailure;
 import static org.elasticsearch.packaging.util.Docker.waitForElasticsearch;
 import static org.elasticsearch.packaging.util.Docker.waitForPathToExist;
+import static org.elasticsearch.packaging.util.DockerRun.builder;
 import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File;
 import static org.elasticsearch.packaging.util.FileMatcher.file;
 import static org.elasticsearch.packaging.util.FileMatcher.p600;
@@ -280,7 +281,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
         // restart ES with password and mounted keystore
         Map<Path, Path> volumes = Map.of(localKeystoreFile, dockerKeystore);
         Map<String, String> envVars = Map.of("KEYSTORE_PASSWORD", password);
-        runContainer(distribution(), volumes, envVars);
+        runContainer(distribution(), builder().volumes(volumes).envVars(envVars));
         waitForElasticsearch(installation);
         ServerUtils.runElasticsearchTests();
     }
@@ -309,7 +310,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
             Map<Path, Path> volumes = Map.of(localKeystoreFile, dockerKeystore, tempDir, Path.of("/run/secrets"));
             Map<String, String> envVars = Map.of("KEYSTORE_PASSWORD_FILE", "/run/secrets/" + passwordFilename);
 
-            runContainer(distribution(), volumes, envVars);
+            runContainer(distribution(), builder().volumes(volumes).envVars(envVars));
 
             waitForElasticsearch(installation);
             ServerUtils.runElasticsearchTests();
@@ -334,7 +335,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
         // restart ES with password and mounted keystore
         Map<Path, Path> volumes = Map.of(localKeystoreFile, dockerKeystore);
         Map<String, String> envVars = Map.of("KEYSTORE_PASSWORD", "wrong");
-        Shell.Result r = runContainerExpectingFailure(distribution(), volumes, envVars);
+        Shell.Result r = runContainerExpectingFailure(distribution(), builder().volumes(volumes).envVars(envVars));
         assertThat(r.stderr, containsString(ERROR_INCORRECT_PASSWORD));
     }
 
@@ -362,7 +363,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
         Files.write(tempDirectory.resolve("set-pass.sh"), setPasswordScript);
 
-        runContainer(distribution(), volumes, null);
+        runContainer(distribution(), builder().volumes(volumes));
         try {
             waitForPathToExist(dockerTemp);
             waitForPathToExist(dockerKeystore);

qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java

@@ -38,9 +38,9 @@ import java.util.Set;
 import java.util.stream.Stream;
 
 import static java.nio.file.attribute.PosixFilePermissions.fromString;
-import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileExists;
 import static org.elasticsearch.packaging.util.FileMatcher.p644;
 import static org.elasticsearch.packaging.util.FileMatcher.p660;
+import static org.elasticsearch.packaging.util.FileMatcher.p664;
 import static org.elasticsearch.packaging.util.FileMatcher.p755;
 import static org.elasticsearch.packaging.util.FileMatcher.p770;
 import static org.elasticsearch.packaging.util.FileMatcher.p775;
@@ -59,7 +59,7 @@ import static org.junit.Assert.fail;
 public class Docker {
     private static final Log logger = LogFactory.getLog(Docker.class);
 
-    private static final Shell sh = new Shell();
+    static final Shell sh = new Shell();
     private static final DockerShell dockerShell = new DockerShell();
     public static final int STARTUP_SLEEP_INTERVAL_MILLISECONDS = 1000;
     public static final int STARTUP_ATTEMPTS_MAX = 10;
@@ -88,42 +88,24 @@ public class Docker {
     }
 
     /**
-     * Runs an Elasticsearch Docker container.
-     * @param distribution details about the docker image being tested.
+     * Runs an Elasticsearch Docker container, and checks that it has started up
+     * successfully.
+     *
+     * @param distribution details about the docker image being tested
      */
     public static Installation runContainer(Distribution distribution) {
-        return runContainer(distribution, null, null);
+        return runContainer(distribution, DockerRun.builder());
     }
 
     /**
-     * Runs an Elasticsearch Docker container, with options for overriding the config directory
-     * through a bind mount, and passing additional environment variables.
+     * Runs an Elasticsearch Docker container, and checks that it has started up
+     * successfully.
      *
-     * @param distribution details about the docker image being tested.
-     * @param volumes a map that declares any volume mappings to apply, or null
-     * @param envVars environment variables to set when running the container, or null
-     */
-    public static Installation runContainer(Distribution distribution, Map<Path, Path> volumes, Map<String, String> envVars) {
-        return runContainer(distribution, volumes, envVars, null, null);
-    }
-
-    /**
-     * Runs an Elasticsearch Docker container, with options for overriding the config directory
-     * through a bind mount, and passing additional environment variables.
-     * @param distribution details about the docker image being tested.
-     * @param volumes a map that declares any volume mappings to apply, or null
-     * @param envVars environment variables to set when running the container, or null
-     * @param uid optional UID to run the container under
-     * @param gid optional GID to run the container under
+     * @param distribution details about the docker image being tested
+     * @param builder the command to run
      */
-    public static Installation runContainer(
-        Distribution distribution,
-        Map<Path, Path> volumes,
-        Map<String, String> envVars,
-        Integer uid,
-        Integer gid
-    ) {
-        executeDockerRun(distribution, volumes, envVars, uid, gid);
+    public static Installation runContainer(Distribution distribution, DockerRun builder) {
+        executeDockerRun(distribution, builder);
 
         waitForElasticsearchToStart();
 
@@ -131,87 +113,26 @@ public class Docker {
     }
 
     /**
-     * Similar to {@link #runContainer(Distribution, Map, Map)} in that it runs an Elasticsearch Docker
+     * Similar to {@link #runContainer(Distribution, DockerRun)} in that it runs an Elasticsearch Docker
      * container, expect that the container expecting it to exit e.g. due to configuration problem.
      *
      * @param distribution details about the docker image being tested.
-     * @param volumes a map that declares any volume mappings to apply, or null
-     * @param envVars environment variables to set when running the container, or null
+     * @param builder the command to run
      * @return the docker logs of the container
      */
-    public static Shell.Result runContainerExpectingFailure(
-        Distribution distribution,
-        Map<Path, Path> volumes,
-        Map<String, String> envVars
-    ) {
-        executeDockerRun(distribution, volumes, envVars, null, null);
+    public static Shell.Result runContainerExpectingFailure(Distribution distribution, DockerRun builder) {
+        executeDockerRun(distribution, builder);
 
         waitForElasticsearchToExit();
 
         return getContainerLogs();
     }
 
-    private static void executeDockerRun(
-        Distribution distribution,
-        Map<Path, Path> volumes,
-        Map<String, String> envVars,
-        Integer uid,
-        Integer gid
-    ) {
+    private static void executeDockerRun(Distribution distribution, DockerRun builder) {
         removeContainer();
 
-        final List<String> args = new ArrayList<>();
-
-        args.add("docker run");
-
-        // Run the container in the background
-        args.add("--detach");
+        final String command = builder.distribution(distribution).build();
 
-        if (envVars != null) {
-            envVars.forEach((key, value) -> args.add("--env " + key + "=\"" + value + "\""));
-        }
-
-        // The container won't run without configuring discovery
-        args.add("--env discovery.type=single-node");
-
-        // Map ports in the container to the host, so that we can send requests
-        args.add("--publish 9200:9200");
-        args.add("--publish 9300:9300");
-
-        // Bind-mount any volumes
-        if (volumes != null) {
-            volumes.forEach((localPath, containerPath) -> {
-                assertThat(localPath, fileExists());
-
-                if (Platforms.WINDOWS == false && System.getProperty("user.name").equals("root") && uid == null) {
-                    // The tests are running as root, but the process in the Docker container runs as `elasticsearch` (UID 1000),
-                    // so we need to ensure that the container process is able to read the bind-mounted files.
-                    //
-                    // NOTE that we don't do this if a UID is specified - in that case, we assume that the caller knows
-                    // what they're doing!
-                    sh.run("chown -R 1000:0 " + localPath);
-                }
-                args.add("--volume \"" + localPath + ":" + containerPath + "\"");
-            });
-        }
-
-        if (uid == null) {
-            if (gid != null) {
-                throw new IllegalArgumentException("Cannot override GID without also overriding UID");
-            }
-        } else {
-            args.add("--user");
-            if (gid != null) {
-                args.add(uid + ":" + gid);
-            } else {
-                args.add(uid.toString());
-            }
-        }
-
-        // Image name
-        args.add(getImageName(distribution));
-
-        final String command = String.join(" ", args);
         logger.info("Running command: " + command);
         containerId = sh.run(command).stdout.trim();
     }
@@ -486,7 +407,7 @@ public class Docker {
         // also don't want any SELinux security context indicator.
         Set<PosixFilePermission> actualPermissions = fromString(permissions.substring(1, 10));
 
-        assertEquals("Permissions of " + path + " are wrong", actualPermissions, expectedPermissions);
+        assertEquals("Permissions of " + path + " are wrong", expectedPermissions, actualPermissions);
         assertThat("File owner of " + path + " is wrong", username, equalTo("elasticsearch"));
         assertThat("File group of " + path + " is wrong", group, equalTo("root"));
     }
@@ -530,8 +451,10 @@ public class Docker {
 
         Stream.of(es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, p755));
 
-        Stream.of("elasticsearch.keystore", "elasticsearch.yml", "jvm.options", "log4j2.properties")
-            .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p660));
+        assertPermissionsAndOwnership(es.config("elasticsearch.keystore"), p660);
+
+        Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties")
+            .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p664));
 
         assertThat(dockerShell.run(es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed"));
 
@@ -580,7 +503,7 @@ public class Docker {
         assertPermissionsAndOwnership(es.bin("elasticsearch-sql-cli-" + getCurrentVersion() + ".jar"), p755);
 
         Stream.of("role_mapping.yml", "roles.yml", "users", "users_roles")
-            .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p660));
+            .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p664));
     }
 
     public static void waitForElasticsearch(Installation installation) throws Exception {

qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java

@@ -0,0 +1,135 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.packaging.util;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileExists;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class DockerRun {
+
+    private Distribution distribution;
+    private final Map<String, String> envVars = new HashMap<>();
+    private final Map<Path, Path> volumes = new HashMap<>();
+    private Integer uid;
+    private Integer gid;
+    private final List<String> extraArgs = new ArrayList<>();
+
+    private DockerRun() {}
+
+    public static DockerRun builder() {
+        return new DockerRun();
+    }
+
+    public DockerRun distribution(Distribution distribution) {
+        this.distribution = Objects.requireNonNull(distribution);
+        return this;
+    }
+
+    public DockerRun envVars(Map<String, String> envVars) {
+        if (envVars != null) {
+            this.envVars.putAll(envVars);
+        }
+        return this;
+    }
+
+    public DockerRun volumes(Map<Path, Path> volumes) {
+        if (volumes != null) {
+            this.volumes.putAll(volumes);
+        }
+        return this;
+    }
+
+    public DockerRun uid(Integer uid, Integer gid) {
+        if (uid == null) {
+            if (gid != null) {
+                throw new IllegalArgumentException("Cannot override GID without also overriding UID");
+            }
+        }
+        this.uid = uid;
+        this.gid = gid;
+        return this;
+    }
+
+    public DockerRun extraArgs(String... args) {
+        Collections.addAll(this.extraArgs, args);
+        return this;
+    }
+
+    String build() {
+        final List<String> cmd = new ArrayList<>();
+
+        cmd.add("docker run");
+
+        // Run the container in the background
+        cmd.add("--detach");
+
+        this.envVars.forEach((key, value) -> cmd.add("--env " + key + "=\"" + value + "\""));
+
+        // The container won't run without configuring discovery
+        cmd.add("--env discovery.type=single-node");
+
+        // Map ports in the container to the host, so that we can send requests
+        cmd.add("--publish 9200:9200");
+        cmd.add("--publish 9300:9300");
+
+        // Bind-mount any volumes
+        volumes.forEach((localPath, containerPath) -> {
+            assertThat(localPath, fileExists());
+
+            if (Platforms.WINDOWS == false && System.getProperty("user.name").equals("root") && uid == null) {
+                // The tests are running as root, but the process in the Docker container runs as `elasticsearch` (UID 1000),
+                // so we need to ensure that the container process is able to read the bind-mounted files.
+                //
+                // NOTE that we don't do this if a UID is specified - in that case, we assume that the caller knows
+                // what they're doing!
+                Docker.sh.run("chown -R 1000:0 " + localPath);
+            }
+            cmd.add("--volume \"" + localPath + ":" + containerPath + "\"");
+        });
+
+        if (uid != null) {
+            cmd.add("--user");
+            if (gid != null) {
+                cmd.add(uid + ":" + gid);
+            } else {
+                cmd.add(uid.toString());
+            }
+        }
+
+        cmd.addAll(this.extraArgs);
+
+        // Image name
+        cmd.add(getImageName(distribution));
+
+        return String.join(" ", cmd);
+    }
+
+    static String getImageName(Distribution distribution) {
+        return distribution.flavor.name + (distribution.packaging == Distribution.Packaging.DOCKER_UBI ? "-ubi8" : "") + ":test";
+    }
+}

qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java

@@ -54,6 +54,7 @@ public class FileMatcher extends TypeSafeMatcher<Path> {
     public static final Set<PosixFilePermission> p750 = fromString("rwxr-x---");
     public static final Set<PosixFilePermission> p660 = fromString("rw-rw----");
     public static final Set<PosixFilePermission> p644 = fromString("rw-r--r--");
+    public static final Set<PosixFilePermission> p664 = fromString("rw-rw-r--");
     public static final Set<PosixFilePermission> p600 = fromString("rw-------");
 
     private final Fileness fileness;