From 9ef552bebe6a5a3099b9ffe246035d91e47bc6f2 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 22 Mar 2021 13:19:34 +0000 Subject: [PATCH 01/12] Tighten up write permissions in Docker image Recursively remove write access from the `bin`, `jdk`, `lib` and `modules` directories, since this access is not required, and removing it makes it harder to exploit other issues in an ES distribution. --- distribution/docker/src/docker/Dockerfile | 24 ++++++++++--------- .../packaging/test/DockerTests.java | 2 +- .../packaging/test/PackagingTestCase.java | 2 +- .../elasticsearch/packaging/util/Docker.java | 15 +++++------- .../packaging/util/FileMatcher.java | 1 + 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/distribution/docker/src/docker/Dockerfile b/distribution/docker/src/docker/Dockerfile index b8a2f766ab0cf..16cabfb02b310 100644 --- a/distribution/docker/src/docker/Dockerfile +++ b/distribution/docker/src/docker/Dockerfile @@ -231,17 +231,18 @@ RUN tar -zxf /opt/elasticsearch.tar.gz --strip-components=1 COPY ${config_dir}/elasticsearch.yml config/ 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 -# 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. 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. 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 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 && \\ @@ -249,6 +250,7 @@ RUN sed -i -e 's/ES_DISTRIBUTION_TYPE=tar/ES_DISTRIBUTION_TYPE=docker/' bin/elas 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 && \\ 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 {} + diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index 437c3b9d10ba1..a5b84384fb06f 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/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); } /** diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 3f8179b7e2058..dab11348d52f0 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/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."); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index d9691f9ac6397..0adf36d73b657 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -28,9 +28,9 @@ import java.util.stream.Stream; import static java.nio.file.attribute.PosixFilePermissions.fromString; +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.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; @@ -442,9 +442,8 @@ 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 */ - public static void verifyContainerInstallation(Installation installation, Distribution distribution) { + public static void verifyContainerInstallation(Installation installation) { verifyOssInstallation(installation); verifyDefaultInstallation(installation); } @@ -459,15 +458,13 @@ private static void verifyOssInstallation(Installation es) { Stream.of(es.home, es.data, es.logs, es.config, es.plugins).forEach(dir -> assertPermissionsAndOwnership(dir, p775)); - Stream.of(es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, p755)); + Stream.of(es.bin, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, p555)); 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")); - Stream.of(es.bin, es.lib).forEach(dir -> assertPermissionsAndOwnership(dir, p755)); - Stream.of( "elasticsearch", "elasticsearch-cli", @@ -476,7 +473,7 @@ private static void verifyOssInstallation(Installation es) { "elasticsearch-node", "elasticsearch-plugin", "elasticsearch-shard" - ).forEach(executable -> assertPermissionsAndOwnership(es.bin(executable), p755)); + ).forEach(executable -> assertPermissionsAndOwnership(es.bin(executable), p555)); Stream.of("LICENSE.txt", "NOTICE.txt", "README.asciidoc").forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), p644)); @@ -505,11 +502,11 @@ private static void verifyDefaultInstallation(Installation es) { "x-pack-env", "x-pack-security-env", "x-pack-watcher-env" - ).forEach(executable -> assertPermissionsAndOwnership(es.bin(executable), p755)); + ).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"), p755); + assertPermissionsAndOwnership(es.bin("elasticsearch-sql-cli-" + getCurrentVersion() + ".jar"), p555); Stream.of("role_mapping.yml", "roles.yml", "users", "users_roles") .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p664)); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java index fc5258095e7fc..6861c9d22724e 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java @@ -37,6 +37,7 @@ public enum Fileness { Directory } + public static final Set p555 = fromString("r-xr-xr-x"); public static final Set p775 = fromString("rwxrwxr-x"); public static final Set p770 = fromString("rwxrwx---"); public static final Set p755 = fromString("rwxr-xr-x"); From 000e018384d27f0811b9845152f89802ead3cdad Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Tue, 23 Mar 2021 13:40:05 +0000 Subject: [PATCH 02/12] Make file permissions settings completely explicit --- distribution/docker/src/docker/Dockerfile | 31 +++--- .../elasticsearch/packaging/util/Docker.java | 100 +++++++----------- .../packaging/util/FileMatcher.java | 13 +-- 3 files changed, 60 insertions(+), 84 deletions(-) diff --git a/distribution/docker/src/docker/Dockerfile b/distribution/docker/src/docker/Dockerfile index 16cabfb02b310..affc134421399 100644 --- a/distribution/docker/src/docker/Dockerfile +++ b/distribution/docker/src/docker/Dockerfile @@ -233,26 +233,21 @@ 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 -# 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 +# 3. Move the distribution's default logging config aside +# 4. Generate a docker logging config, to be used by default +# 5. Ensure directories have the correct permissions +# 6. Ensure files have the correct permissions. This also clears the setuid and setgid bit, +# in order to mitigate "stackclash" attacks. +# 7. Ensure binaries are executable +# 8. Allow write access on specific directories 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 && \\ 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 && \\ - 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 +x bin/* jdk/bin/* jdk/lib/jspawnhelper modules/x-pack-ml/platform/*/bin/* && \\ + chmod -R ug+w config data logs plugins <% if (docker_base == "ubi" || docker_base == "iron_bank") { %> @@ -332,10 +327,12 @@ 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) RUN chmod g=u /etc/passwd && \\ chmod 0775 /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 555 /usr/share/elasticsearch EXPOSE 9200 9300 diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 0adf36d73b657..01f15d4bfa1ce 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -28,12 +28,11 @@ import java.util.stream.Stream; import static java.nio.file.attribute.PosixFilePermissions.fromString; +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; @@ -400,25 +399,38 @@ 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 + *

