Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use readFully() to read bytes from CipherInputStream (#28515) #30640

Merged
merged 2 commits into from
May 16, 2018
Merged

Use readFully() to read bytes from CipherInputStream (#28515) #30640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f68eb2
Use readFully() to read bytes from CipherInputStream (#28515)
jkakavas May 4, 2018
8d45e82
Merge remote-tracking branch 'origin/6.x' into keystore-wrapper-6x
jkakavas May 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.io.ByteArrayOutputStream;
import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.EOFException;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
Expand Down Expand Up @@ -318,38 +319,39 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio
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");
}
input.readFully(salt);
int ivLen = input.readInt();
iv = new byte[ivLen];
if (input.read(iv) != ivLen) {
throw new SecurityException("Keystore has been corrupted or tampered with");
}
input.readFully(iv);
int encryptedLen = input.readInt();
encryptedBytes = new byte[encryptedLen];
if (input.read(encryptedBytes) != encryptedLen) {
input.readFully(encryptedBytes);
if (input.read() != -1) {
throw new SecurityException("Keystore has been corrupted or tampered with");
}
} catch (EOFException e) {
throw new SecurityException("Keystore has been corrupted or tampered with", e);
}

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");
}
input.readFully(entryBytes);
entries.get().put(setting, new Entry(entryType, entryBytes));
}
if (input.read() != -1) {
throw new SecurityException("Keystore has been corrupted or tampered with");
}
} catch (EOFException e) {
throw new SecurityException("Keystore has been corrupted or tampered with", e);
}
}

Expand All @@ -361,7 +363,6 @@ private byte[] encrypt(char[] password, byte[] salt, byte[] iv) throws GeneralSe
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<String, Entry> mapEntry : entries.get().entrySet()) {
output.writeUTF(mapEntry.getKey());
Expand All @@ -371,7 +372,6 @@ private byte[] encrypt(char[] password, byte[] salt, byte[] iv) throws GeneralSe
output.write(entry.bytes);
}
}

return bytes.toByteArray();
}

Expand Down
156 changes: 151 additions & 5 deletions server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,33 @@

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.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.security.SecureRandom;
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.elasticsearch.common.Randomness;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.bootstrap.BootstrapSettings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
import org.hamcrest.Matchers;
Expand Down Expand Up @@ -121,6 +124,149 @@ public void testUpgradeNoop() throws Exception {
assertEquals(seed.toString(), keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey()).toString());
}

public void testFailWhenCannotConsumeSecretStream() throws Exception {
Path configDir = env.configFile();
SimpleFSDirectory directory = new SimpleFSDirectory(configDir);
try (IndexOutput indexOutput = directory.createOutput("elasticsearch.keystore", IOContext.DEFAULT)) {
CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", 3);
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);
// Indicate that the secret string is longer than it is so readFully() fails
possiblyAlterSecretString(output, -4);
cipherStream.close();
final byte[] encryptedBytes = bytes.toByteArray();
possiblyAlterEncryptedBytes(indexOutput, salt, iv, encryptedBytes, 0);
CodecUtil.writeFooter(indexOutput);
}

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));
}

public void testFailWhenCannotConsumeEncryptedBytesStream() throws Exception {
Path configDir = env.configFile();
SimpleFSDirectory directory = new SimpleFSDirectory(configDir);
try (IndexOutput indexOutput = directory.createOutput("elasticsearch.keystore", IOContext.DEFAULT)) {
CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", 3);
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();
// Indicate that the encryptedBytes is larger than it is so readFully() fails
possiblyAlterEncryptedBytes(indexOutput, salt, iv, encryptedBytes, -12);
CodecUtil.writeFooter(indexOutput);
}

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));
}

public void testFailWhenSecretStreamNotConsumed() throws Exception {
Path configDir = env.configFile();
SimpleFSDirectory directory = new SimpleFSDirectory(configDir);
try (IndexOutput indexOutput = directory.createOutput("elasticsearch.keystore", IOContext.DEFAULT)) {
CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", 3);
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);
// So that readFully during decryption will not consume the entire stream
possiblyAlterSecretString(output, 4);
cipherStream.close();
final byte[] encryptedBytes = bytes.toByteArray();
possiblyAlterEncryptedBytes(indexOutput, salt, iv, encryptedBytes, 0);
CodecUtil.writeFooter(indexOutput);
}

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"));
}

public void testFailWhenEncryptedBytesStreamIsNotConsumed() throws Exception {
Path configDir = env.configFile();
SimpleFSDirectory directory = new SimpleFSDirectory(configDir);
try (IndexOutput indexOutput = directory.createOutput("elasticsearch.keystore", IOContext.DEFAULT)) {
CodecUtil.writeHeader(indexOutput, "elasticsearch.keystore", 3);
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();
possiblyAlterEncryptedBytes(indexOutput, salt, iv, encryptedBytes, randomIntBetween(2, encryptedBytes.length));
CodecUtil.writeFooter(indexOutput);
}

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"));
}

private CipherOutputStream getCipherStream(ByteArrayOutputStream bytes, byte[] salt, byte[] iv) throws Exception {
PBEKeySpec keySpec = new PBEKeySpec(new char[0], salt, 10000, 128);
SecretKeyFactory keyFactory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA512");
SecretKey secretKey = keyFactory.generateSecret(keySpec);
SecretKeySpec secret = new SecretKeySpec(secretKey.getEncoded(), "AES");
GCMParameterSpec spec = new GCMParameterSpec(128, iv);
Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding");
cipher.init(Cipher.ENCRYPT_MODE, secret, spec);
cipher.updateAAD(salt);
return new CipherOutputStream(bytes, cipher);
}

private void possiblyAlterSecretString(DataOutputStream output, int truncLength) throws Exception {
byte[] secret_value = "super_secret_value".getBytes(StandardCharsets.UTF_8);
output.writeInt(1); // One entry
output.writeUTF("string_setting");
output.writeUTF("STRING");
output.writeInt(secret_value.length - truncLength);
output.write(secret_value);
}

private void possiblyAlterEncryptedBytes(IndexOutput indexOutput, byte[] salt, byte[] iv, byte[] encryptedBytes, int
truncEncryptedDataLength)
throws Exception {
indexOutput.writeInt(4 + salt.length + 4 + iv.length + 4 + encryptedBytes.length);
indexOutput.writeInt(salt.length);
indexOutput.writeBytes(salt, salt.length);
indexOutput.writeInt(iv.length);
indexOutput.writeBytes(iv, iv.length);
indexOutput.writeInt(encryptedBytes.length - truncEncryptedDataLength);
indexOutput.writeBytes(encryptedBytes, encryptedBytes.length);
}

public void testUpgradeAddsSeed() throws Exception {
KeyStoreWrapper keystore = KeyStoreWrapper.create();
keystore.remove(KeyStoreWrapper.SEED_SETTING.getKey());
Expand Down