diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java index 1d399dc850f13..33d29cdaf860a 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java @@ -20,11 +20,7 @@ package org.elasticsearch.plugins; import org.apache.lucene.util.IOUtils; -import org.elasticsearch.Build; -import org.elasticsearch.ElasticsearchCorruptionException; -import org.elasticsearch.ElasticsearchTimeoutException; -import org.elasticsearch.ExceptionsHelper; -import org.elasticsearch.Version; +import org.elasticsearch.*; import org.elasticsearch.bootstrap.JarHell; import org.elasticsearch.common.Strings; import org.elasticsearch.common.cli.Terminal; @@ -40,21 +36,11 @@ import java.io.OutputStream; import java.net.MalformedURLException; import java.net.URL; -import java.nio.file.DirectoryStream; -import java.nio.file.FileVisitResult; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.SimpleFileVisitor; +import java.nio.file.*; import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFilePermission; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Random; -import java.util.Set; +import java.util.*; import java.util.stream.StreamSupport; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -225,7 +211,6 @@ private Path download(PluginHandle pluginHandle, Terminal terminal) throws IOExc } private void extract(PluginHandle pluginHandle, Terminal terminal, Path pluginFile) throws IOException { - // unzip plugin to a staging temp dir, named for the plugin Path tmp = Files.createTempDirectory(environment.tmpFile(), null); Path root = tmp.resolve(pluginHandle.name); @@ -255,22 +240,74 @@ private void extract(PluginHandle pluginHandle, Terminal terminal, Path pluginFi terminal.println("Installed %s into %s", pluginHandle.name, extractLocation.toAbsolutePath()); // cleanup - IOUtils.rm(tmp, pluginFile); + tryToDeletePath(terminal, tmp, pluginFile); // take care of bin/ by moving and applying permissions if needed - Path binFile = extractLocation.resolve("bin"); - if (Files.isDirectory(binFile)) { - Path toLocation = pluginHandle.binDir(environment); - terminal.println(VERBOSE, "Found bin, moving to %s", toLocation.toAbsolutePath()); - if (Files.exists(toLocation)) { - IOUtils.rm(toLocation); + Path sourcePluginBinDirectory = extractLocation.resolve("bin"); + Path destPluginBinDirectory = pluginHandle.binDir(environment); + boolean needToCopyBinDirectory = Files.exists(sourcePluginBinDirectory); + if (needToCopyBinDirectory) { + if (Files.exists(destPluginBinDirectory) && !Files.isDirectory(destPluginBinDirectory)) { + tryToDeletePath(terminal, extractLocation); + throw new IOException("plugin bin directory " + destPluginBinDirectory + " is not a directory"); + } + + try { + copyBinDirectory(sourcePluginBinDirectory, destPluginBinDirectory, pluginHandle.name, terminal); + } catch (IOException e) { + // rollback and remove potentially before installed leftovers + terminal.printError("Error copying bin directory [%s] to [%s], cleaning up, reason: %s", sourcePluginBinDirectory, pluginHandle.binDir(environment), e.getMessage()); + tryToDeletePath(terminal, extractLocation, pluginHandle.binDir(environment)); + throw e; + } + + } + + Path sourceConfigDirectory = extractLocation.resolve("config"); + Path destConfigDirectory = pluginHandle.configDir(environment); + boolean needToCopyConfigDirectory = Files.exists(sourceConfigDirectory); + if (needToCopyConfigDirectory) { + if (Files.exists(destConfigDirectory) && !Files.isDirectory(destConfigDirectory)) { + tryToDeletePath(terminal, extractLocation, pluginHandle.binDir(environment)); + throw new IOException("plugin config directory " + destConfigDirectory + " is not a directory"); } + try { - FileSystemUtils.move(binFile, toLocation); + terminal.println(VERBOSE, "Found config, moving to %s", destConfigDirectory.toAbsolutePath()); + moveFilesWithoutOverwriting(sourceConfigDirectory, destConfigDirectory, ".new"); + terminal.println(VERBOSE, "Installed %s into %s", pluginHandle.name, destConfigDirectory.toAbsolutePath()); } catch (IOException e) { - throw new IOException("Could not move [" + binFile + "] to [" + toLocation + "]", e); + terminal.printError("Error copying config directory [%s] to [%s], cleaning up, reason: %s", sourceConfigDirectory, pluginHandle.binDir(environment), e.getMessage()); + tryToDeletePath(terminal, extractLocation, pluginHandle.binDir(environment), pluginHandle.configDir(environment)); + throw e; } - if (Environment.getFileStore(toLocation).supportsFileAttributeView(PosixFileAttributeView.class)) { + } + } + + private void tryToDeletePath(Terminal terminal, Path ... paths) { + for (Path path : paths) { + try { + IOUtils.rm(path); + } catch (IOException e) { + terminal.printError(e); + } + } + } + + private void copyBinDirectory(Path sourcePluginBinDirectory, Path destPluginBinDirectory, String pluginName, Terminal terminal) throws IOException { + boolean canCopyFromSource = Files.exists(sourcePluginBinDirectory) && Files.isReadable(sourcePluginBinDirectory) && Files.isDirectory(sourcePluginBinDirectory); + if (canCopyFromSource) { + terminal.println(VERBOSE, "Found bin, moving to %s", destPluginBinDirectory.toAbsolutePath()); + if (Files.exists(destPluginBinDirectory)) { + IOUtils.rm(destPluginBinDirectory); + } + try { + Files.createDirectories(destPluginBinDirectory.getParent()); + FileSystemUtils.move(sourcePluginBinDirectory, destPluginBinDirectory); + } catch (IOException e) { + throw new IOException("Could not move [" + sourcePluginBinDirectory + "] to [" + destPluginBinDirectory + "]", e); + } + if (Environment.getFileStore(destPluginBinDirectory).supportsFileAttributeView(PosixFileAttributeView.class)) { // add read and execute permissions to existing perms, so execution will work. // read should generally be set already, but set it anyway: don't rely on umask... final Set executePerms = new HashSet<>(); @@ -280,7 +317,7 @@ private void extract(PluginHandle pluginHandle, Terminal terminal, Path pluginFi executePerms.add(PosixFilePermission.OWNER_EXECUTE); executePerms.add(PosixFilePermission.GROUP_EXECUTE); executePerms.add(PosixFilePermission.OTHERS_EXECUTE); - Files.walkFileTree(toLocation, new SimpleFileVisitor() { + Files.walkFileTree(destPluginBinDirectory, new SimpleFileVisitor() { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { if (attrs.isRegularFile()) { @@ -294,15 +331,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO } else { terminal.println(VERBOSE, "Skipping posix permissions - filestore doesn't support posix permission"); } - terminal.println(VERBOSE, "Installed %s into %s", pluginHandle.name, toLocation.toAbsolutePath()); - } - - Path configFile = extractLocation.resolve("config"); - if (Files.isDirectory(configFile)) { - Path configDestLocation = pluginHandle.configDir(environment); - terminal.println(VERBOSE, "Found config, moving to %s", configDestLocation.toAbsolutePath()); - moveFilesWithoutOverwriting(configFile, configDestLocation, ".new"); - terminal.println(VERBOSE, "Installed %s into %s", pluginHandle.name, configDestLocation.toAbsolutePath()); + terminal.println(VERBOSE, "Installed %s into %s", pluginName, destPluginBinDirectory.toAbsolutePath()); } } diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginManagerPermissionTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginManagerPermissionTests.java new file mode 100644 index 0000000000000..7384c2d992d77 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/plugins/PluginManagerPermissionTests.java @@ -0,0 +1,296 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.plugins; + +import org.apache.lucene.util.LuceneTestCase; +import org.elasticsearch.Version; +import org.elasticsearch.common.cli.CliToolTestCase.CaptureOutputTerminal; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.env.Environment; +import org.elasticsearch.test.ESTestCase; +import org.junit.Before; + +import java.io.IOException; +import java.net.URL; +import java.nio.charset.Charset; +import java.nio.file.*; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static org.elasticsearch.common.settings.Settings.settingsBuilder; +import static org.elasticsearch.plugins.PluginInfoTests.writeProperties; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*; +import static org.hamcrest.Matchers.*; + +// there are some lucene file systems that seem to cause problems (deleted files, dirs instead of files) +@LuceneTestCase.SuppressFileSystems("*") +public class PluginManagerPermissionTests extends ESTestCase { + + private String pluginName = "my-plugin"; + private CaptureOutputTerminal terminal = new CaptureOutputTerminal(); + private Environment environment; + private boolean supportsPermissions; + + @Before + public void setup() { + Path tempDir = createTempDir(); + Settings.Builder settingsBuilder = settingsBuilder().put("path.home", tempDir); + if (randomBoolean()) { + settingsBuilder.put("path.plugins", createTempDir()); + } + + if (randomBoolean()) { + settingsBuilder.put("path.conf", createTempDir()); + } + + environment = new Environment(settingsBuilder.build()); + + supportsPermissions = tempDir.getFileSystem().supportedFileAttributeViews().contains("posix"); + } + + public void testThatUnaccessibleBinDirectoryAbortsPluginInstallation() throws Exception { + assumeTrue("File system does not support permissions, skipping", supportsPermissions); + + URL pluginUrl = createPlugin(true, randomBoolean()); + + Path binPath = environment.binFile().resolve(pluginName); + Files.createDirectories(binPath); + try { + Files.setPosixFilePermissions(binPath, PosixFilePermissions.fromString("---------")); + + PluginManager pluginManager = new PluginManager(environment, pluginUrl, PluginManager.OutputMode.VERBOSE, TimeValue.timeValueSeconds(10)); + pluginManager.downloadAndExtract(pluginName, terminal); + + fail("Expected IOException but did not happen"); + } catch (IOException e) { + assertFileNotExists(environment.pluginsFile().resolve(pluginName)); + assertFileNotExists(environment.configFile().resolve(pluginName)); + // exists, because of our weird permissions above + assertDirectoryExists(environment.binFile().resolve(pluginName)); + + assertThat(terminal.getTerminalOutput(), hasItem(containsString("Error copying bin directory "))); + } finally { + Files.setPosixFilePermissions(binPath, PosixFilePermissions.fromString("rwxrwxrwx")); + } + } + + public void testThatUnaccessiblePluginConfigDirectoryAbortsPluginInstallation() throws Exception { + assumeTrue("File system does not support permissions, skipping", supportsPermissions); + + URL pluginUrl = createPlugin(randomBoolean(), true); + + Path path = environment.configFile().resolve(pluginName); + Files.createDirectories(path); + Files.createFile(path.resolve("my-custom-config.yaml")); + Path binPath = environment.binFile().resolve(pluginName); + Files.createDirectories(binPath); + + try { + Files.setPosixFilePermissions(path.resolve("my-custom-config.yaml"), PosixFilePermissions.fromString("---------")); + Files.setPosixFilePermissions(path, PosixFilePermissions.fromString("---------")); + + PluginManager pluginManager = new PluginManager(environment, pluginUrl, PluginManager.OutputMode.VERBOSE, TimeValue.timeValueSeconds(10)); + pluginManager.downloadAndExtract(pluginName, terminal); + + fail("Expected IOException but did not happen, terminal output was " + terminal.getTerminalOutput()); + } catch (IOException e) { + assertFileNotExists(environment.pluginsFile().resolve(pluginName)); + assertFileNotExists(environment.binFile().resolve(pluginName)); + // exists, because of our weird permissions above + assertDirectoryExists(environment.configFile().resolve(pluginName)); + + assertThat(terminal.getTerminalOutput(), hasItem(containsString("Error copying config directory "))); + } finally { + Files.setPosixFilePermissions(path, PosixFilePermissions.fromString("rwxrwxrwx")); + Files.setPosixFilePermissions(path.resolve("my-custom-config.yaml"), PosixFilePermissions.fromString("rwxrwxrwx")); + } + } + + // config/bin are not writable, but the plugin does not need to put anything into it + public void testThatPluginWithoutBinAndConfigWorksEvenIfPermissionsAreWrong() throws Exception { + assumeTrue("File system does not support permissions, skipping", supportsPermissions); + + URL pluginUrl = createPlugin(false, false); + Path path = environment.configFile().resolve(pluginName); + Files.createDirectories(path); + Files.createFile(path.resolve("my-custom-config.yaml")); + Path binPath = environment.binFile().resolve(pluginName); + Files.createDirectories(binPath); + + try { + Files.setPosixFilePermissions(path.resolve("my-custom-config.yaml"), PosixFilePermissions.fromString("---------")); + Files.setPosixFilePermissions(path, PosixFilePermissions.fromString("---------")); + Files.setPosixFilePermissions(binPath, PosixFilePermissions.fromString("---------")); + + PluginManager pluginManager = new PluginManager(environment, pluginUrl, PluginManager.OutputMode.VERBOSE, TimeValue.timeValueSeconds(10)); + pluginManager.downloadAndExtract(pluginName, terminal); + } finally { + Files.setPosixFilePermissions(binPath, PosixFilePermissions.fromString("rwxrwxrwx")); + Files.setPosixFilePermissions(path, PosixFilePermissions.fromString("rwxrwxrwx")); + Files.setPosixFilePermissions(path.resolve("my-custom-config.yaml"), PosixFilePermissions.fromString("rwxrwxrwx")); + } + + } + + // plugins directory no accessible, should leave no other left over directories + public void testThatNonWritablePluginsDirectoryLeavesNoLeftOver() throws Exception { + assumeTrue("File system does not support permissions, skipping", supportsPermissions); + + URL pluginUrl = createPlugin(true, true); + Files.createDirectories(environment.pluginsFile()); + + try { + Files.setPosixFilePermissions(environment.pluginsFile(), PosixFilePermissions.fromString("---------")); + PluginManager pluginManager = new PluginManager(environment, pluginUrl, PluginManager.OutputMode.VERBOSE, TimeValue.timeValueSeconds(10)); + try { + pluginManager.downloadAndExtract(pluginName, terminal); + fail("Expected IOException due to read-only plugins/ directory"); + } catch (IOException e) { + assertFileNotExists(environment.binFile().resolve(pluginName)); + assertFileNotExists(environment.configFile().resolve(pluginName)); + + Files.setPosixFilePermissions(environment.pluginsFile(), PosixFilePermissions.fromString("rwxrwxrwx")); + assertDirectoryExists(environment.pluginsFile()); + assertFileNotExists(environment.pluginsFile().resolve(pluginName)); + } + } finally { + Files.setPosixFilePermissions(environment.pluginsFile(), PosixFilePermissions.fromString("rwxrwxrwx")); + } + } + + public void testThatUnwriteableBackupFilesInConfigurationDirectoryAreReplaced() throws Exception { + assumeTrue("File system does not support permissions, skipping", supportsPermissions); + + boolean pluginContainsExecutables = randomBoolean(); + URL pluginUrl = createPlugin(pluginContainsExecutables, true); + Files.createDirectories(environment.configFile().resolve(pluginName)); + + Path configFile = environment.configFile().resolve(pluginName).resolve("my-custom-config.yaml"); + Files.createFile(configFile); + Path backupConfigFile = environment.configFile().resolve(pluginName).resolve("my-custom-config.yaml.new"); + Files.createFile(backupConfigFile); + Files.write(backupConfigFile, "foo".getBytes(Charset.forName("UTF-8"))); + + PluginManager pluginManager = new PluginManager(environment, pluginUrl, PluginManager.OutputMode.VERBOSE, TimeValue.timeValueSeconds(10)); + try { + Files.setPosixFilePermissions(backupConfigFile, PosixFilePermissions.fromString("---------")); + + pluginManager.downloadAndExtract(pluginName, terminal); + + if (pluginContainsExecutables) { + assertDirectoryExists(environment.binFile().resolve(pluginName)); + } + assertDirectoryExists(environment.pluginsFile().resolve(pluginName)); + assertDirectoryExists(environment.configFile().resolve(pluginName)); + + assertFileExists(backupConfigFile); + Files.setPosixFilePermissions(backupConfigFile, PosixFilePermissions.fromString("rw-rw-rw-")); + String content = new String(Files.readAllBytes(backupConfigFile), Charset.forName("UTF-8")); + assertThat(content, is(not("foo"))); + } finally { + Files.setPosixFilePermissions(backupConfigFile, PosixFilePermissions.fromString("rw-rw-rw-")); + } + } + + public void testThatConfigDirectoryBeingAFileAbortsInstallationAndDoesNotAccidentallyDeleteThisFile() throws Exception { + Files.createDirectories(environment.configFile()); + Files.createFile(environment.configFile().resolve(pluginName)); + URL pluginUrl = createPlugin(randomBoolean(), true); + + PluginManager pluginManager = new PluginManager(environment, pluginUrl, PluginManager.OutputMode.VERBOSE, TimeValue.timeValueSeconds(10)); + + try { + pluginManager.downloadAndExtract(pluginName, terminal); + fail("Expected plugin installation to fail, but didnt"); + } catch (IOException e) { + assertFileExists(environment.configFile().resolve(pluginName)); + assertFileNotExists(environment.binFile().resolve(pluginName)); + assertFileNotExists(environment.pluginsFile().resolve(pluginName)); + } + } + + public void testThatBinDirectoryBeingAFileAbortsInstallationAndDoesNotAccidentallyDeleteThisFile() throws Exception { + Files.createDirectories(environment.binFile()); + Files.createFile(environment.binFile().resolve(pluginName)); + URL pluginUrl = createPlugin(true, randomBoolean()); + + PluginManager pluginManager = new PluginManager(environment, pluginUrl, PluginManager.OutputMode.VERBOSE, TimeValue.timeValueSeconds(10)); + + try { + pluginManager.downloadAndExtract(pluginName, terminal); + fail("Expected plugin installation to fail, but didnt"); + } catch (IOException e) { + assertFileExists(environment.binFile().resolve(pluginName)); + assertFileNotExists(environment.configFile().resolve(pluginName)); + assertFileNotExists(environment.pluginsFile().resolve(pluginName)); + } + } + + + private URL createPlugin(boolean withBinDir, boolean withConfigDir) throws IOException { + final Path structure = createTempDir().resolve("fake-plugin"); + writeProperties(structure, "description", "fake desc", + "version", "1.0", + "elasticsearch.version", Version.CURRENT.toString(), + "jvm", "true", + "java.version", "1.7", + "name", pluginName, + "classname", pluginName); + if (withBinDir) { + // create bin dir + Path binDir = structure.resolve("bin"); + Files.createDirectory(binDir); + Files.setPosixFilePermissions(binDir, PosixFilePermissions.fromString("rwxr-xr-x")); + + // create executable + Path executable = binDir.resolve("my-binary"); + Files.createFile(executable); + Files.setPosixFilePermissions(executable, PosixFilePermissions.fromString("rwxr-xr-x")); + } + if (withConfigDir) { + // create bin dir + Path configDir = structure.resolve("config"); + Files.createDirectory(configDir); + Files.setPosixFilePermissions(configDir, PosixFilePermissions.fromString("rwxr-xr-x")); + + // create config file + Path configFile = configDir.resolve("my-custom-config.yaml"); + Files.createFile(configFile); + Files.write(configFile, "my custom config content".getBytes(Charset.forName("UTF-8"))); + Files.setPosixFilePermissions(configFile, PosixFilePermissions.fromString("rw-r--r--")); + } + + Path zip = createTempDir().resolve(structure.getFileName() + ".zip"); + try (ZipOutputStream stream = new ZipOutputStream(Files.newOutputStream(zip))) { + Files.walkFileTree(structure, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + stream.putNextEntry(new ZipEntry(structure.relativize(file).toString())); + Files.copy(file, stream); + return FileVisitResult.CONTINUE; + } + }); + } + return zip.toUri().toURL(); + } +}