Misc. TLS security fixes and improvements

  - The SMTP_PLAIN transport strategy now attempts an (insecure) STARTTLS
    upgrade where possible, but will always permit plaintext fallback to
    preserve backwards-compatibility with unencrypted SMTP.

    The opportunistic STARTTLS handshake in SMTP_PLAIN does not validate
    the server certificate's issuer or identity; therefore, it does not
    protect against active network attackers.

    The STARTTLS handshake, in this transport strategy, is merely a best-effort
    encryption mechanism to defend against passive network eavesdroppers.

  - 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.

Author: cbarcenas

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

@@ -10,23 +10,48 @@
  * <code>TransportStrategy</code> implementation.
  *
  * @author Benny Bottema
+ * @author Christian Barcenas
  */
 public enum TransportStrategy {
 
 	/**
-	 * Simplest possible form: only vanilla ".smtp." property names and no extra properties. Additionally the transport protocol is explicitly set to
-	 * smtp.
+	 * Vanilla SMTP with an insecure STARTTLS upgrade (if supported).
+	 * <p>
+	 * This {@code TransportStrategy} falls back to plaintext when a mail server does not indicate support for
+	 * STARTTLS. Additionally, even if a TLS session is negotiated, <strong>server certificates are not validated in
+	 * any way</strong>.
+	 * <p>
+	 * This {@code TransportStrategy} only offers protection against passive network eavesdroppers when the mail server
+	 * indicates support for STARTTLS. Active network attackers can trivially bypass the encryption 1) by tampering with
+	 * the STARTTLS indicator, 2) by presenting a self-signed certificate, 3) by presenting a certificate issued by an
+	 * untrusted certificate authority; or 4) by presenting a certificate that was issued by a valid certificate
+	 * authority to a domain other than the mail server's.
+	 * <p>
+	 * For proper mail transport encryption, see {@link TransportStrategy#SMTP_SSL} or
+	 * {@link TransportStrategy#SMTP_TLS}.
+	 * <p>
+	 * Implementation notes:
+	 * <ul>
+	 *     <li>The transport protocol is explicitly set to {@code smtp}.</li>
+	 *     <li>Only {@code mail.smtp} properties are set.</li>
+	 *     <li>STARTTLS is enabled by setting {@code mail.smtp.starttls.enable} to {@code true}.</li>
+	 *     <li>STARTTLS plaintext fallback is enabled by setting {@code mail.smtp.starttls.required} to {@code false}.</li>
+	 *     <li>Certificate issuer checks are disabled by setting {@code mail.smtp.ssl.trust} to {@code "*"}.</li>
+	 *     <li>Certificate identity checks are disabled by setting {@code mail.smtp.ssl.checkserveridentity} to {@code false}.</li>
+     * </ul>
 	 */
 	SMTP_PLAIN {
 		/**
-		 * Here protocol "mail.transport.protocol" is set to "smtp".
-		 *
 		 * @see TransportStrategy#SMTP_PLAIN
 		 */
 		@Override
 		public Properties generateProperties() {
 			final Properties props = super.generateProperties();
 			props.put("mail.transport.protocol", "smtp");
+			props.put("mail.smtp.starttls.enable", true);
+			props.put("mail.smtp.starttls.required", false);
+			props.put("mail.smtp.ssl.trust", "*");
+			props.put("mail.smtp.ssl.checkserveridentity", false);
 			return props;
 		}
 
@@ -63,9 +88,18 @@ public String propertyNameAuthenticate() {
 		}
 	},
 	/**
-	 * SMTPS / SSL transport strategy, that returns the ".smtps." variation of the SMTP_PLAIN version. Additionally the transport protocol is
-	 * explicitly set to smtps. Finally, he property "mail.smtps.quitwait" is set to false, to get rid of a strange SSL exception:<br>
+	 * SMTP entirely encapsulated by TLS. Commonly known as SMTPS.
 	 * <p>
+	 * Strict validation of server certificates is enabled. Server certificates must be issued 1) by a certificate
+	 * authority in the system trust store; and 2) to a subject matching the identity of the remote SMTP server.
+	 * <p>
+	 * Implementation notes:
+	 * <ul>
+	 *     <li>The transport protocol is explicitly set to {@code smtps}.</li>
+	 *     <li>Only {@code mail.smtps} properties are set.</li>
+	 *     <li>Certificate identity checks are enabled by setting {@code mail.smtp.ssl.checkserveridentity} to {@code true}.</li>
+	 *     <li>
+	 * {@code mail.smtps.quitwait} is set to {@code false} to get rid of a strange SSLException:
 	 * <pre>
 	 * javax.mail.MessagingException: Exception reading response;
 	 * nested exception is:
@@ -76,17 +110,18 @@ public String propertyNameAuthenticate() {
 	 * <blockquote>The mail is sent but the exception is unwanted. The property <em>quitwait</em> means If set to false, the QUIT command is sent and
 	 * the connection is immediately closed. If set to true (the default), causes the transport to wait for the response to the QUIT
 	 * command</blockquote><br> <strong>- <a href="http://www.rgagnon.com/javadetails/java-0570.html">source</a></strong>
+	 *     </li>
+	 * </ul>
 	 */
 	SMTP_SSL {
 		/**
-		 * Here protocol "mail.transport.protocol" is set to "smtps" and the quitwait property "mail.smtps.quitwait" is set to "false".
-		 *
 		 * @see TransportStrategy#SMTP_SSL
 		 */
 		@Override
 		public Properties generateProperties() {
 			final Properties properties = super.generateProperties();
 			properties.put("mail.transport.protocol", "smtps");
+			properties.put("mail.smtps.ssl.checkserveridentity", "true");
 			properties.put("mail.smtps.quitwait", "false");
 			return properties;
 		}
@@ -124,22 +159,33 @@ public String propertyNameAuthenticate() {
 		}
 	},
 	/**
+	 * Plaintext SMTP with a mandatory, authenticated STARTTLS upgrade.
+	 * <p>
 	 * <strong>NOTE: this code is in untested beta state</strong>
 	 * <p>
-	 * Uses standard ".smtp." property names (like {@link TransportStrategy#SMTP_PLAIN}). Additionally the transport protocol is explicitly set to
-	 * smtp. Finally, the property "mail.smtp.starttls.enable" is being set to true.
+	 * Strict validation of server certificates is enabled. Server certificates must be issued 1) by a certificate
+	 * authority in the system trust store; and 2) to a subject matching the identity of the remote SMTP server.
+	 * <p>
+	 * Implementation notes:
+	 * <ul>
+	 *     <li>The transport protocol is explicitly set to {@code smtp}.</li>
+	 *     <li>Only {@code mail.smtp} properties are set.</li>
+	 *     <li>STARTTLS is enabled by setting {@code mail.smtp.starttls.enable} to {@code true}.</li>
+	 *     <li>STARTTLS plaintext fallback is disabled by setting {@code mail.smtp.starttls.required} to {@code true}.</li>
+	 *     <li>Certificate identity checks are enabled by setting {@code mail.smtp.ssl.checkserveridentity} to {@code true}.</li>
+	 * </ul>
 	 */
 	SMTP_TLS {
 		/**
-		 * Here protocol "mail.transport.protocol" is set to "smtp" and the tls property "mail.smtp.starttls.enable" is set to "true".
-		 *
 		 * @see TransportStrategy#SMTP_TLS
 		 */
 		@Override
 		public Properties generateProperties() {
 			final Properties props = super.generateProperties();
 			props.put("mail.transport.protocol", "smtp");
 			props.put("mail.smtp.starttls.enable", "true");
+			props.put("mail.smtp.starttls.required", "true");
+			props.put("mail.smtp.ssl.checkserveridentity", "true");
 			return props;
 		}
 

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

@@ -90,6 +90,8 @@ public void createMailSession_AnonymousProxyConstructor_WithoutConfig()
 		assertThat(session.getProperty("mail.smtp.port")).isEqualTo("25");
 		assertThat(session.getProperty("mail.transport.protocol")).isEqualTo("smtp");
 		assertThat(session.getProperty("mail.smtp.starttls.enable")).isEqualTo("true");
+		assertThat(session.getProperty("mail.smtp.starttls.required")).isEqualTo("true");
+		assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true");
 		assertThat(session.getProperty("mail.smtp.username")).isEqualTo("username smtp");
 		assertThat(session.getProperty("mail.smtp.auth")).isEqualTo("true");
 		assertThat(session.getProperty("mail.smtp.socks.host")).isEqualTo("proxy host");
@@ -112,6 +114,8 @@ public void createMailSession_MaximumConstructor_WithoutConfig()
 		assertThat(session.getProperty("mail.smtp.port")).isEqualTo("25");
 		assertThat(session.getProperty("mail.transport.protocol")).isEqualTo("smtp");
 		assertThat(session.getProperty("mail.smtp.starttls.enable")).isEqualTo("true");
+		assertThat(session.getProperty("mail.smtp.starttls.required")).isEqualTo("true");
+		assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true");
 		assertThat(session.getProperty("mail.smtp.username")).isEqualTo("username smtp");
 		assertThat(session.getProperty("mail.smtp.auth")).isEqualTo("true");
 		// the following two are because authentication is needed, otherwise proxy would be straightworward
@@ -131,6 +135,8 @@ public void createMailSession_MinimalConstructor_WithConfig() {
 		assertThat(session.getProperty("mail.smtp.port")).isEqualTo("25");
 		assertThat(session.getProperty("mail.transport.protocol")).isEqualTo("smtp");
 		assertThat(session.getProperty("mail.smtp.starttls.enable")).isEqualTo("true");
+		assertThat(session.getProperty("mail.smtp.starttls.required")).isEqualTo("true");
+		assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true");
 		assertThat(session.getProperty("mail.smtp.username")).isEqualTo("username smtp");
 		assertThat(session.getProperty("mail.smtp.auth")).isEqualTo("true");
 		// the following two are because authentication is needed, otherwise proxy would be straightworward
@@ -150,6 +156,8 @@ public void createMailSession_MaximumConstructor_WithConfig()
 		assertThat(session.getProperty("mail.smtp.port")).isEqualTo("25");
 		assertThat(session.getProperty("mail.transport.protocol")).isEqualTo("smtp");
 		assertThat(session.getProperty("mail.smtp.starttls.enable")).isEqualTo("true");
+		assertThat(session.getProperty("mail.smtp.starttls.required")).isEqualTo("true");
+		assertThat(session.getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true");
 		assertThat(session.getProperty("mail.smtp.username")).isEqualTo("overridden username smtp");
 		assertThat(session.getProperty("mail.smtp.auth")).isEqualTo("true");
 		// the following two are because authentication is needed, otherwise proxy would be straightworward