From 059e4d2beeac2c1aa7139816c67190fb4b23bfae Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 16 Jan 2020 14:23:15 +0000 Subject: [PATCH] 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 a9fa84962490f..42e4262e4b947 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 @@ -59,6 +59,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; @@ -73,6 +74,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; public class DockerTests extends PackagingTestCase { @@ -334,10 +336,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 @@ -368,7 +411,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 @@ -390,11 +433,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()); @@ -405,10 +497,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")); } /**