diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java index 068dd2a1be9bd..01518092389cc 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java @@ -138,7 +138,8 @@ public Collection createComponents(Client client, ClusterService cluster Map exporterFactories = new HashMap<>(); exporterFactories.put(HttpExporter.TYPE, config -> new HttpExporter(config, dynamicSSLService, threadPool.getThreadContext())); exporterFactories.put(LocalExporter.TYPE, config -> new LocalExporter(config, client, cleanerService)); - exporters = new Exporters(settings, exporterFactories, clusterService, getLicenseState(), threadPool.getThreadContext()); + exporters = new Exporters(settings, exporterFactories, clusterService, getLicenseState(), threadPool.getThreadContext(), + dynamicSSLService); Set collectors = new HashSet<>(); collectors.add(new IndexStatsCollector(clusterService, getLicenseState(), client)); diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporters.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporters.java index 138ecccecd199..f48d87d365537 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporters.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporters.java @@ -22,6 +22,7 @@ import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.gateway.GatewayService; import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.xpack.core.ssl.SSLService; import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporter; @@ -51,7 +52,7 @@ public class Exporters extends AbstractLifecycleComponent { public Exporters(Settings settings, Map factories, ClusterService clusterService, XPackLicenseState licenseState, - ThreadContext threadContext) { + ThreadContext threadContext, SSLService sslService) { this.settings = settings; this.factories = factories; this.exporters = new AtomicReference<>(emptyMap()); @@ -62,7 +63,7 @@ public Exporters(Settings settings, Map factories, final List> dynamicSettings = getSettings().stream().filter(Setting::isDynamic).collect(Collectors.toList()); clusterService.getClusterSettings().addSettingsUpdateConsumer(this::setExportersSetting, dynamicSettings); - HttpExporter.registerSettingValidators(clusterService); + HttpExporter.registerSettingValidators(clusterService, sslService); // this ensures that logging is happening by adding an empty consumer per affix setting for (Setting.AffixSetting affixSetting : dynamicSettings) { clusterService.getClusterSettings().addAffixUpdateConsumer(affixSetting, (s, o) -> {}, (s, o) -> {}); diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java index 656848f6bed06..ec3398e402dd4 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java @@ -307,6 +307,7 @@ public Iterator> settings() { "ssl", (key) -> Setting.groupSetting(key + ".", Property.Dynamic, Property.NodeScope, Property.Filtered), TYPE_DEPENDENCY); + /** * Proxy setting to allow users to send requests to a remote cluster that requires a proxy base path. */ @@ -482,24 +483,36 @@ public HttpExporter(final Config config, final SSLService sslService, final Thre * Because it is not possible to re-read the secure settings during a dynamic update, we cannot rebuild the {@link SSLIOSessionStrategy} * (see {@link #configureSecurity(RestClientBuilder, Config, SSLService)} if this exporter has been configured with secure settings */ - public static void registerSettingValidators(ClusterService clusterService) { + public static void registerSettingValidators(ClusterService clusterService, SSLService sslService) { clusterService.getClusterSettings().addAffixUpdateConsumer(SSL_SETTING, (ignoreKey, ignoreSettings) -> { // no-op update. We only care about the validator }, - (namespace, settings) -> { - final List secureSettings = SSLConfigurationSettings.withoutPrefix() - .getSecureSettingsInUse(settings) - .stream() - .map(Setting::getKey) - .collect(Collectors.toList()); - if (secureSettings.isEmpty() == false) { - throw new IllegalStateException("Cannot dynamically update SSL settings for the exporter [" + namespace - + "] as it depends on the secure setting(s) [" + Strings.collectionToCommaDelimitedString(secureSettings) + "]"); - } + (key, settings) -> { + validateSslSettings(key, settings); + configureSslStrategy(settings, null, sslService); }); } + /** + * Validates that secure settings are not being used to rebuild the {@link SSLIOSessionStrategy}. + * + * @param exporter Name of the exporter to validate + * @param settings Settings for the exporter + * @throws IllegalStateException if any secure settings are used in the SSL configuration + */ + private static void validateSslSettings(String exporter, Settings settings) { + final List secureSettings = SSLConfigurationSettings.withoutPrefix() + .getSecureSettingsInUse(settings) + .stream() + .map(Setting::getKey) + .collect(Collectors.toList()); + if (secureSettings.isEmpty() == false) { + throw new IllegalStateException("Cannot dynamically update SSL settings for the exporter [" + exporter + + "] as it depends on the secure setting(s) [" + Strings.collectionToCommaDelimitedString(secureSettings) + "]"); + } + } + /** * Create a {@link RestClientBuilder} from the HTTP Exporter's {@code config}. * @@ -667,6 +680,30 @@ private static void configureHeaders(final RestClientBuilder builder, final Conf private static void configureSecurity(final RestClientBuilder builder, final Config config, final SSLService sslService) { final Setting concreteSetting = SSL_SETTING.getConcreteSettingForNamespace(config.name()); final Settings sslSettings = concreteSetting.get(config.settings()); + final SSLIOSessionStrategy sslStrategy = configureSslStrategy(sslSettings, concreteSetting, sslService); + final CredentialsProvider credentialsProvider = createCredentialsProvider(config); + List hostList = HOST_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); + // sending credentials in plaintext! + if (credentialsProvider != null && hostList.stream().findFirst().orElse("").startsWith("https") == false) { + logger.warn("exporter [{}] is not using https, but using user authentication with plaintext " + + "username/password!", config.name()); + } + + if (sslStrategy != null) { + builder.setHttpClientConfigCallback(new SecurityHttpClientConfigCallback(sslStrategy, credentialsProvider)); + } + } + + /** + * Configures the {@link SSLIOSessionStrategy} to use. Relies on {@link #registerSettingValidators(ClusterService, SSLService)} + * to prevent invalid usage of secure settings in the SSL strategy. + * @param sslSettings The exporter's SSL settings + * @param concreteSetting Settings to use for {@link SSLConfiguration} if secure settings are used + * @param sslService The SSL Service used to create the SSL Context necessary for TLS / SSL communication + * @return Appropriately configured instance of {@link SSLIOSessionStrategy} + */ + private static SSLIOSessionStrategy configureSslStrategy(final Settings sslSettings, final Setting concreteSetting, + final SSLService sslService) { final SSLIOSessionStrategy sslStrategy; if (SSLConfigurationSettings.withoutPrefix().getSecureSettingsInUse(sslSettings).isEmpty()) { // This configuration does not use secure settings, so it is possible that is has been dynamically updated. @@ -679,17 +716,7 @@ private static void configureSecurity(final RestClientBuilder builder, final Con final SSLConfiguration sslConfiguration = sslService.getSSLConfiguration(concreteSetting.getKey()); sslStrategy = sslService.sslIOSessionStrategy(sslConfiguration); } - final CredentialsProvider credentialsProvider = createCredentialsProvider(config); - List hostList = HOST_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); - // sending credentials in plaintext! - if (credentialsProvider != null && hostList.stream().findFirst().orElse("").startsWith("https") == false) { - logger.warn("exporter [{}] is not using https, but using user authentication with plaintext " + - "username/password!", config.name()); - } - - if (sslStrategy != null) { - builder.setHttpClientConfigCallback(new SecurityHttpClientConfigCallback(sslStrategy, credentialsProvider)); - } + return sslStrategy; } /** diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java index 14c2f332c0f54..81fc814d5baca 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java @@ -15,6 +15,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; +import org.elasticsearch.xpack.core.ssl.SSLService; import org.elasticsearch.xpack.monitoring.exporter.ExportException; import org.elasticsearch.xpack.monitoring.exporter.Exporters; import org.junit.After; @@ -39,6 +40,7 @@ public class MonitoringServiceTests extends ESTestCase { private XPackLicenseState licenseState = mock(XPackLicenseState.class); private ClusterService clusterService; private ClusterSettings clusterSettings; + private SSLService sslService; @Before public void setUp() throws Exception { @@ -59,6 +61,7 @@ protected XPackLicenseState getLicenseState() { when(clusterService.getClusterSettings()).thenReturn(clusterSettings); when(clusterService.state()).thenReturn(mock(ClusterState.class)); when(clusterService.lifecycleState()).thenReturn(Lifecycle.State.STARTED); + sslService = mock(SSLService.class); } @After @@ -144,7 +147,7 @@ class CountingExporter extends Exporters { private final AtomicInteger exports = new AtomicInteger(0); CountingExporter() { - super(Settings.EMPTY, Collections.emptyMap(), clusterService, licenseState, threadPool.getThreadContext()); + super(Settings.EMPTY, Collections.emptyMap(), clusterService, licenseState, threadPool.getThreadContext(), sslService); } @Override diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java index 711caf3a6468c..40e865cd964a5 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.monitoring.MonitoredSystem; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; +import org.elasticsearch.xpack.core.ssl.SSLService; import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.cleaner.CleanerService; import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter; @@ -68,6 +69,7 @@ public class ExportersTests extends ESTestCase { private Map factories; private ClusterService clusterService; private ClusterState state; + private SSLService sslService; private final ClusterBlocks blocks = mock(ClusterBlocks.class); private final MetaData metadata = mock(MetaData.class); private final XPackLicenseState licenseState = mock(XPackLicenseState.class); @@ -93,11 +95,12 @@ public void init() { when(clusterService.state()).thenReturn(state); when(state.blocks()).thenReturn(blocks); when(state.metaData()).thenReturn(metadata); + sslService = mock(SSLService.class); // we always need to have the local exporter as it serves as the default one factories.put(LocalExporter.TYPE, config -> new LocalExporter(config, client, mock(CleanerService.class))); - exporters = new Exporters(Settings.EMPTY, factories, clusterService, licenseState, threadContext); + exporters = new Exporters(Settings.EMPTY, factories, clusterService, licenseState, threadContext, sslService); } public void testHostsMustBeSetIfTypeIsHttp() { @@ -229,7 +232,7 @@ public void testSettingsUpdate() throws Exception { clusterSettings = new ClusterSettings(nodeSettings, new HashSet<>(Exporters.getSettings())); when(clusterService.getClusterSettings()).thenReturn(clusterSettings); - exporters = new Exporters(nodeSettings, factories, clusterService, licenseState, threadContext) { + exporters = new Exporters(nodeSettings, factories, clusterService, licenseState, threadContext, sslService) { @Override Map initExporters(Settings settings) { settingsHolder.set(settings); @@ -275,7 +278,7 @@ public void testExporterBlocksOnClusterState() { settings.put("xpack.monitoring.exporters._name" + String.valueOf(i) + ".type", "record"); } - final Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext); + final Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext, sslService); // synchronously checks the cluster state exporters.wrapExportBulk(ActionListener.wrap( @@ -295,7 +298,7 @@ public void testNoExporters() throws Exception { .put("xpack.monitoring.exporters.explicitly_disabled.type", "local") .put("xpack.monitoring.exporters.explicitly_disabled.enabled", false); - Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext); + Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext, sslService); exporters.start(); assertThat(exporters.getEnabledExporters(), empty()); @@ -319,7 +322,7 @@ public void testConcurrentExports() throws Exception { factories.put("record", (s) -> new CountingExporter(s, threadContext)); - Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext); + Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext, sslService); exporters.start(); assertThat(exporters.getEnabledExporters(), hasSize(nbExporters));