Tighten up write permissions in Docker image (#70635)

Explicitly set permissions for all files in the Elasticsearch home
directory to the minimum required set, and change ownership to
`root:root` where possible.

distribution/docker/src/docker/Dockerfile

@@ -47,7 +47,7 @@ RUN set -eux ; \\
     sha256sum -c \${tini_bin}.sha256sum ; \\
     rm \${tini_bin}.sha256sum ; \\
     mv \${tini_bin} /bin/tini ; \\
-    chmod +x /bin/tini
+    chmod 0555 /bin/tini
 
 <% } else if (docker_base == 'iron_bank') { %>
 ################################################################################
@@ -62,7 +62,7 @@ FROM ${base_image} AS builder
 # `tini` is a tiny but valid init for containers. This is used to cleanly
 # control how ES and any child processes are shut down.
 COPY tini /bin/tini
-RUN chmod 0755 /bin/tini
+RUN chmod 0555 /bin/tini
 
 <% } else { %>
 
@@ -171,7 +171,7 @@ RUN set -e ; \\
     sha256sum -c "\${TINI_BIN}.sha256sum" ; \\
     rm "\${TINI_BIN}.sha256sum" ; \\
     mv "\${TINI_BIN}" /rootfs/bin/tini ; \\
-    chmod +x /rootfs/bin/tini ; \\
+    chmod 0555 /rootfs/bin/tini ; \\
     curl --retry 10 -L -O \\
       # Here we're fetching the same binaries used for the official busybox docker image from their GtiHub repository
       "https://github.com/docker-library/busybox/raw/\${BUSYBOX_COMMIT}/stable/musl/busybox.tar.xz" ; \\
@@ -235,27 +235,24 @@ COPY ${config_dir}/elasticsearch.yml config/
 COPY ${config_dir}/log4j2.properties config/log4j2.docker.properties
 
 #  1. Configure the distribution for Docker
-#  2. Ensure directories are created. Most already are, but make sure
-#  3. Apply correct permissions
-#  4. Move the distribution's default logging config aside
-#  5. Generate a docker logging config, to be used by default
-#  6. Apply more correct permissions
-#  7. The JDK's directories' permissions don't allow `java` to be executed under a different
-#     group to the default. Fix this.
-#  8. Remove write permissions from all files under `lib`, `bin`, `jdk` and `modules`
-#  9. Ensure that there are no files with setuid or setgid, in order to mitigate "stackclash" attacks.
-# 10. Ensure all files are world-readable by default. It should be possible to
-#     examine the contents of the image under any UID:GID
+#  2. Create required directory
+#  3. Move the distribution's default logging config aside
+#  4. Move the generated docker logging config so that it is the default
+#  5. Reset permissions on all directories
+#  6. Reset permissions on all files
+#  7. Make CLI tools executable
+#  8. Make some directories writable. `bin` must be writable because
+#     plugins can install their own CLI utilities.
+#  9. Make some files writable
 RUN sed -i -e 's/ES_DISTRIBUTION_TYPE=tar/ES_DISTRIBUTION_TYPE=docker/' bin/elasticsearch-env && \\
-    mkdir -p config/jvm.options.d data logs plugins && \\
-    chmod 0775 config config/jvm.options.d data logs plugins && \\
+    mkdir data && \\
     mv config/log4j2.properties config/log4j2.file.properties && \\
     mv config/log4j2.docker.properties config/log4j2.properties && \\
-    chmod 0660 config/elasticsearch.yml config/log4j2*.properties && \\
-    find ./jdk -type d -exec chmod 0755 {} + && \\
-    chmod -R a-w lib bin jdk modules && \\
-    find . -xdev -perm -4000 -exec chmod ug-s {} + && \\
-    find . -type f -exec chmod o+r {} +
+    find . -type d -exec chmod 0555 {} + && \\
+    find . -type f -exec chmod 0444 {} + && \\
+    chmod 0555 bin/* jdk/bin/* jdk/lib/jspawnhelper modules/x-pack-ml/platform/linux-*/bin/* && \\
+    chmod 0775 bin config config/jvm.options.d data logs plugins && \\
+    find config -type f -exec chmod 0664 {} +
 
 <% if (docker_base == "ubi" || docker_base == "iron_bank") { %>
 
@@ -293,8 +290,7 @@ RUN ${package_manager} update --setopt=tsflags=nodocs -y && \\
 
 RUN groupadd -g 1000 elasticsearch && \\
     adduser -u 1000 -g 1000 -G 0 -d /usr/share/elasticsearch elasticsearch && \\
-    chmod 0775 /usr/share/elasticsearch && \\
-    chown -R 1000:0 /usr/share/elasticsearch
+    chown -R 0:0 /usr/share/elasticsearch
 
 <% } else { %>
 
@@ -310,15 +306,14 @@ COPY --from=rootfs /rootfs /
 RUN addgroup -g 1000 elasticsearch && \\
     adduser -D -u 1000 -G elasticsearch -g elasticsearch -h /usr/share/elasticsearch elasticsearch && \\
     addgroup elasticsearch root && \\
-    chmod 0775 /usr/share/elasticsearch && \\
-    chgrp 0 /usr/share/elasticsearch
+    chown -R 0:0 /usr/share/elasticsearch
 
 <% } %>
 
 ENV ELASTIC_CONTAINER true
 
 WORKDIR /usr/share/elasticsearch
-COPY --from=builder --chown=1000:0 /usr/share/elasticsearch /usr/share/elasticsearch
+COPY --from=builder --chown=0:0 /usr/share/elasticsearch /usr/share/elasticsearch
 
 <% if (docker_base == "ubi" || docker_base == "iron_bank") { %>
 COPY --from=builder --chown=0:0 /bin/tini /bin/tini
@@ -335,10 +330,16 @@ COPY ${bin_dir}/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
 # 4. Replace OpenJDK's built-in CA certificate keystore with the one from the OS
 #    vendor. The latter is superior in several ways.
 #    REF: https://github.com/elastic/elasticsearch-docker/issues/171
+# 5. Tighten up permissions on the ES home dir (the permissions of the contents are handled earlier)
+# 6. You can't install plugins that include configuration when running as `elasticsearch` and the `config`
+#    dir is owned by `root`, because the installed tries to manipulate the permissions on the plugin's
+#    config directory.
 RUN chmod g=u /etc/passwd && \\
-    chmod 0775 /usr/local/bin/docker-entrypoint.sh && \\
+    chmod 0555 /usr/local/bin/docker-entrypoint.sh && \\
     find / -xdev -perm -4000 -exec chmod ug-s {} + && \\
-    ln -sf /etc/pki/ca-trust/extracted/java/cacerts /usr/share/elasticsearch/jdk/lib/security/cacerts
+    ln -sf /etc/pki/ca-trust/extracted/java/cacerts /usr/share/elasticsearch/jdk/lib/security/cacerts && \\
+    chmod 0775 /usr/share/elasticsearch && \\
+    chown elasticsearch bin config config/jvm.options.d data logs plugins
 
 EXPOSE 9200 9300
 

docs/changelog/70635.yaml

@@ -0,0 +1,8 @@
+pr: 70635
+summary: Tighten up write permissions in Docker image
+area: Packaging
+type: enhancement
+issues: []
+versions:
+ - v8.0.0
+ - v7.15.0

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

@@ -149,7 +149,7 @@ public class DockerTests extends PackagingTestCase {
     public void test042KeystorePermissionsAreCorrect() throws Exception {
         waitForElasticsearch(installation);
 
-        assertPermissionsAndOwnership(installation.config("elasticsearch.keystore"), p660);
+        assertPermissionsAndOwnership(installation.config("elasticsearch.keystore"), "elasticsearch", "root", p660);
     }
 
     /**

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

@@ -450,7 +450,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
             case DOCKER:
             case DOCKER_UBI:
             case DOCKER_IRON_BANK:
-                assertPermissionsAndOwnership(keystore, p660);
+                assertPermissionsAndOwnership(keystore, "elasticsearch", "root", p660);
                 break;
             default:
                 throw new IllegalStateException("Unknown Elasticsearch packaging type.");

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

@@ -30,12 +30,11 @@ import java.util.stream.Stream;
 
 import static java.nio.file.attribute.PosixFilePermissions.fromString;
 import static org.elasticsearch.packaging.util.DockerRun.getImageName;
+import static org.elasticsearch.packaging.util.FileMatcher.p444;
 import static org.elasticsearch.packaging.util.FileMatcher.p555;
-import static org.elasticsearch.packaging.util.FileMatcher.p644;
 import static org.elasticsearch.packaging.util.FileMatcher.p664;
 import static org.elasticsearch.packaging.util.FileMatcher.p770;
 import static org.elasticsearch.packaging.util.FileMatcher.p775;
-import static org.elasticsearch.packaging.util.FileUtils.getCurrentVersion;
 import static org.elasticsearch.packaging.util.ServerUtils.makeRequest;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
@@ -214,7 +213,7 @@ public class Docker {
 
                     // I'm not sure why we're already removing this container, but that's OK.
                     if (isErrorAcceptable == false) {
-                        throw new RuntimeException("Command was not successful: [" + command + "] result: " + result.toString());
+                        throw new RuntimeException("Command was not successful: [" + command + "] result: " + result);
                     }
                 }
             } finally {
@@ -248,8 +247,6 @@ public class Docker {
             List<String> cmd = new ArrayList<>();
             cmd.add("docker");
             cmd.add("exec");
-            cmd.add("--user");
-            cmd.add("elasticsearch:root");
             cmd.add("--tty");
 
             env.forEach((key, value) -> cmd.add("--env " + key + "=\"" + value + "\""));
@@ -402,25 +399,45 @@ public class Docker {
 
     /**
      * Checks that the specified path's permissions and ownership match those specified.
-     * @param path the path to check
+     * <p>
+     * The implementation supports multiple files being matched by the path, via bash expansion, although
+     * it is expected that only the final part of the path will contain expansions.
+     *
+     * @param path the path to check, possibly with e.g. a wildcard (<code>*</code>)
+     * @param expectedUser the file's expected user
+     * @param expectedGroup the file's expected group
      * @param expectedPermissions the unix permissions that the path ought to have
      */
-    public static void assertPermissionsAndOwnership(Path path, Set<PosixFilePermission> expectedPermissions) {
+    public static void assertPermissionsAndOwnership(
+        Path path,
+        String expectedUser,
+        String expectedGroup,
+        Set<PosixFilePermission> expectedPermissions
+    ) {
         logger.debug("Checking permissions and ownership of [" + path + "]");
 
-        final String[] components = dockerShell.run("stat -c \"%U %G %A\" " + path).stdout.split("\\s+");
+        final Shell.Result result = dockerShell.run("bash -c 'stat -c \"%n %U %G %A\" " + path + "'");
+
+        final Path parent = path.getParent();
+
+        result.stdout.lines().forEach(line -> {
+            final String[] components = line.split("\\s+");
+
+            final String filename = components[0];
+            final String username = components[1];
+            final String group = components[2];
+            final String permissions = components[3];
 
-        final String username = components[0];
-        final String group = components[1];
-        final String permissions = components[2];
+            // The final substring() is because we don't check the directory bit, and we
+            // also don't want any SELinux security context indicator.
+            Set<PosixFilePermission> actualPermissions = fromString(permissions.substring(1, 10));
 
-        // The final substring() is because we don't check the directory bit, and we
-        // also don't want any SELinux security context indicator.
-        Set<PosixFilePermission> actualPermissions = fromString(permissions.substring(1, 10));
+            String fullPath = filename.startsWith("/") ? filename : parent + "/" + filename;
 
-        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"));
+            assertEquals("Permissions of " + fullPath + " are wrong", expectedPermissions, actualPermissions);
+            assertThat("File owner of " + fullPath + " is wrong", username, equalTo(expectedUser));
+            assertThat("File group of " + fullPath + " is wrong", group, equalTo(expectedGroup));
+        });
     }
 
     /**
@@ -442,42 +459,39 @@ public class Docker {
     }
 
     /**
-     * Perform a variety of checks on an installation. If the current distribution is not OSS, additional checks are carried out.
-     * @param installation the installation to verify
+     * Perform a variety of checks on an installation.
+     * @param es the installation to verify
      */
-    public static void verifyContainerInstallation(Installation installation) {
-        verifyOssInstallation(installation);
-        verifyDefaultInstallation(installation);
-    }
-
-    private static void verifyOssInstallation(Installation es) {
+    public static void verifyContainerInstallation(Installation es) {
+        // Ensure the `elasticsearch` user and group exist.
+        // These lines will both throw an exception if the command fails
         dockerShell.run("id elasticsearch");
         dockerShell.run("getent group elasticsearch");
 
         final Shell.Result passwdResult = dockerShell.run("getent passwd elasticsearch");
         final String homeDir = passwdResult.stdout.trim().split(":")[5];
-        assertThat(homeDir, equalTo("/usr/share/elasticsearch"));
+        assertThat("elasticsearch user's home directory is incorrect", homeDir, equalTo("/usr/share/elasticsearch"));
 
-        Stream.of(es.home, es.data, es.logs, es.config, es.plugins).forEach(dir -> assertPermissionsAndOwnership(dir, p775));
+        assertPermissionsAndOwnership(es.home, "root", "root", p775);
 
-        Stream.of(es.bin, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, p555));
+        Stream.of(es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p555));
 
-        Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties")
-            .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p664));
+        // You can't install plugins that include configuration when running as `elasticsearch` and the `config`
+        // dir is owned by `root`, because the installed tries to manipulate the permissions on the plugin's
+        // config directory.
+        Stream.of(es.bin, es.config, es.logs, es.config.resolve("jvm.options.d"), es.data, es.plugins)
+            .forEach(dir -> assertPermissionsAndOwnership(dir, "elasticsearch", "root", p775));
 
