Skip to content

Commit 15a6442

Browse files
authored
Validate SSL settings at parse time (#49196)
1 parent dba8fe1 commit 15a6442

File tree

5 files changed

+66
-31
lines changed

5 files changed

+66
-31
lines changed

x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
114114
Map<String, Exporter.Factory> exporterFactories = new HashMap<>();
115115
exporterFactories.put(HttpExporter.TYPE, config -> new HttpExporter(config, dynamicSSLService, threadPool.getThreadContext()));
116116
exporterFactories.put(LocalExporter.TYPE, config -> new LocalExporter(config, client, cleanerService));
117-
exporters = new Exporters(settings, exporterFactories, clusterService, getLicenseState(), threadPool.getThreadContext());
117+
exporters = new Exporters(settings, exporterFactories, clusterService, getLicenseState(), threadPool.getThreadContext(),
118+
dynamicSSLService);
118119

119120
Set<Collector> collectors = new HashSet<>();
120121
collectors.add(new IndexStatsCollector(clusterService, getLicenseState(), client));

x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporters.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.elasticsearch.common.util.concurrent.ThreadContext;
2323
import org.elasticsearch.gateway.GatewayService;
2424
import org.elasticsearch.license.XPackLicenseState;
25+
import org.elasticsearch.xpack.core.ssl.SSLService;
2526
import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter;
2627
import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc;
2728
import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporter;
@@ -51,7 +52,7 @@ public class Exporters extends AbstractLifecycleComponent {
5152

5253
public Exporters(Settings settings, Map<String, Exporter.Factory> factories,
5354
ClusterService clusterService, XPackLicenseState licenseState,
54-
ThreadContext threadContext) {
55+
ThreadContext threadContext, SSLService sslService) {
5556
this.settings = settings;
5657
this.factories = factories;
5758
this.exporters = new AtomicReference<>(emptyMap());
@@ -62,7 +63,7 @@ public Exporters(Settings settings, Map<String, Exporter.Factory> factories,
6263
final List<Setting.AffixSetting<?>> dynamicSettings =
6364
getSettings().stream().filter(Setting::isDynamic).collect(Collectors.toList());
6465
clusterService.getClusterSettings().addSettingsUpdateConsumer(this::setExportersSetting, dynamicSettings);
65-
HttpExporter.registerSettingValidators(clusterService);
66+
HttpExporter.registerSettingValidators(clusterService, sslService);
6667
// this ensures that logging is happening by adding an empty consumer per affix setting
6768
for (Setting.AffixSetting<?> affixSetting : dynamicSettings) {
6869
clusterService.getClusterSettings().addAffixUpdateConsumer(affixSetting, (s, o) -> {}, (s, o) -> {});

x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java

Lines changed: 49 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ public Iterator<Setting<?>> settings() {
307307
"ssl",
308308
(key) -> Setting.groupSetting(key + ".", Property.Dynamic, Property.NodeScope, Property.Filtered),
309309
TYPE_DEPENDENCY);
310+
310311
/**
311312
* Proxy setting to allow users to send requests to a remote cluster that requires a proxy base path.
312313
*/
@@ -482,24 +483,36 @@ public HttpExporter(final Config config, final SSLService sslService, final Thre
482483
* Because it is not possible to re-read the secure settings during a dynamic update, we cannot rebuild the {@link SSLIOSessionStrategy}
483484
* (see {@link #configureSecurity(RestClientBuilder, Config, SSLService)} if this exporter has been configured with secure settings
484485
*/
485-
public static void registerSettingValidators(ClusterService clusterService) {
486+
public static void registerSettingValidators(ClusterService clusterService, SSLService sslService) {
486487
clusterService.getClusterSettings().addAffixUpdateConsumer(SSL_SETTING,
487488
(ignoreKey, ignoreSettings) -> {
488489
// no-op update. We only care about the validator
489490
},
490-
(namespace, settings) -> {
491-
final List<String> secureSettings = SSLConfigurationSettings.withoutPrefix()
492-
.getSecureSettingsInUse(settings)
493-
.stream()
494-
.map(Setting::getKey)
495-
.collect(Collectors.toList());
496-
if (secureSettings.isEmpty() == false) {
497-
throw new IllegalStateException("Cannot dynamically update SSL settings for the exporter [" + namespace
498-
+ "] as it depends on the secure setting(s) [" + Strings.collectionToCommaDelimitedString(secureSettings) + "]");
499-
}
491+
(key, settings) -> {
492+
validateSslSettings(key, settings);
493+
configureSslStrategy(settings, null, sslService);
500494
});
501495
}
502496

497+
/**
498+
* Validates that secure settings are not being used to rebuild the {@link SSLIOSessionStrategy}.
499+
*
500+
* @param exporter Name of the exporter to validate
501+
* @param settings Settings for the exporter
502+
* @throws IllegalStateException if any secure settings are used in the SSL configuration
503+
*/
504+
private static void validateSslSettings(String exporter, Settings settings) {
505+
final List<String> secureSettings = SSLConfigurationSettings.withoutPrefix()
506+
.getSecureSettingsInUse(settings)
507+
.stream()
508+
.map(Setting::getKey)
509+
.collect(Collectors.toList());
510+
if (secureSettings.isEmpty() == false) {
511+
throw new IllegalStateException("Cannot dynamically update SSL settings for the exporter [" + exporter
512+
+ "] as it depends on the secure setting(s) [" + Strings.collectionToCommaDelimitedString(secureSettings) + "]");
513+
}
514+
}
515+
503516
/**
504517
* Create a {@link RestClientBuilder} from the HTTP Exporter's {@code config}.
505518
*
@@ -658,6 +671,30 @@ private static void configureHeaders(final RestClientBuilder builder, final Conf
658671
private static void configureSecurity(final RestClientBuilder builder, final Config config, final SSLService sslService) {
659672
final Setting<Settings> concreteSetting = SSL_SETTING.getConcreteSettingForNamespace(config.name());
660673
final Settings sslSettings = concreteSetting.get(config.settings());
674+
final SSLIOSessionStrategy sslStrategy = configureSslStrategy(sslSettings, concreteSetting, sslService);
675+
final CredentialsProvider credentialsProvider = createCredentialsProvider(config);
676+
List<String> hostList = HOST_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings());
677+
// sending credentials in plaintext!
678+
if (credentialsProvider != null && hostList.stream().findFirst().orElse("").startsWith("https") == false) {
679+
logger.warn("exporter [{}] is not using https, but using user authentication with plaintext " +
680+
"username/password!", config.name());
681+
}
682+
683+
if (sslStrategy != null) {
684+
builder.setHttpClientConfigCallback(new SecurityHttpClientConfigCallback(sslStrategy, credentialsProvider));
685+
}
686+
}
687+
688+
/**
689+
* Configures the {@link SSLIOSessionStrategy} to use. Relies on {@link #registerSettingValidators(ClusterService, SSLService)}
690+
* to prevent invalid usage of secure settings in the SSL strategy.
691+
* @param sslSettings The exporter's SSL settings
692+
* @param concreteSetting Settings to use for {@link SSLConfiguration} if secure settings are used
693+
* @param sslService The SSL Service used to create the SSL Context necessary for TLS / SSL communication
694+
* @return Appropriately configured instance of {@link SSLIOSessionStrategy}
695+
*/
696+
private static SSLIOSessionStrategy configureSslStrategy(final Settings sslSettings, final Setting<Settings> concreteSetting,
697+
final SSLService sslService) {
661698
final SSLIOSessionStrategy sslStrategy;
662699
if (SSLConfigurationSettings.withoutPrefix().getSecureSettingsInUse(sslSettings).isEmpty()) {
663700
// This configuration does not use secure settings, so it is possible that is has been dynamically updated.
@@ -670,17 +707,7 @@ private static void configureSecurity(final RestClientBuilder builder, final Con
670707
final SSLConfiguration sslConfiguration = sslService.getSSLConfiguration(concreteSetting.getKey());
671708
sslStrategy = sslService.sslIOSessionStrategy(sslConfiguration);
672709
}
673-
final CredentialsProvider credentialsProvider = createCredentialsProvider(config);
674-
List<String> hostList = HOST_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings());
675-
// sending credentials in plaintext!
676-
if (credentialsProvider != null && hostList.stream().findFirst().orElse("").startsWith("https") == false) {
677-
logger.warn("exporter [{}] is not using https, but using user authentication with plaintext " +
678-
"username/password!", config.name());
679-
}
680-
681-
if (sslStrategy != null) {
682-
builder.setHttpClientConfigCallback(new SecurityHttpClientConfigCallback(sslStrategy, credentialsProvider));
683-
}
710+
return sslStrategy;
684711
}
685712

686713
/**

x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.test.ESTestCase;
1616
import org.elasticsearch.threadpool.TestThreadPool;
1717
import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc;
18+
import org.elasticsearch.xpack.core.ssl.SSLService;
1819
import org.elasticsearch.xpack.monitoring.exporter.ExportException;
1920
import org.elasticsearch.xpack.monitoring.exporter.Exporters;
2021
import org.junit.After;
@@ -39,6 +40,7 @@ public class MonitoringServiceTests extends ESTestCase {
3940
private XPackLicenseState licenseState = mock(XPackLicenseState.class);
4041
private ClusterService clusterService;
4142
private ClusterSettings clusterSettings;
43+
private SSLService sslService;
4244

4345
@Before
4446
public void setUp() throws Exception {
@@ -59,6 +61,7 @@ protected XPackLicenseState getLicenseState() {
5961
when(clusterService.getClusterSettings()).thenReturn(clusterSettings);
6062
when(clusterService.state()).thenReturn(mock(ClusterState.class));
6163
when(clusterService.lifecycleState()).thenReturn(Lifecycle.State.STARTED);
64+
sslService = mock(SSLService.class);
6265
}
6366

6467
@After
@@ -144,7 +147,7 @@ class CountingExporter extends Exporters {
144147
private final AtomicInteger exports = new AtomicInteger(0);
145148

146149
CountingExporter() {
147-
super(Settings.EMPTY, Collections.emptyMap(), clusterService, licenseState, threadPool.getThreadContext());
150+
super(Settings.EMPTY, Collections.emptyMap(), clusterService, licenseState, threadPool.getThreadContext(), sslService);
148151
}
149152

150153
@Override

x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.elasticsearch.threadpool.ThreadPool;
2626
import org.elasticsearch.xpack.core.monitoring.MonitoredSystem;
2727
import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc;
28+
import org.elasticsearch.xpack.core.ssl.SSLService;
2829
import org.elasticsearch.xpack.monitoring.MonitoringService;
2930
import org.elasticsearch.xpack.monitoring.cleaner.CleanerService;
3031
import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter;
@@ -68,6 +69,7 @@ public class ExportersTests extends ESTestCase {
6869
private Map<String, Exporter.Factory> factories;
6970
private ClusterService clusterService;
7071
private ClusterState state;
72+
private SSLService sslService;
7173
private final ClusterBlocks blocks = mock(ClusterBlocks.class);
7274
private final MetaData metadata = mock(MetaData.class);
7375
private final XPackLicenseState licenseState = mock(XPackLicenseState.class);
@@ -93,11 +95,12 @@ public void init() {
9395
when(clusterService.state()).thenReturn(state);
9496
when(state.blocks()).thenReturn(blocks);
9597
when(state.metaData()).thenReturn(metadata);
98+
sslService = mock(SSLService.class);
9699

97100
// we always need to have the local exporter as it serves as the default one
98101
factories.put(LocalExporter.TYPE, config -> new LocalExporter(config, client, mock(CleanerService.class)));
99102

100-
exporters = new Exporters(Settings.EMPTY, factories, clusterService, licenseState, threadContext);
103+
exporters = new Exporters(Settings.EMPTY, factories, clusterService, licenseState, threadContext, sslService);
101104
}
102105

103106
public void testHostsMustBeSetIfTypeIsHttp() {
@@ -229,7 +232,7 @@ public void testSettingsUpdate() throws Exception {
229232
clusterSettings = new ClusterSettings(nodeSettings, new HashSet<>(Exporters.getSettings()));
230233
when(clusterService.getClusterSettings()).thenReturn(clusterSettings);
231234

232-
exporters = new Exporters(nodeSettings, factories, clusterService, licenseState, threadContext) {
235+
exporters = new Exporters(nodeSettings, factories, clusterService, licenseState, threadContext, sslService) {
233236
@Override
234237
Map<String, Exporter> initExporters(Settings settings) {
235238
settingsHolder.set(settings);
@@ -275,7 +278,7 @@ public void testExporterBlocksOnClusterState() {
275278
settings.put("xpack.monitoring.exporters._name" + String.valueOf(i) + ".type", "record");
276279
}
277280

278-
final Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext);
281+
final Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext, sslService);
279282

280283
// synchronously checks the cluster state
281284
exporters.wrapExportBulk(ActionListener.wrap(
@@ -295,7 +298,7 @@ public void testNoExporters() throws Exception {
295298
.put("xpack.monitoring.exporters.explicitly_disabled.type", "local")
296299
.put("xpack.monitoring.exporters.explicitly_disabled.enabled", false);
297300

298-
Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext);
301+
Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext, sslService);
299302
exporters.start();
300303

301304
assertThat(exporters.getEnabledExporters(), empty());
@@ -319,7 +322,7 @@ public void testConcurrentExports() throws Exception {
319322

320323
factories.put("record", (s) -> new CountingExporter(s, threadContext));
321324

322-
Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext);
325+
Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext, sslService);
323326
exporters.start();
324327

325328
assertThat(exporters.getEnabledExporters(), hasSize(nbExporters));

0 commit comments

Comments
 (0)