From ce7ebb2d3905220ef3a9e72202fa8d5c7df1a4f5 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Fri, 21 Feb 2020 19:36:15 +0000 Subject: [PATCH] Limit _FILE env var support to specific vars (#52645) Backport of #52525. Closes #52503. Implement a list of `_FILE` env vars that will be used to populate env vars with file content, instead of processing all `_FILE` vars in the environment. --- .../src/bin/elasticsearch-env-from-file | 8 ++- .../packaging/test/DockerTests.java | 66 +++++-------------- .../test/KeystoreManagementTests.java | 43 +++++++++++- 3 files changed, 66 insertions(+), 51 deletions(-) diff --git a/distribution/src/bin/elasticsearch-env-from-file b/distribution/src/bin/elasticsearch-env-from-file index d2cca3d729951..c34e9cafb9e45 100644 --- a/distribution/src/bin/elasticsearch-env-from-file +++ b/distribution/src/bin/elasticsearch-env-from-file @@ -7,11 +7,15 @@ set -e -o pipefail # point to it. This can be used to provide secrets to a container, without # the values being specified explicitly when running the container. # +# Note that only supported environment variables are processed, in order +# to avoid unexpected failures when an environment sets a "*_FILE" variable +# that doesn't contain a filename. +# # This script is intended to be sourced, not executed, and modifies the # environment. -for VAR_NAME_FILE in $(env | cut -f1 -d= | grep '_FILE$'); do - if [[ -n "$VAR_NAME_FILE" ]]; then +for VAR_NAME_FILE in ELASTIC_PASSWORD_FILE KEYSTORE_PASSWORD_FILE ; do + if [[ -n "${!VAR_NAME_FILE}" ]]; then VAR_NAME="${VAR_NAME_FILE%_FILE}" if env | grep "^${VAR_NAME}="; then 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 7f9b0c8a9da2c..f008cef336fa8 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,7 +57,6 @@ import static org.elasticsearch.packaging.util.FileUtils.append; import static org.elasticsearch.packaging.util.FileUtils.getTempDir; import static org.elasticsearch.packaging.util.FileUtils.rm; -import static org.elasticsearch.packaging.util.ServerUtils.makeRequest; import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.emptyString; @@ -219,38 +218,10 @@ public void test071BindMountCustomPathWithDifferentUID() throws Exception { }); } - /** - * Check that environment variables can be populated by setting variables with the suffix "_FILE", - * which point to files that hold the required values. - */ - public void test080SetEnvironmentVariablesUsingFiles() throws Exception { - final String optionsFilename = "esJavaOpts.txt"; - - // ES_JAVA_OPTS_FILE - append(tempDir.resolve(optionsFilename), "-XX:-UseCompressedOops\n"); - - Map envVars = singletonMap("ES_JAVA_OPTS_FILE", "/run/secrets/" + optionsFilename); - - // File permissions need to be secured in order for the ES wrapper to accept - // them for populating env var values - Files.setPosixFilePermissions(tempDir.resolve(optionsFilename), p600); - - final Map volumes = singletonMap(tempDir, Paths.get("/run/secrets")); - - // Restart the container - runContainer(distribution(), volumes, envVars); - - waitForElasticsearch(installation); - - final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes")); - - assertThat(nodesResponse, containsString("\"using_compressed_ordinary_object_pointers\":\"false\"")); - } - /** * Check that the elastic user's password can be configured via a file and the ELASTIC_PASSWORD_FILE environment variable. */ - public void test081ConfigurePasswordThroughEnvironmentVariableFile() throws Exception { + public void test080ConfigurePasswordThroughEnvironmentVariableFile() throws Exception { // Test relies on configuring security assumeTrue(distribution.isDefault()); @@ -293,7 +264,7 @@ public void test081ConfigurePasswordThroughEnvironmentVariableFile() throws Exce * Check that when verifying the file permissions of _FILE environment variables, symlinks * are followed. */ - public void test082SymlinksAreFollowedWithEnvironmentVariableFiles() throws Exception { + public void test081SymlinksAreFollowedWithEnvironmentVariableFiles() throws Exception { // Test relies on configuring security assumeTrue(distribution.isDefault()); // Test relies on symlinks @@ -330,19 +301,18 @@ public void test082SymlinksAreFollowedWithEnvironmentVariableFiles() throws Exce /** * Check that environment variables cannot be used with _FILE environment variables. */ - public void test083CannotUseEnvVarsAndFiles() throws Exception { - final String optionsFilename = "esJavaOpts.txt"; + public void test082CannotUseEnvVarsAndFiles() throws Exception { + final String passwordFilename = "password.txt"; - // ES_JAVA_OPTS_FILE - append(tempDir.resolve(optionsFilename), "-XX:-UseCompressedOops\n"); + Files.write(tempDir.resolve(passwordFilename), "other_hunter2\n".getBytes(StandardCharsets.UTF_8)); Map envVars = new HashMap<>(); - envVars.put("ES_JAVA_OPTS", "-XX:+UseCompressedOops"); - envVars.put("ES_JAVA_OPTS_FILE", "/run/secrets/" + optionsFilename); + envVars.put("ELASTIC_PASSWORD", "hunter2"); + envVars.put("ELASTIC_PASSWORD_FILE", "/run/secrets/" + passwordFilename); // File permissions need to be secured in order for the ES wrapper to accept // them for populating env var values - Files.setPosixFilePermissions(tempDir.resolve(optionsFilename), p600); + Files.setPosixFilePermissions(tempDir.resolve(passwordFilename), p600); final Map volumes = singletonMap(tempDir, Paths.get("/run/secrets")); @@ -350,7 +320,7 @@ public void test083CannotUseEnvVarsAndFiles() throws Exception { assertThat( dockerLogs.stderr, - containsString("ERROR: Both ES_JAVA_OPTS_FILE and ES_JAVA_OPTS are set. These are mutually " + "exclusive.") + containsString("ERROR: Both ELASTIC_PASSWORD_FILE and ELASTIC_PASSWORD are set. These are mutually exclusive.") ); } @@ -358,16 +328,16 @@ public void test083CannotUseEnvVarsAndFiles() throws Exception { * Check that when populating environment variables by setting variables with the suffix "_FILE", * the files' permissions are checked. */ - public void test084EnvironmentVariablesUsingFilesHaveCorrectPermissions() throws Exception { - final String optionsFilename = "esJavaOpts.txt"; + public void test083EnvironmentVariablesUsingFilesHaveCorrectPermissions() throws Exception { + final String passwordFilename = "password.txt"; - // ES_JAVA_OPTS_FILE - append(tempDir.resolve(optionsFilename), "-XX:-UseCompressedOops\n"); + Files.write(tempDir.resolve(passwordFilename), "hunter2\n".getBytes(StandardCharsets.UTF_8)); - Map envVars = singletonMap("ES_JAVA_OPTS_FILE", "/run/secrets/" + optionsFilename); + Map envVars = new HashMap<>(); + envVars.put("ELASTIC_PASSWORD_FILE", "/run/secrets/" + passwordFilename); // Set invalid file permissions - Files.setPosixFilePermissions(tempDir.resolve(optionsFilename), p660); + Files.setPosixFilePermissions(tempDir.resolve(passwordFilename), p660); final Map volumes = singletonMap(tempDir, Paths.get("/run/secrets")); @@ -377,7 +347,7 @@ public void test084EnvironmentVariablesUsingFilesHaveCorrectPermissions() throws assertThat( dockerLogs.stderr, containsString( - "ERROR: File /run/secrets/" + optionsFilename + " from ES_JAVA_OPTS_FILE must have " + "file permissions 400 or 600" + "ERROR: File /run/secrets/" + passwordFilename + " from ELASTIC_PASSWORD_FILE must have file permissions 400 or 600" ) ); } @@ -386,7 +356,7 @@ public void test084EnvironmentVariablesUsingFilesHaveCorrectPermissions() 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 { + public void test084SymlinkToFileWithInvalidPermissionsIsRejected() throws Exception { // Test relies on configuring security assumeTrue(distribution.isDefault()); // Test relies on symlinks @@ -432,7 +402,7 @@ public void test085SymlinkToFileWithInvalidPermissionsIsRejected() throws Except * 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 test086EnvironmentVariablesAreRespectedUnderDockerExec() { + public void test085EnvironmentVariablesAreRespectedUnderDockerExec() { // 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()); 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 a77f00fa9bae7..f9e020fff1666 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 @@ -49,6 +49,7 @@ import static org.elasticsearch.packaging.util.Docker.waitForPathToExist; import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File; import static org.elasticsearch.packaging.util.FileMatcher.file; +import static org.elasticsearch.packaging.util.FileMatcher.p600; import static org.elasticsearch.packaging.util.FileMatcher.p660; import static org.elasticsearch.packaging.util.FileUtils.getTempDir; import static org.elasticsearch.packaging.util.FileUtils.rm; @@ -309,11 +310,51 @@ public void test60DockerEnvironmentVariablePassword() throws Exception { ServerUtils.runElasticsearchTests(); } + /** + * Check that we can mount a password-protected keystore to a docker image + * and provide a password via a file, pointed at from an environment variable. + */ + public void test61DockerEnvironmentVariablePasswordFromFile() throws Exception { + assumeTrue(distribution().isDocker()); + + Path tempDir = null; + try { + tempDir = Files.createTempDirectory(getTempDir(), DockerTests.class.getSimpleName()); + + String password = "password"; + String passwordFilename = "password.txt"; + Files.write(tempDir.resolve(passwordFilename), (password + "\n").getBytes(StandardCharsets.UTF_8)); + Files.setPosixFilePermissions(tempDir.resolve(passwordFilename), p600); + + Path dockerKeystore = installation.config("elasticsearch.keystore"); + + Path localKeystoreFile = getKeystoreFileFromDockerContainer(password, dockerKeystore); + + // restart ES with password and mounted keystore + Map volumes = new HashMap<>(); + volumes.put(localKeystoreFile, dockerKeystore); + volumes.put(tempDir, Paths.get("/run/secrets")); + + Map envVars = new HashMap<>(); + envVars.put("KEYSTORE_PASSWORD_FILE", "/run/secrets/" + passwordFilename); + + runContainer(distribution(), volumes, envVars); + + waitForElasticsearch(installation); + ServerUtils.runElasticsearchTests(); + } + finally { + if (tempDir != null) { + rm(tempDir); + } + } + } + /** * Check that if we provide the wrong password for a mounted and password-protected * keystore, Elasticsearch doesn't start. */ - public void test61DockerEnvironmentVariableBadPassword() throws Exception { + public void test62DockerEnvironmentVariableBadPassword() throws Exception { assumeTrue(distribution().isDocker()); String password = "password"; Path dockerKeystore = installation.config("elasticsearch.keystore");