Skip to content

Commit

Permalink
Add secure setting for watcher email password (#31620)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@hub-cap
hub-cap authored Jul 13, 2018
1 parent c1a81e5 commit bf76890
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 8 deletions.
40 changes: 35 additions & 5 deletions ...gin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/Account.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

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;
Expand All @@ -24,10 +27,13 @@
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<SecureString> SECURE_PASSWORD_SETTING = SecureSetting.secureString("secure_" + SMTP_PASSWORD, null);

static {
SecurityManager sm = System.getSecurityManager();
Expand Down Expand Up @@ -101,7 +107,7 @@ public Email send(Email email, Authentication auth, Profile profile) throws Mess
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) {
Expand Down Expand Up @@ -199,18 +205,40 @@ static class Smtp {
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<SecureString> 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
Expand All @@ -231,7 +259,9 @@ static Properties loadSmtpProperties(Settings settings) {

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<String> insecureSettings = settings.filter(s -> s.startsWith("secure_") == false).keySet();
for (String key : insecureSettings) {
props.setProperty(SMTP_SETTINGS_PREFIX + key, settings.get(key));
}
return props;
Expand Down
10 changes: 9 additions & 1 deletion ...atcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

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;
Expand Down Expand Up @@ -63,6 +65,10 @@ public class EmailService extends NotificationService<Account> {
Setting.affixKeySetting("xpack.notification.email.account.", "smtp.password",
(key) -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope, Property.Filtered));

private static final Setting.AffixSetting<SecureString> SETTING_SECURE_PASSWORD =
Setting.affixKeySetting("xpack.notification.email.account.", "smtp.secure_password",
(key) -> SecureSetting.secureString(key, null));

private static final Setting.AffixSetting<TimeValue> SETTING_SMTP_TIMEOUT =
Setting.affixKeySetting("xpack.notification.email.account.", "smtp.timeout",
(key) -> Setting.timeSetting(key, TimeValue.timeValueMinutes(2), Property.Dynamic, Property.NodeScope));
Expand Down Expand Up @@ -111,6 +117,7 @@ public EmailService(Settings settings, @Nullable CryptoService cryptoService, Cl
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) -> {});
Expand Down Expand Up @@ -172,7 +179,8 @@ public static List<Setting<?>> getSettings() {
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);
}

}
30 changes: 28 additions & 2 deletions ...atcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/AccountTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -16,7 +17,6 @@
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;
Expand Down Expand Up @@ -149,7 +149,7 @@ public void testConfig() throws Exception {
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());
}
Expand Down Expand Up @@ -292,4 +292,30 @@ public void testAccountTimeoutsConfiguredAsNumberAreRejected() {
.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()));
}
}

0 comments on commit bf76890

Please sign in to comment.