Project: javamail
Repository: mercurial
Revision: 623
Author: shannon
Date: 2013-12-07 00:02:14 UTC
Link:
Log Message:
------------
Properly handle SASL authentication failure for SMTP - bug 6207
Don't fall back to non-SASL methods if SASL mechanism auth fails - bug 6208
Revisions:
----------
622
623
Modified Paths:
---------------
doc/release/CHANGES.txt
mail/src/main/java/com/sun/mail/smtp/SMTPSaslAuthenticator.java
mail/src/test/java/com/sun/mail/smtp/SMTPHandler.java
mail/src/test/java/com/sun/mail/smtp/SMTPSaslLoginTest.java
mail/src/main/java/com/sun/mail/imap/IMAPStore.java
mail/src/main/java/com/sun/mail/imap/protocol/IMAPSaslAuthenticator.java
mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java
Diffs:
------
diff -r 170294344bc0 -r 25cd2a357d78 doc/release/CHANGES.txt
--- a/doc/release/CHANGES.txt Wed Dec 04 12:08:41 2013 -0800
+++ b/doc/release/CHANGES.txt Fri Dec 06 14:00:15 2013 -0800
@@ -22,6 +22,7 @@
K 6181 NullPointerException at IMAPFolder#getMessagesByUID if msg not found
K 6201 add option to use canonical host name for SASL
K 6203 IMAP astring parsing incorrect
+K 6207 SMTP SASL support doesn't handle authentication failure properly
CHANGES IN THE 1.5.1 RELEASE
diff -r 170294344bc0 -r 25cd2a357d78 mail/src/main/java/com/sun/mail/smtp/SMTPSaslAuthenticator.java
--- a/mail/src/main/java/com/sun/mail/smtp/SMTPSaslAuthenticator.java Wed Dec 04 12:08:41 2013 -0800
+++ b/mail/src/main/java/com/sun/mail/smtp/SMTPSaslAuthenticator.java Fri Dec 06 14:00:15 2013 -0800
@@ -203,6 +203,8 @@
// XXX - ultimately return true???
}
}
+ if (resp != 235)
+ return false;
if (sc.isComplete() /*&& res.status == SUCCESS*/) {
String qop = (String)sc.getNegotiatedProperty(Sasl.QOP);
diff -r 170294344bc0 -r 25cd2a357d78 mail/src/test/java/com/sun/mail/smtp/SMTPHandler.java
--- a/mail/src/test/java/com/sun/mail/smtp/SMTPHandler.java Wed Dec 04 12:08:41 2013 -0800
+++ b/mail/src/test/java/com/sun/mail/smtp/SMTPHandler.java Fri Dec 06 14:00:15 2013 -0800
@@ -153,7 +153,8 @@
currentLine = reader.readLine();
if (currentLine == null) {
- LOGGER.severe("Current line is null!");
+ // XXX - often happens when shutting down
+ //LOGGER.severe("Current line is null!");
exit();
return;
}
diff -r 170294344bc0 -r 25cd2a357d78 mail/src/test/java/com/sun/mail/smtp/SMTPSaslLoginTest.java
--- a/mail/src/test/java/com/sun/mail/smtp/SMTPSaslLoginTest.java Wed Dec 04 12:08:41 2013 -0800
+++ b/mail/src/test/java/com/sun/mail/smtp/SMTPSaslLoginTest.java Fri Dec 06 14:00:15 2013 -0800
@@ -45,6 +45,7 @@
import javax.mail.Session;
import javax.mail.Transport;
+import javax.mail.AuthenticationFailedException;
import org.junit.Test;
import static org.junit.Assert.assertTrue;
@@ -56,7 +57,7 @@
public class SMTPSaslLoginTest {
@Test
- public void test() {
+ public void testSuccess() {
SMTPServer server = null;
try {
server = new SMTPServer(new SMTPSaslHandler(), 26423);
@@ -69,6 +70,7 @@
properties.setProperty("mail.smtp.sasl.enable", "true");
properties.setProperty("mail.smtp.sasl.mechanisms", "DIGEST-MD5");
properties.setProperty("mail.smtp.auth.digest-md5.disable", "true");
+ //properties.setProperty("mail.debug.auth", "true");
Session session = Session.getInstance(properties);
//session.setDebug(true);
@@ -77,7 +79,47 @@
t.connect("test", "test");
// success!
} catch (Exception ex) {
- // expect an exception when connect times out
+ fail(ex.toString());
+ } finally {
+ t.close();
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail(e.getMessage());
+ } finally {
+ if (server != null) {
+ server.quit();
+ server.interrupt();
+ }
+ }
+ }
+
+ @Test
+ public void testFailure() {
+ SMTPServer server = null;
+ try {
+ server = new SMTPServer(new SMTPSaslHandler(), 26423);
+ server.start();
+ Thread.sleep(1000);
+
+ Properties properties = new Properties();
+ properties.setProperty("mail.smtp.host", "localhost");
+ properties.setProperty("mail.smtp.port", "26423");
+ properties.setProperty("mail.smtp.sasl.enable", "true");
+ properties.setProperty("mail.smtp.sasl.mechanisms", "DIGEST-MD5");
+ properties.setProperty("mail.smtp.auth.digest-md5.disable", "true");
+ //properties.setProperty("mail.debug.auth", "true");
+ Session session = Session.getInstance(properties);
+ //session.setDebug(true);
+
+ Transport t = session.getTransport("smtp");
+ try {
+ t.connect("test", "xtest");
+ // should have failed
+ fail("wrong password succeeded");
+ } catch (AuthenticationFailedException ex) {
+ // success!
+ } catch (Exception ex) {
fail(ex.toString());
} finally {
t.close();
diff -r 25cd2a357d78 -r 9da4f8cb1d80 doc/release/CHANGES.txt
--- a/doc/release/CHANGES.txt Fri Dec 06 14:00:15 2013 -0800
+++ b/doc/release/CHANGES.txt Fri Dec 06 16:02:14 2013 -0800
@@ -23,6 +23,7 @@
K 6201 add option to use canonical host name for SASL
K 6203 IMAP astring parsing incorrect
K 6207 SMTP SASL support doesn't handle authentication failure properly
+K 6208 SASL authentication failures should not try other methods
CHANGES IN THE 1.5.1 RELEASE
diff -r 25cd2a357d78 -r 9da4f8cb1d80 mail/src/main/java/com/sun/mail/imap/IMAPStore.java
--- a/mail/src/main/java/com/sun/mail/imap/IMAPStore.java Fri Dec 06 14:00:15 2013 -0800
+++ b/mail/src/main/java/com/sun/mail/imap/IMAPStore.java Fri Dec 06 16:02:14 2013 -0800
@@ -777,8 +777,16 @@
else
authzid = null;
- if (enableSASL)
- p.sasllogin(saslMechanisms, saslRealm, authzid, u, pw);
+ if (enableSASL) {
+ try {
+ p.sasllogin(saslMechanisms, saslRealm, authzid, u, pw);
+ if (!p.isAuthenticated())
+ throw new CommandFailedException(
+ "SASL authentication failed");
+ } catch (UnsupportedOperationException ex) {
+ // continue to try other authentication methods below
+ }
+ }
if (p.isAuthenticated())
; // SASL login succeeded, go to bottom
diff -r 25cd2a357d78 -r 9da4f8cb1d80 mail/src/main/java/com/sun/mail/imap/protocol/IMAPSaslAuthenticator.java
--- a/mail/src/main/java/com/sun/mail/imap/protocol/IMAPSaslAuthenticator.java Fri Dec 06 14:00:15 2013 -0800
+++ b/mail/src/main/java/com/sun/mail/imap/protocol/IMAPSaslAuthenticator.java Fri Dec 06 16:02:14 2013 -0800
@@ -132,11 +132,11 @@
(Map)props, cbh);
} catch (SaslException sex) {
logger.log(Level.FINE, "Failed to create SASL client", sex);
- return false;
+ throw new UnsupportedOperationException(sex.getMessage(), sex);
}
if (sc == null) {
logger.fine("No SASL support");
- return false;
+ throw new UnsupportedOperationException("No SASL support");
}
if (logger.isLoggable(Level.FINE))
logger.fine("SASL client " + sc.getMechanismName());
diff -r 25cd2a357d78 -r 9da4f8cb1d80 mail/src/main/java/com/sun/mail/smtp/SMTPSaslAuthenticator.java
--- a/mail/src/main/java/com/sun/mail/smtp/SMTPSaslAuthenticator.java Fri Dec 06 14:00:15 2013 -0800
+++ b/mail/src/main/java/com/sun/mail/smtp/SMTPSaslAuthenticator.java Fri Dec 06 16:02:14 2013 -0800
@@ -126,12 +126,12 @@
sc = Sasl.createSaslClient(mechs, authzid, name, host,
(Map)props, cbh);
} catch (SaslException sex) {
- logger.log(Level.FINE, "Failed to create SASL client: ", sex);
- return false;
+ logger.log(Level.FINE, "Failed to create SASL client", sex);
+ throw new UnsupportedOperationException(sex.getMessage(), sex);
}
if (sc == null) {
logger.fine("No SASL support");
- return false;
+ throw new UnsupportedOperationException("No SASL support");
}
if (logger.isLoggable(Level.FINE))
logger.fine("SASL client " + sc.getMechanismName());
diff -r 25cd2a357d78 -r 9da4f8cb1d80 mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java
--- a/mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java Fri Dec 06 14:00:15 2013 -0800
+++ b/mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java Fri Dec 06 16:02:14 2013 -0800
@@ -763,10 +763,18 @@
authzid = user;
if (enableSASL) {
logger.fine("Authenticate with SASL");
- if (sasllogin(getSASLMechanisms(), getSASLRealm(), authzid,
- user, passwd))
- return true; // success
- logger.fine("SASL authentication failed");
+ try {
+ if (sasllogin(getSASLMechanisms(), getSASLRealm(), authzid,
+ user, passwd)) {
+ return true; // success
+ } else {
+ logger.fine("SASL authentication failed");
+ return false;
+ }
+ } catch (UnsupportedOperationException ex) {
+ logger.log(Level.FINE, "SASL support failed", ex);
+ // if the SASL support fails, fall back to non-SASL
+ }
}
if (logger.isLoggable(Level.FINE))