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 { %>
 
@@ -168,7 +168,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 /rootfs/bin/busybox \\
       "https://busybox.net/downloads/binaries/\${BUSYBOX_VERSION}-defconfig-multiarch-musl/busybox-\${BUSYBOX_ARCH}" ; \\
     chmod +x /rootfs/bin/busybox
@@ -233,7 +233,7 @@ COPY ${bin_dir}/transform-log4j-config-${version}.jar /tmp/
 
 # 1. Configure the distribution for Docker
 # 2. Ensure directories are created. Most already are, but make sure
-# 3. Apply correct permissions
+# 3. Apply correct permissions. The `bin` dir needs to be writable because plugins can (in theory) install their own commands
 # 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
@@ -244,7 +244,7 @@ COPY ${bin_dir}/transform-log4j-config-${version}.jar /tmp/
 #    examine the contents of the image under any UID:GID
 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 && \\
+    chmod 0775 bin config config/jvm.options.d data logs plugins && \\
     mv config/log4j2.properties config/log4j2.file.properties && \\
     jdk/bin/java -jar /tmp/transform-log4j-config-${version}.jar config/log4j2.file.properties > config/log4j2.properties && \\
     chmod 0660 config/elasticsearch.yml config/log4j2*.properties && \\
@@ -288,8 +288,8 @@ 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
+    chmod 0755 /usr/share/elasticsearch && \\
+    chown -R 0:0 /usr/share/elasticsearch
 
 <% } else { %>
 
@@ -305,15 +305,15 @@ 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
+    chmod 0755 /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
@@ -330,10 +330,15 @@ 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. Make it possible for a user in the `elasticsearch` group to write to certain directories
 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 0755 /usr/share/elasticsearch && \\
+    chgrp 1000 bin && \\
+    chgrp -R 1000 config data logs plugins
 
 EXPOSE 9200 9300
 
@@ -376,7 +381,9 @@ RUN mkdir /licenses && cp LICENSE.txt /licenses/LICENSE
 COPY LICENSE /licenses/LICENSE.addendum
 <% } %>
 
-USER elasticsearch:root
+<% /* We used to set `elasticsearch:root` but by specifying the group, it caused the user that Docker runs */ %>
+<% /* the container with to **only** have that group, and not the `elasticsearch` group as well. */ %>
+USER elasticsearch
 
 # Our actual entrypoint is `tini`, a minimal but functional init program. It
 # calls the entrypoint we provide, while correctly forwarding signals.

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

@@ -98,7 +98,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);
     }
 
     /**
@@ -145,7 +145,7 @@ public void test041AmazonCaCertsAreInTheKeystore() {
     public void test042KeystorePermissionsAreCorrect() throws Exception {
         waitForElasticsearch(installation);
 
-        assertPermissionsAndOwnership(installation.config("elasticsearch.keystore"), p660);
+        assertPermissionsAndOwnership(installation.config("elasticsearch.keystore"), "elasticsearch", "elasticsearch", p660);
     }
 
     /**

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

@@ -444,7 +444,7 @@ private void verifyKeystorePermissions() {
                 break;
             case DOCKER:
             case DOCKER_UBI:
-                assertPermissionsAndOwnership(keystore, p660);
+                assertPermissionsAndOwnership(keystore, "elasticsearch", "elasticsearch", p660);
                 break;
             default:
                 throw new IllegalStateException("Unknown Elasticsearch packaging type.");

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

@@ -216,7 +216,7 @@ protected static void install() throws Exception {
             case DOCKER:
             case DOCKER_UBI:
                 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

@@ -33,7 +33,6 @@
 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,7 +246,7 @@ protected String[] getScriptCommand(String script) {
             cmd.add("docker");
             cmd.add("exec");
             cmd.add("--user");
-            cmd.add("elasticsearch:root");
+            cmd.add("elasticsearch");
             cmd.add("--tty");
 
             env.forEach((key, value) -> cmd.add("--env " + key + "=\"" + value + "\""));
@@ -400,25 +399,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));
+        });
     }
 
     /**
@@ -440,45 +459,36 @@ 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));
+        Stream.of(es.home, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p755));
 
-        Stream.of(es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, p755));
+        assertPermissionsAndOwnership(es.bin, "root", "elasticsearch", p775);
 
-        Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties")
-            .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p664));
+        Stream.of(es.data, es.logs, es.config, es.plugins, es.config.resolve("jvm.options.d"))
+            .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "elasticsearch", p775));
 
-        assertThat(dockerShell.run(es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed"));
+        for (Path binariesPath : List.of(es.bin, es.bundledJdk.resolve("bin"))) {
+            assertPermissionsAndOwnership(binariesPath.resolve("*"), "root", "root", p755);
+        }
 
-        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", "elasticsearch", 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", p644));
 
-        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.
@@ -491,30 +501,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;