Close SMTP connections that failed to open properly

This commit is contained in:
Philip Whitehouse 2017-03-02 20:22:53 +00:00
parent 47b86f6146
commit 3670f94424
2 changed files with 89 additions and 64 deletions

View file

@ -290,7 +290,7 @@ public class SmtpTransport extends Transport {
} }
} }
Map<String,String> extensions = sendHello(localHost); Map<String, String> extensions = sendHello(localHost);
m8bitEncodingAllowed = extensions.containsKey("8BITMIME"); m8bitEncodingAllowed = extensions.containsKey("8BITMIME");
@ -306,7 +306,7 @@ public class SmtpTransport extends Transport {
mClientCertificateAlias); mClientCertificateAlias);
mIn = new PeekableInputStream(new BufferedInputStream(mSocket.getInputStream(), mIn = new PeekableInputStream(new BufferedInputStream(mSocket.getInputStream(),
1024)); 1024));
mOut = new BufferedOutputStream(mSocket.getOutputStream(), 1024); mOut = new BufferedOutputStream(mSocket.getOutputStream(), 1024);
/* /*
* Now resend the EHLO. Required by RFC2487 Sec. 5.2, and more specifically, * Now resend the EHLO. Required by RFC2487 Sec. 5.2, and more specifically,
@ -344,8 +344,8 @@ public class SmtpTransport extends Transport {
if (!TextUtils.isEmpty(mUsername) if (!TextUtils.isEmpty(mUsername)
&& (!TextUtils.isEmpty(mPassword) || && (!TextUtils.isEmpty(mPassword) ||
AuthType.EXTERNAL == mAuthType || AuthType.EXTERNAL == mAuthType ||
AuthType.XOAUTH2 == mAuthType)) { AuthType.XOAUTH2 == mAuthType)) {
switch (mAuthType) { switch (mAuthType) {
@ -354,36 +354,37 @@ public class SmtpTransport extends Transport {
* but it still may exist in a user's settings from a previous * but it still may exist in a user's settings from a previous
* version, or it may have been imported. * version, or it may have been imported.
*/ */
case LOGIN: case LOGIN:
case PLAIN: case PLAIN:
// try saslAuthPlain first, because it supports UTF-8 explicitly // try saslAuthPlain first, because it supports UTF-8 explicitly
if (authPlainSupported) { if (authPlainSupported) {
saslAuthPlain(mUsername, mPassword); saslAuthPlain(mUsername, mPassword);
} else if (authLoginSupported) { } else if (authLoginSupported) {
saslAuthLogin(mUsername, mPassword); saslAuthLogin(mUsername, mPassword);
} else { } else {
throw new MessagingException("Authentication methods SASL PLAIN and LOGIN are unavailable."); throw new MessagingException(
} "Authentication methods SASL PLAIN and LOGIN are unavailable.");
break; }
break;
case CRAM_MD5: case CRAM_MD5:
if (authCramMD5Supported) { if (authCramMD5Supported) {
saslAuthCramMD5(mUsername, mPassword); saslAuthCramMD5(mUsername, mPassword);
} else { } else {
throw new MessagingException("Authentication method CRAM-MD5 is unavailable."); throw new MessagingException("Authentication method CRAM-MD5 is unavailable.");
} }
break; break;
case XOAUTH2: case XOAUTH2:
if (authXoauth2Supported && oauthTokenProvider != null) { if (authXoauth2Supported && oauthTokenProvider != null) {
saslXoauth2(mUsername); saslXoauth2(mUsername);
} else { } else {
throw new MessagingException("Authentication method XOAUTH2 is unavailable."); throw new MessagingException("Authentication method XOAUTH2 is unavailable.");
} }
break; break;
case EXTERNAL: case EXTERNAL:
if (authExternalSupported) { if (authExternalSupported) {
saslAuthExternal(mUsername); saslAuthExternal(mUsername);
} else { } else {
/* /*
* Some SMTP servers are known to provide no error * Some SMTP servers are known to provide no error
* indication when a client certificate fails to * indication when a client certificate fails to
@ -394,53 +395,60 @@ public class SmtpTransport extends Transport {
* EXTERNAL when using client certificates. That way, the * EXTERNAL when using client certificates. That way, the
* user can be notified of a problem during account setup. * user can be notified of a problem during account setup.
*/ */
throw new CertificateValidationException(MissingCapability); throw new CertificateValidationException(MissingCapability);
} }
break; break;
/* /*
* AUTOMATIC is an obsolete option which is unavailable to users, * AUTOMATIC is an obsolete option which is unavailable to users,
* but it still may exist in a user's settings from a previous * but it still may exist in a user's settings from a previous
* version, or it may have been imported. * version, or it may have been imported.
*/ */
case AUTOMATIC: case AUTOMATIC:
if (secureConnection) { if (secureConnection) {
// try saslAuthPlain first, because it supports UTF-8 explicitly // try saslAuthPlain first, because it supports UTF-8 explicitly
if (authPlainSupported) { if (authPlainSupported) {
saslAuthPlain(mUsername, mPassword); saslAuthPlain(mUsername, mPassword);
} else if (authLoginSupported) { } else if (authLoginSupported) {
saslAuthLogin(mUsername, mPassword); saslAuthLogin(mUsername, mPassword);
} else if (authCramMD5Supported) { } else if (authCramMD5Supported) {
saslAuthCramMD5(mUsername, mPassword); saslAuthCramMD5(mUsername, mPassword);
} else { } else {
throw new MessagingException("No supported authentication methods available."); throw new MessagingException("No supported authentication methods available.");
} }
} else {
if (authCramMD5Supported) {
saslAuthCramMD5(mUsername, mPassword);
} else { } else {
if (authCramMD5Supported) {
saslAuthCramMD5(mUsername, mPassword);
} else {
/* /*
* We refuse to insecurely transmit the password * We refuse to insecurely transmit the password
* using the obsolete AUTOMATIC setting because of * using the obsolete AUTOMATIC setting because of
* the potential for a MITM attack. Affected users * the potential for a MITM attack. Affected users
* must choose a different setting. * must choose a different setting.
*/ */
throw new MessagingException( throw new MessagingException(
"Update your outgoing server authentication setting. AUTOMATIC auth. is unavailable."); "Update your outgoing server authentication setting. AUTOMATIC auth. is unavailable.");
}
} }
} break;
break;
default: default:
throw new MessagingException("Unhandled authentication method found in the server settings (bug)."); throw new MessagingException(
"Unhandled authentication method found in the server settings (bug).");
} }
} }
} catch (MessagingException e) {
close();
throw e;
} catch (SSLException e) { } catch (SSLException e) {
close();
throw new CertificateValidationException(e.getMessage(), e); throw new CertificateValidationException(e.getMessage(), e);
} catch (GeneralSecurityException gse) { } catch (GeneralSecurityException gse) {
close();
throw new MessagingException( throw new MessagingException(
"Unable to open connection to SMTP server due to security error.", gse); "Unable to open connection to SMTP server due to security error.", gse);
} catch (IOException ioe) { } catch (IOException ioe) {
close();
throw new MessagingException("Unable to open connection to SMTP server.", ioe); throw new MessagingException("Unable to open connection to SMTP server.", ioe);
} }
} }

