classification
Title: Possible bug in smtplib when initial_response_ok=False
Type: behavior Stage: resolved
Components: email, Library (Lib), Tests Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: Dario D'Amico, Mario Colombo, barry, junpengruan, miss-islington, orsenthil, pepoluan, r.david.murray, rahul-kumi, redstone-cold
Priority: normal Keywords: patch

Created on 2016-08-21 00:29 by Dario D'Amico, last changed 2021-04-27 05:03 by orsenthil. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 24118 merged pepoluan, 2021-01-05 14:06
PR 24832 merged miss-islington, 2021-03-12 23:26
PR 24833 merged orsenthil, 2021-03-12 23:38
Messages (17)
msg273255 - (view) Author: Dario D'Amico (Dario D'Amico) Date: 2016-08-21 00:34
oo
msg273256 - (view) Author: Dario D'Amico (Dario D'Amico) Date: 2016-08-21 00:35
I have reasons to believe that smtlib.py does not support AUTH LOGIN well.

My guts feeling are that the auth_login method should be changed into:

    def auth_login(self, challenge=None):
        print("auth_login", challenge)
        """ Authobject to use with LOGIN authentication. Requires self.user and
        self.password to be set."""
        if challenge is None:
            return self.user
        elif challenge == b'Username:':
            return self.user
        elif challenge == b'Password:':
            return self.password

While the if at line 634, in the auth method, should actually be a while,
so that this:

        # If server responds with a challenge, send the response.
        if code == 334:
            challenge = base64.decodebytes(resp)
            response = encode_base64(
                authobject(challenge).encode('ascii'), eol='')
            (code, resp) = self.docmd(response)

is turned into this:

        # If server responds with a challenge, send the response.
        # Note that there may be multiple, sequential challenges.
        while code == 334:
            challenge = base64.decodebytes(resp)
            response = encode_base64(
                authobject(challenge).encode('ascii'), eol='')
            (code, resp) = self.docmd(response)

First, some background on AUTH LOGIN; based on my understanding of
http://www.fehcom.de/qmail/smtpauth.html there are two possible ways
to authenticate a client using AUTH LOGIN:

Method A
    C: AUTH LOGIN
    S: 334 VXNlcm5hbWU6
    C: <ENCODED_USERNAME>
    S: 334 UGFzc3dvcmQ6
    C: <ENCODED_PASSWORD>

Method B
    C: AUTH LOGIN <ENCODED_USERNAME>
    S: 334 UGFzc3dvcmQ6
    C: <ENCODED_PASSWORD>

The second method saves two round trips because the client sends
the username together with the AUTH LOGIN command. Note that the
strings VXNlcm5hbWU6 and UGFzc3dvcmQ6 are fixed and they are,
respectively, the Base64 encodings of 'Username:' and 'Password:'.

In the following I will detail my experience with smtplib.

Everything begun from this code fragment:

    smtpObj = smtplib.SMTP("smtp.example.com", "25")
    smtpObj.set_debuglevel(2)
    smtpObj.login("noreply@example.com", "chocolaterain")
    smtpObj.sendmail(sender, receivers, message)

The debug log produced by smtplib looked like this:

01:53:32.420185 send: 'ehlo localhost.localdomain\r\n'
01:53:32.624123 reply: b'250-smtp.example.com\r\n'
01:53:32.862965 reply: b'250-AUTH LOGIN\r\n'
01:53:32.863490 reply: b'250 8BITMIME\r\n'
01:53:32.863844 reply: retcode (250); Msg: b'smtp.example.com\nAUTH LOGIN\n8BITMIME'
01:53:32.868414 send: 'AUTH LOGIN <<<ENCODED_USERNAME>>>\r\n'
01:53:33.069884 reply: b'501 syntax error\r\n'
01:53:33.070479 reply: retcode (501); Msg: b'syntax error'
Traceback (most recent call last):
  File "/usr/lib/python3.5/runpy.py", line 184, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.5/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/dario/Programming/DigitalOcean/s.py", line 48, in <module>
    smtpObj.login("noreply@example.com", "chocolaterain")
  File "/usr/lib/python3.5/smtplib.py", line 729, in login
    raise last_exception
  File "/usr/lib/python3.5/smtplib.py", line 720, in login
    initial_response_ok=initial_response_ok)
  File "/usr/lib/python3.5/smtplib.py", line 641, in auth
    raise SMTPAuthenticationError(code, resp)
