From 176a2ff408ac9c49063c2ef7b86a40f6eb197dd4 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 16 Jan 2018 17:21:39 -0800 Subject: [PATCH 1/3] Settings: Reimplement keystore format to use FIPS compliant algorithms This commit switches the internal format of the elasticsearch keystore to no longer use java's KeyStore class, but instead encrypt the binary data of the secrets using AES-GCM. The cipher key is generated using PBKDF2WithHmacSHA512. Tests are also added for backcompat reading the v1 and v2 formats. --- .../plugins/InstallPluginCommand.java | 4 +- .../plugins/InstallPluginCommandTests.java | 4 +- .../elasticsearch/bootstrap/Bootstrap.java | 2 +- .../settings/AddFileKeyStoreCommand.java | 6 +- .../settings/AddStringKeyStoreCommand.java | 6 +- .../settings/CreateKeyStoreCommand.java | 4 +- .../common/settings/KeyStoreWrapper.java | 482 +++++++++++------- .../RemoveSettingKeyStoreCommand.java | 2 +- .../bootstrap/BootstrapTests.java | 4 +- .../settings/AddFileKeyStoreCommandTests.java | 2 +- .../AddStringKeyStoreCommandTests.java | 14 +- .../settings/KeyStoreCommandTestCase.java | 4 +- .../common/settings/KeyStoreWrapperTests.java | 125 ++++- 13 files changed, 441 insertions(+), 218 deletions(-) diff --git a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index a8b7db48a7c1c..75c6a91625595 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -811,8 +811,8 @@ private void createKeystoreIfNeeded(Terminal terminal, Environment env, PluginIn KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); if (keystore == null) { terminal.println("Elasticsearch keystore is required by plugin [" + info.getName() + "], creating..."); - keystore = KeyStoreWrapper.create(new char[0]); - keystore.save(env.configFile()); + keystore = KeyStoreWrapper.create(); + keystore.save(env.configFile(), new char[0]); } } diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java index db5a6a16a9d34..837aedf939d29 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java @@ -1138,8 +1138,8 @@ public void testKeystoreNotRequired() throws Exception { public void testKeystoreRequiredAlreadyExists() throws Exception { Tuple env = createEnv(fs, temp); - KeyStoreWrapper keystore = KeyStoreWrapper.create(new char[0]); - keystore.save(env.v2().configFile()); + KeyStoreWrapper keystore = KeyStoreWrapper.create(); + keystore.save(env.v2().configFile(), new char[0]); byte[] expectedBytes = Files.readAllBytes(KeyStoreWrapper.keystorePath(env.v2().configFile())); Path pluginDir = createPluginDir(temp); String pluginZip = createPluginUrl("fake", pluginDir, "requires.keystore", "true"); diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 2f86489bce39f..0cbbd119e4853 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -232,7 +232,7 @@ static SecureSettings loadSecureSettings(Environment initialEnv) throws Bootstra try { keystore.decrypt(new char[0] /* TODO: read password from stdin */); - KeyStoreWrapper.upgrade(keystore, initialEnv.configFile()); + KeyStoreWrapper.upgrade(keystore, initialEnv.configFile(), new char[0]); } catch (Exception e) { throw new BootstrapException(e); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java index a488d238859aa..ba65f1453ad11 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java @@ -66,8 +66,8 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th terminal.println("Exiting without creating keystore."); return; } - keystore = KeyStoreWrapper.create(new char[0] /* always use empty passphrase for auto created keystore */); - keystore.save(env.configFile()); + keystore = KeyStoreWrapper.create(); + keystore.save(env.configFile(), new char[0] /* always use empty passphrase for auto created keystore */); terminal.println("Created elasticsearch keystore in " + env.configFile()); } else { keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); @@ -97,7 +97,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); } keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile()); + keystore.save(env.configFile(), new char[0]); } @SuppressForbidden(reason="file arg for cli") diff --git a/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index 69a76f0f18fac..ee6614618010b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -63,8 +63,8 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th terminal.println("Exiting without creating keystore."); return; } - keystore = KeyStoreWrapper.create(new char[0] /* always use empty passphrase for auto created keystore */); - keystore.save(env.configFile()); + keystore = KeyStoreWrapper.create(); + keystore.save(env.configFile(), new char[0] /* always use empty passphrase for auto created keystore */); terminal.println("Created elasticsearch keystore in " + env.configFile()); } else { keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); @@ -94,6 +94,6 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } catch (IllegalArgumentException e) { throw new UserException(ExitCodes.DATA_ERROR, "String value must contain only ASCII"); } - keystore.save(env.configFile()); + keystore.save(env.configFile(), new char[0]); } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 08860cb5ea992..3529d7f6810bd 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -54,8 +54,8 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); }*/ - KeyStoreWrapper keystore = KeyStoreWrapper.create(password); - keystore.save(env.configFile()); + KeyStoreWrapper keystore = KeyStoreWrapper.create(); + keystore.save(env.configFile(), password); terminal.println("Created elasticsearch keystore in " + env.configFile()); } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java index 6ebc47c825288..903e712a40c27 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -19,16 +19,22 @@ package org.elasticsearch.common.settings; +import javax.crypto.Cipher; +import javax.crypto.CipherInputStream; +import javax.crypto.CipherOutputStream; import javax.crypto.SecretKey; import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.GCMParameterSpec; import javax.crypto.spec.PBEKeySpec; -import javax.security.auth.DestroyFailedException; +import javax.crypto.spec.SecretKeySpec; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.ByteBuffer; import java.nio.CharBuffer; -import java.nio.charset.CharsetEncoder; import java.nio.charset.StandardCharsets; import java.nio.file.AccessDeniedException; import java.nio.file.Files; @@ -38,8 +44,6 @@ import java.nio.file.attribute.PosixFilePermissions; import java.security.GeneralSecurityException; import java.security.KeyStore; -import java.security.KeyStoreException; -import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.Arrays; import java.util.Base64; @@ -50,7 +54,6 @@ import java.util.Map; import java.util.Set; import java.util.regex.Pattern; -import java.util.stream.Collectors; import org.apache.lucene.codecs.CodecUtil; import org.apache.lucene.store.BufferedChecksumIndexInput; @@ -65,16 +68,32 @@ import org.elasticsearch.common.Randomness; /** - * A wrapper around a Java KeyStore which provides supplements the keystore with extra metadata. + * A disk based container for sensitive settings in Elasticsearch. * * Loading a keystore has 2 phases. First, call {@link #load(Path)}. Then call * {@link #decrypt(char[])} with the keystore password, or an empty char array if * {@link #hasPassword()} is {@code false}. Loading and decrypting should happen - * in a single thread. Once decrypted, keys may be read with the wrapper in - * multiple threads. + * in a single thread. Once decrypted, settings may be read in multiple threads. */ public class KeyStoreWrapper implements SecureSettings { + /** An identifier for the type of data that may be stored in a keystore entry. */ + private enum EntryType { + STRING, + FILE + } + + /** An entry in the keystore. The bytes are opaque and interpreted based on the entry type. */ + private static class Entry { + final EntryType type; + final byte[] bytes; + + Entry(EntryType type, byte[] bytes) { + this.type = type; + this.bytes = bytes; + } + } + /** * A regex for the valid characters that a setting name in the keystore may use. */ @@ -86,76 +105,57 @@ public class KeyStoreWrapper implements SecureSettings { private static final char[] SEED_CHARS = ("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789" + "~!@#$%^&*-_=+?").toCharArray(); - /** 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 = 2; + private static final int FORMAT_VERSION = 3; /** 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 derive the cipher key from a password. */ + private static final String KDF_ALGO = "PBKDF2WithHmacSHA512"; - /** The algorithm used to store string setting contents. */ - private static final String NEW_KEYSTORE_STRING_KEY_ALGO = "PBE"; + /** The number of iterations to derive the cipher key. */ + private static final int KDF_ITERS = 10000; - /** The algorithm used to store file setting contents. */ - private static final String NEW_KEYSTORE_FILE_KEY_ALGO = "PBE"; + /** The number of bits for the cipher key. */ + private static final int CIPHER_KEY_BITS = 256; - /** An encoder to check whether string values are ascii. */ - private static final CharsetEncoder ASCII_ENCODER = StandardCharsets.US_ASCII.newEncoder(); + /** The number of bits for the GCM tag. */ + private static final int GCM_TAG_BITS = 128; - /** The metadata format version used to read the current keystore wrapper. */ - private final int formatVersion; + /** The cipher used to encrypt the keystore data. */ + private static final String CIPHER_ALGO = "AES"; - /** True iff the keystore has a password needed to read. */ - private final boolean hasPassword; + /** The mode used with the cipher algorithm. */ + private static final String CIPHER_MODE = "GCM"; - /** The type of the keystore, as passed to {@link java.security.KeyStore#getInstance(String)} */ - private final String type; + /** The padding used with the cipher algorithm. */ + private static final String CIPHER_PADDING = "NoPadding"; - /** A factory necessary for constructing instances of string secrets in a {@link KeyStore}. */ - private final SecretKeyFactory stringFactory; + // format version changelog: + // 1: initial version, ES 5.3 + // 2: file setting, ES 5.4 + // 3: FIPS compliant algos, ES 6.3 - /** A factory necessary for constructing instances of file secrets in a {@link KeyStore}. */ - private final SecretKeyFactory fileFactory; + /** The metadata format version used to read the current keystore wrapper. */ + private final int formatVersion; - /** - * The settings that exist in the keystore, mapped to their type of data. - */ - private final Map settingTypes; + /** True iff the keystore has a password needed to read. */ + private final boolean hasPassword; /** The raw bytes of the encrypted keystore. */ - private final byte[] keystoreBytes; + private final byte[] dataBytes; - /** The loaded keystore. See {@link #decrypt(char[])}. */ - private final SetOnce keystore = new SetOnce<>(); + /** The decrypted secret data. See {@link #decrypt(char[])}. */ + private final SetOnce> entries = new SetOnce<>(); - /** The password for the keystore. See {@link #decrypt(char[])}. */ - private final SetOnce keystorePassword = new SetOnce<>(); - - private KeyStoreWrapper(int formatVersion, boolean hasPassword, String type, - String stringKeyAlgo, String fileKeyAlgo, - Map settingTypes, byte[] keystoreBytes) { + private KeyStoreWrapper(int formatVersion, boolean hasPassword, byte[] dataBytes) { this.formatVersion = formatVersion; this.hasPassword = hasPassword; - this.type = type; - try { - stringFactory = SecretKeyFactory.getInstance(stringKeyAlgo); - fileFactory = SecretKeyFactory.getInstance(fileKeyAlgo); - } catch (NoSuchAlgorithmException e) { - throw new RuntimeException(e); - } - this.settingTypes = settingTypes; - this.keystoreBytes = keystoreBytes; + this.dataBytes = dataBytes; } /** Returns a path representing the ES keystore in the given config dir. */ @@ -164,19 +164,15 @@ public static Path keystorePath(Path configDir) { } /** Constructs a new keystore with the given password. */ - public static KeyStoreWrapper create(char[] password) throws Exception { - 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); - wrapper.keystorePassword.set(new KeyStore.PasswordProtection(password)); + public static KeyStoreWrapper create() { + KeyStoreWrapper wrapper = new KeyStoreWrapper(FORMAT_VERSION, false, null); + wrapper.entries.set(new HashMap<>()); addBootstrapSeed(wrapper); return wrapper; } /** Add the bootstrap seed setting, which may be used as a unique, secure, random value by the node */ - public static void addBootstrapSeed(KeyStoreWrapper wrapper) throws GeneralSecurityException { + public static void addBootstrapSeed(KeyStoreWrapper wrapper) { assert wrapper.getSettingNames().contains(SEED_SETTING.getKey()) == false; SecureRandom random = Randomness.createSecure(); int passwordLength = 20; // Generate 20 character passwords @@ -210,42 +206,69 @@ public static KeyStoreWrapper load(Path configDir) throws IOException { throw new IllegalStateException("hasPassword boolean is corrupt: " + String.format(Locale.ROOT, "%02x", hasPasswordByte)); } - String type = input.readString(); - String stringKeyAlgo = input.readString(); - final String fileKeyAlgo; - if (formatVersion >= 2) { - fileKeyAlgo = input.readString(); - } else { - fileKeyAlgo = NEW_KEYSTORE_FILE_KEY_ALGO; + + if (formatVersion <= 2) { + String type = input.readString(); + if (type.equals("PKCS12") == false) { + throw new IllegalStateException("Corrupted legacy keystore string encryption algorithm"); + } + + final String stringKeyAlgo = input.readString(); + if (stringKeyAlgo.equals("PBE") == false) { + throw new IllegalStateException("Corrupted legacy keystore string encryption algorithm"); + } + if (formatVersion == 2) { + final String fileKeyAlgo = input.readString(); + if (fileKeyAlgo.equals("PBE") == false) { + throw new IllegalStateException("Corrupted legacy keystore file encryption algorithm"); + } + } } - final Map settingTypes; - if (formatVersion >= 2) { - settingTypes = input.readMapOfStrings().entrySet().stream().collect(Collectors.toMap( - Map.Entry::getKey, - e -> KeyType.valueOf(e.getValue()))); + + final byte[] dataBytes; + if (formatVersion == 2) { + // For v2 we had a map of strings containing the types for each setting. In v3 this map is now + // part of the encrypted bytes. Unfortunately we cannot seek backwards with checksum input, so + // we cannot just read the map and find out how long it is. So instead we read the map and + // store it back using java's builtin DataOutput in a byte array, along with the actual keystore bytes + Map settingTypes = input.readMapOfStrings(); + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (DataOutputStream output = new DataOutputStream(bytes)) { + output.writeInt(settingTypes.size()); + for (Map.Entry entry : settingTypes.entrySet()) { + output.writeUTF(entry.getKey()); + output.writeUTF(entry.getValue()); + } + int keystoreLen = input.readInt(); + byte[] keystoreBytes = new byte[keystoreLen]; + input.readBytes(keystoreBytes, 0, keystoreLen); + output.write(keystoreBytes); + } + dataBytes = bytes.toByteArray(); } else { - settingTypes = new HashMap<>(); + int dataBytesLen = input.readInt(); + dataBytes = new byte[dataBytesLen]; + input.readBytes(dataBytes, 0, dataBytesLen); } - byte[] keystoreBytes = new byte[input.readInt()]; - input.readBytes(keystoreBytes, 0, keystoreBytes.length); + CodecUtil.checkFooter(input); - return new KeyStoreWrapper(formatVersion, hasPassword, type, stringKeyAlgo, fileKeyAlgo, settingTypes, keystoreBytes); + return new KeyStoreWrapper(formatVersion, hasPassword, dataBytes); } } /** Upgrades the format of the keystore, if necessary. */ - public static void upgrade(KeyStoreWrapper wrapper, Path configDir) throws Exception { + public static void upgrade(KeyStoreWrapper wrapper, Path configDir, char[] password) throws Exception { // ensure keystore.seed exists if (wrapper.getSettingNames().contains(SEED_SETTING.getKey())) { return; } addBootstrapSeed(wrapper); - wrapper.save(configDir); + wrapper.save(configDir, password); } @Override public boolean isLoaded() { - return keystore.get() != null; + return entries.get() != null; } /** Return true iff calling {@link #decrypt(char[])} requires a non-empty password. */ @@ -253,28 +276,120 @@ public boolean hasPassword() { return hasPassword; } + private Cipher createCipher(int opmode, char[] password, byte[] salt, byte[] iv) throws GeneralSecurityException { + PBEKeySpec keySpec = new PBEKeySpec(password, salt, KDF_ITERS, CIPHER_KEY_BITS); + SecretKeyFactory keyFactory = SecretKeyFactory.getInstance(KDF_ALGO); + SecretKey secretKey = keyFactory.generateSecret(keySpec); + SecretKeySpec secret = new SecretKeySpec(secretKey.getEncoded(), CIPHER_ALGO); + + GCMParameterSpec spec = new GCMParameterSpec(GCM_TAG_BITS, iv); + Cipher cipher = Cipher.getInstance(CIPHER_ALGO + "/" + CIPHER_MODE + "/" + CIPHER_PADDING); + cipher.init(opmode, secret, spec); + cipher.updateAAD(salt); + return cipher; + } + /** - * Decrypts the underlying java keystore. + * Decrypts the underlying keystore data. * - * This may only be called once. The provided password will be zeroed out. + * This may only be called once. */ public void decrypt(char[] password) throws GeneralSecurityException, IOException { - if (keystore.get() != null) { + if (entries.get() != null) { throw new IllegalStateException("Keystore has already been decrypted"); } - keystore.set(KeyStore.getInstance(type)); - try (InputStream in = new ByteArrayInputStream(keystoreBytes)) { - keystore.get().load(in, password); - } finally { - Arrays.fill(keystoreBytes, (byte)0); + if (formatVersion <= 2) { + decryptLegacyEntries(); + assert password.length == 0; + return; + } + + final byte[] salt; + final byte[] iv; + final byte[] encryptedBytes; + try (ByteArrayInputStream bytesStream = new ByteArrayInputStream(dataBytes); + DataInputStream input = new DataInputStream(bytesStream)) { + int saltLen = input.readInt(); + salt = new byte[saltLen]; + if (input.read(salt) != saltLen) { + throw new SecurityException("Keystore has been corrupted or tampered with"); + } + int ivLen = input.readInt(); + iv = new byte[ivLen]; + if (input.read(iv) != ivLen) { + throw new SecurityException("Keystore has been corrupted or tampered with"); + } + int encryptedLen = input.readInt(); + encryptedBytes = new byte[encryptedLen]; + if (input.read(encryptedBytes) != encryptedLen) { + throw new SecurityException("Keystore has been corrupted or tampered with"); + } + } + + Cipher cipher = createCipher(Cipher.DECRYPT_MODE, password, salt, iv); + try (ByteArrayInputStream bytesStream = new ByteArrayInputStream(encryptedBytes); + CipherInputStream cipherStream = new CipherInputStream(bytesStream, cipher); + DataInputStream input = new DataInputStream(cipherStream)) { + + entries.set(new HashMap<>()); + int numEntries = input.readInt(); + while (numEntries-- > 0) { + String setting = input.readUTF(); + EntryType entryType = EntryType.valueOf(input.readUTF()); + int entrySize = input.readInt(); + byte[] entryBytes = new byte[entrySize]; + if (input.read(entryBytes) != entrySize) { + throw new SecurityException("Keystore has been corrupted or tampered with"); + } + entries.get().put(setting, new Entry(entryType, entryBytes)); + } + } + } + + /** Encrypt the keystore entries and return the encrypted data. */ + private byte[] encrypt(char[] password, byte[] salt, byte[] iv) throws GeneralSecurityException, IOException { + assert isLoaded(); + + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + Cipher cipher = createCipher(Cipher.ENCRYPT_MODE, password, salt, iv); + try (CipherOutputStream cipherStream = new CipherOutputStream(bytes, cipher); + DataOutputStream output = new DataOutputStream(cipherStream)) { + + output.writeInt(entries.get().size()); + for (Map.Entry mapEntry : entries.get().entrySet()) { + output.writeUTF(mapEntry.getKey()); + Entry entry = mapEntry.getValue(); + output.writeUTF(entry.type.name()); + output.writeInt(entry.bytes.length); + output.write(entry.bytes); + } + } + + return bytes.toByteArray(); + } + + private void decryptLegacyEntries() throws GeneralSecurityException, IOException { + // v1 and v2 keystores never had passwords actually used, so we always use an empty password + KeyStore keystore = KeyStore.getInstance("PKCS12"); + Map settingTypes = new HashMap<>(); + ByteArrayInputStream inputBytes = new ByteArrayInputStream(dataBytes); + try (DataInputStream input = new DataInputStream(inputBytes)) { + // first read the setting types map + int numSettings = input.readInt(); + for (int i = 0; i < numSettings; ++i) { + String key = input.readUTF(); + String value = input.readUTF(); + settingTypes.put(key, EntryType.valueOf(value)); + } + // then read the actual keystore + keystore.load(input, "".toCharArray()); } - keystorePassword.set(new KeyStore.PasswordProtection(password)); - Arrays.fill(password, '\0'); - Enumeration aliases = keystore.get().aliases(); + // verify the settings metadata matches the keystore entries + Enumeration aliases = keystore.aliases(); if (formatVersion == 1) { while (aliases.hasMoreElements()) { - settingTypes.put(aliases.nextElement(), KeyType.STRING); + settingTypes.put(aliases.nextElement(), EntryType.STRING); } } else { // verify integrity: keys in keystore match what the metadata thinks exist @@ -289,12 +404,44 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio throw new SecurityException("Keystore has been corrupted or tampered with"); } } + + // fill in the entries now that we know all the types to expect + this.entries.set(new HashMap<>()); + SecretKeyFactory keyFactory = SecretKeyFactory.getInstance("PBE"); + KeyStore.PasswordProtection password = new KeyStore.PasswordProtection("".toCharArray()); + + for (Map.Entry settingEntry : settingTypes.entrySet()) { + String setting = settingEntry.getKey(); + EntryType settingType = settingEntry.getValue(); + KeyStore.SecretKeyEntry keystoreEntry = (KeyStore.SecretKeyEntry) keystore.getEntry(setting, password); + PBEKeySpec keySpec = (PBEKeySpec) keyFactory.getKeySpec(keystoreEntry.getSecretKey(), PBEKeySpec.class); + char[] chars = keySpec.getPassword(); + keySpec.clearPassword(); + + final byte[] bytes; + if (settingType == EntryType.STRING) { + ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(CharBuffer.wrap(chars)); + bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); + Arrays.fill(byteBuffer.array(), (byte)0); + } else { + assert settingType == EntryType.FILE; + // The PBE keyspec gives us chars, we convert to bytes + byte[] tmpBytes = new byte[chars.length]; + for (int i = 0; i < tmpBytes.length; ++i) { + tmpBytes[i] = (byte)chars[i]; // PBE only stores the lower 8 bits, so this narrowing is ok + } + bytes = Base64.getDecoder().decode(tmpBytes); + Arrays.fill(tmpBytes, (byte)0); + } + Arrays.fill(chars, '\0'); + + entries.get().put(setting, new Entry(settingType, bytes)); + } } /** Write the keystore to the given config directory. */ - public void save(Path configDir) throws Exception { + public void save(Path configDir, char[] password) throws Exception { assert isLoaded(); - char[] password = this.keystorePassword.get().getPassword(); SimpleFSDirectory directory = new SimpleFSDirectory(configDir); // write to tmp file first, then overwrite @@ -302,26 +449,32 @@ public void save(Path configDir) throws Exception { 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(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); - byte[] keystoreBytes = keystoreBytesStream.toByteArray(); - output.writeInt(keystoreBytes.length); - output.writeBytes(keystoreBytes, keystoreBytes.length); + + // new cipher params + SecureRandom random = Randomness.createSecure(); + // use 64 bytes salt, which surpasses that recommended by OWASP + // see https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet + byte[] salt = new byte[64]; + random.nextBytes(salt); + // use 96 bits (12 bytes) for IV as recommended by NIST + // see http://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf section 5.2.1.1 + byte[] iv = new byte[12]; + random.nextBytes(iv); + // encrypted data + byte[] encryptedBytes = encrypt(password, salt, iv); + + // size of data block + output.writeInt(4 + salt.length + 4 + iv.length + 4 + encryptedBytes.length); + + output.writeInt(salt.length); + output.writeBytes(salt, salt.length); + output.writeInt(iv.length); + output.writeBytes(iv, iv.length); + output.writeInt(encryptedBytes.length); + output.writeBytes(encryptedBytes, encryptedBytes.length); + CodecUtil.writeFooter(output); + } catch (final AccessDeniedException e) { final String message = String.format( Locale.ROOT, @@ -341,51 +494,32 @@ public void save(Path configDir) throws Exception { @Override public Set getSettingNames() { - return settingTypes.keySet(); + assert isLoaded(); + return entries.get().keySet(); } // TODO: make settings accessible only to code that registered the setting @Override - public SecureString getString(String setting) throws GeneralSecurityException { + public SecureString getString(String setting) { assert isLoaded(); - KeyStore.Entry entry = keystore.get().getEntry(setting, keystorePassword.get()); - if (settingTypes.get(setting) != KeyType.STRING || - entry instanceof KeyStore.SecretKeyEntry == false) { - throw new IllegalStateException("Secret setting " + setting + " is not a string"); + Entry entry = entries.get().get(setting); + if (entry == null || entry.type != EntryType.STRING) { + throw new IllegalArgumentException("Secret setting " + setting + " is not a string"); } - // TODO: only allow getting a setting once? - KeyStore.SecretKeyEntry secretKeyEntry = (KeyStore.SecretKeyEntry) entry; - PBEKeySpec keySpec = (PBEKeySpec) stringFactory.getKeySpec(secretKeyEntry.getSecretKey(), PBEKeySpec.class); - SecureString value = new SecureString(keySpec.getPassword()); - keySpec.clearPassword(); - return value; + ByteBuffer byteBuffer = ByteBuffer.wrap(entry.bytes); + CharBuffer charBuffer = StandardCharsets.UTF_8.decode(byteBuffer); + return new SecureString(charBuffer.array()); } @Override - public InputStream getFile(String setting) throws GeneralSecurityException { + public InputStream getFile(String setting) { assert isLoaded(); - 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"); + Entry entry = entries.get().get(setting); + if (entry == null || entry.type != EntryType.FILE) { + throw new IllegalArgumentException("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); + + return new ByteArrayInputStream(entry.bytes); } /** @@ -400,51 +534,43 @@ public static void validateSettingName(String setting) { } } - /** - * Set a string setting. - * - * @throws IllegalArgumentException if the value is not ASCII - */ - void setString(String setting, char[] value) throws GeneralSecurityException { + /** Set a string setting. */ + void setString(String setting, char[] value) { assert isLoaded(); validateSettingName(setting); - if (ASCII_ENCODER.canEncode(CharBuffer.wrap(value)) == false) { - throw new IllegalArgumentException("Value must be ascii"); + + ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(CharBuffer.wrap(value)); + byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); + Entry oldEntry = entries.get().put(setting, new Entry(EntryType.STRING, bytes)); + if (oldEntry != null) { + Arrays.fill(oldEntry.bytes, (byte)0); } - SecretKey secretKey = stringFactory.generateSecret(new PBEKeySpec(value)); - keystore.get().setEntry(setting, new KeyStore.SecretKeyEntry(secretKey), keystorePassword.get()); - settingTypes.put(setting, KeyType.STRING); } /** Set a file setting. */ - void setFile(String setting, byte[] bytes) throws GeneralSecurityException { + void setFile(String setting, byte[] bytes) { assert isLoaded(); validateSettingName(setting); - 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 + + Entry oldEntry = entries.get().put(setting, new Entry(EntryType.FILE, Arrays.copyOf(bytes, bytes.length))); + if (oldEntry != null) { + Arrays.fill(oldEntry.bytes, (byte)0); } - 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 { + void remove(String setting) { assert isLoaded(); - keystore.get().deleteEntry(setting); - settingTypes.remove(setting); + Entry oldEntry = entries.get().remove(setting); + if (oldEntry != null) { + Arrays.fill(oldEntry.bytes, (byte)0); + } } @Override - public void close() throws IOException { - try { - if (keystorePassword.get() != null) { - keystorePassword.get().destroy(); - } - } catch (DestroyFailedException e) { - throw new IOException(e); + public void close() { + for (Entry entry : entries.get().values()) { + Arrays.fill(entry.bytes, (byte)0); } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java index c766dd769e22b..9a83375e6e01a 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java @@ -61,6 +61,6 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } keystore.remove(setting); } - keystore.save(env.configFile()); + keystore.save(env.configFile(), new char[0]); } } diff --git a/server/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java b/server/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java index 6c390a5a8a7d7..aa8b5d092fafa 100644 --- a/server/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java +++ b/server/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java @@ -53,11 +53,11 @@ public void setupEnv() throws IOException { public void testLoadSecureSettings() throws Exception { final Path configPath = env.configFile(); final SecureString seed; - try (KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.create(new char[0])) { + try (KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.create()) { seed = KeyStoreWrapper.SEED_SETTING.get(Settings.builder().setSecureSettings(keyStoreWrapper).build()); assertNotNull(seed); assertTrue(seed.length() > 0); - keyStoreWrapper.save(configPath); + keyStoreWrapper.save(configPath, new char[0]); } assertTrue(Files.exists(configPath.resolve("elasticsearch.keystore"))); try (SecureSettings secureSettings = Bootstrap.loadSecureSettings(env)) { diff --git a/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java index b562c72011b3c..6f59a17f21562 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java @@ -56,7 +56,7 @@ private Path createRandomFile() throws IOException { private void addFile(KeyStoreWrapper keystore, String setting, Path file) throws Exception { keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile()); + keystore.save(env.configFile(), new char[0]); } public void testMissingPromptCreate() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java index 733832a500f3c..71c1de012b5f2 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -112,34 +112,26 @@ public void testForceNonExistent() throws Exception { } public void testPromptForValue() throws Exception { - KeyStoreWrapper.create(new char[0]).save(env.configFile()); + KeyStoreWrapper.create().save(env.configFile(), new char[0]); terminal.addSecretInput("secret value"); execute("foo"); assertSecureString("foo", "secret value"); } public void testStdinShort() throws Exception { - KeyStoreWrapper.create(new char[0]).save(env.configFile()); + KeyStoreWrapper.create().save(env.configFile(), new char[0]); setInput("secret value 1"); execute("-x", "foo"); assertSecureString("foo", "secret value 1"); } public void testStdinLong() throws Exception { - KeyStoreWrapper.create(new char[0]).save(env.configFile()); + KeyStoreWrapper.create().save(env.configFile(), new char[0]); setInput("secret value 2"); execute("--stdin", "foo"); assertSecureString("foo", "secret value 2"); } - public void testNonAsciiValue() throws Exception { - KeyStoreWrapper.create(new char[0]).save(env.configFile()); - terminal.addSecretInput("non-äsčîï"); - UserException e = expectThrows(UserException.class, () -> execute("foo")); - assertEquals(ExitCodes.DATA_ERROR, e.exitCode); - assertEquals("String value must contain only ASCII", e.getMessage()); - } - public void testMissingSettingName() throws Exception { createKeystore(""); terminal.addTextInput(""); diff --git a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java index c1118b3bc6513..53dbc8589d8d4 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java +++ b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java @@ -74,12 +74,12 @@ public static Environment setupEnv(boolean posix, List fileSystems) } KeyStoreWrapper createKeystore(String password, String... settings) throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.create(password.toCharArray()); + KeyStoreWrapper keystore = KeyStoreWrapper.create(); assertEquals(0, settings.length % 2); for (int i = 0; i < settings.length; i += 2) { keystore.setString(settings[i], settings[i + 1].toCharArray()); } - keystore.save(env.configFile()); + keystore.save(env.configFile(), password.toCharArray()); return keystore; } diff --git a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java index c3b34b7c3eff0..9414931f996e1 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java @@ -19,12 +19,28 @@ package org.elasticsearch.common.settings; +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.CharBuffer; +import java.nio.charset.CharsetEncoder; +import java.nio.charset.StandardCharsets; import java.nio.file.FileSystem; +import java.nio.file.Path; +import java.security.KeyStore; import java.util.ArrayList; +import java.util.Base64; import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.lucene.codecs.CodecUtil; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.IndexOutput; +import org.apache.lucene.store.SimpleFSDirectory; import org.apache.lucene.util.IOUtils; import org.elasticsearch.bootstrap.BootstrapSettings; import org.elasticsearch.env.Environment; @@ -32,6 +48,8 @@ import org.junit.After; import org.junit.Before; +import static org.hamcrest.Matchers.equalTo; + public class KeyStoreWrapperTests extends ESTestCase { Environment env; @@ -48,13 +66,13 @@ public void setupEnv() throws IOException { } public void testFileSettingExhaustiveBytes() throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.create(new char[0]); + KeyStoreWrapper keystore = KeyStoreWrapper.create(); 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.save(env.configFile(), new char[0]); keystore = KeyStoreWrapper.load(env.configFile()); keystore.decrypt(new char[0]); try (InputStream stream = keystore.getFile("foo")) { @@ -70,16 +88,16 @@ public void testFileSettingExhaustiveBytes() throws Exception { } public void testCreate() throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.create(new char[0]); + KeyStoreWrapper keystore = KeyStoreWrapper.create(); assertTrue(keystore.getSettingNames().contains(KeyStoreWrapper.SEED_SETTING.getKey())); } public void testUpgradeNoop() throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.create(new char[0]); + KeyStoreWrapper keystore = KeyStoreWrapper.create(); SecureString seed = keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey()); - keystore.save(env.configFile()); + keystore.save(env.configFile(), new char[0]); // upgrade does not overwrite seed - KeyStoreWrapper.upgrade(keystore, env.configFile()); + KeyStoreWrapper.upgrade(keystore, env.configFile(), new char[0]); assertEquals(seed.toString(), keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey()).toString()); keystore = KeyStoreWrapper.load(env.configFile()); keystore.decrypt(new char[0]); @@ -87,10 +105,10 @@ public void testUpgradeNoop() throws Exception { } public void testUpgradeAddsSeed() throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.create(new char[0]); + KeyStoreWrapper keystore = KeyStoreWrapper.create(); keystore.remove(KeyStoreWrapper.SEED_SETTING.getKey()); - keystore.save(env.configFile()); - KeyStoreWrapper.upgrade(keystore, env.configFile()); + keystore.save(env.configFile(), new char[0]); + KeyStoreWrapper.upgrade(keystore, env.configFile(), new char[0]); SecureString seed = keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey()); assertNotNull(seed); keystore = KeyStoreWrapper.load(env.configFile()); @@ -101,10 +119,97 @@ public void testUpgradeAddsSeed() throws Exception { public void testIllegalSettingName() throws Exception { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> KeyStoreWrapper.validateSettingName("UpperCase")); assertTrue(e.getMessage().contains("does not match the allowed setting name pattern")); - KeyStoreWrapper keystore = KeyStoreWrapper.create(new char[0]); + KeyStoreWrapper keystore = KeyStoreWrapper.create(); e = expectThrows(IllegalArgumentException.class, () -> keystore.setString("UpperCase", new char[0])); assertTrue(e.getMessage().contains("does not match the allowed setting name pattern")); e = expectThrows(IllegalArgumentException.class, () -> keystore.setFile("UpperCase", new byte[0])); assertTrue(e.getMessage().contains("does not match the allowed setting name pattern")); } + + public void testBackcompatV1() throws Exception { + Path configDir = env.configFile(); + SimpleFSDirectory directory = new SimpleFSDirectory(configDir); + try (IndexOutput output = directory.createOutput("elasticsearch.keystore", IOContext.DEFAULT)) { + CodecUtil.writeHeader(output, "elasticsearch.keystore", 1); + output.writeByte((byte) 0); // hasPassword = false + output.writeString("PKCS12"); + output.writeString("PBE"); + + SecretKeyFactory secretFactory = SecretKeyFactory.getInstance("PBE"); + KeyStore keystore = KeyStore.getInstance("PKCS12"); + keystore.load(null, null); + SecretKey secretKey = secretFactory.generateSecret(new PBEKeySpec("stringSecretValue".toCharArray())); + KeyStore.ProtectionParameter protectionParameter = new KeyStore.PasswordProtection(new char[0]); + keystore.setEntry("string_setting", new KeyStore.SecretKeyEntry(secretKey), protectionParameter); + + ByteArrayOutputStream keystoreBytesStream = new ByteArrayOutputStream(); + keystore.store(keystoreBytesStream, new char[0]); + byte[] keystoreBytes = keystoreBytesStream.toByteArray(); + output.writeInt(keystoreBytes.length); + output.writeBytes(keystoreBytes, keystoreBytes.length); + CodecUtil.writeFooter(output); + } + + KeyStoreWrapper keystore = KeyStoreWrapper.load(configDir); + keystore.decrypt(new char[0]); + SecureString testValue = keystore.getString("string_setting"); + assertThat(testValue.toString(), equalTo("stringSecretValue")); + } + + public void testBackcompatV2() throws Exception { + Path configDir = env.configFile(); + SimpleFSDirectory directory = new SimpleFSDirectory(configDir); + byte[] fileBytes = new byte[20]; + random().nextBytes(fileBytes); + try (IndexOutput output = directory.createOutput("elasticsearch.keystore", IOContext.DEFAULT)) { + + CodecUtil.writeHeader(output, "elasticsearch.keystore", 2); + output.writeByte((byte) 0); // hasPassword = false + output.writeString("PKCS12"); + output.writeString("PBE"); // string algo + output.writeString("PBE"); // file algo + + output.writeVInt(2); // num settings + output.writeString("string_setting"); + output.writeString("STRING"); + output.writeString("file_setting"); + output.writeString("FILE"); + + SecretKeyFactory secretFactory = SecretKeyFactory.getInstance("PBE"); + KeyStore keystore = KeyStore.getInstance("PKCS12"); + keystore.load(null, null); + SecretKey secretKey = secretFactory.generateSecret(new PBEKeySpec("stringSecretValue".toCharArray())); + KeyStore.ProtectionParameter protectionParameter = new KeyStore.PasswordProtection(new char[0]); + keystore.setEntry("string_setting", new KeyStore.SecretKeyEntry(secretKey), protectionParameter); + + byte[] base64Bytes = Base64.getEncoder().encode(fileBytes); + char[] chars = new char[base64Bytes.length]; + for (int i = 0; i < chars.length; ++i) { + chars[i] = (char)base64Bytes[i]; // PBE only stores the lower 8 bits, so this narrowing is ok + } + secretKey = secretFactory.generateSecret(new PBEKeySpec(chars)); + keystore.setEntry("file_setting", new KeyStore.SecretKeyEntry(secretKey), protectionParameter); + + ByteArrayOutputStream keystoreBytesStream = new ByteArrayOutputStream(); + keystore.store(keystoreBytesStream, new char[0]); + byte[] keystoreBytes = keystoreBytesStream.toByteArray(); + output.writeInt(keystoreBytes.length); + output.writeBytes(keystoreBytes, keystoreBytes.length); + CodecUtil.writeFooter(output); + } + + KeyStoreWrapper keystore = KeyStoreWrapper.load(configDir); + keystore.decrypt(new char[0]); + SecureString testValue = keystore.getString("string_setting"); + assertThat(testValue.toString(), equalTo("stringSecretValue")); + + try (InputStream fileInput = keystore.getFile("file_setting")) { + byte[] readBytes = new byte[20]; + assertEquals(20, fileInput.read(readBytes)); + for (int i = 0; i < fileBytes.length; ++i) { + assertThat("byte " + i, readBytes[i], equalTo(fileBytes[i])); + } + assertEquals(-1, fileInput.read()); + } + } } From 5103261ce569ea0c487138c19e320512425bb3ed Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 18 Jan 2018 08:43:53 -0800 Subject: [PATCH 2/3] knock down aes bits to 128 --- .../elasticsearch/common/settings/KeyStoreWrapper.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java index 903e712a40c27..d46d57d2d05fa 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -120,8 +120,14 @@ private static class Entry { /** The number of iterations to derive the cipher key. */ private static final int KDF_ITERS = 10000; - /** The number of bits for the cipher key. */ - private static final int CIPHER_KEY_BITS = 256; + /** + * The number of bits for the cipher key. + * + * Note: The Oracle JDK 8 ships with a limited JCE policy that restricts key length for AES to 128 bits. + * This can be increased to 256 bits once minimum java 9 is the minimum java version. + * See http://www.oracle.com/technetwork/java/javase/terms/readme/jdk9-readme-3852447.html#jce + * */ + private static final int CIPHER_KEY_BITS = 128; /** The number of bits for the GCM tag. */ private static final int GCM_TAG_BITS = 128; From f07000de4fa6b186fbebedc1014eeee286a89f18 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 18 Jan 2018 08:53:39 -0800 Subject: [PATCH 3/3] fix v1 backcompat --- .../common/settings/KeyStoreWrapper.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java index d46d57d2d05fa..9b994089be0c2 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -381,11 +381,13 @@ private void decryptLegacyEntries() throws GeneralSecurityException, IOException ByteArrayInputStream inputBytes = new ByteArrayInputStream(dataBytes); try (DataInputStream input = new DataInputStream(inputBytes)) { // first read the setting types map - int numSettings = input.readInt(); - for (int i = 0; i < numSettings; ++i) { - String key = input.readUTF(); - String value = input.readUTF(); - settingTypes.put(key, EntryType.valueOf(value)); + if (formatVersion == 2) { + int numSettings = input.readInt(); + for (int i = 0; i < numSettings; ++i) { + String key = input.readUTF(); + String value = input.readUTF(); + settingTypes.put(key, EntryType.valueOf(value)); + } } // then read the actual keystore keystore.load(input, "".toCharArray());