View file

@ -133,6 +133,8 @@ public class SmtpTransportTest {
server.expect("EHLO localhost"); server.expect("EHLO localhost");
server.output("250-localhost Hello client.localhost"); server.output("250-localhost Hello client.localhost");
server.output("250 AUTH"); server.output("250 AUTH");
server.expect("QUIT");
server.output("221 BYE");
SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.PLAIN, ConnectionSecurity.NONE); SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.PLAIN, ConnectionSecurity.NONE);
try { try {
@ -172,6 +174,8 @@ public class SmtpTransportTest {
server.expect("EHLO localhost"); server.expect("EHLO localhost");
server.output("250-localhost Hello client.localhost"); server.output("250-localhost Hello client.localhost");
server.output("250 AUTH PLAIN LOGIN"); server.output("250 AUTH PLAIN LOGIN");
server.expect("QUIT");
server.output("221 BYE");
SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.CRAM_MD5, ConnectionSecurity.NONE); SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.CRAM_MD5, ConnectionSecurity.NONE);
try { try {
@ -181,7 +185,7 @@ public class SmtpTransportTest {
assertEquals("Authentication method CRAM-MD5 is unavailable.", e.getMessage()); assertEquals("Authentication method CRAM-MD5 is unavailable.", e.getMessage());
} }
server.verifyConnectionStillOpen(); server.verifyConnectionClosed();
server.verifyInteractionCompleted(); server.verifyInteractionCompleted();
} }
@ -214,6 +218,8 @@ public class SmtpTransportTest {
server.expect(""); server.expect("");
server.output("535-5.7.1 Username and Password not accepted. Learn more at"); server.output("535-5.7.1 Username and Password not accepted. Learn more at");
server.output("535 5.7.1 http://support.google.com/mail/bin/answer.py?answer=14257 hx9sm5317360pbc.68"); server.output("535 5.7.1 http://support.google.com/mail/bin/answer.py?answer=14257 hx9sm5317360pbc.68");
server.expect("QUIT");
server.output("221 BYE");
SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.XOAUTH2, ConnectionSecurity.NONE); SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.XOAUTH2, ConnectionSecurity.NONE);
try { try {
@ -228,7 +234,7 @@ public class SmtpTransportTest {
InOrder inOrder = inOrder(oAuth2TokenProvider); InOrder inOrder = inOrder(oAuth2TokenProvider);
inOrder.verify(oAuth2TokenProvider).getToken(eq(USERNAME), anyInt()); inOrder.verify(oAuth2TokenProvider).getToken(eq(USERNAME), anyInt());
inOrder.verify(oAuth2TokenProvider).invalidateToken(USERNAME); inOrder.verify(oAuth2TokenProvider).invalidateToken(USERNAME);
server.verifyConnectionStillOpen(); server.verifyConnectionClosed();
server.verifyInteractionCompleted(); server.verifyInteractionCompleted();
} }
@ -327,6 +333,9 @@ public class SmtpTransportTest {
server.expect(""); server.expect("");
server.output("535-5.7.1 Username and Password not accepted. Learn more at"); server.output("535-5.7.1 Username and Password not accepted. Learn more at");
server.output("535 5.7.1 http://support.google.com/mail/bin/answer.py?answer=14257 hx9sm5317360pbc.68"); server.output("535 5.7.1 http://support.google.com/mail/bin/answer.py?answer=14257 hx9sm5317360pbc.68");
server.expect("QUIT");
server.output("221 BYE");
SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.XOAUTH2, ConnectionSecurity.NONE); SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.XOAUTH2, ConnectionSecurity.NONE);
try { try {
@ -338,7 +347,7 @@ public class SmtpTransportTest {
e.getMessage()); e.getMessage());
} }
server.verifyConnectionStillOpen(); server.verifyConnectionClosed();
server.verifyInteractionCompleted(); server.verifyInteractionCompleted();
} }
@ -349,6 +358,8 @@ public class SmtpTransportTest {
server.expect("EHLO localhost"); server.expect("EHLO localhost");
server.output("250-localhost Hello client.localhost"); server.output("250-localhost Hello client.localhost");
server.output("250 AUTH XOAUTH2"); server.output("250 AUTH XOAUTH2");
server.expect("QUIT");
server.output("221 BYE");
when(oAuth2TokenProvider.getToken(anyString(), anyInt())).thenThrow(new AuthenticationFailedException("Failed to fetch token")); when(oAuth2TokenProvider.getToken(anyString(), anyInt())).thenThrow(new AuthenticationFailedException("Failed to fetch token"));
SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.XOAUTH2, ConnectionSecurity.NONE); SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.XOAUTH2, ConnectionSecurity.NONE);
@ -359,7 +370,7 @@ public class SmtpTransportTest {
assertEquals("Failed to fetch token", e.getMessage()); assertEquals("Failed to fetch token", e.getMessage());
} }
server.verifyConnectionStillOpen(); server.verifyConnectionClosed();
server.verifyInteractionCompleted(); server.verifyInteractionCompleted();
} }
@ -371,6 +382,8 @@ public class SmtpTransportTest {
server.expect("EHLO localhost"); server.expect("EHLO localhost");
server.output("250-localhost Hello client.localhost"); server.output("250-localhost Hello client.localhost");
server.output("250 AUTH PLAIN LOGIN"); server.output("250 AUTH PLAIN LOGIN");
server.expect("QUIT");
server.output("221 BYE");
SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.XOAUTH2, ConnectionSecurity.NONE); SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.XOAUTH2, ConnectionSecurity.NONE);
try { try {
@ -380,7 +393,7 @@ public class SmtpTransportTest {
assertEquals("Authentication method XOAUTH2 is unavailable.", e.getMessage()); assertEquals("Authentication method XOAUTH2 is unavailable.", e.getMessage());
} }
server.verifyConnectionStillOpen(); server.verifyConnectionClosed();
server.verifyInteractionCompleted(); server.verifyInteractionCompleted();
} }
@ -408,6 +421,8 @@ public class SmtpTransportTest {
server.expect("EHLO localhost"); server.expect("EHLO localhost");
server.output("250-localhost Hello client.localhost"); server.output("250-localhost Hello client.localhost");
server.output("250 AUTH"); server.output("250 AUTH");
server.expect("QUIT");
server.output("221 BYE");
SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.EXTERNAL, ConnectionSecurity.NONE); SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.EXTERNAL, ConnectionSecurity.NONE);
try { try {
@ -417,7 +432,7 @@ public class SmtpTransportTest {
assertEquals(CertificateValidationException.Reason.MissingCapability, e.getReason()); assertEquals(CertificateValidationException.Reason.MissingCapability, e.getReason());
} }
server.verifyConnectionStillOpen(); server.verifyConnectionClosed();
server.verifyInteractionCompleted(); server.verifyInteractionCompleted();
} }
@ -449,6 +464,8 @@ public class SmtpTransportTest {
server.expect("EHLO localhost"); server.expect("EHLO localhost");
server.output("250-localhost Hello client.localhost"); server.output("250-localhost Hello client.localhost");
server.output("250 AUTH PLAIN LOGIN"); server.output("250 AUTH PLAIN LOGIN");
server.expect("QUIT");
server.output("221 BYE");
SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.AUTOMATIC, SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.AUTOMATIC,
ConnectionSecurity.NONE); ConnectionSecurity.NONE);
@ -460,7 +477,7 @@ public class SmtpTransportTest {
e.getMessage()); e.getMessage());
} }
server.verifyConnectionStillOpen(); server.verifyConnectionClosed();
server.verifyInteractionCompleted(); server.verifyInteractionCompleted();
} }