Skip to content

Commit 17e19ef

Browse files
authored
Fix smtp.ssl.trust setting for watcher email (#57269)
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
1 parent 746de41 commit 17e19ef

File tree

5 files changed

+73
-10
lines changed

5 files changed

+73
-10
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLConfiguration.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public final class SSLConfiguration {
3838
private final List<String> supportedProtocols;
3939
private final SSLClientAuth sslClientAuth;
4040
private final VerificationMode verificationMode;
41+
private final boolean explicitlyConfigured;
4142

4243
/**
4344
* 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 {
5253
this.supportedProtocols = getListOrDefault(SETTINGS_PARSER.supportedProtocols, settings, XPackSettings.DEFAULT_SUPPORTED_PROTOCOLS);
5354
this.sslClientAuth = SETTINGS_PARSER.clientAuth.get(settings).orElse(XPackSettings.CLIENT_AUTH_DEFAULT);
5455
this.verificationMode = SETTINGS_PARSER.verificationMode.get(settings).orElse(XPackSettings.VERIFICATION_MODE_DEFAULT);
56+
this.explicitlyConfigured = settings.isEmpty() == false;
5557
}
5658

5759
/**
@@ -108,6 +110,10 @@ List<Path> filesToMonitor(@Nullable Environment environment) {
108110
return paths;
109111
}
110112

113+
public boolean isExplicitlyConfigured() {
114+
return explicitlyConfigured;
115+
}
116+
111117
@Override
112118
public String toString() {
113119
return "SSLConfiguration{" +

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
import java.security.PrivilegedExceptionAction;
3232
import java.util.Properties;
3333
import java.util.Set;
34+
import java.util.stream.Collectors;
35+
36+
import static org.elasticsearch.xpack.core.watcher.WatcherField.EMAIL_NOTIFICATION_SSL_PREFIX;
3437

3538
public class Account {
3639

@@ -187,7 +190,7 @@ static class Config {
187190
final Smtp smtp;
188191
final EmailDefaults defaults;
189192

190-
Config(String name, Settings settings, @Nullable SSLSocketFactory sslSocketFactory) {
193+
Config(String name, Settings settings, @Nullable SSLSocketFactory sslSocketFactory, Logger logger) {
191194
this.name = name;
192195
profile = Profile.resolve(settings.get("profile"), Profile.STANDARD);
193196
defaults = new EmailDefaults(name, settings.getAsSettings("email_defaults"));
@@ -197,6 +200,15 @@ static class Config {
197200
throw new SettingsException(msg);
198201
}
199202
if (sslSocketFactory != null) {
203+
String sslKeys = smtp.properties.keySet().stream()
204+
.map(String::valueOf)
205+
.filter(key -> key.startsWith("mail.smtp.ssl."))
206+
.collect(Collectors.joining(","));
207+
if (sslKeys.isEmpty() == false) {
208+
logger.warn("The SMTP SSL settings [{}] that are configured for Account [{}]" +
209+
" will be ignored due to the notification SSL settings in [{}]",
210+
sslKeys, name, EMAIL_NOTIFICATION_SSL_PREFIX);
211+
}
200212
smtp.setSocketFactory(sslSocketFactory);
201213
}
202214
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,14 @@ public EmailService(Settings settings, @Nullable CryptoService cryptoService, SS
141141

142142
@Override
143143
protected Account createAccount(String name, Settings accountSettings) {
144-
Account.Config config = new Account.Config(name, accountSettings, getSmtpSslSocketFactory());
144+
Account.Config config = new Account.Config(name, accountSettings, getSmtpSslSocketFactory(), logger);
145145
return new Account(config, cryptoService, logger);
146146
}
147147

148148
@Nullable
149149
private SSLSocketFactory getSmtpSslSocketFactory() {
150150
final SSLConfiguration sslConfiguration = sslService.getSSLConfiguration(EMAIL_NOTIFICATION_SSL_PREFIX);
151-
if (sslConfiguration == null) {
151+
if (sslConfiguration == null || sslConfiguration.isExplicitlyConfigured() == false) {
152152
return null;
153153
}
154154
return sslService.sslSocketFactory(sslConfiguration);

x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailSslTests.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,51 @@ public void testCanSendMessageToSmtpServerByDisablingVerification() throws Excep
124124
}
125125
}
126126

127+
public void testCanSendMessageToSmtpServerUsingSmtpSslTrust() throws Exception {
128+
assumeFalse("Can't run in a FIPS JVM with verification mode None", inFipsJvm());
129+
List<MimeMessage> messages = new ArrayList<>();
130+
server.addListener(messages::add);
131+
try {
132+
final Settings.Builder settings = Settings.builder()
133+
.put("xpack.notification.email.account.test.smtp.ssl.trust", "localhost");
134+
final MockSecureSettings secureSettings = new MockSecureSettings();
135+
ExecutableEmailAction emailAction = buildEmailAction(settings, secureSettings);
136+
137+
WatchExecutionContext ctx = WatcherTestUtils.createWatchExecutionContext();
138+
emailAction.execute("my_action_id", ctx, Payload.EMPTY);
139+
140+
assertThat(messages, hasSize(1));
141+
} finally {
142+
server.clearListeners();
143+
}
144+
}
145+
146+
/**
147+
* This orderining could be considered to be backwards (the global "notification" settings take precedence
148+
* over the account level "smtp.ssl.trust" setting) but smtp.ssl.trust was ignored for a period of time (see #52153)
149+
* so this is the least breaking way to resolve that.
150+
*/
151+
public void testNotificationSslSettingsOverrideSmtpSslTrust() throws Exception {
152+
List<MimeMessage> messages = new ArrayList<>();
153+
server.addListener(messages::add);
154+
try {
155+
final Settings.Builder settings = Settings.builder()
156+
.put("xpack.notification.email.account.test.smtp.ssl.trust", "localhost")
157+
.put("xpack.notification.email.ssl.verification_mode", "full");
158+
final MockSecureSettings secureSettings = new MockSecureSettings();
159+
ExecutableEmailAction emailAction = buildEmailAction(settings, secureSettings);
160+
161+
WatchExecutionContext ctx = WatcherTestUtils.createWatchExecutionContext();
162+
final MessagingException exception = expectThrows(MessagingException.class,
163+
() -> emailAction.execute("my_action_id", ctx, Payload.EMPTY));
164+
165+
final List<Throwable> allCauses = getAllCauses(exception);
166+
assertThat(allCauses, Matchers.hasItem(Matchers.instanceOf(SSLException.class)));
167+
} finally {
168+
server.clearListeners();
169+
}
170+
}
171+
127172
private ExecutableEmailAction buildEmailAction(Settings.Builder baseSettings, MockSecureSettings secureSettings) {
128173
secureSettings.setString("xpack.notification.email.account.test.smtp.secure_password", EmailServer.PASSWORD);
129174
Settings settings = baseSettings

x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/AccountTests.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ public void testConfig() throws Exception {
141141

142142
Settings settings = builder.build();
143143

144-
Account.Config config = new Account.Config(accountName, settings, null);
144+
Account.Config config = new Account.Config(accountName, settings, null, logger);
145145

146146
assertThat(config.profile, is(profile));
147147
assertThat(config.defaults, equalTo(emailDefaults));
@@ -165,7 +165,7 @@ public void testSend() throws Exception {
165165
.put("smtp.port", server.port())
166166
.put("smtp.user", EmailServer.USERNAME)
167167
.setSecureSettings(secureSettings)
168-
.build(), null), null, logger);
168+
.build(), null, logger), null, logger);
169169

170170
Email email = Email.builder()
171171
.id("_id")
@@ -202,7 +202,7 @@ public void testSendCCAndBCC() throws Exception {
202202
.put("smtp.port", server.port())
203203
.put("smtp.user", EmailServer.USERNAME)
204204
.setSecureSettings(secureSettings)
205-
.build(), null), null, logger);
205+
.build(), null, logger), null, logger);
206206

