Skip to content

Commit

Permalink
Follow symlinks in Docker entrypoint (#51102)
Browse files Browse the repository at this point in the history
Backport of #50927.

Closes #49653. When using _FILE environment variables to supply values
to Elasticsearch, following symlinks when checking that file permissions
are secure.
  • Loading branch information
pugnascotia authored Jan 18, 2020
1 parent e74b271 commit cb43513
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 13 deletions.
12 changes: 8 additions & 4 deletions distribution/src/bin/elasticsearch-env-from-file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
101 changes: 92 additions & 9 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -52,14 +53,15 @@
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;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
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;
Expand Down Expand Up @@ -338,10 +340,48 @@ 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.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.
Files.createSymbolicLink(tempDir.resolve(symlinkFilename), Paths.get(passwordFilename));

// Enable security so that we can test that the password has been used
Map<String, String> 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<Path, Path> 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
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
Expand Down Expand Up @@ -369,7 +409,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
Expand All @@ -393,25 +433,68 @@ 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.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.
Files.createSymbolicLink(tempDir.resolve(symlinkFilename), Paths.get(passwordFilename));

// Enable security so that we can test that the password has been used
Map<String, String> 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<Path, Path> volumes = singletonMap(tempDir, Paths.get("/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());

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");

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"));
}

/**
Expand Down

0 comments on commit cb43513

Please sign in to comment.