Skip to content

Commit 0e2be11

Browse files
committed
Security fixes in SMTP_SSL and SMTP_TLS strategies
- The SMTP_SSL and SMTP_TLS transport strategies now validate certificates by setting JavaMail's `mail.<protocol>.ssl.checkserveridentity` property to true. Previously, no identity validation was performed, leaving SMTPS and STARTTLS connections vulnerable to man-in-the-middle attacks. Without identity validation, JavaMail accepts _any_ certificate issued by a JVM-trusted CA, regardless of the identity encoded in the certificate. - The SMTP_TLS transport strategy now requires STARTTLS support by setting JavaMail's `mail.smtp.starttls.required` property to true. Previously, STARTTLS support was not required, enabling a man-in-the-middle attack whereby an attacker could strip the STARTTLS request from an SMTP connection, causing JavaMail to fall back to plaintext SMTP for authentication and email transport.
1 parent 43d42dc commit 0e2be11

File tree

2 files changed

+19
-4
lines changed

2 files changed

+19
-4
lines changed

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ public String propertyNameAuthenticate() {
6464
},
6565
/**
6666
* SMTPS / SSL transport strategy, that returns the ".smtps." variation of the SMTP_PLAIN version. Additionally the transport protocol is
67-
* explicitly set to smtps. Finally, he property "mail.smtps.quitwait" is set to false, to get rid of a strange SSL exception:<br>
67+
* explicitly set to smtps, and strict certificate validation is enabled by setting "mail.smtps.ssl.checkserveridentity" to true. Finally,
68+
* the property "mail.smtps.quitwait" is set to false, to get rid of a strange SSL exception:<br>
6869
* <p>
6970
* <pre>
7071
* javax.mail.MessagingException: Exception reading response;
@@ -79,14 +80,16 @@ public String propertyNameAuthenticate() {
7980
*/
8081
SMTP_SSL {
8182
/**
82-
* Here protocol "mail.transport.protocol" is set to "smtps" and the quitwait property "mail.smtps.quitwait" is set to "false".
83+
* Here protocol "mail.transport.protocol" is set to "smtps", certificate validation "mail.smtps.ssl.checkserveridentity" is set to "true", and
84+
* the quitwait property "mail.smtps.quitwait" is set to "false".
8385
*
8486
* @see TransportStrategy#SMTP_SSL
8587
*/
8688
@Override
8789
public Properties generateProperties() {
8890
final Properties properties = super.generateProperties();
8991
properties.put("mail.transport.protocol", "smtps");
92+
properties.put("mail.smtps.ssl.checkserveridentity", "true");
9093
properties.put("mail.smtps.quitwait", "false");
9194
return properties;
9295
}
@@ -127,11 +130,13 @@ public String propertyNameAuthenticate() {
127130
* <strong>NOTE: this code is in untested beta state</strong>
128131
* <p>
129132
* Uses standard ".smtp." property names (like {@link TransportStrategy#SMTP_PLAIN}). Additionally the transport protocol is explicitly set to
130-
* smtp. Finally, the property "mail.smtp.starttls.enable" is being set to true.
133+
* smtp. STARTTLS is enabled by setting "mail.smtp.starttls.enable" and "mail.smtp.starttls.required" to true, and strict certificate validation
134+
* is enabled by setting "mail.smtp.ssl.checkserveridentity" to true.
131135
*/
132136
SMTP_TLS {
133137
/**
134-
* Here protocol "mail.transport.protocol" is set to "smtp" and the tls property "mail.smtp.starttls.enable" is set to "true".
138+
* Here protocol "mail.transport.protocol" is set to "smtp", STARTTLS properties "mail.smtp.starttls.enable" and "mail.smtp.starttls.required"
139+
* are set to "true", and certificate validation "mail.smtp.ssl.checkserveridentity" is set to "true".
135140
*
136141
* @see TransportStrategy#SMTP_TLS
137142
*/
@@ -140,6 +145,8 @@ public Properties generateProperties() {
140145
final Properties props = super.generateProperties();
141146
props.put("mail.transport.protocol", "smtp");
142147
props.put("mail.smtp.starttls.enable", "true");
148+
props.put("mail.smtp.starttls.required", "true");
149+
props.put("mail.smtp.ssl.checkserveridentity", "true");
143150
return props;
144151
}
145152

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ public void createMailSession_AnonymousProxyConstructor_WithoutConfig()
9090
assertThat(session.getProperty("mail.smtp.port")).isEqualTo("25");
9191
assertThat(session.getProperty("mail.transport.protocol")).isEqualTo("smtp");
9292
assertThat(session.getProperty("mail.smtp.starttls.enable")).isEqualTo("true");
93+
assertThat(session.getProperty("mail.smtp.starttls.required")).isEqualTo("true");
94+
assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true");
9395
assertThat(session.getProperty("mail.smtp.username")).isEqualTo("username smtp");
9496
assertThat(session.getProperty("mail.smtp.auth")).isEqualTo("true");
9597
assertThat(session.getProperty("mail.smtp.socks.host")).isEqualTo("proxy host");
@@ -112,6 +114,8 @@ public void createMailSession_MaximumConstructor_WithoutConfig()
112114
assertThat(session.getProperty("mail.smtp.port")).isEqualTo("25");
113115
assertThat(session.getProperty("mail.transport.protocol")).isEqualTo("smtp");
114116
assertThat(session.getProperty("mail.smtp.starttls.enable")).isEqualTo("true");
117+
assertThat(session.getProperty("mail.smtp.starttls.required")).isEqualTo("true");
118+
assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true");
115119
assertThat(session.getProperty("mail.smtp.username")).isEqualTo("username smtp");
116120
assertThat(session.getProperty("mail.smtp.auth")).isEqualTo("true");
117121
// the following two are because authentication is needed, otherwise proxy would be straightworward
@@ -131,6 +135,8 @@ public void createMailSession_MinimalConstructor_WithConfig() {
131135
assertThat(session.getProperty("mail.smtp.port")).isEqualTo("25");
132136
assertThat(session.getProperty("mail.transport.protocol")).isEqualTo("smtp");
133137
assertThat(session.getProperty("mail.smtp.starttls.enable")).isEqualTo("true");
138+
assertThat(session.getProperty("mail.smtp.starttls.required")).isEqualTo("true");
139+
assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true");
134140
assertThat(session.getProperty("mail.smtp.username")).isEqualTo("username smtp");
135141
assertThat(session.getProperty("mail.smtp.auth")).isEqualTo("true");
136142
// the following two are because authentication is needed, otherwise proxy would be straightworward
@@ -150,6 +156,8 @@ public void createMailSession_MaximumConstructor_WithConfig()
150156
assertThat(session.getProperty("mail.smtp.port")).isEqualTo("25");
151157
assertThat(session.getProperty("mail.transport.protocol")).isEqualTo("smtp");
152158
assertThat(session.getProperty("mail.smtp.starttls.enable")).isEqualTo("true");
159+
assertThat(session.getProperty("mail.smtp.starttls.required")).isEqualTo("true");
160+
assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true");
153161
assertThat(session.getProperty("mail.smtp.username")).isEqualTo("overridden username smtp");
154162
assertThat(session.getProperty("mail.smtp.auth")).isEqualTo("true");
155163
// the following two are because authentication is needed, otherwise proxy would be straightworward

0 commit comments

Comments
 (0)