[3.8] bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP (GH-24118) (#24833)

* bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP (GH-24118)

* Fix auth_login logic (bpo-27820)

* Also fix a longstanding bug in the SimSMTPChannel.found_terminator() method that causes inability to test
SMTP AUTH with initial_response_ok=False.

(cherry picked from commit 7591d9455eb37525c832da3d65e1a7b3e6dbf613)

* Set timeout to 15 directly.

Co-authored-by: Pandu E POLUAN <pepoluan@gmail.com>

diff --git a/Lib/smtplib.py b/Lib/smtplib.py
index 6513842eb6c6127b4cc1b5c0d9839763e964d4a3..b7a2715cd5899915d66b9504cbbd4283bf1724ed 100755 (executable)
--- a/Lib/smtplib.py
+++ b/Lib/smtplib.py
@@ -64,6 +64,7 @@ SMTP_SSL_PORT = 465
 CRLF = "\r\n"
 bCRLF = b"\r\n"
 _MAXLINE = 8192 # more than 8 times larger than RFC 821, 4.5.3
+_MAXCHALLENGE = 5  # Maximum number of AUTH challenges sent
 
 OLDSTYLE_AUTH = re.compile(r"auth=(.*)", re.I)
 
@@ -248,6 +249,7 @@ class SMTP:
         self.esmtp_features = {}
         self.command_encoding = 'ascii'
         self.source_address = source_address
+        self._auth_challenge_count = 0
 
         if host:
             (code, msg) = self.connect(host, port)
@@ -631,14 +633,23 @@ class SMTP:
         if initial_response is not None:
             response = encode_base64(initial_response.encode('ascii'), eol='')
             (code, resp) = self.docmd("AUTH", mechanism + " " + response)
+            self._auth_challenge_count = 1
         else:
             (code, resp) = self.docmd("AUTH", mechanism)
+            self._auth_challenge_count = 0
         # If server responds with a challenge, send the response.
-        if code == 334:
+        while code == 334:
+            self._auth_challenge_count += 1
             challenge = base64.decodebytes(resp)
             response = encode_base64(
                 authobject(challenge).encode('ascii'), eol='')
             (code, resp) = self.docmd(response)
+            # If server keeps sending challenges, something is wrong.
+            if self._auth_challenge_count > _MAXCHALLENGE:
+                raise SMTPException(
+                    "Server AUTH mechanism infinite loop. Last response: "
+                    + repr((code, resp))
+                )
         if code in (235, 503):
             return (code, resp)
         raise SMTPAuthenticationError(code, resp)
@@ -660,7 +671,7 @@ class SMTP:
     def auth_login(self, challenge=None):
         """ Authobject to use with LOGIN authentication. Requires self.user and
         self.password to be set."""
-        if challenge is None:
+        if challenge is None or self._auth_challenge_count < 2:
             return self.user
         else:
             return self.password

diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py
index d0c9862edea93ffad364cd3648872fe094410cb8..c9205ae7ffa501aa930bdb7741257244d8255e92 100644 (file)
--- a/Lib/test/test_smtplib.py
+++ b/Lib/test/test_smtplib.py
@@ -733,7 +733,7 @@ class SimSMTPChannel(smtpd.SMTPChannel):
             except ResponseException as e:
                 self.smtp_state = self.COMMAND
                 self.push('%s %s' % (e.smtp_code, e.smtp_error))
-                return
+            return
         super().found_terminator()
 
 
@@ -799,6 +799,11 @@ class SimSMTPChannel(smtpd.SMTPChannel):
             self._authenticated(self._auth_login_user, password == sim_auth[1])
             del self._auth_login_user
 
+    def _auth_buggy(self, arg=None):
+        # This AUTH mechanism will 'trap' client in a neverending 334
+        # base64 encoded 'BuGgYbUgGy'
+        self.push('334 QnVHZ1liVWdHeQ==')
+
     def _auth_cram_md5(self, arg=None):
         if arg is None:
             self.push('334 {}'.format(sim_cram_md5_challenge))
@@ -1011,6 +1016,39 @@ class SMTPSimTests(unittest.TestCase):
         self.assertEqual(resp, (235, b'Authentication Succeeded'))
         smtp.close()
 
+    def testAUTH_LOGIN_initial_response_ok(self):
+        self.serv.add_feature("AUTH LOGIN")
+        with smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=15) as smtp:
+            smtp.user, smtp.password = sim_auth
+            smtp.ehlo("test_auth_login")
+            resp = smtp.auth("LOGIN", smtp.auth_login, initial_response_ok=True)
+            self.assertEqual(resp, (235, b'Authentication Succeeded'))
+
+    def testAUTH_LOGIN_initial_response_notok(self):
+        self.serv.add_feature("AUTH LOGIN")
+        with smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=15) as smtp:
+            smtp.user, smtp.password = sim_auth
+            smtp.ehlo("test_auth_login")
+            resp = smtp.auth("LOGIN", smtp.auth_login, initial_response_ok=False)
+            self.assertEqual(resp, (235, b'Authentication Succeeded'))
+
+    def testAUTH_BUGGY(self):
+        self.serv.add_feature("AUTH BUGGY")
+
+        def auth_buggy(challenge=None):
+            self.assertEqual(b"BuGgYbUgGy", challenge)
+            return "\0"
+
+        smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=15)
+        try:
+            smtp.user, smtp.password = sim_auth
+            smtp.ehlo("test_auth_buggy")
+            expect = r"^Server AUTH mechanism infinite loop.*"
+            with self.assertRaisesRegex(smtplib.SMTPException, expect) as cm:
+                smtp.auth("BUGGY", auth_buggy, initial_response_ok=False)
+        finally:
+            smtp.close()
+
     @requires_hashdigest('md5')
     def testAUTH_CRAM_MD5(self):
         self.serv.add_feature("AUTH CRAM-MD5")

diff --git a/Misc/NEWS.d/next/Library/2021-03-10-14-07-44.bpo-27820.Wwdy-r.rst b/Misc/NEWS.d/next/Library/2021-03-10-14-07-44.bpo-27820.Wwdy-r.rst
new file mode 100644 (file)
index 0000000..7f1014d
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2021-03-10-14-07-44.bpo-27820.Wwdy-r.rst
@@ -0,0 +1,8 @@
+Fixed long-standing bug of smtplib.SMTP where doing AUTH LOGIN with
+initial_response_ok=False will fail.
+
+The cause is that SMTP.auth_login _always_ returns a password if provided
+with a challenge string, thus non-compliant with the standard for AUTH
+LOGIN.
+
+Also fixes bug with the test for smtpd.