From b75b030d8808049912e3ecb440a4413ceb633b98 Mon Sep 17 00:00:00 2001 From: Ignacio Vera Date: Mon, 27 Sep 2021 11:10:14 +0200 Subject: [PATCH] don't use endianness reverse util for writing KeyStore file (#78304) The different endianness of different versions is handled when reading files. --- .../cli/keystore/KeyStoreWrapperTests.java | 69 +++++++++----- .../keystore/UpgradeKeyStoreCommandTests.java | 19 ++-- .../format-v4-elasticsearch.keystore | Bin 0 -> 259 bytes .../common/settings/KeyStoreWrapper.java | 84 +++++++++++------- 4 files changed, 113 insertions(+), 59 deletions(-) create mode 100644 distribution/tools/keystore-cli/src/test/resources/format-v4-elasticsearch.keystore diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/KeyStoreWrapperTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/KeyStoreWrapperTests.java index 1c58e9bbd92c6..10fc229ac53e7 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/KeyStoreWrapperTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/KeyStoreWrapperTests.java @@ -10,7 +10,6 @@ import org.apache.lucene.backward_codecs.store.EndiannessReverserUtil; import org.apache.lucene.codecs.CodecUtil; -import org.apache.lucene.store.DataOutput; import org.apache.lucene.store.Directory; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexOutput; @@ -204,9 +203,9 @@ public void testFailWhenCannotConsumeSecretStream() throws Exception { Path configDir = env.configFile(); try ( Directory directory = newFSDirectory(configDir); - IndexOutput indexOutput = directory.createOutput("elasticsearch.keystore", IOContext.DEFAULT) + IndexOutput indexOutput = EndiannessReverserUtil.createOutput(directory, "elasticsearch.keystore", IOContext.DEFAULT) ) { - CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", 3); + CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", KeyStoreWrapper.V3_VERSION); indexOutput.writeByte((byte) 0); // No password SecureRandom random = Randomness.createSecure(); byte[] salt = new byte[64]; @@ -235,19 +234,19 @@ public void testFailWhenCannotConsumeEncryptedBytesStream() throws Exception { Path configDir = env.configFile(); try ( Directory directory = newFSDirectory(configDir); - IndexOutput indexOutput = directory.createOutput("elasticsearch.keystore", IOContext.DEFAULT) + IndexOutput indexOutput = EndiannessReverserUtil.createOutput(directory, "elasticsearch.keystore", IOContext.DEFAULT) ) { - CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", 3); + CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", KeyStoreWrapper.V3_VERSION); indexOutput.writeByte((byte) 0); // No password SecureRandom random = Randomness.createSecure(); byte[] salt = new byte[64]; random.nextBytes(salt); byte[] iv = new byte[12]; random.nextBytes(iv); + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); CipherOutputStream cipherStream = getCipherStream(bytes, salt, iv); DataOutputStream output = new DataOutputStream(cipherStream); - possiblyAlterSecretString(output, 0); cipherStream.close(); final byte[] encryptedBytes = bytes.toByteArray(); @@ -259,7 +258,7 @@ public void testFailWhenCannotConsumeEncryptedBytesStream() throws Exception { KeyStoreWrapper keystore = KeyStoreWrapper.load(configDir); SecurityException e = expectThrows(SecurityException.class, () -> keystore.decrypt(new char[0])); assertThat(e.getMessage(), containsString("Keystore has been corrupted or tampered with")); - assertThat(e.getCause(), instanceOf(EOFException.class)); + assertThat(e.getCause(), instanceOf(ArrayIndexOutOfBoundsException.class)); } public void testFailWhenSecretStreamNotConsumed() throws Exception { @@ -267,9 +266,9 @@ public void testFailWhenSecretStreamNotConsumed() throws Exception { Path configDir = env.configFile(); try ( Directory directory = newFSDirectory(configDir); - IndexOutput indexOutput = directory.createOutput("elasticsearch.keystore", IOContext.DEFAULT) + IndexOutput indexOutput = EndiannessReverserUtil.createOutput(directory, "elasticsearch.keystore", IOContext.DEFAULT) ) { - CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", 3); + CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", KeyStoreWrapper.V3_VERSION); indexOutput.writeByte((byte) 0); // No password SecureRandom random = Randomness.createSecure(); byte[] salt = new byte[64]; @@ -297,9 +296,9 @@ public void testFailWhenEncryptedBytesStreamIsNotConsumed() throws Exception { Path configDir = env.configFile(); try ( Directory directory = newFSDirectory(configDir); - IndexOutput indexOutput = directory.createOutput("elasticsearch.keystore", IOContext.DEFAULT) + IndexOutput indexOutput = EndiannessReverserUtil.createOutput(directory, "elasticsearch.keystore", IOContext.DEFAULT) ) { - CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", 3); + CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", KeyStoreWrapper.V3_VERSION); indexOutput.writeByte((byte) 0); // No password SecureRandom random = Randomness.createSecure(); byte[] salt = new byte[64]; @@ -342,14 +341,8 @@ private void possiblyAlterSecretString(DataOutputStream output, int truncLength) output.write(secret_value); } - private void possiblyAlterEncryptedBytes( - IndexOutput indexOutput, - byte[] salt, - byte[] iv, - byte[] encryptedBytes, - int truncEncryptedDataLength - ) throws Exception { - DataOutput out = EndiannessReverserUtil.wrapDataOutput(indexOutput); + private void possiblyAlterEncryptedBytes(IndexOutput out, byte[] salt, byte[] iv, byte[] encryptedBytes, int truncEncryptedDataLength) + throws Exception { out.writeInt(4 + salt.length + 4 + iv.length + 4 + encryptedBytes.length); out.writeInt(salt.length); out.writeBytes(salt, salt.length); @@ -424,7 +417,7 @@ public void testBackcompatV2() throws Exception { Directory directory = newFSDirectory(configDir); IndexOutput output = EndiannessReverserUtil.createOutput(directory, "elasticsearch.keystore", IOContext.DEFAULT); ) { - CodecUtil.writeHeader(output, "elasticsearch.keystore", 2); + CodecUtil.writeHeader(output, "elasticsearch.keystore", KeyStoreWrapper.V2_VERSION); output.writeByte((byte) 0); // hasPassword = false output.writeString("PKCS12"); output.writeString("PBE"); // string algo @@ -474,6 +467,42 @@ public void testBackcompatV2() throws Exception { } } + public void testBackcompatV4() throws Exception { + assumeFalse("Can't run in a FIPS JVM as PBE is not available", inFipsJvm()); + Path configDir = env.configFile(); + try ( + Directory directory = newFSDirectory(configDir); + IndexOutput indexOutput = EndiannessReverserUtil.createOutput(directory, "elasticsearch.keystore", IOContext.DEFAULT) + ) { + CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", KeyStoreWrapper.V4_VERSION); + indexOutput.writeByte((byte) 0); // No password + SecureRandom random = Randomness.createSecure(); + byte[] salt = new byte[64]; + random.nextBytes(salt); + byte[] iv = new byte[12]; + random.nextBytes(iv); + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + CipherOutputStream cipherStream = getCipherStream(bytes, salt, iv); + DataOutputStream output = new DataOutputStream(cipherStream); + { + byte[] secret_value = "super_secret_value".getBytes(StandardCharsets.UTF_8); + output.writeInt(1); // One entry + output.writeUTF("string_setting"); + output.writeInt(secret_value.length); + output.write(secret_value); + } + cipherStream.close(); + final byte[] encryptedBytes = bytes.toByteArray(); + possiblyAlterEncryptedBytes(indexOutput, salt, iv, encryptedBytes, 0); + CodecUtil.writeFooter(indexOutput); + } + + KeyStoreWrapper keystore = KeyStoreWrapper.load(configDir); + keystore.decrypt(new char[0]); + SecureString testValue = keystore.getString("string_setting"); + assertThat(testValue.toString(), equalTo("super_secret_value")); + } + public void testStringAndFileDistinction() throws Exception { final char[] password = getPossibleKeystorePassword(); final KeyStoreWrapper wrapper = KeyStoreWrapper.create(); diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/UpgradeKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/UpgradeKeyStoreCommandTests.java index 5c0138fac8bad..755c753d8db07 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/UpgradeKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/UpgradeKeyStoreCommandTests.java @@ -38,23 +38,28 @@ protected Environment createEnv(final Map settings) { }; } - public void testKeystoreUpgrade() throws Exception { + public void testKeystoreUpgradeV3() throws Exception { + assertKeystoreUpgrade("/format-v3-elasticsearch.keystore", KeyStoreWrapper.V3_VERSION); + } + + public void testKeystoreUpgradeV4() throws Exception { + assertKeystoreUpgrade("/format-v4-elasticsearch.keystore", KeyStoreWrapper.V4_VERSION); + } + + private void assertKeystoreUpgrade(String file, int version) throws Exception { assumeFalse("Cannot open unprotected keystore on FIPS JVM", inFipsJvm()); final Path keystore = KeyStoreWrapper.keystorePath(env.configFile()); - try ( - InputStream is = KeyStoreWrapperTests.class.getResourceAsStream("/format-v3-elasticsearch.keystore"); - OutputStream os = Files.newOutputStream(keystore) - ) { + try (InputStream is = KeyStoreWrapperTests.class.getResourceAsStream(file); OutputStream os = Files.newOutputStream(keystore)) { is.transferTo(os); } try (KeyStoreWrapper beforeUpgrade = KeyStoreWrapper.load(env.configFile())) { assertNotNull(beforeUpgrade); - assertThat(beforeUpgrade.getFormatVersion(), equalTo(3)); + assertThat(beforeUpgrade.getFormatVersion(), equalTo(version)); } execute(); try (KeyStoreWrapper afterUpgrade = KeyStoreWrapper.load(env.configFile())) { assertNotNull(afterUpgrade); - assertThat(afterUpgrade.getFormatVersion(), equalTo(KeyStoreWrapper.FORMAT_VERSION)); + assertThat(afterUpgrade.getFormatVersion(), equalTo(KeyStoreWrapper.CURRENT_VERSION)); afterUpgrade.decrypt(new char[0]); assertThat(afterUpgrade.getSettingNames(), hasItem(KeyStoreWrapper.SEED_SETTING.getKey())); } diff --git a/distribution/tools/keystore-cli/src/test/resources/format-v4-elasticsearch.keystore b/distribution/tools/keystore-cli/src/test/resources/format-v4-elasticsearch.keystore new file mode 100644 index 0000000000000000000000000000000000000000..e534175cc42b0d160941cd068ecc039363b76036 GIT binary patch literal 259 zcmcD&o+B=nnv+;ul9^nbnpl*ap_iRnSzMA|l*+)szyib!=YiNE#i3N{#>U*)od+I& z>vFs|l_}?z$g3;=cTFrVX4sM0?E1_nkipzwTTgWFnO97O)?O>`8Rgo4Z}!XD^LpO< z()); addBootstrapSeed(wrapper); return wrapper; @@ -234,10 +240,10 @@ public static KeyStoreWrapper load(Path configDir) throws IOException { } Directory directory = new NIOFSDirectory(configDir); - try (ChecksumIndexInput input = EndiannessReverserUtil.openChecksumInput(directory, KEYSTORE_FILENAME, IOContext.READONCE)) { + try (ChecksumIndexInput input = directory.openChecksumInput(KEYSTORE_FILENAME, IOContext.READONCE)) { final int formatVersion; try { - formatVersion = CodecUtil.checkHeader(input, KEYSTORE_FILENAME, MIN_FORMAT_VERSION, FORMAT_VERSION); + formatVersion = CodecUtil.checkHeader(input, KEYSTORE_FILENAME, MIN_FORMAT_VERSION, CURRENT_VERSION); } catch (IndexFormatTooOldException e) { throw new IllegalStateException("The Elasticsearch keystore [" + keystoreFile + "] format is too old. " + "You should delete and recreate it in order to upgrade.", e); @@ -252,7 +258,7 @@ public static KeyStoreWrapper load(Path configDir) throws IOException { + String.format(Locale.ROOT, "%02x", hasPasswordByte)); } - if (formatVersion <= 2) { + if (formatVersion <= V2_VERSION) { String type = input.readString(); if (type.equals("PKCS12") == false) { throw new IllegalStateException("Corrupted legacy keystore string encryption algorithm"); @@ -262,7 +268,7 @@ public static KeyStoreWrapper load(Path configDir) throws IOException { if (stringKeyAlgo.equals("PBE") == false) { throw new IllegalStateException("Corrupted legacy keystore string encryption algorithm"); } - if (formatVersion == 2) { + if (formatVersion == V2_VERSION) { final String fileKeyAlgo = input.readString(); if (fileKeyAlgo.equals("PBE") == false) { throw new IllegalStateException("Corrupted legacy keystore file encryption algorithm"); @@ -271,7 +277,7 @@ public static KeyStoreWrapper load(Path configDir) throws IOException { } final byte[] dataBytes; - if (formatVersion == 2) { + if (formatVersion == V2_VERSION) { // 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 @@ -284,14 +290,24 @@ public static KeyStoreWrapper load(Path configDir) throws IOException { output.writeUTF(entry.getKey()); output.writeUTF(entry.getValue()); } - int keystoreLen = input.readInt(); + final int keystoreLen; + if (formatVersion < LE_VERSION) { + keystoreLen = Integer.reverseBytes(input.readInt()); + } else { + keystoreLen = input.readInt(); + } byte[] keystoreBytes = new byte[keystoreLen]; input.readBytes(keystoreBytes, 0, keystoreLen); output.write(keystoreBytes); } dataBytes = bytes.toByteArray(); } else { - int dataBytesLen = input.readInt(); + int dataBytesLen; + if (formatVersion < LE_VERSION) { + dataBytesLen = Integer.reverseBytes(input.readInt()); + } else { + dataBytesLen = input.readInt(); + } dataBytes = new byte[dataBytesLen]; input.readBytes(dataBytes, 0, dataBytesLen); } @@ -303,7 +319,7 @@ public static KeyStoreWrapper load(Path configDir) throws IOException { /** Upgrades the format of the keystore, if necessary. */ public static void upgrade(KeyStoreWrapper wrapper, Path configDir, char[] password) throws Exception { - if (wrapper.getFormatVersion() == FORMAT_VERSION && wrapper.getSettingNames().contains(SEED_SETTING.getKey())) { + if (wrapper.getFormatVersion() == CURRENT_VERSION && wrapper.getSettingNames().contains(SEED_SETTING.getKey())) { return; } // add keystore.seed if necessary @@ -352,7 +368,7 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio if (entries.get() != null) { throw new IllegalStateException("Keystore has already been decrypted"); } - if (formatVersion <= 2) { + if (formatVersion <= V2_VERSION) { decryptLegacyEntries(); if (password.length != 0) { throw new IllegalArgumentException("Keystore format does not accept non-empty passwords"); @@ -363,21 +379,18 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio 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]; - input.readFully(salt); - int ivLen = input.readInt(); - iv = new byte[ivLen]; - input.readFully(iv); - int encryptedLen = input.readInt(); - encryptedBytes = new byte[encryptedLen]; - input.readFully(encryptedBytes); - if (input.read() != -1) { + try { + final ByteArrayDataInput input = new ByteArrayDataInput(dataBytes); + // Wrap the DataInput for old version that are written in BE + final DataInput maybeWrappedInput = formatVersion < LE_VERSION ? EndiannessReverserUtil.wrapDataInput(input) : input; + salt = readByteArray(maybeWrappedInput); + iv = readByteArray(maybeWrappedInput); + encryptedBytes = readByteArray(maybeWrappedInput); + // check we read all the buffer + if (input.eof() == false) { throw new SecurityException("Keystore has been corrupted or tampered with"); } - } catch (EOFException e) { + } catch (ArrayIndexOutOfBoundsException e) { throw new SecurityException("Keystore has been corrupted or tampered with", e); } @@ -389,7 +402,7 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio int numEntries = input.readInt(); while (numEntries-- > 0) { String setting = input.readUTF(); - if (formatVersion == 3) { + if (formatVersion == V3_VERSION) { // legacy, the keystore format would previously store the entry type input.readUTF(); } @@ -409,6 +422,13 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio } } + private byte[] readByteArray(DataInput input) throws IOException { + final int len = input.readInt(); + final byte[] b = new byte[len]; + input.readBytes(b, 0, len); + return b; + } + /** Encrypt the keystore entries and return the encrypted data. */ private byte[] encrypt(char[] password, byte[] salt, byte[] iv) throws GeneralSecurityException, IOException { assert isLoaded(); @@ -435,7 +455,7 @@ private void decryptLegacyEntries() throws GeneralSecurityException, IOException ByteArrayInputStream inputBytes = new ByteArrayInputStream(dataBytes); try (DataInputStream input = new DataInputStream(inputBytes)) { // first read the setting types map - if (formatVersion == 2) { + if (formatVersion == V2_VERSION) { int numSettings = input.readInt(); for (int i = 0; i < numSettings; ++i) { String key = input.readUTF(); @@ -449,7 +469,7 @@ private void decryptLegacyEntries() throws GeneralSecurityException, IOException // verify the settings metadata matches the keystore entries Enumeration aliases = keystore.aliases(); - if (formatVersion == 1) { + if (formatVersion == MIN_FORMAT_VERSION) { while (aliases.hasMoreElements()) { settingTypes.put(aliases.nextElement(), EntryType.STRING); } @@ -513,8 +533,8 @@ public synchronized void save(Path configDir, char[] password, boolean preserveP // write to tmp file first, then overwrite String tmpFile = KEYSTORE_FILENAME + ".tmp"; Path keystoreTempFile = configDir.resolve(tmpFile); - try (IndexOutput output = EndiannessReverserUtil.createOutput(directory, tmpFile, IOContext.DEFAULT)) { - CodecUtil.writeHeader(output, KEYSTORE_FILENAME, FORMAT_VERSION); + try (IndexOutput output = directory.createOutput(tmpFile, IOContext.DEFAULT)) { + CodecUtil.writeHeader(output, KEYSTORE_FILENAME, CURRENT_VERSION); output.writeByte(password.length == 0 ? (byte)0 : (byte)1); // new cipher params