Skip to content
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

Possible bug in smtplib when initial_response_ok=False #72007

Closed
DarioDAmico mannequin opened this issue Aug 21, 2016 · 17 comments
Closed

Possible bug in smtplib when initial_response_ok=False #72007

DarioDAmico mannequin opened this issue Aug 21, 2016 · 17 comments
Assignees
Labels
3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir tests Tests in the Lib/test dir topic-email type-bug An unexpected behavior, bug, or error

Comments

@DarioDAmico
Copy link
Mannequin

DarioDAmico mannequin commented Aug 21, 2016

BPO 27820
Nosy @warsaw, @orsenthil, @bitdancer, @miss-islington, @rahul-kumi, @pepoluan
PRs
  • bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP #24118
  • [3.9] bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP (GH-24118) #24832
  • [3.8] bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP (GH-24118) #24833
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/orsenthil'
    closed_at = <Date 2021-03-13.00:53:52.065>
    created_at = <Date 2016-08-21.00:29:10.427>
    labels = ['type-bug', '3.8', 'expert-email', '3.10', 'library', 'tests', '3.9']
    title = 'Possible bug in smtplib when initial_response_ok=False'
    updated_at = <Date 2021-04-27.05:03:58.150>
    user = 'https://bugs.python.org/DarioDAmico'

    bugs.python.org fields:

    activity = <Date 2021-04-27.05:03:58.150>
    actor = 'orsenthil'
    assignee = 'orsenthil'
    closed = True
    closed_date = <Date 2021-03-13.00:53:52.065>
    closer = 'orsenthil'
    components = ['Library (Lib)', 'Tests', 'email']
    creation = <Date 2016-08-21.00:29:10.427>
    creator = "Dario D'Amico"
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 27820
    keywords = ['patch']
    message_count = 17.0
    messages = ['273255', '273256', '273405', '325234', '382955', '382956', '384081', '384108', '384368', '387708', '388336', '388408', '388566', '388571', '388575', '392025', '392026']
    nosy_count = 10.0
    nosy_names = ['barry', 'orsenthil', 'r.david.murray', 'redstone-cold', "Dario D'Amico", 'Mario Colombo', 'miss-islington', 'rahul-kumi', 'pepoluan', 'junpengruan']
    pr_nums = ['24118', '24832', '24833']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue27820'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @DarioDAmico DarioDAmico mannequin added type-crash A hard crash of the interpreter, possibly with a core dump stdlib Python modules in the Lib dir labels Aug 21, 2016
    @DarioDAmico
    Copy link
    Mannequin Author

    DarioDAmico mannequin commented Aug 21, 2016

    oo

    @DarioDAmico
    Copy link
    Mannequin Author

    DarioDAmico mannequin commented Aug 21, 2016

    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'

    @MarioColombo
    Copy link
    Mannequin

    MarioColombo mannequin commented Aug 22, 2016

    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.

    @serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 11, 2018
    @redstone-cold
    Copy link
    Mannequin

    redstone-cold mannequin commented Sep 13, 2018

    I encountered the same issue , Dario D'Amico's changing works ! please fix the problem !

    @pepoluan
    Copy link
    Mannequin

    pepoluan mannequin commented Dec 14, 2020

    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

    @pepoluan
    Copy link
    Mannequin

    pepoluan mannequin commented Dec 14, 2020

    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

    @pepoluan pepoluan mannequin added the 3.8 (EOL) end of life label Dec 14, 2020
    @pepoluan
    Copy link
    Mannequin

    pepoluan mannequin commented Dec 30, 2020

    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

    @pepoluan
    Copy link
    Mannequin

    pepoluan mannequin commented Dec 31, 2020

    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')

    @pepoluan
    Copy link
    Mannequin

    pepoluan mannequin commented Jan 5, 2021

    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.

    @pepoluan pepoluan mannequin added the tests Tests in the Lib/test dir label Jan 12, 2021
    @pepoluan
    Copy link
    Mannequin

    pepoluan mannequin commented Feb 26, 2021

    PR available on GitHub and it's already more than one month since the PR was submitted, so I'm pinging this issue.

    @orsenthil
    Copy link
    Member

    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

    @orsenthil orsenthil added 3.9 only security fixes 3.10 only security fixes labels Mar 9, 2021
    @orsenthil orsenthil self-assigned this Mar 9, 2021
    @pepoluan
    Copy link
    Mannequin

    pepoluan mannequin commented Mar 10, 2021

    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,

    @orsenthil
    Copy link
    Member

    New changeset 7591d94 by Pandu E POLUAN in branch 'master':
    bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP (GH-24118)
    7591d94

    @orsenthil
    Copy link
    Member

    New changeset 32717b9 by Miss Islington (bot) in branch '3.9':
    bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP (GH-24118) (bpo-24832)
    32717b9

    @orsenthil
    Copy link
    Member

    New changeset 8cadc2c by Senthil Kumaran in branch '3.8':
    [3.8] bpo-27820: Fix AUTH LOGIN logic in smtplib.SMTP (GH-24118) (bpo-24833)
    8cadc2c

    @junpengruan
    Copy link
    Mannequin

    junpengruan mannequin commented Apr 27, 2021

    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!

    @orsenthil
    Copy link
    Member

    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\>


    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir tests Tests in the Lib/test dir topic-email type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants