This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: smtplib.SMTP_SSL leaks socket connections on SSL error
Type: resource usage Stage: patch review
Components: Library (Lib) Versions: Python 3.7, Python 3.6, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: bhm, catalin.iacob, christian.heimes, giampaolo.rodola, jesstess, joeshaw, kasun, pitrou, r.david.murray, zvyn
Priority: normal Keywords: patch

Created on 2011-06-20 19:44 by joeshaw, last changed 2022-04-11 14:57 by admin.

Files
File name Uploaded Description Edit
issue12378_py27.patch bhm, 2014-06-23 23:13 review
issue12378_py35.patch zvyn, 2014-07-22 23:14 review
Messages (10)
msg138753 - (view) Author: Joe Shaw (joeshaw) Date: 2011-06-20 19:44
Start a non-SSL server on port 2525:

$ python -m smtpd -n -c DebuggingServer localhost:2525

In another terminal, fire up a python interpreter and run the following code:

>>> import smtplib
>>> s = smtplib.SMTP_SSL("localhost", 2525)
[...]
ssl.SSLError: [Errno 1] _ssl.c:480: error:140770FC:SSL routines:SSL23_GET_SERVER_HELLO:unknown protocol

The underlying socket connection is still open, but you can't access it or close it:

$ lsof -P -p 76318 | grep 2525
Python  76318 joeshaw    3u  IPv4 0x09a9fb18       0t0      TCP localhost:64328->localhost:2525 (ESTABLISHED)

This wreaks havoc if you're trying to write a unit test using the smtpd module and asyncore in a thread and try to clean up after yourself.

The code inside SMTP_SSL looks something like this (on 2.6.5 anyway):

        def _get_socket(self, host, port, timeout):
            if self.debuglevel > 0: print>>stderr, 'connect:', (host, port)
            new_socket = socket.create_connection((host, port), timeout)
            new_socket = ssl.wrap_socket(new_socket, self.keyfile, self.certfile)
            self.file = SSLFakeFile(new_socket)
            return new_socket

Something like:

            new_socket = socket.create_connection((host, port), timeout)
            try:
                new_socket = ssl.wrap_socket(new_socket, self.keyfile, self.certfile)
            except:
                new_socket.close()
                raise
            self.file = SSLFakeFile(new_socket)
            return new_socket

I think will do the trick.
msg138754 - (view) Author: Joe Shaw (joeshaw) Date: 2011-06-20 19:59
From some experimentation, closing the underlying socket isn't enough.  You also need to close the SSL socket, so you'd need to do something like:


new_socket = socket.create_connection((host, port), timeout)
ssl_socket = ssl.wrap_socket(new_socket, self.keyfile, self.certfile, do_handshake_on_connect=False)
try:
    ssl_socket.do_handshake()
except:
    ssl_socket.close()
    new_socket.close()
    raise
self.file = SSLFakeFile(ssl_socket)
return ssl_socket
msg138764 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-20 22:23
2.6 is in security-fix-only mode.

