From 323f80216d300cbda0fce844dd7e239c7676b8ae Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Tue, 5 Apr 2016 11:46:20 +0200 Subject: [PATCH] Security: Fix systemkey CLI tool When called without arguments, systemkey tool returned with an AIOOE. This fixes the issue, but also ports over the tests to jimfs, so they can actually run, as the security manager is always enabled and thus the tests never ran before. Closes elastic/elasticsearch#1926 Original commit: elastic/x-pack-elasticsearch@887b681607e147c36916c6be44bcc521babd875e --- .../shield/crypto/tool/SystemKeyTool.java | 2 +- .../crypto/tool/SystemKeyToolTests.java | 49 ++++++++++--------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/crypto/tool/SystemKeyTool.java b/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/crypto/tool/SystemKeyTool.java index 83687f4eeb4..8973afe7f22 100644 --- a/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/crypto/tool/SystemKeyTool.java +++ b/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/crypto/tool/SystemKeyTool.java @@ -50,7 +50,7 @@ public class SystemKeyTool extends Command { @Override protected void execute(Terminal terminal, OptionSet options) throws Exception { final Path keyPath; - if (options.has(arguments)) { + if (options.hasArgument(arguments)) { List args = arguments.values(options); if (args.size() > 1) { throw new UserError(ExitCodes.USAGE, "No more than one key path can be supplied"); diff --git a/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/shield/crypto/tool/SystemKeyToolTests.java b/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/shield/crypto/tool/SystemKeyToolTests.java index f7056504463..2d700cccc8d 100644 --- a/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/shield/crypto/tool/SystemKeyToolTests.java +++ b/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/shield/crypto/tool/SystemKeyToolTests.java @@ -5,31 +5,37 @@ */ package org.elasticsearch.shield.crypto.tool; +import com.google.common.jimfs.Configuration; +import com.google.common.jimfs.Jimfs; +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.CommandTestCase; +import org.elasticsearch.common.io.PathUtilsForTesting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; +import org.elasticsearch.shield.crypto.InternalCryptoService; +import org.junit.Before; + +import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFilePermission; import java.util.Set; -import org.elasticsearch.cli.Command; -import org.elasticsearch.cli.CommandTestCase; -import org.elasticsearch.cli.MockTerminal; -import org.elasticsearch.cli.Terminal; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.Environment; -import org.elasticsearch.shield.crypto.InternalCryptoService; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.XPackPlugin; -import org.junit.Before; - -// TODO: use jimfs in these tests so they actually run! public class SystemKeyToolTests extends CommandTestCase { - Settings.Builder settingsBuilder; - Path homeDir; + + private FileSystem jimfs; + private Settings.Builder settingsBuilder; + private Path homeDir; @Before public void init() throws Exception { - homeDir = createTempDir(); + String view = randomFrom("basic", "posix"); + Configuration conf = Configuration.unix().toBuilder().setAttributeViews(view).build(); + jimfs = Jimfs.newFileSystem(conf); + PathUtilsForTesting.installMock(jimfs); + homeDir = jimfs.getPath("eshome"); + settingsBuilder = Settings.builder() .put(Environment.PATH_HOME_SETTING.getKey(), homeDir); } @@ -40,8 +46,8 @@ public class SystemKeyToolTests extends CommandTestCase { } public void testGenerate() throws Exception { - assumeTrue("test cannot run with security manager enabled", System.getSecurityManager() == null); - Path path = createTempDir().resolve("key"); + Path path = jimfs.getPath(randomAsciiOfLength(10)).resolve("key"); + Files.createDirectory(path.getParent()); execute(path.toString()); byte[] bytes = Files.readAllBytes(path); // TODO: maybe we should actually check the key is...i dunno...valid? @@ -54,8 +60,8 @@ public class SystemKeyToolTests extends CommandTestCase { } public void testGeneratePathInSettings() throws Exception { - assumeTrue("test cannot run with security manager enabled", System.getSecurityManager() == null); - Path path = createTempDir().resolve("key"); + Path path = jimfs.getPath(randomAsciiOfLength(10)).resolve("key"); + Files.createDirectories(path.getParent()); settingsBuilder.put("shield.system_key.file", path.toAbsolutePath().toString()); execute(); byte[] bytes = Files.readAllBytes(path); @@ -63,7 +69,6 @@ public class SystemKeyToolTests extends CommandTestCase { } public void testGenerateDefaultPath() throws Exception { - assumeTrue("test cannot run with security manager enabled", System.getSecurityManager() == null); Path keyPath = homeDir.resolve("config/x-pack/system_key"); Files.createDirectories(keyPath.getParent()); execute(); @@ -72,8 +77,8 @@ public class SystemKeyToolTests extends CommandTestCase { } public void testThatSystemKeyMayOnlyBeReadByOwner() throws Exception { - assumeTrue("test cannot run with security manager enabled", System.getSecurityManager() == null); - Path path = createTempDir().resolve("key"); + Path path = jimfs.getPath(randomAsciiOfLength(10)).resolve("key"); + Files.createDirectories(path.getParent()); boolean isPosix = Files.getFileAttributeView(path.getParent(), PosixFileAttributeView.class) != null; assumeTrue("posix filesystem", isPosix);