+ * 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 (*) * @param expectedPermissions the unix permissions that the path ought to have */ public static void assertPermissionsAndOwnership(Path path, Set 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 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 actualPermissions = fromString(permissions.substring(1, 10)); + String fullPath = 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("elasticsearch")); + assertThat("File group of " + fullPath + " is wrong", group, equalTo("root")); + }); } /** @@ -440,42 +452,32 @@ 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 + * 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) { 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.bin, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, p555)); - Stream.of(es.bin, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, p555)); + Stream.of(es.data, es.logs, es.config, es.plugins, es.config.resolve("jvm.options.d")) + .forEach(dir -> assertPermissionsAndOwnership(dir, p775)); - Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties") - .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p664)); + for (Path binariesPath : List.of(es.bin, es.bundledJdk.resolve("bin"))) { + assertPermissionsAndOwnership(binariesPath.resolve("*"), p555); + } - assertThat(dockerShell.run(es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed")); + Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties", "role_mapping.yml", "roles.yml", "users", "users_roles") + .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p664)); - Stream.of( - "elasticsearch", - "elasticsearch-cli", - "elasticsearch-env", - "elasticsearch-keystore", - "elasticsearch-node", - "elasticsearch-plugin", - "elasticsearch-shard" - ).forEach(executable -> assertPermissionsAndOwnership(es.bin(executable), p555)); + Stream.of("LICENSE.txt", "NOTICE.txt", "README.asciidoc").forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), 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. @@ -488,30 +490,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), 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); - - 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)); } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java index 6861c9d22724e..8e30258696a5c 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java @@ -37,15 +37,16 @@ public enum Fileness { Directory } + public static final Set p444 = fromString("r--r--r--"); public static final Set p555 = fromString("r-xr-xr-x"); - public static final Set p775 = fromString("rwxrwxr-x"); - public static final Set p770 = fromString("rwxrwx---"); - public static final Set p755 = fromString("rwxr-xr-x"); - public static final Set p750 = fromString("rwxr-x---"); - public static final Set p660 = fromString("rw-rw----"); + public static final Set p600 = fromString("rw-------"); public static final Set p644 = fromString("rw-r--r--"); + public static final Set p660 = fromString("rw-rw----"); public static final Set p664 = fromString("rw-rw-r--"); - public static final Set p600 = fromString("rw-------"); + public static final Set p750 = fromString("rwxr-x---"); + public static final Set p755 = fromString("rwxr-xr-x"); + public static final Set p770 = fromString("rwxrwx---"); + public static final Set p775 = fromString("rwxrwxr-x"); private final Fileness fileness; private final String owner; From 0327e44286fa587d2d588d916b3c6b64dd6b4cd1 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 24 Mar 2021 15:17:05 +0000 Subject: [PATCH 03/12] Keep the file ownership as similar as possible to rpm/deb --- distribution/docker/src/docker/Dockerfile | 56 +++++++++++-------- .../packaging/test/DockerTests.java | 2 +- .../test/KeystoreManagementTests.java | 2 +- .../elasticsearch/packaging/util/Docker.java | 34 +++++++---- 4 files changed, 57 insertions(+), 37 deletions(-) diff --git a/distribution/docker/src/docker/Dockerfile b/distribution/docker/src/docker/Dockerfile index affc134421399..67e07c8a3c6d9 100644 --- a/distribution/docker/src/docker/Dockerfile +++ b/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 @@ -231,23 +231,26 @@ RUN tar -zxf /opt/elasticsearch.tar.gz --strip-components=1 COPY ${config_dir}/elasticsearch.yml config/ 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. Move the distribution's default logging config aside -# 4. Generate a docker logging config, to be used by default -# 5. Ensure directories have the correct permissions -# 6. Ensure files have the correct permissions. This also clears the setuid and setgid bit, -# in order to mitigate "stackclash" attacks. -# 7. Ensure binaries are executable -# 8. Allow write access on specific directories +# 1. Configure the distribution for Docker +# 2. Ensure directories are created. Most already are, but make sure +# 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 +# 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 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 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 && \\ - find . -type d -exec chmod 0555 {} + && \\ - find . -type f -exec chmod 0444 {} + && \\ - chmod +x bin/* jdk/bin/* jdk/lib/jspawnhelper modules/x-pack-ml/platform/*/bin/* && \\ - chmod -R ug+w config data logs plugins + 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 {} + <% if (docker_base == "ubi" || docker_base == "iron_bank") { %> @@ -285,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 { %> @@ -302,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 @@ -328,11 +331,14 @@ COPY ${bin_dir}/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh # 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 && \\ - chmod 555 /usr/share/elasticsearch + chmod 0755 /usr/share/elasticsearch && \\ + chgrp 1000 bin && \\ + chgrp -R 1000 config data logs plugins EXPOSE 9200 9300 @@ -375,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. diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index a5b84384fb06f..5009b2d8921d6 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -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"), "root", "elasticsearch", p660); } /** diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java index 5c89d313e9ab1..2235716f48b95 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java +++ b/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, "root", "elasticsearch", p660); break; default: throw new IllegalStateException("Unknown Elasticsearch packaging type."); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 01f15d4bfa1ce..ffce877536332 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -28,9 +28,9 @@ import java.util.stream.Stream; import static java.nio.file.attribute.PosixFilePermissions.fromString; -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.p755; import static org.elasticsearch.packaging.util.FileMatcher.p770; import static org.elasticsearch.packaging.util.FileMatcher.p775; import static org.elasticsearch.packaging.util.ServerUtils.makeRequest; @@ -404,9 +404,16 @@ public static void chownWithPrivilegeEscalation(Path localPath, String ownership * 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 (*) + * @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 expectedPermissions) { + public static void assertPermissionsAndOwnership( + Path path, + String expectedUser, + String expectedGroup, + Set expectedPermissions + ) { logger.debug("Checking permissions and ownership of [" + path + "]"); final Shell.Result result = dockerShell.run("bash -c 'stat -c \"%n %U %G %A\" " + path + "'"); @@ -425,11 +432,11 @@ public static void assertPermissionsAndOwnership(Path path, Set actualPermissions = fromString(permissions.substring(1, 10)); - String fullPath = parent + "/" + filename; + String fullPath = filename.startsWith("/") ? filename : parent + "/" + filename; assertEquals("Permissions of " + fullPath + " are wrong", expectedPermissions, actualPermissions); - assertThat("File owner of " + fullPath + " is wrong", username, equalTo("elasticsearch")); - assertThat("File group of " + fullPath + " is wrong", group, equalTo("root")); + assertThat("File owner of " + fullPath + " is wrong", username, equalTo(expectedUser)); + assertThat("File group of " + fullPath + " is wrong", group, equalTo(expectedGroup)); }); } @@ -456,6 +463,7 @@ public static void waitForPathToExist(Path path) throws InterruptedException { * @param es the installation to verify */ 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"); @@ -463,19 +471,23 @@ public static void verifyContainerInstallation(Installation es) { final String homeDir = passwdResult.stdout.trim().split(":")[5]; assertThat("elasticsearch user's home directory is incorrect", homeDir, equalTo("/usr/share/elasticsearch")); - Stream.of(es.home, es.bin, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, p555)); + Stream.of(es.home, es.bundledJdk, es.lib, es.modules) + .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p755)); + + assertPermissionsAndOwnership(es.bin, "root", "elasticsearch", p775); Stream.of(es.data, es.logs, es.config, es.plugins, es.config.resolve("jvm.options.d")) - .forEach(dir -> assertPermissionsAndOwnership(dir, p775)); + .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "elasticsearch", p775)); for (Path binariesPath : List.of(es.bin, es.bundledJdk.resolve("bin"))) { - assertPermissionsAndOwnership(binariesPath.resolve("*"), p555); + assertPermissionsAndOwnership(binariesPath.resolve("*"), "root", "root", p755); } Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties", "role_mapping.yml", "roles.yml", "users", "users_roles") - .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p664)); + .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), "root", "elasticsearch", p664)); - Stream.of("LICENSE.txt", "NOTICE.txt", "README.asciidoc").forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), p444)); + Stream.of("LICENSE.txt", "NOTICE.txt", "README.asciidoc") + .forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), "root", "root", p644)); assertThat(dockerShell.run(es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed")); From 6217e574af9806cf540b2155150cdddf01a5f7c2 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 24 Mar 2021 20:20:56 +0000 Subject: [PATCH 04/12] Formatting --- .../src/test/java/org/elasticsearch/packaging/util/Docker.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index ffce877536332..d0de93801dbbf 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -471,8 +471,7 @@ public static void verifyContainerInstallation(Installation es) { final String homeDir = passwdResult.stdout.trim().split(":")[5]; assertThat("elasticsearch user's home directory is incorrect", homeDir, equalTo("/usr/share/elasticsearch")); - Stream.of(es.home, es.bundledJdk, es.lib, es.modules) - .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p755)); + Stream.of(es.home, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p755)); assertPermissionsAndOwnership(es.bin, "root", "elasticsearch", p775); From a1bf98384c2c39784081962de5da86bad60d62f1 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 24 Mar 2021 20:31:09 +0000 Subject: [PATCH 05/12] Test fixes --- .../test/java/org/elasticsearch/packaging/test/DockerTests.java | 2 +- .../elasticsearch/packaging/test/KeystoreManagementTests.java | 2 +- .../src/test/java/org/elasticsearch/packaging/util/Docker.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index 5009b2d8921d6..bca2f570e203c 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -145,7 +145,7 @@ public void test041AmazonCaCertsAreInTheKeystore() { public void test042KeystorePermissionsAreCorrect() throws Exception { waitForElasticsearch(installation); - assertPermissionsAndOwnership(installation.config("elasticsearch.keystore"), "root", "elasticsearch", p660); + assertPermissionsAndOwnership(installation.config("elasticsearch.keystore"), "elasticsearch", "elasticsearch", p660); } /** diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java index 2235716f48b95..8b816df2f1181 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java +++ b/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, "root", "elasticsearch", p660); + assertPermissionsAndOwnership(keystore, "elasticsearch", "elasticsearch", p660); break; default: throw new IllegalStateException("Unknown Elasticsearch packaging type."); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index d0de93801dbbf..6f3e500f06cf1 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -246,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 + "\"")); From e13ae49c7a365e07b2117b64c140411a2c7271e4 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 14 Apr 2021 14:58:13 +0100 Subject: [PATCH 06/12] Revert some changes that would have been considered breaking --- distribution/docker/src/docker/Dockerfile | 9 ++------- .../org/elasticsearch/packaging/test/DockerTests.java | 2 +- .../java/org/elasticsearch/packaging/util/Docker.java | 8 ++++---- .../org/elasticsearch/packaging/util/FileMatcher.java | 1 - 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/distribution/docker/src/docker/Dockerfile b/distribution/docker/src/docker/Dockerfile index b8f47433125e5..1febc94d9c284 100644 --- a/distribution/docker/src/docker/Dockerfile +++ b/distribution/docker/src/docker/Dockerfile @@ -330,14 +330,11 @@ COPY ${bin_dir}/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh # 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 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 && \\ - chmod 0755 /usr/share/elasticsearch && \\ - chgrp 1000 bin && \\ - chgrp -R 1000 config data logs plugins + chmod 0755 /usr/share/elasticsearch EXPOSE 9200 9300 @@ -380,9 +377,7 @@ RUN mkdir /licenses && cp LICENSE.txt /licenses/LICENSE COPY LICENSE /licenses/LICENSE.addendum <% } %> -<% /* 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 +USER elasticsearch:root # Our actual entrypoint is `tini`, a minimal but functional init program. It # calls the entrypoint we provide, while correctly forwarding signals. diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index c34621529ad2a..30c42373928b6 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -147,7 +147,7 @@ public void test041AmazonCaCertsAreInTheKeystore() { public void test042KeystorePermissionsAreCorrect() throws Exception { waitForElasticsearch(installation); - assertPermissionsAndOwnership(installation.config("elasticsearch.keystore"), "elasticsearch", "elasticsearch", p660); + assertPermissionsAndOwnership(installation.config("elasticsearch.keystore"), "elasticsearch", "root", p660); } /** diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 5b0db916f669c..72a95d502377e 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -474,17 +474,17 @@ public static void verifyContainerInstallation(Installation es) { Stream.of(es.home, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p755)); - assertPermissionsAndOwnership(es.bin, "root", "elasticsearch", p775); + Stream.of(es.bin, es.plugins).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p775)); - Stream.of(es.data, es.logs, es.config, es.plugins, es.config.resolve("jvm.options.d")) - .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "elasticsearch", p775)); + Stream.of(es.data, es.logs, es.config, es.config.resolve("jvm.options.d")) + .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p775)); for (Path binariesPath : List.of(es.bin, es.bundledJdk.resolve("bin"))) { assertPermissionsAndOwnership(binariesPath.resolve("*"), "root", "root", 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)); + .forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), "root", "root", p664)); Stream.of("LICENSE.txt", "NOTICE.txt", "README.asciidoc") .forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), "root", "root", p644)); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java index 8e30258696a5c..5fea50c59d8fa 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java @@ -37,7 +37,6 @@ public enum Fileness { Directory } - public static final Set p444 = fromString("r--r--r--"); public static final Set p555 = fromString("r-xr-xr-x"); public static final Set p600 = fromString("rw-------"); public static final Set p644 = fromString("rw-r--r--"); From 4a0482d7071dd6486e96c22604af3335677f68e1 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 14 Apr 2021 16:05:09 +0100 Subject: [PATCH 07/12] More tweaks --- distribution/docker/src/docker/Dockerfile | 8 ++++---- .../java/org/elasticsearch/packaging/util/Docker.java | 4 +--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/distribution/docker/src/docker/Dockerfile b/distribution/docker/src/docker/Dockerfile index 1febc94d9c284..98f0dae8a7530 100644 --- a/distribution/docker/src/docker/Dockerfile +++ b/distribution/docker/src/docker/Dockerfile @@ -231,8 +231,8 @@ 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. The `bin` dir needs to be writable because plugins can (in theory) install their own commands +# 2. Create required directory +# 3. Change permissions to allow group writes in some places # 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 @@ -242,8 +242,8 @@ COPY ${config_dir}/log4j2.properties config/log4j2.docker.properties # 9. Ensure all files are world-readable by default. It should be possible to # 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 bin config config/jvm.options.d data logs plugins && \\ + mkdir data && \\ + chmod 0775 config config/jvm.options.d data logs && \\ mv config/log4j2.properties config/log4j2.file.properties && \\ mv config/log4j2.docker.properties config/log4j2.properties && \\ chmod 0660 config/elasticsearch.yml config/log4j2*.properties && \\ diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 72a95d502377e..4e1bd9d33c164 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -472,9 +472,7 @@ public static void verifyContainerInstallation(Installation es) { final String homeDir = passwdResult.stdout.trim().split(":")[5]; assertThat("elasticsearch user's home directory is incorrect", homeDir, equalTo("/usr/share/elasticsearch")); - Stream.of(es.home, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p755)); - - Stream.of(es.bin, es.plugins).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p775)); + Stream.of(es.home, es.bin, es.bundledJdk, es.lib, es.modules, es.plugins).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p755)); Stream.of(es.data, es.logs, es.config, es.config.resolve("jvm.options.d")) .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p775)); From 12e455a285b0f6de94807eb4c18f904db9ef24ab Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 14 Apr 2021 16:12:55 +0100 Subject: [PATCH 08/12] Formatting --- .../src/test/java/org/elasticsearch/packaging/util/Docker.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 4e1bd9d33c164..e1a3bbf6b42ac 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -472,7 +472,8 @@ public static void verifyContainerInstallation(Installation es) { final String homeDir = passwdResult.stdout.trim().split(":")[5]; assertThat("elasticsearch user's home directory is incorrect", homeDir, equalTo("/usr/share/elasticsearch")); - Stream.of(es.home, es.bin, es.bundledJdk, es.lib, es.modules, es.plugins).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p755)); + Stream.of(es.home, es.bin, es.bundledJdk, es.lib, es.modules, es.plugins) + .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p755)); Stream.of(es.data, es.logs, es.config, es.config.resolve("jvm.options.d")) .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p775)); From 4ed7857d6dd10938fa4d40b8a90afad8147cedb5 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 15 Apr 2021 13:30:34 +0100 Subject: [PATCH 09/12] Revert back to manualy hardening permissions --- distribution/docker/src/docker/Dockerfile | 38 +++++++++---------- .../test/KeystoreManagementTests.java | 2 +- .../elasticsearch/packaging/util/Docker.java | 26 +++++++------ .../packaging/util/FileMatcher.java | 1 + 4 files changed, 36 insertions(+), 31 deletions(-) diff --git a/distribution/docker/src/docker/Dockerfile b/distribution/docker/src/docker/Dockerfile index 98f0dae8a7530..5c7e3205831fc 100644 --- a/distribution/docker/src/docker/Dockerfile +++ b/distribution/docker/src/docker/Dockerfile @@ -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. Create required directory -# 3. Change permissions to allow group writes in some places -# 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 data && \\ - chmod 0775 config config/jvm.options.d data logs && \\ 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,7 +285,6 @@ 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 0755 /usr/share/elasticsearch && \\ chown -R 0:0 /usr/share/elasticsearch <% } else { %> @@ -304,7 +301,6 @@ 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 0755 /usr/share/elasticsearch && \\ chown -R 0:0 /usr/share/elasticsearch <% } %> @@ -330,11 +326,15 @@ COPY ${bin_dir}/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh # 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 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 && \\ - chmod 0755 /usr/share/elasticsearch + chmod 0775 /usr/share/elasticsearch && \\ + chown elasticsearch /usr/share/elasticsearch/config EXPOSE 9200 9300 diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java index 8eef9ff4960c4..1ede698359062 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java @@ -445,7 +445,7 @@ private void verifyKeystorePermissions() { case DOCKER: case DOCKER_UBI: case DOCKER_IRON_BANK: - assertPermissionsAndOwnership(keystore, "elasticsearch", "elasticsearch", p660); + assertPermissionsAndOwnership(keystore, "elasticsearch", "root", p660); break; default: throw new IllegalStateException("Unknown Elasticsearch packaging type."); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index e1a3bbf6b42ac..2b76924e03351 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -29,9 +29,9 @@ 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.ServerUtils.makeRequest; @@ -246,8 +246,6 @@ protected String[] getScriptCommand(String script) { List cmd = new ArrayList<>(); cmd.add("docker"); cmd.add("exec"); - cmd.add("--user"); - cmd.add("elasticsearch"); cmd.add("--tty"); env.forEach((key, value) -> cmd.add("--env " + key + "=\"" + value + "\"")); @@ -472,21 +470,27 @@ public static void verifyContainerInstallation(Installation es) { final String homeDir = passwdResult.stdout.trim().split(":")[5]; assertThat("elasticsearch user's home directory is incorrect", homeDir, equalTo("/usr/share/elasticsearch")); - Stream.of(es.home, es.bin, es.bundledJdk, es.lib, es.modules, es.plugins) - .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p755)); + assertPermissionsAndOwnership(es.home, "root", "root", p775); - Stream.of(es.data, es.logs, es.config, es.config.resolve("jvm.options.d")) + Stream.of(es.bin, es.bundledJdk, es.lib, es.modules) + .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p555)); + + Stream.of(es.data, es.logs, es.config.resolve("jvm.options.d"), es.plugins) .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p775)); - for (Path binariesPath : List.of(es.bin, es.bundledJdk.resolve("bin"))) { - assertPermissionsAndOwnership(binariesPath.resolve("*"), "root", "root", p755); - } + // 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("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), "root", "root", p644)); + .forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), "root", "root", p444)); assertThat(dockerShell.run(es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed")); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java index 5fea50c59d8fa..8e30258696a5c 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java @@ -37,6 +37,7 @@ public enum Fileness { Directory } + public static final Set p444 = fromString("r--r--r--"); public static final Set p555 = fromString("r-xr-xr-x"); public static final Set p600 = fromString("rw-------"); public static final Set p644 = fromString("rw-r--r--"); From 01d3a6b2b5dbe767a7ac681544a440d2e23f784a Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 15 Apr 2021 14:26:13 +0100 Subject: [PATCH 10/12] Formatting --- .../src/test/java/org/elasticsearch/packaging/util/Docker.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 2b76924e03351..941dcec52cd07 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -472,8 +472,7 @@ public static void verifyContainerInstallation(Installation es) { assertPermissionsAndOwnership(es.home, "root", "root", p775); - Stream.of(es.bin, es.bundledJdk, es.lib, es.modules) - .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p555)); + Stream.of(es.bin, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p555)); Stream.of(es.data, es.logs, es.config.resolve("jvm.options.d"), es.plugins) .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p775)); From c8a617d4603048399469f7e6ccd86f7d894a32ee Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Tue, 13 Jul 2021 14:42:35 +0100 Subject: [PATCH 11/12] Tweaks --- distribution/docker/src/docker/Dockerfile | 2 +- .../elasticsearch/packaging/util/Docker.java | 19 +++++-------------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/distribution/docker/src/docker/Dockerfile b/distribution/docker/src/docker/Dockerfile index fe84b2ff69f77..046e6666434e9 100644 --- a/distribution/docker/src/docker/Dockerfile +++ b/distribution/docker/src/docker/Dockerfile @@ -339,7 +339,7 @@ RUN chmod g=u /etc/passwd && \\ find / -xdev -perm -4000 -exec chmod ug-s {} + && \\ 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 + chown elasticsearch bin config config/jvm.options.d data logs plugins EXPOSE 9200 9300 diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 13724a7850537..d5dcb3d2ad8cf 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -213,7 +213,7 @@ public static void removeContainer() { // 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 { @@ -463,6 +463,7 @@ public static void waitForPathToExist(Path path) throws InterruptedException { * @param es the installation to verify */ 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"); @@ -473,15 +474,13 @@ public static void verifyContainerInstallation(Installation es) { assertPermissionsAndOwnership(es.home, "root", "root", p775); - Stream.of(es.bin, es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p555)); - - Stream.of(es.data, es.logs, es.config.resolve("jvm.options.d"), es.plugins) - .forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p775)); + Stream.of(es.bundledJdk, es.lib, es.modules).forEach(dir -> assertPermissionsAndOwnership(dir, "root", "root", p555)); // 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.config, es.logs, es.config.resolve("jvm.options.d"), es.data, es.plugins) + .forEach(dir -> assertPermissionsAndOwnership(dir, "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)); @@ -608,12 +607,4 @@ public static Shell.Result getContainerLogs() { 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; - } } From 31acf7bab72c600674da9105ca85514888f98e59 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 28 Jul 2021 16:13:30 +0100 Subject: [PATCH 12/12] Update docs/changelog/70635.yaml --- docs/changelog/70635.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 docs/changelog/70635.yaml diff --git a/docs/changelog/70635.yaml b/docs/changelog/70635.yaml new file mode 100644 index 0000000000000..d877a7bbba0d5 --- /dev/null +++ b/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