207207
Email email = Email.builder()
208208
.id("_id")
@@ -240,7 +240,7 @@ public void testSendAuthentication() throws Exception {
240240
Account account = new Account(new Account.Config("default", Settings.builder()
241241
.put("smtp.host", "localhost")
242242
.put("smtp.port", server.port())
243-
.build(), null), null, logger);
243+
.build(), null, logger), null, logger);
244244

245245
Email email = Email.builder()
246246
.id("_id")
@@ -264,7 +264,7 @@ public void testDefaultAccountTimeout() {
264264
Account account = new Account(new Account.Config("default", Settings.builder()
265265
.put("smtp.host", "localhost")
266266
.put("smtp.port", server.port())
267-
.build(), null), null, logger);
267+
.build(), null, logger), null, logger);
268268

269269
Properties mailProperties = account.getConfig().smtp.properties;
270270
assertThat(mailProperties.get("mail.smtp.connectiontimeout"), is(String.valueOf(TimeValue.timeValueMinutes(2).millis())));
@@ -279,7 +279,7 @@ public void testAccountTimeoutsCanBeConfigureAsTimeValue() {
279279
.put("smtp.connection_timeout", TimeValue.timeValueMinutes(4))
280280
.put("smtp.write_timeout", TimeValue.timeValueMinutes(6))
281281
.put("smtp.timeout", TimeValue.timeValueMinutes(8))
282-
.build(), null), null, logger);
282+
.build(), null, logger), null, logger);
283283

284284
Properties mailProperties = account.getConfig().smtp.properties;
285285

@@ -294,7 +294,7 @@ public void testAccountTimeoutsConfiguredAsNumberAreRejected() {
294294
.put("smtp.host", "localhost")
295295
.put("smtp.port", server.port())
296296
.put("smtp.connection_timeout", 4000)
297-
.build(), null), null, logger);
297+
.build(), null, logger), null, logger);
298298
});
299299
}
300300

0 commit comments

Comments
 (0)