diff --git a/docs/changelog/134349.yaml b/docs/changelog/134349.yaml new file mode 100644 index 0000000000000..e3e41af748dcf --- /dev/null +++ b/docs/changelog/134349.yaml @@ -0,0 +1,5 @@ +pr: 134349 +summary: Add `LoadedSecureSettings` for keeping temporary secure settings loaded +area: Security +type: enhancement +issues: [] diff --git a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/EnterpriseGeoIpDownloaderTaskExecutor.java b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/EnterpriseGeoIpDownloaderTaskExecutor.java index 1eb42493d2716..8313c8dbc4717 100644 --- a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/EnterpriseGeoIpDownloaderTaskExecutor.java +++ b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/EnterpriseGeoIpDownloaderTaskExecutor.java @@ -17,7 +17,7 @@ import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterStateListener; import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.settings.InMemoryClonedSecureSettings; import org.elasticsearch.common.settings.SecureSetting; import org.elasticsearch.common.settings.SecureSettings; import org.elasticsearch.common.settings.SecureString; @@ -34,14 +34,10 @@ import org.elasticsearch.tasks.TaskId; import org.elasticsearch.threadpool.ThreadPool; -import java.io.IOException; -import java.io.InputStream; import java.security.GeneralSecurityException; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import static org.elasticsearch.ingest.EnterpriseGeoIpTask.ENTERPRISE_GEOIP_DOWNLOADER; @@ -178,82 +174,13 @@ public synchronized void reload(Settings settings) { // `SecureSettings` are available here! cache them as they will be needed // whenever dynamic cluster settings change and we have to rebuild the accounts try { - this.cachedSecureSettings = extractSecureSettings(settings, List.of(MAXMIND_LICENSE_KEY_SETTING, IPINFO_TOKEN_SETTING)); + this.cachedSecureSettings = InMemoryClonedSecureSettings.cloneSecureSettings( + settings, + List.of(MAXMIND_LICENSE_KEY_SETTING, IPINFO_TOKEN_SETTING) + ); } catch (GeneralSecurityException e) { // rethrow as a runtime exception, there's logging higher up the call chain around ReloadablePlugin throw new ElasticsearchException("Exception while reloading enterprise geoip download task executor", e); } } - - /** - * Extracts the {@link SecureSettings}` out of the passed in {@link Settings} object. The {@code Setting} argument has to have the - * {@code SecureSettings} open/available. Normally {@code SecureSettings} are available only under specific callstacks (eg. during node - * initialization or during a `reload` call). The returned copy can be reused freely as it will never be closed (this is a bit of - * cheating, but it is necessary in this specific circumstance). Only works for secure settings of type string (not file). - * - * @param source A {@code Settings} object with its {@code SecureSettings} open/available. - * @param securePluginSettings The list of settings to copy. - * @return A copy of the {@code SecureSettings} of the passed in {@code Settings} argument. - */ - private static SecureSettings extractSecureSettings(Settings source, List> securePluginSettings) - throws GeneralSecurityException { - // get the secure settings out - final SecureSettings sourceSecureSettings = Settings.builder().put(source, true).getSecureSettings(); - // filter and cache them... - final Map innerMap = new HashMap<>(); - if (sourceSecureSettings != null && securePluginSettings != null) { - for (final String settingKey : sourceSecureSettings.getSettingNames()) { - for (final Setting secureSetting : securePluginSettings) { - if (secureSetting.match(settingKey)) { - innerMap.put( - settingKey, - new SecureSettingValue( - sourceSecureSettings.getString(settingKey), - sourceSecureSettings.getSHA256Digest(settingKey) - ) - ); - } - } - } - } - return new SecureSettings() { - @Override - public boolean isLoaded() { - return true; - } - - @Override - public SecureString getString(String setting) { - return innerMap.get(setting).value(); - } - - @Override - public Set getSettingNames() { - return innerMap.keySet(); - } - - @Override - public InputStream getFile(String setting) { - throw new UnsupportedOperationException("A cached SecureSetting cannot be a file"); - } - - @Override - public byte[] getSHA256Digest(String setting) { - return innerMap.get(setting).sha256Digest(); - } - - @Override - public void close() throws IOException {} - - @Override - public void writeTo(StreamOutput out) throws IOException { - throw new UnsupportedOperationException("A cached SecureSetting cannot be serialized"); - } - }; - } - - /** - * A single-purpose record for the internal implementation of extractSecureSettings - */ - private record SecureSettingValue(SecureString value, byte[] sha256Digest) {} } diff --git a/server/src/main/java/org/elasticsearch/common/settings/InMemoryClonedSecureSettings.java b/server/src/main/java/org/elasticsearch/common/settings/InMemoryClonedSecureSettings.java new file mode 100644 index 0000000000000..752f6ee3beede --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/settings/InMemoryClonedSecureSettings.java @@ -0,0 +1,97 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.common.settings; + +import org.elasticsearch.common.io.stream.StreamOutput; + +import java.io.InputStream; +import java.security.GeneralSecurityException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class InMemoryClonedSecureSettings { + + /** + * Creates a cloned (detached) {@link SecureSettings} instance by copying selected secure settings from the provided {@link Settings}. + * The returned instance does not require the original {@link SecureSettings} to remain open and will always report as loaded. + *

+ * Only secure settings of type {@code String} are supported (file-based secure settings are not). The returned instance cannot be + * serialized. Also, attempting to {@code close} it will not have any effect. + *

+ * The cloned secure settings will remain in memory for the lifetime of the returned object. This bypasses the normal lifecycle of + * {@link SecureSettings}. Great care must be taken when using this method to avoid unintentionally retaining sensitive data in memory. + * + * @param source the {@link Settings} object with open/available {@link SecureSettings} + * @param settingsToClone the list of secure settings definitions to copy if present + * @return a cloned {@link SecureSettings} containing only the selected settings if present + * @throws GeneralSecurityException if any secure setting cannot be accessed + */ + + public static SecureSettings cloneSecureSettings(Settings source, List> settingsToClone) throws GeneralSecurityException { + final SecureSettings sourceSecureSettings = Settings.builder().put(source, true).getSecureSettings(); + final Map clonedSettings = new HashMap<>(); + + if (sourceSecureSettings != null && settingsToClone != null) { + for (final String settingKey : sourceSecureSettings.getSettingNames()) { + for (final Setting secureSetting : settingsToClone) { + if (secureSetting.match(settingKey)) { + clonedSettings.put( + settingKey, + new SecureSettingValue( + sourceSecureSettings.getString(settingKey), + sourceSecureSettings.getSHA256Digest(settingKey) + ) + ); + } + } + } + } + + return new SecureSettings() { + @Override + public boolean isLoaded() { + return true; + } + + @Override + public SecureString getString(String setting) { + var secureSettingValue = clonedSettings.get(setting); + return secureSettingValue != null ? secureSettingValue.value().clone() : null; + } + + @Override + public Set getSettingNames() { + return clonedSettings.keySet(); + } + + @Override + public InputStream getFile(String setting) { + throw new UnsupportedOperationException("A cloned SecureSetting cannot be a file"); + } + + @Override + public byte[] getSHA256Digest(String setting) { + return clonedSettings.get(setting).sha256Digest(); + } + + @Override + public void close() {} + + @Override + public void writeTo(StreamOutput out) { + throw new UnsupportedOperationException("A cloned SecureSetting cannot be serialized"); + } + }; + } + + private record SecureSettingValue(SecureString value, byte[] sha256Digest) {} +} diff --git a/server/src/test/java/org/elasticsearch/common/settings/InMemoryClonedSecureSettingsTests.java b/server/src/test/java/org/elasticsearch/common/settings/InMemoryClonedSecureSettingsTests.java new file mode 100644 index 0000000000000..f75c661b46f26 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/common/settings/InMemoryClonedSecureSettingsTests.java @@ -0,0 +1,120 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.common.settings; + +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.security.GeneralSecurityException; +import java.util.List; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.notNullValue; + +public class InMemoryClonedSecureSettingsTests extends ESTestCase { + + public void testClonesMatchingSecureSettings() throws GeneralSecurityException, IOException { + var mockSecureSettings = new MockSecureSettings(); + mockSecureSettings.setString("secure.password", "changeme"); + mockSecureSettings.setString("secure.api_key", "abcd1234"); + + var settings = Settings.builder().put("some.other", "value").setSecureSettings(mockSecureSettings).build(); + + var securePasswordSetting = SecureSetting.secureString("secure.password", null); + var secureApiKeySetting = SecureSetting.secureString("secure.api_key", null); + + var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, List.of(securePasswordSetting, secureApiKeySetting)); + mockSecureSettings.close(); + + assertTrue(cloned.isLoaded()); + assertThat(cloned.getSettingNames(), containsInAnyOrder("secure.password", "secure.api_key")); + assertThat(cloned.getString("secure.password").toString(), equalTo("changeme")); + assertThat(cloned.getString("secure.api_key").toString(), equalTo("abcd1234")); + + assertThat(cloned.getSHA256Digest("secure.password"), notNullValue()); + assertThat(cloned.getSHA256Digest("secure.api_key"), notNullValue()); + } + + public void testIgnoresNonMatchingSettings() throws GeneralSecurityException { + var mockSecureSettings = new MockSecureSettings(); + mockSecureSettings.setString("secure.password", "changeme"); + + var settings = Settings.builder().setSecureSettings(mockSecureSettings).build(); + var differentSetting = SecureSetting.secureString("secure.token", null); + + var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, List.of(differentSetting)); + assertThat(cloned.getSettingNames().isEmpty(), equalTo(true)); + } + + public void testFileSettingThrows() throws GeneralSecurityException { + var mockSecureSettings = new MockSecureSettings(); + mockSecureSettings.setFile("secure.file", randomByteArrayOfLength(16)); + var settings = Settings.builder().setSecureSettings(mockSecureSettings).build(); + + var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, List.of()); + + UnsupportedOperationException ex = expectThrows(UnsupportedOperationException.class, () -> cloned.getFile("secure.file")); + assertThat(ex.getMessage(), equalTo("A cloned SecureSetting cannot be a file")); + } + + public void testWriteToThrows() throws Exception { + var mockSecureSettings = new MockSecureSettings(); + mockSecureSettings.setString("secure.secret", "topsecret"); + + var settings = Settings.builder().setSecureSettings(mockSecureSettings).build(); + var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, List.of()); + + UnsupportedOperationException ex = expectThrows(UnsupportedOperationException.class, () -> cloned.writeTo(null)); + assertThat(ex.getMessage(), equalTo("A cloned SecureSetting cannot be serialized")); + } + + public void testNullSourceOrSettingsList() throws Exception { + var empty = Settings.EMPTY; + + { + var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(empty, null); + assertThat(cloned.isLoaded(), equalTo(true)); + assertThat(cloned.getSettingNames().isEmpty(), equalTo(true)); + + } + var mockSecureSettings = new MockSecureSettings(); + mockSecureSettings.setString("secure.password", "changeme"); + + var settings = Settings.builder().setSecureSettings(mockSecureSettings).build(); + { + var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, null); + assertTrue(cloned.getSettingNames().isEmpty()); + } + } + + public void testClonesDoNotCloseSource() throws GeneralSecurityException, IOException { + var mockSecureSettings = new MockSecureSettings(); + mockSecureSettings.setString("secure.password", "changeme"); + var settings = Settings.builder().put("some.other", "value").setSecureSettings(mockSecureSettings).build(); + var securePasswordSetting = SecureSetting.secureString("secure.password", null); + var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, List.of(securePasswordSetting)); + mockSecureSettings.close(); + + { + SecureString clonedSecureString = cloned.getString("secure.password"); + assertArrayEquals(clonedSecureString.getChars(), "changeme".toCharArray()); + clonedSecureString.close(); + var exception = assertThrows(IllegalStateException.class, clonedSecureString::getChars); + assertThat(exception.getMessage(), equalTo("SecureString has already been closed")); + } + { + SecureString clonedSecureString = cloned.getString("secure.password"); + assertArrayEquals(clonedSecureString.getChars(), "changeme".toCharArray()); + assertFalse(clonedSecureString.isEmpty()); + } + } + +} 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 62c78659ad5a7..73d4b737ba259 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,19 +8,15 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.InMemoryClonedSecureSettings; import org.elasticsearch.common.settings.SecureSettings; -import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.common.util.LazyInitializable; import org.elasticsearch.core.Nullable; -import org.elasticsearch.core.Tuple; -import java.io.IOException; -import java.io.InputStream; import java.security.GeneralSecurityException; import java.util.Collections; import java.util.HashMap; @@ -81,7 +77,7 @@ public synchronized void reload(Settings settings) { // `SecureSettings` are available here! cache them as they will be needed // whenever dynamic cluster settings change and we have to rebuild the accounts try { - this.cachedSecureSettings = extractSecureSettings(settings, pluginSecureSettings); + this.cachedSecureSettings = InMemoryClonedSecureSettings.cloneSecureSettings(settings, pluginSecureSettings); } catch (GeneralSecurityException e) { logger.error("Keystore exception while reloading watcher notification service", e); return; @@ -180,70 +176,4 @@ protected Map> createAccou return account; } } - - /** - * Extracts the {@link SecureSettings}` out of the passed in {@link Settings} object. The {@code Setting} argument has to have the - * {@code SecureSettings} open/available. Normally {@code SecureSettings} are available only under specific callstacks (eg. during node - * initialization or during a `reload` call). The returned copy can be reused freely as it will never be closed (this is a bit of - * cheating, but it is necessary in this specific circumstance). Only works for secure settings of type string (not file). - * - * @param source - * A {@code Settings} object with its {@code SecureSettings} open/available. - * @param securePluginSettings - * The list of settings to copy. - * @return A copy of the {@code SecureSettings} of the passed in {@code Settings} argument. - */ - private static SecureSettings extractSecureSettings(Settings source, List> securePluginSettings) - throws GeneralSecurityException { - // get the secure settings out - final SecureSettings sourceSecureSettings = Settings.builder().put(source, true).getSecureSettings(); - // filter and cache them... - 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, - new Tuple<>(sourceSecureSettings.getString(settingKey), sourceSecureSettings.getSHA256Digest(settingKey)) - ); - } - } - } - } - return new SecureSettings() { - @Override - public boolean isLoaded() { - return true; - } - - @Override - public SecureString getString(String setting) { - return cache.get(setting).v1(); - } - - @Override - public Set getSettingNames() { - return cache.keySet(); - } - - @Override - 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 {} - - @Override - public void writeTo(StreamOutput out) throws IOException { - throw new IllegalStateException("Unsupported operation"); - } - }; - } }