Skip to content

Commit

Permalink
Revert "Tighten up write permissions in Docker image (#70635)"
Browse files Browse the repository at this point in the history 
This reverts commit c04dc4c.
  • Loading branch information
@mark-vieira
mark-vieira committed Aug 2, 2021
1 parent 41dc515 commit 8effc56
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 117 deletions.
64 changes: 28 additions & 36 deletions distribution/docker/src/docker/Dockerfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ RUN set -eux ; \\
sha256sum -c \${tini_bin}.sha256sum ; \\
rm \${tini_bin}.sha256sum ; \\
mv \${tini_bin} /bin/tini ; \\
chmod 0555 /bin/tini
chmod +x /bin/tini
<% } else if (docker_base == 'iron_bank') { %>
################################################################################
Expand All @@ -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 0555 /bin/tini
RUN chmod 0755 /bin/tini
<% } else { %>
Expand Down Expand Up @@ -122,7 +122,7 @@ RUN set -e ; \\
sha256sum -c "\${TINI_BIN}.sha256sum" ; \\
rm "\${TINI_BIN}.sha256sum" ; \\
mv "\${TINI_BIN}" /rootfs/bin/tini ; \\
chmod 0555 /rootfs/bin/tini
chmod +x /rootfs/bin/tini
RUN echo "NETWORKING=yes" > /rootfs/etc/sysconfig/network && \\
echo "HOSTNAME=localhost.localdomain" >> /rootfs/etc/sysconfig/network
Expand Down Expand Up @@ -170,24 +170,27 @@ 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. Move the distribution's default logging config aside
# 4. Move the generated docker logging config so that it is the default
# 5. Reset permissions on all directories
# 6. Reset permissions on all files
# 7. Make CLI tools executable
# 8. Make some directories writable. `bin` must be writable because
# plugins can install their own CLI utilities.
# 9. Make some files writable
# 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 data && \\
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 && \\
mv config/log4j2.docker.properties config/log4j2.properties && \\
find . -type d -exec chmod 0555 {} + && \\
find . -type f -exec chmod 0444 {} + && \\
chmod 0555 bin/* jdk/bin/* jdk/lib/jspawnhelper modules/x-pack-ml/platform/linux-*/bin/* && \\
chmod 0775 bin config config/jvm.options.d data logs plugins && \\
find config -type f -exec chmod 0664 {} +
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 {} +

<% if (docker_base == "ubi" || docker_base == "iron_bank") { %>

Expand Down Expand Up @@ -223,11 +226,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 { %>
################################################################################
Expand All @@ -239,17 +237,17 @@ FROM scratch
# Setup the initial filesystem.
COPY --from=rootfs /rootfs /
<% } %>
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
<% } %>
chmod 0775 /usr/share/elasticsearch && \\
chown -R 1000:0 /usr/share/elasticsearch
ENV ELASTIC_CONTAINER true
WORKDIR /usr/share/elasticsearch
COPY --from=builder --chown=0:0 /usr/share/elasticsearch /usr/share/elasticsearch
COPY --from=builder --chown=1000: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
Expand All @@ -266,16 +264,10 @@ 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 0555 /usr/local/bin/docker-entrypoint.sh && \\
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 && \\
chmod 0775 /usr/share/elasticsearch && \\
chown elasticsearch bin config config/jvm.options.d data logs plugins
ln -sf /etc/pki/ca-trust/extracted/java/cacerts /usr/share/elasticsearch/jdk/lib/security/cacerts

EXPOSE 9200 9300

Expand Down
8 changes: 0 additions & 8 deletions docs/changelog/70635.yaml
View file

This file was deleted.

2 changes: 1 addition & 1 deletion qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public void test041AmazonCaCertsAreInTheKeystore() {
public void test042KeystorePermissionsAreCorrect() throws Exception {
waitForElasticsearch(installation);

assertPermissionsAndOwnership(installation.config("elasticsearch.keystore"), "elasticsearch", "root", p660);
assertPermissionsAndOwnership(installation.config("elasticsearch.keystore"), p660);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ private void verifyKeystorePermissions() {
case DOCKER:
case DOCKER_UBI:
case DOCKER_IRON_BANK:
assertPermissionsAndOwnership(keystore, "elasticsearch", "root", p660);
assertPermissionsAndOwnership(keystore, p660);
break;
default:
throw new IllegalStateException("Unknown Elasticsearch packaging type.");
Expand Down
150 changes: 86 additions & 64 deletions qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@

import static java.nio.file.attribute.PosixFilePermissions.fromString;
import static org.elasticsearch.packaging.util.DockerRun.getImageName;
import static org.elasticsearch.packaging.util.FileMatcher.p444;
import static org.elasticsearch.packaging.util.FileMatcher.p555;
import static org.elasticsearch.packaging.util.FileMatcher.p644;
import static org.elasticsearch.packaging.util.FileMatcher.p664;
import static org.elasticsearch.packaging.util.FileMatcher.p770;
import static org.elasticsearch.packaging.util.FileMatcher.p775;
import static org.elasticsearch.packaging.util.FileUtils.getCurrentVersion;
import static org.elasticsearch.packaging.util.ServerUtils.makeRequest;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -215,7 +216,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);
throw new RuntimeException("Command was not successful: [" + command + "] result: " + result.toString());
}
}
} finally {
Expand Down Expand Up @@ -249,6 +250,8 @@ 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 + "\""));
Expand Down Expand Up @@ -371,45 +374,25 @@ public static void chownWithPrivilegeEscalation(Path localPath, String ownership

/**
* Checks that the specified path's permissions and ownership match those specified.
* <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 path the path to check
* @param expectedPermissions the unix permissions that the path ought to have
*/
public static void assertPermissionsAndOwnership(
Path path,
String expectedUser,
String expectedGroup,
Set<PosixFilePermission> expectedPermissions
) {
public static void assertPermissionsAndOwnership(Path path, Set<PosixFilePermission> expectedPermissions) {
logger.debug("Checking permissions and ownership of [" + path + "]");

final Shell.Result result = dockerShell.run("bash -c 'stat --format=\"%n %U %G %A\" '" + path);

final Path parent = path.getParent();

Arrays.asList(result.stdout.split("\\n")).forEach(line -> {
final String[] components = line.split("\\s+");
final String[] components = dockerShell.run("stat --format=\"%U %G %A\" " + path).stdout.split("\\s+");

final String filename = components[0];
final String username = components[1];
final String group = components[2];
final String permissions = components[3];
final String username = components[0];
final String group = components[1];
final String permissions = components[2];

// The final substring() is because we don't check the directory bit, and we
// also don't want any SELinux security context indicator.
Set<PosixFilePermission> actualPermissions = fromString(permissions.substring(1, 10));
// The final substring() is because we don't check the directory bit, and we
// also don't want any SELinux security context indicator.
Set<PosixFilePermission> actualPermissions = fromString(permissions.substring(1, 10));

String fullPath = filename.startsWith("/") ? filename : parent + "/" + filename;

assertEquals("Permissions of " + 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));
});
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"));
}

/**
Expand All @@ -431,49 +414,80 @@ public static void waitForPathToExist(Path path) throws InterruptedException {
}

/**
* Perform a variety of checks on an installation.
* @param es the installation to verify
* 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
*/
public static void verifyContainerInstallation(Installation es) {
// Ensure the `elasticsearch` user and group exist.
// These lines will both throw an exception if the command fails
public static void verifyContainerInstallation(Installation installation) {
verifyOssInstallation(installation);
verifyDefaultInstallation(installation);
}

private static void verifyOssInstallation(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("elasticsearch user's home directory is incorrect", homeDir, equalTo("/usr/share/elasticsearch"));
assertThat(homeDir, equalTo("/usr/share/elasticsearch"));

assertPermissionsAndOwnership(es.home, "root", "root", p775);
Stream.of(es.home, es.data, es.logs, es.config, es.plugins).forEach(dir -> assertPermissionsAndOwnership(dir, p775));

Stream.of(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, 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.
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("elasticsearch.yml", "jvm.options", "log4j2.properties")
.forEach(configFile -> assertPermissionsAndOwnership(es.config(configFile), p664));

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));
assertThat(dockerShell.run(es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed"));

Stream.of("LICENSE.txt", "NOTICE.txt", "README.asciidoc")
.forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), "root", "root", p444));
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), p644));

