-
Notifications
You must be signed in to change notification settings - Fork 25.6k
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.
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
Changes from 11 commits
11f3d34
13a1a3e
d8bd711
79e0267
322e690
ce03cd1
25f9515
da29f4e
a0e8064
4840a0f
99811a2
ceb13b3
e158752
15624ab
e6be346
b8f8cbe
8f9c13c
c625867
9e3a591
a7974e7
1252a8a
9019ab5
5f97171
3ccd9e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -206,17 +209,9 @@ public void validate(final String username, final Map<Setting<?>, 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) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -232,8 +227,7 @@ public Iterator<Setting<?>> settings() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HttpExporter.AUTH_USERNAME_SETTING.getConcreteSetting(key)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final List<Setting<?>> settings = List.of( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
danhermann marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HttpExporter.AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return settings.iterator(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -285,8 +279,18 @@ 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), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TYPE_DEPENDENCY); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * The SSL settings. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -401,6 +405,7 @@ public Iterator<Setting<?>> settings() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final AtomicBoolean clusterAlertsAllowed = new AtomicBoolean(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static final Map<String, String> SECURE_AUTH_PASSWORDS = new HashMap<>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 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.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto about SecureString
danhermann marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
danhermann marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
danhermann marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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() { | ||
|
|
@@ -331,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"); | ||
| } | ||
|
|
@@ -346,6 +349,10 @@ public void testCreateRestClient() throws IOException { | |
|
|
||
| // doesn't explode | ||
| HttpExporter.createRestClient(config, sslService, listener).close(); | ||
| 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."); | ||
| } | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add some unit tests for changing the secure setting and ensure it is changed (or no-op) as well as trying to set this for a non-http type setting ? See org.elasticsearch.xpack.watcher.notification.NotificationService for some examples. Ideally, we would also have Integration tests, but I didn't see any framework already setup to read from the keystore for a Integration tests (and an almost complete lack of Rest tests) :( |
||
| public void testCreateSnifferDisabledByDefault() { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.