smtplib.SMTPAuthenticationError: (501, b'syntax error')

This is most likely not an issue with smtplib, but simply an
indication that smtp.example.com does not support receiving the
username together with AUTH LOGIN (method B), and it replies with 501 syntax error.

I figured out that I could force the alternate data flow (method A), in which
the username is issued in a separate command, by setting
initial_response_ok=False when logging in:

    smtpObj = smtplib.SMTP("smtp.example.com", "25")
    smtpObj.set_debuglevel(2)
    smtpObj.login("noreply@example.com", "chocolaterain", initial_response_ok=False)
    smtpObj.sendmail(sender, receivers, message)

This resulted in a slightly more interesting behaviour:
    
01:53:54.445118 send: 'ehlo localhost.localdomain\r\n'
01:53:54.648136 reply: b'250-smtp.example.com\r\n'
01:53:54.884669 reply: b'250-AUTH LOGIN\r\n'
01:53:54.885197 reply: b'250 8BITMIME\r\n'
01:53:54.885555 reply: retcode (250); Msg: b'smtp.example.com\nAUTH LOGIN\n8BITMIME'
01:53:54.890051 send: 'AUTH LOGIN\r\n'
01:53:55.089540 reply: b'334 VXNlcm5hbWU6\r\n'
01:53:55.090119 reply: retcode (334); Msg: b'VXNlcm5hbWU6'
01:53:55.090955 send: '<<<ENCODED_PASSWORD>>>=\r\n'
01:53:55.296243 reply: b'334 UGFzc3dvcmQ6\r\n'
01:53:55.296717 reply: retcode (334); Msg: b'UGFzc3dvcmQ6'
Traceback (most recent call last):
  File "/usr/lib/python3.5/runpy.py", line 184, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.5/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/dario/Programming/DigitalOcean/s.py", line 57, in <module>
    smtpObj.login("noreply@example.com", "16226464", initial_response_ok=False)
  File "/usr/lib/python3.5/smtplib.py", line 729, in login
    raise last_exception
  File "/usr/lib/python3.5/smtplib.py", line 720, in login
    initial_response_ok=initial_response_ok)
  File "/usr/lib/python3.5/smtplib.py", line 641, in auth
    raise SMTPAuthenticationError(code, resp)
smtplib.SMTPAuthenticationError: (334, b'UGFzc3dvcmQ6')

The above log shows two issues:

- When server says '334 VXNlcm5hbWU6' is actually asking for the
  username, and instead the client replies with the password.
- The above would be enough to make the authentication fail, but
  there is more; when the server asks for the password,
  `334 UGFzc3dvcmQ6`, the client remains silent.
  
Due to the way this method is written, the final (code, resp),
which is (334, b'UGFzc3dvcmQ6') is raised as an error; but in
reality is just another challenge from the server.

The fix is in two part:

 - As of now, method auth_login is called twice only
   if initial_response_ok is set. The first time without challenge
   from line 627:
   
       initial_response = (authobject() if initial_response_ok else None)
       
   The second time with a challenge from line 636-637:
   
       response = encode_base64(
           authobject(challenge).encode('ascii'), eol='')
           
   auth_login will return the username the first time and the
   password the second time. Everything works. If the server
   supports the client sending the initial response and the
   client does so, everything works properly.
   
   But if initial_response_ok is False, the server will first
   ask for the username and, as shown before, the client will
   reply with the password. This is wrong and can be fixed by
   explicitly checking what the challenge is:
   
       def auth_login(self, challenge=None):
           print("auth_login", challenge)
           """ Authobject to use with LOGIN authentication. Requires self.user and
           self.password to be set."""
           if challenge is None:
               return self.user
           elif challenge == b'Username:':
               return self.user
           elif challenge == b'Password:':
               return self.password

 - Second thing to alter is the fact that if the server keeps
   issuing challenge, the client must respond. This happens
   because if initial_response_ok is False, the server will
   issue '334 VXNlcm5hbWU6' and '334 UGFzc3dvcmQ6' separately,
   and the client must solve both challenges. This is an easy fix
   as it simly amount to turning an if into a while:
               
   From:
   
        # If server responds with a challenge, send the response.
        if code == 334:
            challenge = base64.decodebytes(resp)
            response = encode_base64(
                authobject(challenge).encode('ascii'), eol='')
            (code, resp) = self.docmd(response)

   to:

        # If server responds with a challenge, send the response.
        # Note that there may be multiple, sequential challenges.
        while code == 334:
            challenge = base64.decodebytes(resp)
            response = encode_base64(
                authobject(challenge).encode('ascii'), eol='')
            (code, resp) = self.docmd(response)

