Fix smtp.ssl.trust setting for watcher email (#57268)
The ssl.trust setting for Watcher provides a list of hostnames that should be automatically trusted for SSL hostname verification. It was accidentally broken when we added the full ssl.* settings for email notifications (see #45272) This commit corrects this, so the setting is once again respected, as long as none of the other ssl settings are configured for email notifications. Resolves: #52153 Backport of: #56090
This commit is contained in:
parent
1b59e9ab22
commit
408250dcc4
|
@ -38,6 +38,7 @@ public final class SSLConfiguration {
|
|||
private final List<String> supportedProtocols;
|
||||
private final SSLClientAuth sslClientAuth;
|
||||
private final VerificationMode verificationMode;
|
||||
private final boolean explicitlyConfigured;
|
||||
|
||||
/**
|
||||
* Creates a new SSLConfiguration from the given settings. There is no fallback configuration when invoking this constructor so
|
||||
|
@ -52,6 +53,7 @@ public final class SSLConfiguration {
|
|||
this.supportedProtocols = getListOrDefault(SETTINGS_PARSER.supportedProtocols, settings, XPackSettings.DEFAULT_SUPPORTED_PROTOCOLS);
|
||||
this.sslClientAuth = SETTINGS_PARSER.clientAuth.get(settings).orElse(XPackSettings.CLIENT_AUTH_DEFAULT);
|
||||
this.verificationMode = SETTINGS_PARSER.verificationMode.get(settings).orElse(XPackSettings.VERIFICATION_MODE_DEFAULT);
|
||||
this.explicitlyConfigured = settings.isEmpty() == false;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -108,6 +110,10 @@ public final class SSLConfiguration {
|
|||
return paths;
|
||||
}
|
||||
|
||||
public boolean isExplicitlyConfigured() {
|
||||
return explicitlyConfigured;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return "SSLConfiguration{" +
|
||||
|
|
|
@ -31,6 +31,9 @@ import java.security.PrivilegedActionException;
|
|||
import java.security.PrivilegedExceptionAction;
|
||||
import java.util.Properties;
|
||||
import java.util.Set;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import static org.elasticsearch.xpack.core.watcher.WatcherField.EMAIL_NOTIFICATION_SSL_PREFIX;
|
||||
|
||||
public class Account {
|
||||
|
||||
|
@ -187,7 +190,7 @@ public class Account {
|
|||
final Smtp smtp;
|
||||
final EmailDefaults defaults;
|
||||
|
||||
Config(String name, Settings settings, @Nullable SSLSocketFactory sslSocketFactory) {
|
||||
Config(String name, Settings settings, @Nullable SSLSocketFactory sslSocketFactory, Logger logger) {
|
||||
this.name = name;
|
||||
profile = Profile.resolve(settings.get("profile"), Profile.STANDARD);
|
||||
defaults = new EmailDefaults(name, settings.getAsSettings("email_defaults"));
|
||||
|
@ -197,6 +200,15 @@ public class Account {
|
|||
throw new SettingsException(msg);
|
||||
}
|
||||
if (sslSocketFactory != null) {
|
||||
String sslKeys = smtp.properties.keySet().stream()
|
||||
.map(String::valueOf)
|
||||
.filter(key -> key.startsWith("mail.smtp.ssl."))
|
||||
.collect(Collectors.joining(","));
|
||||
if (sslKeys.isEmpty() == false) {
|
||||
logger.warn("The SMTP SSL settings [{}] that are configured for Account [{}]" +
|
||||
" will be ignored due to the notification SSL settings in [{}]",
|
||||
sslKeys, name, EMAIL_NOTIFICATION_SSL_PREFIX);
|
||||
}
|
||||
smtp.setSocketFactory(sslSocketFactory);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -141,14 +141,14 @@ public class EmailService extends NotificationService<Account> {
|
|||
|
||||
@Override
|
||||
protected Account createAccount(String name, Settings accountSettings) {
|
||||
Account.Config config = new Account.Config(name, accountSettings, getSmtpSslSocketFactory());
|
||||
Account.Config config = new Account.Config(name, accountSettings, getSmtpSslSocketFactory(), logger);
|
||||
return new Account(config, cryptoService, logger);
|
||||
}
|
||||
|
||||
@Nullable
|
||||
private SSLSocketFactory getSmtpSslSocketFactory() {
|
||||
final SSLConfiguration sslConfiguration = sslService.getSSLConfiguration(EMAIL_NOTIFICATION_SSL_PREFIX);
|
||||
if (sslConfiguration == null) {
|
||||
if (sslConfiguration == null || sslConfiguration.isExplicitlyConfigured() == false) {
|
||||
return null;
|
||||
}
|
||||
return sslService.sslSocketFactory(sslConfiguration);
|
||||
|
|
|
@ -125,6 +125,51 @@ public class EmailSslTests extends ESTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
public void testCanSendMessageToSmtpServerUsingSmtpSslTrust() throws Exception {
|
||||
assumeFalse("Can't run in a FIPS JVM with verification mode None", inFipsJvm());
|
||||
List<MimeMessage> messages = new ArrayList<>();
|
||||
server.addListener(messages::add);
|
||||
try {
|
||||
final Settings.Builder settings = Settings.builder()
|
||||
.put("xpack.notification.email.account.test.smtp.ssl.trust", "localhost");
|
||||
final MockSecureSettings secureSettings = new MockSecureSettings();
|
||||
ExecutableEmailAction emailAction = buildEmailAction(settings, secureSettings);
|
||||
|
||||
WatchExecutionContext ctx = WatcherTestUtils.createWatchExecutionContext();
|
||||
emailAction.execute("my_action_id", ctx, Payload.EMPTY);
|
||||
|
||||
assertThat(messages, hasSize(1));
|
||||
} finally {
|
||||
server.clearListeners();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* This orderining could be considered to be backwards (the global "notification" settings take precedence
|
||||
* over the account level "smtp.ssl.trust" setting) but smtp.ssl.trust was ignored for a period of time (see #52153)
|
||||
* so this is the least breaking way to resolve that.
|
||||
*/
|
||||
public void testNotificationSslSettingsOverrideSmtpSslTrust() throws Exception {
|
||||
List<MimeMessage> messages = new ArrayList<>();
|
||||
server.addListener(messages::add);
|
||||
try {
|
||||
final Settings.Builder settings = Settings.builder()
|
||||
.put("xpack.notification.email.account.test.smtp.ssl.trust", "localhost")
|
||||
.put("xpack.notification.email.ssl.verification_mode", "full");
|
||||
final MockSecureSettings secureSettings = new MockSecureSettings();
|
||||
ExecutableEmailAction emailAction = buildEmailAction(settings, secureSettings);
|
||||
|
||||
WatchExecutionContext ctx = WatcherTestUtils.createWatchExecutionContext();
|
||||
final MessagingException exception = expectThrows(MessagingException.class,
|
||||
() -> emailAction.execute("my_action_id", ctx, Payload.EMPTY));
|
||||
|
||||
final List<Throwable> allCauses = getAllCauses(exception);
|
||||
assertThat(allCauses, Matchers.hasItem(Matchers.instanceOf(SSLException.class)));
|
||||
} finally {
|
||||
server.clearListeners();
|
||||
}
|
||||
}
|
||||
|
||||
private ExecutableEmailAction buildEmailAction(Settings.Builder baseSettings, MockSecureSettings secureSettings) {
|
||||
secureSettings.setString("xpack.notification.email.account.test.smtp.secure_password", EmailServer.PASSWORD);
|
||||
Settings settings = baseSettings
|
||||
|
|
|
@ -141,7 +141,7 @@ public class AccountTests extends ESTestCase {
|
|||
|
||||
Settings settings = builder.build();
|
||||
|
||||
Account.Config config = new Account.Config(accountName, settings, null);
|
||||
Account.Config config = new Account.Config(accountName, settings, null, logger);
|
||||
|
||||
assertThat(config.profile, is(profile));
|
||||
assertThat(config.defaults, equalTo(emailDefaults));
|
||||
|
@ -165,7 +165,7 @@ public class AccountTests extends ESTestCase {
|
|||
.put("smtp.port", server.port())
|
||||
.put("smtp.user", EmailServer.USERNAME)
|
||||
.setSecureSettings(secureSettings)
|
||||
.build(), null), null, logger);
|
||||
.build(), null, logger), null, logger);
|
||||
|
||||
Email email = Email.builder()
|
||||
.id("_id")
|
||||
|
@ -202,7 +202,7 @@ public class AccountTests extends ESTestCase {
|
|||
.put("smtp.port", server.port())
|
||||
.put("smtp.user", EmailServer.USERNAME)
|
||||
.setSecureSettings(secureSettings)
|
||||
.build(), null), null, logger);
|
||||
.build(), null, logger), null, logger);
|
||||
|
||||
Email email = Email.builder()
|
||||
.id("_id")
|
||||
|
@ -240,7 +240,7 @@ public class AccountTests extends ESTestCase {
|
|||
Account account = new Account(new Account.Config("default", Settings.builder()
|
||||
.put("smtp.host", "localhost")
|
||||
.put("smtp.port", server.port())
|
||||
.build(), null), null, logger);
|
||||
.build(), null, logger), null, logger);
|
||||
|
||||
Email email = Email.builder()
|
||||
.id("_id")
|
||||
|
@ -264,7 +264,7 @@ public class AccountTests extends ESTestCase {
|
|||
Account account = new Account(new Account.Config("default", Settings.builder()
|
||||
.put("smtp.host", "localhost")
|
||||
.put("smtp.port", server.port())
|
||||
.build(), null), null, logger);
|
||||
.build(), null, logger), null, logger);
|
||||
|
||||
Properties mailProperties = account.getConfig().smtp.properties;
|
||||
assertThat(mailProperties.get("mail.smtp.connectiontimeout"), is(String.valueOf(TimeValue.timeValueMinutes(2).millis())));
|
||||
|
@ -279,7 +279,7 @@ public class AccountTests extends ESTestCase {
|
|||
.put("smtp.connection_timeout", TimeValue.timeValueMinutes(4))
|
||||
.put("smtp.write_timeout", TimeValue.timeValueMinutes(6))
|
||||
.put("smtp.timeout", TimeValue.timeValueMinutes(8))
|
||||
.build(), null), null, logger);
|
||||
.build(), null, logger), null, logger);
|
||||
|
||||
Properties mailProperties = account.getConfig().smtp.properties;
|
||||
|
||||
|
@ -294,7 +294,7 @@ public class AccountTests extends ESTestCase {
|
|||
.put("smtp.host", "localhost")
|
||||
.put("smtp.port", server.port())
|
||||
.put("smtp.connection_timeout", 4000)
|
||||
.build(), null), null, logger);
|
||||
.build(), null, logger), null, logger);
|
||||
});
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue