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

Drop distinction in entries for keystore #41701

Merged
merged 9 commits into from
May 1, 2019
Merged

Drop distinction in entries for keystore #41701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ceb739f
Drop distinction in entries for keystore
jasontedor Apr 30, 2019
60109d9
Fix imports
jasontedor Apr 30, 2019
93ff28e
More assertions
jasontedor Apr 30, 2019
baccd3e
Clearer version
jasontedor Apr 30, 2019
6e8e7d4
Fix formatting
jasontedor Apr 30, 2019
9788ef9
Add exclusion for forbidden patterns
jasontedor Apr 30, 2019
b7fc213
Stricter version
jasontedor Apr 30, 2019
527c8d3
Fix forbidden
jasontedor Apr 30, 2019
fe47151
Fix imports
jasontedor May 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ForbiddenPatternsTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public class ForbiddenPatternsTask extends DefaultTask {
.exclude("**/*.zip")
.exclude("**/*.jks")
.exclude("**/*.crt")
.exclude("**/*.keystore")
.exclude("**/*.png");

/*
Expand Down
83 changes: 70 additions & 13 deletions ...ls/keystore-cli/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,43 +19,48 @@

package org.elasticsearch.common.settings;

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.env.Environment;
import org.elasticsearch.test.ESTestCase;
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 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.env.Environment;
import org.elasticsearch.test.ESTestCase;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
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 {

Expand Down Expand Up @@ -386,4 +391,56 @@ public void testBackcompatV2() throws Exception {
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();
}

}
Binary file added distribution/tools/keystore-cli/src/test/resources/format-v3-elasticsearch.keystore
View file
Open in desktop
Binary file not shown.
97 changes: 43 additions & 54 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 @@ -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;
Expand All @@ -27,6 +39,7 @@
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;
Expand Down Expand Up @@ -56,18 +69,6 @@
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.
*
Expand All @@ -84,17 +85,6 @@ private enum EntryType {
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.
*/
Expand All @@ -110,7 +100,7 @@ private static class Entry {
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;
Expand Down Expand Up @@ -146,6 +136,7 @@ private static class Entry {
// 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;
Expand All @@ -157,7 +148,7 @@ private static class Entry {
private final byte[] dataBytes;

/** The decrypted secret data. See {@link #decrypt(char[])}. */
private final SetOnce<Map<String, Entry>> entries = new SetOnce<>();
private final SetOnce<Map<String, byte[]>> entries = new SetOnce<>();
private volatile boolean closed;

private KeyStoreWrapper(int formatVersion, boolean hasPassword, byte[] dataBytes) {
Expand Down Expand Up @@ -273,11 +264,13 @@ 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 {
// 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);
}

Expand Down Expand Up @@ -350,11 +343,14 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio
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");
Expand All @@ -373,12 +369,11 @@ private byte[] encrypt(char[] password, byte[] salt, byte[] iv) throws GeneralSe
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()) {
for (Map.Entry<String, byte[]> 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();
Expand Down Expand Up @@ -453,7 +448,7 @@ private void decryptLegacyEntries() throws GeneralSecurityException, IOException
}
Arrays.fill(chars, '\0');

entries.get().put(setting, new Entry(settingType, bytes));
entries.get().put(setting, bytes);
}
}

Expand Down Expand Up @@ -526,23 +521,17 @@ public Set<String> getSettingNames() {
@Override
public synchronized SecureString getString(String setting) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be done in a followup, but with the removal of the distinction within the keystore I think it would be best if the keystore only exposed a method to get the bytes and the individual setting types could handle the conversion to SecureString/InputStream

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

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

/**
Expand All @@ -564,9 +553,9 @@ synchronized void setString(String setting, char[] value) {

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

Expand All @@ -575,18 +564,18 @@ synchronized void setFile(String setting, byte[] bytes) {
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);
}
}

Expand All @@ -601,8 +590,8 @@ private void ensureOpen() {
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);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...y/src/main/java/org/elasticsearch/xpack/security/FIPS140SecureSettingsBootstrapCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public BootstrapCheckResult check(BootstrapContext context) {
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) {
Expand Down