From b96cb9b534841cd959088964e70626c043798cd5 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 16 Jan 2020 14:23:15 +0000 Subject: [PATCH 1/4] Follow symlinks in Docker entrypoint (#50927) Closes #49653. When using _FILE environment variables to supply values to Elasticsearch, following symlinks when checking that file permissions are secure. --- .../src/bin/elasticsearch-env-from-file | 12 +- .../packaging/test/DockerTests.java | 103 ++++++++++++++++-- 2 files changed, 104 insertions(+), 11 deletions(-) diff --git a/distribution/src/bin/elasticsearch-env-from-file b/distribution/src/bin/elasticsearch-env-from-file index fd5326afcc6b7..d2cca3d729951 100644 --- a/distribution/src/bin/elasticsearch-env-from-file +++ b/distribution/src/bin/elasticsearch-env-from-file @@ -24,10 +24,14 @@ for VAR_NAME_FILE in $(env | cut -f1 -d= | grep '_FILE$'); do exit 1 fi - FILE_PERMS="$(stat -c '%a' ${!VAR_NAME_FILE})" - - if [[ "$FILE_PERMS" != "400" && "$FILE_PERMS" != 600 ]]; then - echo "ERROR: File ${!VAR_NAME_FILE} from $VAR_NAME_FILE must have file permissions 400 or 600, but actually has: $FILE_PERMS" >&2 + FILE_PERMS="$(stat -L -c '%a' ${!VAR_NAME_FILE})" + + if [[ "$FILE_PERMS" != "400" && "$FILE_PERMS" != "600" ]]; then + if [[ -h "${!VAR_NAME_FILE}" ]]; then + echo "ERROR: File $(readlink "${!VAR_NAME_FILE}") (target of symlink ${!VAR_NAME_FILE} from $VAR_NAME_FILE) must have file permissions 400 or 600, but actually has: $FILE_PERMS" >&2 + else + echo "ERROR: File ${!VAR_NAME_FILE} from $VAR_NAME_FILE must have file permissions 400 or 600, but actually has: $FILE_PERMS" >&2 + fi exit 1 fi 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 b5529165e7f48..4091f73739d1a 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 @@ -38,6 +38,7 @@ import static org.elasticsearch.packaging.util.Docker.waitForPathToExist; import static org.elasticsearch.packaging.util.FileMatcher.p600; import static org.elasticsearch.packaging.util.FileMatcher.p660; +import static org.elasticsearch.packaging.util.FileMatcher.p775; import static org.elasticsearch.packaging.util.FileUtils.append; import static org.elasticsearch.packaging.util.FileUtils.getTempDir; import static org.elasticsearch.packaging.util.FileUtils.rm; @@ -52,6 +53,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; import java.io.IOException; @@ -338,10 +340,51 @@ public void test081ConfigurePasswordThroughEnvironmentVariableFile() throws Exce assertThat("Expected server to require authentication", statusCode, equalTo(401)); } + /** + * Check that when verifying the file permissions of _FILE environment variables, symlinks + * are followed. + */ + public void test082SymlinksAreFollowedWithEnvironmentVariableFiles() throws Exception { + // Test relies on configuring security + assumeTrue(distribution.isDefault()); + // Test relies on symlinks + assumeFalse(Platforms.WINDOWS); + + final String xpackPassword = "hunter2"; + final String passwordFilename = "password.txt"; + final String symlinkFilename = "password_symlink"; + + // ELASTIC_PASSWORD_FILE + Files.writeString(tempDir.resolve(passwordFilename), xpackPassword + "\n"); + + // Link to the password file. We can't use an absolute path for the target, because + // it won't resolve inside the container. + Files.createSymbolicLink(tempDir.resolve(symlinkFilename), Path.of(passwordFilename)); + + Map envVars = Map.of( + "ELASTIC_PASSWORD_FILE", + "/run/secrets/" + symlinkFilename, + // Enable security so that we can test that the password has been used + "xpack.security.enabled", + "true" + ); + + // File permissions need to be secured in order for the ES wrapper to accept + // them for populating env var values. The wrapper will resolve the symlink + // and check the target's permissions. + Files.setPosixFilePermissions(tempDir.resolve(passwordFilename), p600); + + final Map volumes = Map.of(tempDir, Path.of("/run/secrets")); + + // Restart the container - this will check that Elasticsearch started correctly, + // and didn't fail to follow the symlink and check the file permissions + runContainer(distribution(), volumes, envVars); + } + /** * Check that environment variables cannot be used with _FILE environment variables. */ - public void test081CannotUseEnvVarsAndFiles() throws Exception { + public void test083CannotUseEnvVarsAndFiles() throws Exception { final String optionsFilename = "esJavaOpts.txt"; // ES_JAVA_OPTS_FILE @@ -369,7 +412,7 @@ public void test081CannotUseEnvVarsAndFiles() throws Exception { * Check that when populating environment variables by setting variables with the suffix "_FILE", * the files' permissions are checked. */ - public void test082EnvironmentVariablesUsingFilesHaveCorrectPermissions() throws Exception { + public void test084EnvironmentVariablesUsingFilesHaveCorrectPermissions() throws Exception { final String optionsFilename = "esJavaOpts.txt"; // ES_JAVA_OPTS_FILE @@ -393,11 +436,60 @@ public void test082EnvironmentVariablesUsingFilesHaveCorrectPermissions() throws ); } + /** + * Check that when verifying the file permissions of _FILE environment variables, symlinks + * are followed, and that invalid target permissions are detected. + */ + public void test085SymlinkToFileWithInvalidPermissionsIsRejected() throws Exception { + // Test relies on configuring security + assumeTrue(distribution.isDefault()); + // Test relies on symlinks + assumeFalse(Platforms.WINDOWS); + + final String xpackPassword = "hunter2"; + final String passwordFilename = "password.txt"; + final String symlinkFilename = "password_symlink"; + + // ELASTIC_PASSWORD_FILE + Files.writeString(tempDir.resolve(passwordFilename), xpackPassword + "\n"); + + // Link to the password file. We can't use an absolute path for the target, because + // it won't resolve inside the container. + Files.createSymbolicLink(tempDir.resolve(symlinkFilename), Path.of(passwordFilename)); + + Map envVars = Map.of( + "ELASTIC_PASSWORD_FILE", + "/run/secrets/" + symlinkFilename, + // Enable security so that we can test that the password has been used + "xpack.security.enabled", + "true" + ); + + // Set invalid permissions on the file that the symlink targets + Files.setPosixFilePermissions(tempDir.resolve(passwordFilename), p775); + + final Map volumes = Map.of(tempDir, Path.of("/run/secrets")); + + // Restart the container + final Result dockerLogs = runContainerExpectingFailure(distribution(), volumes, envVars); + + assertThat( + dockerLogs.stderr, + containsString( + "ERROR: File " + + passwordFilename + + " (target of symlink /run/secrets/" + + symlinkFilename + + " from ELASTIC_PASSWORD_FILE) must have file permissions 400 or 600, but actually has: 775" + ) + ); + } + /** * Check that environment variables are translated to -E options even for commands invoked under * `docker exec`, where the Docker image's entrypoint is not executed. */ - public void test83EnvironmentVariablesAreRespectedUnderDockerExec() { + public void test086EnvironmentVariablesAreRespectedUnderDockerExec() { // This test relies on a CLI tool attempting to connect to Elasticsearch, and the // tool in question is only in the default distribution. assumeTrue(distribution.isDefault()); @@ -408,10 +500,7 @@ public void test83EnvironmentVariablesAreRespectedUnderDockerExec() { final Result result = sh.runIgnoreExitCode("elasticsearch-setup-passwords auto"); assertFalse("elasticsearch-setup-passwords command should have failed", result.isSuccess()); - assertThat( - result.stdout, - containsString("java.net.UnknownHostException: this.is.not.valid: Name or service not known") - ); + assertThat(result.stdout, containsString("java.net.UnknownHostException: this.is.not.valid: Name or service not known")); } /** From f390b03238f9eeeccd0c36b7b74d5f86acaea04b Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 16 Jan 2020 15:26:05 +0000 Subject: [PATCH 2/4] Fix JVM version compatibility --- .../packaging/test/DockerTests.java | 39 ++++++++----------- 1 file changed, 16 insertions(+), 23 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 4091f73739d1a..24c53fa21097b 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 @@ -61,7 +61,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -359,22 +358,19 @@ public void test082SymlinksAreFollowedWithEnvironmentVariableFiles() throws Exce // Link to the password file. We can't use an absolute path for the target, because // it won't resolve inside the container. - Files.createSymbolicLink(tempDir.resolve(symlinkFilename), Path.of(passwordFilename)); - - Map envVars = Map.of( - "ELASTIC_PASSWORD_FILE", - "/run/secrets/" + symlinkFilename, - // Enable security so that we can test that the password has been used - "xpack.security.enabled", - "true" - ); + Files.createSymbolicLink(tempDir.resolve(symlinkFilename), Paths.get(passwordFilename)); + + // Enable security so that we can test that the password has been used + Map envVars = new HashMap<>(); + envVars.put("ELASTIC_PASSWORD_FILE", "/run/secrets/" + symlinkFilename); + envVars.put("xpack.security.enabled", "true"); // File permissions need to be secured in order for the ES wrapper to accept // them for populating env var values. The wrapper will resolve the symlink // and check the target's permissions. Files.setPosixFilePermissions(tempDir.resolve(passwordFilename), p600); - final Map volumes = Map.of(tempDir, Path.of("/run/secrets")); + final Map volumes = singletonMap(tempDir, Paths.get("/run/secrets")); // Restart the container - this will check that Elasticsearch started correctly, // and didn't fail to follow the symlink and check the file permissions @@ -451,24 +447,21 @@ public void test085SymlinkToFileWithInvalidPermissionsIsRejected() throws Except final String symlinkFilename = "password_symlink"; // ELASTIC_PASSWORD_FILE - Files.writeString(tempDir.resolve(passwordFilename), xpackPassword + "\n"); + Files.write(tempDir.resolve(passwordFilename), (xpackPassword + "\n").getBytes()); // Link to the password file. We can't use an absolute path for the target, because // it won't resolve inside the container. - Files.createSymbolicLink(tempDir.resolve(symlinkFilename), Path.of(passwordFilename)); - - Map envVars = Map.of( - "ELASTIC_PASSWORD_FILE", - "/run/secrets/" + symlinkFilename, - // Enable security so that we can test that the password has been used - "xpack.security.enabled", - "true" - ); + Files.createSymbolicLink(tempDir.resolve(symlinkFilename), Paths.get(passwordFilename)); + + // Enable security so that we can test that the password has been used + Map envVars = new HashMap<>(); + envVars.put("ELASTIC_PASSWORD_FILE", "/run/secrets/" + symlinkFilename); + envVars.put("xpack.security.enabled", "true"); // Set invalid permissions on the file that the symlink targets Files.setPosixFilePermissions(tempDir.resolve(passwordFilename), p775); - final Map volumes = Map.of(tempDir, Path.of("/run/secrets")); + final Map volumes = singletonMap(tempDir, Paths.get("/run/secrets")); // Restart the container final Result dockerLogs = runContainerExpectingFailure(distribution(), volumes, envVars); @@ -494,7 +487,7 @@ public void test086EnvironmentVariablesAreRespectedUnderDockerExec() { // tool in question is only in the default distribution. assumeTrue(distribution.isDefault()); - runContainer(distribution(), null, Collections.singletonMap("http.host", "this.is.not.valid")); + runContainer(distribution(), null, singletonMap("http.host", "this.is.not.valid")); // This will fail if the env var above is passed as a -E argument final Result result = sh.runIgnoreExitCode("elasticsearch-setup-passwords auto"); From c165741663eebc16d835269c81b3142f1e350098 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 16 Jan 2020 15:36:52 +0000 Subject: [PATCH 3/4] Another JDK version fix --- .../test/java/org/elasticsearch/packaging/test/DockerTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 24c53fa21097b..b08fcb5e76273 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 @@ -354,7 +354,7 @@ public void test082SymlinksAreFollowedWithEnvironmentVariableFiles() throws Exce final String symlinkFilename = "password_symlink"; // ELASTIC_PASSWORD_FILE - Files.writeString(tempDir.resolve(passwordFilename), xpackPassword + "\n"); + Files.write(tempDir.resolve(passwordFilename), (xpackPassword + "\n").getBytes()); // Link to the password file. We can't use an absolute path for the target, because // it won't resolve inside the container. From debd7d73c22ff9b17f6384623659f5a044395673 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 16 Jan 2020 15:56:37 +0000 Subject: [PATCH 4/4] Fix forbidden APIs --- .../java/org/elasticsearch/packaging/test/DockerTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 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 b08fcb5e76273..93c8ba5b9db76 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 @@ -57,6 +57,7 @@ import static org.junit.Assume.assumeTrue; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -354,7 +355,7 @@ public void test082SymlinksAreFollowedWithEnvironmentVariableFiles() throws Exce final String symlinkFilename = "password_symlink"; // ELASTIC_PASSWORD_FILE - Files.write(tempDir.resolve(passwordFilename), (xpackPassword + "\n").getBytes()); + Files.write(tempDir.resolve(passwordFilename), (xpackPassword + "\n").getBytes(StandardCharsets.UTF_8)); // Link to the password file. We can't use an absolute path for the target, because // it won't resolve inside the container. @@ -447,7 +448,7 @@ public void test085SymlinkToFileWithInvalidPermissionsIsRejected() throws Except final String symlinkFilename = "password_symlink"; // ELASTIC_PASSWORD_FILE - Files.write(tempDir.resolve(passwordFilename), (xpackPassword + "\n").getBytes()); + Files.write(tempDir.resolve(passwordFilename), (xpackPassword + "\n").getBytes(StandardCharsets.UTF_8)); // Link to the password file. We can't use an absolute path for the target, because // it won't resolve inside the container.