-        assertThat(dockerShell.run(es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed"));
+        Stream.of(es.bin, es.bundledJdk.resolve("bin"), es.modules.resolve("x-pack-ml/platform/linux-*/bin"))
+            .forEach(binariesPath -> assertPermissionsAndOwnership(binariesPath.resolve("*"), "root", "root", p555));
 
-        Stream.of(
-            "elasticsearch",
-            "elasticsearch-cli",
-            "elasticsearch-env",
-            "elasticsearch-keystore",
-            "elasticsearch-node",
-            "elasticsearch-plugin",
-            "elasticsearch-shard"
-        ).forEach(executable -> assertPermissionsAndOwnership(es.bin(executable), p555));
+        Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties", "role_mapping.yml", "roles.yml", "users", "users_roles")
+            .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), "root", "root", p664));
 
-        Stream.of("LICENSE.txt", "NOTICE.txt", "README.asciidoc").forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), p644));
+        Stream.of("LICENSE.txt", "NOTICE.txt", "README.asciidoc")
+            .forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), "root", "root", p444));
+
+        assertThat(dockerShell.run(es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed"));
 
         // nc is useful for checking network issues
         // zip/unzip are installed to help users who are working with certificates.
@@ -490,38 +504,6 @@ public class Docker {
             );
     }
 
-    private static void verifyDefaultInstallation(Installation es) {
-        Stream.of(
-            "elasticsearch-certgen",
-            "elasticsearch-certutil",
-            "elasticsearch-croneval",
-            "elasticsearch-saml-metadata",
-            "elasticsearch-security-config",
-            "elasticsearch-setup-passwords",
-            "elasticsearch-sql-cli",
-            "elasticsearch-syskeygen",
-            "elasticsearch-users",
-            "elasticsearch-service-tokens",
-            "x-pack-env",
-            "x-pack-security-env",
-            "x-pack-watcher-env"
-        ).forEach(executable -> assertPermissionsAndOwnership(es.bin(executable), p555));
-
-        // at this time we only install the current version of archive distributions, but if that changes we'll need to pass
-        // the version through here
-        assertPermissionsAndOwnership(es.bin("elasticsearch-sql-cli-" + getCurrentVersion() + ".jar"), p555);
-
-        final String architecture = getArchitecture();
-        Stream.of("autodetect", "categorize", "controller", "data_frame_analyzer", "normalize", "pytorch_inference")
-            .forEach(executableName -> {
-                final Path executablePath = es.modules.resolve("x-pack-ml/platform/linux-" + architecture + "/bin/" + executableName);
-                assertPermissionsAndOwnership(executablePath, p555);
-            });
-
-        Stream.of("role_mapping.yml", "roles.yml", "users", "users_roles")
-            .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p664));
-    }
-
     public static void waitForElasticsearch(Installation installation) throws Exception {
         withLogging(() -> ServerUtils.waitForElasticsearch(installation));
     }
