Skip to content

Commit

Permalink
Follow symlinks in Docker entrypoint (elastic#50927)
Browse files Browse the repository at this point in the history
Closes elastic#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 and SivagurunathanV committed Jan 21, 2020
1 parent d96755c commit 059e4d2
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 11 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
103 changes: 96 additions & 7 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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<String, String> 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<Path, Path> 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
Expand Down Expand Up @@ -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
Expand All @@ -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<String, String> 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<Path, Path> 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());
Expand All @@ -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"));
}

/**
Expand Down

0 comments on commit 059e4d2

Please sign in to comment.