Skip to content

Commit 191351d

Browse files
bbottemabbottema
committed
Properly split api regarding trusting hosts (non-SSL related) and verifying hosts (SSL related)
1 parent 55afce2 commit 191351d

File tree

6 files changed

+132
-28
lines changed

6 files changed

+132
-28
lines changed

‎src/main/java/org/simplejavamail/mailer/MailerGenericBuilder.java‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public abstract class MailerGenericBuilder<T extends MailerGenericBuilder> {
5454
public static final int DEFAULT_PROXY_BRIDGE_PORT = 1081;
5555

5656
/**
57-
* Defaults to {@value DEFAULT_TRANSPORT_MODE_LOGGING_ONLY}, sending mails rather than just only logging the mails.
57+
* Defaults to {@value}, sending mails rather than just only logging the mails.
5858
*/
5959
public static final boolean DEFAULT_TRANSPORT_MODE_LOGGING_ONLY = false;
6060

@@ -113,6 +113,11 @@ public abstract class MailerGenericBuilder<T extends MailerGenericBuilder> {
113113
*/
114114
private Boolean trustAllSSLHost;
115115

116+
/**
117+
* @see #verifyingServerIdentity(Boolean)
118+
*/
119+
private Boolean verifyingServerIdentity;
120+
116121
/**
117122
* @see #withProperties(Properties)
118123
*/
@@ -150,6 +155,7 @@ public abstract class MailerGenericBuilder<T extends MailerGenericBuilder> {
150155

151156
withEmailAddressCriteria(EmailAddressCriteria.RFC_COMPLIANT);
152157
trustingAllHosts(true);
158+
verifyingServerIdentity(true);
153159
}
154160

155161
/**
@@ -178,7 +184,7 @@ private void validateProxy() {
178184
*/
179185
OperationalConfig buildOperationalConfig() {
180186
return new OperationalConfig(getProperties(), getSessionTimeout(), getThreadPoolSize(), getTransportModeLoggingOnly(), getDebugLogging(),
181-
getSslHostsToTrust(), getTrustAllSSLHost());
187+
getSslHostsToTrust(), getTrustAllSSLHost(), getVerifyingServerIdentity());
182188
}
183189

184190
/**
@@ -335,17 +341,34 @@ public T trustingSSLHosts(String... sslHostsToTrust) {
335341
}
336342

337343
/**
338-
* Configures the current session to trust all hosts and don't validate any SSL keys. The property "mail.smtp(s).ssl.trust" is set to "*".
344+
* Configures the current session to trust all hosts. Defaults to true, but this allows you to white list <em>only</em> certain hosts.
339345
* <p>
340-
* Refer to https://javamail.java.net/nonav/docs/api/com/sun/mail/smtp/package-summary.html#mail.smtp.ssl.trust
346+
* Note that this is <em>not</em> the same as server identity verification, which is an SSL feature enabled through {@link #verifyingServerIdentity(Boolean)}.
347+
* It would be prudent to have at least one of these features turned on, lest you be vulnerable to man-in-the-middle attacks.
341348
*
349+
* @see <a href="https://javaee.github.io/javamail/docs/api/com/sun/mail/smtp/package-summary.html#mail.smtp.ssl.trust">mail.smtp.ssl.trust</a>
342350
* @see #trustingSSLHosts(String...)
343351
*/
344352
public T trustingAllHosts(@Nonnull final Boolean trustAllHosts) {
345353
this.trustAllSSLHost = trustAllHosts;
346354
return (T) this;
347355
}
348356

357+
/**
358+
* Configures the current session to not verify the server's identity on an SSL connection. Defaults to true.
359+
* <p>
360+
* Note that this is <em>not</em> the same as {@link #trustingAllHosts(Boolean)} or {@link #trustingSSLHosts(String...)}, which are not related to SSL connections.
361+
* It would be prudent to have at least one of these features turned on, lest you be vulnerable to man-in-the-middle attacks.
362+
*
363+
* @see <a href="https://javaee.github.io/javamail/docs/api/com/sun/mail/smtp/package-summary.html#mail.smtp.ssl.checkserveridentity">mail.smtp.ssl.checkserveridentity</a>
364+
* @see #trustingAllHosts(Boolean)
365+
* @see #trustingSSLHosts(String...)
366+
*/
367+
public T verifyingServerIdentity(@Nonnull final Boolean verifyingServerIdentity) {
368+
this.verifyingServerIdentity = verifyingServerIdentity;
369+
return (T) this;
370+
}
371+
349372
/**
350373
* Adds the given properties to the total list applied to the {@link Session} when building a mailer.
351374
*
@@ -535,6 +558,13 @@ public Boolean getTrustAllSSLHost() {
535558
return trustAllSSLHost;
536559
}
537560

561+
/**
562+
* @see #verifyingServerIdentity(Boolean)
563+
*/
564+
public Boolean getVerifyingServerIdentity() {
565+
return verifyingServerIdentity;
566+
}
567+
538568
/**
539569
* @see #withTransportModeLoggingOnly(Boolean)
540570
*/

‎src/main/java/org/simplejavamail/mailer/config/TransportStrategy.java‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ public enum TransportStrategy {
3939
* <li>Only {@code mail.smtp} properties are set.</li>
4040
* <li>STARTTLS is enabled by setting {@code mail.smtp.starttls.enable} to {@code true}.</li>
4141
* <li>STARTTLS plaintext fallback is enabled by setting {@code mail.smtp.starttls.required} to {@code false}.</li>
42-
* <li>Certificate issuer checks are disabled by setting {@code mail.smtp.ssl.trust} to {@code "*"}.</li>
43-
* <li>Certificate identity checks are disabled by setting {@code mail.smtp.ssl.checkserveridentity} to {@code false}.</li>
4442
* </ul>
4543
*/
4644
SMTP {
@@ -71,8 +69,6 @@ public Properties generateProperties() {
7169
LOGGER.debug("Opportunistic TLS mode enabled for SMTP plain protocol.");
7270
props.put("mail.smtp.starttls.enable", "true");
7371
props.put("mail.smtp.starttls.required", "false");
74-
props.put("mail.smtp.ssl.trust", "*");
75-
props.put("mail.smtp.ssl.checkserveridentity", "false");
7672
}
7773
return props;
7874
}
@@ -165,6 +161,14 @@ public String propertyNameSSLTrust() {
165161
return "mail.smtp.ssl.trust";
166162
}
167163

164+
/**
165+
* @return "mail.smtp.ssl.checkserveridentity"
166+
*/
167+
@Override
168+
public String propertyNameCheckServerIdentity() {
169+
throw new AssertionError("This property is not relevant for plain SMTP");
170+
}
171+
168172
/**
169173
* Sets {@link #opportunisticTLS}. Setting <code>null</code> will revert to property value if available or default to {@value
170174
* DEFAULT_OPPORTUNISTIC_TLS}
@@ -184,7 +188,6 @@ public void setOpportunisticTLS(@Nullable final Boolean opportunisticTLS) {
184188
* <ul>
185189
* <li>The transport protocol is explicitly set to {@code smtps}.</li>
186190
* <li>Only {@code mail.smtps} properties are set.</li>
187-
* <li>Certificate identity checks are enabled by setting {@code mail.smtp.ssl.checkserveridentity} to {@code true}.</li>
188191
* <li>
189192
* {@code mail.smtps.quitwait} is set to {@code false} to get rid of a strange SSLException:
190193
* <pre>
@@ -206,7 +209,6 @@ public void setOpportunisticTLS(@Nullable final Boolean opportunisticTLS) {
206209
public Properties generateProperties() {
207210
final Properties properties = super.generateProperties();
208211
properties.put("mail.transport.protocol", "smtps");
209-
properties.put("mail.smtps.ssl.checkserveridentity", "true");
210212
properties.put("mail.smtps.quitwait", "false");
211213
return properties;
212214
}
@@ -298,6 +300,14 @@ public String propertyNameEnvelopeFrom() {
298300
public String propertyNameSSLTrust() {
299301
return "mail.smtps.ssl.trust";
300302
}
303+
304+
/**
305+
* @return "mail.smtps.ssl.checkserveridentity"
306+
*/
307+
@Override
308+
public String propertyNameCheckServerIdentity() {
309+
return "mail.smtps.ssl.checkserveridentity";
310+
}
301311
},
302312
/**
303313
* Plaintext SMTP with a mandatory, authenticated STARTTLS upgrade.
@@ -311,7 +321,6 @@ public String propertyNameSSLTrust() {
311321
* <li>Only {@code mail.smtp} properties are set.</li>
312322
* <li>STARTTLS is enabled by setting {@code mail.smtp.starttls.enable} to {@code true}.</li>
313323
* <li>STARTTLS plaintext fallback is disabled by setting {@code mail.smtp.starttls.required} to {@code true}.</li>
314-
* <li>Certificate identity checks are enabled by setting {@code mail.smtp.ssl.checkserveridentity} to {@code true}.</li>
315324
* </ul>
316325
*/
317326
SMTP_TLS {
@@ -324,7 +333,6 @@ public Properties generateProperties() {
324333
props.put("mail.transport.protocol", "smtp");
325334
props.put("mail.smtp.starttls.enable", "true");
326335
props.put("mail.smtp.starttls.required", "true");
327-
props.put("mail.smtp.ssl.checkserveridentity", "true");
328336
return props;
329337
}
330338

@@ -415,6 +423,14 @@ public String propertyNameEnvelopeFrom() {
415423
public String propertyNameSSLTrust() {
416424
return "mail.smtp.ssl.trust";
417425
}
426+
427+
/**
428+
* @return "mail.smtp.ssl.checkserveridentity"
429+
*/
430+
@Override
431+
public String propertyNameCheckServerIdentity() {
432+
return "mail.smtp.ssl.checkserveridentity";
433+
}
418434
};
419435

420436
private static final Logger LOGGER = LoggerFactory.getLogger(TransportStrategy.class);
@@ -476,6 +492,10 @@ public Properties generateProperties() {
476492
* For internal use only.
477493
*/
478494
public abstract String propertyNameSSLTrust();
495+
/**
496+
* For internal use only.
497+
*/
498+
public abstract String propertyNameCheckServerIdentity();
479499
/**
480500
* For internal use only.
481501
*/

‎src/main/java/org/simplejavamail/mailer/internal/mailsender/MailSender.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,10 @@ public MailSender(@Nonnull final Session session,
9797
this.operationalConfig = operationalConfig;
9898
this.transportStrategy = transportStrategy;
9999
this.proxyServer = configureSessionWithProxy(proxyConfig, session, transportStrategy);
100-
init(operationalConfig);
100+
init(transportStrategy, operationalConfig);
101101
}
102102

103-
private void init(@Nonnull OperationalConfig operationalConfig) {
103+
private void init(@Nullable TransportStrategy transportStrategy, @Nonnull OperationalConfig operationalConfig) {
104104
session.setDebug(operationalConfig.isDebugLogging());
105105
session.getProperties().putAll(operationalConfig.getProperties());
106106
if (transportStrategy != null) {
@@ -109,6 +109,14 @@ private void init(@Nonnull OperationalConfig operationalConfig) {
109109
} else {
110110
trustHosts(operationalConfig.getSslHostsToTrust());
111111
}
112+
configureServerVerification(transportStrategy, operationalConfig);
113+
}
114+
}
115+
116+
private void configureServerVerification(@Nonnull TransportStrategy transportStrategy, @Nonnull OperationalConfig operationalConfig) {
117+
if (transportStrategy != TransportStrategy.SMTP) {
118+
session.getProperties().setProperty(transportStrategy.propertyNameCheckServerIdentity(),
119+
Boolean.toString(operationalConfig.isVerifyingServerIdentity()));
112120
}
113121
}
114122

‎src/main/java/org/simplejavamail/mailer/internal/mailsender/OperationalConfig.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,17 +49,23 @@ public class OperationalConfig {
4949
*/
5050
private final boolean trustAllSSLHost;
5151

52+
/**
53+
* @see org.simplejavamail.mailer.MailerBuilder.MailerRegularBuilder#verifyingServerIdentity(Boolean)
54+
*/
55+
private final boolean verifyingServerIdentity;
56+
5257
/**
5358
* For internal use only.
5459
*/
55-
public OperationalConfig(@Nonnull Properties properties, int sessionTimeout, int threadPoolSize, boolean transportModeLoggingOnly, boolean debugLogging, List<String> sslHostsToTrust, boolean trustAllSSLHost) {
60+
public OperationalConfig(@Nonnull Properties properties, int sessionTimeout, int threadPoolSize, boolean transportModeLoggingOnly, boolean debugLogging, List<String> sslHostsToTrust, boolean trustAllSSLHost, boolean verifyingServerIdentity) {
5661
this.properties = properties;
5762
this.sessionTimeout = sessionTimeout;
5863
this.threadPoolSize = threadPoolSize;
5964
this.transportModeLoggingOnly = transportModeLoggingOnly;
6065
this.debugLogging = debugLogging;
6166
this.sslHostsToTrust = Collections.unmodifiableList(sslHostsToTrust);
6267
this.trustAllSSLHost = trustAllSSLHost;
68+
this.verifyingServerIdentity = verifyingServerIdentity;
6369
}
6470

6571
/**
@@ -104,6 +110,13 @@ public boolean isTrustAllSSLHost() {
104110
return trustAllSSLHost;
105111
}
106112

113+
/**
114+
* @see org.simplejavamail.mailer.MailerBuilder.MailerRegularBuilder#verifyingServerIdentity(Boolean)
115+
*/
116+
public boolean isVerifyingServerIdentity() {
117+
return verifyingServerIdentity;
118+
}
119+
107120
/**
108121
* @see org.simplejavamail.mailer.MailerBuilder.MailerRegularBuilder#withProperties(Properties)
109122
*/

‎src/test/java/org/simplejavamail/mailer/MailerTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public void createMailSession_MinimalConstructor_WithoutConfig()
6262
assertThat(session.getProperty("mail.smtp.starttls.enable")).isEqualTo("true");
6363
assertThat(session.getProperty("mail.smtp.starttls.required")).isEqualTo("false");
6464
assertThat(session.getProperty("mail.smtp.ssl.trust")).isEqualTo("*");
65-
assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("false");
65+
assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isNull();
6666

6767
assertThat(session.getProperty("mail.smtp.username")).isNull();
6868
assertThat(session.getProperty("mail.smtp.auth")).isNull();
@@ -191,7 +191,7 @@ public void createMailSession_MinimalConstructor_WithConfig_OPPORTUNISTIC_TLS_Ma
191191
assertThat(session.getProperty("mail.smtp.starttls.enable")).isEqualTo("true");
192192
assertThat(session.getProperty("mail.smtp.starttls.required")).isEqualTo("false");
193193
assertThat(session.getProperty("mail.smtp.ssl.trust")).isEqualTo("*");
194-
assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("false");
194+
assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isNull();
195195

196196
assertThat(session.getProperty("mail.smtp.username")).isEqualTo("username smtp");
197197
assertThat(session.getProperty("mail.smtp.auth")).isEqualTo("true");
@@ -283,6 +283,7 @@ private Mailer createFullyConfiguredMailer(boolean authenticateProxy, String pre
283283
MailerRegularBuilder mailerBuilder = MailerBuilder
284284
.withSMTPServer(prefix + "smtp host", 25, prefix + "username smtp", prefix + "password smtp")
285285
.withTransportStrategy(transportStrategy)
286+
.verifyingServerIdentity(true)
286287
.withDebugLogging(true);
287288

288289
if (transportStrategy == SMTP_TLS) {

‎src/test/java/org/simplejavamail/mailer/internal/mailsender/MailSenderTest.java‎

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import static org.mockito.Mockito.when;
1616
import static org.simplejavamail.mailer.config.TransportStrategy.SMTP;
1717
import static org.simplejavamail.mailer.config.TransportStrategy.SMTPS;
18+
import static org.simplejavamail.mailer.config.TransportStrategy.SMTP_TLS;
1819

1920
public class MailSenderTest {
2021

@@ -24,7 +25,7 @@ public class MailSenderTest {
2425

2526
@Before
2627
public void setup() {
27-
session = Session.getDefaultInstance(new Properties());
28+
session = Session.getInstance(new Properties());
2829
}
2930

3031
@Nonnull
@@ -34,41 +35,72 @@ private ProxyConfig createEmptyProxyConfig() {
3435

3536
@Test
3637
public void trustAllHosts_PLAIN() {
37-
new MailSender(session, createDummyOperationalConfig(EMPTY_LIST, true), createEmptyProxyConfig(), SMTP);
38+
new MailSender(session, createDummyOperationalConfig(EMPTY_LIST, true, false), createEmptyProxyConfig(), SMTP);
3839
assertThat(session.getProperties().getProperty("mail.smtp.ssl.trust")).isEqualTo("*");
39-
new MailSender(session, createDummyOperationalConfig(EMPTY_LIST, false), createEmptyProxyConfig(), SMTP);
40+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.checkserveridentity")).isNull();
41+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.checkserveridentity")).isNull();
42+
new MailSender(session, createDummyOperationalConfig(EMPTY_LIST, false, true), createEmptyProxyConfig(), SMTP);
4043
assertThat(session.getProperties().getProperty("mail.smtp.ssl.trust")).isNull();
44+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.checkserveridentity")).isNull();
45+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.checkserveridentity")).isNull();
4146
}
4247

4348
@Test
4449
public void trustAllHosts_SMTPS() {
4550
ProxyConfig proxyBypassingMock = mock(ProxyConfig.class);
4651
when(proxyBypassingMock.requiresProxy()).thenReturn(false);
47-
new MailSender(session, createDummyOperationalConfig(EMPTY_LIST, true), proxyBypassingMock, SMTPS);
52+
new MailSender(session, createDummyOperationalConfig(EMPTY_LIST, true, false), proxyBypassingMock, SMTPS);
4853
assertThat(session.getProperties().getProperty("mail.smtps.ssl.trust")).isEqualTo("*");
49-
new MailSender(session, createDummyOperationalConfig(EMPTY_LIST, false), proxyBypassingMock, SMTPS);
54+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.checkserveridentity")).isNull();
55+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.checkserveridentity")).isEqualTo("false");
56+
new MailSender(session, createDummyOperationalConfig(EMPTY_LIST, false, true), proxyBypassingMock, SMTPS);
5057
assertThat(session.getProperties().getProperty("mail.smtps.ssl.trust")).isNull();
58+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.checkserveridentity")).isNull();
59+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.checkserveridentity")).isEqualTo("true");
60+
}
61+
62+
@Test
63+
public void trustAllHosts_SMTP_TLS() {
64+
ProxyConfig proxyBypassingMock = mock(ProxyConfig.class);
65+
when(proxyBypassingMock.requiresProxy()).thenReturn(false);
66+
new MailSender(session, createDummyOperationalConfig(EMPTY_LIST, true, false), proxyBypassingMock, SMTP_TLS);
67+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.trust")).isEqualTo("*");
68+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("false");
69+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.checkserveridentity")).isNull();
70+
new MailSender(session, createDummyOperationalConfig(EMPTY_LIST, false, true), proxyBypassingMock, SMTP_TLS);
71+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.trust")).isNull();
72+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true");
73+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.checkserveridentity")).isNull();
5174
}
5275

5376
@Test
5477
public void trustHosts() {
55-
new MailSender(session, createDummyOperationalConfig(asList(), false), createEmptyProxyConfig(), SMTP);
78+
new MailSender(session, createDummyOperationalConfig(asList(), false, false), createEmptyProxyConfig(), SMTP);
5679
assertThat(session.getProperties().getProperty("mail.smtp.ssl.trust")).isNull();
57-
new MailSender(session, createDummyOperationalConfig(asList("a"), false), createEmptyProxyConfig(), SMTP);
80+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.checkserveridentity")).isNull();
81+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.checkserveridentity")).isNull();
82+
new MailSender(session, createDummyOperationalConfig(asList("a"), false, false), createEmptyProxyConfig(), SMTP);
5883
assertThat(session.getProperties().getProperty("mail.smtp.ssl.trust")).isEqualTo("a");
59-
new MailSender(session, createDummyOperationalConfig(asList("a", "b"), false), createEmptyProxyConfig(), SMTP);
84+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.checkserveridentity")).isNull();
85+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.checkserveridentity")).isNull();
86+
new MailSender(session, createDummyOperationalConfig(asList("a", "b"), false, false), createEmptyProxyConfig(), SMTP);
6087
assertThat(session.getProperties().getProperty("mail.smtp.ssl.trust")).isEqualTo("a b");
61-
new MailSender(session, createDummyOperationalConfig(asList("a", "b", "c"), false), createEmptyProxyConfig(), SMTP);
88+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.checkserveridentity")).isNull();
89+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.checkserveridentity")).isNull();
90+
new MailSender(session, createDummyOperationalConfig(asList("a", "b", "c"), false, true), createEmptyProxyConfig(), SMTP);
6291
assertThat(session.getProperties().getProperty("mail.smtp.ssl.trust")).isEqualTo("a b c");
92+
assertThat(session.getProperties().getProperty("mail.smtp.ssl.checkserveridentity")).isNull();
93+
assertThat(session.getProperties().getProperty("mail.smtps.ssl.checkserveridentity")).isNull();
6394
}
6495

6596
@Nonnull
6697
private List<String> asList(String... args) {
6798
return Arrays.asList(args);
6899
}
69100

101+
@SuppressWarnings("SameParameterValue")
70102
@Nonnull
71-
private OperationalConfig createDummyOperationalConfig(List<String> hostsToTrust, boolean trustAllSSLHost) {
72-
return new OperationalConfig(new Properties(), 0, 0, false, false, hostsToTrust, trustAllSSLHost);
103+
private OperationalConfig createDummyOperationalConfig(List<String> hostsToTrust, boolean trustAllSSLHost, boolean verifyServerIdentity) {
104+
return new OperationalConfig(new Properties(), 0, 0, false, false, hostsToTrust, trustAllSSLHost, verifyServerIdentity);
73105
}
74106
}

0 commit comments

Comments
 (0)