From 3d934fea30854ba89b06f6d08ff04f79ba7bc690 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 20 Mar 2019 15:39:47 +0200 Subject: [PATCH 01/39] SettingsModule --- .../common/settings/SecureSetting.java | 2 +- .../common/settings/Setting.java | 5 +++ .../common/settings/SettingsModule.java | 35 +++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index 33f4718aa45e4..cdd64622b2b14 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -37,7 +37,7 @@ public abstract class SecureSetting extends Setting { /** Determines whether legacy settings with sensitive values should be allowed. */ private static final boolean ALLOW_INSECURE_SETTINGS = Booleans.parseBoolean(System.getProperty("es.allow_insecure_settings", "false")); - private static final Set ALLOWED_PROPERTIES = EnumSet.of(Property.Deprecated); + private static final Set ALLOWED_PROPERTIES = EnumSet.of(Property.Deprecated, Property.Consistent); private static final Property[] FIXED_PROPERTIES = { Property.NodeScope diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index 9c3762f857e4a..ce90d32dcf379 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -113,6 +113,11 @@ public enum Property { */ NodeScope, + /** + * Setting value not allowed to differ across the nodes + */ + Consistent, + /** * Index scope */ diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 6a78e81d7f3f4..263587f6ab8f6 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -24,11 +24,19 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Binder; import org.elasticsearch.common.inject.Module; +import org.elasticsearch.common.io.Streams; +import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.nio.CharBuffer; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -49,6 +57,7 @@ public class SettingsModule implements Module { private final Set settingsFilterPattern = new HashSet<>(); private final Map> nodeSettings = new HashMap<>(); private final Map> indexSettings = new HashMap<>(); + private final Map privateHashesOfConsistentSettings = new HashMap<>(); private final IndexScopedSettings indexScopedSettings; private final ClusterSettings clusterSettings; private final SettingsFilter settingsFilter; @@ -176,6 +185,32 @@ private void registerSetting(Setting setting) { throw new IllegalArgumentException("Cannot register setting [" + setting.getKey() + "] twice"); } nodeSettings.put(setting.getKey(), setting); + if (setting instanceof SecureSetting && setting.getProperties().contains(Property.Consistent)) { + final Object verbatimValue = setting.get(this.settings); + final MessageDigest digest; + try { + digest = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException("SHA-256 algorithm is required for the consistent secure settings functionality", e); + } + if (verbatimValue instanceof InputStream) { + try (InputStream is = ((InputStream)verbatimValue)) { + try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { + Streams.copy(is, out); + digest.update(out.toByteArray()); + } + } catch (IOException e) { + throw new RuntimeException("Exception while reading the [" + setting.getKey() + "] secure setting.", e); + } + } else if (verbatimValue instanceof SecureString) { + try (SecureString secureString = ((SecureString)verbatimValue)) { + digest.update(StandardCharsets.UTF_8.encode(CharBuffer.wrap(secureString.getChars()))); + } + } else { + throw new IllegalArgumentException("Unrecognized consistent secure setting [" + setting.getKey() + "]"); + } + privateHashesOfConsistentSettings.put(setting.getKey(), digest.digest()); + } } if (setting.hasIndexScope()) { Setting existingSetting = indexSettings.get(setting.getKey()); From 8709b461c45264b3523822c9d563c880f04c50d1 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Thu, 21 Mar 2019 08:28:45 +0200 Subject: [PATCH 02/39] SettingsModule looking sharp ~ --- .../common/settings/Setting.java | 16 ++++- .../common/settings/SettingsModule.java | 67 ++++++++++++------- 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index ce90d32dcf379..379d01884efb1 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -114,7 +114,7 @@ public enum Property { NodeScope, /** - * Setting value not allowed to differ across the nodes + * Setting values equal on all nodes */ Consistent, @@ -173,6 +173,7 @@ private Setting(Key key, @Nullable Setting fallbackSetting, Function properties, } } + private void checkPropertyRequiresNodeScope(final EnumSet properties, final Property property) { + if (properties.contains(property) && properties.contains(Property.NodeScope) == false) { + throw new IllegalArgumentException("non-node-scoped setting [" + key + "] can not have property [" + property + "]"); + } + } + /** * Creates a new Setting instance * @param key the settings key for this setting. @@ -327,6 +334,13 @@ public boolean hasNodeScope() { return properties.contains(Property.NodeScope); } + /** + * Returns true if this setting's value can be checked for equality across all nodes + */ + public boolean isConsistent() { + return properties.contains(Property.Consistent); + } + /** * Returns true if this setting has an index scope, otherwise false */ diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 263587f6ab8f6..1e47c00d31eeb 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -25,7 +25,6 @@ import org.elasticsearch.common.inject.Binder; import org.elasticsearch.common.inject.Module; import org.elasticsearch.common.io.Streams; -import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; @@ -185,34 +184,22 @@ private void registerSetting(Setting setting) { throw new IllegalArgumentException("Cannot register setting [" + setting.getKey() + "] twice"); } nodeSettings.put(setting.getKey(), setting); - if (setting instanceof SecureSetting && setting.getProperties().contains(Property.Consistent)) { - final Object verbatimValue = setting.get(this.settings); - final MessageDigest digest; - try { - digest = MessageDigest.getInstance("SHA-256"); - } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("SHA-256 algorithm is required for the consistent secure settings functionality", e); - } - if (verbatimValue instanceof InputStream) { - try (InputStream is = ((InputStream)verbatimValue)) { - try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { - Streams.copy(is, out); - digest.update(out.toByteArray()); + if (setting.isConsistent()) { + if (setting instanceof Setting.AffixSetting) { + ((Setting.AffixSetting)setting).getAllConcreteSettings(this.settings).forEach(concreteSetting -> { + if (concreteSetting instanceof SecureSetting) { + saveSecureSettingForLaterComparison((SecureSetting)concreteSetting); + } else { + throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); } - } catch (IOException e) { - throw new RuntimeException("Exception while reading the [" + setting.getKey() + "] secure setting.", e); - } - } else if (verbatimValue instanceof SecureString) { - try (SecureString secureString = ((SecureString)verbatimValue)) { - digest.update(StandardCharsets.UTF_8.encode(CharBuffer.wrap(secureString.getChars()))); - } + }); + } else if (setting instanceof SecureSetting) { + saveSecureSettingForLaterComparison((SecureSetting) setting); } else { - throw new IllegalArgumentException("Unrecognized consistent secure setting [" + setting.getKey() + "]"); + throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); } - privateHashesOfConsistentSettings.put(setting.getKey(), digest.digest()); } - } - if (setting.hasIndexScope()) { + } else if (setting.hasIndexScope()) { Setting existingSetting = indexSettings.get(setting.getKey()); if (existingSetting != null) { throw new IllegalArgumentException("Cannot register setting [" + setting.getKey() + "] twice"); @@ -224,6 +211,36 @@ private void registerSetting(Setting setting) { } } + /** + * Save unsalted hash for subsequent comparison with the master node. + */ + private void saveSecureSettingForLaterComparison(SecureSetting secureSetting) { + final Object verbatimValue = secureSetting.get(this.settings); + final MessageDigest digest; + try { + digest = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException("SHA-256 algorithm is required for the consistent secure settings functionality", e); + } + if (verbatimValue instanceof InputStream) { + try (InputStream is = ((InputStream)verbatimValue)) { + try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { + Streams.copy(is, out); + digest.update(out.toByteArray()); + } + } catch (IOException e) { + throw new RuntimeException("Exception while reading the [" + secureSetting.getKey() + "] secure setting.", e); + } + } else if (verbatimValue instanceof SecureString) { + try (SecureString secureString = ((SecureString)verbatimValue)) { + digest.update(StandardCharsets.UTF_8.encode(CharBuffer.wrap(secureString.getChars()))); + } + } else { + throw new IllegalArgumentException("Unrecognized consistent secure setting [" + secureSetting.getKey() + "]"); + } + privateHashesOfConsistentSettings.put(secureSetting.getKey(), digest.digest()); + } + /** * Registers a settings filter pattern that allows to filter out certain settings that for instance contain sensitive information * or if a setting is for internal purposes only. The given pattern must either be a valid settings key or a simple regexp pattern. From 753fe0340273e222ef5b393c585bc34fa12ef2e5 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Thu, 21 Mar 2019 13:25:52 +0200 Subject: [PATCH 03/39] Metadata looking sharp --- .../cluster/metadata/MetaData.java | 54 +++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java index 720c6f7d8ebae..06d785ac9df63 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -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; @@ -166,6 +167,7 @@ public interface Custom extends NamedDiffable, ToXContentFragment, Clust private final Settings transientSettings; private final Settings persistentSettings; private final Settings settings; + private final DiffableStringMap hashesOfConsistentSettings; private final ImmutableOpenMap indices; private final ImmutableOpenMap templates; private final ImmutableOpenMap customs; @@ -180,7 +182,7 @@ public interface Custom extends NamedDiffable, ToXContentFragment, Clust private final SortedMap aliasAndIndexLookup; MetaData(String clusterUUID, boolean clusterUUIDCommitted, long version, CoordinationMetaData coordinationMetaData, - Settings transientSettings, Settings persistentSettings, + Settings transientSettings, Settings persistentSettings, DiffableStringMap hashesOfConsistentSettings, ImmutableOpenMap indices, ImmutableOpenMap templates, ImmutableOpenMap customs, String[] allIndices, String[] allOpenIndices, String[] allClosedIndices, SortedMap aliasAndIndexLookup) { @@ -191,6 +193,7 @@ public interface Custom extends NamedDiffable, 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; @@ -242,6 +245,10 @@ public Settings persistentSettings() { return this.persistentSettings; } + public Map hashesOfConsistentSettings() { + return this.hashesOfConsistentSettings; + } + public CoordinationMetaData coordinationMetaData() { return this.coordinationMetaData; } @@ -763,6 +770,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; } @@ -817,6 +827,7 @@ private static class MetaDataDiff implements Diff { private CoordinationMetaData coordinationMetaData; private Settings transientSettings; private Settings persistentSettings; + private Diff hashesOfConsistentSettings; private Diff> indices; private Diff> templates; private Diff> customs; @@ -828,6 +839,7 @@ private static class MetaDataDiff implements Diff { 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); @@ -840,6 +852,13 @@ private static class MetaDataDiff implements Diff { coordinationMetaData = new CoordinationMetaData(in); transientSettings = Settings.readSettingsFromStream(in); persistentSettings = Settings.readSettingsFromStream(in); + if (in.getVersion().onOrAfter(Version.CURRENT)) { + hashesOfConsistentSettings = DiffableStringMap.readDiffFrom(in); + } else { + // empty diff + hashesOfConsistentSettings = new DiffableStringMap(Collections.emptyMap()) + .diff(new DiffableStringMap(Collections.emptyMap())); + } indices = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), IndexMetaData::readFrom, IndexMetaData::readDiffFrom); templates = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), IndexTemplateMetaData::readFrom, @@ -855,6 +874,9 @@ public void writeTo(StreamOutput out) throws IOException { coordinationMetaData.writeTo(out); Settings.writeSettingsToStream(transientSettings, out); Settings.writeSettingsToStream(persistentSettings, out); + if (out.getVersion().onOrAfter(Version.CURRENT)) { + hashesOfConsistentSettings.writeTo(out); + } indices.writeTo(out); templates.writeTo(out); customs.writeTo(out); @@ -869,6 +891,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)); @@ -884,6 +907,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.CURRENT)) { + builder.hashesOfConsistentSettings(new DiffableStringMap(in)); + } int size = in.readVInt(); for (int i = 0; i < size; i++) { builder.put(IndexMetaData.readFrom(in), false); @@ -908,6 +934,9 @@ public void writeTo(StreamOutput out) throws IOException { coordinationMetaData.writeTo(out); writeSettingsToStream(transientSettings, out); writeSettingsToStream(persistentSettings, out); + if (out.getVersion().onOrAfter(Version.CURRENT)) { + hashesOfConsistentSettings.writeTo(out); + } out.writeVInt(indices.size()); for (IndexMetaData indexMetaData : this) { indexMetaData.writeTo(out); @@ -948,6 +977,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 indices; private final ImmutableOpenMap.Builder templates; @@ -967,6 +997,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); @@ -1129,6 +1160,15 @@ 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 version(long version) { this.version = version; return this; @@ -1202,8 +1242,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 buildAliasAndIndexLookup() { @@ -1264,6 +1304,12 @@ public static void toXContent(MetaData metaData, XContentBuilder builder, ToXCon builder.endObject(); } + if (context == XContentContext.API && false == metaData.hashesOfConsistentSettings().isEmpty()) { + builder.startObject("hashes_of_consistent_settings"); + builder.map(metaData.hashesOfConsistentSettings()); + builder.endObject(); + } + builder.startObject("templates"); for (ObjectCursor cursor : metaData.templates().values()) { IndexTemplateMetaData.Builder.toXContentWithTypes(cursor.value, builder, params); @@ -1327,6 +1373,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(new DiffableStringMap(parser.mapStrings())); } else if ("templates".equals(currentFieldName)) { while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { builder.put(IndexTemplateMetaData.Builder.fromXContent(parser, parser.currentName())); From 05c3249a8a472073c1d71849767449cf01bbecfc Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Thu, 21 Mar 2019 19:01:37 +0200 Subject: [PATCH 04/39] SettingsModule compute public hashes --- .../common/settings/SettingsModule.java | 71 ++++++++++++++++--- 1 file changed, 62 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 1e47c00d31eeb..df4b86f091432 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -21,7 +21,9 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.elasticsearch.common.CharArrays; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.inject.Binder; import org.elasticsearch.common.inject.Module; import org.elasticsearch.common.io.Streams; @@ -32,11 +34,14 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.ByteBuffer; import java.nio.CharBuffer; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.security.spec.InvalidKeySpecException; import java.util.Arrays; +import java.util.Base64; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -46,6 +51,10 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; + /** * A module that binds the provided settings to the {@link Settings} interface. */ @@ -56,7 +65,8 @@ public class SettingsModule implements Module { private final Set settingsFilterPattern = new HashSet<>(); private final Map> nodeSettings = new HashMap<>(); private final Map> indexSettings = new HashMap<>(); - private final Map privateHashesOfConsistentSettings = new HashMap<>(); + private final Map localHashesOfConsistentSettings = new HashMap<>(); + private final Map publicHashesOfConsistentSettings = new HashMap<>(); private final IndexScopedSettings indexScopedSettings; private final ClusterSettings clusterSettings; private final SettingsFilter settingsFilter; @@ -165,7 +175,6 @@ public void configure(Binder binder) { binder.bind(IndexScopedSettings.class).toInstance(indexScopedSettings); } - /** * Registers a new setting. This method should be used by plugins in order to expose any custom settings the plugin defines. * Unless a setting is registered the setting is unusable. If a setting is never the less specified the node will reject @@ -188,13 +197,13 @@ private void registerSetting(Setting setting) { if (setting instanceof Setting.AffixSetting) { ((Setting.AffixSetting)setting).getAllConcreteSettings(this.settings).forEach(concreteSetting -> { if (concreteSetting instanceof SecureSetting) { - saveSecureSettingForLaterComparison((SecureSetting)concreteSetting); + buildSecureSettingHashes((SecureSetting) concreteSetting); } else { throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); } }); } else if (setting instanceof SecureSetting) { - saveSecureSettingForLaterComparison((SecureSetting) setting); + buildSecureSettingHashes((SecureSetting) setting); } else { throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); } @@ -211,16 +220,26 @@ private void registerSetting(Setting setting) { } } + private void buildSecureSettingHashes(SecureSetting secureConcreteSetting) { + // compute unsalted hash to store in memory on the local node + final char[] localHash = computeSecureSettingLocalHash(secureConcreteSetting); + localHashesOfConsistentSettings.put(secureConcreteSetting.getKey(), localHash); + // compute and store a salted hash to store on the cluster state, so that other nodes can cross check against it + publicHashesOfConsistentSettings.put(secureConcreteSetting.getKey(), computePublicHash(localHash)); + } + /** - * Save unsalted hash for subsequent comparison with the master node. + * Compute UNSALTED hash of a secure setting value for subsequent comparison with the master's reference. This values IS NOT to be + * published in the cluster state, but rather compared (and stored), on the local node, to the SALTED hash value published in the + * cluster state by the master node. */ - private void saveSecureSettingForLaterComparison(SecureSetting secureSetting) { + private char[] computeSecureSettingLocalHash(SecureSetting secureSetting) { final Object verbatimValue = secureSetting.get(this.settings); final MessageDigest digest; try { - digest = MessageDigest.getInstance("SHA-256"); + digest = MessageDigest.getInstance("SHA-512"); } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("SHA-256 algorithm is required for the consistent secure settings functionality", e); + throw new RuntimeException("\"SHA-512\" algorithm is required for consistent secure settings", e); } if (verbatimValue instanceof InputStream) { try (InputStream is = ((InputStream)verbatimValue)) { @@ -238,7 +257,41 @@ private void saveSecureSettingForLaterComparison(SecureSetting secureSetting) } else { throw new IllegalArgumentException("Unrecognized consistent secure setting [" + secureSetting.getKey() + "]"); } - privateHashesOfConsistentSettings.put(secureSetting.getKey(), digest.digest()); + // computing a base64 encoded unsalted SHA512 hash + byte[] unencodedSHA512AsByteArray = null; + byte[] SHA512EncodedBase64AsByteArray = null; + try { + unencodedSHA512AsByteArray = digest.digest(); + SHA512EncodedBase64AsByteArray = Base64.getEncoder().encode(unencodedSHA512AsByteArray); + return CharArrays.utf8BytesToChars(SHA512EncodedBase64AsByteArray); + } finally { + if (unencodedSHA512AsByteArray != null) { + Arrays.fill(unencodedSHA512AsByteArray, (byte) 0); + } + if (SHA512EncodedBase64AsByteArray != null) { + Arrays.fill(SHA512EncodedBase64AsByteArray, (byte) 0); + } + } + } + + public static byte[] computeSaltedHash(char[] value, byte[] salt) { + final int iterations = 5000; + final int keyLength = 512; + try { + final SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA512"); + final PBEKeySpec spec = new PBEKeySpec(value, salt, iterations, keyLength); + final SecretKey key = skf.generateSecret(spec); + return key.getEncoded(); + } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { + throw new RuntimeException("\"PBKDF2WithHmacSHA512\" algorithm is required for consistent secure settings", e); + } + } + + public static String computePublicHash(char[] value) { + final String saltString = UUIDs.randomBase64UUID(); + final byte[] saltedHashBytes = computeSaltedHash(value, saltString.getBytes(StandardCharsets.UTF_8)); + final String base64SaltedHashString = new String(saltedHashBytes, StandardCharsets.UTF_8); + return saltString + ":" + base64SaltedHashString; } /** From f5d7ea310ca92f15391e452d2242f26bfa2dc537 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Thu, 21 Mar 2019 19:06:50 +0200 Subject: [PATCH 05/39] Get setting hashes to ClusterSettings --- .../common/settings/ClusterSettings.java | 18 +++++++++++++++--- .../common/settings/SettingsModule.java | 3 ++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 315e6175a3a36..22c3cedf49700 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -108,6 +108,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.Predicate; @@ -115,13 +116,24 @@ * Encapsulates all valid cluster level settings. */ public final class ClusterSettings extends AbstractScopedSettings { + + private final Map localHashesOfConsistentSettings; + private final Map publicHashesOfConsistentSettings; + public ClusterSettings(final Settings nodeSettings, final Set> settingsSet) { - this(nodeSettings, settingsSet, Collections.emptySet()); + this(nodeSettings, settingsSet, Collections.emptySet(), Collections.emptyMap(), Collections.emptyMap()); + } + + public ClusterSettings(final Settings nodeSettings, final Set> settingsSet, final Set> settingUpgraders) { + this(nodeSettings, settingsSet, settingUpgraders, Collections.emptyMap(), Collections.emptyMap()); } - public ClusterSettings( - final Settings nodeSettings, final Set> settingsSet, final Set> settingUpgraders) { + public ClusterSettings(final Settings nodeSettings, final Set> settingsSet, final Set> settingUpgraders, + final Map localHashesOfConsistentSettings, + final Map publicHashesOfConsistentSettings) { super(nodeSettings, settingsSet, settingUpgraders, Property.NodeScope); + this.localHashesOfConsistentSettings = localHashesOfConsistentSettings; + this.publicHashesOfConsistentSettings = publicHashesOfConsistentSettings; addSettingsUpdater(new LoggingSettingUpdater(nodeSettings)); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index df4b86f091432..06e2943be2ce7 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -106,7 +106,8 @@ public SettingsModule( assert added : settingUpgrader.getSetting().getKey(); } this.indexScopedSettings = new IndexScopedSettings(settings, new HashSet<>(this.indexSettings.values())); - this.clusterSettings = new ClusterSettings(settings, new HashSet<>(this.nodeSettings.values()), clusterSettingUpgraders); + this.clusterSettings = new ClusterSettings(settings, new HashSet<>(this.nodeSettings.values()), clusterSettingUpgraders, + localHashesOfConsistentSettings, publicHashesOfConsistentSettings); Settings indexSettings = settings.filter((s) -> (s.startsWith("index.") && // special case - we want to get Did you mean indices.query.bool.max_clause_count // which means we need to by-pass this check for this setting From 7594ab2bb0c743ed143bfc22d6e34d4c16bbaa32 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Fri, 22 Mar 2019 21:10:51 +0200 Subject: [PATCH 06/39] WIP --- .../service/ClusterApplierService.java | 1 + .../common/settings/ClusterSettings.java | 14 +- .../ConsistentSecureSettingsValidator.java | 237 ++++++++++++++++++ .../common/settings/SettingsModule.java | 104 +------- 4 files changed, 247 insertions(+), 109 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java diff --git a/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java b/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java index 11b6f451d594b..5ef37d691428c 100644 --- a/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java +++ b/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java @@ -456,6 +456,7 @@ private void applyChanges(UpdateTask task, ClusterState previousClusterState, Cl // nothing to do until we actually recover from the gateway or any other block indicates we need to disable persistency if (clusterChangedEvent.state().blocks().disableStatePersistence() == false && clusterChangedEvent.metaDataChanged()) { logger.debug("applying settings from cluster state with version {}", newClusterState.version()); + albert final Settings incomingSettings = clusterChangedEvent.state().metaData().settings(); clusterSettings.applySettings(incomingSettings); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 22c3cedf49700..66f80f940d6a1 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -108,7 +108,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.function.Predicate; @@ -117,23 +116,12 @@ */ public final class ClusterSettings extends AbstractScopedSettings { - private final Map localHashesOfConsistentSettings; - private final Map publicHashesOfConsistentSettings; - public ClusterSettings(final Settings nodeSettings, final Set> settingsSet) { - this(nodeSettings, settingsSet, Collections.emptySet(), Collections.emptyMap(), Collections.emptyMap()); + this(nodeSettings, settingsSet, Collections.emptySet()); } public ClusterSettings(final Settings nodeSettings, final Set> settingsSet, final Set> settingUpgraders) { - this(nodeSettings, settingsSet, settingUpgraders, Collections.emptyMap(), Collections.emptyMap()); - } - - public ClusterSettings(final Settings nodeSettings, final Set> settingsSet, final Set> settingUpgraders, - final Map localHashesOfConsistentSettings, - final Map publicHashesOfConsistentSettings) { super(nodeSettings, settingsSet, settingUpgraders, Property.NodeScope); - this.localHashesOfConsistentSettings = localHashesOfConsistentSettings; - this.publicHashesOfConsistentSettings = publicHashesOfConsistentSettings; addSettingsUpdater(new LoggingSettingUpdater(nodeSettings)); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java new file mode 100644 index 0000000000000..200ef2a3b54c1 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java @@ -0,0 +1,237 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.cluster.ClusterChangedEvent; +import org.elasticsearch.cluster.ClusterStateApplier; +import org.elasticsearch.cluster.LocalNodeMasterListener; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.CharArrays; +import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.io.Streams; +import org.elasticsearch.threadpool.ThreadPool; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.CharBuffer; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.spec.InvalidKeySpecException; +import java.util.Arrays; +import java.util.Base64; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; + +public class ConsistentSecureSettingsValidator implements ClusterStateApplier, LocalNodeMasterListener { + private static final Logger logger = LogManager.getLogger(ConsistentSecureSettingsValidator.class); + + private final ClusterService clusterService; + private final Map localHashesOfConsistentSettings = new HashMap<>(); + private volatile Map publicHashesOfConsistentSettings = new HashMap<>(); + private volatile boolean allSecureSettingsConsistent = true; + + public ConsistentSecureSettingsValidator(Settings settings, ClusterService clusterService, Set> consistentSecureSettings) { + this.clusterService = clusterService; + for (SecureSetting secureSetting : consistentSecureSettings) { + // compute unsalted hash to store in memory on the local node + final char[] localHash = computeSecureSettingLocalHash(settings, secureSetting); + localHashesOfConsistentSettings.put(secureSetting.getKey(), localHash); + // salted hash (of the unsalted hash) suitable for cluster-wide publication + final String publicHash = computePublicHash(UUIDs.randomBase64UUID(), localHash); + publicHashesOfConsistentSettings.put(secureSetting.getKey(), publicHash); + } + } + + public boolean allSecureSettingsConsistent() { + return allSecureSettingsConsistent; + } + + /** + * Compute UNSALTED hash of a secure setting value for subsequent comparison with the master's reference. This values IS NOT to be + * published in the cluster state, but rather compared (and stored), on the local node, to the SALTED hash value published in the + * cluster state by the master node. + */ + private static char[] computeSecureSettingLocalHash(Settings settings, SecureSetting secureSetting) { + final Object verbatimValue = secureSetting.get(settings); + final MessageDigest digest; + try { + digest = MessageDigest.getInstance("SHA-512"); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException("\"SHA-512\" algorithm is required for consistent secure settings", e); + } + if (verbatimValue instanceof InputStream) { + try (InputStream is = ((InputStream)verbatimValue)) { + try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { + Streams.copy(is, out); + digest.update(out.toByteArray()); + } + } catch (IOException e) { + throw new RuntimeException("Exception while reading the [" + secureSetting.getKey() + "] secure setting.", e); + } + } else if (verbatimValue instanceof SecureString) { + try (SecureString secureString = ((SecureString)verbatimValue)) { + digest.update(StandardCharsets.UTF_8.encode(CharBuffer.wrap(secureString.getChars()))); + } + } else { + throw new IllegalArgumentException("Unrecognized consistent secure setting [" + secureSetting.getKey() + "]"); + } + // computing a base64 encoded unsalted SHA512 hash + byte[] unencodedSHA512AsByteArray = null; + byte[] SHA512EncodedBase64AsByteArray = null; + try { + unencodedSHA512AsByteArray = digest.digest(); + SHA512EncodedBase64AsByteArray = Base64.getEncoder().encode(unencodedSHA512AsByteArray); + return CharArrays.utf8BytesToChars(SHA512EncodedBase64AsByteArray); + } finally { + if (unencodedSHA512AsByteArray != null) { + Arrays.fill(unencodedSHA512AsByteArray, (byte) 0); + } + if (SHA512EncodedBase64AsByteArray != null) { + Arrays.fill(SHA512EncodedBase64AsByteArray, (byte) 0); + } + } + } + + private static byte[] computeSaltedHash(char[] value, byte[] salt) { + final int iterations = 5000; + final int keyLength = 512; + try { + final SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA512"); + final PBEKeySpec spec = new PBEKeySpec(value, salt, iterations, keyLength); + final SecretKey key = skf.generateSecret(spec); + return key.getEncoded(); + } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { + throw new RuntimeException("\"PBKDF2WithHmacSHA512\" algorithm is required for consistent secure settings", e); + } + } + + private static String computePublicHash(String salt, char[] localHash) { + final byte[] saltedHashBytes = computeSaltedHash(localHash, salt.getBytes(StandardCharsets.UTF_8)); + final String base64SaltedHashString = new String(saltedHashBytes, StandardCharsets.UTF_8); + return salt + ":" + base64SaltedHashString; + } + + private boolean validatePublicHash(String publishedSettingName, String publishedHashWithSalt) { + logger.trace("published hash for secure setting [{}] is [{}]", publishedSettingName, publishedHashWithSalt); + if (publishedHashWithSalt == null || false == publishedHashWithSalt.contains(":")) { + logger.trace("published hash [{}] for secure setting [{}] is invalid", publishedHashWithSalt, publishedSettingName); + return false; + } + final String[] parts = publishedHashWithSalt.split(":"); + if (parts == null || parts.length != 2) { + logger.trace("published hash [{}] for secure setting [{}] is invalid", publishedHashWithSalt, publishedSettingName); + return false; + } + final char[] localHash = localHashesOfConsistentSettings.get(publishedSettingName); + if (localHash == null) { + logger.trace("secure setting [{}] is missing on the local node", publishedSettingName); + return false; + } + final String publishedSalt = parts[0]; + final String localHashWithSalt = computePublicHash(publishedSalt, localHash); + logger.trace("local hash for secure setting [{}] is [{}]", publishedSettingName, localHashWithSalt); + return publishedHashWithSalt.equals(localHashWithSalt); + } + + @Override + public void applyClusterState(ClusterChangedEvent event) { + boolean allConsistent = true; + final Map publishedHashesOfConsistentSettings = event.state().metaData().hashesOfConsistentSettings(); + if (event.localNodeMaster()) { + logger.debug("local node is elected master, no hashes consistency check is required"); + } else if (publishedHashesOfConsistentSettings.equals(publicHashesOfConsistentSettings)) { + logger.debug("hashes of secure settings are identical to the master's"); + } else { + for (Map.Entry publishedSettingAndHash : publishedHashesOfConsistentSettings.entrySet()) { + if (false == validatePublicHash(publishedSettingAndHash.getKey(), publishedSettingAndHash.getValue())) { + allConsistent = false; + if (logger.isDebugEnabled()) { + logger.debug("secure setting [{}] differs on the local node [{}] compared to master [{}].", + publishedSettingAndHash.getKey(), event.state().nodes().getLocalNodeId(), + event.state().nodes().getMasterNodeId()); + } else { + break; + } + } + } + // also verify that local node does not hold a superset of the master's + for (String localSecureSettingName : localHashesOfConsistentSettings.keySet()) { + if (false == publishedHashesOfConsistentSettings.containsKey(localSecureSettingName)) { + allConsistent = false; + if (logger.isTraceEnabled()) { + logger.trace("secure setting [{}] is missing on master, but it is present on the local node", + localSecureSettingName); + } else { + break; + } + } + } + // public hashes of master and local node are equivalent but salts differ. Copy master's to local to speedup future checks + if (allConsistent) { + publicHashesOfConsistentSettings = publishedHashesOfConsistentSettings; + } + } + allSecureSettingsConsistent = allConsistent; + } + + @Override + public void onMaster() { + logger.trace("I have been elected master, publishing hashes of consistent secure settings' values"); +// +// clusterService.submitStateUpdateTask( +// "clean up snapshot restore state", +// new CleanRestoreStateTaskExecutor.Task(entry.uuid()), +// ClusterStateTaskConfig.build(Priority.URGENT), +// cleanRestoreStateTaskExecutor, +// cleanRestoreStateTaskExecutor); +// +// // Submit a job that will start after DEFAULT_STARTING_INTERVAL, and reschedule itself after running +// threadPool.scheduleUnlessShuttingDown(updateFrequency, executorName(), new SubmitReschedulingClusterInfoUpdatedJob()); +// +// try { +// if (clusterService.state().getNodes().getDataNodes().size() > 1) { +// // Submit an info update job to be run immediately +// threadPool.executor(executorName()).execute(() -> maybeRefresh()); +// } +// } catch (EsRejectedExecutionException ex) { +// logger.debug("Couldn't schedule cluster info update task - node might be shutting down", ex); +// } + } + + @Override + public void offMaster() { + logger.trace("I am no longer master, nothing to do"); + } + + @Override + public String executorName() { + return ThreadPool.Names.MANAGEMENT; + } + +} diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 06e2943be2ce7..dd4847c0a5551 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -21,27 +21,15 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; -import org.elasticsearch.common.CharArrays; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.inject.Binder; import org.elasticsearch.common.inject.Module; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; -import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.InputStream; -import java.nio.ByteBuffer; -import java.nio.CharBuffer; -import java.nio.charset.StandardCharsets; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; -import java.security.spec.InvalidKeySpecException; import java.util.Arrays; -import java.util.Base64; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -51,10 +39,6 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; -import javax.crypto.SecretKey; -import javax.crypto.SecretKeyFactory; -import javax.crypto.spec.PBEKeySpec; - /** * A module that binds the provided settings to the {@link Settings} interface. */ @@ -65,8 +49,7 @@ public class SettingsModule implements Module { private final Set settingsFilterPattern = new HashSet<>(); private final Map> nodeSettings = new HashMap<>(); private final Map> indexSettings = new HashMap<>(); - private final Map localHashesOfConsistentSettings = new HashMap<>(); - private final Map publicHashesOfConsistentSettings = new HashMap<>(); + private final Set> consistentSecureSettings = new HashSet<>(); private final IndexScopedSettings indexScopedSettings; private final ClusterSettings clusterSettings; private final SettingsFilter settingsFilter; @@ -106,8 +89,7 @@ public SettingsModule( assert added : settingUpgrader.getSetting().getKey(); } this.indexScopedSettings = new IndexScopedSettings(settings, new HashSet<>(this.indexSettings.values())); - this.clusterSettings = new ClusterSettings(settings, new HashSet<>(this.nodeSettings.values()), clusterSettingUpgraders, - localHashesOfConsistentSettings, publicHashesOfConsistentSettings); + this.clusterSettings = new ClusterSettings(settings, new HashSet<>(this.nodeSettings.values()), clusterSettingUpgraders); Settings indexSettings = settings.filter((s) -> (s.startsWith("index.") && // special case - we want to get Did you mean indices.query.bool.max_clause_count // which means we need to by-pass this check for this setting @@ -198,13 +180,13 @@ private void registerSetting(Setting setting) { if (setting instanceof Setting.AffixSetting) { ((Setting.AffixSetting)setting).getAllConcreteSettings(this.settings).forEach(concreteSetting -> { if (concreteSetting instanceof SecureSetting) { - buildSecureSettingHashes((SecureSetting) concreteSetting); + consistentSecureSettings.add((SecureSetting) concreteSetting); } else { throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); } }); } else if (setting instanceof SecureSetting) { - buildSecureSettingHashes((SecureSetting) setting); + consistentSecureSettings.add((SecureSetting) setting); } else { throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); } @@ -221,80 +203,6 @@ private void registerSetting(Setting setting) { } } - private void buildSecureSettingHashes(SecureSetting secureConcreteSetting) { - // compute unsalted hash to store in memory on the local node - final char[] localHash = computeSecureSettingLocalHash(secureConcreteSetting); - localHashesOfConsistentSettings.put(secureConcreteSetting.getKey(), localHash); - // compute and store a salted hash to store on the cluster state, so that other nodes can cross check against it - publicHashesOfConsistentSettings.put(secureConcreteSetting.getKey(), computePublicHash(localHash)); - } - - /** - * Compute UNSALTED hash of a secure setting value for subsequent comparison with the master's reference. This values IS NOT to be - * published in the cluster state, but rather compared (and stored), on the local node, to the SALTED hash value published in the - * cluster state by the master node. - */ - private char[] computeSecureSettingLocalHash(SecureSetting secureSetting) { - final Object verbatimValue = secureSetting.get(this.settings); - final MessageDigest digest; - try { - digest = MessageDigest.getInstance("SHA-512"); - } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("\"SHA-512\" algorithm is required for consistent secure settings", e); - } - if (verbatimValue instanceof InputStream) { - try (InputStream is = ((InputStream)verbatimValue)) { - try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { - Streams.copy(is, out); - digest.update(out.toByteArray()); - } - } catch (IOException e) { - throw new RuntimeException("Exception while reading the [" + secureSetting.getKey() + "] secure setting.", e); - } - } else if (verbatimValue instanceof SecureString) { - try (SecureString secureString = ((SecureString)verbatimValue)) { - digest.update(StandardCharsets.UTF_8.encode(CharBuffer.wrap(secureString.getChars()))); - } - } else { - throw new IllegalArgumentException("Unrecognized consistent secure setting [" + secureSetting.getKey() + "]"); - } - // computing a base64 encoded unsalted SHA512 hash - byte[] unencodedSHA512AsByteArray = null; - byte[] SHA512EncodedBase64AsByteArray = null; - try { - unencodedSHA512AsByteArray = digest.digest(); - SHA512EncodedBase64AsByteArray = Base64.getEncoder().encode(unencodedSHA512AsByteArray); - return CharArrays.utf8BytesToChars(SHA512EncodedBase64AsByteArray); - } finally { - if (unencodedSHA512AsByteArray != null) { - Arrays.fill(unencodedSHA512AsByteArray, (byte) 0); - } - if (SHA512EncodedBase64AsByteArray != null) { - Arrays.fill(SHA512EncodedBase64AsByteArray, (byte) 0); - } - } - } - - public static byte[] computeSaltedHash(char[] value, byte[] salt) { - final int iterations = 5000; - final int keyLength = 512; - try { - final SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA512"); - final PBEKeySpec spec = new PBEKeySpec(value, salt, iterations, keyLength); - final SecretKey key = skf.generateSecret(spec); - return key.getEncoded(); - } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { - throw new RuntimeException("\"PBKDF2WithHmacSHA512\" algorithm is required for consistent secure settings", e); - } - } - - public static String computePublicHash(char[] value) { - final String saltString = UUIDs.randomBase64UUID(); - final byte[] saltedHashBytes = computeSaltedHash(value, saltString.getBytes(StandardCharsets.UTF_8)); - final String base64SaltedHashString = new String(saltedHashBytes, StandardCharsets.UTF_8); - return saltString + ":" + base64SaltedHashString; - } - /** * Registers a settings filter pattern that allows to filter out certain settings that for instance contain sensitive information * or if a setting is for internal purposes only. The given pattern must either be a valid settings key or a simple regexp pattern. @@ -321,6 +229,10 @@ public ClusterSettings getClusterSettings() { return clusterSettings; } + public Set> getConsistentSecureSettings() { + return consistentSecureSettings; + } + public SettingsFilter getSettingsFilter() { return settingsFilter; } From c4cc9da29a9308999325ddff87d73db161d3509f Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 25 Mar 2019 12:21:48 +0200 Subject: [PATCH 07/39] ConsistentSecureSettingsValidator --- .../cluster/metadata/MetaData.java | 7 +- .../service/ClusterApplierService.java | 1 - .../ConsistentSecureSettingsValidator.java | 71 +++++++++++-------- 3 files changed, 49 insertions(+), 30 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java index 06d785ac9df63..ba37257246927 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -1169,6 +1169,11 @@ public Builder hashesOfConsistentSettings(DiffableStringMap hashesOfConsistentSe return this; } + public Builder hashesOfConsistentSettings(Map hashesOfConsistentSettings) { + this.hashesOfConsistentSettings = new DiffableStringMap(hashesOfConsistentSettings); + return this; + } + public Builder version(long version) { this.version = version; return this; @@ -1374,7 +1379,7 @@ public static MetaData fromXContent(XContentParser parser) throws IOException { builder.put(IndexMetaData.Builder.fromXContent(parser), false); } } else if ("hashes_of_consistent_settings".equals(currentFieldName)) { - builder.hashesOfConsistentSettings(new DiffableStringMap(parser.mapStrings())); + 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())); diff --git a/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java b/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java index 5ef37d691428c..11b6f451d594b 100644 --- a/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java +++ b/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java @@ -456,7 +456,6 @@ private void applyChanges(UpdateTask task, ClusterState previousClusterState, Cl // nothing to do until we actually recover from the gateway or any other block indicates we need to disable persistency if (clusterChangedEvent.state().blocks().disableStatePersistence() == false && clusterChangedEvent.metaDataChanged()) { logger.debug("applying settings from cluster state with version {}", newClusterState.version()); - albert final Settings incomingSettings = clusterChangedEvent.state().metaData().settings(); clusterSettings.applySettings(incomingSettings); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java index 200ef2a3b54c1..5e7321f723cce 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java @@ -22,10 +22,14 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.cluster.ClusterChangedEvent; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateApplier; +import org.elasticsearch.cluster.ClusterStateUpdateTask; import org.elasticsearch.cluster.LocalNodeMasterListener; +import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.CharArrays; +import org.elasticsearch.common.Priority; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.io.Streams; import org.elasticsearch.threadpool.ThreadPool; @@ -58,12 +62,17 @@ public class ConsistentSecureSettingsValidator implements ClusterStateApplier, L public ConsistentSecureSettingsValidator(Settings settings, ClusterService clusterService, Set> consistentSecureSettings) { this.clusterService = clusterService; + // eagerly compute hashes for the secure settings + computeHashesOfLocalSecureSettingValues(settings, consistentSecureSettings); + } + + private void computeHashesOfLocalSecureSettingValues(Settings settings, Set> consistentSecureSettings) { for (SecureSetting secureSetting : consistentSecureSettings) { // compute unsalted hash to store in memory on the local node final char[] localHash = computeSecureSettingLocalHash(settings, secureSetting); localHashesOfConsistentSettings.put(secureSetting.getKey(), localHash); // salted hash (of the unsalted hash) suitable for cluster-wide publication - final String publicHash = computePublicHash(UUIDs.randomBase64UUID(), localHash); + final String publicHash = computePublicHash(localHash, UUIDs.randomBase64UUID()); publicHashesOfConsistentSettings.put(secureSetting.getKey(), publicHash); } } @@ -72,10 +81,14 @@ public boolean allSecureSettingsConsistent() { return allSecureSettingsConsistent; } + Map publicHashesOfConsistentSettings() { + return publicHashesOfConsistentSettings; + } + /** * Compute UNSALTED hash of a secure setting value for subsequent comparison with the master's reference. This values IS NOT to be - * published in the cluster state, but rather compared (and stored), on the local node, to the SALTED hash value published in the - * cluster state by the master node. + * published in the cluster state, but rather compared (and stored) locally, to the SALTED hash value published in the cluster state by + * the master node. */ private static char[] computeSecureSettingLocalHash(Settings settings, SecureSetting secureSetting) { final Object verbatimValue = secureSetting.get(settings); @@ -131,13 +144,13 @@ private static byte[] computeSaltedHash(char[] value, byte[] salt) { } } - private static String computePublicHash(String salt, char[] localHash) { + private static String computePublicHash(char[] localHash, String salt) { final byte[] saltedHashBytes = computeSaltedHash(localHash, salt.getBytes(StandardCharsets.UTF_8)); final String base64SaltedHashString = new String(saltedHashBytes, StandardCharsets.UTF_8); return salt + ":" + base64SaltedHashString; } - private boolean validatePublicHash(String publishedSettingName, String publishedHashWithSalt) { + private boolean checkPublicHashAgainstLocalValue(String publishedSettingName, String publishedHashWithSalt) { logger.trace("published hash for secure setting [{}] is [{}]", publishedSettingName, publishedHashWithSalt); if (publishedHashWithSalt == null || false == publishedHashWithSalt.contains(":")) { logger.trace("published hash [{}] for secure setting [{}] is invalid", publishedHashWithSalt, publishedSettingName); @@ -154,7 +167,7 @@ private boolean validatePublicHash(String publishedSettingName, String published return false; } final String publishedSalt = parts[0]; - final String localHashWithSalt = computePublicHash(publishedSalt, localHash); + final String localHashWithSalt = computePublicHash(localHash, publishedSalt); logger.trace("local hash for secure setting [{}] is [{}]", publishedSettingName, localHashWithSalt); return publishedHashWithSalt.equals(localHashWithSalt); } @@ -169,7 +182,7 @@ public void applyClusterState(ClusterChangedEvent event) { logger.debug("hashes of secure settings are identical to the master's"); } else { for (Map.Entry publishedSettingAndHash : publishedHashesOfConsistentSettings.entrySet()) { - if (false == validatePublicHash(publishedSettingAndHash.getKey(), publishedSettingAndHash.getValue())) { + if (false == checkPublicHashAgainstLocalValue(publishedSettingAndHash.getKey(), publishedSettingAndHash.getValue())) { allConsistent = false; if (logger.isDebugEnabled()) { logger.debug("secure setting [{}] differs on the local node [{}] compared to master [{}].", @@ -202,26 +215,28 @@ public void applyClusterState(ClusterChangedEvent event) { @Override public void onMaster() { - logger.trace("I have been elected master, publishing hashes of consistent secure settings' values"); -// -// clusterService.submitStateUpdateTask( -// "clean up snapshot restore state", -// new CleanRestoreStateTaskExecutor.Task(entry.uuid()), -// ClusterStateTaskConfig.build(Priority.URGENT), -// cleanRestoreStateTaskExecutor, -// cleanRestoreStateTaskExecutor); -// -// // Submit a job that will start after DEFAULT_STARTING_INTERVAL, and reschedule itself after running -// threadPool.scheduleUnlessShuttingDown(updateFrequency, executorName(), new SubmitReschedulingClusterInfoUpdatedJob()); -// -// try { -// if (clusterService.state().getNodes().getDataNodes().size() > 1) { -// // Submit an info update job to be run immediately -// threadPool.executor(executorName()).execute(() -> maybeRefresh()); -// } -// } catch (EsRejectedExecutionException ex) { -// logger.debug("Couldn't schedule cluster info update task - node might be shutting down", ex); -// } + clusterService.submitStateUpdateTask("publish-secure-settings-hashes", new ClusterStateUpdateTask(Priority.URGENT) { + @Override + public ClusterState execute(ClusterState currentState) { + final Map publishedHashesOfConsistentSettings = currentState.metaData().hashesOfConsistentSettings(); + final Map publicHashesOfConsistentSettings = publicHashesOfConsistentSettings(); + if (publicHashesOfConsistentSettings.equals(publishedHashesOfConsistentSettings)) { + logger.debug("Nothing to publish, what is already published matches this master's view as well"); + return currentState; + } else { + return ClusterState.builder(currentState) + .metaData(MetaData.builder(currentState.metaData()) + .hashesOfConsistentSettings(publicHashesOfConsistentSettings)) + .build(); + } + } + + @Override + public void onFailure(String source, Exception e) { + logger.error("unable to install publish secure settings hashes", e); + } + + }); } @Override @@ -231,7 +246,7 @@ public void offMaster() { @Override public String executorName() { - return ThreadPool.Names.MANAGEMENT; + return ThreadPool.Names.SAME; } } From 4547f9be671cc6e9c90810a6018189e6ed56839c Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 25 Mar 2019 13:42:14 +0200 Subject: [PATCH 08/39] ConsistentSecureSettingsValidatorService --- .../cluster/service/ClusterService.java | 2 +- ...istentSecureSettingsValidatorService.java} | 19 ++++++++++++------- .../java/org/elasticsearch/node/Node.java | 4 ++++ 3 files changed, 17 insertions(+), 8 deletions(-) rename server/src/main/java/org/elasticsearch/common/settings/{ConsistentSecureSettingsValidator.java => ConsistentSecureSettingsValidatorService.java} (95%) diff --git a/server/src/main/java/org/elasticsearch/cluster/service/ClusterService.java b/server/src/main/java/org/elasticsearch/cluster/service/ClusterService.java index f83f2606b14b6..fded43a4bdd19 100644 --- a/server/src/main/java/org/elasticsearch/cluster/service/ClusterService.java +++ b/server/src/main/java/org/elasticsearch/cluster/service/ClusterService.java @@ -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; diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java similarity index 95% rename from server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java rename to server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java index 5e7321f723cce..29c7e632f1eb9 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidator.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java @@ -52,18 +52,23 @@ import javax.crypto.SecretKeyFactory; import javax.crypto.spec.PBEKeySpec; -public class ConsistentSecureSettingsValidator implements ClusterStateApplier, LocalNodeMasterListener { - private static final Logger logger = LogManager.getLogger(ConsistentSecureSettingsValidator.class); +public class ConsistentSecureSettingsValidatorService implements ClusterStateApplier, LocalNodeMasterListener { + private static final Logger logger = LogManager.getLogger(ConsistentSecureSettingsValidatorService.class); private final ClusterService clusterService; private final Map localHashesOfConsistentSettings = new HashMap<>(); private volatile Map publicHashesOfConsistentSettings = new HashMap<>(); private volatile boolean allSecureSettingsConsistent = true; - public ConsistentSecureSettingsValidator(Settings settings, ClusterService clusterService, Set> consistentSecureSettings) { + public ConsistentSecureSettingsValidatorService(Settings settings, ClusterService clusterService, + Set> consistentSecureSettings) { this.clusterService = clusterService; // eagerly compute hashes for the secure settings computeHashesOfLocalSecureSettingValues(settings, consistentSecureSettings); + // this cross checks the published hashes against the local ones + clusterService.addStateApplier(this); + // when this node is the master it publishes hashes + clusterService.addLocalNodeMasterListener(this); } private void computeHashesOfLocalSecureSettingValues(Settings settings, Set> consistentSecureSettings) { @@ -81,10 +86,6 @@ public boolean allSecureSettingsConsistent() { return allSecureSettingsConsistent; } - Map publicHashesOfConsistentSettings() { - return publicHashesOfConsistentSettings; - } - /** * Compute UNSALTED hash of a secure setting value for subsequent comparison with the master's reference. This values IS NOT to be * published in the cluster state, but rather compared (and stored) locally, to the SALTED hash value published in the cluster state by @@ -172,6 +173,10 @@ private boolean checkPublicHashAgainstLocalValue(String publishedSettingName, St return publishedHashWithSalt.equals(localHashWithSalt); } + Map publicHashesOfConsistentSettings() { + return publicHashesOfConsistentSettings; + } + @Override public void applyClusterState(ClusterChangedEvent event) { boolean allConsistent = true; diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 78df7db6be314..76b3f13b71e74 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -72,6 +72,7 @@ import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.network.NetworkService; import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.ConsistentSecureSettingsValidatorService; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.SettingUpgrader; @@ -351,6 +352,8 @@ protected Node( final ClusterService clusterService = new ClusterService(settings, settingsModule.getClusterSettings(), threadPool); clusterService.addStateApplier(scriptModule.getScriptService()); resourcesToClose.add(clusterService); + final ConsistentSecureSettingsValidatorService consistentSecureSettingsValidatorService = new ConsistentSecureSettingsValidatorService( + settings, clusterService, settingsModule.getConsistentSecureSettings()); final IngestService ingestService = new IngestService(clusterService, threadPool, this.environment, scriptModule.getScriptService(), analysisModule.getAnalysisRegistry(), pluginsService.filterPlugins(IngestPlugin.class)); final DiskThresholdMonitor listener = new DiskThresholdMonitor(settings, clusterService::state, @@ -525,6 +528,7 @@ protected Node( b.bind(ScriptService.class).toInstance(scriptModule.getScriptService()); b.bind(AnalysisRegistry.class).toInstance(analysisModule.getAnalysisRegistry()); b.bind(IngestService.class).toInstance(ingestService); + b.bind(ConsistentSecureSettingsValidatorService.class).toInstance(consistentSecureSettingsValidatorService); b.bind(UsageService.class).toInstance(usageService); b.bind(NamedWriteableRegistry.class).toInstance(namedWriteableRegistry); b.bind(MetaDataUpgrader.class).toInstance(metaDataUpgrader); From beed446303621fe26dba0e4c4e49ec39e6c48903 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 25 Mar 2019 15:53:34 +0200 Subject: [PATCH 09/39] Base64 hash --- .../settings/ConsistentSecureSettingsValidatorService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java index 29c7e632f1eb9..df3760d308429 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java @@ -147,7 +147,7 @@ private static byte[] computeSaltedHash(char[] value, byte[] salt) { private static String computePublicHash(char[] localHash, String salt) { final byte[] saltedHashBytes = computeSaltedHash(localHash, salt.getBytes(StandardCharsets.UTF_8)); - final String base64SaltedHashString = new String(saltedHashBytes, StandardCharsets.UTF_8); + final String base64SaltedHashString = new String(Base64.getEncoder().encode(saltedHashBytes), StandardCharsets.UTF_8); return salt + ":" + base64SaltedHashString; } From 933041bf23e8022afb85b3a5a622cba61f7f3c2d Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 25 Mar 2019 16:19:10 +0200 Subject: [PATCH 10/39] Index and Node scoped settings... --- .../java/org/elasticsearch/common/settings/SettingsModule.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index dd4847c0a5551..5acf7f874cb54 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -191,7 +191,8 @@ private void registerSetting(Setting setting) { throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); } } - } else if (setting.hasIndexScope()) { + } + if (setting.hasIndexScope()) { Setting existingSetting = indexSettings.get(setting.getKey()); if (existingSetting != null) { throw new IllegalArgumentException("Cannot register setting [" + setting.getKey() + "] twice"); From 77711ed060224b213f9bf95bbf7352815236d507 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 25 Mar 2019 18:34:48 +0200 Subject: [PATCH 11/39] Checkstyle --- server/src/main/java/org/elasticsearch/node/Node.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 405e61d19a40a..e63ab9832445d 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -352,8 +352,8 @@ protected Node( final ClusterService clusterService = new ClusterService(settings, settingsModule.getClusterSettings(), threadPool); clusterService.addStateApplier(scriptModule.getScriptService()); resourcesToClose.add(clusterService); - final ConsistentSecureSettingsValidatorService consistentSecureSettingsValidatorService = new ConsistentSecureSettingsValidatorService( - settings, clusterService, settingsModule.getConsistentSecureSettings()); + final ConsistentSecureSettingsValidatorService consistentSecureSettingsValidatorService = + new ConsistentSecureSettingsValidatorService(settings, clusterService, settingsModule.getConsistentSecureSettings()); final IngestService ingestService = new IngestService(clusterService, threadPool, this.environment, scriptModule.getScriptService(), analysisModule.getAnalysisRegistry(), pluginsService.filterPlugins(IngestPlugin.class)); final DiskThresholdMonitor listener = new DiskThresholdMonitor(settings, clusterService::state, From 0d2d634f9a3aa199f865f4c60ab92d8d5a79ad07 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Fri, 19 Apr 2019 13:57:23 +0300 Subject: [PATCH 12/39] WIP --- .../common/settings/SecureSetting.java | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index cdd64622b2b14..4ef027a993139 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -19,12 +19,22 @@ package org.elasticsearch.common.settings; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.InputStream; +import java.nio.CharBuffer; +import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; +import java.security.MessageDigest; +import java.util.Arrays; +import java.util.Base64; import java.util.EnumSet; import java.util.Set; import org.elasticsearch.common.Booleans; +import org.elasticsearch.common.CharArrays; +import org.elasticsearch.common.hash.MessageDigests; +import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.util.ArrayUtils; /** @@ -103,6 +113,25 @@ public T get(Settings settings) { /** Returns the value from a fallback setting. Returns null if no fallback exists. */ abstract T getFallback(Settings settings); + abstract char[] computeSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException; + + char[] toBase64EncodedHash(MessageDigest digest) { + byte[] unencodedDigest = null; + byte[] base64EncodedDigest = null; + try { + unencodedDigest = digest.digest(); + base64EncodedDigest = Base64.getEncoder().encode(unencodedDigest); + return CharArrays.utf8BytesToChars(base64EncodedDigest); + } finally { + if (unencodedDigest != null) { + Arrays.fill(unencodedDigest, (byte) 0); + } + if (base64EncodedDigest != null) { + Arrays.fill(base64EncodedDigest, (byte) 0); + } + } + } + // TODO: override toXContent /** @@ -160,6 +189,15 @@ SecureString getFallback(Settings settings) { } return new SecureString(new char[0]); // this means "setting does not exist" } + + @Override + char[] computeSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException { + final MessageDigest digest = MessageDigests.sha256(); + try (SecureString secureString = getSecret(secureSettings)) { + digest.update(StandardCharsets.UTF_8.encode(CharBuffer.wrap(secureString.getChars()))); + } + return toBase64EncodedHash(digest); + } } private static class InsecureStringSetting extends Setting { @@ -200,5 +238,19 @@ InputStream getFallback(Settings settings) { } return null; } + + @Override + char[] computeSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException { + final MessageDigest digest = MessageDigests.sha256(); + try (InputStream is = getSecret(secureSettings)) { + try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { + Streams.copy(is, out); + digest.update(out.toByteArray()); + } + } catch (IOException e) { + throw new RuntimeException("Exception while reading the [" + getKey() + "] secure setting.", e); + } + return toBase64EncodedHash(digest); + } } } From ee6d2591d8f5ae6d80fbbcacabffa45864d10881 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 22 Apr 2019 10:05:48 +0300 Subject: [PATCH 13/39] WIP --- .../common/settings/SecureSetting.java | 42 ++++++++----------- .../common/settings/SettingsModule.java | 14 ++++--- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index 4ef027a993139..6d59c96d61915 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -26,13 +26,10 @@ import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.security.MessageDigest; -import java.util.Arrays; -import java.util.Base64; import java.util.EnumSet; import java.util.Set; import org.elasticsearch.common.Booleans; -import org.elasticsearch.common.CharArrays; import org.elasticsearch.common.hash.MessageDigests; import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.util.ArrayUtils; @@ -107,30 +104,25 @@ public T get(Settings settings) { } } + public byte[] getSecretValueSHA256(Settings settings) { + final SecureSettings secureSettings = settings.getSecureSettings(); + if (secureSettings == null || false == secureSettings.getSettingNames().contains(getKey())) { + return null; + } + try { + return getSecretValueSHA256(secureSettings); + } catch (GeneralSecurityException e) { + throw new RuntimeException("failed to read secure setting " + getKey(), e); + } + } + /** Returns the secret setting from the keyStoreReader store. */ abstract T getSecret(SecureSettings secureSettings) throws GeneralSecurityException; /** Returns the value from a fallback setting. Returns null if no fallback exists. */ abstract T getFallback(Settings settings); - abstract char[] computeSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException; - - char[] toBase64EncodedHash(MessageDigest digest) { - byte[] unencodedDigest = null; - byte[] base64EncodedDigest = null; - try { - unencodedDigest = digest.digest(); - base64EncodedDigest = Base64.getEncoder().encode(unencodedDigest); - return CharArrays.utf8BytesToChars(base64EncodedDigest); - } finally { - if (unencodedDigest != null) { - Arrays.fill(unencodedDigest, (byte) 0); - } - if (base64EncodedDigest != null) { - Arrays.fill(base64EncodedDigest, (byte) 0); - } - } - } + abstract byte[] getSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException; // TODO: override toXContent @@ -191,12 +183,12 @@ SecureString getFallback(Settings settings) { } @Override - char[] computeSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException { + byte[] getSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException { final MessageDigest digest = MessageDigests.sha256(); try (SecureString secureString = getSecret(secureSettings)) { digest.update(StandardCharsets.UTF_8.encode(CharBuffer.wrap(secureString.getChars()))); } - return toBase64EncodedHash(digest); + return digest.digest(); } } @@ -240,7 +232,7 @@ InputStream getFallback(Settings settings) { } @Override - char[] computeSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException { + byte[] getSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException { final MessageDigest digest = MessageDigests.sha256(); try (InputStream is = getSecret(secureSettings)) { try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { @@ -250,7 +242,7 @@ char[] computeSecretValueSHA256(SecureSettings secureSettings) throws GeneralSec } catch (IOException e) { throw new RuntimeException("Exception while reading the [" + getKey() + "] secure setting.", e); } - return toBase64EncodedHash(digest); + return digest.digest(); } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 5acf7f874cb54..14c30762c2bed 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -49,7 +49,7 @@ public class SettingsModule implements Module { private final Set settingsFilterPattern = new HashSet<>(); private final Map> nodeSettings = new HashMap<>(); private final Map> indexSettings = new HashMap<>(); - private final Set> consistentSecureSettings = new HashSet<>(); + private final Map, byte[]> hashesOfConsistentSecureSettings = new HashMap<>(); private final IndexScopedSettings indexScopedSettings; private final ClusterSettings clusterSettings; private final SettingsFilter settingsFilter; @@ -180,13 +180,17 @@ private void registerSetting(Setting setting) { if (setting instanceof Setting.AffixSetting) { ((Setting.AffixSetting)setting).getAllConcreteSettings(this.settings).forEach(concreteSetting -> { if (concreteSetting instanceof SecureSetting) { - consistentSecureSettings.add((SecureSetting) concreteSetting); + final SecureSetting concreteSecureSetting = (SecureSetting) concreteSetting; + hashesOfConsistentSecureSettings.put(concreteSecureSetting, + concreteSecureSetting.getSecretValueSHA256(this.settings)); } else { throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); } }); } else if (setting instanceof SecureSetting) { - consistentSecureSettings.add((SecureSetting) setting); + final SecureSetting concreteSecureSetting = (SecureSetting) setting; + hashesOfConsistentSecureSettings.put(concreteSecureSetting, + concreteSecureSetting.getSecretValueSHA256(this.settings)); } else { throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); } @@ -230,8 +234,8 @@ public ClusterSettings getClusterSettings() { return clusterSettings; } - public Set> getConsistentSecureSettings() { - return consistentSecureSettings; + public Map, byte[]> getHashesOfConsistentSecureSettings() { + return hashesOfConsistentSecureSettings; } public SettingsFilter getSettingsFilter() { From a118c8d414705e619d184c3d17b30484a3bb311b Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 22 Apr 2019 13:18:56 +0300 Subject: [PATCH 14/39] byte[] getSHA256Digest(String setting) throws GeneralSecurityException --- .../common/settings/KeyStoreWrapper.java | 13 +++++++++++++ .../common/settings/SecureSettings.java | 2 ++ .../elasticsearch/common/settings/Settings.java | 9 +++++++-- .../common/settings/MockSecureSettings.java | 13 +++++++++++++ .../notification/NotificationService.java | 17 ++++++++++++----- .../notification/NotificationServiceTests.java | 7 +++++++ 6 files changed, 54 insertions(+), 7 deletions(-) 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 e3fbf30a47ab5..4abef97d8250f 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -67,6 +67,7 @@ import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; import org.elasticsearch.common.Randomness; +import org.elasticsearch.common.hash.MessageDigests; /** * A disk based container for sensitive settings in Elasticsearch. @@ -88,10 +89,12 @@ private enum EntryType { private static class Entry { final EntryType type; final byte[] bytes; + final byte[] bytesSHA256Digest; Entry(EntryType type, byte[] bytes) { this.type = type; this.bytes = bytes; + this.bytesSHA256Digest = MessageDigests.sha256().digest(bytes); } } @@ -545,6 +548,16 @@ public synchronized InputStream getFile(String setting) { return new ByteArrayInputStream(entry.bytes); } + @Override + public byte[] getSHA256Digest(String setting) { + assert entries.get() != null : "Keystore is not loaded"; + final Entry entry = entries.get().get(setting); + if (entry == null) { + throw new IllegalArgumentException("Secret setting " + setting + " does not exist"); + } + return entry.bytesSHA256Digest; + } + /** * Ensure the given setting name is allowed. * diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSettings.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSettings.java index 98f980c1ec6c8..7f92b382dd7b1 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSettings.java @@ -42,6 +42,8 @@ public interface SecureSettings extends Closeable { /** Return a file setting. The {@link InputStream} should be closed once it is used. */ InputStream getFile(String setting) throws GeneralSecurityException; + byte[] getSHA256Digest(String setting) throws GeneralSecurityException; + @Override void close() throws IOException; } diff --git a/server/src/main/java/org/elasticsearch/common/settings/Settings.java b/server/src/main/java/org/elasticsearch/common/settings/Settings.java index 3a7d719105afe..e0e13bc549324 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -1325,15 +1325,20 @@ public Set getSettingNames() { } @Override - public SecureString getString(String setting) throws GeneralSecurityException{ + public SecureString getString(String setting) throws GeneralSecurityException { return delegate.getString(addPrefix.apply(setting)); } @Override - public InputStream getFile(String setting) throws GeneralSecurityException{ + public InputStream getFile(String setting) throws GeneralSecurityException { return delegate.getFile(addPrefix.apply(setting)); } + @Override + public byte[] getSHA256Digest(String setting) throws GeneralSecurityException { + return delegate.getSHA256Digest(addPrefix.apply(setting)); + } + @Override public void close() throws IOException { delegate.close(); diff --git a/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java b/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java index 3a6161a9f7fa0..84689cf223d20 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java +++ b/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java @@ -19,9 +19,12 @@ package org.elasticsearch.common.settings; +import org.elasticsearch.common.hash.MessageDigests; + import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -35,6 +38,7 @@ public class MockSecureSettings implements SecureSettings { private Map secureStrings = new HashMap<>(); private Map files = new HashMap<>(); + private Map sha256Digests = new HashMap<>(); private Set settingNames = new HashSet<>(); private final AtomicBoolean closed = new AtomicBoolean(false); @@ -44,6 +48,7 @@ public MockSecureSettings() { private MockSecureSettings(MockSecureSettings source) { secureStrings.putAll(source.secureStrings); files.putAll(source.files); + sha256Digests.putAll(source.sha256Digests); settingNames.addAll(source.settingNames); } @@ -69,15 +74,22 @@ public InputStream getFile(String setting) { return new ByteArrayInputStream(files.get(setting)); } + @Override + public byte[] getSHA256Digest(String setting) { + return sha256Digests.get(setting); + } + public void setString(String setting, String value) { ensureOpen(); secureStrings.put(setting, new SecureString(value.toCharArray())); + sha256Digests.put(setting, MessageDigests.sha256().digest(value.getBytes(StandardCharsets.UTF_8))); settingNames.add(setting); } public void setFile(String setting, byte[] value) { ensureOpen(); files.put(setting, value); + sha256Digests.put(setting, MessageDigests.sha256().digest(value)); settingNames.add(setting); } @@ -90,6 +102,7 @@ public void merge(MockSecureSettings secureSettings) { } settingNames.addAll(secureSettings.settingNames); secureStrings.putAll(secureSettings.secureStrings); + sha256Digests.putAll(secureSettings.sha256Digests); files.putAll(secureSettings.files); } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java index c2a079e519f0f..c6c041a6571b1 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java @@ -8,6 +8,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.SecureSettings; import org.elasticsearch.common.settings.SecureString; @@ -179,12 +180,13 @@ private static SecureSettings extractSecureSettings(Settings source, List cache = new HashMap<>(); + final Map> cache = new HashMap<>(); if (sourceSecureSettings != null && securePluginSettings != null) { for (final String settingKey : sourceSecureSettings.getSettingNames()) { for (final Setting secureSetting : securePluginSettings) { if (secureSetting.match(settingKey)) { - cache.put(settingKey, sourceSecureSettings.getString(settingKey)); + cache.put(settingKey, + new Tuple<>(sourceSecureSettings.getString(settingKey), sourceSecureSettings.getSHA256Digest(settingKey))); } } } @@ -197,8 +199,8 @@ public boolean isLoaded() { } @Override - public SecureString getString(String setting) throws GeneralSecurityException { - return cache.get(setting); + public SecureString getString(String setting) { + return cache.get(setting).v1(); } @Override @@ -207,10 +209,15 @@ public Set getSettingNames() { } @Override - public InputStream getFile(String setting) throws GeneralSecurityException { + public InputStream getFile(String setting) { throw new IllegalStateException("A NotificationService setting cannot be File."); } + @Override + public byte[] getSHA256Digest(String setting) { + return cache.get(setting).v2(); + } + @Override public void close() throws IOException { } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/NotificationServiceTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/NotificationServiceTests.java index efbefdd640893..0fa05e900e518 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/NotificationServiceTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/NotificationServiceTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.watcher.notification; +import org.elasticsearch.common.hash.MessageDigests; import org.elasticsearch.common.settings.SecureSetting; import org.elasticsearch.common.settings.SecureSettings; import org.elasticsearch.common.settings.SecureString; @@ -16,6 +17,7 @@ import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.util.Arrays; import java.util.Collections; @@ -247,6 +249,11 @@ public InputStream getFile(String setting) throws GeneralSecurityException { return null; } + @Override + public byte[] getSHA256Digest(String setting) throws GeneralSecurityException { + return MessageDigests.sha256().digest(new String(secureSettingsMap.get(setting)).getBytes(StandardCharsets.UTF_8)); + } + @Override public void close() throws IOException { } From 92c08edf40e2955f7e9d3fa532a813030a778a75 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 22 Apr 2019 13:53:15 +0300 Subject: [PATCH 15/39] SettingsModule --- .../common/settings/SettingsModule.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 14c30762c2bed..4546262d1e7f9 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -49,7 +49,7 @@ public class SettingsModule implements Module { private final Set settingsFilterPattern = new HashSet<>(); private final Map> nodeSettings = new HashMap<>(); private final Map> indexSettings = new HashMap<>(); - private final Map, byte[]> hashesOfConsistentSecureSettings = new HashMap<>(); + private final Set> consistentSecureSettings = new HashSet<>(); private final IndexScopedSettings indexScopedSettings; private final ClusterSettings clusterSettings; private final SettingsFilter settingsFilter; @@ -180,19 +180,15 @@ private void registerSetting(Setting setting) { if (setting instanceof Setting.AffixSetting) { ((Setting.AffixSetting)setting).getAllConcreteSettings(this.settings).forEach(concreteSetting -> { if (concreteSetting instanceof SecureSetting) { - final SecureSetting concreteSecureSetting = (SecureSetting) concreteSetting; - hashesOfConsistentSecureSettings.put(concreteSecureSetting, - concreteSecureSetting.getSecretValueSHA256(this.settings)); + consistentSecureSettings.add((SecureSetting) concreteSetting); } else { - throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); + throw new IllegalArgumentException("Unrecognized consistent setting [" + setting.getKey() + "]"); } }); } else if (setting instanceof SecureSetting) { - final SecureSetting concreteSecureSetting = (SecureSetting) setting; - hashesOfConsistentSecureSettings.put(concreteSecureSetting, - concreteSecureSetting.getSecretValueSHA256(this.settings)); + consistentSecureSettings.add((SecureSetting) setting); } else { - throw new RuntimeException("Unrecognized consistent setting [" + setting.getKey() + "]"); + throw new IllegalArgumentException("Unrecognized consistent setting [" + setting.getKey() + "]"); } } } @@ -201,6 +197,9 @@ private void registerSetting(Setting setting) { if (existingSetting != null) { throw new IllegalArgumentException("Cannot register setting [" + setting.getKey() + "] twice"); } + if (setting.isConsistent()) { + throw new IllegalArgumentException("Consistent setting [" + setting.getKey() + "] cannot be index scoped"); + } indexSettings.put(setting.getKey(), setting); } } else { @@ -234,8 +233,8 @@ public ClusterSettings getClusterSettings() { return clusterSettings; } - public Map, byte[]> getHashesOfConsistentSecureSettings() { - return hashesOfConsistentSecureSettings; + public Set> getConsistentSecureSettings() { + return consistentSecureSettings; } public SettingsFilter getSettingsFilter() { From 33ccb81a500c5315ce007366903645f06ef8fd11 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 22 Apr 2019 14:15:14 +0300 Subject: [PATCH 16/39] SecureSetting --- .../common/settings/SecureSetting.java | 34 +------------------ 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index 6d59c96d61915..f7ff2389da17b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -19,19 +19,12 @@ package org.elasticsearch.common.settings; -import java.io.ByteArrayOutputStream; -import java.io.IOException; import java.io.InputStream; -import java.nio.CharBuffer; -import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; -import java.security.MessageDigest; import java.util.EnumSet; import java.util.Set; import org.elasticsearch.common.Booleans; -import org.elasticsearch.common.hash.MessageDigests; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.util.ArrayUtils; /** @@ -110,7 +103,7 @@ public byte[] getSecretValueSHA256(Settings settings) { return null; } try { - return getSecretValueSHA256(secureSettings); + return secureSettings.getSHA256Digest(getKey()); } catch (GeneralSecurityException e) { throw new RuntimeException("failed to read secure setting " + getKey(), e); } @@ -122,8 +115,6 @@ public byte[] getSecretValueSHA256(Settings settings) { /** Returns the value from a fallback setting. Returns null if no fallback exists. */ abstract T getFallback(Settings settings); - abstract byte[] getSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException; - // TODO: override toXContent /** @@ -181,15 +172,6 @@ SecureString getFallback(Settings settings) { } return new SecureString(new char[0]); // this means "setting does not exist" } - - @Override - byte[] getSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException { - final MessageDigest digest = MessageDigests.sha256(); - try (SecureString secureString = getSecret(secureSettings)) { - digest.update(StandardCharsets.UTF_8.encode(CharBuffer.wrap(secureString.getChars()))); - } - return digest.digest(); - } } private static class InsecureStringSetting extends Setting { @@ -230,19 +212,5 @@ InputStream getFallback(Settings settings) { } return null; } - - @Override - byte[] getSecretValueSHA256(SecureSettings secureSettings) throws GeneralSecurityException { - final MessageDigest digest = MessageDigests.sha256(); - try (InputStream is = getSecret(secureSettings)) { - try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { - Streams.copy(is, out); - digest.update(out.toByteArray()); - } - } catch (IOException e) { - throw new RuntimeException("Exception while reading the [" + getKey() + "] secure setting.", e); - } - return digest.digest(); - } } } From 26b1f4503820b69f09a6aa3afed9382a8b535f64 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 23 Apr 2019 12:52:27 +0300 Subject: [PATCH 17/39] ConsistentSettingsService --- .../common/hash/MessageDigests.java | 19 +- ...sistentSecureSettingsValidatorService.java | 257 ------------------ .../settings/ConsistentSettingsService.java | 199 ++++++++++++++ .../common/settings/SettingsModule.java | 20 +- .../java/org/elasticsearch/node/Node.java | 8 +- 5 files changed, 226 insertions(+), 277 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java create mode 100644 server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java diff --git a/server/src/main/java/org/elasticsearch/common/hash/MessageDigests.java b/server/src/main/java/org/elasticsearch/common/hash/MessageDigests.java index 8bcef7b8ff4cb..df8f3e2fa7f43 100644 --- a/server/src/main/java/org/elasticsearch/common/hash/MessageDigests.java +++ b/server/src/main/java/org/elasticsearch/common/hash/MessageDigests.java @@ -95,15 +95,24 @@ private static MessageDigest get(ThreadLocal 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; } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java deleted file mode 100644 index df3760d308429..0000000000000 --- a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSecureSettingsValidatorService.java +++ /dev/null @@ -1,257 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.common.settings; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.elasticsearch.cluster.ClusterChangedEvent; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.ClusterStateApplier; -import org.elasticsearch.cluster.ClusterStateUpdateTask; -import org.elasticsearch.cluster.LocalNodeMasterListener; -import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.CharArrays; -import org.elasticsearch.common.Priority; -import org.elasticsearch.common.UUIDs; -import org.elasticsearch.common.io.Streams; -import org.elasticsearch.threadpool.ThreadPool; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.nio.CharBuffer; -import java.nio.charset.StandardCharsets; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; -import java.security.spec.InvalidKeySpecException; -import java.util.Arrays; -import java.util.Base64; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; - -import javax.crypto.SecretKey; -import javax.crypto.SecretKeyFactory; -import javax.crypto.spec.PBEKeySpec; - -public class ConsistentSecureSettingsValidatorService implements ClusterStateApplier, LocalNodeMasterListener { - private static final Logger logger = LogManager.getLogger(ConsistentSecureSettingsValidatorService.class); - - private final ClusterService clusterService; - private final Map localHashesOfConsistentSettings = new HashMap<>(); - private volatile Map publicHashesOfConsistentSettings = new HashMap<>(); - private volatile boolean allSecureSettingsConsistent = true; - - public ConsistentSecureSettingsValidatorService(Settings settings, ClusterService clusterService, - Set> consistentSecureSettings) { - this.clusterService = clusterService; - // eagerly compute hashes for the secure settings - computeHashesOfLocalSecureSettingValues(settings, consistentSecureSettings); - // this cross checks the published hashes against the local ones - clusterService.addStateApplier(this); - // when this node is the master it publishes hashes - clusterService.addLocalNodeMasterListener(this); - } - - private void computeHashesOfLocalSecureSettingValues(Settings settings, Set> consistentSecureSettings) { - for (SecureSetting secureSetting : consistentSecureSettings) { - // compute unsalted hash to store in memory on the local node - final char[] localHash = computeSecureSettingLocalHash(settings, secureSetting); - localHashesOfConsistentSettings.put(secureSetting.getKey(), localHash); - // salted hash (of the unsalted hash) suitable for cluster-wide publication - final String publicHash = computePublicHash(localHash, UUIDs.randomBase64UUID()); - publicHashesOfConsistentSettings.put(secureSetting.getKey(), publicHash); - } - } - - public boolean allSecureSettingsConsistent() { - return allSecureSettingsConsistent; - } - - /** - * Compute UNSALTED hash of a secure setting value for subsequent comparison with the master's reference. This values IS NOT to be - * published in the cluster state, but rather compared (and stored) locally, to the SALTED hash value published in the cluster state by - * the master node. - */ - private static char[] computeSecureSettingLocalHash(Settings settings, SecureSetting secureSetting) { - final Object verbatimValue = secureSetting.get(settings); - final MessageDigest digest; - try { - digest = MessageDigest.getInstance("SHA-512"); - } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("\"SHA-512\" algorithm is required for consistent secure settings", e); - } - if (verbatimValue instanceof InputStream) { - try (InputStream is = ((InputStream)verbatimValue)) { - try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { - Streams.copy(is, out); - digest.update(out.toByteArray()); - } - } catch (IOException e) { - throw new RuntimeException("Exception while reading the [" + secureSetting.getKey() + "] secure setting.", e); - } - } else if (verbatimValue instanceof SecureString) { - try (SecureString secureString = ((SecureString)verbatimValue)) { - digest.update(StandardCharsets.UTF_8.encode(CharBuffer.wrap(secureString.getChars()))); - } - } else { - throw new IllegalArgumentException("Unrecognized consistent secure setting [" + secureSetting.getKey() + "]"); - } - // computing a base64 encoded unsalted SHA512 hash - byte[] unencodedSHA512AsByteArray = null; - byte[] SHA512EncodedBase64AsByteArray = null; - try { - unencodedSHA512AsByteArray = digest.digest(); - SHA512EncodedBase64AsByteArray = Base64.getEncoder().encode(unencodedSHA512AsByteArray); - return CharArrays.utf8BytesToChars(SHA512EncodedBase64AsByteArray); - } finally { - if (unencodedSHA512AsByteArray != null) { - Arrays.fill(unencodedSHA512AsByteArray, (byte) 0); - } - if (SHA512EncodedBase64AsByteArray != null) { - Arrays.fill(SHA512EncodedBase64AsByteArray, (byte) 0); - } - } - } - - private static byte[] computeSaltedHash(char[] value, byte[] salt) { - final int iterations = 5000; - final int keyLength = 512; - try { - final SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA512"); - final PBEKeySpec spec = new PBEKeySpec(value, salt, iterations, keyLength); - final SecretKey key = skf.generateSecret(spec); - return key.getEncoded(); - } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { - throw new RuntimeException("\"PBKDF2WithHmacSHA512\" algorithm is required for consistent secure settings", e); - } - } - - private static String computePublicHash(char[] localHash, String salt) { - final byte[] saltedHashBytes = computeSaltedHash(localHash, salt.getBytes(StandardCharsets.UTF_8)); - final String base64SaltedHashString = new String(Base64.getEncoder().encode(saltedHashBytes), StandardCharsets.UTF_8); - return salt + ":" + base64SaltedHashString; - } - - private boolean checkPublicHashAgainstLocalValue(String publishedSettingName, String publishedHashWithSalt) { - logger.trace("published hash for secure setting [{}] is [{}]", publishedSettingName, publishedHashWithSalt); - if (publishedHashWithSalt == null || false == publishedHashWithSalt.contains(":")) { - logger.trace("published hash [{}] for secure setting [{}] is invalid", publishedHashWithSalt, publishedSettingName); - return false; - } - final String[] parts = publishedHashWithSalt.split(":"); - if (parts == null || parts.length != 2) { - logger.trace("published hash [{}] for secure setting [{}] is invalid", publishedHashWithSalt, publishedSettingName); - return false; - } - final char[] localHash = localHashesOfConsistentSettings.get(publishedSettingName); - if (localHash == null) { - logger.trace("secure setting [{}] is missing on the local node", publishedSettingName); - return false; - } - final String publishedSalt = parts[0]; - final String localHashWithSalt = computePublicHash(localHash, publishedSalt); - logger.trace("local hash for secure setting [{}] is [{}]", publishedSettingName, localHashWithSalt); - return publishedHashWithSalt.equals(localHashWithSalt); - } - - Map publicHashesOfConsistentSettings() { - return publicHashesOfConsistentSettings; - } - - @Override - public void applyClusterState(ClusterChangedEvent event) { - boolean allConsistent = true; - final Map publishedHashesOfConsistentSettings = event.state().metaData().hashesOfConsistentSettings(); - if (event.localNodeMaster()) { - logger.debug("local node is elected master, no hashes consistency check is required"); - } else if (publishedHashesOfConsistentSettings.equals(publicHashesOfConsistentSettings)) { - logger.debug("hashes of secure settings are identical to the master's"); - } else { - for (Map.Entry publishedSettingAndHash : publishedHashesOfConsistentSettings.entrySet()) { - if (false == checkPublicHashAgainstLocalValue(publishedSettingAndHash.getKey(), publishedSettingAndHash.getValue())) { - allConsistent = false; - if (logger.isDebugEnabled()) { - logger.debug("secure setting [{}] differs on the local node [{}] compared to master [{}].", - publishedSettingAndHash.getKey(), event.state().nodes().getLocalNodeId(), - event.state().nodes().getMasterNodeId()); - } else { - break; - } - } - } - // also verify that local node does not hold a superset of the master's - for (String localSecureSettingName : localHashesOfConsistentSettings.keySet()) { - if (false == publishedHashesOfConsistentSettings.containsKey(localSecureSettingName)) { - allConsistent = false; - if (logger.isTraceEnabled()) { - logger.trace("secure setting [{}] is missing on master, but it is present on the local node", - localSecureSettingName); - } else { - break; - } - } - } - // public hashes of master and local node are equivalent but salts differ. Copy master's to local to speedup future checks - if (allConsistent) { - publicHashesOfConsistentSettings = publishedHashesOfConsistentSettings; - } - } - allSecureSettingsConsistent = allConsistent; - } - - @Override - public void onMaster() { - clusterService.submitStateUpdateTask("publish-secure-settings-hashes", new ClusterStateUpdateTask(Priority.URGENT) { - @Override - public ClusterState execute(ClusterState currentState) { - final Map publishedHashesOfConsistentSettings = currentState.metaData().hashesOfConsistentSettings(); - final Map publicHashesOfConsistentSettings = publicHashesOfConsistentSettings(); - if (publicHashesOfConsistentSettings.equals(publishedHashesOfConsistentSettings)) { - logger.debug("Nothing to publish, what is already published matches this master's view as well"); - return currentState; - } else { - return ClusterState.builder(currentState) - .metaData(MetaData.builder(currentState.metaData()) - .hashesOfConsistentSettings(publicHashesOfConsistentSettings)) - .build(); - } - } - - @Override - public void onFailure(String source, Exception e) { - logger.error("unable to install publish secure settings hashes", e); - } - - }); - } - - @Override - public void offMaster() { - logger.trace("I am no longer master, nothing to do"); - } - - @Override - public String executorName() { - return ThreadPool.Names.SAME; - } - -} diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java new file mode 100644 index 0000000000000..3246d3f904753 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java @@ -0,0 +1,199 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateUpdateTask; +import org.elasticsearch.cluster.LocalNodeMasterListener; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Priority; +import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.hash.MessageDigests; +import org.elasticsearch.threadpool.ThreadPool; + +import java.nio.charset.StandardCharsets; +import java.security.NoSuchAlgorithmException; +import java.security.spec.InvalidKeySpecException; +import java.util.Arrays; +import java.util.Base64; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; + +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; + +public final class ConsistentSettingsService { + private static final Logger logger = LogManager.getLogger(ConsistentSettingsService.class); + + private final Settings settings; + private final ClusterService clusterService; + private final Collection> secureSettingsCollection; + private final SecretKeyFactory PBKDF2KeyFactory; + + public ConsistentSettingsService(Settings settings, ClusterService clusterService, + Collection> secureSettingsCollection) { + this.settings = settings; + this.clusterService = clusterService; + this.secureSettingsCollection = secureSettingsCollection; + // this is used to compute the PBKDF2 hash (the published one) + try { + this.PBKDF2KeyFactory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA512"); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException("The \"PBKDF2WithHmacSHA512\" algorithm is required for consistent secure settings' hashes", e); + } + } + + public LocalNodeMasterListener newHashPublisher() { + return new LocalNodeMasterListener() { + + // eagerly compute hashes to be published + final Map computedHashesOfConsistentSettings = computeHashesOfConsistentSecureSettings(); + + @Override + public void onMaster() { + clusterService.submitStateUpdateTask("publish-secure-settings-hashes", new ClusterStateUpdateTask(Priority.URGENT) { + @Override + public ClusterState execute(ClusterState currentState) { + final Map publishedHashesOfConsistentSettings = currentState.metaData() + .hashesOfConsistentSettings(); + if (computedHashesOfConsistentSettings.equals(publishedHashesOfConsistentSettings)) { + logger.debug("Nothing to publish. What is already published matches this master's view."); + return currentState; + } else { + return ClusterState.builder(currentState).metaData(MetaData.builder(currentState.metaData()) + .hashesOfConsistentSettings(computedHashesOfConsistentSettings)).build(); + } + } + + @Override + public void onFailure(String source, Exception e) { + logger.error("unable to publish secure settings hashes", e); + } + + }); + } + + @Override + public void offMaster() { + logger.trace("I am no longer master, nothing to do"); + } + + @Override + public String executorName() { + return ThreadPool.Names.SAME; + } + }; + } + + public boolean areAllConsistent() { + final ClusterState state = clusterService.state(); + final Map publishedHashesOfConsistentSettings = state.metaData().hashesOfConsistentSettings(); + final AtomicBoolean allConsistent = new AtomicBoolean(true); + forEachConcreteSecureSettingDo(concreteSecureSetting -> { + final String publishedSaltAndHash = publishedHashesOfConsistentSettings.get(concreteSecureSetting.getKey()); + if (publishedSaltAndHash == null) { + logger.warn("no published hash for consistent secure setting [{}]", concreteSecureSetting.getKey()); + if (state.nodes().isLocalNodeElectedMaster()) { + throw new IllegalStateException("Master node cannot validate consistent setting. No published hash for [" + + concreteSecureSetting.getKey() + "]."); + } + allConsistent.set(false); + } else { + final String[] parts = publishedSaltAndHash.split(":"); + if (parts == null || parts.length != 2) { + throw new IllegalArgumentException("published hash [" + publishedSaltAndHash + " ] for secure setting [" + + concreteSecureSetting.getKey() + "] is invalid"); + } + final String publishedSalt = parts[0]; + final String publishedHash = parts[1]; + final byte[] localHash = concreteSecureSetting.getSecretValueSHA256(settings); + final byte[] computedSaltedHashBytes = computeSaltedPBKDF2Hash(localHash, publishedSalt.getBytes(StandardCharsets.UTF_8)); + final String computedSaltedHash = new String(Base64.getEncoder().encode(computedSaltedHashBytes), StandardCharsets.UTF_8); + if (false == publishedHash.equals(computedSaltedHash)) { + logger.warn("the published hash [{}] of the consistent secure setting [{}] differs from the locally computed one [{}]", + publishedHash, concreteSecureSetting.getKey(), computedSaltedHash); + if (state.nodes().isLocalNodeElectedMaster()) { + throw new IllegalStateException("Master node cannot validate consistent setting. The published hash [" + + publishedHash + "] of the consistent secure setting [" + concreteSecureSetting.getKey() + + "] differs from the locally computed one [" + computedSaltedHash + "]."); + } + allConsistent.set(false); + } + } + }); + return allConsistent.get(); + } + + private void forEachConcreteSecureSettingDo(Consumer> secureSettingConsumer) { + for (Setting setting : secureSettingsCollection) { + assert setting.isConsistent() : "[" + setting.getKey() + "] is not a consistent setting"; + if (setting instanceof Setting.AffixSetting) { + ((Setting.AffixSetting)setting).getAllConcreteSettings(settings).forEach(concreteSetting -> { + assert concreteSetting instanceof SecureSetting : "[" + concreteSetting.getKey() + "] is not a secure setting"; + secureSettingConsumer.accept((SecureSetting)concreteSetting); + }); + } else if (setting instanceof SecureSetting) { + secureSettingConsumer.accept((SecureSetting) setting); + } else { + assert false : "Unrecognized consistent secure setting [" + setting.getKey() + "]"; + } + } + } + + private Map computeHashesOfConsistentSecureSettings() { + final Map result = new HashMap<>(); + forEachConcreteSecureSettingDo(concreteSecureSetting -> { + final byte[] localHash = concreteSecureSetting.getSecretValueSHA256(settings); + if (localHash != null) { + final String salt = UUIDs.randomBase64UUID(); + final byte[] publicHash = computeSaltedPBKDF2Hash(localHash, salt.getBytes(StandardCharsets.UTF_8)); + final String encodedPublicHash = new String(Base64.getEncoder().encode(publicHash), StandardCharsets.UTF_8); + result.put(concreteSecureSetting.getKey(), salt + ":" + encodedPublicHash); + } + }); + return result; + } + + private byte[] computeSaltedPBKDF2Hash(byte[] bytes, byte[] salt) { + final int iterations = 5000; + final int keyLength = 512; + char[] value = null; + try { + value = MessageDigests.toHexCharArray(bytes); + final PBEKeySpec spec = new PBEKeySpec(value, salt, iterations, keyLength); + final SecretKey key = PBKDF2KeyFactory.generateSecret(spec); + return key.getEncoded(); + } catch (InvalidKeySpecException e) { + throw new RuntimeException("Unexpected exception when computing PBKDF2 hash", e); + } finally { + if (value != null) { + Arrays.fill(value, '0'); + } + } + } + +} diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 4546262d1e7f9..c4aab2bf9b5a6 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -49,7 +49,7 @@ public class SettingsModule implements Module { private final Set settingsFilterPattern = new HashSet<>(); private final Map> nodeSettings = new HashMap<>(); private final Map> indexSettings = new HashMap<>(); - private final Set> consistentSecureSettings = new HashSet<>(); + private final Set> consistentSecureSettings = new HashSet<>(); private final IndexScopedSettings indexScopedSettings; private final ClusterSettings clusterSettings; private final SettingsFilter settingsFilter; @@ -178,17 +178,15 @@ private void registerSetting(Setting setting) { nodeSettings.put(setting.getKey(), setting); if (setting.isConsistent()) { if (setting instanceof Setting.AffixSetting) { - ((Setting.AffixSetting)setting).getAllConcreteSettings(this.settings).forEach(concreteSetting -> { - if (concreteSetting instanceof SecureSetting) { - consistentSecureSettings.add((SecureSetting) concreteSetting); - } else { - throw new IllegalArgumentException("Unrecognized consistent setting [" + setting.getKey() + "]"); - } - }); + if (((Setting.AffixSetting)setting).getConcreteSettingForNamespace("_na_") instanceof SecureSetting) { + consistentSecureSettings.add(setting); + } else { + throw new IllegalArgumentException("Invalid consistent secure setting [" + setting.getKey() + "]"); + } } else if (setting instanceof SecureSetting) { - consistentSecureSettings.add((SecureSetting) setting); + consistentSecureSettings.add(setting); } else { - throw new IllegalArgumentException("Unrecognized consistent setting [" + setting.getKey() + "]"); + throw new IllegalArgumentException("Invalid consistent secure setting [" + setting.getKey() + "]"); } } } @@ -233,7 +231,7 @@ public ClusterSettings getClusterSettings() { return clusterSettings; } - public Set> getConsistentSecureSettings() { + public Set> getConsistentSecureSettings() { return consistentSecureSettings; } diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 70dde6715bf71..bbb0be360da21 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -72,7 +72,7 @@ import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.network.NetworkService; import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.ConsistentSecureSettingsValidatorService; +import org.elasticsearch.common.settings.ConsistentSettingsService; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.SettingUpgrader; @@ -351,8 +351,8 @@ protected Node( final ClusterService clusterService = new ClusterService(settings, settingsModule.getClusterSettings(), threadPool); clusterService.addStateApplier(scriptModule.getScriptService()); resourcesToClose.add(clusterService); - final ConsistentSecureSettingsValidatorService consistentSecureSettingsValidatorService = - new ConsistentSecureSettingsValidatorService(settings, clusterService, settingsModule.getConsistentSecureSettings()); + final ConsistentSettingsService consistentSecureSettingsValidatorService = + new ConsistentSettingsService(settings, clusterService, settingsModule.getConsistentSecureSettings()); final IngestService ingestService = new IngestService(clusterService, threadPool, this.environment, scriptModule.getScriptService(), analysisModule.getAnalysisRegistry(), pluginsService.filterPlugins(IngestPlugin.class)); final DiskThresholdMonitor listener = new DiskThresholdMonitor(settings, clusterService::state, @@ -527,7 +527,7 @@ protected Node( b.bind(ScriptService.class).toInstance(scriptModule.getScriptService()); b.bind(AnalysisRegistry.class).toInstance(analysisModule.getAnalysisRegistry()); b.bind(IngestService.class).toInstance(ingestService); - b.bind(ConsistentSecureSettingsValidatorService.class).toInstance(consistentSecureSettingsValidatorService); + b.bind(ConsistentSettingsService.class).toInstance(consistentSecureSettingsValidatorService); b.bind(UsageService.class).toInstance(usageService); b.bind(NamedWriteableRegistry.class).toInstance(namedWriteableRegistry); b.bind(MetaDataUpgrader.class).toInstance(metaDataUpgrader); From e889da99c7b753c3b20e33dabd3ac8b98f62f03b Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 23 Apr 2019 14:10:39 +0300 Subject: [PATCH 18/39] ConsistentSettingsService++ --- .../settings/ConsistentSettingsService.java | 27 ++++++++++++++----- .../java/org/elasticsearch/node/Node.java | 5 ++-- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java index 3246d3f904753..7c5daa24b2a6a 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java @@ -115,14 +115,28 @@ public boolean areAllConsistent() { final AtomicBoolean allConsistent = new AtomicBoolean(true); forEachConcreteSecureSettingDo(concreteSecureSetting -> { final String publishedSaltAndHash = publishedHashesOfConsistentSettings.get(concreteSecureSetting.getKey()); - if (publishedSaltAndHash == null) { - logger.warn("no published hash for consistent secure setting [{}]", concreteSecureSetting.getKey()); + final byte[] localHash = concreteSecureSetting.getSecretValueSHA256(settings); + if (publishedSaltAndHash == null && localHash == null) { + // consistency of missing + logger.debug("no published hash for the consistent secure setting [{}] but also it does NOT exist on the local node", + concreteSecureSetting.getKey()); + } else if (publishedSaltAndHash == null && localHash != null) { + // setting missing on master but present locally + logger.warn("no published hash for the consistent secure setting [{}] but it exists on the local node", + concreteSecureSetting.getKey()); if (state.nodes().isLocalNodeElectedMaster()) { throw new IllegalStateException("Master node cannot validate consistent setting. No published hash for [" - + concreteSecureSetting.getKey() + "]."); + + concreteSecureSetting.getKey() + "] but setting exists."); } allConsistent.set(false); + } else if (publishedSaltAndHash != null && localHash == null) { + // setting missing locally but present on master + logger.warn("the consistent secure setting [{}] does not exist on the local node but there is published hash for it", + concreteSecureSetting.getKey()); + allConsistent.set(false); } else { + assert publishedSaltAndHash != null; + assert localHash != null; final String[] parts = publishedSaltAndHash.split(":"); if (parts == null || parts.length != 2) { throw new IllegalArgumentException("published hash [" + publishedSaltAndHash + " ] for secure setting [" @@ -130,7 +144,6 @@ public boolean areAllConsistent() { } final String publishedSalt = parts[0]; final String publishedHash = parts[1]; - final byte[] localHash = concreteSecureSetting.getSecretValueSHA256(settings); final byte[] computedSaltedHashBytes = computeSaltedPBKDF2Hash(localHash, publishedSalt.getBytes(StandardCharsets.UTF_8)); final String computedSaltedHash = new String(Base64.getEncoder().encode(computedSaltedHashBytes), StandardCharsets.UTF_8); if (false == publishedHash.equals(computedSaltedHash)) { @@ -165,17 +178,17 @@ private void forEachConcreteSecureSettingDo(Consumer> secureSet } private Map computeHashesOfConsistentSecureSettings() { - final Map result = new HashMap<>(); + final Map hashesBySettingKey = new HashMap<>(); forEachConcreteSecureSettingDo(concreteSecureSetting -> { final byte[] localHash = concreteSecureSetting.getSecretValueSHA256(settings); if (localHash != null) { final String salt = UUIDs.randomBase64UUID(); final byte[] publicHash = computeSaltedPBKDF2Hash(localHash, salt.getBytes(StandardCharsets.UTF_8)); final String encodedPublicHash = new String(Base64.getEncoder().encode(publicHash), StandardCharsets.UTF_8); - result.put(concreteSecureSetting.getKey(), salt + ":" + encodedPublicHash); + hashesBySettingKey.put(concreteSecureSetting.getKey(), salt + ":" + encodedPublicHash); } }); - return result; + return hashesBySettingKey; } private byte[] computeSaltedPBKDF2Hash(byte[] bytes, byte[] salt) { diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index bbb0be360da21..24e0ab8a370c4 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -351,8 +351,8 @@ protected Node( final ClusterService clusterService = new ClusterService(settings, settingsModule.getClusterSettings(), threadPool); clusterService.addStateApplier(scriptModule.getScriptService()); resourcesToClose.add(clusterService); - final ConsistentSettingsService consistentSecureSettingsValidatorService = - new ConsistentSettingsService(settings, clusterService, settingsModule.getConsistentSecureSettings()); + final ConsistentSettingsService consistentSettingsService = new ConsistentSettingsService(settings, clusterService, + settingsModule.getConsistentSecureSettings()); final IngestService ingestService = new IngestService(clusterService, threadPool, this.environment, scriptModule.getScriptService(), analysisModule.getAnalysisRegistry(), pluginsService.filterPlugins(IngestPlugin.class)); final DiskThresholdMonitor listener = new DiskThresholdMonitor(settings, clusterService::state, @@ -527,7 +527,6 @@ protected Node( b.bind(ScriptService.class).toInstance(scriptModule.getScriptService()); b.bind(AnalysisRegistry.class).toInstance(analysisModule.getAnalysisRegistry()); b.bind(IngestService.class).toInstance(ingestService); - b.bind(ConsistentSettingsService.class).toInstance(consistentSecureSettingsValidatorService); b.bind(UsageService.class).toInstance(usageService); b.bind(NamedWriteableRegistry.class).toInstance(namedWriteableRegistry); b.bind(MetaDataUpgrader.class).toInstance(metaDataUpgrader); From 23d76af39564e3bfb40190dcd7fee7c609ffca58 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 23 Apr 2019 14:40:00 +0300 Subject: [PATCH 19/39] ConsistentSettingsService++ --- .../common/settings/ConsistentSettingsService.java | 4 ++-- .../org/elasticsearch/common/settings/SettingsModule.java | 2 +- server/src/main/java/org/elasticsearch/node/Node.java | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java index 7c5daa24b2a6a..82e24a7a2c0ee 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java @@ -55,7 +55,7 @@ public final class ConsistentSettingsService { private final SecretKeyFactory PBKDF2KeyFactory; public ConsistentSettingsService(Settings settings, ClusterService clusterService, - Collection> secureSettingsCollection) { + Collection> secureSettingsCollection) { this.settings = settings; this.clusterService = clusterService; this.secureSettingsCollection = secureSettingsCollection; @@ -118,7 +118,7 @@ public boolean areAllConsistent() { final byte[] localHash = concreteSecureSetting.getSecretValueSHA256(settings); if (publishedSaltAndHash == null && localHash == null) { // consistency of missing - logger.debug("no published hash for the consistent secure setting [{}] but also it does NOT exist on the local node", + logger.debug("no published hash for the consistent secure setting [{}] but it also does NOT exist on the local node", concreteSecureSetting.getKey()); } else if (publishedSaltAndHash == null && localHash != null) { // setting missing on master but present locally diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index c4aab2bf9b5a6..5120591455f7a 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -196,7 +196,7 @@ private void registerSetting(Setting setting) { throw new IllegalArgumentException("Cannot register setting [" + setting.getKey() + "] twice"); } if (setting.isConsistent()) { - throw new IllegalArgumentException("Consistent setting [" + setting.getKey() + "] cannot be index scoped"); + throw new IllegalStateException("Consistent setting [" + setting.getKey() + "] cannot be index scoped"); } indexSettings.put(setting.getKey(), setting); } diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 24e0ab8a370c4..b409494f20d75 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -351,8 +351,9 @@ protected Node( final ClusterService clusterService = new ClusterService(settings, settingsModule.getClusterSettings(), threadPool); clusterService.addStateApplier(scriptModule.getScriptService()); resourcesToClose.add(clusterService); - final ConsistentSettingsService consistentSettingsService = new ConsistentSettingsService(settings, clusterService, - settingsModule.getConsistentSecureSettings()); + clusterService.addLocalNodeMasterListener( + new ConsistentSettingsService(settings, clusterService, settingsModule.getConsistentSecureSettings()) + .newHashPublisher()); final IngestService ingestService = new IngestService(clusterService, threadPool, this.environment, scriptModule.getScriptService(), analysisModule.getAnalysisRegistry(), pluginsService.filterPlugins(IngestPlugin.class)); final DiskThresholdMonitor listener = new DiskThresholdMonitor(settings, clusterService::state, From bed37c58a4e624aa82ae299f3d2183225419b5af Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 23 Apr 2019 16:49:50 +0300 Subject: [PATCH 20/39] KeyStoreWrapperTests --- .../common/settings/KeyStoreWrapperTests.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java index bb2b1df7f8c03..4e87f247a7701 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java @@ -35,8 +35,10 @@ import java.nio.file.FileSystem; import java.nio.file.Path; import java.security.KeyStore; +import java.security.MessageDigest; import java.security.SecureRandom; import java.util.ArrayList; +import java.util.Arrays; import java.util.Base64; import java.util.List; @@ -121,6 +123,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() + "1"; + final String stringSettingValue = randomAlphaOfLength(32); + keystore.setString(stringSettingKeyName, stringSettingValue.toCharArray()); + final String fileSettingKeyName = randomAlphaOfLength(5).toLowerCase() + "2"; + final byte[] fileSettingValue = randomByteArrayOfLength(32); + keystore.setFile(fileSettingKeyName, fileSettingValue); + + final byte[] stringSettingHash = MessageDigest.getInstance("SHA-256").digest(stringSettingValue.getBytes(StandardCharsets.UTF_8)); + assertTrue(Arrays.equals(keystore.getSHA256Digest(stringSettingKeyName), stringSettingHash)); + final byte[] fileSettingHash = MessageDigest.getInstance("SHA-256").digest(fileSettingValue); + assertTrue(Arrays.equals(keystore.getSHA256Digest(fileSettingKeyName), fileSettingHash)); + + keystore.close(); + + // value hashes accessible even when the keystore is closed + assertTrue(Arrays.equals(keystore.getSHA256Digest(stringSettingKeyName), stringSettingHash)); + assertTrue(Arrays.equals(keystore.getSHA256Digest(fileSettingKeyName), fileSettingHash)); + } + public void testUpgradeNoop() throws Exception { KeyStoreWrapper keystore = KeyStoreWrapper.create(); SecureString seed = keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey()); From f685e27c00ee7f91cb09de2bafaf7b68dfa46c37 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 23 Apr 2019 17:37:40 +0300 Subject: [PATCH 21/39] SettingsModuleTests --- .../common/settings/SettingsModuleTests.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index c6182eac8f680..f91f370a53cdb 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -83,6 +83,18 @@ public void testRegisterSettings() { assertEquals("Failed to parse value [false] for setting [some.custom.setting]", ex.getMessage()); } } + { + MockSecureSettings secureSettings = new MockSecureSettings(); + if (randomBoolean()) { + secureSettings.setString("some.custom.secure.consistent.setting", "secure_value"); + } else { + secureSettings.setFile("some.custom.secure.consistent.setting", new byte[] {0, 1, 1, 0}); + } + Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); + SettingsModule module = new SettingsModule(settings, + SecureSetting.secureString("some.custom.secure.consistent.setting", null, Setting.Property.Consistent)); + assertInstanceBinding(module, Settings.class, (s) -> s == settings); + } } public void testLoggerSettings() { From 4d92566afaef9ccfec59fe17b86e52d0c465c803 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 23 Apr 2019 20:55:37 +0300 Subject: [PATCH 22/39] Checkstyle --- .../common/settings/KeyStoreWrapperTests.java | 5 +- .../common/settings/ConsistentSettingsIT.java | 78 +++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java 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 4e87f247a7701..e72113fce4da4 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 @@ -41,6 +41,7 @@ import java.util.Arrays; import java.util.Base64; import java.util.List; +import java.util.Locale; import org.apache.lucene.codecs.CodecUtil; import org.apache.lucene.store.IOContext; @@ -125,9 +126,9 @@ public void testCannotReadStringFromClosedKeystore() throws Exception { public void testValueSHA256Digest() throws Exception { final KeyStoreWrapper keystore = KeyStoreWrapper.create(); - final String stringSettingKeyName = randomAlphaOfLength(5).toLowerCase() + "1"; + final String stringSettingKeyName = randomAlphaOfLength(5).toLowerCase(Locale.ROOT) + "1"; final String stringSettingValue = randomAlphaOfLength(32); - keystore.setString(stringSettingKeyName, stringSettingValue.toCharArray()); + keystore.setString(stringSettingKeyName, stringSettingValue.toCharArray(Locale.ROOT)); final String fileSettingKeyName = randomAlphaOfLength(5).toLowerCase() + "2"; final byte[] fileSettingValue = randomByteArrayOfLength(32); keystore.setFile(fileSettingKeyName, fileSettingValue); diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java new file mode 100644 index 0000000000000..8a67aaef59cac --- /dev/null +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java @@ -0,0 +1,78 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings; + +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.env.Environment; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.ESIntegTestCase; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +public class ConsistentSettingsIT extends ESIntegTestCase { + + static final Setting DUMMY_STRING_CONSISTENT_SETTING = SecureSetting.secureString("dummy.consistent.secure.string.setting", + null, Setting.Property.Consistent); +// static final AffixSetting DUMMY_AFIX_STRING_CONSISTENT_SETTING = Setting.affixKeySetting( +// "dummy.consistent.secure.string.afix.setting.", "suffix", +// key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); + + public void testAllConsistent() throws Exception { + final Environment environment = internalCluster().getInstance(Environment.class); + final ClusterService clusterService = internalCluster().getInstance(ClusterService.class); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + } + + @Override + protected Settings nodeSettings(int nodeOrdinal) { + Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); +// secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".sufix", "afix_value_1"); +// secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".sufix", "afix_value_2"); + assert builder.getSecureSettings() == null : "Deal with the settings merge"; + builder.setSecureSettings(secureSettings); + return builder.build(); + } + + @Override + protected Collection> nodePlugins() { + Collection> classes = new ArrayList<>(super.nodePlugins()); + classes.add(DummyPlugin.class); + return classes; + } + + public static final class DummyPlugin extends Plugin { + + public DummyPlugin() { + } + + @Override + public List> getSettings() { + List> settings = new ArrayList<>(super.getSettings()); + settings.add(DUMMY_STRING_CONSISTENT_SETTING); + //settings.add(DUMMY_AFIX_STRING_CONSISTENT_SETTING); + return settings; + } + } +} From e63ae25972f973371734b9748230b455c8b5250a Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 24 Apr 2019 01:15:49 +0300 Subject: [PATCH 23/39] Dumb checkstyle --- .../common/settings/ConsistentSettingsIT.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java index 8a67aaef59cac..3f76b7e446a9b 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java @@ -20,6 +20,7 @@ package org.elasticsearch.common.settings; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.Setting.AffixSetting; import org.elasticsearch.env.Environment; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase; @@ -31,16 +32,17 @@ public class ConsistentSettingsIT extends ESIntegTestCase { - static final Setting DUMMY_STRING_CONSISTENT_SETTING = SecureSetting.secureString("dummy.consistent.secure.string.setting", - null, Setting.Property.Consistent); -// static final AffixSetting DUMMY_AFIX_STRING_CONSISTENT_SETTING = Setting.affixKeySetting( -// "dummy.consistent.secure.string.afix.setting.", "suffix", -// key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); + static final Setting DUMMY_STRING_CONSISTENT_SETTING = SecureSetting + .secureString("dummy.consistent.secure.string.setting", null, Setting.Property.Consistent); + static final AffixSetting DUMMY_AFIX_STRING_CONSISTENT_SETTING = Setting.affixKeySetting( + "dummy.consistent.secure.string.afix.setting.", "suffix", + key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); public void testAllConsistent() throws Exception { final Environment environment = internalCluster().getInstance(Environment.class); final ClusterService clusterService = internalCluster().getInstance(ClusterService.class); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); } @Override @@ -48,8 +50,8 @@ protected Settings nodeSettings(int nodeOrdinal) { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); -// secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".sufix", "afix_value_1"); -// secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".sufix", "afix_value_2"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); return builder.build(); @@ -71,7 +73,7 @@ public DummyPlugin() { public List> getSettings() { List> settings = new ArrayList<>(super.getSettings()); settings.add(DUMMY_STRING_CONSISTENT_SETTING); - //settings.add(DUMMY_AFIX_STRING_CONSISTENT_SETTING); + settings.add(DUMMY_AFIX_STRING_CONSISTENT_SETTING); return settings; } } From 2dedbe13ff0dd84b47b2fe4217cc04a8cd3a4ead Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 24 Apr 2019 02:05:48 +0300 Subject: [PATCH 24/39] Nits --- .../common/settings/KeyStoreWrapperTests.java | 4 ++-- .../common/settings/ConsistentSettingsIT.java | 14 ++++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) 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 e72113fce4da4..712fb90a098b8 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 @@ -128,8 +128,8 @@ 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(Locale.ROOT)); - final String fileSettingKeyName = randomAlphaOfLength(5).toLowerCase() + "2"; + keystore.setString(stringSettingKeyName, stringSettingValue.toCharArray()); + final String fileSettingKeyName = randomAlphaOfLength(5).toLowerCase(Locale.ROOT) + "2"; final byte[] fileSettingValue = randomByteArrayOfLength(32); keystore.setFile(fileSettingKeyName, fileSettingValue); diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java index 3f76b7e446a9b..825a2628fb072 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java @@ -39,10 +39,16 @@ public class ConsistentSettingsIT extends ESIntegTestCase { key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); public void testAllConsistent() throws Exception { - final Environment environment = internalCluster().getInstance(Environment.class); - final ClusterService clusterService = internalCluster().getInstance(ClusterService.class); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + internalCluster().getInstances(Environment.class).forEach(environment -> { + ClusterService clusterService = internalCluster().getInstance(ClusterService.class); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + }); } @Override From 3335dc9bb5b4bcb403a0fbf0cbadf8b897592c6e Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 24 Apr 2019 21:41:39 +0300 Subject: [PATCH 25/39] WIP --- .../common/settings/ConsistentSettingsIT.java | 148 +++++++++++++++++- .../common/settings/SettingsModuleTests.java | 39 ++++- 2 files changed, 178 insertions(+), 9 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java index 825a2628fb072..c595b6001a90e 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java @@ -29,7 +29,10 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; +@ESIntegTestCase.ClusterScope(minNumDataNodes = 2, scope = ESIntegTestCase.Scope.TEST) public class ConsistentSettingsIT extends ESIntegTestCase { static final Setting DUMMY_STRING_CONSISTENT_SETTING = SecureSetting @@ -37,8 +40,14 @@ public class ConsistentSettingsIT extends ESIntegTestCase { static final AffixSetting DUMMY_AFIX_STRING_CONSISTENT_SETTING = Setting.affixKeySetting( "dummy.consistent.secure.string.afix.setting.", "suffix", key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); + static final Setting EXTRA_DUMMY_STRING_CONSISTENT_SETTING = SecureSetting + .secureString("extra.dummy.consistent.secure.string.setting", null, Setting.Property.Consistent); + static final AffixSetting DUMMY_AFIX_STRING_INCONSISTENT_SETTING = Setting.affixKeySetting( + "dummy.inconsistent.secure.string.afix.setting.", "suffix", + key -> SecureSetting.secureString(key, null)); + private final AtomicReference> nodeSettingsOverride = new AtomicReference<>(null); - public void testAllConsistent() throws Exception { + public void testAllConsistentSuccess() throws Exception { internalCluster().getInstances(Environment.class).forEach(environment -> { ClusterService clusterService = internalCluster().getInstance(ClusterService.class); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); @@ -51,13 +60,148 @@ public void testAllConsistent() throws Exception { }); } + public void testAllConsistentFail() throws Exception { + nodeSettingsOverride.set(nodeOrdinal -> { + Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); + MockSecureSettings secureSettings = new MockSecureSettings(); + if (randomBoolean()) { + secureSettings.setString("dummy.consistent.secure.string.setting", "DIFFERENT_VALUE"); + } else { + // missing value + } + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); + assert builder.getSecureSettings() == null : "Deal with the settings merge"; + builder.setSecureSettings(secureSettings); + return builder.build(); + }); + String newNodeName = internalCluster().startNode(); + Environment environment = internalCluster().getInstance(Environment.class, newNodeName); + ClusterService clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + nodeSettingsOverride.set(null); + newNodeName = internalCluster().startNode(); + environment = internalCluster().getInstance(Environment.class, newNodeName); + clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + nodeSettingsOverride.set(null); + } + + public void testAllConsistentAfixFail() throws Exception { + nodeSettingsOverride.set(nodeOrdinal -> { + Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); + if (randomBoolean()) { + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "DIFFERENT_VALUE"); + } else { + // missing value + } + assert builder.getSecureSettings() == null : "Deal with the settings merge"; + builder.setSecureSettings(secureSettings); + return builder.build(); + }); + String newNodeName = internalCluster().startNode(); + Environment environment = internalCluster().getInstance(Environment.class, newNodeName); + ClusterService clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + nodeSettingsOverride.set(null); + newNodeName = internalCluster().startNode(); + environment = internalCluster().getInstance(Environment.class, newNodeName); + clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + nodeSettingsOverride.set(null); + } + + public void testAllConsistentAfixFailWithExtra() throws Exception { + nodeSettingsOverride.set(nodeOrdinal -> { + Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); + secureSettings.setString("extra.dummy.consistent.secure.string.setting", "extra_string_value"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + assert builder.getSecureSettings() == null : "Deal with the settings merge"; + builder.setSecureSettings(secureSettings); + return builder.build(); + }); + String newNodeName = internalCluster().startNode(); + Environment environment = internalCluster().getInstance(Environment.class, newNodeName); + ClusterService clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(EXTRA_DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING, EXTRA_DUMMY_STRING_CONSISTENT_SETTING)) + .areAllConsistent()); + nodeSettingsOverride.set(nodeOrdinal -> { + Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix_extra" + ".suffix", "extra_afix_value"); + assert builder.getSecureSettings() == null : "Deal with the settings merge"; + builder.setSecureSettings(secureSettings); + return builder.build(); + }); + newNodeName = internalCluster().startNode(); + environment = internalCluster().getInstance(Environment.class, newNodeName); + clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + nodeSettingsOverride.set(null); + } + @Override protected Settings nodeSettings(int nodeOrdinal) { + Function nodeSettingsOverrideFunction = nodeSettingsOverride.get(); + if (nodeSettingsOverrideFunction != null) { + final Settings overrideSettings = nodeSettingsOverrideFunction.apply(nodeOrdinal); + if (overrideSettings != null) { + return overrideSettings; + } + } Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + // this is "inconsistent" hence it can differ from node to node + secureSettings.setString("dummy.inconsistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_" + nodeOrdinal); assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); return builder.build(); @@ -80,6 +224,8 @@ public List> getSettings() { List> settings = new ArrayList<>(super.getSettings()); settings.add(DUMMY_STRING_CONSISTENT_SETTING); settings.add(DUMMY_AFIX_STRING_CONSISTENT_SETTING); + settings.add(DUMMY_AFIX_STRING_INCONSISTENT_SETTING); + settings.add(EXTRA_DUMMY_STRING_CONSISTENT_SETTING); return settings; } } diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index f91f370a53cdb..2a2ecdc62d620 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -21,11 +21,13 @@ import org.elasticsearch.common.inject.ModuleTestCase; import org.elasticsearch.common.settings.Setting.Property; +import org.hamcrest.Matchers; import java.util.Arrays; import static java.util.Collections.emptySet; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; public class SettingsModuleTests extends ModuleTestCase { @@ -85,15 +87,36 @@ public void testRegisterSettings() { } { MockSecureSettings secureSettings = new MockSecureSettings(); - if (randomBoolean()) { - secureSettings.setString("some.custom.secure.consistent.setting", "secure_value"); - } else { - secureSettings.setFile("some.custom.secure.consistent.setting", new byte[] {0, 1, 1, 0}); - } - Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); - SettingsModule module = new SettingsModule(settings, - SecureSetting.secureString("some.custom.secure.consistent.setting", null, Setting.Property.Consistent)); + secureSettings.setString("some.custom.secure.consistent.setting", "secure_value"); + final Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); + final Setting concreteConsistentSetting = SecureSetting.secureString("some.custom.secure.consistent.setting", null, + Setting.Property.Consistent); + SettingsModule module = new SettingsModule(settings, concreteConsistentSetting); assertInstanceBinding(module, Settings.class, (s) -> s == settings); + assertThat(module.getConsistentSecureSettings(), Matchers.containsInAnyOrder(concreteConsistentSetting)); + + final Setting concreteUnsecureConsistentSetting = Setting.simpleString("some.custom.UNSECURE.consistent.setting", + Property.Consistent, Property.NodeScope); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new SettingsModule(Settings.builder().build(), concreteUnsecureConsistentSetting)); + assertThat(e.getMessage(), is("Invalid consistent secure setting [some.custom.UNSECURE.consistent.setting]")); + + secureSettings = new MockSecureSettings(); + secureSettings.setString("some.custom.secure.consistent.afix.wow.setting", "secure_value"); + final Settings settings2 = Settings.builder().setSecureSettings(secureSettings).build(); + final Setting afixConcreteConsistentSetting = Setting.affixKeySetting( + "some.custom.secure.consistent.afix.", "setting", + key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); + module = new SettingsModule(settings2,afixConcreteConsistentSetting); + assertInstanceBinding(module, Settings.class, (s) -> s == settings2); + assertThat(module.getConsistentSecureSettings(), Matchers.containsInAnyOrder(afixConcreteConsistentSetting)); + + final Setting concreteUnsecureConsistentAfixSetting = Setting.affixKeySetting( + "some.custom.secure.consistent.afix.", "setting", + key -> Setting.simpleString(key, Setting.Property.Consistent, Property.NodeScope)); + e = expectThrows(IllegalArgumentException.class, + () -> new SettingsModule(Settings.builder().build(), concreteUnsecureConsistentAfixSetting)); + assertThat(e.getMessage(), is("Invalid consistent secure setting [some.custom.secure.consistent.afix.*.setting]")); } } From 84e2ba8d68a26cfdc22628babe7e6bf47703df95 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 24 Apr 2019 22:55:13 +0300 Subject: [PATCH 26/39] Javadocs --- .../settings/ConsistentSettingsService.java | 36 ++++++++- .../common/settings/ConsistentSettingsIT.java | 75 ++++++++++++++++--- 2 files changed, 101 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java index 82e24a7a2c0ee..ece0e9f1da7f8 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java @@ -38,7 +38,9 @@ import java.util.Base64; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; @@ -46,6 +48,11 @@ import javax.crypto.SecretKeyFactory; import javax.crypto.spec.PBEKeySpec; +/** + * Used to publish secure setting hashes in the cluster state and to validate those hashes against the local values of those same settings. + * This is colloquially referred to as the secure setting consistency check. It will publish and verify hashes only for the collection + * of settings passed in the constructor. The settings have to have the {@link Setting.Property#Consistent} property. + */ public final class ConsistentSettingsService { private static final Logger logger = LogManager.getLogger(ConsistentSettingsService.class); @@ -67,6 +74,10 @@ public ConsistentSettingsService(Settings settings, ClusterService clusterServic } } + /** + * Returns a {@link LocalNodeMasterListener} that will publish hashes of all the settings passed in the constructor. These hashes are + * published by the master node only. Note that this is not designed for {@link SecureStrings} implementations that are mutable. + */ public LocalNodeMasterListener newHashPublisher() { return new LocalNodeMasterListener() { @@ -109,9 +120,16 @@ public String executorName() { }; } + /** + * Verifies that the hashes of consistent secure settings in the latest {@code ClusterState} verify for the values of those same + * settings on the local node. The settings to be checked are passed in the constructor. Also, validates that a missing local + * value is also missing in the published set, and vice-versa. + */ public boolean areAllConsistent() { final ClusterState state = clusterService.state(); final Map publishedHashesOfConsistentSettings = state.metaData().hashesOfConsistentSettings(); + final Set publishedSettingKeysToVerify = new HashSet<>(); + publishedSettingKeysToVerify.addAll(publishedHashesOfConsistentSettings.keySet()); final AtomicBoolean allConsistent = new AtomicBoolean(true); forEachConcreteSecureSettingDo(concreteSecureSetting -> { final String publishedSaltAndHash = publishedHashesOfConsistentSettings.get(concreteSecureSetting.getKey()); @@ -131,7 +149,7 @@ public boolean areAllConsistent() { allConsistent.set(false); } else if (publishedSaltAndHash != null && localHash == null) { // setting missing locally but present on master - logger.warn("the consistent secure setting [{}] does not exist on the local node but there is published hash for it", + logger.warn("the consistent secure setting [{}] does not exist on the local node but there is a published hash for it", concreteSecureSetting.getKey()); allConsistent.set(false); } else { @@ -157,10 +175,26 @@ public boolean areAllConsistent() { allConsistent.set(false); } } + publishedSettingKeysToVerify.remove(concreteSecureSetting.getKey()); }); + // another case of settings missing locally, when group settings have not expanded to all the keys published + for (String publishedSettingKey : publishedSettingKeysToVerify) { + for (Setting setting : secureSettingsCollection) { + if (setting.match(publishedSettingKey)) { + // setting missing locally but present on master + logger.warn("the consistent secure setting [{}] does not exist on the local node but there is a published hash for it", + publishedSettingKey); + allConsistent.set(false); + } + } + } return allConsistent.get(); } + /** + * Iterate over the passed in secure settings, expanding {@link Setting.AffixSetting} to concrete settings, in the scope of the local + * settings. + */ private void forEachConcreteSecureSettingDo(Consumer> secureSettingConsumer) { for (Setting setting : secureSettingsCollection) { assert setting.isConsistent() : "[" + setting.getKey() + "] is not a consistent setting"; diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java index c595b6001a90e..a336718ca6324 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java @@ -32,7 +32,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; -@ESIntegTestCase.ClusterScope(minNumDataNodes = 2, scope = ESIntegTestCase.Scope.TEST) +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST) public class ConsistentSettingsIT extends ESIntegTestCase { static final Setting DUMMY_STRING_CONSISTENT_SETTING = SecureSetting @@ -60,16 +60,14 @@ public void testAllConsistentSuccess() throws Exception { }); } - public void testAllConsistentFail() throws Exception { + public void testAllConsistentFailForSimpleSetting() throws Exception { nodeSettingsOverride.set(nodeOrdinal -> { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); - if (randomBoolean()) { - secureSettings.setString("dummy.consistent.secure.string.setting", "DIFFERENT_VALUE"); - } else { - // missing value - } + // different value + secureSettings.setString("dummy.consistent.secure.string.setting", "DIFFERENT_VALUE"); secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); return builder.build(); @@ -95,10 +93,31 @@ public void testAllConsistentFail() throws Exception { Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + nodeSettingsOverride.set(nodeOrdinal -> { + Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); + MockSecureSettings secureSettings = new MockSecureSettings(); + // missing value + // secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + assert builder.getSecureSettings() == null : "Deal with the settings merge"; + builder.setSecureSettings(secureSettings); + return builder.build(); + }); + newNodeName = internalCluster().startNode(); + environment = internalCluster().getInstance(Environment.class, newNodeName); + clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(null); } - public void testAllConsistentAfixFail() throws Exception { + public void testAllConsistentFailForAfixSetting() throws Exception { nodeSettingsOverride.set(nodeOrdinal -> { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); @@ -107,7 +126,8 @@ public void testAllConsistentAfixFail() throws Exception { secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "DIFFERENT_VALUE"); } else { - // missing value + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "DIFFERENT_VALUE_1"); + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "DIFFERENT_VALUE_2"); } assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); @@ -134,6 +154,32 @@ public void testAllConsistentAfixFail() throws Exception { Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + nodeSettingsOverride.set(nodeOrdinal -> { + Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); + // missing value + if (randomBoolean()) { + secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); + //secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + } else { + //secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); + //secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + } + assert builder.getSecureSettings() == null : "Deal with the settings merge"; + builder.setSecureSettings(secureSettings); + return builder.build(); + }); + newNodeName = internalCluster().startNode(); + environment = internalCluster().getInstance(Environment.class, newNodeName); + clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(null); } @@ -162,6 +208,17 @@ public void testAllConsistentAfixFailWithExtra() throws Exception { assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING, EXTRA_DUMMY_STRING_CONSISTENT_SETTING)) .areAllConsistent()); + nodeSettingsOverride.set(null); + newNodeName = internalCluster().startNode(); + environment = internalCluster().getInstance(Environment.class, newNodeName); + clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(nodeOrdinal -> { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); From b8ec087edcb62baf9685b950175d32a01366df40 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 24 Apr 2019 23:00:50 +0300 Subject: [PATCH 27/39] Javadoc --- .../common/settings/ConsistentSettingsService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java index ece0e9f1da7f8..0cdb0614b393d 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java @@ -76,7 +76,7 @@ public ConsistentSettingsService(Settings settings, ClusterService clusterServic /** * Returns a {@link LocalNodeMasterListener} that will publish hashes of all the settings passed in the constructor. These hashes are - * published by the master node only. Note that this is not designed for {@link SecureStrings} implementations that are mutable. + * published by the master node only. Note that this is not designed for {@link SecureSettings} implementations that are mutable. */ public LocalNodeMasterListener newHashPublisher() { return new LocalNodeMasterListener() { From 308efa32d781d326b54991830b6bf0566c0ceb17 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 8 May 2019 15:50:24 +0300 Subject: [PATCH 28/39] Merge fallout --- .../common/settings/KeyStoreWrapper.java | 68 +++++++++---------- 1 file changed, 31 insertions(+), 37 deletions(-) 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 e72540556c2f6..aa82f7a02c26b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -86,18 +86,16 @@ 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; -// final byte[] bytesSHA256Digest; -// -// Entry(EntryType type, byte[] bytes) { -// this.type = type; -// this.bytes = bytes; -// this.bytesSHA256Digest = MessageDigests.sha256().digest(bytes); -// } -// } + /** An entry in the keystore. The bytes are opaque and interpreted based on the entry type. */ + private static class Entry { + final byte[] bytes; + final byte[] sha256Digest; + + Entry(byte[] bytes) { + this.bytes = bytes; + this.sha256Digest = MessageDigests.sha256().digest(bytes); + } + } /** * A regex for the valid characters that a setting name in the keystore may use. @@ -162,7 +160,7 @@ private enum EntryType { 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) { @@ -364,7 +362,7 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio int entrySize = input.readInt(); byte[] entryBytes = new byte[entrySize]; input.readFully(entryBytes); - entries.get().put(setting, entryBytes); + entries.get().put(setting, new Entry(entryBytes)); } if (input.read() != -1) { throw new SecurityException("Keystore has been corrupted or tampered with"); @@ -383,11 +381,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 mapEntry : entries.get().entrySet()) { + for (Map.Entry mapEntry : entries.get().entrySet()) { output.writeUTF(mapEntry.getKey()); - byte[] entry = mapEntry.getValue(); - output.writeInt(entry.length); - output.write(entry); + byte[] entryBytes = mapEntry.getValue().bytes; + output.writeInt(entryBytes.length); + output.write(entryBytes); } } return bytes.toByteArray(); @@ -462,7 +460,7 @@ private void decryptLegacyEntries() throws GeneralSecurityException, IOException } Arrays.fill(chars, '\0'); - entries.get().put(setting, bytes); + entries.get().put(setting, new Entry(bytes)); } } @@ -535,8 +533,8 @@ public Set getSettingNames() { @Override public synchronized SecureString getString(String setting) { ensureOpen(); - byte[] entry = entries.get().get(setting); - ByteBuffer byteBuffer = ByteBuffer.wrap(entry); + Entry entry = entries.get().get(setting); + ByteBuffer byteBuffer = ByteBuffer.wrap(entry.bytes); CharBuffer charBuffer = StandardCharsets.UTF_8.decode(byteBuffer); return new SecureString(Arrays.copyOfRange(charBuffer.array(), charBuffer.position(), charBuffer.limit())); } @@ -544,19 +542,15 @@ public synchronized SecureString getString(String setting) { @Override public synchronized InputStream getFile(String setting) { ensureOpen(); - byte[] entry = entries.get().get(setting); - return new ByteArrayInputStream(entry); + Entry entry = entries.get().get(setting); + return new ByteArrayInputStream(entry.bytes); } @Override public byte[] getSHA256Digest(String setting) { assert entries.get() != null : "Keystore is not loaded"; - //final Entry entry = entries.get().get(setting); - //if (entry == null) { - // throw new IllegalArgumentException("Secret setting " + setting + " does not exist"); - //} - //return entry.bytesSHA256Digest; - return null; + Entry entry = entries.get().get(setting); + return entry.sha256Digest; } /** @@ -578,9 +572,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()); - byte[] oldEntry = entries.get().put(setting, bytes); + Entry oldEntry = entries.get().put(setting, new Entry(bytes)); if (oldEntry != null) { - Arrays.fill(oldEntry, (byte)0); + Arrays.fill(oldEntry.bytes, (byte)0); } } @@ -589,18 +583,18 @@ synchronized void setFile(String setting, byte[] bytes) { ensureOpen(); validateSettingName(setting); - byte[] oldEntry = entries.get().put(setting, Arrays.copyOf(bytes, bytes.length)); + Entry oldEntry = entries.get().put(setting, new Entry(Arrays.copyOf(bytes, bytes.length))); if (oldEntry != null) { - Arrays.fill(oldEntry, (byte)0); + Arrays.fill(oldEntry.bytes, (byte)0); } } /** Remove the given setting from the keystore. */ void remove(String setting) { ensureOpen(); - byte[] oldEntry = entries.get().remove(setting); + Entry oldEntry = entries.get().remove(setting); if (oldEntry != null) { - Arrays.fill(oldEntry, (byte)0); + Arrays.fill(oldEntry.bytes, (byte)0); } } @@ -615,8 +609,8 @@ private void ensureOpen() { public synchronized void close() { this.closed = true; if (null != entries.get() && entries.get().isEmpty() == false) { - for (byte[] entry : entries.get().values()) { - Arrays.fill(entry, (byte) 0); + for (Entry entry : entries.get().values()) { + Arrays.fill(entry.bytes, (byte) 0); } } } From fd9294ce4c7734d30fa776992b08eecaaefa7a6c Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 8 May 2019 23:03:55 +0300 Subject: [PATCH 29/39] Review part 1 --- .../cluster/metadata/DiffableStringMap.java | 4 ++++ .../org/elasticsearch/cluster/metadata/MetaData.java | 12 +++++------- .../common/settings/ConsistentSettingsService.java | 12 ++++++------ .../common/settings/KeyStoreWrapper.java | 4 ++++ .../elasticsearch/common/settings/SecureSetting.java | 7 ++++++- .../common/settings/SettingsModule.java | 12 ++++++------ .../src/main/java/org/elasticsearch/node/Node.java | 2 +- .../common/settings/SettingsModuleTests.java | 4 ++-- 8 files changed, 34 insertions(+), 23 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/DiffableStringMap.java b/server/src/main/java/org/elasticsearch/cluster/metadata/DiffableStringMap.java index 46433eed8a657..b6e31e92698e6 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/DiffableStringMap.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/DiffableStringMap.java @@ -39,6 +39,8 @@ */ public class DiffableStringMap extends AbstractMap implements Diffable { + public static final DiffableStringMap EMPTY = new DiffableStringMap(Collections.emptyMap()); + private final Map innerMap; DiffableStringMap(final Map map) { @@ -75,6 +77,8 @@ public static Diff readDiffFrom(StreamInput in) throws IOExce */ public static class DiffableStringMapDiff implements Diff { + public static final DiffableStringMapDiff EMPTY = new DiffableStringMapDiff(DiffableStringMap.EMPTY, DiffableStringMap.EMPTY); + private final List deletes; private final Map upserts; // diffs also become upserts diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java index 5a173ae5e782b..bfc6ddef456ec 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -854,12 +854,10 @@ private static class MetaDataDiff implements Diff { coordinationMetaData = new CoordinationMetaData(in); transientSettings = Settings.readSettingsFromStream(in); persistentSettings = Settings.readSettingsFromStream(in); - if (in.getVersion().onOrAfter(Version.CURRENT)) { + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { hashesOfConsistentSettings = DiffableStringMap.readDiffFrom(in); } else { - // empty diff - hashesOfConsistentSettings = new DiffableStringMap(Collections.emptyMap()) - .diff(new DiffableStringMap(Collections.emptyMap())); + hashesOfConsistentSettings = DiffableStringMap.DiffableStringMapDiff.EMPTY; } indices = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), IndexMetaData::readFrom, IndexMetaData::readDiffFrom); @@ -876,7 +874,7 @@ public void writeTo(StreamOutput out) throws IOException { coordinationMetaData.writeTo(out); Settings.writeSettingsToStream(transientSettings, out); Settings.writeSettingsToStream(persistentSettings, out); - if (out.getVersion().onOrAfter(Version.CURRENT)) { + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { hashesOfConsistentSettings.writeTo(out); } indices.writeTo(out); @@ -909,7 +907,7 @@ 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.CURRENT)) { + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { builder.hashesOfConsistentSettings(new DiffableStringMap(in)); } int size = in.readVInt(); @@ -936,7 +934,7 @@ public void writeTo(StreamOutput out) throws IOException { coordinationMetaData.writeTo(out); writeSettingsToStream(transientSettings, out); writeSettingsToStream(persistentSettings, out); - if (out.getVersion().onOrAfter(Version.CURRENT)) { + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { hashesOfConsistentSettings.writeTo(out); } out.writeVInt(indices.size()); diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java index 0cdb0614b393d..2de17663f24a4 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java @@ -59,7 +59,7 @@ public final class ConsistentSettingsService { private final Settings settings; private final ClusterService clusterService; private final Collection> secureSettingsCollection; - private final SecretKeyFactory PBKDF2KeyFactory; + private final SecretKeyFactory pbkdf2KeyFactory; public ConsistentSettingsService(Settings settings, ClusterService clusterService, Collection> secureSettingsCollection) { @@ -68,7 +68,7 @@ public ConsistentSettingsService(Settings settings, ClusterService clusterServic this.secureSettingsCollection = secureSettingsCollection; // this is used to compute the PBKDF2 hash (the published one) try { - this.PBKDF2KeyFactory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA512"); + this.pbkdf2KeyFactory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA512"); } catch (NoSuchAlgorithmException e) { throw new RuntimeException("The \"PBKDF2WithHmacSHA512\" algorithm is required for consistent secure settings' hashes", e); } @@ -92,7 +92,7 @@ public ClusterState execute(ClusterState currentState) { final Map publishedHashesOfConsistentSettings = currentState.metaData() .hashesOfConsistentSettings(); if (computedHashesOfConsistentSettings.equals(publishedHashesOfConsistentSettings)) { - logger.debug("Nothing to publish. What is already published matches this master's view."); + logger.debug("Nothing to publish. What is already published matches this node's view."); return currentState; } else { return ClusterState.builder(currentState).metaData(MetaData.builder(currentState.metaData()) @@ -133,7 +133,7 @@ public boolean areAllConsistent() { final AtomicBoolean allConsistent = new AtomicBoolean(true); forEachConcreteSecureSettingDo(concreteSecureSetting -> { final String publishedSaltAndHash = publishedHashesOfConsistentSettings.get(concreteSecureSetting.getKey()); - final byte[] localHash = concreteSecureSetting.getSecretValueSHA256(settings); + final byte[] localHash = concreteSecureSetting.getSecretDigest(settings); if (publishedSaltAndHash == null && localHash == null) { // consistency of missing logger.debug("no published hash for the consistent secure setting [{}] but it also does NOT exist on the local node", @@ -214,7 +214,7 @@ private void forEachConcreteSecureSettingDo(Consumer> secureSet private Map computeHashesOfConsistentSecureSettings() { final Map hashesBySettingKey = new HashMap<>(); forEachConcreteSecureSettingDo(concreteSecureSetting -> { - final byte[] localHash = concreteSecureSetting.getSecretValueSHA256(settings); + final byte[] localHash = concreteSecureSetting.getSecretDigest(settings); if (localHash != null) { final String salt = UUIDs.randomBase64UUID(); final byte[] publicHash = computeSaltedPBKDF2Hash(localHash, salt.getBytes(StandardCharsets.UTF_8)); @@ -232,7 +232,7 @@ private byte[] computeSaltedPBKDF2Hash(byte[] bytes, byte[] salt) { try { value = MessageDigests.toHexCharArray(bytes); final PBEKeySpec spec = new PBEKeySpec(value, salt, iterations, keyLength); - final SecretKey key = PBKDF2KeyFactory.generateSecret(spec); + final SecretKey key = pbkdf2KeyFactory.generateSecret(spec); return key.getEncoded(); } catch (InvalidKeySpecException e) { throw new RuntimeException("Unexpected exception when computing PBKDF2 hash", e); 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 aa82f7a02c26b..7ad69c1eebe0c 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -546,6 +546,10 @@ public synchronized InputStream getFile(String setting) { return new ByteArrayInputStream(entry.bytes); } + /** + * Returns the SHA256 digest for the setting's value, even after {@code #close()} has been called. The setting must exist. The digest is + * used to check for value changes without actually storing the value. + */ @Override public byte[] getSHA256Digest(String setting) { assert entries.get() != null : "Keystore is not loaded"; diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index f7ff2389da17b..93e9228f45731 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -97,7 +97,12 @@ public T get(Settings settings) { } } - public byte[] getSecretValueSHA256(Settings settings) { + /** + * Returns the digest of this secure setting's value. This method can be called at all times, there is no requirement that + * {@code SecureSettings} be open, unlike {@code #get(Settings)}. The digest is used to check for changes of the value, without + * actually storing the value. + */ + public byte[] getSecretDigest(Settings settings) { final SecureSettings secureSettings = settings.getSecureSettings(); if (secureSettings == null || false == secureSettings.getSettingNames().contains(getKey())) { return null; diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 5120591455f7a..58c9cbc520456 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -49,7 +49,7 @@ public class SettingsModule implements Module { private final Set settingsFilterPattern = new HashSet<>(); private final Map> nodeSettings = new HashMap<>(); private final Map> indexSettings = new HashMap<>(); - private final Set> consistentSecureSettings = new HashSet<>(); + private final Set> consistentSettings = new HashSet<>(); private final IndexScopedSettings indexScopedSettings; private final ClusterSettings clusterSettings; private final SettingsFilter settingsFilter; @@ -175,20 +175,20 @@ private void registerSetting(Setting setting) { if (existingSetting != null) { throw new IllegalArgumentException("Cannot register setting [" + setting.getKey() + "] twice"); } - nodeSettings.put(setting.getKey(), setting); if (setting.isConsistent()) { if (setting instanceof Setting.AffixSetting) { if (((Setting.AffixSetting)setting).getConcreteSettingForNamespace("_na_") instanceof SecureSetting) { - consistentSecureSettings.add(setting); + consistentSettings.add(setting); } else { throw new IllegalArgumentException("Invalid consistent secure setting [" + setting.getKey() + "]"); } } else if (setting instanceof SecureSetting) { - consistentSecureSettings.add(setting); + consistentSettings.add(setting); } else { throw new IllegalArgumentException("Invalid consistent secure setting [" + setting.getKey() + "]"); } } + nodeSettings.put(setting.getKey(), setting); } if (setting.hasIndexScope()) { Setting existingSetting = indexSettings.get(setting.getKey()); @@ -231,8 +231,8 @@ public ClusterSettings getClusterSettings() { return clusterSettings; } - public Set> getConsistentSecureSettings() { - return consistentSecureSettings; + public Set> getConsistentSettings() { + return consistentSettings; } public SettingsFilter getSettingsFilter() { diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index b409494f20d75..9bf102458070e 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -352,7 +352,7 @@ protected Node( clusterService.addStateApplier(scriptModule.getScriptService()); resourcesToClose.add(clusterService); clusterService.addLocalNodeMasterListener( - new ConsistentSettingsService(settings, clusterService, settingsModule.getConsistentSecureSettings()) + new ConsistentSettingsService(settings, clusterService, settingsModule.getConsistentSettings()) .newHashPublisher()); final IngestService ingestService = new IngestService(clusterService, threadPool, this.environment, scriptModule.getScriptService(), analysisModule.getAnalysisRegistry(), pluginsService.filterPlugins(IngestPlugin.class)); diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index 2a2ecdc62d620..e5cbff582e991 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -93,7 +93,7 @@ public void testRegisterSettings() { Setting.Property.Consistent); SettingsModule module = new SettingsModule(settings, concreteConsistentSetting); assertInstanceBinding(module, Settings.class, (s) -> s == settings); - assertThat(module.getConsistentSecureSettings(), Matchers.containsInAnyOrder(concreteConsistentSetting)); + assertThat(module.getConsistentSettings(), Matchers.containsInAnyOrder(concreteConsistentSetting)); final Setting concreteUnsecureConsistentSetting = Setting.simpleString("some.custom.UNSECURE.consistent.setting", Property.Consistent, Property.NodeScope); @@ -109,7 +109,7 @@ public void testRegisterSettings() { key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); module = new SettingsModule(settings2,afixConcreteConsistentSetting); assertInstanceBinding(module, Settings.class, (s) -> s == settings2); - assertThat(module.getConsistentSecureSettings(), Matchers.containsInAnyOrder(afixConcreteConsistentSetting)); + assertThat(module.getConsistentSettings(), Matchers.containsInAnyOrder(afixConcreteConsistentSetting)); final Setting concreteUnsecureConsistentAfixSetting = Setting.affixKeySetting( "some.custom.secure.consistent.afix.", "setting", From 3de5eefabf99ba6c56b4fdd2f3a9cbd8c61d7ac5 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 8 May 2019 23:05:20 +0300 Subject: [PATCH 30/39] Do not display "consistentHashes" in the API result --- .../java/org/elasticsearch/cluster/metadata/MetaData.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java index bfc6ddef456ec..6eaf0ed849e4f 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -1310,12 +1310,6 @@ public static void toXContent(MetaData metaData, XContentBuilder builder, ToXCon builder.endObject(); } - if (context == XContentContext.API && false == metaData.hashesOfConsistentSettings().isEmpty()) { - builder.startObject("hashes_of_consistent_settings"); - builder.map(metaData.hashesOfConsistentSettings()); - builder.endObject(); - } - builder.startObject("templates"); for (ObjectCursor cursor : metaData.templates().values()) { IndexTemplateMetaData.Builder.toXContentWithTypes(cursor.value, builder, params); From 3760bfcae20095d20aa5f4d71540decb0c4cd948 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Thu, 9 May 2019 13:57:38 +0300 Subject: [PATCH 31/39] ConsistentSettings unit tests --- .../settings/ConsistentSettingsService.java | 88 +++++----- .../common/settings/ConsistentSettingsIT.java | 2 + .../ConsistentSettingsServiceTests.java | 161 ++++++++++++++++++ 3 files changed, 212 insertions(+), 39 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java diff --git a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java index 2de17663f24a4..411a470238638 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ConsistentSettingsService.java @@ -79,45 +79,9 @@ public ConsistentSettingsService(Settings settings, ClusterService clusterServic * published by the master node only. Note that this is not designed for {@link SecureSettings} implementations that are mutable. */ public LocalNodeMasterListener newHashPublisher() { - return new LocalNodeMasterListener() { - - // eagerly compute hashes to be published - final Map computedHashesOfConsistentSettings = computeHashesOfConsistentSecureSettings(); - - @Override - public void onMaster() { - clusterService.submitStateUpdateTask("publish-secure-settings-hashes", new ClusterStateUpdateTask(Priority.URGENT) { - @Override - public ClusterState execute(ClusterState currentState) { - final Map publishedHashesOfConsistentSettings = currentState.metaData() - .hashesOfConsistentSettings(); - if (computedHashesOfConsistentSettings.equals(publishedHashesOfConsistentSettings)) { - logger.debug("Nothing to publish. What is already published matches this node's view."); - return currentState; - } else { - return ClusterState.builder(currentState).metaData(MetaData.builder(currentState.metaData()) - .hashesOfConsistentSettings(computedHashesOfConsistentSettings)).build(); - } - } - - @Override - public void onFailure(String source, Exception e) { - logger.error("unable to publish secure settings hashes", e); - } - - }); - } - - @Override - public void offMaster() { - logger.trace("I am no longer master, nothing to do"); - } - - @Override - public String executorName() { - return ThreadPool.Names.SAME; - } - }; + // eagerly compute hashes to be published + final Map computedHashesOfConsistentSettings = computeHashesOfConsistentSecureSettings(); + return new HashesPublisher(computedHashesOfConsistentSettings, clusterService); } /** @@ -243,4 +207,50 @@ private byte[] computeSaltedPBKDF2Hash(byte[] bytes, byte[] salt) { } } + static final class HashesPublisher implements LocalNodeMasterListener { + + // eagerly compute hashes to be published + final Map computedHashesOfConsistentSettings; + final ClusterService clusterService; + + HashesPublisher(Map computedHashesOfConsistentSettings, ClusterService clusterService) { + this.computedHashesOfConsistentSettings = Map.copyOf(computedHashesOfConsistentSettings); + this.clusterService = clusterService; + } + + @Override + public void onMaster() { + clusterService.submitStateUpdateTask("publish-secure-settings-hashes", new ClusterStateUpdateTask(Priority.URGENT) { + @Override + public ClusterState execute(ClusterState currentState) { + final Map publishedHashesOfConsistentSettings = currentState.metaData() + .hashesOfConsistentSettings(); + if (computedHashesOfConsistentSettings.equals(publishedHashesOfConsistentSettings)) { + logger.debug("Nothing to publish. What is already published matches this node's view."); + return currentState; + } else { + return ClusterState.builder(currentState).metaData(MetaData.builder(currentState.metaData()) + .hashesOfConsistentSettings(computedHashesOfConsistentSettings)).build(); + } + } + + @Override + public void onFailure(String source, Exception e) { + logger.error("unable to publish secure settings hashes", e); + } + + }); + } + + @Override + public void offMaster() { + logger.trace("I am no longer master, nothing to do"); + } + + @Override + public String executorName() { + return ThreadPool.Names.SAME; + } + } + } diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java index a336718ca6324..2b356c67aedbd 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.settings; +import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Setting.AffixSetting; import org.elasticsearch.env.Environment; @@ -29,6 +30,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java new file mode 100644 index 0000000000000..9478c3f0da374 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java @@ -0,0 +1,161 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings; + +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateUpdateTask; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.mock.orig.Mockito; +import org.elasticsearch.test.ESTestCase; +import org.junit.Before; +import org.mockito.stubbing.Answer; + +import java.util.List; +import java.util.Locale; +import java.util.concurrent.atomic.AtomicReference; + +import static org.mockito.Mockito.mock; +import static org.hamcrest.Matchers.is; + +public class ConsistentSettingsServiceTests extends ESTestCase { + + private AtomicReference clusterState = new AtomicReference<>(); + private ClusterService clusterService; + + @Before + public void init() throws Exception { + clusterState.set(ClusterState.EMPTY_STATE); + clusterService = mock(ClusterService.class); + Mockito.doAnswer((Answer) invocation -> { + return clusterState.get(); + }).when(clusterService).state(); + Mockito.doAnswer((Answer) invocation -> { + final ClusterStateUpdateTask arg0 = (ClusterStateUpdateTask) invocation.getArguments()[1]; + this.clusterState.set(arg0.execute(this.clusterState.get())); + return null; + }).when(clusterService).submitStateUpdateTask(Mockito.isA(String.class), Mockito.isA(ClusterStateUpdateTask.class)); + } + + public void testSingleStringSetting() throws Exception { + Setting stringSetting = SecureSetting.secureString("test.simple.foo", null, Setting.Property.Consistent); + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString(stringSetting.getKey(), "somethingsecure"); + secureSettings.setString("test.noise.setting", "noise"); + Settings.Builder builder = Settings.builder(); + builder.setSecureSettings(secureSettings); + Settings settings = builder.build(); + // hashes not yet published + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting)).areAllConsistent(), is(false)); + // publish + new ConsistentSettingsService(settings, clusterService, List.of(stringSetting)).newHashPublisher().onMaster(); + ConsistentSettingsService consistentService = new ConsistentSettingsService(settings, clusterService, List.of(stringSetting)); + assertThat(consistentService.areAllConsistent(), is(true)); + // change value + secureSettings.setString(stringSetting.getKey(), "_TYPO_somethingsecure"); + assertThat(consistentService.areAllConsistent(), is(false)); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting)).areAllConsistent(), is(false)); + // publish change + new ConsistentSettingsService(settings, clusterService, List.of(stringSetting)).newHashPublisher().onMaster(); + assertThat(consistentService.areAllConsistent(), is(true)); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting)).areAllConsistent(), is(true)); + } + + public void testSingleAffixSetting() throws Exception { + Setting.AffixSetting affixStringSetting = Setting.affixKeySetting("test.afix.", "bar", + (key) -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); + // add two affix settings to the keystore + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("test.noise.setting", "noise"); + secureSettings.setString("test.afix.first.bar", "first_secure"); + secureSettings.setString("test.afix.second.bar", "second_secure"); + Settings.Builder builder = Settings.builder(); + builder.setSecureSettings(secureSettings); + Settings settings = builder.build(); + // hashes not yet published + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(false)); + // publish + new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).newHashPublisher().onMaster(); + ConsistentSettingsService consistentService = new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)); + assertThat(consistentService.areAllConsistent(), is(true)); + // change value + secureSettings.setString("test.afix.second.bar", "_TYPO_second_secure"); + assertThat(consistentService.areAllConsistent(), is(false)); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(false)); + // publish change + new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).newHashPublisher().onMaster(); + assertThat(consistentService.areAllConsistent(), is(true)); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(true)); + // add value + secureSettings.setString("test.afix.third.bar", "third_secure"); + builder = Settings.builder(); + builder.setSecureSettings(secureSettings); + settings = builder.build(); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(false)); + // publish + new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).newHashPublisher().onMaster(); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(true)); + // remove value + secureSettings = new MockSecureSettings(); + secureSettings.setString("test.another.noise.setting", "noise"); + // missing value + //secureSettings.setString("test.afix.first.bar", "first_secure"); + secureSettings.setString("test.afix.second.bar", "second_secure"); + secureSettings.setString("test.afix.third.bar", "third_secure"); + builder = Settings.builder(); + builder.setSecureSettings(secureSettings); + settings = builder.build(); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(false)); + } + + public void testStringAndAffixSettings() throws Exception { + Setting stringSetting = SecureSetting.secureString("mock.simple.foo", null, Setting.Property.Consistent); + Setting.AffixSetting affixStringSetting = Setting.affixKeySetting("mock.afix.", "bar", + (key) -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString(randomAlphaOfLength(8).toLowerCase(Locale.ROOT), "noise"); + secureSettings.setString(stringSetting.getKey(), "somethingsecure"); + secureSettings.setString("mock.afix.foo.bar", "another_secure"); + Settings.Builder builder = Settings.builder(); + builder.setSecureSettings(secureSettings); + Settings settings = builder.build(); + // hashes not yet published + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting, affixStringSetting)).areAllConsistent(), + is(false)); + // publish only the simple string setting + new ConsistentSettingsService(settings, clusterService, List.of(stringSetting)).newHashPublisher().onMaster(); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting)).areAllConsistent(), is(true)); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(false)); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting, affixStringSetting)).areAllConsistent(), + is(false)); + // publish only the affix string setting + new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).newHashPublisher().onMaster(); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting)).areAllConsistent(), is(false)); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(true)); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting, affixStringSetting)).areAllConsistent(), + is(false)); + // publish both settings + new ConsistentSettingsService(settings, clusterService, List.of(stringSetting, affixStringSetting)).newHashPublisher().onMaster(); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting)).areAllConsistent(), is(true)); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(true)); + assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting, affixStringSetting)).areAllConsistent(), + is(true)); + + } +} From 86ee31410d3ff2f8d35ec2b0676829bd533e6781 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 20 May 2019 16:23:25 +0300 Subject: [PATCH 32/39] Review javadoc --- .../org/elasticsearch/common/settings/SecureSetting.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index 93e9228f45731..26c0dea20b9d9 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -98,9 +98,9 @@ public T get(Settings settings) { } /** - * Returns the digest of this secure setting's value. This method can be called at all times, there is no requirement that - * {@code SecureSettings} be open, unlike {@code #get(Settings)}. The digest is used to check for changes of the value, without - * actually storing the value. + * Returns the digest of this secure setting's value or {@code null} if the setting is missing. This method can be called even after the + * {@code SecureSettings} have been closed, unlike {@code #get(Settings)}. The digest is used to check for changes of the value (by + * re-reading the {@code SecureSettings}), without actually storing the value to compare with. */ public byte[] getSecretDigest(Settings settings) { final SecureSettings secureSettings = settings.getSecureSettings(); From 77a6092ef2f417f34788439eacc4489532795ad0 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 20 May 2019 16:42:29 +0300 Subject: [PATCH 33/39] unused imports --- .../org/elasticsearch/common/settings/ConsistentSettingsIT.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java index 2b356c67aedbd..a336718ca6324 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java @@ -19,7 +19,6 @@ package org.elasticsearch.common.settings; -import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Setting.AffixSetting; import org.elasticsearch.env.Environment; @@ -30,7 +29,6 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; From cb1e59b1f5855b592a5695185094b42b62e0daf2 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 22 May 2019 17:09:45 +0300 Subject: [PATCH 34/39] comment code in test --- .../common/settings/ConsistentSettingsServiceTests.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java index 9478c3f0da374..28256d414cb44 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java @@ -114,8 +114,7 @@ public void testSingleAffixSetting() throws Exception { // remove value secureSettings = new MockSecureSettings(); secureSettings.setString("test.another.noise.setting", "noise"); - // missing value - //secureSettings.setString("test.afix.first.bar", "first_secure"); + // missing value test.afix.first.bar secureSettings.setString("test.afix.second.bar", "second_secure"); secureSettings.setString("test.afix.third.bar", "third_secure"); builder = Settings.builder(); From d25738807ea87851a4c696515edcbda18f2de5f2 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 18 Jun 2019 12:51:51 +0300 Subject: [PATCH 35/39] Change bwc strategy --- build.gradle | 4 ++-- .../java/org/elasticsearch/cluster/metadata/MetaData.java | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/build.gradle b/build.gradle index 5c1fe80668283..8ee0712383367 100644 --- a/build.gradle +++ b/build.gradle @@ -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") diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java index 6eaf0ed849e4f..3bf4032ba79f6 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -854,7 +854,7 @@ private static class MetaDataDiff implements Diff { coordinationMetaData = new CoordinationMetaData(in); transientSettings = Settings.readSettingsFromStream(in); persistentSettings = Settings.readSettingsFromStream(in); - if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + if (in.getVersion().onOrAfter(Version.V_7_3_0)) { hashesOfConsistentSettings = DiffableStringMap.readDiffFrom(in); } else { hashesOfConsistentSettings = DiffableStringMap.DiffableStringMapDiff.EMPTY; @@ -874,7 +874,7 @@ public void writeTo(StreamOutput out) throws IOException { coordinationMetaData.writeTo(out); Settings.writeSettingsToStream(transientSettings, out); Settings.writeSettingsToStream(persistentSettings, out); - if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + if (out.getVersion().onOrAfter(Version.V_7_3_0)) { hashesOfConsistentSettings.writeTo(out); } indices.writeTo(out); @@ -907,7 +907,7 @@ 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_8_0_0)) { + if (in.getVersion().onOrAfter(Version.V_7_3_0)) { builder.hashesOfConsistentSettings(new DiffableStringMap(in)); } int size = in.readVInt(); @@ -934,7 +934,7 @@ public void writeTo(StreamOutput out) throws IOException { coordinationMetaData.writeTo(out); writeSettingsToStream(transientSettings, out); writeSettingsToStream(persistentSettings, out); - if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + if (out.getVersion().onOrAfter(Version.V_7_3_0)) { hashesOfConsistentSettings.writeTo(out); } out.writeVInt(indices.size()); From 208f27d7fb545db90b4ef673ba27e33e8fae8116 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Fri, 21 Jun 2019 18:32:24 +0300 Subject: [PATCH 36/39] Review --- .../common/settings/KeyStoreWrapperTests.java | 8 +- .../common/settings/SecureSetting.java | 6 +- .../common/settings/Setting.java | 5 +- .../common/settings/ConsistentSettingsIT.java | 96 +++++++++---------- .../common/settings/SettingsModuleTests.java | 67 ++++++------- 5 files changed, 92 insertions(+), 90 deletions(-) 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 75a3a58cec8cf..b2233ce125bfa 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 @@ -139,15 +139,15 @@ public void testValueSHA256Digest() throws Exception { keystore.setFile(fileSettingKeyName, fileSettingValue); final byte[] stringSettingHash = MessageDigest.getInstance("SHA-256").digest(stringSettingValue.getBytes(StandardCharsets.UTF_8)); - assertTrue(Arrays.equals(keystore.getSHA256Digest(stringSettingKeyName), stringSettingHash)); + assertThat(keystore.getSHA256Digest(stringSettingKeyName), equalTo(stringSettingHash)); final byte[] fileSettingHash = MessageDigest.getInstance("SHA-256").digest(fileSettingValue); - assertTrue(Arrays.equals(keystore.getSHA256Digest(fileSettingKeyName), fileSettingHash)); + assertThat(keystore.getSHA256Digest(fileSettingKeyName), equalTo(fileSettingHash)); keystore.close(); // value hashes accessible even when the keystore is closed - assertTrue(Arrays.equals(keystore.getSHA256Digest(stringSettingKeyName), stringSettingHash)); - assertTrue(Arrays.equals(keystore.getSHA256Digest(fileSettingKeyName), fileSettingHash)); + assertThat(keystore.getSHA256Digest(stringSettingKeyName), equalTo(stringSettingHash)); + assertThat(keystore.getSHA256Digest(fileSettingKeyName), equalTo(fileSettingHash)); } public void testUpgradeNoop() throws Exception { diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index 26c0dea20b9d9..e022e4e3760a5 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -98,9 +98,9 @@ public T get(Settings settings) { } /** - * Returns the digest of this secure setting's value or {@code null} if the setting is missing. This method can be called even after the - * {@code SecureSettings} have been closed, unlike {@code #get(Settings)}. The digest is used to check for changes of the value (by - * re-reading the {@code SecureSettings}), without actually storing the value to compare with. + * Returns the digest of this secure setting's value or {@code null} if the setting is missing (inside the keystore). This method can be + * called even after the {@code SecureSettings} have been closed, unlike {@code #get(Settings)}. The digest is used to check for changes + * of the value (by re-reading the {@code SecureSettings}), without actually transmitting the value to compare with. */ public byte[] getSecretDigest(Settings settings) { final SecureSettings secureSettings = settings.getSecureSettings(); diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index 687db528c4ea7..d4164b474de0b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -113,7 +113,7 @@ public enum Property { NodeScope, /** - * Setting values equal on all nodes + * Secure setting values equal on all nodes */ Consistent, @@ -334,7 +334,8 @@ public boolean hasNodeScope() { } /** - * Returns true if this setting's value can be checked for equality across all nodes + * Returns true if this setting's value can be checked for equality across all nodes. Only {@link SecureSetting} instances + * may have this qualifier. */ public boolean isConsistent() { return properties.contains(Property.Consistent); diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java index a336718ca6324..0d5ac55d21219 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java @@ -37,13 +37,13 @@ public class ConsistentSettingsIT extends ESIntegTestCase { static final Setting DUMMY_STRING_CONSISTENT_SETTING = SecureSetting .secureString("dummy.consistent.secure.string.setting", null, Setting.Property.Consistent); - static final AffixSetting DUMMY_AFIX_STRING_CONSISTENT_SETTING = Setting.affixKeySetting( - "dummy.consistent.secure.string.afix.setting.", "suffix", + static final AffixSetting DUMMY_AFFIX_STRING_CONSISTENT_SETTING = Setting.affixKeySetting( + "dummy.consistent.secure.string.affix.setting.", "suffix", key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); static final Setting EXTRA_DUMMY_STRING_CONSISTENT_SETTING = SecureSetting .secureString("extra.dummy.consistent.secure.string.setting", null, Setting.Property.Consistent); - static final AffixSetting DUMMY_AFIX_STRING_INCONSISTENT_SETTING = Setting.affixKeySetting( - "dummy.inconsistent.secure.string.afix.setting.", "suffix", + static final AffixSetting DUMMY_AFFIX_STRING_INCONSISTENT_SETTING = Setting.affixKeySetting( + "dummy.inconsistent.secure.string.affix.setting.", "suffix", key -> SecureSetting.secureString(key, null)); private final AtomicReference> nodeSettingsOverride = new AtomicReference<>(null); @@ -54,9 +54,9 @@ public void testAllConsistentSuccess() throws Exception { assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); }); } @@ -66,8 +66,8 @@ public void testAllConsistentFailForSimpleSetting() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); // different value secureSettings.setString("dummy.consistent.secure.string.setting", "DIFFERENT_VALUE"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); return builder.build(); @@ -79,9 +79,9 @@ public void testAllConsistentFailForSimpleSetting() throws Exception { assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(null); newNodeName = internalCluster().startNode(); environment = internalCluster().getInstance(Environment.class, newNodeName); @@ -90,16 +90,16 @@ public void testAllConsistentFailForSimpleSetting() throws Exception { assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(nodeOrdinal -> { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); // missing value // secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); return builder.build(); @@ -111,23 +111,23 @@ public void testAllConsistentFailForSimpleSetting() throws Exception { assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(null); } - public void testAllConsistentFailForAfixSetting() throws Exception { + public void testAllConsistentFailForAffixSetting() throws Exception { nodeSettingsOverride.set(nodeOrdinal -> { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); if (randomBoolean()) { - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "DIFFERENT_VALUE"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "DIFFERENT_VALUE"); } else { - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "DIFFERENT_VALUE_1"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "DIFFERENT_VALUE_2"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "DIFFERENT_VALUE_1"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "DIFFERENT_VALUE_2"); } assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); @@ -140,9 +140,9 @@ public void testAllConsistentFailForAfixSetting() throws Exception { assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(null); newNodeName = internalCluster().startNode(); environment = internalCluster().getInstance(Environment.class, newNodeName); @@ -151,20 +151,20 @@ public void testAllConsistentFailForAfixSetting() throws Exception { assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(nodeOrdinal -> { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); // missing value if (randomBoolean()) { - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); - //secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); + // missing dummy.consistent.secure.string.affix.setting.affix2.suffix" } else { - //secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); - //secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + // missing dummy.consistent.secure.string.affix.setting.affix1.suffix + // missing dummy.consistent.secure.string.affix.setting.affix2.suffix } assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); @@ -177,20 +177,20 @@ public void testAllConsistentFailForAfixSetting() throws Exception { assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(null); } - public void testAllConsistentAfixFailWithExtra() throws Exception { + public void testAllConsistentAffixFailWithExtra() throws Exception { nodeSettingsOverride.set(nodeOrdinal -> { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); secureSettings.setString("extra.dummy.consistent.secure.string.setting", "extra_string_value"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); return builder.build(); @@ -202,11 +202,11 @@ public void testAllConsistentAfixFailWithExtra() throws Exception { assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(EXTRA_DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING, EXTRA_DUMMY_STRING_CONSISTENT_SETTING)) + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING, EXTRA_DUMMY_STRING_CONSISTENT_SETTING)) .areAllConsistent()); nodeSettingsOverride.set(null); newNodeName = internalCluster().startNode(); @@ -216,16 +216,16 @@ public void testAllConsistentAfixFailWithExtra() throws Exception { assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(nodeOrdinal -> { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix_extra" + ".suffix", "extra_afix_value"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix_extra" + ".suffix", "extra_affix_value"); assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); return builder.build(); @@ -237,9 +237,9 @@ public void testAllConsistentAfixFailWithExtra() throws Exception { assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(null); } @@ -255,10 +255,10 @@ protected Settings nodeSettings(int nodeOrdinal) { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.afix.setting." + "afix2" + ".suffix", "afix_value_2"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); // this is "inconsistent" hence it can differ from node to node - secureSettings.setString("dummy.inconsistent.secure.string.afix.setting." + "afix1" + ".suffix", "afix_value_" + nodeOrdinal); + secureSettings.setString("dummy.inconsistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_" + nodeOrdinal); assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); return builder.build(); @@ -280,8 +280,8 @@ public DummyPlugin() { public List> getSettings() { List> settings = new ArrayList<>(super.getSettings()); settings.add(DUMMY_STRING_CONSISTENT_SETTING); - settings.add(DUMMY_AFIX_STRING_CONSISTENT_SETTING); - settings.add(DUMMY_AFIX_STRING_INCONSISTENT_SETTING); + settings.add(DUMMY_AFFIX_STRING_CONSISTENT_SETTING); + settings.add(DUMMY_AFFIX_STRING_INCONSISTENT_SETTING); settings.add(EXTRA_DUMMY_STRING_CONSISTENT_SETTING); return settings; } diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index e5cbff582e991..c374984eb5d15 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -85,39 +85,40 @@ public void testRegisterSettings() { assertEquals("Failed to parse value [false] for setting [some.custom.setting]", ex.getMessage()); } } - { - MockSecureSettings secureSettings = new MockSecureSettings(); - secureSettings.setString("some.custom.secure.consistent.setting", "secure_value"); - final Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); - final Setting concreteConsistentSetting = SecureSetting.secureString("some.custom.secure.consistent.setting", null, - Setting.Property.Consistent); - SettingsModule module = new SettingsModule(settings, concreteConsistentSetting); - assertInstanceBinding(module, Settings.class, (s) -> s == settings); - assertThat(module.getConsistentSettings(), Matchers.containsInAnyOrder(concreteConsistentSetting)); - - final Setting concreteUnsecureConsistentSetting = Setting.simpleString("some.custom.UNSECURE.consistent.setting", - Property.Consistent, Property.NodeScope); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> new SettingsModule(Settings.builder().build(), concreteUnsecureConsistentSetting)); - assertThat(e.getMessage(), is("Invalid consistent secure setting [some.custom.UNSECURE.consistent.setting]")); - - secureSettings = new MockSecureSettings(); - secureSettings.setString("some.custom.secure.consistent.afix.wow.setting", "secure_value"); - final Settings settings2 = Settings.builder().setSecureSettings(secureSettings).build(); - final Setting afixConcreteConsistentSetting = Setting.affixKeySetting( - "some.custom.secure.consistent.afix.", "setting", - key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); - module = new SettingsModule(settings2,afixConcreteConsistentSetting); - assertInstanceBinding(module, Settings.class, (s) -> s == settings2); - assertThat(module.getConsistentSettings(), Matchers.containsInAnyOrder(afixConcreteConsistentSetting)); - - final Setting concreteUnsecureConsistentAfixSetting = Setting.affixKeySetting( - "some.custom.secure.consistent.afix.", "setting", - key -> Setting.simpleString(key, Setting.Property.Consistent, Property.NodeScope)); - e = expectThrows(IllegalArgumentException.class, - () -> new SettingsModule(Settings.builder().build(), concreteUnsecureConsistentAfixSetting)); - assertThat(e.getMessage(), is("Invalid consistent secure setting [some.custom.secure.consistent.afix.*.setting]")); - } + } + + public void testRegisterConsistentSettings() { + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("some.custom.secure.consistent.setting", "secure_value"); + final Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); + final Setting concreteConsistentSetting = SecureSetting.secureString("some.custom.secure.consistent.setting", null, + Setting.Property.Consistent); + SettingsModule module = new SettingsModule(settings, concreteConsistentSetting); + assertInstanceBinding(module, Settings.class, (s) -> s == settings); + assertThat(module.getConsistentSettings(), Matchers.containsInAnyOrder(concreteConsistentSetting)); + + final Setting concreteUnsecureConsistentSetting = Setting.simpleString("some.custom.UNSECURE.consistent.setting", + Property.Consistent, Property.NodeScope); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new SettingsModule(Settings.builder().build(), concreteUnsecureConsistentSetting)); + assertThat(e.getMessage(), is("Invalid consistent secure setting [some.custom.UNSECURE.consistent.setting]")); + + secureSettings = new MockSecureSettings(); + secureSettings.setString("some.custom.secure.consistent.afix.wow.setting", "secure_value"); + final Settings settings2 = Settings.builder().setSecureSettings(secureSettings).build(); + final Setting afixConcreteConsistentSetting = Setting.affixKeySetting( + "some.custom.secure.consistent.afix.", "setting", + key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); + module = new SettingsModule(settings2,afixConcreteConsistentSetting); + assertInstanceBinding(module, Settings.class, (s) -> s == settings2); + assertThat(module.getConsistentSettings(), Matchers.containsInAnyOrder(afixConcreteConsistentSetting)); + + final Setting concreteUnsecureConsistentAfixSetting = Setting.affixKeySetting( + "some.custom.secure.consistent.afix.", "setting", + key -> Setting.simpleString(key, Setting.Property.Consistent, Property.NodeScope)); + e = expectThrows(IllegalArgumentException.class, + () -> new SettingsModule(Settings.builder().build(), concreteUnsecureConsistentAfixSetting)); + assertThat(e.getMessage(), is("Invalid consistent secure setting [some.custom.secure.consistent.afix.*.setting]")); } public void testLoggerSettings() { From f3dd7f07e89c8c2175074926365f766550a4ad18 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Sun, 23 Jun 2019 11:50:01 +0300 Subject: [PATCH 37/39] truncate ConsistentSettingsIT --- .../common/settings/ConsistentSettingsIT.java | 243 +++++------------- .../ConsistentSettingsServiceTests.java | 19 +- 2 files changed, 80 insertions(+), 182 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java index 0d5ac55d21219..4ee0c6849c2bc 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsIT.java @@ -40,95 +40,43 @@ public class ConsistentSettingsIT extends ESIntegTestCase { static final AffixSetting DUMMY_AFFIX_STRING_CONSISTENT_SETTING = Setting.affixKeySetting( "dummy.consistent.secure.string.affix.setting.", "suffix", key -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); - static final Setting EXTRA_DUMMY_STRING_CONSISTENT_SETTING = SecureSetting - .secureString("extra.dummy.consistent.secure.string.setting", null, Setting.Property.Consistent); - static final AffixSetting DUMMY_AFFIX_STRING_INCONSISTENT_SETTING = Setting.affixKeySetting( - "dummy.inconsistent.secure.string.affix.setting.", "suffix", - key -> SecureSetting.secureString(key, null)); private final AtomicReference> nodeSettingsOverride = new AtomicReference<>(null); - public void testAllConsistentSuccess() throws Exception { - internalCluster().getInstances(Environment.class).forEach(environment -> { - ClusterService clusterService = internalCluster().getInstance(ClusterService.class); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - }); - } - - public void testAllConsistentFailForSimpleSetting() throws Exception { - nodeSettingsOverride.set(nodeOrdinal -> { - Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); - MockSecureSettings secureSettings = new MockSecureSettings(); - // different value - secureSettings.setString("dummy.consistent.secure.string.setting", "DIFFERENT_VALUE"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); - assert builder.getSecureSettings() == null : "Deal with the settings merge"; - builder.setSecureSettings(secureSettings); - return builder.build(); - }); - String newNodeName = internalCluster().startNode(); - Environment environment = internalCluster().getInstance(Environment.class, newNodeName); - ClusterService clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - nodeSettingsOverride.set(null); - newNodeName = internalCluster().startNode(); - environment = internalCluster().getInstance(Environment.class, newNodeName); - clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - nodeSettingsOverride.set(nodeOrdinal -> { - Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); - MockSecureSettings secureSettings = new MockSecureSettings(); - // missing value - // secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); - assert builder.getSecureSettings() == null : "Deal with the settings merge"; - builder.setSecureSettings(secureSettings); - return builder.build(); - }); - newNodeName = internalCluster().startNode(); - environment = internalCluster().getInstance(Environment.class, newNodeName); - clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - nodeSettingsOverride.set(null); + public void testAllConsistentOnAllNodesSuccess() throws Exception { + for (String nodeName : internalCluster().getNodeNames()) { + Environment environment = internalCluster().getInstance(Environment.class, nodeName); + ClusterService clusterService = internalCluster().getInstance(ClusterService.class, nodeName); + assertTrue("Empty settings list always consistent.", + new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertTrue( + "Simple consistent secure setting is consistent [" + clusterService.state().metaData().hashesOfConsistentSettings() + + "].", + new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue( + "Affix consistent secure setting is consistent [" + clusterService.state().metaData().hashesOfConsistentSettings() + + "].", + new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue("All secure settings are consistent [" + clusterService.state().metaData().hashesOfConsistentSettings() + "].", + new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + } } - public void testAllConsistentFailForAffixSetting() throws Exception { + public void testConsistencyFailures() throws Exception { nodeSettingsOverride.set(nodeOrdinal -> { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); - secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); if (randomBoolean()) { - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "DIFFERENT_VALUE"); + // different value + secureSettings.setString("dummy.consistent.secure.string.setting", "DIFFERENT_VALUE"); } else { - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "DIFFERENT_VALUE_1"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "DIFFERENT_VALUE_2"); + // missing value + // secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); } + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); return builder.build(); @@ -136,35 +84,42 @@ public void testAllConsistentFailForAffixSetting() throws Exception { String newNodeName = internalCluster().startNode(); Environment environment = internalCluster().getInstance(Environment.class, newNodeName); ClusterService clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - nodeSettingsOverride.set(null); - newNodeName = internalCluster().startNode(); - environment = internalCluster().getInstance(Environment.class, newNodeName); - clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue("Empty settings list always consistent.", + new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertFalse( + "Simple consistent secure setting is NOT consistent [" + clusterService.state().metaData().hashesOfConsistentSettings() + + "].", + new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue( + "Affix consistent secure setting is consistent [" + clusterService.state().metaData().hashesOfConsistentSettings() + + "].", + new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse("All secure settings are NOT consistent [" + clusterService.state().metaData().hashesOfConsistentSettings() + "].", + new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(nodeOrdinal -> { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); - // missing value if (randomBoolean()) { secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); - // missing dummy.consistent.secure.string.affix.setting.affix2.suffix" + if (randomBoolean()) { + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "DIFFERENT_VALUE"); + } else { + // missing value + // "dummy.consistent.secure.string.affix.setting.affix2.suffix" + } } else { - // missing dummy.consistent.secure.string.affix.setting.affix1.suffix - // missing dummy.consistent.secure.string.affix.setting.affix2.suffix + if (randomBoolean()) { + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "DIFFERENT_VALUE_1"); + secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "DIFFERENT_VALUE_2"); + } else { + // missing values + // dummy.consistent.secure.string.affix.setting.affix1.suffix + // dummy.consistent.secure.string.affix.setting.affix2.suffix + } } assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); @@ -173,73 +128,21 @@ public void testAllConsistentFailForAffixSetting() throws Exception { newNodeName = internalCluster().startNode(); environment = internalCluster().getInstance(Environment.class, newNodeName); clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - nodeSettingsOverride.set(null); - } - - public void testAllConsistentAffixFailWithExtra() throws Exception { - nodeSettingsOverride.set(nodeOrdinal -> { - Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); - MockSecureSettings secureSettings = new MockSecureSettings(); - secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); - secureSettings.setString("extra.dummy.consistent.secure.string.setting", "extra_string_value"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); - assert builder.getSecureSettings() == null : "Deal with the settings merge"; - builder.setSecureSettings(secureSettings); - return builder.build(); - }); - String newNodeName = internalCluster().startNode(); - Environment environment = internalCluster().getInstance(Environment.class, newNodeName); - ClusterService clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(EXTRA_DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING, EXTRA_DUMMY_STRING_CONSISTENT_SETTING)) - .areAllConsistent()); - nodeSettingsOverride.set(null); - newNodeName = internalCluster().startNode(); - environment = internalCluster().getInstance(Environment.class, newNodeName); - clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - nodeSettingsOverride.set(nodeOrdinal -> { - Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); - MockSecureSettings secureSettings = new MockSecureSettings(); - secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); - secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix_extra" + ".suffix", "extra_affix_value"); - assert builder.getSecureSettings() == null : "Deal with the settings merge"; - builder.setSecureSettings(secureSettings); - return builder.build(); - }); - newNodeName = internalCluster().startNode(); - environment = internalCluster().getInstance(Environment.class, newNodeName); - clusterService = internalCluster().getInstance(ClusterService.class, newNodeName); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); - assertTrue(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); - assertFalse(new ConsistentSettingsService(environment.settings(), clusterService, - List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertTrue("Empty settings list always consistent.", + new ConsistentSettingsService(environment.settings(), clusterService, Collections.emptyList()).areAllConsistent()); + assertTrue( + "Simple consistent secure setting is consistent [" + clusterService.state().metaData().hashesOfConsistentSettings() + + "].", + new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse( + "Affix consistent secure setting is NOT consistent [" + clusterService.state().metaData().hashesOfConsistentSettings() + + "].", + new ConsistentSettingsService(environment.settings(), clusterService, + Collections.singletonList(DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); + assertFalse("All secure settings are NOT consistent [" + clusterService.state().metaData().hashesOfConsistentSettings() + "].", + new ConsistentSettingsService(environment.settings(), clusterService, + List.of(DUMMY_STRING_CONSISTENT_SETTING, DUMMY_AFFIX_STRING_CONSISTENT_SETTING)).areAllConsistent()); nodeSettingsOverride.set(null); } @@ -257,8 +160,6 @@ protected Settings nodeSettings(int nodeOrdinal) { secureSettings.setString("dummy.consistent.secure.string.setting", "string_value"); secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_1"); secureSettings.setString("dummy.consistent.secure.string.affix.setting." + "affix2" + ".suffix", "affix_value_2"); - // this is "inconsistent" hence it can differ from node to node - secureSettings.setString("dummy.inconsistent.secure.string.affix.setting." + "affix1" + ".suffix", "affix_value_" + nodeOrdinal); assert builder.getSecureSettings() == null : "Deal with the settings merge"; builder.setSecureSettings(secureSettings); return builder.build(); @@ -281,8 +182,6 @@ public List> getSettings() { List> settings = new ArrayList<>(super.getSettings()); settings.add(DUMMY_STRING_CONSISTENT_SETTING); settings.add(DUMMY_AFFIX_STRING_CONSISTENT_SETTING); - settings.add(DUMMY_AFFIX_STRING_INCONSISTENT_SETTING); - settings.add(EXTRA_DUMMY_STRING_CONSISTENT_SETTING); return settings; } } diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java index 28256d414cb44..7892f3cdac2ab 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java @@ -83,8 +83,8 @@ public void testSingleAffixSetting() throws Exception { // add two affix settings to the keystore MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("test.noise.setting", "noise"); - secureSettings.setString("test.afix.first.bar", "first_secure"); - secureSettings.setString("test.afix.second.bar", "second_secure"); + secureSettings.setString("test.affix.first.bar", "first_secure"); + secureSettings.setString("test.affix.second.bar", "second_secure"); Settings.Builder builder = Settings.builder(); builder.setSecureSettings(secureSettings); Settings settings = builder.build(); @@ -95,7 +95,7 @@ public void testSingleAffixSetting() throws Exception { ConsistentSettingsService consistentService = new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)); assertThat(consistentService.areAllConsistent(), is(true)); // change value - secureSettings.setString("test.afix.second.bar", "_TYPO_second_secure"); + secureSettings.setString("test.affix.second.bar", "_TYPO_second_secure"); assertThat(consistentService.areAllConsistent(), is(false)); assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(false)); // publish change @@ -103,7 +103,7 @@ public void testSingleAffixSetting() throws Exception { assertThat(consistentService.areAllConsistent(), is(true)); assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(true)); // add value - secureSettings.setString("test.afix.third.bar", "third_secure"); + secureSettings.setString("test.affix.third.bar", "third_secure"); builder = Settings.builder(); builder.setSecureSettings(secureSettings); settings = builder.build(); @@ -114,9 +114,9 @@ public void testSingleAffixSetting() throws Exception { // remove value secureSettings = new MockSecureSettings(); secureSettings.setString("test.another.noise.setting", "noise"); - // missing value test.afix.first.bar - secureSettings.setString("test.afix.second.bar", "second_secure"); - secureSettings.setString("test.afix.third.bar", "third_secure"); + // missing value test.affix.first.bar + secureSettings.setString("test.affix.second.bar", "second_secure"); + secureSettings.setString("test.affix.third.bar", "third_secure"); builder = Settings.builder(); builder.setSecureSettings(secureSettings); settings = builder.build(); @@ -125,12 +125,12 @@ public void testSingleAffixSetting() throws Exception { public void testStringAndAffixSettings() throws Exception { Setting stringSetting = SecureSetting.secureString("mock.simple.foo", null, Setting.Property.Consistent); - Setting.AffixSetting affixStringSetting = Setting.affixKeySetting("mock.afix.", "bar", + Setting.AffixSetting affixStringSetting = Setting.affixKeySetting("mock.affix.", "bar", (key) -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString(randomAlphaOfLength(8).toLowerCase(Locale.ROOT), "noise"); secureSettings.setString(stringSetting.getKey(), "somethingsecure"); - secureSettings.setString("mock.afix.foo.bar", "another_secure"); + secureSettings.setString("mock.affix.foo.bar", "another_secure"); Settings.Builder builder = Settings.builder(); builder.setSecureSettings(secureSettings); Settings settings = builder.build(); @@ -155,6 +155,5 @@ public void testStringAndAffixSettings() throws Exception { assertThat(new ConsistentSettingsService(settings, clusterService, List.of(affixStringSetting)).areAllConsistent(), is(true)); assertThat(new ConsistentSettingsService(settings, clusterService, List.of(stringSetting, affixStringSetting)).areAllConsistent(), is(true)); - } } From d38f7a56b91ce85e068fb828c47f5e9abdf43c07 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Sun, 23 Jun 2019 12:12:30 +0300 Subject: [PATCH 38/39] KeyStoreWrapperTests --- .../org/elasticsearch/common/settings/KeyStoreWrapperTests.java | 1 - 1 file changed, 1 deletion(-) 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 b2233ce125bfa..568ddfe97df16 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 @@ -54,7 +54,6 @@ import java.security.MessageDigest; import java.security.SecureRandom; import java.util.ArrayList; -import java.util.Arrays; import java.util.Base64; import java.util.List; import java.util.Locale; From 7cee3019f561e1a4cecf082e5fc66d7e8a3e8433 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Sun, 23 Jun 2019 13:06:55 +0300 Subject: [PATCH 39/39] afix -> affix fallout --- .../common/settings/ConsistentSettingsServiceTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java index 7892f3cdac2ab..687b74e3397cb 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ConsistentSettingsServiceTests.java @@ -78,7 +78,7 @@ public void testSingleStringSetting() throws Exception { } public void testSingleAffixSetting() throws Exception { - Setting.AffixSetting affixStringSetting = Setting.affixKeySetting("test.afix.", "bar", + Setting.AffixSetting affixStringSetting = Setting.affixKeySetting("test.affix.", "bar", (key) -> SecureSetting.secureString(key, null, Setting.Property.Consistent)); // add two affix settings to the keystore MockSecureSettings secureSettings = new MockSecureSettings();