diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ForbiddenPatternsTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ForbiddenPatternsTask.java index 7361b78ad072..f858ec26fc15 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ForbiddenPatternsTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ForbiddenPatternsTask.java @@ -66,6 +66,7 @@ public class ForbiddenPatternsTask extends DefaultTask { .exclude("**/*.zip") .exclude("**/*.jks") .exclude("**/*.crt") + .exclude("**/*.keystore") .exclude("**/*.png"); /* diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java index bb2b1df7f8c0..f68a731edf8f 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java @@ -19,27 +19,6 @@ package org.elasticsearch.common.settings; -import javax.crypto.Cipher; -import javax.crypto.CipherOutputStream; -import javax.crypto.SecretKey; -import javax.crypto.SecretKeyFactory; -import javax.crypto.spec.GCMParameterSpec; -import javax.crypto.spec.PBEKeySpec; -import javax.crypto.spec.SecretKeySpec; -import java.io.ByteArrayOutputStream; -import java.io.DataOutputStream; -import java.io.EOFException; -import java.io.IOException; -import java.io.InputStream; -import java.nio.charset.StandardCharsets; -import java.nio.file.FileSystem; -import java.nio.file.Path; -import java.security.KeyStore; -import java.security.SecureRandom; -import java.util.ArrayList; -import java.util.Base64; -import java.util.List; - import org.apache.lucene.codecs.CodecUtil; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexOutput; @@ -52,10 +31,36 @@ import org.hamcrest.Matchers; import org.junit.After; import org.junit.Before; +import javax.crypto.Cipher; +import javax.crypto.CipherOutputStream; +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.PBEKeySpec; +import javax.crypto.spec.SecretKeySpec; + +import java.io.ByteArrayOutputStream; +import java.io.DataOutputStream; +import java.io.EOFException; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.FileSystem; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.security.SecureRandom; +import java.util.ArrayList; +import java.util.Base64; +import java.util.List; +import java.util.Set; + import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.notNullValue; public class KeyStoreWrapperTests extends ESTestCase { @@ -386,4 +391,56 @@ public class KeyStoreWrapperTests extends ESTestCase { assertEquals(-1, fileInput.read()); } } + + public void testStringAndFileDistinction() throws Exception { + final KeyStoreWrapper wrapper = KeyStoreWrapper.create(); + wrapper.setString("string_setting", "string_value".toCharArray()); + final Path temp = createTempDir(); + Files.writeString(temp.resolve("file_setting"), "file_value", StandardCharsets.UTF_8); + wrapper.setFile("file_setting", Files.readAllBytes(temp.resolve("file_setting"))); + wrapper.save(env.configFile(), new char[0]); + wrapper.close(); + + final KeyStoreWrapper afterSave = KeyStoreWrapper.load(env.configFile()); + assertNotNull(afterSave); + afterSave.decrypt(new char[0]); + assertThat(afterSave.getSettingNames(), equalTo(Set.of("keystore.seed", "string_setting", "file_setting"))); + assertThat(afterSave.getString("string_setting"), equalTo("string_value")); + assertThat(toByteArray(afterSave.getFile("string_setting")), equalTo("string_value".getBytes(StandardCharsets.UTF_8))); + assertThat(afterSave.getString("file_setting"), equalTo("file_value")); + assertThat(toByteArray(afterSave.getFile("file_setting")), equalTo("file_value".getBytes(StandardCharsets.UTF_8))); + } + + public void testLegacyV3() throws GeneralSecurityException, IOException { + final Path configDir = createTempDir(); + final Path keystore = configDir.resolve("elasticsearch.keystore"); + try (InputStream is = KeyStoreWrapperTests.class.getResourceAsStream("/format-v3-elasticsearch.keystore"); + OutputStream os = Files.newOutputStream(keystore)) { + final byte[] buffer = new byte[4096]; + int readBytes; + while ((readBytes = is.read(buffer)) > 0) { + os.write(buffer, 0, readBytes); + } + } + final KeyStoreWrapper wrapper = KeyStoreWrapper.load(configDir); + assertNotNull(wrapper); + wrapper.decrypt(new char[0]); + assertThat(wrapper.getFormatVersion(), equalTo(3)); + assertThat(wrapper.getSettingNames(), equalTo(Set.of("keystore.seed", "string_setting", "file_setting"))); + assertThat(wrapper.getString("string_setting"), equalTo("string_value")); + assertThat(toByteArray(wrapper.getFile("string_setting")), equalTo("string_value".getBytes(StandardCharsets.UTF_8))); + assertThat(wrapper.getString("file_setting"), equalTo("file_value")); + assertThat(toByteArray(wrapper.getFile("file_setting")), equalTo("file_value".getBytes(StandardCharsets.UTF_8))); + } + + private byte[] toByteArray(final InputStream is) throws IOException { + final ByteArrayOutputStream os = new ByteArrayOutputStream(); + final byte[] buffer = new byte[1024]; + int readBytes; + while ((readBytes = is.read(buffer)) > 0) { + os.write(buffer, 0, readBytes); + } + return os.toByteArray(); + } + } diff --git a/distribution/tools/keystore-cli/src/test/resources/format-v3-elasticsearch.keystore b/distribution/tools/keystore-cli/src/test/resources/format-v3-elasticsearch.keystore new file mode 100644 index 000000000000..6b845c7e9d6f Binary files /dev/null and b/distribution/tools/keystore-cli/src/test/resources/format-v3-elasticsearch.keystore differ 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 e3fbf30a47ab..2ae90a868af1 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -19,6 +19,18 @@ package org.elasticsearch.common.settings; +import org.apache.lucene.codecs.CodecUtil; +import org.apache.lucene.store.BufferedChecksumIndexInput; +import org.apache.lucene.store.ChecksumIndexInput; +import org.apache.lucene.store.IOContext; +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.cli.ExitCodes; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.common.Randomness; + import javax.crypto.Cipher; import javax.crypto.CipherInputStream; import javax.crypto.CipherOutputStream; @@ -27,6 +39,7 @@ import javax.crypto.SecretKeyFactory; import javax.crypto.spec.GCMParameterSpec; import javax.crypto.spec.PBEKeySpec; import javax.crypto.spec.SecretKeySpec; + import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.DataInputStream; @@ -56,18 +69,6 @@ import java.util.Map; import java.util.Set; import java.util.regex.Pattern; -import org.apache.lucene.codecs.CodecUtil; -import org.apache.lucene.store.BufferedChecksumIndexInput; -import org.apache.lucene.store.ChecksumIndexInput; -import org.apache.lucene.store.IOContext; -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.cli.ExitCodes; -import org.elasticsearch.cli.UserException; -import org.elasticsearch.common.Randomness; - /** * A disk based container for sensitive settings in Elasticsearch. * @@ -84,17 +85,6 @@ public class KeyStoreWrapper implements SecureSettings { 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. */ @@ -110,7 +100,7 @@ public class KeyStoreWrapper implements SecureSettings { private static final String KEYSTORE_FILENAME = "elasticsearch.keystore"; /** The version of the metadata written before the keystore data. */ - private static final int FORMAT_VERSION = 3; + private static final int FORMAT_VERSION = 4; /** The oldest metadata format version that can be read. */ private static final int MIN_FORMAT_VERSION = 1; @@ -146,6 +136,7 @@ public class KeyStoreWrapper implements SecureSettings { // 1: initial version, ES 5.3 // 2: file setting, ES 5.4 // 3: FIPS compliant algos, ES 6.3 + // 4: remove distinction between string/files, ES 6.8/7.1 /** The metadata format version used to read the current keystore wrapper. */ private final int formatVersion; @@ -157,7 +148,7 @@ public class KeyStoreWrapper implements SecureSettings { private final byte[] dataBytes; /** The decrypted secret data. See {@link #decrypt(char[])}. */ - private final SetOnce> entries = new SetOnce<>(); + private final SetOnce> entries = new SetOnce<>(); private volatile boolean closed; private KeyStoreWrapper(int formatVersion, boolean hasPassword, byte[] dataBytes) { @@ -273,11 +264,13 @@ public class KeyStoreWrapper implements SecureSettings { /** Upgrades the format of the keystore, if necessary. */ public static void upgrade(KeyStoreWrapper wrapper, Path configDir, char[] password) throws Exception { - // ensure keystore.seed exists - if (wrapper.getSettingNames().contains(SEED_SETTING.getKey())) { + if (wrapper.getFormatVersion() == FORMAT_VERSION && wrapper.getSettingNames().contains(SEED_SETTING.getKey())) { return; } - addBootstrapSeed(wrapper); + // add keystore.seed if necessary + if (wrapper.getSettingNames().contains(SEED_SETTING.getKey()) == false) { + addBootstrapSeed(wrapper); + } wrapper.save(configDir, password); } @@ -350,11 +343,14 @@ public class KeyStoreWrapper implements SecureSettings { int numEntries = input.readInt(); while (numEntries-- > 0) { String setting = input.readUTF(); - EntryType entryType = EntryType.valueOf(input.readUTF()); + if (formatVersion == 3) { + // legacy, the keystore format would previously store the entry type + input.readUTF(); + } int entrySize = input.readInt(); byte[] entryBytes = new byte[entrySize]; input.readFully(entryBytes); - entries.get().put(setting, new Entry(entryType, entryBytes)); + entries.get().put(setting, entryBytes); } if (input.read() != -1) { throw new SecurityException("Keystore has been corrupted or tampered with"); @@ -373,12 +369,11 @@ public class KeyStoreWrapper implements SecureSettings { try (CipherOutputStream cipherStream = new CipherOutputStream(bytes, cipher); DataOutputStream output = new DataOutputStream(cipherStream)) { output.writeInt(entries.get().size()); - for (Map.Entry mapEntry : entries.get().entrySet()) { + 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); + byte[] entry = mapEntry.getValue(); + output.writeInt(entry.length); + output.write(entry); } } return bytes.toByteArray(); @@ -453,7 +448,7 @@ public class KeyStoreWrapper implements SecureSettings { } Arrays.fill(chars, '\0'); - entries.get().put(setting, new Entry(settingType, bytes)); + entries.get().put(setting, bytes); } } @@ -526,11 +521,8 @@ public class KeyStoreWrapper implements SecureSettings { @Override public synchronized SecureString getString(String setting) { ensureOpen(); - Entry entry = entries.get().get(setting); - if (entry == null || entry.type != EntryType.STRING) { - throw new IllegalArgumentException("Secret setting " + setting + " is not a string"); - } - ByteBuffer byteBuffer = ByteBuffer.wrap(entry.bytes); + byte[] entry = entries.get().get(setting); + ByteBuffer byteBuffer = ByteBuffer.wrap(entry); CharBuffer charBuffer = StandardCharsets.UTF_8.decode(byteBuffer); return new SecureString(Arrays.copyOfRange(charBuffer.array(), charBuffer.position(), charBuffer.limit())); } @@ -538,11 +530,8 @@ public class KeyStoreWrapper implements SecureSettings { @Override public synchronized InputStream getFile(String setting) { ensureOpen(); - Entry entry = entries.get().get(setting); - if (entry == null || entry.type != EntryType.FILE) { - throw new IllegalArgumentException("Secret setting " + setting + " is not a file"); - } - return new ByteArrayInputStream(entry.bytes); + byte[] entry = entries.get().get(setting); + return new ByteArrayInputStream(entry); } /** @@ -564,9 +553,9 @@ public class KeyStoreWrapper implements SecureSettings { 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)); + byte[] oldEntry = entries.get().put(setting, bytes); if (oldEntry != null) { - Arrays.fill(oldEntry.bytes, (byte)0); + Arrays.fill(oldEntry, (byte)0); } } @@ -575,18 +564,18 @@ public class KeyStoreWrapper implements SecureSettings { ensureOpen(); validateSettingName(setting); - Entry oldEntry = entries.get().put(setting, new Entry(EntryType.FILE, Arrays.copyOf(bytes, bytes.length))); + byte[] oldEntry = entries.get().put(setting, Arrays.copyOf(bytes, bytes.length)); if (oldEntry != null) { - Arrays.fill(oldEntry.bytes, (byte)0); + Arrays.fill(oldEntry, (byte)0); } } /** Remove the given setting from the keystore. */ void remove(String setting) { ensureOpen(); - Entry oldEntry = entries.get().remove(setting); + byte[] oldEntry = entries.get().remove(setting); if (oldEntry != null) { - Arrays.fill(oldEntry.bytes, (byte)0); + Arrays.fill(oldEntry, (byte)0); } } @@ -601,8 +590,8 @@ public class KeyStoreWrapper implements SecureSettings { public synchronized void close() { this.closed = true; if (null != entries.get() && entries.get().isEmpty() == false) { - for (Entry entry : entries.get().values()) { - Arrays.fill(entry.bytes, (byte) 0); + for (byte[] entry : entries.get().values()) { + Arrays.fill(entry, (byte) 0); } } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140SecureSettingsBootstrapCheck.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140SecureSettingsBootstrapCheck.java index a8cb32d545e8..82a58b94a83f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140SecureSettingsBootstrapCheck.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140SecureSettingsBootstrapCheck.java @@ -36,7 +36,7 @@ public class FIPS140SecureSettingsBootstrapCheck implements BootstrapCheck { if (fipsModeEnabled) { try (KeyStoreWrapper secureSettings = KeyStoreWrapper.load(environment.configFile())) { if (secureSettings != null && secureSettings.getFormatVersion() < 3) { - return BootstrapCheckResult.failure("Secure settings store is not of the latest version. Please use " + + return BootstrapCheckResult.failure("Secure settings store is not of the appropriate version. Please use " + "bin/elasticsearch-keystore create to generate a new secure settings store and migrate the secure settings there."); } } catch (IOException e) {