diff --git a/CHANGELOG.md b/CHANGELOG.md index 208cddaa27a68..04e8e85df6b80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - WLM group custom search settings - groundwork and timeout ([#20536](https://github.com/opensearch-project/OpenSearch/issues/20536)) - Expose JVM runtime metrics via telemetry framework ([#20844](https://github.com/opensearch-project/OpenSearch/pull/20844)) - Add intra segment support for single-value metric aggregations ([#20503](https://github.com/opensearch-project/OpenSearch/pull/20503)) +- Add new setting property 'Sensitive' for tiering dynamic settings ([#20901](https://github.com/opensearch-project/OpenSearch/pull/20901)) - Add ref_path support for package-based hunspell dictionary loading ([#20840](https://github.com/opensearch-project/OpenSearch/pull/20840)) - Add support for enabling pluggable data formats, starting with phase-1 of decoupling shard from engine, and introducing basic abstractions ([#20675](https://github.com/opensearch-project/OpenSearch/pull/20675)) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/settings/SettingsUpdater.java b/server/src/main/java/org/opensearch/action/admin/cluster/settings/SettingsUpdater.java index c7030764a5b47..034b87bb07b29 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/settings/SettingsUpdater.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/settings/SettingsUpdater.java @@ -76,6 +76,7 @@ synchronized ClusterState updateSettings( final Settings persistentToApply, final Logger logger ) { + validateNoTransientSensitiveSettings(transientToApply); boolean changed = false; /* @@ -212,4 +213,12 @@ private void logInvalidSetting( ); } + private void validateNoTransientSensitiveSettings(final Settings transientSettings) { + for (String key : transientSettings.keySet()) { + if (clusterSettings.isSensitiveSetting(key)) { + throw new IllegalArgumentException("sensitive setting [" + key + "] must be updated using persistent settings"); + } + } + } + } diff --git a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java index d3ed1d839093f..f9d57aabe3c2d 100644 --- a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java @@ -775,6 +775,15 @@ public boolean isUnmodifiableOnRestoreSetting(String key) { return setting != null && setting.isUnmodifiableOnRestore(); } + /** + * Returns true if the setting for the given key is sensitive, meaning it requires + * security admin privileges to be updated dynamically. Otherwise false. + */ + public boolean isSensitiveSetting(String key) { + final Setting setting = get(key); + return setting != null && setting.isSensitive(); + } + /** * Returns a settings object that contains all settings that are not * already set in the given source. The diff contains either the default value for each diff --git a/server/src/main/java/org/opensearch/common/settings/Setting.java b/server/src/main/java/org/opensearch/common/settings/Setting.java index f77568bf4f711..aacbefba6838e 100644 --- a/server/src/main/java/org/opensearch/common/settings/Setting.java +++ b/server/src/main/java/org/opensearch/common/settings/Setting.java @@ -177,7 +177,15 @@ public enum Property { * Mark this setting as immutable on snapshot restore * i.e. the setting will not be allowed to be removed or modified during restore */ - UnmodifiableOnRestore + UnmodifiableOnRestore, + + /** + * Marks a setting as sensitive. Can only be applied to dynamic settings. + * The Sensitive property has default enforcement but enables plugins to implement + * different policies for these settings. In practice the security plugin will + * require higher privileges for modifying sensitive settings. + */ + Sensitive } private final Key key; @@ -217,6 +225,9 @@ private Setting( } else if (propertiesAsSet.contains(Property.UnmodifiableOnRestore) && propertiesAsSet.contains(Property.Dynamic)) { throw new IllegalArgumentException("UnmodifiableOnRestore setting [" + key + "] cannot be dynamic"); } + if (propertiesAsSet.contains(Property.Sensitive) && propertiesAsSet.contains(Property.Dynamic) == false) { + throw new IllegalArgumentException("sensitive setting [" + key + "] must be dynamic"); + } checkPropertyRequiresIndexScope(propertiesAsSet, Property.NotCopyableOnResize); checkPropertyRequiresIndexScope(propertiesAsSet, Property.InternalIndex); checkPropertyRequiresIndexScope(propertiesAsSet, Property.PrivateIndex); @@ -361,6 +372,14 @@ public final boolean isUnmodifiableOnRestore() { return properties.contains(Property.UnmodifiableOnRestore); } + /** + * Returns true if this setting is sensitive, meaning it requires security admin + * privileges to be updated dynamically. Otherwise false. + */ + public final boolean isSensitive() { + return properties.contains(Property.Sensitive); + } + public final boolean isInternalIndex() { return properties.contains(Property.InternalIndex); } diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/settings/SettingsUpdaterTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/settings/SettingsUpdaterTests.java index 8d65c24f36b63..7052e2a41a27b 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/settings/SettingsUpdaterTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/settings/SettingsUpdaterTests.java @@ -670,4 +670,24 @@ public void testUpdateOfValidationDependentSettings() { assertThat(exception.getMessage(), equalTo("[high]=2 is lower than [low]=5")); } + public void testRejectSensitiveSettingInTransient() { + Setting sensitiveSetting = Setting.simpleString( + "sensitive.setting", + Property.Dynamic, + Property.NodeScope, + Property.Sensitive + ); + final Set> settingsSet = Stream.concat(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.stream(), Stream.of(sensitiveSetting)) + .collect(Collectors.toSet()); + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, settingsSet); + SettingsUpdater updater = new SettingsUpdater(clusterSettings); + ClusterState state = ClusterState.builder(new ClusterName("foo")).metadata(Metadata.builder().build()).build(); + + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> updater.updateSettings(state, Settings.builder().put("sensitive.setting", "value").build(), Settings.EMPTY, logger) + ); + assertThat(ex.getMessage(), equalTo("sensitive setting [sensitive.setting] must be updated using persistent settings")); + } + } diff --git a/server/src/test/java/org/opensearch/common/settings/SettingTests.java b/server/src/test/java/org/opensearch/common/settings/SettingTests.java index a0788b0c83e11..68cea5a4200c7 100644 --- a/server/src/test/java/org/opensearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/opensearch/common/settings/SettingTests.java @@ -1447,6 +1447,14 @@ public void testRejectConflictingDynamicAndUnmodifiableOnRestoreProperties() { assertThat(ex.getMessage(), containsString("UnmodifiableOnRestore setting [foo.bar] cannot be dynamic")); } + public void testRejectSensitiveWithoutDynamic() { + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> Setting.simpleString("foo.bar", Property.Sensitive, Property.NodeScope) + ); + assertThat(ex.getMessage(), containsString("sensitive setting [foo.bar] must be dynamic")); + } + public void testRejectNonIndexScopedUnmodifiableOnRestoreSetting() { final IllegalArgumentException e = expectThrows( IllegalArgumentException.class,