Skip to content

Secure password for monitoring HTTP exporter #50919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Jan 30, 2020
Merged

Secure password for monitoring HTTP exporter #50919

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
11f3d34
Secure monitoring password
danhermann Dec 18, 2019
13a1a3e
fix tests
danhermann Jan 14, 2020
d8bd711
Merge branch 'master' into 50197_secure_password
elasticmachine Jan 14, 2020
79e0267
Merge branch 'master' into 50197_secure_password
elasticmachine Jan 14, 2020
322e690
making setting reloadable
danhermann Jan 15, 2020
ce03cd1
Merge branch 'master' into 50197_secure_password
elasticmachine Jan 15, 2020
25f9515
fix reloading and test
danhermann Jan 15, 2020
da29f4e
Merge branch 'master' into 50197_secure_password
elasticmachine Jan 15, 2020
a0e8064
handle deprecation warnings in integration tests
danhermann Jan 15, 2020
4840a0f
checkstyle
danhermann Jan 15, 2020
99811a2
accommodate either password in validation
danhermann Jan 17, 2020
ceb13b3
switch to ConcurrentHashMap
danhermann Jan 22, 2020
e158752
add comment about validation
danhermann Jan 22, 2020
15624ab
Merge branch 'master' into 50197_secure_password
elasticmachine Jan 22, 2020
e6be346
remove obsolete test
danhermann Jan 23, 2020
b8f8cbe
integration test that reloads secure password
danhermann Jan 24, 2020
8f9c13c
Merge branch 'master' into 50197_secure_password
elasticmachine Jan 24, 2020
c625867
rename setting and update docs
danhermann Jan 25, 2020
9e3a591
add new tests
danhermann Jan 27, 2020
a7974e7
log warning if both secure password and deprecated password are supplied
danhermann Jan 28, 2020
1252a8a
Update docs/reference/settings/monitoring-settings.asciidoc
danhermann Jan 29, 2020
9019ab5
Update docs/reference/settings/monitoring-settings.asciidoc
danhermann Jan 29, 2020
5f97171
add link from reloadable settings page to monitoring settings page
danhermann Jan 29, 2020
3ccd9e9
update docs
danhermann Jan 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions docs/reference/settings/monitoring-settings.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ For more information, see <<monitor-elasticsearch-cluster>>.
==== 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]
Expand All @@ -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` (<<cluster-update-settings,Dynamic>>)::
`xpack.monitoring.collection.interval` (<<cluster-update-settings,Dynamic>>)::

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.]
Expand Down Expand Up @@ -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`::

Expand Down
8 changes: 7 additions & 1 deletion x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -178,4 +179,9 @@ public List<String> 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);
}
}
3 changes: 3 additions & 0 deletions ...plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
31 changes: 28 additions & 3 deletions ...nitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -285,8 +288,17 @@ public Iterator<Setting<?>> settings() {
},
Property.Dynamic,
Property.NodeScope,
Property.Filtered),
Property.Filtered,
Property.Deprecated),
TYPE_DEPENDENCY);
/**
* Secure password for basic auth.
*/
public static final Setting.AffixSetting<SecureString> SECURE_AUTH_PASSWORD_SETTING =
Setting.affixKeySetting(
"xpack.monitoring.exporters.",
"secure_auth_password",
key -> SecureSetting.secureString(key, null));
/**
* The SSL settings.
*
Expand Down Expand Up @@ -401,6 +413,7 @@ public Iterator<Setting<?>> settings() {
*/
private final AtomicBoolean clusterAlertsAllowed = new AtomicBoolean(false);

private static final Map<String, String> SECURE_AUTH_PASSWORDS = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passwords as String should be avoided where possible due to help avoid leaking to heap since gc happens when ever it feel like it. I think you can use SecureString instead and then close it out to ensure it zero'd out when you are done.

Also since it looks like you are using this for equality, can you just hash it and store the hash in the map ?

Copy link
Contributor Author

@danhermann danhermann Jan 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could change it to a Map<String, SecureString> but I couldn't close any of the SecureString instances because the HTTP exporter has to be recreated (so I need the full value, not just an equality check) any time any of its settings, whether secure or not, change. Unfortunately, this is unavoidable for settings that are both secure and reloadable since the keystore itself is not accessible outside of node startup and reload events. The security folks directed me to

elasticsearch/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java

Lines 167 to 225 in c3b5f71

* Extracts the {@link SecureSettings}` out of the passed in {@link Settings} object. The {@code Setting} argument has to have the
* {@code SecureSettings} open/available. Normally {@code SecureSettings} are available only under specific callstacks (eg. during node
* initialization or during a `reload` call). The returned copy can be reused freely as it will never be closed (this is a bit of
* cheating, but it is necessary in this specific circumstance). Only works for secure settings of type string (not file).
*
* @param source
* A {@code Settings} object with its {@code SecureSettings} open/available.
* @param securePluginSettings
* The list of settings to copy.
* @return A copy of the {@code SecureSettings} of the passed in {@code Settings} argument.
*/
private static SecureSettings extractSecureSettings(Settings source, List<Setting<?>> securePluginSettings)
throws GeneralSecurityException {
// get the secure settings out
final SecureSettings sourceSecureSettings = Settings.builder().put(source, true).getSecureSettings();
// filter and cache them...
final Map<String, Tuple<SecureString, byte[]>> cache = new HashMap<>();
if (sourceSecureSettings != null && securePluginSettings != null) {
for (final String settingKey : sourceSecureSettings.getSettingNames()) {
for (final Setting<?> secureSetting : securePluginSettings) {
if (secureSetting.match(settingKey)) {
cache.put(settingKey,
new Tuple<>(sourceSecureSettings.getString(settingKey), sourceSecureSettings.getSHA256Digest(settingKey)));
}
}
}
}
return new SecureSettings() {
@Override
public boolean isLoaded() {
return true;
}
@Override
public SecureString getString(String setting) {
return cache.get(setting).v1();
}
@Override
public Set<String> getSettingNames() {
return cache.keySet();
}
@Override
public InputStream getFile(String setting) {
throw new IllegalStateException("A NotificationService setting cannot be File.");
}
@Override
public byte[] getSHA256Digest(String setting) {
return cache.get(setting).v2();
}
@Override
public void close() throws IOException {
}
};
}

where they did essentially the same thing for the secure settings in watcher notifications.

private final ThreadContext threadContext;
private final DateFormatter dateTimeFormatter;

Expand Down Expand Up @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto about SecureString

SECURE_AUTH_PASSWORDS.put(namespace, password);
}
}

/**
* Creates the optional {@link CredentialsProvider} with the username/password to use with <em>all</em> requests for user
* authentication.
Expand All @@ -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));
Expand Down Expand Up @@ -868,6 +892,7 @@ public void doClose() {
public static List<Setting.AffixSetting<?>> 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);
}
}