Skip to content

Commit

Permalink
Consistent Secure Settings (#40416)
Browse files Browse the repository at this point in the history
Introduces a new `ConsistentSecureSettingsValidatorService` service that exposes
a single public method, namely `allSecureSettingsConsistent`. The method returns
`true` if the local node's secure settings (inside the keystore) are equal to the
master's, and `false` otherwise. Technically, the local node has to have exactly
the same secure settings - setting names should not be missing or in surplus
- for all `SecureSetting` instances that are flagged with the newly introduced
`Property.Consistent`. It is worth highlighting that the `allSecureSettingsConsistent`
is not a consensus view across the cluster, but rather the local node's perspective
in relation to the master.
  • Loading branch information
albertzaharovits authored Jun 29, 2019
1 parent c108b09 commit cdfc986
Show file tree
Hide file tree
Showing 21 changed files with 879 additions and 41 deletions.
4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ task verifyVersions {
* after the backport of the backcompat code is complete.
*/

boolean bwc_tests_enabled = true
final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */
boolean bwc_tests_enabled = false
final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/40416" /* place a PR link here when committing bwc changes */
if (bwc_tests_enabled == false) {
if (bwc_tests_disabled_issue.isEmpty()) {
throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,12 @@
import java.nio.file.Path;
import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.security.MessageDigest;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Base64;
import java.util.List;
import java.util.Locale;
import java.util.Set;

import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -126,6 +128,27 @@ public void testCannotReadStringFromClosedKeystore() throws Exception {
assertThat(exception.getMessage(), containsString("closed"));
}

public void testValueSHA256Digest() throws Exception {
final KeyStoreWrapper keystore = KeyStoreWrapper.create();
final String stringSettingKeyName = randomAlphaOfLength(5).toLowerCase(Locale.ROOT) + "1";
final String stringSettingValue = randomAlphaOfLength(32);
keystore.setString(stringSettingKeyName, stringSettingValue.toCharArray());
final String fileSettingKeyName = randomAlphaOfLength(5).toLowerCase(Locale.ROOT) + "2";
final byte[] fileSettingValue = randomByteArrayOfLength(32);
keystore.setFile(fileSettingKeyName, fileSettingValue);

final byte[] stringSettingHash = MessageDigest.getInstance("SHA-256").digest(stringSettingValue.getBytes(StandardCharsets.UTF_8));
assertThat(keystore.getSHA256Digest(stringSettingKeyName), equalTo(stringSettingHash));
final byte[] fileSettingHash = MessageDigest.getInstance("SHA-256").digest(fileSettingValue);
assertThat(keystore.getSHA256Digest(fileSettingKeyName), equalTo(fileSettingHash));

keystore.close();

// value hashes accessible even when the keystore is closed
assertThat(keystore.getSHA256Digest(stringSettingKeyName), equalTo(stringSettingHash));
assertThat(keystore.getSHA256Digest(fileSettingKeyName), equalTo(fileSettingHash));
}

public void testUpgradeNoop() throws Exception {
KeyStoreWrapper keystore = KeyStoreWrapper.create();
SecureString seed = keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
*/
public class DiffableStringMap extends AbstractMap<String, String> implements Diffable<DiffableStringMap> {

public static final DiffableStringMap EMPTY = new DiffableStringMap(Collections.emptyMap());

private final Map<String, String> innerMap;

DiffableStringMap(final Map<String, String> map) {
Expand Down Expand Up @@ -75,6 +77,8 @@ public static Diff<DiffableStringMap> readDiffFrom(StreamInput in) throws IOExce
*/
public static class DiffableStringMapDiff implements Diff<DiffableStringMap> {

public static final DiffableStringMapDiff EMPTY = new DiffableStringMapDiff(DiffableStringMap.EMPTY, DiffableStringMap.EMPTY);

private final List<String> deletes;
private final Map<String, String> upserts; // diffs also become upserts

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.util.CollectionUtil;
import org.elasticsearch.Version;
import org.elasticsearch.action.AliasesRequest;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.ClusterState.FeatureAware;
Expand Down Expand Up @@ -168,6 +169,7 @@ public interface Custom extends NamedDiffable<Custom>, ToXContentFragment, Clust
private final Settings transientSettings;
private final Settings persistentSettings;
private final Settings settings;
private final DiffableStringMap hashesOfConsistentSettings;
private final ImmutableOpenMap<String, IndexMetaData> indices;
private final ImmutableOpenMap<String, IndexTemplateMetaData> templates;
private final ImmutableOpenMap<String, Custom> customs;
Expand All @@ -182,7 +184,7 @@ public interface Custom extends NamedDiffable<Custom>, ToXContentFragment, Clust
private final SortedMap<String, AliasOrIndex> aliasAndIndexLookup;

MetaData(String clusterUUID, boolean clusterUUIDCommitted, long version, CoordinationMetaData coordinationMetaData,
Settings transientSettings, Settings persistentSettings,
Settings transientSettings, Settings persistentSettings, DiffableStringMap hashesOfConsistentSettings,
ImmutableOpenMap<String, IndexMetaData> indices, ImmutableOpenMap<String, IndexTemplateMetaData> templates,
ImmutableOpenMap<String, Custom> customs, String[] allIndices, String[] allOpenIndices, String[] allClosedIndices,
SortedMap<String, AliasOrIndex> aliasAndIndexLookup) {
Expand All @@ -193,6 +195,7 @@ public interface Custom extends NamedDiffable<Custom>, ToXContentFragment, Clust
this.transientSettings = transientSettings;
this.persistentSettings = persistentSettings;
this.settings = Settings.builder().put(persistentSettings).put(transientSettings).build();
this.hashesOfConsistentSettings = hashesOfConsistentSettings;
this.indices = indices;
this.customs = customs;
this.templates = templates;
Expand Down Expand Up @@ -244,6 +247,10 @@ public Settings persistentSettings() {
return this.persistentSettings;
}

public Map<String, String> hashesOfConsistentSettings() {
return this.hashesOfConsistentSettings;
}

public CoordinationMetaData coordinationMetaData() {
return this.coordinationMetaData;
}
Expand Down Expand Up @@ -733,6 +740,9 @@ public static boolean isGlobalStateEquals(MetaData metaData1, MetaData metaData2
if (!metaData1.persistentSettings.equals(metaData2.persistentSettings)) {
return false;
}
if (!metaData1.hashesOfConsistentSettings.equals(metaData2.hashesOfConsistentSettings)) {
return false;
}
if (!metaData1.templates.equals(metaData2.templates())) {
return false;
}
Expand Down Expand Up @@ -787,6 +797,7 @@ private static class MetaDataDiff implements Diff<MetaData> {
private CoordinationMetaData coordinationMetaData;
private Settings transientSettings;
private Settings persistentSettings;
private Diff<DiffableStringMap> hashesOfConsistentSettings;
private Diff<ImmutableOpenMap<String, IndexMetaData>> indices;
private Diff<ImmutableOpenMap<String, IndexTemplateMetaData>> templates;
private Diff<ImmutableOpenMap<String, Custom>> customs;
Expand All @@ -798,6 +809,7 @@ private static class MetaDataDiff implements Diff<MetaData> {
coordinationMetaData = after.coordinationMetaData;
transientSettings = after.transientSettings;
persistentSettings = after.persistentSettings;
hashesOfConsistentSettings = after.hashesOfConsistentSettings.diff(before.hashesOfConsistentSettings);
indices = DiffableUtils.diff(before.indices, after.indices, DiffableUtils.getStringKeySerializer());
templates = DiffableUtils.diff(before.templates, after.templates, DiffableUtils.getStringKeySerializer());
customs = DiffableUtils.diff(before.customs, after.customs, DiffableUtils.getStringKeySerializer(), CUSTOM_VALUE_SERIALIZER);
Expand All @@ -810,6 +822,11 @@ private static class MetaDataDiff implements Diff<MetaData> {
coordinationMetaData = new CoordinationMetaData(in);
transientSettings = Settings.readSettingsFromStream(in);
persistentSettings = Settings.readSettingsFromStream(in);
if (in.getVersion().onOrAfter(Version.V_7_3_0)) {
hashesOfConsistentSettings = DiffableStringMap.readDiffFrom(in);
} else {
hashesOfConsistentSettings = DiffableStringMap.DiffableStringMapDiff.EMPTY;
}
indices = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), IndexMetaData::readFrom,
IndexMetaData::readDiffFrom);
templates = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), IndexTemplateMetaData::readFrom,
Expand All @@ -825,6 +842,9 @@ public void writeTo(StreamOutput out) throws IOException {
coordinationMetaData.writeTo(out);
Settings.writeSettingsToStream(transientSettings, out);
Settings.writeSettingsToStream(persistentSettings, out);
if (out.getVersion().onOrAfter(Version.V_7_3_0)) {
hashesOfConsistentSettings.writeTo(out);
}
indices.writeTo(out);
templates.writeTo(out);
customs.writeTo(out);
Expand All @@ -839,6 +859,7 @@ public MetaData apply(MetaData part) {
builder.coordinationMetaData(coordinationMetaData);
builder.transientSettings(transientSettings);
builder.persistentSettings(persistentSettings);
builder.hashesOfConsistentSettings(hashesOfConsistentSettings.apply(part.hashesOfConsistentSettings));
builder.indices(indices.apply(part.indices));
builder.templates(templates.apply(part.templates));
builder.customs(customs.apply(part.customs));
Expand All @@ -854,6 +875,9 @@ public static MetaData readFrom(StreamInput in) throws IOException {
builder.coordinationMetaData(new CoordinationMetaData(in));
builder.transientSettings(readSettingsFromStream(in));
builder.persistentSettings(readSettingsFromStream(in));
if (in.getVersion().onOrAfter(Version.V_7_3_0)) {
builder.hashesOfConsistentSettings(new DiffableStringMap(in));
}
int size = in.readVInt();
for (int i = 0; i < size; i++) {
builder.put(IndexMetaData.readFrom(in), false);
Expand All @@ -878,6 +902,9 @@ public void writeTo(StreamOutput out) throws IOException {
coordinationMetaData.writeTo(out);
writeSettingsToStream(transientSettings, out);
writeSettingsToStream(persistentSettings, out);
if (out.getVersion().onOrAfter(Version.V_7_3_0)) {
hashesOfConsistentSettings.writeTo(out);
}
out.writeVInt(indices.size());
for (IndexMetaData indexMetaData : this) {
indexMetaData.writeTo(out);
Expand Down Expand Up @@ -918,6 +945,7 @@ public static class Builder {
private CoordinationMetaData coordinationMetaData = CoordinationMetaData.EMPTY_META_DATA;
private Settings transientSettings = Settings.Builder.EMPTY_SETTINGS;
private Settings persistentSettings = Settings.Builder.EMPTY_SETTINGS;
private DiffableStringMap hashesOfConsistentSettings = new DiffableStringMap(Collections.emptyMap());

private final ImmutableOpenMap.Builder<String, IndexMetaData> indices;
private final ImmutableOpenMap.Builder<String, IndexTemplateMetaData> templates;
Expand All @@ -937,6 +965,7 @@ public Builder(MetaData metaData) {
this.coordinationMetaData = metaData.coordinationMetaData;
this.transientSettings = metaData.transientSettings;
this.persistentSettings = metaData.persistentSettings;
this.hashesOfConsistentSettings = metaData.hashesOfConsistentSettings;
this.version = metaData.version;
this.indices = ImmutableOpenMap.builder(metaData.indices);
this.templates = ImmutableOpenMap.builder(metaData.templates);
Expand Down Expand Up @@ -1100,6 +1129,20 @@ public Builder persistentSettings(Settings settings) {
return this;
}

public DiffableStringMap hashesOfConsistentSettings() {
return this.hashesOfConsistentSettings;
}

public Builder hashesOfConsistentSettings(DiffableStringMap hashesOfConsistentSettings) {
this.hashesOfConsistentSettings = hashesOfConsistentSettings;
return this;
}

public Builder hashesOfConsistentSettings(Map<String, String> hashesOfConsistentSettings) {
this.hashesOfConsistentSettings = new DiffableStringMap(hashesOfConsistentSettings);
return this;
}

public Builder version(long version) {
this.version = version;
return this;
Expand Down Expand Up @@ -1173,8 +1216,8 @@ public MetaData build() {
String[] allClosedIndicesArray = allClosedIndices.toArray(new String[allClosedIndices.size()]);

return new MetaData(clusterUUID, clusterUUIDCommitted, version, coordinationMetaData, transientSettings, persistentSettings,
indices.build(), templates.build(), customs.build(), allIndicesArray, allOpenIndicesArray, allClosedIndicesArray,
aliasAndIndexLookup);
hashesOfConsistentSettings, indices.build(), templates.build(), customs.build(), allIndicesArray, allOpenIndicesArray,
allClosedIndicesArray, aliasAndIndexLookup);
}

private SortedMap<String, AliasOrIndex> buildAliasAndIndexLookup() {
Expand Down Expand Up @@ -1298,6 +1341,8 @@ public static MetaData fromXContent(XContentParser parser) throws IOException {
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
builder.put(IndexMetaData.Builder.fromXContent(parser), false);
}
} else if ("hashes_of_consistent_settings".equals(currentFieldName)) {
builder.hashesOfConsistentSettings(parser.mapStrings());
} else if ("templates".equals(currentFieldName)) {
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
builder.put(IndexTemplateMetaData.Builder.fromXContent(parser, parser.currentName()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public ClusterService(Settings settings, ClusterSettings clusterSettings, Thread
}

public ClusterService(Settings settings, ClusterSettings clusterSettings, MasterService masterService,
ClusterApplierService clusterApplierService) {
ClusterApplierService clusterApplierService) {
this.settings = settings;
this.nodeName = Node.NODE_NAME_SETTING.get(settings);
this.masterService = masterService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,24 @@ private static MessageDigest get(ThreadLocal<MessageDigest> messageDigest) {
* @return a hex representation of the input as a String.
*/
public static String toHexString(byte[] bytes) {
Objects.requireNonNull(bytes);
StringBuilder sb = new StringBuilder(2 * bytes.length);
return new String(toHexCharArray(bytes));
}

/**
* Encodes the byte array into a newly created hex char array, without allocating any other temporary variables.
*
* @param bytes the input to be encoded as hex.
* @return the hex encoding of the input as a char array.
*/
public static char[] toHexCharArray(byte[] bytes) {
Objects.requireNonNull(bytes);
final char[] result = new char[2 * bytes.length];
for (int i = 0; i < bytes.length; i++) {
byte b = bytes[i];
sb.append(HEX_DIGITS[b >> 4 & 0xf]).append(HEX_DIGITS[b & 0xf]);
result[2 * i] = HEX_DIGITS[b >> 4 & 0xf];
result[2 * i + 1] = HEX_DIGITS[b & 0xf];
}

return sb.toString();
return result;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,12 @@
* Encapsulates all valid cluster level settings.
*/
public final class ClusterSettings extends AbstractScopedSettings {

public ClusterSettings(final Settings nodeSettings, final Set<Setting<?>> settingsSet) {
this(nodeSettings, settingsSet, Collections.emptySet());
}

public ClusterSettings(
final Settings nodeSettings, final Set<Setting<?>> settingsSet, final Set<SettingUpgrader<?>> settingUpgraders) {
public ClusterSettings(final Settings nodeSettings, final Set<Setting<?>> settingsSet, final Set<SettingUpgrader<?>> settingUpgraders) {
super(nodeSettings, settingsSet, settingUpgraders, Property.NodeScope);
addSettingsUpdater(new LoggingSettingUpdater(nodeSettings));
}
Expand Down
Loading

0 comments on commit cdfc986

Please sign in to comment.