Skip to content

Commit fc358eb

Browse files
authored
Add InMemoryClonedSecureSettings for keeping temporary secure settings loaded (#134349)
The pattern to load secure settings for later use during secure settings reload has been repeated across the code base and is now needed by #134137. This PR extracts the functionality to a `InMemoryClonedSecureSettings ` utility class.
1 parent 54152ca commit fc358eb

File tree

5 files changed

+229
-150
lines changed

5 files changed

+229
-150
lines changed

docs/changelog/134349.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 134349
2+
summary: Add `LoadedSecureSettings` for keeping temporary secure settings loaded
3+
area: Security
4+
type: enhancement
5+
issues: []

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/EnterpriseGeoIpDownloaderTaskExecutor.java

Lines changed: 5 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import org.elasticsearch.cluster.ClusterChangedEvent;
1818
import org.elasticsearch.cluster.ClusterStateListener;
1919
import org.elasticsearch.cluster.service.ClusterService;
20-
import org.elasticsearch.common.io.stream.StreamOutput;
20+
import org.elasticsearch.common.settings.InMemoryClonedSecureSettings;
2121
import org.elasticsearch.common.settings.SecureSetting;
2222
import org.elasticsearch.common.settings.SecureSettings;
2323
import org.elasticsearch.common.settings.SecureString;
@@ -34,14 +34,10 @@
3434
import org.elasticsearch.tasks.TaskId;
3535
import org.elasticsearch.threadpool.ThreadPool;
3636

37-
import java.io.IOException;
38-
import java.io.InputStream;
3937
import java.security.GeneralSecurityException;
40-
import java.util.HashMap;
4138
import java.util.List;
4239
import java.util.Map;
4340
import java.util.Objects;
44-
import java.util.Set;
4541
import java.util.concurrent.atomic.AtomicReference;
4642

4743
import static org.elasticsearch.ingest.EnterpriseGeoIpTask.ENTERPRISE_GEOIP_DOWNLOADER;
@@ -178,82 +174,13 @@ public synchronized void reload(Settings settings) {
178174
// `SecureSettings` are available here! cache them as they will be needed
179175
// whenever dynamic cluster settings change and we have to rebuild the accounts
180176
try {
181-
this.cachedSecureSettings = extractSecureSettings(settings, List.of(MAXMIND_LICENSE_KEY_SETTING, IPINFO_TOKEN_SETTING));
177+
this.cachedSecureSettings = InMemoryClonedSecureSettings.cloneSecureSettings(
178+
settings,
179+
List.of(MAXMIND_LICENSE_KEY_SETTING, IPINFO_TOKEN_SETTING)
180+
);
182181
} catch (GeneralSecurityException e) {
183182
// rethrow as a runtime exception, there's logging higher up the call chain around ReloadablePlugin
184183
throw new ElasticsearchException("Exception while reloading enterprise geoip download task executor", e);
185184
}
186185
}
187-
188-
/**
189-
* Extracts the {@link SecureSettings}` out of the passed in {@link Settings} object. The {@code Setting} argument has to have the
190-
* {@code SecureSettings} open/available. Normally {@code SecureSettings} are available only under specific callstacks (eg. during node
191-
* initialization or during a `reload` call). The returned copy can be reused freely as it will never be closed (this is a bit of
192-
* cheating, but it is necessary in this specific circumstance). Only works for secure settings of type string (not file).
193-
*
194-
* @param source A {@code Settings} object with its {@code SecureSettings} open/available.
195-
* @param securePluginSettings The list of settings to copy.
196-
* @return A copy of the {@code SecureSettings} of the passed in {@code Settings} argument.
197-
*/
198-
private static SecureSettings extractSecureSettings(Settings source, List<Setting<?>> securePluginSettings)
199-
throws GeneralSecurityException {
200-
// get the secure settings out
201-
final SecureSettings sourceSecureSettings = Settings.builder().put(source, true).getSecureSettings();
202-
// filter and cache them...
203-
final Map<String, SecureSettingValue> innerMap = new HashMap<>();
204-
if (sourceSecureSettings != null && securePluginSettings != null) {
205-
for (final String settingKey : sourceSecureSettings.getSettingNames()) {
206-
for (final Setting<?> secureSetting : securePluginSettings) {
207-
if (secureSetting.match(settingKey)) {
208-
innerMap.put(
209-
settingKey,
210-
new SecureSettingValue(
211-
sourceSecureSettings.getString(settingKey),
212-
sourceSecureSettings.getSHA256Digest(settingKey)
213-
)
214-
);
215-
}
216-
}
217-
}
218-
}
219-
return new SecureSettings() {
220-
@Override
221-
public boolean isLoaded() {
222-
return true;
223-
}
224-
225-
@Override
226-
public SecureString getString(String setting) {
227-
return innerMap.get(setting).value();
228-
}
229-
230-
@Override
231-
public Set<String> getSettingNames() {
232-
return innerMap.keySet();
233-
}
234-
235-
@Override
236-
public InputStream getFile(String setting) {
237-
throw new UnsupportedOperationException("A cached SecureSetting cannot be a file");
238-
}
239-
240-
@Override
241-
public byte[] getSHA256Digest(String setting) {
242-
return innerMap.get(setting).sha256Digest();
243-
}
244-
245-
@Override
246-
public void close() throws IOException {}
247-
248-
@Override
249-
public void writeTo(StreamOutput out) throws IOException {
250-
throw new UnsupportedOperationException("A cached SecureSetting cannot be serialized");
251-
}
252-
};
253-
}
254-
255-
/**
256-
* A single-purpose record for the internal implementation of extractSecureSettings
257-
*/
258-
private record SecureSettingValue(SecureString value, byte[] sha256Digest) {}
259186
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.common.settings;
11+
12+
import org.elasticsearch.common.io.stream.StreamOutput;
13+
14+
import java.io.InputStream;
15+
import java.security.GeneralSecurityException;
16+
import java.util.HashMap;
17+
import java.util.List;
18+
import java.util.Map;
19+
import java.util.Set;
20+
21+
public class InMemoryClonedSecureSettings {
22+
23+
/**
24+
* Creates a cloned (detached) {@link SecureSettings} instance by copying selected secure settings from the provided {@link Settings}.
25+
* The returned instance does not require the original {@link SecureSettings} to remain open and will always report as loaded.
26+
* <p>
27+
* Only secure settings of type {@code String} are supported (file-based secure settings are not). The returned instance cannot be
28+
* serialized. Also, attempting to {@code close} it will not have any effect.
29+
* <p>
30+
* The cloned secure settings will remain in memory for the lifetime of the returned object. This bypasses the normal lifecycle of
31+
* {@link SecureSettings}. Great care must be taken when using this method to avoid unintentionally retaining sensitive data in memory.
32+
*
33+
* @param source the {@link Settings} object with open/available {@link SecureSettings}
34+
* @param settingsToClone the list of secure settings definitions to copy if present
35+
* @return a cloned {@link SecureSettings} containing only the selected settings if present
36+
* @throws GeneralSecurityException if any secure setting cannot be accessed
37+
*/
38+
39+
public static SecureSettings cloneSecureSettings(Settings source, List<Setting<?>> settingsToClone) throws GeneralSecurityException {
40+
final SecureSettings sourceSecureSettings = Settings.builder().put(source, true).getSecureSettings();
41+
final Map<String, SecureSettingValue> clonedSettings = new HashMap<>();
42+
43+
if (sourceSecureSettings != null && settingsToClone != null) {
44+
for (final String settingKey : sourceSecureSettings.getSettingNames()) {
45+
for (final Setting<?> secureSetting : settingsToClone) {
46+
if (secureSetting.match(settingKey)) {
47+
clonedSettings.put(
48+
settingKey,
49+
new SecureSettingValue(
50+
sourceSecureSettings.getString(settingKey),
51+
sourceSecureSettings.getSHA256Digest(settingKey)
52+
)
53+
);
54+
}
55+
}
56+
}
57+
}
58+
59+
return new SecureSettings() {
60+
@Override
61+
public boolean isLoaded() {
62+
return true;
63+
}
64+
65+
@Override
66+
public SecureString getString(String setting) {
67+
var secureSettingValue = clonedSettings.get(setting);
68+
return secureSettingValue != null ? secureSettingValue.value().clone() : null;
69+
}
70+
71+
@Override
72+
public Set<String> getSettingNames() {
73+
return clonedSettings.keySet();
74+
}
75+
76+
@Override
77+
public InputStream getFile(String setting) {
78+
throw new UnsupportedOperationException("A cloned SecureSetting cannot be a file");
79+
}
80+
81+
@Override
82+
public byte[] getSHA256Digest(String setting) {
83+
return clonedSettings.get(setting).sha256Digest();
84+
}
85+
86+
@Override
87+
public void close() {}
88+
89+
@Override
90+
public void writeTo(StreamOutput out) {
91+
throw new UnsupportedOperationException("A cloned SecureSetting cannot be serialized");
92+
}
93+
};
94+
}
95+
96+
private record SecureSettingValue(SecureString value, byte[] sha256Digest) {}
97+
}
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.common.settings;
11+
12+
import org.elasticsearch.test.ESTestCase;
13+
14+
import java.io.IOException;
15+
import java.security.GeneralSecurityException;
16+
import java.util.List;
17+
18+
import static org.hamcrest.Matchers.containsInAnyOrder;
19+
import static org.hamcrest.Matchers.equalTo;
20+
import static org.hamcrest.Matchers.notNullValue;
21+
22+
public class InMemoryClonedSecureSettingsTests extends ESTestCase {
23+
24+
public void testClonesMatchingSecureSettings() throws GeneralSecurityException, IOException {
25+
var mockSecureSettings = new MockSecureSettings();
26+
mockSecureSettings.setString("secure.password", "changeme");
27+
mockSecureSettings.setString("secure.api_key", "abcd1234");
28+
29+
var settings = Settings.builder().put("some.other", "value").setSecureSettings(mockSecureSettings).build();
30+
31+
var securePasswordSetting = SecureSetting.secureString("secure.password", null);
32+
var secureApiKeySetting = SecureSetting.secureString("secure.api_key", null);
33+
34+
var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, List.of(securePasswordSetting, secureApiKeySetting));
35+
mockSecureSettings.close();
36+
37+
assertTrue(cloned.isLoaded());
38+
assertThat(cloned.getSettingNames(), containsInAnyOrder("secure.password", "secure.api_key"));
39+
assertThat(cloned.getString("secure.password").toString(), equalTo("changeme"));
40+
assertThat(cloned.getString("secure.api_key").toString(), equalTo("abcd1234"));
41+
42+
assertThat(cloned.getSHA256Digest("secure.password"), notNullValue());
43+
assertThat(cloned.getSHA256Digest("secure.api_key"), notNullValue());
44+
}
45+
46+
public void testIgnoresNonMatchingSettings() throws GeneralSecurityException {
47+
var mockSecureSettings = new MockSecureSettings();
48+
mockSecureSettings.setString("secure.password", "changeme");
49+
50+
var settings = Settings.builder().setSecureSettings(mockSecureSettings).build();
51+
var differentSetting = SecureSetting.secureString("secure.token", null);
52+
53+
var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, List.of(differentSetting));
54+
assertThat(cloned.getSettingNames().isEmpty(), equalTo(true));
55+
}
56+
57+
public void testFileSettingThrows() throws GeneralSecurityException {
58+
var mockSecureSettings = new MockSecureSettings();
59+
mockSecureSettings.setFile("secure.file", randomByteArrayOfLength(16));
60+
var settings = Settings.builder().setSecureSettings(mockSecureSettings).build();
61+
62+
var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, List.of());
63+
64+
UnsupportedOperationException ex = expectThrows(UnsupportedOperationException.class, () -> cloned.getFile("secure.file"));
65+
assertThat(ex.getMessage(), equalTo("A cloned SecureSetting cannot be a file"));
66+
}
67+
68+
public void testWriteToThrows() throws Exception {
69+
var mockSecureSettings = new MockSecureSettings();
70+
mockSecureSettings.setString("secure.secret", "topsecret");
71+
72+
var settings = Settings.builder().setSecureSettings(mockSecureSettings).build();
73+
var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, List.of());
74+
75+
UnsupportedOperationException ex = expectThrows(UnsupportedOperationException.class, () -> cloned.writeTo(null));
76+
assertThat(ex.getMessage(), equalTo("A cloned SecureSetting cannot be serialized"));
77+
}
78+
79+
public void testNullSourceOrSettingsList() throws Exception {
80+
var empty = Settings.EMPTY;
81+
82+
{
83+
var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(empty, null);
84+
assertThat(cloned.isLoaded(), equalTo(true));
85+
assertThat(cloned.getSettingNames().isEmpty(), equalTo(true));
86+
87+
}
88+
var mockSecureSettings = new MockSecureSettings();
89+
mockSecureSettings.setString("secure.password", "changeme");
90+
91+
var settings = Settings.builder().setSecureSettings(mockSecureSettings).build();
92+
{
93+
var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, null);
94+
assertTrue(cloned.getSettingNames().isEmpty());
95+
}
96+
}
97+
98+
public void testClonesDoNotCloseSource() throws GeneralSecurityException, IOException {
99+
var mockSecureSettings = new MockSecureSettings();
100+
mockSecureSettings.setString("secure.password", "changeme");
101+
var settings = Settings.builder().put("some.other", "value").setSecureSettings(mockSecureSettings).build();
102+
var securePasswordSetting = SecureSetting.secureString("secure.password", null);
103+
var cloned = InMemoryClonedSecureSettings.cloneSecureSettings(settings, List.of(securePasswordSetting));
104+
mockSecureSettings.close();
105+
106+
{
107+
SecureString clonedSecureString = cloned.getString("secure.password");
108+
assertArrayEquals(clonedSecureString.getChars(), "changeme".toCharArray());
109+
clonedSecureString.close();
110+
var exception = assertThrows(IllegalStateException.class, clonedSecureString::getChars);
111+
assertThat(exception.getMessage(), equalTo("SecureString has already been closed"));
112+
}
113+
{
114+
SecureString clonedSecureString = cloned.getString("secure.password");
115+
assertArrayEquals(clonedSecureString.getChars(), "changeme".toCharArray());
116+
assertFalse(clonedSecureString.isEmpty());
117+
}
118+
}
119+
120+
}

0 commit comments

Comments
 (0)