protocols/smtp: Fix treating auth failure as a permanent failure

A typo in the SMTP configuration would cause messages to be rejected due to the AUTH command being failed. This would result in password typos causing messages to be bounced. This change adds a new temporary error code specifically for authentication failures, which will result in retries (with backoff) instead of bounces. Thanks to Fejes József <[email protected]> for identifying this problem. This also adds tests for SMTP AUTH which had been missing.
bruceg · Jan 17, 2018 · ed9fc96 · ed9fc96
1 parent 8f12d16
commit ed9fc96
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 35 deletions.
diff --git a/NEWS b/NEWS
@@ -1,6 +1,9 @@
 This file lists all the major user-visible changes to nullmailer.
 
 - Fixed compile error in sendmail on GCC older than 4.9.
+
+- Fixed treating authentication failure as message rejection.
+  Thanks Fejes József
 -------------------------------------------------------------------------------
 Changes in version 2.1
 

diff --git a/lib/errcodes.cc b/lib/errcodes.cc
@@ -44,6 +44,7 @@ const char* errorstr(int code)
   case ERR_MSG_REFUSED: return "Server refused the message";
   case ERR_MSG_PERMFAIL: return "Permanent error in sending the message";
   case ERR_BIND_FAILED: return "Failed to bind source address";
+  case ERR_AUTH_FAILED: return "Failed to authenticate to server";
   }
   return (code & ERR_PERMANENT_FLAG)
     ? "Unspecified permanent error"

diff --git a/lib/errcodes.h b/lib/errcodes.h
@@ -20,6 +20,7 @@
 #define ERR_UNKNOWN 17		// Arbitrary error code
 #define ERR_CONFIG 18		// Error reading a config file
 #define ERR_BIND_FAILED 19      // Failed to bind source address
+#define ERR_AUTH_FAILED 20      // Failed to authenticate to server
 
 // Permanent errors
 #define ERR_GHBN_FATAL 33	// gethostbyname failed with NO_RECOVERY

diff --git a/protocols/smtp.cc b/protocols/smtp.cc
@@ -45,8 +45,8 @@ class smtp
   ~smtp();
   int get(mystring& str);
   int put(mystring cmd, mystring& result);
-  void docmd(mystring cmd, int range, mystring& result);
-  void docmd(mystring cmd, int range);
+  void docmd(mystring cmd, int range, mystring& result, int permfail=ERR_MSG_PERMFAIL);
+  void docmd(mystring cmd, int range, int permfail=ERR_MSG_PERMFAIL);
   void dohelo(bool ehlo);
   bool hascap(const char* name, const char* word = NULL);
   void auth_login(void);
@@ -91,7 +91,7 @@ int smtp::put(mystring cmd, mystring& result)
   return get(result);
 }
 
-void smtp::docmd(mystring cmd, int range, mystring& result)
+void smtp::docmd(mystring cmd, int range, mystring& result, int permfail)
 {
   int code;
   if(!cmd)
@@ -101,7 +101,7 @@ void smtp::docmd(mystring cmd, int range, mystring& result)
   if(code < range || code >= (range+100)) {
     int e;
     if(code >= 500)
-      e = ERR_MSG_PERMFAIL;
+      e = permfail;
     else if(code >= 400)
       e = ERR_MSG_TEMPFAIL;
     else
@@ -112,10 +112,10 @@ void smtp::docmd(mystring cmd, int range, mystring& result)
   }
 }
 
-void smtp::docmd(mystring cmd, int range)
+void smtp::docmd(mystring cmd, int range, int permfail)
 {
   mystring msg;
-  docmd(cmd, range, msg);
+  docmd(cmd, range, msg, permfail);
 }
 
 void smtp::dohelo(bool ehlo)
@@ -162,7 +162,7 @@ void smtp::auth_login(void)
 {
   mystring encoded;
   base64_encode(user, encoded);
-  docmd("AUTH LOGIN " + encoded, 300);
+  docmd("AUTH LOGIN " + encoded, 300, ERR_AUTH_FAILED);
   encoded = "";
   base64_encode(pass, encoded);
   docmd(encoded, 200);
@@ -177,7 +177,7 @@ void smtp::auth_plain(void)
   plain += pass;
   mystring encoded = "AUTH PLAIN ";
   base64_encode(plain, encoded);
-  docmd(encoded, 200);
+  docmd(encoded, 200, ERR_AUTH_FAILED);
 }
 
 void smtp::send_envelope(fdibuf& msg)

diff --git a/test/authtest-smtp.sh b/test/authtest-smtp.sh
@@ -0,0 +1,11 @@
+echo 250 ME
+echo 250-ME
+echo 250-AUTH PLAIN
+echo 250 OK
+cat $1
+rm -f $1
+echo 250 OK
+echo 250 OK
+echo 351 OK
+echo 220 OK
+sleep 1
diff --git a/test/functions.in b/test/functions.in
@@ -48,6 +48,10 @@ stop() {
 	wait
     done
 }
+catch-port() {
+    local name=$1
+    port=$( head -n 1 $tmpdir/service/${name}-log )
+}
 
 #not() { if "$@"; then return 1; else return 0; fi }
 not() { ! safe "$@"; }
@@ -128,6 +132,21 @@ splitblank() {
   done
 }
 