With the two changes discussed above login works also for servers
that do not support initial_response_ok=True.
            
01:54:42.256276 send: 'ehlo localhost.localdomain\r\n'
01:54:42.458961 reply: b'250-smtp.example.com\r\n'
01:54:42.697463 reply: b'250-AUTH LOGIN\r\n'
01:54:42.697769 reply: b'250 8BITMIME\r\n'
01:54:42.697962 reply: retcode (250); Msg: b'smtp.example.com\nAUTH LOGIN\n8BITMIME'
01:54:42.700297 send: 'AUTH LOGIN\r\n'
01:54:42.902224 reply: b'334 VXNlcm5hbWU6\r\n'
01:54:42.902728 reply: retcode (334); Msg: b'VXNlcm5hbWU6'
01:54:42.903890 send: '<<<ENCODED_USERNAME>>>\r\n'
01:54:43.109534 reply: b'334 UGFzc3dvcmQ6\r\n'
01:54:43.109899 reply: retcode (334); Msg: b'UGFzc3dvcmQ6'
01:54:43.110521 send: '<<<ENCODED_PASSWORD>>>\r\n'
01:54:43.315055 reply: b'235 authentication successful\r\n'
01:54:43.315417 reply: retcode (235); Msg: b'authentication successful'
msg273405 - (view) Author: Mario Colombo (Mario Colombo) Date: 2016-08-22 21:31
Yes, this (or something similar) totally bit me, when for another unrelated reason 'AUTH PLAIN' authentication failed:

https://gist.github.com/macolo/bf2811c14d985d013dda0741bfd339e0

Python then tries auth_login, but doesn't send 'AUTH LOGIN' to the mail server. The second auth method also fails.
msg325234 - (view) Author: iMath (redstone-cold) Date: 2018-09-13 05:54
I encountered the same issue ,  Dario D'Amico's changing works ! please fix the problem !
msg382955 - (view) Author: Pandu E POLUAN (pepoluan) * Date: 2020-12-14 06:48
Hi, I'm one of the maintainers of aio-libs/aiosmtpd.

This issue also bit me when trying to write unit tests for aio-libs/aiosmtpd AUTH implementation

But I partially disagree with Dario D'Amico's changes, specifically the suggested change in the auth_login() method.

According to draft-murchison-sasl-login-00.txt [1], the two challenges sent by the server SHOULD be ignored. The example in that document uses b"VXNlciBOYW1lAA==" and b"UGFzc3dvcmQA" (b64 of b"User Name\x00" and b"Password\x00", respectively), and this is what we have implemented in aio-libs/aiosmtpd.

Furthermore, the same document never indicated that username may be sent along with "AUTH LOGIN", so we haven't implemented that in aio-libs/aiosmtpd.

So rather than hardcoding the challenges to b"Username:" and b"Password:", a compliant SMTP client must instead _count_ the number of challenges it received.

I propose the following changes instead:

    def auth(self, mechanism, authobject, *, initial_response_ok=True):
        ... snip ...
        if initial_response is not None:
            response = encode_base64(initial_response.encode('ascii'), eol='')
            (code, resp) = self.docmd("AUTH", mechanism + " " + response)
            self._challenge_count = 1
        else:
            (code, resp) = self.docmd("AUTH", mechanism)
            self._challenge_count = 0
        # If server responds with a challenge, send the response.
        while code == 334:
            self._challenge_count += 1
            challenge = base64.decodebytes(resp)
        ... snip ...

    ... snip ...

    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 or self._challenge_count < 2:
            return self.user
        else:
            return self.password


[1] https://www.ietf.org/archive/id/draft-murchison-sasl-login-00.txt
msg382956 - (view) Author: Pandu E POLUAN (pepoluan) * Date: 2020-12-14 06:50
This issue is still a bug for Python 3.6 and Python 3.8

I haven't checked on Python 3.7 and Python 3.9
msg384081 - (view) Author: Pandu E POLUAN (pepoluan) * Date: 2020-12-30 18:18
I tried creating a PR, but for the life of me I couldn't wrap my head around how testAUTH_LOGIN is being performed (it's in Lib/test/test_smtplib.py)