// These are installed to help users who are working with certificates.
Stream.of("zip", "unzip").forEach(cliPackage -> {
// We could run `yum list installed $pkg` but that causes yum to call out to the network.
// rpm does the job just as well.
final Shell.Result result = dockerShell.runIgnoreExitCode("rpm -q " + cliPackage);
assertTrue(cliPackage + " ought to be installed. " + result, result.isSuccess());
});
}

assertThat(dockerShell.run(es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed"));
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);

final String architecture = getArchitecture();
Stream.of("autodetect", "categorize", "controller", "data_frame_analyzer", "normalize").forEach(executableName -> {
final Path executablePath = es.modules.resolve("x-pack-ml/platform/linux-" + architecture + "/bin/" + executableName);
assertPermissionsAndOwnership(executablePath, p555);
});

// nc is useful for checking network issues
// zip/unzip are installed to help users who are working with certificates.
Stream.of("nc", "unzip", "zip")
.forEach(
cliBinary -> assertTrue(
cliBinary + " ought to be available.",
dockerShell.runIgnoreExitCode("bash -c 'hash " + cliBinary + "'").isSuccess()
)
);
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 {
Expand Down Expand Up @@ -579,4 +593,12 @@ 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;
}
}
13 changes: 6 additions & 7 deletions qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,15 @@ public enum Fileness {
Directory
}

public static final Set<PosixFilePermission> p444 = fromString("r--r--r--");
public static final Set<PosixFilePermission> p555 = fromString("r-xr-xr-x");
public static final Set<PosixFilePermission> p600 = fromString("rw-------");
public static final Set<PosixFilePermission> p644 = fromString("rw-r--r--");
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> p644 = fromString("rw-r--r--");
public static final Set<PosixFilePermission> p664 = fromString("rw-rw-r--");
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");
public static final Set<PosixFilePermission> p600 = fromString("rw-------");

private final Fileness fileness;
private final String owner;
Expand Down

0 comments on commit 8effc56

Please sign in to comment.