Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions Lib/smtplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
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)

Expand Down Expand Up @@ -248,6 +249,7 @@ def __init__(self, host='', port=0, local_hostname=None,
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)
Expand Down Expand Up @@ -631,14 +633,23 @@ def auth(self, mechanism, authobject, *, initial_response_ok=True):
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)
Expand All @@ -660,7 +671,7 @@ def auth_plain(self, challenge=None):
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
Expand Down
40 changes: 39 additions & 1 deletion Lib/test/test_smtplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ def found_terminator(self):
except ResponseException as e:
self.smtp_state = self.COMMAND
self.push('%s %s' % (e.smtp_code, e.smtp_error))
return
return
super().found_terminator()


Expand Down Expand Up @@ -799,6 +799,11 @@ def _auth_login(self, arg=None):
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))
Expand Down Expand Up @@ -1011,6 +1016,39 @@ def testAUTH_LOGIN(self):
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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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.