From bf7689071b4e580cc3056ac231c0329fabd7f01d Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Fri, 13 Jul 2018 11:13:10 -0500 Subject: [PATCH] Add secure setting for watcher email password (#31620) Other watcher actions already account for secure settings in their sensitive settings, whereas the email sending action did not. This adds the ability to optionally set a secure_password for email accounts. --- .../watcher/notification/email/Account.java | 40 ++++++++++++++++--- .../notification/email/EmailService.java | 10 ++++- .../notification/email/AccountTests.java | 30 +++++++++++++- 3 files changed, 72 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/Account.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/Account.java index 8ba8d030524..02c0e1167dd 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/Account.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/Account.java @@ -7,6 +7,9 @@ package org.elasticsearch.xpack.watcher.notification.email; import org.apache.logging.log4j.Logger; import org.elasticsearch.SpecialPermission; +import org.elasticsearch.common.settings.SecureSetting; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.common.unit.TimeValue; @@ -24,10 +27,13 @@ import java.security.PrivilegedAction; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.util.Properties; +import java.util.Set; public class Account { static final String SMTP_PROTOCOL = "smtp"; + private static final String SMTP_PASSWORD = "password"; + private static final Setting SECURE_PASSWORD_SETTING = SecureSetting.secureString("secure_" + SMTP_PASSWORD, null); static { SecurityManager sm = System.getSecurityManager(); @@ -101,7 +107,7 @@ public class Account { if (auth != null && auth.password() != null) { password = new String(auth.password().text(cryptoService)); } else if (config.smtp.password != null) { - password = new String(config.smtp.password); + password = new String(config.smtp.password.getChars()); } if (profile == null) { @@ -199,18 +205,40 @@ public class Account { final String host; final int port; final String user; - final char[] password; + final SecureString password; final Properties properties; Smtp(Settings settings) { host = settings.get("host", settings.get("localaddress", settings.get("local_address"))); + port = settings.getAsInt("port", settings.getAsInt("localport", settings.getAsInt("local_port", 25))); user = settings.get("user", settings.get("from", null)); - String passStr = settings.get("password", null); - password = passStr != null ? passStr.toCharArray() : null; + password = getSecureSetting(SMTP_PASSWORD, settings, SECURE_PASSWORD_SETTING); + //password = passStr != null ? passStr.toCharArray() : null; properties = loadSmtpProperties(settings); } + /** + * Finds a setting, and then a secure setting if the setting is null, or returns null if one does not exist. This differs + * from other getSetting calls in that it allows for null whereas the other methods throw an exception. + * + * Note: if your setting was not previously secure, than the string reference that is in the setting object is still + * insecure. This is only constructing a new SecureString with the char[] of the insecure setting. + */ + private static SecureString getSecureSetting(String settingName, Settings settings, Setting secureSetting) { + String value = settings.get(settingName); + if (value == null) { + SecureString secureString = secureSetting.get(settings); + if (secureString != null && secureString.length() > 0) { + return secureString; + } else { + return null; + } + } else { + return new SecureString(value.toCharArray()); + } + } + /** * loads the standard Java Mail properties as settings from the given account settings. * The standard settings are not that readable, therefore we enabled the user to configure @@ -231,7 +259,9 @@ public class Account { settings = builder.build(); Properties props = new Properties(); - for (String key : settings.keySet()) { + // Secure strings can not be retreived out of a settings object and should be handled differently + Set insecureSettings = settings.filter(s -> s.startsWith("secure_") == false).keySet(); + for (String key : insecureSettings) { props.setProperty(SMTP_SETTINGS_PREFIX + key, settings.get(key)); } return props; diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java index 3d2ea583edd..15859a5e044 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.watcher.notification.email; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.settings.ClusterSettings; +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; @@ -63,6 +65,10 @@ public class EmailService extends NotificationService { Setting.affixKeySetting("xpack.notification.email.account.", "smtp.password", (key) -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope, Property.Filtered)); + private static final Setting.AffixSetting SETTING_SECURE_PASSWORD = + Setting.affixKeySetting("xpack.notification.email.account.", "smtp.secure_password", + (key) -> SecureSetting.secureString(key, null)); + private static final Setting.AffixSetting SETTING_SMTP_TIMEOUT = Setting.affixKeySetting("xpack.notification.email.account.", "smtp.timeout", (key) -> Setting.timeSetting(key, TimeValue.timeValueMinutes(2), Property.Dynamic, Property.NodeScope)); @@ -111,6 +117,7 @@ public class EmailService extends NotificationService { clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_PORT, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_USER, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_PASSWORD, (s, o) -> {}, (s, o) -> {}); + clusterSettings.addAffixUpdateConsumer(SETTING_SECURE_PASSWORD, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_TIMEOUT, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_CONNECTION_TIMEOUT, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_WRITE_TIMEOUT, (s, o) -> {}, (s, o) -> {}); @@ -172,7 +179,8 @@ public class EmailService extends NotificationService { return Arrays.asList(SETTING_DEFAULT_ACCOUNT, SETTING_PROFILE, SETTING_EMAIL_DEFAULTS, SETTING_SMTP_AUTH, SETTING_SMTP_HOST, SETTING_SMTP_PASSWORD, SETTING_SMTP_PORT, SETTING_SMTP_STARTTLS_ENABLE, SETTING_SMTP_USER, SETTING_SMTP_STARTTLS_REQUIRED, SETTING_SMTP_TIMEOUT, SETTING_SMTP_CONNECTION_TIMEOUT, SETTING_SMTP_WRITE_TIMEOUT, SETTING_SMTP_LOCAL_ADDRESS, - SETTING_SMTP_LOCAL_PORT, SETTING_SMTP_SEND_PARTIAL, SETTING_SMTP_WAIT_ON_QUIT, SETTING_SMTP_SSL_TRUST_ADDRESS); + SETTING_SMTP_LOCAL_PORT, SETTING_SMTP_SEND_PARTIAL, SETTING_SMTP_WAIT_ON_QUIT, SETTING_SMTP_SSL_TRUST_ADDRESS, + SETTING_SECURE_PASSWORD); } } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/AccountTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/AccountTests.java index 8e83d30ffa5..1cbaecef8fe 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/AccountTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/AccountTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.watcher.notification.email; +import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.test.ESTestCase; @@ -16,7 +17,6 @@ import org.junit.Before; import javax.mail.Address; import javax.mail.Message; import javax.mail.internet.InternetAddress; - import java.util.Properties; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -149,7 +149,7 @@ public class AccountTests extends ESTestCase { assertThat(config.smtp.host, is(host)); assertThat(config.smtp.user, is(user)); if (password != null) { - assertThat(config.smtp.password, is(password.toCharArray())); + assertThat(config.smtp.password.getChars(), is(password.toCharArray())); } else { assertThat(config.smtp.password, nullValue()); } @@ -292,4 +292,30 @@ public class AccountTests extends ESTestCase { .build()), null, logger); }); } + + public void testEnsurePasswordSetAsSecureSetting() { + String password = "password"; + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("smtp.secure_password", password); + + Settings settings = Settings.builder() + .put("smtp.host", "localhost") + .put("smtp.port", server.port()) + .put("smtp.connection_timeout", TimeValue.timeValueMinutes(4)) + .setSecureSettings(secureSettings) + .build(); + + Account.Config config = new Account.Config("default", settings); + assertThat(config.smtp.password.getChars(), equalTo(password.toCharArray())); + + settings = Settings.builder() + .put("smtp.host", "localhost") + .put("smtp.port", server.port()) + .put("smtp.connection_timeout", TimeValue.timeValueMinutes(4)) + .put("smtp.password", password) + .build(); + + config = new Account.Config("default", settings); + assertThat(config.smtp.password.getChars(), equalTo(password.toCharArray())); + } }