All I know is, the test doesn't AT ALL test for situations where initial_response_ok=False. ALL tests are done with initial_response_ok=True.

There needs to be a whole set of additions to test_smtplib.py
msg384108 - (view) Author: Pandu E POLUAN (pepoluan) * Date: 2020-12-31 11:09
I tried adding the code below to test_smtplib.py:

    def testAUTH_LOGIN_initial_response_notok(self):
        self.serv.add_feature("AUTH LOGIN")
        smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost',
                            timeout=support.LOOPBACK_TIMEOUT)
        resp = smtp.login(sim_auth[0], sim_auth[1], initial_response_ok=False)
        self.assertEqual(resp, (235, b'Authentication Succeeded'))
        smtp.close()

and I ended up with:

======================================================================
ERROR: testAUTH_LOGIN_initial_response_notok (test.test_smtplib.SMTPSimTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pepoluan/projects/cpython/Lib/test/test_smtplib.py", line 1065, in testAUTH_LOGIN_initial_response_notok
    resp = smtp.login(sim_auth[0], sim_auth[1], initial_response_ok=False)
  File "/home/pepoluan/projects/cpython/Lib/smtplib.py", line 738, in login
    raise last_exception
  File "/home/pepoluan/projects/cpython/Lib/smtplib.py", line 727, in login
    (code, resp) = self.auth(
  File "/home/pepoluan/projects/cpython/Lib/smtplib.py", line 650, in auth
    raise SMTPAuthenticationError(code, resp)
smtplib.SMTPAuthenticationError: (451, b'Internal confusion')

----------------------------------------------------------------------
msg384368 - (view) Author: Pandu E POLUAN (pepoluan) * Date: 2021-01-05 06:37
Okay, I finally figured out what's wrong.

This piece of code in `test_smtplib.py`:

        if self.smtp_state == self.AUTH:
            line = self._emptystring.join(self.received_lines)
            print('Data:', repr(line), file=smtpd.DEBUGSTREAM)
            self.received_lines = []
            try:
                self.auth_object(line)
            except ResponseException as e:
                self.smtp_state = self.COMMAND
                self.push('%s %s' % (e.smtp_code, e.smtp_error))
                return

The last "return" is over-indented.
msg387708 - (view) Author: Pandu E POLUAN (pepoluan) * Date: 2021-02-26 04:57
PR available on GitHub and it's already more than one month since the PR was submitted, so I'm pinging this issue.
msg388336 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2021-03-09 09:11
Hello Pandu,

Thank you for this patch and the explanation. Does client blocking on repeated challenge from the server (using of while loop) look okay here? 
The conversation here indicates to me that it is fine. Is there any recommendation or implementation strategies to break the loop (on a malformed server)?

Thanks,
Senthil
msg388408 - (view) Author: Pandu E POLUAN (pepoluan) * Date: 2021-03-10 07:17
Hi Senthil,

You're right, it does need a guard. According to my knowledge there is no AUTH mechanism that will send more than 3 challenges; they should fail afterwards with 535 or similar. Servers that don't do that should be considered buggy/broken.

So I've pushed a commit to the GH PR that limits the challenge to 5 times, after which it will raise SMTPException. This will protect users of smtplib.SMTP from being trapped by a buggy/broken server.


Rgds,
--
msg388566 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2021-03-12 23:25
New changeset 7591d9455eb37525c832da3d65e1a7b3e6dbf613 by Pandu E POLUAN in branch 'master':
bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP (GH-24118)
https://github.com/python/cpython/commit/7591d9455eb37525c832da3d65e1a7b3e6dbf613
msg388571 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2021-03-13 00:15
New changeset 32717b982d3347e30ae53eb434e2a32e0d03d51e by Miss Islington (bot) in branch '3.9':
bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP (GH-24118) (#24832)
https://github.com/python/cpython/commit/32717b982d3347e30ae53eb434e2a32e0d03d51e
msg388575 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2021-03-13 00:53
New changeset 8cadc2c9cacfa1710cb5ca28a70f7782cacf09aa by Senthil Kumaran in branch '3.8':
[3.8] bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP (GH-24118) (#24833)
https://github.com/python/cpython/commit/8cadc2c9cacfa1710cb5ca28a70f7782cacf09aa
msg392025 - (view) Author: junpengruan (junpengruan) Date: 2021-04-27 05:02
Hi
I think there is another bug when initial_response_ok=False. When using AUTH PLAIN, the server will response like:
------------------
C: AUTH PLAIN
S: 334 ok. go on
------------------
and it's not base64 encoding, while in the auth() it will decode the resp(here is "ok, go on") which will cause a binascii.Error:

Traceback (most recent call last):
  File "/usr/lib/python3.6/smtplib.py", line 644, in auth
    challenge = base64.decodebytes(resp)
  File "/usr/lib/python3.6/base64.py", line 553, in decodebytes
    return binascii.a2b_base64(s)
binascii.Error: Incorrect padding

I think this fit the title "a bug in smtplib when initial_response_ok=False", should I just comment on this issue or open a new issue?
Thanks!
msg392026 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2021-04-27 05:03
Please open a new issue. It has better chances of being fixed quickly.

On Mon, Apr 26, 2021 at 10:02 PM junpengruan <report@bugs.python.org> wrote:

>
> junpengruan <632077280@qq.com> added the comment:
>
> Hi
> I think there is another bug when initial_response_ok=False. When using
> AUTH PLAIN, the server will response like:
> ------------------
> C: AUTH PLAIN
> S: 334 ok. go on
> ------------------
> and it's not base64 encoding, while in the auth() it will decode the
> resp(here is "ok, go on") which will cause a binascii.Error:
>
> Traceback (most recent call last):
>   File "/usr/lib/python3.6/smtplib.py", line 644, in auth
>     challenge = base64.decodebytes(resp)
>   File "/usr/lib/python3.6/base64.py", line 553, in decodebytes
>     return binascii.a2b_base64(s)
> binascii.Error: Incorrect padding
>
> I think this fit the title "a bug in smtplib when
> initial_response_ok=False", should I just comment on this issue or open a
> new issue?
> Thanks!
>
> ----------
> nosy: +junpengruan
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue27820>
> _______________________________________
>
History
Date User Action Args
2021-04-27 05:03:58orsenthilsetmessages: + msg392026
2021-04-27 05:02:52junpengruansetnosy: + junpengruan
messages: + msg392025
2021-03-13 00:53:52orsenthilsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-03-13 00:53:22orsenthilsetmessages: + msg388575
2021-03-13 00:15:33orsenthilsetmessages: + msg388571
2021-03-12 23:38:07orsenthilsetpull_requests: + pull_request23599
2021-03-12 23:26:14miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request23598
2021-03-12 23:25:54orsenthilsetmessages: + msg388566
2021-03-10 07:17:29pepoluansetmessages: + msg388408
2021-03-09 09:11:01orsenthilsetversions: + Python 3.9, Python 3.10, - Python 3.5, Python 3.6
nosy: + orsenthil

messages: + msg388336

assignee: orsenthil
2021-02-26 04:57:42pepoluansetmessages: + msg387708
2021-01-12 04:17:03pepoluansetcomponents: + Tests
2021-01-05 14:06:11pepoluansetkeywords: + patch
stage: patch review
pull_requests: + pull_request22948
2021-01-05 06:37:22pepoluansetmessages: + msg384368
2020-12-31 11:09:27pepoluansetmessages: + msg384108
2020-12-30 18:18:46pepoluansetmessages: + msg384081
2020-12-22 14:23:46rahul-kumisetnosy: + rahul-kumi
2020-12-14 06:50:33pepoluansetmessages: + msg382956
versions: + Python 3.6, Python 3.8
2020-12-14 06:48:15pepoluansetnosy: + pepoluan
messages: + msg382955
2018-09-13 05:54:03redstone-coldsetnosy: + redstone-cold
messages: + msg325234
2018-07-11 07:44:41serhiy.storchakasettype: crash -> behavior
2016-08-22 21:31:18Mario Colombosetnosy: + Mario Colombo
messages: + msg273405
2016-08-21 05:39:43r.david.murraysetnosy: + barry, r.david.murray
components: + email
2016-08-21 00:35:21Dario D'Amicosetmessages: + msg273256
2016-08-21 00:34:58Dario D'Amicosetmessages: + msg273255
2016-08-21 00:30:09Dario D'Amicosetfiles: - SmtplibBugReport.txt
2016-08-21 00:29:36Dario D'Amicosetfiles: + SmtplibBugReport.txt
2016-08-21 00:29:10Dario D'Amicocreate