By inspection the 2.7 and 3.x code have the same issue (though the 3.x code is very different, there still appears to be a lack of error recovery logic.

Joe, do you have any interest in writing a unit test for this?  I believe the necessary infrastructure already exists in test_smtpnet, though I'm not sure.
msg138795 - (view) Author: Catalin Iacob (catalin.iacob) * Date: 2011-06-21 19:52
Just a note after running Joe's example. smtplib is affected by this issue in 3.x, but the example produces different results in 3.x: smtpd doesn't keep the connection open due to UnicodeDecodeError when the data from the ssl handshake cannot be decoded as utf-8:

error: uncaptured python exception, closing channel <__main__.SMTPChannel connected 127.0.0.1:34160 at 0xb739a4cc> (<class 'UnicodeDecodeError'>:'utf8' codec can't decode bytes in position 13-14: invalid continuation byte [/mnt/data/catalin/hacking/cpython/default/Lib/asyncore.py|read|83] [/mnt/data/catalin/hacking/cpython/default/Lib/asyncore.py|handle_read_event|442] [/mnt/data/catalin/hacking/cpython/default/Lib/asynchat.py|handle_read|190] [/mnt/data/catalin/hacking/cpython/default/Lib/smtpd.py|collect_incoming_data|278])
msg221211 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-21 23:45
I don't know where we stand with this as it references asyncore which is deprecated in favour of asynio, can someone please advise.
msg221379 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-23 21:46
@Giampaolo can you add anything to this?
msg221383 - (view) Author: brian morrow (bhm) * Date: 2014-06-23 23:13
Not sure if this is still relevant, but I've supplied a python2.7 patch for this issue. All regression tests still pass and the underlying socket connection is closed:

bmorrow@xorange:~/cpython$ ./python -m smtpd -n -c DebuggingServer localhost:2525

>>> import smtplib
>>> s = smtplib.SMTP_SSL("localhost", 2525)
[...]
ssl.SSLError: [Errno 1] _ssl.c:510: error:140770FC:SSL routines:SSL23_GET_SERVER_HELLO:unknown protocol

bmorrow@xorange:~/cpython$ ps -ef | grep "./python"
bmorrow  19052 19742  0 19:08 pts/17   00:00:00 ./python

bmorrow@xorange:~/cpython$ lsof -P -p 19052 | grep 2525
bmorrow@xorange:~/cpython$ 

bmorrow@xorange:~/cpython$ lsof -P -p 19742 | grep 2525
bmorrow@xorange:~/cpython$
msg221837 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-06-29 12:39
> @Giampaolo can you add anything to this?

As for smptd encoding error I think a reasonable thing to do would be to reply with "501 Can't decode command.". There's no way for the server to return a more specific error message (e.g. "SSL is not supported"). ...And just for clarity/completeness, smtpd module cannot support SSL because asyncore doesn't.
msg223706 - (view) Author: Milan Oberkirch (zvyn) * Date: 2014-07-22 23:14
I agree that there is not much we can do on the server side (see issue 19806) and with the fix for issue 19662 the server won't recognize this error at all (patiently waiting for b'\r\n\' which is unlikely to appear in the first handshake-message from the client).

Attached is a smtplib patch for 3.x.
msg301482 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-09-06 16:49
Not a bug in the SSL module.
History
Date User Action Args
2022-04-11 14:57:18adminsetgithub: 56587
2017-09-06 16:49:07christian.heimessetversions: + Python 2.7, - Python 3.5
nosy: pitrou, giampaolo.rodola, christian.heimes, r.david.murray, joeshaw, jesstess, catalin.iacob, kasun, zvyn, bhm
messages: + msg301482

assignee: christian.heimes ->
components: - SSL
2016-09-15 07:49:41christian.heimessetassignee: christian.heimes

components: + SSL
nosy: + christian.heimes
2016-09-09 00:26:55BreamoreBoysetnosy: - BreamoreBoy
2016-09-08 22:55:35christian.heimessetstage: needs patch -> patch review
versions: + Python 3.5, Python 3.6, Python 3.7, - Python 2.7, Python 3.2, Python 3.3
2014-07-22 23:14:34zvynsetfiles: + issue12378_py35.patch
nosy: + zvyn, jesstess
messages: + msg223706

2014-06-29 12:39:52giampaolo.rodolasetmessages: + msg221837
2014-06-23 23:13:11bhmsetfiles: + issue12378_py27.patch

nosy: + bhm
messages: + msg221383

keywords: + patch
2014-06-23 21:46:57BreamoreBoysetnosy: + giampaolo.rodola
messages: + msg221379
2014-06-21 23:45:40BreamoreBoysetnosy: + BreamoreBoy
messages: + msg221211
2011-06-21 19:52:30catalin.iacobsetmessages: + msg138795
2011-06-21 18:51:55catalin.iacobsetnosy: + catalin.iacob
2011-06-20 22:23:24r.david.murraysetversions: + Python 2.7, Python 3.2, Python 3.3, - Python 2.6
nosy: + r.david.murray, pitrou, kasun

messages: + msg138764

stage: needs patch
2011-06-20 19:59:41joeshawsetmessages: + msg138754
2011-06-20 19:44:23joeshawcreate