+make-testmail() {
+    testmail=$tmpdir/testmail
+    rm -f $testmail
+    cat >$testmail <<EOF
+[email protected]
+[email protected]
+
+From: <[email protected]>
+To: <[email protected]>
+Subject: Nullmailer automated test message
+
+Just testing, please ignore
+EOF
+}
+
 export PATH=/bin:/usr/bin:/usr/local/bin
 rm -f $SYSCONFDIR/*
 echo f.q.d.n >$SYSCONFDIR/me

diff --git a/test/tests/protocols b/test/tests/protocols
@@ -1,69 +1,59 @@
 . functions
 export HELOHOST=f.q.d.n
 
-rm -f testmail
-cat >testmail <<EOF
-[email protected]
-[email protected]
-
-From: <[email protected]>
-To: <[email protected]>
-Subject: Nullmailer automated test message
-
-Just testing, please ignore
-EOF
+make-testmail
 
 for p in smtp qmqp
 do
 	start server "tcpserver -1 ::0 0 sh $srcdir/test/accept-$p.sh"
-	port=$( head -n 1 $tmpdir/service/server-log )
+	catch-port server
 
 	echo "Testing protocol success with $p (command-line)"
-	protocol $p --host=localhost --port=$port 3<testmail
+	protocol $p --host=localhost --port=$port 3<$testmail
 
 	echo "Testing protocol success with $p (stdin)"
-	protocol $p -- host=localhost port=$port 3<testmail
+	protocol $p -- host=localhost port=$port 3<$testmail
 
 	echo "Testing protocol success with $p (stdin, IPv6)"
-	protocol $p -- host=::1 port=$port 3<testmail
+	protocol $p -- host=::1 port=$port 3<$testmail
 
 	echo "Testing protocol success with $p (source addr)"
-	protocol $p -- host=::1 port=$port source=::1 3<testmail
+	protocol $p -- host=::1 port=$port source=::1 3<$testmail
 
 	echo "Testing protocol failure with $p (bad source addr 1)"
-	error 19 protocol $p -- host=::1 port=$port source=127.0.0.1 3<testmail
+	error 19 protocol $p -- host=::1 port=$port source=127.0.0.1 3<$testmail
 
 	echo "Testing protocol failure with $p (bad source addr 2)"
-	error 19 protocol $p -- host=127.0.0.1 port=$port source=::1 3<testmail
+	error 19 protocol $p -- host=127.0.0.1 port=$port source=::1 3<$testmail
 
 	stop server
 
 	echo "Testing host not found error with $p."
-	error 2 protocol $p --host=this.host.can.not.exist 3<testmail
+	error 2 protocol $p --host=this.host.can.not.exist 3<$testmail
 
 	echo "Testing connection refused error with $p."
-	error 7 protocol $p -p $port --host=localhost 3<testmail
+	error 7 protocol $p -p $port --host=localhost 3<$testmail
 
 	echo "Testing connection refused error with $p (IPv6)."
-	error 7 protocol $p -p $port --host=::1 3<testmail
+	error 7 protocol $p -p $port --host=::1 3<$testmail
 
 	echo "Testing usage error with $p (zero args)."
-	error 1 protocol $p 3<testmail
+	error 1 protocol $p 3<$testmail
 
 	echo "Testing usage error with $p (two args)."
-	error 1 protocol $p --host=localhost foobar 3<testmail
+	error 1 protocol $p --host=localhost foobar 3<$testmail
 
 	echo "Testing usage error with $p (unknown option)."
-	error 1 protocol $p -x --host=localhost 3<testmail
+	error 1 protocol $p -x --host=localhost 3<$testmail
 
 	echo "Testing usage error with $p (invalid integer)."
-	error 1 protocol $p -p foo --host=localhost 3<testmail
+	error 1 protocol $p -p foo --host=localhost 3<$testmail
 
 	start server tcpserver -1 0 0 date
 	port=$( head -n 1 $tmpdir/service/server-log )
 	echo "Testing protocol failure with $p."
-	error 11 protocol $p -p $port --host=localhost 3<testmail
+	error 11 protocol $p -p $port --host=localhost 3<$testmail
 	stop server
 done
 
-rm -f testmail
+stop server
diff --git a/test/tests/smtp-auth b/test/tests/smtp-auth
@@ -0,0 +1,25 @@
+. functions
+export HELOHOST=f.q.d.n
+
+make-testmail
+
+start server "tcpserver -1 ::0 0 sh $srcdir/test/authtest-smtp.sh $tmpdir/smtp-result"
+catch-port server
+
+echo 'Testing auth success with smtp'
+echo '250 OK' > $tmpdir/smtp-result
+protocol smtp --host=localhost --port=$port --user=example --pass=example 3<$testmail
+
+echo 'Testing auth login success with smtp'
+echo $'350 Go ahead\n250 AUTH' > $tmpdir/smtp-result
+protocol smtp --host=localhost --port=$port --user=example --pass=example --auth-login 3<$testmail
+
+echo 'Testing auth temporary failure with smtp'
+echo '450 No' > $tmpdir/smtp-result
+error 16 protocol smtp --host=localhost --port $port --user=example --pass=example 3<$testmail
+
+echo 'Testing auth permanent failure with smtp'
+echo '550 No' > $tmpdir/smtp-result
+error 20 protocol smtp --host=localhost --port $port --user=example --pass=example 3<$testmail
+
+stop server