@@ -625,12 +607,4 @@ public class Docker {
     public static void restartContainer() {
         sh.run("docker restart " + containerId);
     }
-
-    private static String getArchitecture() {
-        String architecture = System.getProperty("os.arch", "x86_64");
-        if (architecture.equals("amd64")) {
-            architecture = "x86_64";
-        }
-        return architecture;
-    }
 }

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

@@ -37,15 +37,16 @@ public class FileMatcher extends TypeSafeMatcher<Path> {
         Directory
     }
 
+    public static final Set<PosixFilePermission> p444 = fromString("r--r--r--");
     public static final Set<PosixFilePermission> p555 = fromString("r-xr-xr-x");
-    public static final Set<PosixFilePermission> p775 = fromString("rwxrwxr-x");
-    public static final Set<PosixFilePermission> p770 = fromString("rwxrwx---");
-    public static final Set<PosixFilePermission> p755 = fromString("rwxr-xr-x");
-    public static final Set<PosixFilePermission> p750 = fromString("rwxr-x---");
-    public static final Set<PosixFilePermission> p660 = fromString("rw-rw----");
+    public static final Set<PosixFilePermission> p600 = fromString("rw-------");
     public static final Set<PosixFilePermission> p644 = fromString("rw-r--r--");
+    public static final Set<PosixFilePermission> p660 = fromString("rw-rw----");
     public static final Set<PosixFilePermission> p664 = fromString("rw-rw-r--");
-    public static final Set<PosixFilePermission> p600 = fromString("rw-------");
+    public static final Set<PosixFilePermission> p750 = fromString("rwxr-x---");
+    public static final Set<PosixFilePermission> p755 = fromString("rwxr-xr-x");
+    public static final Set<PosixFilePermission> p770 = fromString("rwxrwx---");
+    public static final Set<PosixFilePermission> p775 = fromString("rwxrwxr-x");
 
     private final Fileness fileness;
     private final String owner;