Tighten up write permissions in Docker image

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 { %>
 
@@ -167,7 +167,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" ; \\
@@ -230,26 +230,24 @@ RUN tar -zxf /opt/elasticsearch.tar.gz --strip-components=1
 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. Move the generated docker logging config so that it is the 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. Ensure that there are no files with setuid or setgid, in order to mitigate "stackclash" attacks.
-# 9. Ensure all files are world-readable by default. It should be possible to
-#    examine the contents of the image under any UID:GID
+#  1. Configure the distribution for Docker
+#  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. Remove write permissions from all directories
+#  6. Remove write and exec permissions from all files
+#  7. Make tools executable again
+#  8. Make some directories writable again
+#  9. Make some files writable again
 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 {} + && \\
-    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-\$(arch)/bin/* && \\
+    chmod 0775 config config/jvm.options.d data logs plugins && \\
+    find config -type f -exec chmod 0664 {} +
 
 <% if (docker_base == "ubi" || docker_base == "iron_bank") { %>
 
@@ -287,8 +285,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 { %>
 
@@ -304,15 +301,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
@@ -329,10 +325,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 /usr/share/elasticsearch/config
 
 EXPOSE 9200 9300
 

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

@@ -100,7 +100,7 @@ public void teardownTest() {
      * Checks that the Docker image can be run, and that it passes various checks.
      */
     public void test010Install() {
-        verifyContainerInstallation(installation, distribution());
+        verifyContainerInstallation(installation);
     }
 
     /**
@@ -147,7 +147,7 @@ public void test041AmazonCaCertsAreInTheKeystore() {
     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 @@ private void verifyKeystorePermissions() {
             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/test/PackagingTestCase.java

@@ -221,7 +221,7 @@ protected static void install() throws Exception {
             case DOCKER_UBI:
             case DOCKER_IRON_BANK:
                 installation = Docker.runContainer(distribution);
-                Docker.verifyContainerInstallation(installation, distribution);
+                Docker.verifyContainerInstallation(installation);
                 break;
             default:
                 throw new IllegalStateException("Unknown Elasticsearch packaging type.");

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

@@ -29,12 +29,11 @@
 
 import static java.nio.file.attribute.PosixFilePermissions.fromString;
 import static org.elasticsearch.packaging.util.DockerRun.getImageName;
-import static org.elasticsearch.packaging.util.FileMatcher.p644;
+import static org.elasticsearch.packaging.util.FileMatcher.p444;
+import static org.elasticsearch.packaging.util.FileMatcher.p555;
 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;
-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;
@@ -247,8 +246,6 @@ protected String[] getScriptCommand(String script) {
             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 + "\""));
@@ -401,25 +398,45 @@ public static void chownWithPrivilegeEscalation(Path localPath, String ownership
 
     /**
      * 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 username = components[0];
-        final String group = components[1];
-        final String permissions = components[2];
+            final String filename = components[0];
+            final String username = components[1];
+            final String group = components[2];
+            final String permissions = components[3];
 
-        // 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));
 
-        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"));
+            String fullPath = filename.startsWith("/") ? filename : parent + "/" + filename;
+
+            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));
+        });
     }
 
     /**
@@ -441,45 +458,40 @@ public static void waitForPathToExist(Path path) throws InterruptedException {
     }
 
     /**
-     * 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
-     * @param distribution the distribution to verify
+     * Perform a variety of checks on an installation.
+     * @param es the installation to verify
      */
-    public static void verifyContainerInstallation(Installation installation, Distribution distribution) {
-        verifyOssInstallation(installation);
-        verifyDefaultInstallation(installation);
-    }
-
-    private static void verifyOssInstallation(Installation es) {
+    public static void verifyContainerInstallation(Installation es) {
+        // 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.modules).forEach(dir -> assertPermissionsAndOwnership(dir, p755));
+        Stream.of(es.bin, 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));
+        Stream.of(es.data, es.logs, es.config.resolve("jvm.options.d"), es.plugins)
+            .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p775));
 
-        assertThat(dockerShell.run(es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed"));
+        // 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.
+        assertPermissionsAndOwnership(es.config, "elasticsearch", "root", p775);
+
+        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(es.bin, es.lib).forEach(dir -> assertPermissionsAndOwnership(dir, p755));
+        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(
-            "elasticsearch",
-            "elasticsearch-cli",
-            "elasticsearch-env",
-            "elasticsearch-keystore",
-            "elasticsearch-node",
-            "elasticsearch-plugin",
-            "elasticsearch-shard"
-        ).forEach(executable -> assertPermissionsAndOwnership(es.bin(executable), p755));
+        Stream.of("LICENSE.txt", "NOTICE.txt", "README.asciidoc")
+            .forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), "root", "root", p444));
 
-        Stream.of("LICENSE.txt", "NOTICE.txt", "README.asciidoc").forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), p644));
+        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.
@@ -492,30 +504,6 @@ private static void verifyOssInstallation(Installation es) {
             );
     }
 
-    private static void verifyDefaultInstallation(Installation es) {
-        Stream.of(
-            "elasticsearch-certgen",
-            "elasticsearch-certutil",
-            "elasticsearch-croneval",
-            "elasticsearch-saml-metadata",
-            "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), p755));
-
-        // 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"), p755);
-
-        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));
     }

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

@@ -37,14 +37,16 @@ public enum Fileness {
         Directory
     }
 
-    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> p444 = fromString("r--r--r--");
+    public static final Set<PosixFilePermission> p555 = fromString("r-xr-xr-x");
+    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;