diff --git a/core/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java new file mode 100644 index 00000000000..5ccac9a2ac3 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java @@ -0,0 +1,100 @@ +/* + * 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.common.settings; + +import java.io.BufferedReader; +import java.io.File; +import java.io.InputStream; +import java.io.InputStreamReader; +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.List; + +import joptsimple.OptionSet; +import joptsimple.OptionSpec; +import org.elasticsearch.cli.EnvironmentAwareCommand; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.common.io.PathUtils; +import org.elasticsearch.env.Environment; + +/** + * A subcommand for the keystore cli which adds a file setting. + */ +class AddFileKeyStoreCommand extends EnvironmentAwareCommand { + + private final OptionSpec forceOption; + private final OptionSpec arguments; + + AddFileKeyStoreCommand() { + super("Add a file setting to the keystore"); + this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), "Overwrite existing setting without prompting"); + // jopt simple has issue with multiple non options, so we just get one set of them here + // and convert to File when necessary + // see https://github.com/jopt-simple/jopt-simple/issues/103 + this.arguments = parser.nonOptions("setting [filepath]"); + } + + @Override + protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); + if (keystore == null) { + throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); + } + + keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); + + List argumentValues = arguments.values(options); + if (argumentValues.size() == 0) { + throw new UserException(ExitCodes.USAGE, "Missing setting name"); + } + String setting = argumentValues.get(0); + if (keystore.getSettingNames().contains(setting) && options.has(forceOption) == false) { + if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { + terminal.println("Exiting without modifying keystore."); + return; + } + } + + if (argumentValues.size() == 1) { + throw new UserException(ExitCodes.USAGE, "Missing file name"); + } + Path file = getPath(argumentValues.get(1)); + if (Files.exists(file) == false) { + throw new UserException(ExitCodes.IO_ERROR, "File [" + file.toString() + "] does not exist"); + } + if (argumentValues.size() > 2) { + throw new UserException(ExitCodes.USAGE, "Unrecognized extra arguments [" + + String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); + } + keystore.setFile(setting, Files.readAllBytes(file)); + keystore.save(env.configFile()); + } + + @SuppressForbidden(reason="file arg for cli") + private Path getPath(String file) { + return PathUtils.get(file); + } +} diff --git a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java index 5bded392fdb..c2345f2ddd8 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java +++ b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java @@ -32,6 +32,7 @@ public class KeyStoreCli extends MultiCommand { subcommands.put("create", new CreateKeyStoreCommand()); subcommands.put("list", new ListKeyStoreCommand()); subcommands.put("add", new AddStringKeyStoreCommand()); + subcommands.put("add-file", new AddStringKeyStoreCommand()); subcommands.put("remove", new RemoveSettingKeyStoreCommand()); } diff --git a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java index e4dd982512d..338987cc714 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -25,7 +25,6 @@ import javax.crypto.spec.PBEKeySpec; import javax.security.auth.DestroyFailedException; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; -import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.nio.CharBuffer; @@ -41,10 +40,14 @@ import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.util.Arrays; +import java.util.Base64; import java.util.Enumeration; +import java.util.HashMap; import java.util.HashSet; import java.util.Locale; +import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import org.apache.lucene.codecs.CodecUtil; import org.apache.lucene.store.BufferedChecksumIndexInput; @@ -54,7 +57,6 @@ import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.IndexOutput; import org.apache.lucene.store.SimpleFSDirectory; import org.apache.lucene.util.SetOnce; -import org.elasticsearch.ElasticsearchException; /** * A wrapper around a Java KeyStore which provides supplements the keystore with extra metadata. @@ -67,29 +69,52 @@ import org.elasticsearch.ElasticsearchException; */ public class KeyStoreWrapper implements SecureSettings { + /** An identifier for the type of data that may be stored in a keystore entry. */ + private enum KeyType { + STRING, + FILE + } + /** The name of the keystore file to read and write. */ private static final String KEYSTORE_FILENAME = "elasticsearch.keystore"; /** The version of the metadata written before the keystore data. */ - private static final int FORMAT_VERSION = 1; + private static final int FORMAT_VERSION = 2; + + /** The oldest metadata format version that can be read. */ + private static final int MIN_FORMAT_VERSION = 1; /** The keystore type for a newly created keystore. */ private static final String NEW_KEYSTORE_TYPE = "PKCS12"; - /** The algorithm used to store password for a newly created keystore. */ - private static final String NEW_KEYSTORE_SECRET_KEY_ALGO = "PBE";//"PBEWithHmacSHA256AndAES_128"; + /** The algorithm used to store string setting contents. */ + private static final String NEW_KEYSTORE_STRING_KEY_ALGO = "PBE"; + + /** The algorithm used to store file setting contents. */ + private static final String NEW_KEYSTORE_FILE_KEY_ALGO = "PBE"; /** An encoder to check whether string values are ascii. */ private static final CharsetEncoder ASCII_ENCODER = StandardCharsets.US_ASCII.newEncoder(); + /** The metadata format version used to read the current keystore wrapper. */ + private final int formatVersion; + /** True iff the keystore has a password needed to read. */ private final boolean hasPassword; /** The type of the keystore, as passed to {@link java.security.KeyStore#getInstance(String)} */ private final String type; - /** A factory necessary for constructing instances of secrets in a {@link KeyStore}. */ - private final SecretKeyFactory secretFactory; + /** A factory necessary for constructing instances of string secrets in a {@link KeyStore}. */ + private final SecretKeyFactory stringFactory; + + /** A factory necessary for constructing instances of file secrets in a {@link KeyStore}. */ + private final SecretKeyFactory fileFactory; + + /** + * The settings that exist in the keystore, mapped to their type of data. + */ + private final Map settingTypes; /** The raw bytes of the encrypted keystore. */ private final byte[] keystoreBytes; @@ -100,17 +125,19 @@ public class KeyStoreWrapper implements SecureSettings { /** The password for the keystore. See {@link #decrypt(char[])}. */ private final SetOnce keystorePassword = new SetOnce<>(); - /** The setting names contained in the loaded keystore. */ - private final Set settingNames = new HashSet<>(); - - private KeyStoreWrapper(boolean hasPassword, String type, String secretKeyAlgo, byte[] keystoreBytes) { + private KeyStoreWrapper(int formatVersion, boolean hasPassword, String type, + String stringKeyAlgo, String fileKeyAlgo, + Map settingTypes, byte[] keystoreBytes) { + this.formatVersion = formatVersion; this.hasPassword = hasPassword; this.type = type; try { - secretFactory = SecretKeyFactory.getInstance(secretKeyAlgo); + stringFactory = SecretKeyFactory.getInstance(stringKeyAlgo); + fileFactory = SecretKeyFactory.getInstance(fileKeyAlgo); } catch (NoSuchAlgorithmException e) { throw new RuntimeException(e); } + this.settingTypes = settingTypes; this.keystoreBytes = keystoreBytes; } @@ -121,7 +148,8 @@ public class KeyStoreWrapper implements SecureSettings { /** Constructs a new keystore with the given password. */ static KeyStoreWrapper create(char[] password) throws Exception { - KeyStoreWrapper wrapper = new KeyStoreWrapper(password.length != 0, NEW_KEYSTORE_TYPE, NEW_KEYSTORE_SECRET_KEY_ALGO, null); + KeyStoreWrapper wrapper = new KeyStoreWrapper(FORMAT_VERSION, password.length != 0, NEW_KEYSTORE_TYPE, + NEW_KEYSTORE_STRING_KEY_ALGO, NEW_KEYSTORE_FILE_KEY_ALGO, new HashMap<>(), null); KeyStore keyStore = KeyStore.getInstance(NEW_KEYSTORE_TYPE); keyStore.load(null, null); wrapper.keystore.set(keyStore); @@ -144,7 +172,7 @@ public class KeyStoreWrapper implements SecureSettings { SimpleFSDirectory directory = new SimpleFSDirectory(configDir); try (IndexInput indexInput = directory.openInput(KEYSTORE_FILENAME, IOContext.READONCE)) { ChecksumIndexInput input = new BufferedChecksumIndexInput(indexInput); - CodecUtil.checkHeader(input, KEYSTORE_FILENAME, FORMAT_VERSION, FORMAT_VERSION); + int formatVersion = CodecUtil.checkHeader(input, KEYSTORE_FILENAME, MIN_FORMAT_VERSION, FORMAT_VERSION); byte hasPasswordByte = input.readByte(); boolean hasPassword = hasPasswordByte == 1; if (hasPassword == false && hasPasswordByte != 0) { @@ -152,11 +180,25 @@ public class KeyStoreWrapper implements SecureSettings { + String.format(Locale.ROOT, "%02x", hasPasswordByte)); } String type = input.readString(); - String secretKeyAlgo = input.readString(); + String stringKeyAlgo = input.readString(); + final String fileKeyAlgo; + if (formatVersion >= 2) { + fileKeyAlgo = input.readString(); + } else { + fileKeyAlgo = NEW_KEYSTORE_FILE_KEY_ALGO; + } + final Map settingTypes; + if (formatVersion >= 2) { + settingTypes = input.readMapOfStrings().entrySet().stream().collect(Collectors.toMap( + Map.Entry::getKey, + e -> KeyType.valueOf(e.getValue()))); + } else { + settingTypes = new HashMap<>(); + } byte[] keystoreBytes = new byte[input.readInt()]; input.readBytes(keystoreBytes, 0, keystoreBytes.length); CodecUtil.checkFooter(input); - return new KeyStoreWrapper(hasPassword, type, secretKeyAlgo, keystoreBytes); + return new KeyStoreWrapper(formatVersion, hasPassword, type, stringKeyAlgo, fileKeyAlgo, settingTypes, keystoreBytes); } } @@ -189,10 +231,24 @@ public class KeyStoreWrapper implements SecureSettings { keystorePassword.set(new KeyStore.PasswordProtection(password)); Arrays.fill(password, '\0'); - // convert keystore aliases enum into a set for easy lookup + Enumeration aliases = keystore.get().aliases(); - while (aliases.hasMoreElements()) { - settingNames.add(aliases.nextElement()); + if (formatVersion == 1) { + while (aliases.hasMoreElements()) { + settingTypes.put(aliases.nextElement(), KeyType.STRING); + } + } else { + // verify integrity: keys in keystore match what the metadata thinks exist + Set expectedSettings = new HashSet<>(settingTypes.keySet()); + while (aliases.hasMoreElements()) { + String settingName = aliases.nextElement(); + if (expectedSettings.remove(settingName) == false) { + throw new SecurityException("Keystore has been corrupted or tampered with"); + } + } + if (expectedSettings.isEmpty() == false) { + throw new SecurityException("Keystore has been corrupted or tampered with"); + } } } @@ -206,8 +262,19 @@ public class KeyStoreWrapper implements SecureSettings { try (IndexOutput output = directory.createOutput(tmpFile, IOContext.DEFAULT)) { CodecUtil.writeHeader(output, KEYSTORE_FILENAME, FORMAT_VERSION); output.writeByte(password.length == 0 ? (byte)0 : (byte)1); - output.writeString(type); - output.writeString(secretFactory.getAlgorithm()); + output.writeString(NEW_KEYSTORE_TYPE); + output.writeString(NEW_KEYSTORE_STRING_KEY_ALGO); + output.writeString(NEW_KEYSTORE_FILE_KEY_ALGO); + output.writeMapOfStrings(settingTypes.entrySet().stream().collect(Collectors.toMap( + Map.Entry::getKey, + e -> e.getValue().name()))); + + // TODO: in the future if we ever change any algorithms used above, we need + // to create a new KeyStore here instead of using the existing one, so that + // the encoded material inside the keystore is updated + assert type.equals(NEW_KEYSTORE_TYPE) : "keystore type changed"; + assert stringFactory.getAlgorithm().equals(NEW_KEYSTORE_STRING_KEY_ALGO) : "string pbe algo changed"; + assert fileFactory.getAlgorithm().equals(NEW_KEYSTORE_FILE_KEY_ALGO) : "file pbe algo changed"; ByteArrayOutputStream keystoreBytesStream = new ByteArrayOutputStream(); keystore.get().store(keystoreBytesStream, password); @@ -228,25 +295,51 @@ public class KeyStoreWrapper implements SecureSettings { @Override public Set getSettingNames() { - return settingNames; + return settingTypes.keySet(); } // TODO: make settings accessible only to code that registered the setting - /** Retrieve a string setting. The {@link SecureString} should be closed once it is used. */ @Override public SecureString getString(String setting) throws GeneralSecurityException { KeyStore.Entry entry = keystore.get().getEntry(setting, keystorePassword.get()); - if (entry instanceof KeyStore.SecretKeyEntry == false) { + if (settingTypes.get(setting) != KeyType.STRING || + entry instanceof KeyStore.SecretKeyEntry == false) { throw new IllegalStateException("Secret setting " + setting + " is not a string"); } // TODO: only allow getting a setting once? KeyStore.SecretKeyEntry secretKeyEntry = (KeyStore.SecretKeyEntry) entry; - PBEKeySpec keySpec = (PBEKeySpec) secretFactory.getKeySpec(secretKeyEntry.getSecretKey(), PBEKeySpec.class); + PBEKeySpec keySpec = (PBEKeySpec) stringFactory.getKeySpec(secretKeyEntry.getSecretKey(), PBEKeySpec.class); SecureString value = new SecureString(keySpec.getPassword()); keySpec.clearPassword(); return value; } + @Override + public InputStream getFile(String setting) throws GeneralSecurityException { + KeyStore.Entry entry = keystore.get().getEntry(setting, keystorePassword.get()); + if (settingTypes.get(setting) != KeyType.FILE || + entry instanceof KeyStore.SecretKeyEntry == false) { + throw new IllegalStateException("Secret setting " + setting + " is not a file"); + } + KeyStore.SecretKeyEntry secretKeyEntry = (KeyStore.SecretKeyEntry) entry; + PBEKeySpec keySpec = (PBEKeySpec) fileFactory.getKeySpec(secretKeyEntry.getSecretKey(), PBEKeySpec.class); + // The PBE keyspec gives us chars, we first convert to bytes, then decode base64 inline. + char[] chars = keySpec.getPassword(); + byte[] bytes = new byte[chars.length]; + for (int i = 0; i < bytes.length; ++i) { + bytes[i] = (byte)chars[i]; // PBE only stores the lower 8 bits, so this narrowing is ok + } + keySpec.clearPassword(); // wipe the original copy + InputStream bytesStream = new ByteArrayInputStream(bytes) { + @Override + public void close() throws IOException { + super.close(); + Arrays.fill(bytes, (byte)0); // wipe our second copy when the stream is exhausted + } + }; + return Base64.getDecoder().wrap(bytesStream); + } + /** * Set a string setting. * @@ -256,15 +349,27 @@ public class KeyStoreWrapper implements SecureSettings { if (ASCII_ENCODER.canEncode(CharBuffer.wrap(value)) == false) { throw new IllegalArgumentException("Value must be ascii"); } - SecretKey secretKey = secretFactory.generateSecret(new PBEKeySpec(value)); + SecretKey secretKey = stringFactory.generateSecret(new PBEKeySpec(value)); keystore.get().setEntry(setting, new KeyStore.SecretKeyEntry(secretKey), keystorePassword.get()); - settingNames.add(setting); + settingTypes.put(setting, KeyType.STRING); + } + + /** Set a file setting. */ + void setFile(String setting, byte[] bytes) throws GeneralSecurityException { + bytes = Base64.getEncoder().encode(bytes); + char[] chars = new char[bytes.length]; + for (int i = 0; i < chars.length; ++i) { + chars[i] = (char)bytes[i]; // PBE only stores the lower 8 bits, so this narrowing is ok + } + SecretKey secretKey = stringFactory.generateSecret(new PBEKeySpec(chars)); + keystore.get().setEntry(setting, new KeyStore.SecretKeyEntry(secretKey), keystorePassword.get()); + settingTypes.put(setting, KeyType.FILE); } /** Remove the given setting from the keystore. */ void remove(String setting) throws KeyStoreException { keystore.get().deleteEntry(setting); - settingNames.remove(setting); + settingTypes.remove(setting); } @Override diff --git a/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index a9e4effb0d9..2efb36696c5 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.settings; +import java.io.InputStream; import java.security.GeneralSecurityException; import java.util.Arrays; import java.util.HashSet; @@ -137,5 +138,26 @@ public abstract class SecureSetting extends Setting { }; } + /** + * A setting which contains a file. Reading the setting opens an input stream to the file. + * + * This may be any sensitive file, e.g. a set of credentials normally in plaintext. + */ + public static Setting secureFile(String name, Setting fallback, + Property... properties) { + return new SecureSetting(name, properties) { + @Override + protected InputStream getSecret(SecureSettings secureSettings) throws GeneralSecurityException { + return secureSettings.getFile(getKey()); + } + @Override + InputStream getFallback(Settings settings) { + if (fallback != null) { + return fallback.get(settings); + } + return null; + } + }; + } } diff --git a/core/src/main/java/org/elasticsearch/common/settings/SecureSettings.java b/core/src/main/java/org/elasticsearch/common/settings/SecureSettings.java index f7098686466..c5a364f5473 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/SecureSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/SecureSettings.java @@ -20,6 +20,7 @@ package org.elasticsearch.common.settings; import java.io.Closeable; +import java.io.InputStream; import java.security.GeneralSecurityException; import java.util.Set; @@ -36,4 +37,7 @@ public interface SecureSettings extends Closeable { /** Return a string setting. The {@link SecureString} should be closed once it is used. */ SecureString getString(String setting) throws GeneralSecurityException; + + /** Return a file setting. The {@link InputStream} should be closed once it is used. */ + InputStream getFile(String setting) throws GeneralSecurityException; } diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index db1cf44db22..374d923d30a 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -1294,6 +1294,11 @@ public final class Settings implements ToXContent { return delegate.getString(keyTransform.apply(setting)); } + @Override + public InputStream getFile(String setting) throws GeneralSecurityException{ + return delegate.getFile(keyTransform.apply(setting)); + } + @Override public void close() throws IOException { delegate.close(); diff --git a/core/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java new file mode 100644 index 00000000000..9044103e43b --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java @@ -0,0 +1,149 @@ +/* + * 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.common.settings; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; + +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +import static org.hamcrest.Matchers.containsString; + +public class AddFileKeyStoreCommandTests extends KeyStoreCommandTestCase { + @Override + protected Command newCommand() { + return new AddFileKeyStoreCommand() { + @Override + protected Environment createEnv(Terminal terminal, Map settings) { + return env; + } + }; + } + + private Path createRandomFile() throws IOException { + int length = randomIntBetween(10, 20); + byte[] bytes = new byte[length]; + for (int i = 0; i < length; ++i) { + bytes[i] = randomByte(); + } + Path file = env.configFile().resolve("randomfile"); + Files.write(file, bytes); + return file; + } + + private void addFile(KeyStoreWrapper keystore, String setting, Path file) throws Exception { + keystore.setFile(setting, Files.readAllBytes(file)); + keystore.save(env.configFile()); + } + + public void testMissing() throws Exception { + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("keystore not found")); + } + + public void testOverwritePromptDefault() throws Exception { + Path file = createRandomFile(); + KeyStoreWrapper keystore = createKeystore(""); + addFile(keystore, "foo", file); + terminal.addTextInput(""); + execute("foo", "path/dne"); + assertSecureFile("foo", file); + } + + public void testOverwritePromptExplicitNo() throws Exception { + Path file = createRandomFile(); + KeyStoreWrapper keystore = createKeystore(""); + addFile(keystore, "foo", file); + terminal.addTextInput("n"); // explicit no + execute("foo", "path/dne"); + assertSecureFile("foo", file); + } + + public void testOverwritePromptExplicitYes() throws Exception { + Path file1 = createRandomFile(); + KeyStoreWrapper keystore = createKeystore(""); + addFile(keystore, "foo", file1); + terminal.addTextInput("y"); + Path file2 = createRandomFile(); + execute("foo", file2.toString()); + assertSecureFile("foo", file2); + } + + public void testOverwriteForceShort() throws Exception { + Path file1 = createRandomFile(); + KeyStoreWrapper keystore = createKeystore(""); + addFile(keystore, "foo", file1); + Path file2 = createRandomFile(); + execute("-f", "foo", file2.toString()); + assertSecureFile("foo", file2); + } + + public void testOverwriteForceLong() throws Exception { + Path file1 = createRandomFile(); + KeyStoreWrapper keystore = createKeystore(""); + addFile(keystore, "foo", file1); + Path file2 = createRandomFile(); + execute("--force", "foo", file2.toString()); + assertSecureFile("foo", file2); + } + + public void testForceNonExistent() throws Exception { + createKeystore(""); + Path file = createRandomFile(); + execute("--force", "foo", file.toString()); + assertSecureFile("foo", file); + } + + public void testMissingSettingName() throws Exception { + createKeystore(""); + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(ExitCodes.USAGE, e.exitCode); + assertThat(e.getMessage(), containsString("Missing setting name")); + } + + public void testMissingFileName() throws Exception { + createKeystore(""); + UserException e = expectThrows(UserException.class, () -> execute("foo")); + assertEquals(ExitCodes.USAGE, e.exitCode); + assertThat(e.getMessage(), containsString("Missing file name")); + } + + public void testFileDNE() throws Exception { + createKeystore(""); + UserException e = expectThrows(UserException.class, () -> execute("foo", "path/dne")); + assertEquals(ExitCodes.IO_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("File [path/dne] does not exist")); + } + + public void testExtraArguments() throws Exception { + createKeystore(""); + Path file = createRandomFile(); + UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString(), "bar")); + assertEquals(e.getMessage(), ExitCodes.USAGE, e.exitCode); + assertThat(e.getMessage(), containsString("Unrecognized extra arguments [bar]")); + } +} diff --git a/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java index ef732c1e29c..11c3f107fe7 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -127,7 +127,7 @@ public class AddStringKeyStoreCommandTests extends KeyStoreCommandTestCase { assertEquals("String value must contain only ASCII", e.getMessage()); } - public void testNpe() throws Exception { + public void testMissingSettingName() throws Exception { createKeystore(""); terminal.addTextInput(""); UserException e = expectThrows(UserException.class, this::execute); diff --git a/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java index 8584d4d1555..5d4741c7291 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java @@ -47,7 +47,7 @@ public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase { } public void testNotPosix() throws Exception { - setupEnv(false); + env = setupEnv(false, fileSystems); execute(); Path configDir = env.configFile(); assertNotNull(KeyStoreWrapper.load(configDir)); diff --git a/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java b/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java index 1e4d24a344e..500b7b627b8 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java +++ b/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java @@ -20,7 +20,9 @@ package org.elasticsearch.common.settings; import java.io.IOException; +import java.io.InputStream; import java.nio.file.FileSystem; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -53,10 +55,10 @@ public abstract class KeyStoreCommandTestCase extends CommandTestCase { @Before public void setupEnv() throws IOException { - setupEnv(true); // default to posix, but tests may call setupEnv(false) to overwrite + env = setupEnv(true, fileSystems); // default to posix, but tests may call setupEnv(false) to overwrite } - void setupEnv(boolean posix) throws IOException { + static Environment setupEnv(boolean posix, List fileSystems) throws IOException { final Configuration configuration; if (posix) { configuration = Configuration.unix().toBuilder().setAttributeViews("basic", "owner", "posix", "unix").build(); @@ -68,7 +70,7 @@ public abstract class KeyStoreCommandTestCase extends CommandTestCase { PathUtilsForTesting.installMock(fs); // restored by restoreFileSystem in ESTestCase Path home = fs.getPath("/", "test-home"); Files.createDirectories(home.resolve("config")); - env = new Environment(Settings.builder().put("path.home", home).build()); + return new Environment(Settings.builder().put("path.home", home).build()); } KeyStoreWrapper createKeystore(String password, String... settings) throws Exception { @@ -94,4 +96,28 @@ public abstract class KeyStoreCommandTestCase extends CommandTestCase { void assertSecureString(KeyStoreWrapper keystore, String setting, String value) throws Exception { assertEquals(value, keystore.getString(setting).toString()); } + + void assertSecureFile(String setting, Path file) throws Exception { + assertSecureFile(loadKeystore(""), setting, file); + } + + void assertSecureFile(KeyStoreWrapper keystore, String setting, Path file) throws Exception { + byte[] expectedBytes = Files.readAllBytes(file); + try (InputStream input = keystore.getFile(setting)) { + for (int i = 0; i < expectedBytes.length; ++i) { + int got = input.read(); + int expected = Byte.toUnsignedInt(expectedBytes[i]); + if (got < 0) { + fail("Got EOF from keystore stream at position " + i + " but expected 0x" + Integer.toHexString(expected)); + } + assertEquals("Byte " + i, expected, got); + } + int eof = input.read(); + if (eof != -1) { + fail("Found extra bytes in file stream from keystore, expected " + expectedBytes.length + + " bytes but found 0x" + Integer.toHexString(eof)); + } + } + + } } diff --git a/core/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java b/core/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java new file mode 100644 index 00000000000..0b42eb59f82 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java @@ -0,0 +1,70 @@ +/* + * 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.common.settings; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.FileSystem; +import java.util.ArrayList; +import java.util.List; + +import org.apache.lucene.util.IOUtils; +import org.elasticsearch.env.Environment; +import org.elasticsearch.test.ESTestCase; +import org.junit.After; +import org.junit.Before; + +public class KeyStoreWrapperTests extends ESTestCase { + + Environment env; + List fileSystems = new ArrayList<>(); + + @After + public void closeMockFileSystems() throws IOException { + IOUtils.close(fileSystems); + } + + @Before + public void setupEnv() throws IOException { + env = KeyStoreCommandTestCase.setupEnv(true, fileSystems); + } + + public void testFileSettingExhaustiveBytes() throws Exception { + KeyStoreWrapper keystore = KeyStoreWrapper.create(new char[0]); + byte[] bytes = new byte[256]; + for (int i = 0; i < 256; ++i) { + bytes[i] = (byte)i; + } + keystore.setFile("foo", bytes); + keystore.save(env.configFile()); + keystore = KeyStoreWrapper.load(env.configFile()); + keystore.decrypt(new char[0]); + try (InputStream stream = keystore.getFile("foo")) { + for (int i = 0; i < 256; ++i) { + int got = stream.read(); + if (got < 0) { + fail("Expected 256 bytes but read " + i); + } + assertEquals(i, got); + } + assertEquals(-1, stream.read()); // nothing left + } + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java b/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java index 81c3b47bde4..21cd1961d7c 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java +++ b/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java @@ -19,8 +19,11 @@ package org.elasticsearch.common.settings; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -30,6 +33,8 @@ import java.util.Set; public class MockSecureSettings implements SecureSettings { private Map secureStrings = new HashMap<>(); + private Map files = new HashMap<>(); + private Set settingNames = new HashSet<>(); @Override public boolean isLoaded() { @@ -38,7 +43,7 @@ public class MockSecureSettings implements SecureSettings { @Override public Set getSettingNames() { - return secureStrings.keySet(); + return settingNames; } @Override @@ -46,8 +51,19 @@ public class MockSecureSettings implements SecureSettings { return secureStrings.get(setting); } + @Override + public InputStream getFile(String setting) { + return new ByteArrayInputStream(files.get(setting)); + } + public void setString(String setting, String value) { secureStrings.put(setting, new SecureString(value.toCharArray())); + settingNames.add(setting); + } + + public void setFile(String setting, byte[] value) { + files.put(setting, value); + settingNames.add(setting); } @Override