From 11f3d348fa1ca74558526d98cfe58a67a4f74ed2 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 18 Dec 2019 12:58:57 -0600 Subject: [PATCH 01/18] Secure monitoring password --- .../settings/monitoring-settings.asciidoc | 15 ++++++--- .../xpack/monitoring/Monitoring.java | 8 ++++- .../xpack/monitoring/MonitoringService.java | 3 ++ .../exporter/http/HttpExporter.java | 31 +++++++++++++++++-- 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/docs/reference/settings/monitoring-settings.asciidoc b/docs/reference/settings/monitoring-settings.asciidoc index 5c4663a98cd6d..bc7d2ec00a241 100644 --- a/docs/reference/settings/monitoring-settings.asciidoc +++ b/docs/reference/settings/monitoring-settings.asciidoc @@ -29,11 +29,11 @@ For more information, see <>. ==== General Monitoring Settings `xpack.monitoring.enabled`:: -Set to `true` (default) to enable {es} {monitoring} for {es} on the node. +Set to `true` (default) to enable {es} {monitoring} for {es} on the node. + -- -NOTE: To enable data collection, you must also set `xpack.monitoring.collection.enabled` -to `true`. Its default value is `false`. +NOTE: To enable data collection, you must also set `xpack.monitoring.collection.enabled` +to `true`. Its default value is `false`. -- [float] @@ -51,7 +51,7 @@ this setting is `false` (default), {es} monitoring data is not collected and all monitoring data from other sources such as {kib}, Beats, and Logstash is ignored. -`xpack.monitoring.collection.interval` (<>):: +`xpack.monitoring.collection.interval` (<>):: Setting to `-1` to disable data collection is no longer supported beginning with 7.0.0. deprecated[6.3.0, Use `xpack.monitoring.collection.enabled` set to `false` instead.] @@ -200,9 +200,14 @@ xpack.monitoring.exporters: The username is required if a `auth.password` is supplied. +`secure_auth.password` ({ref}/secure-settings.html[Secure], {ref}/secure-settings.html#reloadable-secure-settings[reloadable]):: + +The password for the `auth.username`. May not be specified with `auth.password`. + `auth.password`:: -The password for the `auth.username`. +The password for the `auth.username`. May not be specified with `secure_auth.password`. This setting is deprecated[7.6.0, Use +`secure_auth.password` instead.]. `connection.timeout`:: 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 77ef1f365f11d..48d02b77cacf5 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 @@ -24,6 +24,7 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.ReloadablePlugin; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.script.ScriptService; @@ -68,7 +69,7 @@ import static java.util.Collections.singletonList; import static org.elasticsearch.common.settings.Setting.boolSetting; -public class Monitoring extends Plugin implements ActionPlugin { +public class Monitoring extends Plugin implements ActionPlugin, ReloadablePlugin { /** * The ability to automatically cleanup ".watcher_history*" indices while also cleaning up Monitoring indices. @@ -178,4 +179,9 @@ public List getSettingsFilter() { final String exportersKey = "xpack.monitoring.exporters."; return List.of(exportersKey + "*.auth.*", exportersKey + "*.ssl.*"); } + + @Override + public void reload(Settings settings) throws Exception { + HttpExporter.loadSettings(settings); + } } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java index 9f56d02253877..be45c7d19702a 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java @@ -22,6 +22,7 @@ import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; import org.elasticsearch.xpack.monitoring.collector.Collector; import org.elasticsearch.xpack.monitoring.exporter.Exporters; +import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter; import java.io.Closeable; import java.util.ArrayList; @@ -104,6 +105,8 @@ public class MonitoringService extends AbstractLifecycleComponent { .addSettingsUpdateConsumer(ELASTICSEARCH_COLLECTION_ENABLED, this::setElasticsearchCollectionEnabled); clusterService.getClusterSettings().addSettingsUpdateConsumer(ENABLED, this::setMonitoringActive); clusterService.getClusterSettings().addSettingsUpdateConsumer(INTERVAL, this::setInterval); + + HttpExporter.loadSettings(settings); } void setElasticsearchCollectionEnabled(final boolean enabled) { 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 a73cee446c280..7f72e0992338b 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 @@ -25,6 +25,8 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.settings.SecureSetting; +import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; @@ -47,6 +49,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -285,8 +288,17 @@ public Iterator> settings() { }, Property.Dynamic, Property.NodeScope, - Property.Filtered), + Property.Filtered, + Property.Deprecated), TYPE_DEPENDENCY); + /** + * Secure password for basic auth. + */ + public static final Setting.AffixSetting SECURE_AUTH_PASSWORD_SETTING = + Setting.affixKeySetting( + "xpack.monitoring.exporters.", + "secure_auth_password", + key -> SecureSetting.secureString(key, null)); /** * The SSL settings. * @@ -401,6 +413,7 @@ public Iterator> settings() { */ private final AtomicBoolean clusterAlertsAllowed = new AtomicBoolean(false); + private static final Map SECURE_AUTH_PASSWORDS = new HashMap<>(); private final ThreadContext threadContext; private final DateFormatter dateTimeFormatter; @@ -689,6 +702,14 @@ private static void configureTimeouts(final RestClientBuilder builder, final Con builder.setRequestConfigCallback(new TimeoutRequestConfigCallback(connectTimeout, socketTimeout)); } + public static void loadSettings(Settings settings) { + for (final String namespace : SECURE_AUTH_PASSWORD_SETTING.getNamespaces(settings)) { + final Setting s = SECURE_AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace); + final String password = s.get(settings).toString(); + SECURE_AUTH_PASSWORDS.put(namespace, password); + } + } + /** * Creates the optional {@link CredentialsProvider} with the username/password to use with all requests for user * authentication. @@ -700,7 +721,10 @@ private static void configureTimeouts(final RestClientBuilder builder, final Con @Nullable private static CredentialsProvider createCredentialsProvider(final Config config) { final String username = AUTH_USERNAME_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); - final String password = AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); + + final String password = SECURE_AUTH_PASSWORDS.containsKey(config.name()) + ? SECURE_AUTH_PASSWORDS.get(config.name()) + : AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(username, password)); @@ -868,6 +892,7 @@ public void doClose() { public static List> getSettings() { return Arrays.asList(HOST_SETTING, TEMPLATE_CREATE_LEGACY_VERSIONS_SETTING, AUTH_PASSWORD_SETTING, AUTH_USERNAME_SETTING, BULK_TIMEOUT_SETTING, CONNECTION_READ_TIMEOUT_SETTING, CONNECTION_TIMEOUT_SETTING, PIPELINE_CHECK_TIMEOUT_SETTING, - PROXY_BASE_PATH_SETTING, SNIFF_ENABLED_SETTING, TEMPLATE_CHECK_TIMEOUT_SETTING, SSL_SETTING, HEADERS_SETTING); + PROXY_BASE_PATH_SETTING, SNIFF_ENABLED_SETTING, TEMPLATE_CHECK_TIMEOUT_SETTING, SSL_SETTING, HEADERS_SETTING, + SECURE_AUTH_PASSWORD_SETTING); } } From 13a1a3e36945856d7a87ced602757df091e1a517 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Tue, 14 Jan 2020 11:00:43 -0600 Subject: [PATCH 02/18] fix tests --- .../xpack/monitoring/exporter/http/HttpExporterTests.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java index a50839ffbadc2..7ed63e8380dbb 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java @@ -233,6 +233,8 @@ public void testExporterWithPasswordButNoUsername() { IllegalArgumentException.class, () -> HttpExporter.AUTH_PASSWORD_SETTING.getConcreteSetting(prefix + ".auth.password").get(settings)); assertThat(e, hasToString(containsString(expected))); + assertWarnings("[xpack.monitoring.exporters._http.auth.password] setting was deprecated in Elasticsearch and will be removed " + + "in a future release! See the breaking changes documentation for the next major version."); } public void testExporterWithUsernameButNoPassword() { @@ -346,6 +348,8 @@ public void testCreateRestClient() throws IOException { // doesn't explode HttpExporter.createRestClient(config, sslService, listener).close(); + assertWarnings("[xpack.monitoring.exporters._http.auth.password] setting was deprecated in Elasticsearch and will be removed " + + "in a future release! See the breaking changes documentation for the next major version."); } public void testCreateSnifferDisabledByDefault() { From 322e690abe16c6c31e9c44fa75a8e20b8f58c0fb Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Tue, 14 Jan 2020 21:36:34 -0600 Subject: [PATCH 03/18] making setting reloadable --- .../xpack/monitoring/Monitoring.java | 11 +++-- .../xpack/monitoring/exporter/Exporters.java | 11 +++-- .../exporter/http/HttpExporter.java | 46 +++++++++++++++---- 3 files changed, 52 insertions(+), 16 deletions(-) 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 48d02b77cacf5..031a76a4b290a 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 @@ -81,6 +81,8 @@ public class Monitoring extends Plugin implements ActionPlugin, ReloadablePlugin protected final Settings settings; private final boolean enabled; + private Exporters exporters; + public Monitoring(Settings settings) { this.settings = settings; this.enabled = XPackSettings.MONITORING_ENABLED.get(settings); @@ -111,8 +113,7 @@ 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)); - final Exporters exporters = new Exporters(settings, exporterFactories, clusterService, getLicenseState(), - threadPool.getThreadContext()); + exporters = new Exporters(settings, exporterFactories, clusterService, getLicenseState(), threadPool.getThreadContext()); Set collectors = new HashSet<>(); collectors.add(new IndexStatsCollector(clusterService, getLicenseState(), client)); @@ -182,6 +183,10 @@ public List getSettingsFilter() { @Override public void reload(Settings settings) throws Exception { - HttpExporter.loadSettings(settings); + final List changedExporters = HttpExporter.loadSettings(settings); + for (String changedExporter : changedExporters) { + final Settings settingsForChangedExporter = settings.getByPrefix("xpack.monitoring.exporters." + changedExporter); + exporters.setExportersSetting(settingsForChangedExporter); + } } } 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 1b8f5dab9e356..138ecccecd199 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 @@ -35,6 +35,7 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; import static java.util.Collections.emptyMap; @@ -58,15 +59,17 @@ public Exporters(Settings settings, Map factories, this.clusterService = Objects.requireNonNull(clusterService); this.licenseState = Objects.requireNonNull(licenseState); - clusterService.getClusterSettings().addSettingsUpdateConsumer(this::setExportersSetting, getSettings()); + final List> dynamicSettings = + getSettings().stream().filter(Setting::isDynamic).collect(Collectors.toList()); + clusterService.getClusterSettings().addSettingsUpdateConsumer(this::setExportersSetting, dynamicSettings); HttpExporter.registerSettingValidators(clusterService); - // this ensures, that logging is happening by adding an empty consumer per affix setting - for (Setting.AffixSetting affixSetting : getSettings()) { + // 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) -> {}); } } - private void setExportersSetting(Settings exportersSetting) { + public void setExportersSetting(Settings exportersSetting) { if (this.lifecycle.started()) { Map updated = initExporters(exportersSetting); closeExporters(logger, this.exporters.getAndSet(updated)); 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 7f72e0992338b..23363159c304b 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 @@ -298,7 +298,8 @@ public Iterator> settings() { Setting.affixKeySetting( "xpack.monitoring.exporters.", "secure_auth_password", - key -> SecureSetting.secureString(key, null)); + key -> SecureSetting.secureString(key, null), + TYPE_DEPENDENCY); /** * The SSL settings. * @@ -702,12 +703,27 @@ private static void configureTimeouts(final RestClientBuilder builder, final Con builder.setRequestConfigCallback(new TimeoutRequestConfigCallback(connectTimeout, socketTimeout)); } - public static void loadSettings(Settings settings) { + + /** + * Caches secure settings for use when dynamically configuring HTTP exporters + * @param settings settings used for configuring HTTP exporter + * @return names of HTTP exporters whose secure settings changed, if any + */ + public static List loadSettings(Settings settings) { + final List changedExporters = new ArrayList<>(); for (final String namespace : SECURE_AUTH_PASSWORD_SETTING.getNamespaces(settings)) { final Setting s = SECURE_AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace); final String password = s.get(settings).toString(); - SECURE_AUTH_PASSWORDS.put(namespace, password); + String existingPassword; + synchronized (SECURE_AUTH_PASSWORDS) { + existingPassword = SECURE_AUTH_PASSWORDS.get(namespace); + SECURE_AUTH_PASSWORDS.put(namespace, password); + } + if (existingPassword.equals(password) == false) { + changedExporters.add(namespace); + } } + return changedExporters; } /** @@ -722,9 +738,12 @@ public static void loadSettings(Settings settings) { private static CredentialsProvider createCredentialsProvider(final Config config) { final String username = AUTH_USERNAME_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); - final String password = SECURE_AUTH_PASSWORDS.containsKey(config.name()) - ? SECURE_AUTH_PASSWORDS.get(config.name()) - : AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); + String password; + synchronized (SECURE_AUTH_PASSWORDS) { + password = SECURE_AUTH_PASSWORDS.containsKey(config.name()) + ? SECURE_AUTH_PASSWORDS.get(config.name()) + : AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); + } final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(username, password)); @@ -889,10 +908,19 @@ public void doClose() { } } - public static List> getSettings() { + public static List> getDynamicSettings() { return Arrays.asList(HOST_SETTING, TEMPLATE_CREATE_LEGACY_VERSIONS_SETTING, AUTH_PASSWORD_SETTING, AUTH_USERNAME_SETTING, BULK_TIMEOUT_SETTING, CONNECTION_READ_TIMEOUT_SETTING, CONNECTION_TIMEOUT_SETTING, PIPELINE_CHECK_TIMEOUT_SETTING, - PROXY_BASE_PATH_SETTING, SNIFF_ENABLED_SETTING, TEMPLATE_CHECK_TIMEOUT_SETTING, SSL_SETTING, HEADERS_SETTING, - SECURE_AUTH_PASSWORD_SETTING); + PROXY_BASE_PATH_SETTING, SNIFF_ENABLED_SETTING, TEMPLATE_CHECK_TIMEOUT_SETTING, SSL_SETTING, HEADERS_SETTING); + } + + public static List> getSecureSettings() { + return List.of(SECURE_AUTH_PASSWORD_SETTING); + } + + public static List> getSettings() { + List> allSettings = new ArrayList<>(getDynamicSettings()); + allSettings.addAll(getSecureSettings()); + return allSettings; } } From 25f9515bdebe62e8438c1d0cf43cc62c1825c5ae Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 15 Jan 2020 12:06:38 -0600 Subject: [PATCH 04/18] fix reloading and test --- .../org/elasticsearch/xpack/monitoring/Monitoring.java | 2 +- .../xpack/monitoring/exporter/http/HttpExporter.java | 2 +- .../monitoring/exporter/http/HttpExporterTests.java | 9 ++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) 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 031a76a4b290a..c0981d17f177c 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 @@ -185,7 +185,7 @@ public List getSettingsFilter() { public void reload(Settings settings) throws Exception { final List changedExporters = HttpExporter.loadSettings(settings); for (String changedExporter : changedExporters) { - final Settings settingsForChangedExporter = settings.getByPrefix("xpack.monitoring.exporters." + changedExporter); + final Settings settingsForChangedExporter = settings.filter(x -> x.startsWith("xpack.monitoring.exporters." + changedExporter)); exporters.setExportersSetting(settingsForChangedExporter); } } 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 23363159c304b..bc4f659040bc5 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 @@ -719,7 +719,7 @@ public static List loadSettings(Settings settings) { existingPassword = SECURE_AUTH_PASSWORDS.get(namespace); SECURE_AUTH_PASSWORDS.put(namespace, password); } - if (existingPassword.equals(password) == false) { + if (password.equals(existingPassword) == false) { changedExporters.add(namespace); } } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java index 7ed63e8380dbb..cf6d93b7fbf78 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java @@ -333,7 +333,8 @@ public void testCreateRestClient() throws IOException { .put("xpack.monitoring.exporters._http.host", "http://localhost:9200"); // use basic auth - if (randomBoolean()) { + final boolean useBasicAuth = randomBoolean(); + if (useBasicAuth) { builder.put("xpack.monitoring.exporters._http.auth.username", "_user") .put("xpack.monitoring.exporters._http.auth.password", "_pass"); } @@ -348,8 +349,10 @@ public void testCreateRestClient() throws IOException { // doesn't explode HttpExporter.createRestClient(config, sslService, listener).close(); - assertWarnings("[xpack.monitoring.exporters._http.auth.password] setting was deprecated in Elasticsearch and will be removed " + - "in a future release! See the breaking changes documentation for the next major version."); + if (useBasicAuth) { + assertWarnings("[xpack.monitoring.exporters._http.auth.password] setting was deprecated in Elasticsearch and will be " + + "removed in a future release! See the breaking changes documentation for the next major version."); + } } public void testCreateSnifferDisabledByDefault() { From a0e8064a7e949e504ee7aa2f956ed76fda8976ae Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 15 Jan 2020 17:22:00 -0600 Subject: [PATCH 05/18] handle deprecation warnings in integration tests --- .../smoketest/SmokeTestMonitoringWithSecurityIT.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/x-pack/qa/smoke-test-plugins-ssl/src/test/java/org/elasticsearch/smoketest/SmokeTestMonitoringWithSecurityIT.java b/x-pack/qa/smoke-test-plugins-ssl/src/test/java/org/elasticsearch/smoketest/SmokeTestMonitoringWithSecurityIT.java index 2da9ac9f7881f..f816c7042cbd0 100644 --- a/x-pack/qa/smoke-test-plugins-ssl/src/test/java/org/elasticsearch/smoketest/SmokeTestMonitoringWithSecurityIT.java +++ b/x-pack/qa/smoke-test-plugins-ssl/src/test/java/org/elasticsearch/smoketest/SmokeTestMonitoringWithSecurityIT.java @@ -18,6 +18,7 @@ import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; import org.elasticsearch.client.RestHighLevelClient; +import org.elasticsearch.client.WarningsHandler; import org.elasticsearch.client.indices.GetIndexRequest; import org.elasticsearch.client.indices.GetIndexTemplatesRequest; import org.elasticsearch.client.indices.GetIndexTemplatesResponse; @@ -160,7 +161,7 @@ public void enableExporter() throws Exception { .put("xpack.monitoring.exporters._http.ssl.certificate_authorities", "testnode.crt") .build(); ClusterUpdateSettingsResponse response = newHighLevelClient().cluster().putSettings( - new ClusterUpdateSettingsRequest().transientSettings(exporterSettings), RequestOptions.DEFAULT); + new ClusterUpdateSettingsRequest().transientSettings(exporterSettings), getRequestOptions()); assertTrue(response.isAcknowledged()); } @@ -177,10 +178,17 @@ public void disableExporter() throws IOException { .putNull("xpack.monitoring.exporters._http.ssl.certificate_authorities") .build(); ClusterUpdateSettingsResponse response = newHighLevelClient().cluster().putSettings( - new ClusterUpdateSettingsRequest().transientSettings(exporterSettings), RequestOptions.DEFAULT); + new ClusterUpdateSettingsRequest().transientSettings(exporterSettings), getRequestOptions()); assertTrue(response.isAcknowledged()); } + private RequestOptions getRequestOptions() { + String deprecationWarning = "[xpack.monitoring.exporters._http.auth.password] setting was deprecated in Elasticsearch and will " + + "be removed in a future release! See the breaking changes documentation for the next major version."; + return RequestOptions.DEFAULT.toBuilder().setWarningsHandler(warnings -> warnings.size() != 1 || + warnings.get(0).equals(deprecationWarning) == false).build(); + } + private boolean getMonitoringUsageExportersDefined() throws Exception { RestHighLevelClient client = newHighLevelClient(); final XPackUsageResponse usageResponse = client.xpack().usage(new XPackUsageRequest(), RequestOptions.DEFAULT); From 4840a0f12c916cdd7ff5a11d00894895cc4acc7e Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 15 Jan 2020 17:53:19 -0600 Subject: [PATCH 06/18] checkstyle --- .../smoketest/SmokeTestMonitoringWithSecurityIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/qa/smoke-test-plugins-ssl/src/test/java/org/elasticsearch/smoketest/SmokeTestMonitoringWithSecurityIT.java b/x-pack/qa/smoke-test-plugins-ssl/src/test/java/org/elasticsearch/smoketest/SmokeTestMonitoringWithSecurityIT.java index f816c7042cbd0..0b651272f0204 100644 --- a/x-pack/qa/smoke-test-plugins-ssl/src/test/java/org/elasticsearch/smoketest/SmokeTestMonitoringWithSecurityIT.java +++ b/x-pack/qa/smoke-test-plugins-ssl/src/test/java/org/elasticsearch/smoketest/SmokeTestMonitoringWithSecurityIT.java @@ -18,7 +18,6 @@ import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; import org.elasticsearch.client.RestHighLevelClient; -import org.elasticsearch.client.WarningsHandler; import org.elasticsearch.client.indices.GetIndexRequest; import org.elasticsearch.client.indices.GetIndexTemplatesRequest; import org.elasticsearch.client.indices.GetIndexTemplatesResponse; From 99811a2354763be25ce85ea5c7b8bd5bb933b654 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Fri, 17 Jan 2020 15:17:24 -0600 Subject: [PATCH 07/18] accommodate either password in validation --- .../xpack/monitoring/exporter/http/HttpExporter.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) 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 bc4f659040bc5..f3a96b889161c 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 @@ -209,17 +209,9 @@ public void validate(final String username, final Map, Object> settin final String namespace = HttpExporter.AUTH_USERNAME_SETTING.getNamespace( HttpExporter.AUTH_USERNAME_SETTING.getConcreteSetting(key)); - final String password = - (String) settings.get(AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace)); // password must be specified along with username for any auth if (Strings.isNullOrEmpty(username) == false) { - if (Strings.isNullOrEmpty(password)) { - throw new SettingsException( - "[" + AUTH_USERNAME_SETTING.getConcreteSettingForNamespace(namespace).getKey() + "] is set " + - "but [" + AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace).getKey() + "] is " + - "missing"); - } final String type = (String) settings.get(Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace)); if ("http".equals(type) == false) { @@ -235,8 +227,7 @@ public Iterator> settings() { HttpExporter.AUTH_USERNAME_SETTING.getConcreteSetting(key)); final List> settings = List.of( - Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace), - HttpExporter.AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace)); + Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace)); return settings.iterator(); } From ceb13b3af65899df728ed9cc4e7eff37ed818954 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 22 Jan 2020 13:58:01 -0600 Subject: [PATCH 08/18] switch to ConcurrentHashMap --- .../exporter/http/HttpExporter.java | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) 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 f3a96b889161c..c71ac97440f21 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 @@ -49,12 +49,12 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import java.util.function.Supplier; @@ -405,7 +405,7 @@ public Iterator> settings() { */ private final AtomicBoolean clusterAlertsAllowed = new AtomicBoolean(false); - private static final Map SECURE_AUTH_PASSWORDS = new HashMap<>(); + private static final ConcurrentHashMap SECURE_AUTH_PASSWORDS = new ConcurrentHashMap<>(); private final ThreadContext threadContext; private final DateFormatter dateTimeFormatter; @@ -703,14 +703,10 @@ private static void configureTimeouts(final RestClientBuilder builder, final Con public static List loadSettings(Settings settings) { final List changedExporters = new ArrayList<>(); for (final String namespace : SECURE_AUTH_PASSWORD_SETTING.getNamespaces(settings)) { - final Setting s = SECURE_AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace); - final String password = s.get(settings).toString(); - String existingPassword; - synchronized (SECURE_AUTH_PASSWORDS) { - existingPassword = SECURE_AUTH_PASSWORDS.get(namespace); - SECURE_AUTH_PASSWORDS.put(namespace, password); - } - if (password.equals(existingPassword) == false) { + final Setting s = SECURE_AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace); + final SecureString securePassword = s.get(settings); + final SecureString existingPassword = SECURE_AUTH_PASSWORDS.put(namespace, securePassword); + if (securePassword.equals(existingPassword) == false) { changedExporters.add(namespace); } } @@ -729,12 +725,10 @@ public static List loadSettings(Settings settings) { private static CredentialsProvider createCredentialsProvider(final Config config) { final String username = AUTH_USERNAME_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); - String password; - synchronized (SECURE_AUTH_PASSWORDS) { - password = SECURE_AUTH_PASSWORDS.containsKey(config.name()) - ? SECURE_AUTH_PASSWORDS.get(config.name()) - : AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); - } + final SecureString securePassword = SECURE_AUTH_PASSWORDS.get(config.name()); + final String password = securePassword == null + ? AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()) + : securePassword.toString(); final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(username, password)); From e158752235337d652dad740d85b5995175537917 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 22 Jan 2020 14:23:40 -0600 Subject: [PATCH 09/18] add comment about validation --- .../xpack/monitoring/exporter/http/HttpExporter.java | 5 +++++ 1 file changed, 5 insertions(+) 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 c71ac97440f21..a070678a63310 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 @@ -218,6 +218,11 @@ public void validate(final String username, final Map, Object> settin throw new SettingsException("username for [" + key + "] is set but type is [" + type + "]"); } } + + // it would be ideal to validate that just one of either AUTH_PASSWORD_SETTING or + // SECURE_AUTH_PASSWORD_SETTING were present here, but that is not currently possible with the settings + // validation framework. + // https://github.com/elastic/elasticsearch/issues/51332 } @Override From e6be3460a30861e8fd177472308a265d1b95ae30 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Thu, 23 Jan 2020 06:33:47 -0600 Subject: [PATCH 10/18] remove obsolete test --- .../exporter/http/HttpExporterTests.java | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java index cf6d93b7fbf78..283e6acb6d9e4 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java @@ -237,28 +237,6 @@ public void testExporterWithPasswordButNoUsername() { "in a future release! See the breaking changes documentation for the next major version."); } - public void testExporterWithUsernameButNoPassword() { - final String expected = - "[xpack.monitoring.exporters._http.auth.username] is set but [xpack.monitoring.exporters._http.auth.password] is missing"; - final String prefix = "xpack.monitoring.exporters._http"; - final Settings settings = Settings.builder() - .put(prefix + ".type", HttpExporter.TYPE) - .put(prefix + ".host", "localhost:9200") - .put(prefix + ".auth.username", "_user") - .build(); - - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, - () -> HttpExporter.AUTH_USERNAME_SETTING.getConcreteSetting(prefix + ".auth.username").get(settings)); - assertThat( - e, - hasToString( - containsString("Failed to parse value for setting [xpack.monitoring.exporters._http.auth.username]"))); - - assertThat(e.getCause(), instanceOf(SettingsException.class)); - assertThat(e.getCause(), hasToString(containsString(expected))); - } - public void testExporterWithUnknownBlacklistedClusterAlerts() { final SSLIOSessionStrategy sslStrategy = mock(SSLIOSessionStrategy.class); when(sslService.sslIOSessionStrategy(any(Settings.class))).thenReturn(sslStrategy); From b8f8cbe75cdb3b3527b4fb85cfadb6642d0f7f71 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Fri, 24 Jan 2020 17:48:10 -0600 Subject: [PATCH 11/18] integration test that reloads secure password --- .../test/http/MockWebServer.java | 7 +++ .../monitoring/LocalStateMonitoring.java | 11 +++- .../exporter/http/HttpExporterIT.java | 60 ++++++++++++++++++- 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/test/http/MockWebServer.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/test/http/MockWebServer.java index 9de12ff7d75c5..fc3e37c87db2d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/test/http/MockWebServer.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/test/http/MockWebServer.java @@ -244,6 +244,13 @@ public MockRequest takeRequest() { return requests.poll(); } + /** + * Removes all requests from the queue. + */ + public void clearRequests() { + requests.clear(); + } + /** * A utility method to peek into the requests and find out if #MockWebServer.takeRequests will not throw an out of bound exception * @return true if more requests are available, false otherwise diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/LocalStateMonitoring.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/LocalStateMonitoring.java index 16122b2aa9e20..7791ce6de9afd 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/LocalStateMonitoring.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/LocalStateMonitoring.java @@ -44,11 +44,13 @@ protected List usageActions() { } } + final Monitoring monitoring; + public LocalStateMonitoring(final Settings settings, final Path configPath) throws Exception { super(settings, configPath); LocalStateMonitoring thisVar = this; - plugins.add(new Monitoring(settings) { + monitoring = new Monitoring(settings) { @Override protected SSLService getSslService() { return thisVar.getSslService(); @@ -63,7 +65,8 @@ protected LicenseService getLicenseService() { protected XPackLicenseState getLicenseState() { return thisVar.getLicenseState(); } - }); + }; + plugins.add(monitoring); plugins.add(new Watcher(settings) { @Override protected SSLService getSslService() { @@ -78,6 +81,10 @@ protected XPackLicenseState getLicenseState() { plugins.add(new IndexLifecycle(settings)); } + public Monitoring getMonitoring() { + return monitoring; + } + @Override protected Class> getUsageAction() { return MonitoringTransportXPackUsageAction.class; diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java index c27162d003ccf..e61c573263e0c 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.monitoring.exporter.http; +import com.unboundid.util.Base64; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DocWriteRequest; @@ -19,6 +20,7 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -33,6 +35,7 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.plugins.PluginsService; import org.elasticsearch.rest.RestUtils; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase.Scope; @@ -42,6 +45,8 @@ import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils; import org.elasticsearch.xpack.core.ssl.SSLService; +import org.elasticsearch.xpack.monitoring.LocalStateMonitoring; +import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.MonitoringTestUtils; import org.elasticsearch.xpack.monitoring.collector.indices.IndexRecoveryMonitoringDoc; import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil; @@ -93,9 +98,27 @@ public class HttpExporterIT extends MonitoringIntegTestCase { private final boolean currentLicenseAllowsWatcher = true; private final boolean watcherAlreadyExists = randomBoolean(); private final Environment environment = TestEnvironment.newEnvironment(Settings.builder().put("path.home", createTempDir()).build()); + private final String userName = "elasticuser"; + private final String pass = "elasticpass"; + private final String pass2 = "anotherpassword"; + private final String authHeaderValue = Base64.encode(userName + ":" + pass); + private final String authHeaderValue2 = Base64.encode(userName + ":" + pass2); private MockWebServer webServer; + private MockSecureSettings mockSecureSettings = new MockSecureSettings(); + @Override + protected Settings nodeSettings(int nodeOrdinal) { + + Settings.Builder builder = Settings.builder() + .put(super.nodeSettings(nodeOrdinal)) + .put(MonitoringService.INTERVAL.getKey(), MonitoringService.MIN_INTERVAL) + // we do this by default in core, but for monitoring this isn't needed and only adds noise. + .put("index.lifecycle.history_index_enabled", false) + .put("index.store.mock.check_index_on_close", false); + return builder.build(); + } + @Before public void startWebServer() throws IOException { webServer = createMockWebServer(); @@ -113,6 +136,11 @@ protected boolean ignoreExternalCluster() { return true; } + private Settings.Builder secureSettings(String password) { + mockSecureSettings.setString("xpack.monitoring.exporters._http.secure_auth_password", password); + return baseSettings().setSecureSettings(mockSecureSettings); + } + private Settings.Builder baseSettings() { return Settings.builder() .put("xpack.monitoring.exporters._http.enabled", false) @@ -121,7 +149,8 @@ private Settings.Builder baseSettings() { .put("xpack.monitoring.exporters._http.headers.ignored", "value") // ensure that headers can be used by settings .put("xpack.monitoring.exporters._http.host", getFormattedAddress(webServer)) .putList("xpack.monitoring.exporters._http.cluster_alerts.management.blacklist", clusterAlertBlacklist) - .put("xpack.monitoring.exporters._http.index.template.create_legacy_templates", includeOldTemplates); + .put("xpack.monitoring.exporters._http.index.template.create_legacy_templates", includeOldTemplates) + .put("xpack.monitoring.exporters._http.auth.username", userName); } public void testExport() throws Exception { @@ -142,6 +171,35 @@ public void testExport() throws Exception { assertBulk(webServer, nbDocs); } + public void testSecureSetting() throws Exception { + Settings settings = secureSettings(pass).build(); + PluginsService pluginsService = internalCluster().getInstances(PluginsService.class).iterator().next(); + LocalStateMonitoring localStateMonitoring = pluginsService.filterPlugins(LocalStateMonitoring.class).iterator().next(); + localStateMonitoring.getMonitoring().reload(settings); + + enqueueGetClusterVersionResponse(Version.CURRENT); + enqueueSetupResponses(webServer, + templatesExistsAlready, includeOldTemplates, pipelineExistsAlready, + remoteClusterAllowsWatcher, currentLicenseAllowsWatcher, watcherAlreadyExists); + enqueueResponse(200, "{\"errors\": false, \"msg\": \"successful bulk request\"}"); + + final int nbDocs = randomIntBetween(1, 25); + export(settings, newRandomMonitoringDocs(nbDocs)); + assertEquals(webServer.takeRequest().getHeader("Authorization").replace("Basic", "").replace(" ", ""), authHeaderValue); + webServer.clearRequests(); + + settings = secureSettings(pass2).build(); + localStateMonitoring.getMonitoring().reload(settings); + enqueueGetClusterVersionResponse(Version.CURRENT); + enqueueSetupResponses(webServer, + templatesExistsAlready, includeOldTemplates, pipelineExistsAlready, + remoteClusterAllowsWatcher, currentLicenseAllowsWatcher, watcherAlreadyExists); + enqueueResponse(200, "{\"errors\": false, \"msg\": \"successful bulk request\"}"); + + export(settings, newRandomMonitoringDocs(nbDocs)); + assertEquals(webServer.takeRequest().getHeader("Authorization").replace("Basic", "").replace(" ", ""), authHeaderValue2); + } + public void testExportWithHeaders() throws Exception { final String headerValue = randomAlphaOfLengthBetween(3, 9); final String[] array = generateRandomStringArray(2, 4, false, false); From c6258675e4a5d3f2906c6a2d71a20219509130f3 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Fri, 24 Jan 2020 18:12:05 -0600 Subject: [PATCH 12/18] rename setting and update docs --- docs/reference/settings/monitoring-settings.asciidoc | 10 +++++----- .../xpack/monitoring/exporter/http/HttpExporter.java | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/reference/settings/monitoring-settings.asciidoc b/docs/reference/settings/monitoring-settings.asciidoc index bc7d2ec00a241..9c890596e7950 100644 --- a/docs/reference/settings/monitoring-settings.asciidoc +++ b/docs/reference/settings/monitoring-settings.asciidoc @@ -198,16 +198,16 @@ xpack.monitoring.exporters: `auth.username`:: -The username is required if a `auth.password` is supplied. +The username is required if `auth.secure_password` or `auth.password` is supplied. -`secure_auth.password` ({ref}/secure-settings.html[Secure], {ref}/secure-settings.html#reloadable-secure-settings[reloadable]):: +`auth.secure_password` ({ref}/secure-settings.html[Secure], {ref}/secure-settings.html#reloadable-secure-settings[reloadable]):: -The password for the `auth.username`. May not be specified with `auth.password`. +The password for the `auth.username`. Takes precedence over `auth.password` if it is also specified. `auth.password`:: -The password for the `auth.username`. May not be specified with `secure_auth.password`. This setting is deprecated[7.6.0, Use -`secure_auth.password` instead.]. +The password for the `auth.username`. This setting is ignored if `auth.secure_password` is also specified. This setting is +deprecated[7.7.0, Use `auth.secure_password` instead.]. `connection.timeout`:: 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 a070678a63310..8e84f1bfcfd10 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 @@ -220,7 +220,7 @@ public void validate(final String username, final Map, Object> settin } // it would be ideal to validate that just one of either AUTH_PASSWORD_SETTING or - // SECURE_AUTH_PASSWORD_SETTING were present here, but that is not currently possible with the settings + // AUTH_SECURE_PASSWORD_SETTING were present here, but that is not currently possible with the settings // validation framework. // https://github.com/elastic/elasticsearch/issues/51332 } @@ -290,10 +290,10 @@ public Iterator> settings() { /** * Secure password for basic auth. */ - public static final Setting.AffixSetting SECURE_AUTH_PASSWORD_SETTING = + public static final Setting.AffixSetting AUTH_SECURE_PASSWORD_SETTING = Setting.affixKeySetting( "xpack.monitoring.exporters.", - "secure_auth_password", + "auth.secure_password", key -> SecureSetting.secureString(key, null), TYPE_DEPENDENCY); /** @@ -707,8 +707,8 @@ private static void configureTimeouts(final RestClientBuilder builder, final Con */ public static List loadSettings(Settings settings) { final List changedExporters = new ArrayList<>(); - for (final String namespace : SECURE_AUTH_PASSWORD_SETTING.getNamespaces(settings)) { - final Setting s = SECURE_AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace); + for (final String namespace : AUTH_SECURE_PASSWORD_SETTING.getNamespaces(settings)) { + final Setting s = AUTH_SECURE_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace); final SecureString securePassword = s.get(settings); final SecureString existingPassword = SECURE_AUTH_PASSWORDS.put(namespace, securePassword); if (securePassword.equals(existingPassword) == false) { @@ -905,7 +905,7 @@ public static List> getDynamicSettings() { } public static List> getSecureSettings() { - return List.of(SECURE_AUTH_PASSWORD_SETTING); + return List.of(AUTH_SECURE_PASSWORD_SETTING); } public static List> getSettings() { From 9e3a59164521c88474996348e227cfa89d74a55e Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Mon, 27 Jan 2020 11:04:35 -0600 Subject: [PATCH 13/18] add new tests --- .../exporter/http/HttpExporterIT.java | 17 ++++++++++------- .../exporter/http/HttpExporterTests.java | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java index e61c573263e0c..669f97e3c87ad 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java @@ -99,10 +99,6 @@ public class HttpExporterIT extends MonitoringIntegTestCase { private final boolean watcherAlreadyExists = randomBoolean(); private final Environment environment = TestEnvironment.newEnvironment(Settings.builder().put("path.home", createTempDir()).build()); private final String userName = "elasticuser"; - private final String pass = "elasticpass"; - private final String pass2 = "anotherpassword"; - private final String authHeaderValue = Base64.encode(userName + ":" + pass); - private final String authHeaderValue2 = Base64.encode(userName + ":" + pass2); private MockWebServer webServer; @@ -137,7 +133,7 @@ protected boolean ignoreExternalCluster() { } private Settings.Builder secureSettings(String password) { - mockSecureSettings.setString("xpack.monitoring.exporters._http.secure_auth_password", password); + mockSecureSettings.setString("xpack.monitoring.exporters._http.auth.secure_password", password); return baseSettings().setSecureSettings(mockSecureSettings); } @@ -172,7 +168,14 @@ public void testExport() throws Exception { } public void testSecureSetting() throws Exception { - Settings settings = secureSettings(pass).build(); + final String securePassword1 = "elasticpass"; + final String securePassword2 = "anotherpassword"; + final String authHeaderValue = Base64.encode(userName + ":" + securePassword1); + final String authHeaderValue2 = Base64.encode(userName + ":" + securePassword2); + + Settings settings = secureSettings(securePassword1) + .put("xpack.monitoring.exporters._http.auth.password", "insecurePassword") // verify this password is not used + .build(); PluginsService pluginsService = internalCluster().getInstances(PluginsService.class).iterator().next(); LocalStateMonitoring localStateMonitoring = pluginsService.filterPlugins(LocalStateMonitoring.class).iterator().next(); localStateMonitoring.getMonitoring().reload(settings); @@ -188,7 +191,7 @@ public void testSecureSetting() throws Exception { assertEquals(webServer.takeRequest().getHeader("Authorization").replace("Basic", "").replace(" ", ""), authHeaderValue); webServer.clearRequests(); - settings = secureSettings(pass2).build(); + settings = secureSettings(securePassword2).build(); localStateMonitoring.getMonitoring().reload(settings); enqueueGetClusterVersionResponse(Version.CURRENT); enqueueSetupResponses(webServer, diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java index 283e6acb6d9e4..0ca836ff862a5 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java @@ -18,6 +18,7 @@ import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.common.unit.TimeValue; @@ -140,6 +141,24 @@ public void testHostListIsRejectedIfTypeIsNotHttp() { assertThat(e, hasToString(containsString("[" + prefix + ".host] is set but type is [local]"))); } + public void testSecurePasswordIsRejectedIfTypeIsNotHttp() { + final String prefix = "xpack.monitoring.exporters.example"; + final Settings.Builder builder = Settings.builder().put(prefix + ".type", "local"); + + final String settingName = ".auth.secure_password"; + final String settingValue = "securePassword"; + MockSecureSettings mockSecureSettings = new MockSecureSettings(); + mockSecureSettings.setString(prefix + settingName, settingValue); + + builder.setSecureSettings(mockSecureSettings); + + final Settings settings = builder.build(); + final ClusterSettings clusterSettings = + new ClusterSettings(settings, Set.of(HttpExporter.AUTH_SECURE_PASSWORD_SETTING, Exporter.TYPE_SETTING)); + final SettingsException e = expectThrows(SettingsException.class, () -> clusterSettings.validate(settings, true)); + assertThat(e, hasToString(containsString("[" + prefix + settingName + "] is set but type is [local]"))); + } + public void testInvalidHost() { final String prefix = "xpack.monitoring.exporters.example"; final String host = "https://example.com:443/"; From a7974e774aa4a974eaeadeadda7d65b82e5f548e Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Tue, 28 Jan 2020 16:30:34 -0600 Subject: [PATCH 14/18] log warning if both secure password and deprecated password are supplied --- .../monitoring/exporter/http/HttpExporter.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) 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 8e84f1bfcfd10..f401f435dff9d 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 @@ -730,10 +730,18 @@ public static List loadSettings(Settings settings) { private static CredentialsProvider createCredentialsProvider(final Config config) { final String username = AUTH_USERNAME_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); + final String deprecatedPassword = AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()); final SecureString securePassword = SECURE_AUTH_PASSWORDS.get(config.name()); - final String password = securePassword == null - ? AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings()) - : securePassword.toString(); + final String password; + if (securePassword != null) { + password = securePassword.toString(); + if (Strings.isNullOrEmpty(deprecatedPassword) == false) { + logger.warn("exporter [{}] specified both auth.secure_password and auth.password. using auth.secure_password and " + + "ignoring auth.password", config.name()); + } + } else { + password = deprecatedPassword; + } final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(username, password)); From 1252a8a872079f2bc0c13976cab5cb6fe0f2aabf Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 29 Jan 2020 11:20:48 -0600 Subject: [PATCH 15/18] Update docs/reference/settings/monitoring-settings.asciidoc Co-Authored-By: Lisa Cawley --- docs/reference/settings/monitoring-settings.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/settings/monitoring-settings.asciidoc b/docs/reference/settings/monitoring-settings.asciidoc index 9c890596e7950..19a081678d757 100644 --- a/docs/reference/settings/monitoring-settings.asciidoc +++ b/docs/reference/settings/monitoring-settings.asciidoc @@ -200,7 +200,7 @@ xpack.monitoring.exporters: The username is required if `auth.secure_password` or `auth.password` is supplied. -`auth.secure_password` ({ref}/secure-settings.html[Secure], {ref}/secure-settings.html#reloadable-secure-settings[reloadable]):: +`auth.secure_password` (<>, <>):: The password for the `auth.username`. Takes precedence over `auth.password` if it is also specified. From 9019ab5fdb289a94cf3df23652881b042ce27019 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 29 Jan 2020 11:23:41 -0600 Subject: [PATCH 16/18] Update docs/reference/settings/monitoring-settings.asciidoc Co-Authored-By: Lisa Cawley --- docs/reference/settings/monitoring-settings.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/settings/monitoring-settings.asciidoc b/docs/reference/settings/monitoring-settings.asciidoc index 19a081678d757..4e6522b8dd70b 100644 --- a/docs/reference/settings/monitoring-settings.asciidoc +++ b/docs/reference/settings/monitoring-settings.asciidoc @@ -206,7 +206,7 @@ The password for the `auth.username`. Takes precedence over `auth.password` if i `auth.password`:: -The password for the `auth.username`. This setting is ignored if `auth.secure_password` is also specified. This setting is +The password for the `auth.username`. If `auth.secure_password` is also specified, this setting is ignored. deprecated[7.7.0, Use `auth.secure_password` instead.]. `connection.timeout`:: From 5f97171d21508dabef1b20fa58b2d2f3482ad1a7 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 29 Jan 2020 11:29:46 -0600 Subject: [PATCH 17/18] add link from reloadable settings page to monitoring settings page --- docs/reference/setup/secure-settings.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/reference/setup/secure-settings.asciidoc b/docs/reference/setup/secure-settings.asciidoc index e565877f22f5e..47a9330fb969d 100644 --- a/docs/reference/setup/secure-settings.asciidoc +++ b/docs/reference/setup/secure-settings.asciidoc @@ -56,3 +56,4 @@ There are reloadable secure settings for: * {plugins}/discovery-ec2-usage.html#_configuring_ec2_discovery[The EC2 discovery plugin] * {plugins}/repository-gcs-client.html[The GCS repository plugin] * {plugins}/repository-s3-client.html[The S3 repository plugin] +* <>[Monitoring settings] From 3ccd9e94e2454d86fc55380bbd53b7cd2c191c1c Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 29 Jan 2020 11:47:46 -0600 Subject: [PATCH 18/18] update docs --- docs/reference/settings/monitoring-settings.asciidoc | 3 ++- docs/reference/setup/secure-settings.asciidoc | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/reference/settings/monitoring-settings.asciidoc b/docs/reference/settings/monitoring-settings.asciidoc index 4e6522b8dd70b..565780cc5b7a7 100644 --- a/docs/reference/settings/monitoring-settings.asciidoc +++ b/docs/reference/settings/monitoring-settings.asciidoc @@ -207,7 +207,8 @@ The password for the `auth.username`. Takes precedence over `auth.password` if i `auth.password`:: The password for the `auth.username`. If `auth.secure_password` is also specified, this setting is ignored. -deprecated[7.7.0, Use `auth.secure_password` instead.]. + +deprecated[7.7.0, Use `auth.secure_password` instead.] `connection.timeout`:: diff --git a/docs/reference/setup/secure-settings.asciidoc b/docs/reference/setup/secure-settings.asciidoc index 47a9330fb969d..fafbfd19393bc 100644 --- a/docs/reference/setup/secure-settings.asciidoc +++ b/docs/reference/setup/secure-settings.asciidoc @@ -56,4 +56,4 @@ There are reloadable secure settings for: * {plugins}/discovery-ec2-usage.html#_configuring_ec2_discovery[The EC2 discovery plugin] * {plugins}/repository-gcs-client.html[The GCS repository plugin] * {plugins}/repository-s3-client.html[The S3 repository plugin] -* <>[Monitoring settings] +* <>