SSL-ed sockets don't close correct?

@loewis

BPO	978833
Nosy	@loewis, @brettcannon, @arigo, @arekm
Files	test_ftpslib.py: FTPS client with simple test function httplib.patch: httplib patch for full shutdown of ssl fake sockets httplib_sslsockclose.patch smptlib_sslclose.patch socket.diff fakeclose.diff

^{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/loewis'
closed_at = <Date 2007-09-05.19:12:27.293>
created_at = <Date 2004-06-24.09:57:53.000>
labels = ['library', 'release-blocker']
title = "SSL-ed sockets don't close correct?"
updated_at = <Date 2007-09-05.19:12:27.293>
user = 'https://bugs.python.org/kxroberto'

bugs.python.org fields:

activity = <Date 2007-09-05.19:12:27.293>
actor = 'loewis'
assignee = 'loewis'
closed = True
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2004-06-24.09:57:53.000>
creator = 'kxroberto'
dependencies = []
files = ['1315', '1316', '1317', '1318', '1319', '1320']
hgrepos = []
issue_num = 978833
keywords = []
message_count = 15.0
messages = ['21278', '21279', '21280', '21281', '21282', '21283', '21284', '21285', '21286', '21287', '21288', '21289', '21290', '55667', '55668']
nosy_count = 6.0
nosy_names = ['loewis', 'brett.cannon', 'arigo', 'arekm', 'tinolange', 'kxroberto']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue978833'
versions = ['Python 2.5']

When testing FTP over SSL I have strong doubt, that
ssl-ed sockets are not closed correctly. (This doesn't
show with https because nobody takes care about whats
going on "after the party".) See the following :

---

I need to run FTP over SSL from windows (not shitty
sftp via ssh etc!)
as explained on
http://www.ford-hutchinson.com/~fh-1-pfh/ftps-ext.html
(good variant
3: FTP_TLS )

I tried to learn from M2Crypto's ftpslib.py (uses
OpenSSL - not
Pythons SSL) and made a wrapper for ftplib.FTP using
Pythons SSL.

I wrap the cmd socket like:

        self.voidcmd('AUTH TLS')
        ssl = socket.ssl(self.sock, self.key_file,
self.cert_file)
        import httplib
        self.sock = httplib.FakeSocket(self.sock, ssl)
        self.file = self.sock.makefile('rb')

Everything works ok, if I don't SSL the data port
connection, but only
the
If I SSL the data port connection too, it almosts work,
but ...

        self.voidcmd('PBSZ 0')
        self.voidcmd('PROT P')

wrap the data connection with SSL:

            ssl = socket.ssl(conn, self.key_file,
self.cert_file)
            import httplib
            conn = httplib.FakeSocket(conn, ssl)

than in retrbinary it hangs endless in the last 'return
self.voidresp()'. all data of the retrieved file is
already correctly
in my basket! The ftp server just won't send the final
'226 Transfer
complete.' on the cmd socket. Why?

    def retrbinary(self, cmd, callback, blocksize=8192,
rest=None):
        self.voidcmd('TYPE I')
        conn = self.transfercmd(cmd, rest)
        fp = conn.makefile('rb')
        while 1:
            #data = conn.recv(blocksize)
            data = fp.read()    #blocksize)
            if not data:
                break
            callback(data)
        fp.close()
        conn.close()
        return self.voidresp()

what could be reason?
The server is a ProFTPD 1.2.9 Server.
I debugged, that the underlying (Shared)socket of the
conn object is
really closed.
(If I simly omit the self.voidresp(), I have one file
in the box, but
subsequent ftp communication on that connection is not
anymore
correct.)

Logged In: YES
user_id=212920

Hi Roberto!

Today a patch for _ssl.c was checked in (see bpo-945642) that
might solve your problem, too.

Could you please grab the *next* alpha (this will be Python
2.4 Alpha 2) and test and report afterwards if it is solved?

Thanks for your help!

Tino

Logged In: YES
user_id=357491

Is this still a problem for you, Roberto, with Python 2.4a2?

Logged In: YES
user_id=357491

Since I believe this was fixed with the patch Tino mentions and Roberto
has not replied, I am closing as fixed.

Logged In: YES
user_id=972995

I retried that again with py2.4.1. The problem/bug is still
there.

In attachment I supplied a full FTPS client test_ftpslib.py
with simple test function - ready to run into the problem:
At the end of ftp.retrlines 'return self.voidresp()'
freezes : waiting eternally for response bytes at the
command port. the same at the end of .storelines after the
data is transfered on the data port.

My preliminary guess is still, that python's ssl has a
severe (EOF?) problem closing a socket/connection correctly.
obviously this problem doesn't show up with https because
everything is done on one connection - no dependency on a
correct socket/connect close signal.

(from other https communication with some proxies in between
my users get ssl-eof-error errors also. I still can't solve
that bug too. this shows python's ssl has a severe EOF bug
with complex https also - or cannot handle certain
situations correct.)

I learned the FTPS/TLS client from M2crypto's ftpslib. the
only difference in (transposed) logic, that I can see is,
that M2crpyto uses an additional line
"conn.set_session(self.sock.get_session())" during setup of
the data port ssl connection. I don't know what it is,
pythons ssl doesn't have sucht "session"-functions, but I
think it has no severe meaning.?

Robert

Logged In: YES
user_id=972995

Now I managed to solve the problem for me with attached
patch of httplib.py: a explicit shutdown ( 2 or 1 ) of the
(faked) ssl'ed socket solves it.
I still guess the ssl'ed socket (ssl dll) should do that
auto on sock.close()
Thus I leave it as a bug HERE. Right?

[ I also have the hope, that this also solves the
ssl-eof-errors with https (of some of my users; will see
this in future)

*** \usr\src\py24old/httplib.py Sat Sep 24 21:35:28 2005
--- httplib.py Sat Sep 24 21:37:48 2005
*************** class SharedSocket:
*** 899,904 ****
--- 899,905 ----
self._refcnt -= 1
assert self._refcnt >= 0
if self._refcnt == 0:
+ self.sock.shutdown(2)
self.sock.close()

      def __del__(self):

Logged In: YES
user_id=972995

Testing it with Python2.5a2, the problem is still there.

Without the .shutdown(2) (or .shutdown(1)) patch to the
httplib.SharedSocket (base for FakeSocket), the ftps example
freezes on the cmd channel, because the SSL'ed data channel
doesn't close/terminate --> FTPS server doesn't respond on
the cmd channel. The ftps example is most specific to show
the bug.

Yet you can also easily blow up a HTTPS-server with this
decent test code who only opens (bigger!) files and closes
without reading everything:

Python 2.5a2 (r25a2:45740, May 11 2006, 11:25:30)
[GCC 3.3.5 (Debian 1:3.3.5-13)] on linux2
Type "help", "copyright", "credits" or "license" for more
information.
Robert's Interactive Python - TAB=complete
import sys,os,re,string,time,glob,thread,pdb
>>> import urllib
>>> l=[]
>>> for i in range(10):
...    f=urllib.urlopen('https://srv/big-Python-2.5a2.tgz')
...    f.close()
...    l.append(f)
...
>>>

=> in the (apache) servers ssl_engine_log you can see that
connections remain open (until apache times out after 2
minutes) and lots of extra apache daemons are started!

=> f.close() doesn't really close the connection (until it
is __del__'ed )

Trying around I found that the original undeleted f.fp._ssl
is most probably the cause and holds the real socket open.
a f.fp._sock.close() doesn't close also - but only when del
f.fp._ssl is done. (only a f.fp._sock._sock.close() would
force the close). The original fp is held in closures of
.readline(s)/iter/next...

--

I now tried an alternative patch (instead of the
shutdown(2)-patch), which also so far seems to cure
everything . Maybe thats the right solution for the bug:

--- httplib.py.orig     2006-05-11 11:25:32.000000000 +0200
+++ httplib.py  2006-05-11 13:45:07.000000000 +0200
@@ -970,6 +970,7 @@
             self._shared.decref()
             self._closed = 1
             self._shared = None
+            self._ssl = None

 class SSLFile(SharedSocketClient):
     """File-like object wrapping an SSL socket."""
@@ -1085,6 +1086,7 @@
     def close(self):
         SharedSocketClient.close(self)
         self._sock = self.__class__._closedsocket()
+        self._ssl = None

     def makefile(self, mode, bufsize=None):
         if mode != 'r' and mode != 'rb':

In another application with SSL'ed SMTP connections there
arose similar problems.

I also worked around the problem in smtplib.py so far in a
similar style:

    def close(self):
        self.realsock.shutdown(2)
        self.realsock.close()

Yet, the right patch maybe (not tested extensively so far):

--- Lib-orig/smtplib.py 2006-05-03 13:10:40.000000000 +0200
+++ Lib/smtplib.py 2006-05-11 13:50:12.000000000 +0200
@@ -142,6 +142,7 @@
sendall = send

     def close(self):
+        self.sslobj = None
         self.realsock.close()

 class SSLFakeFile:
@@ -161,7 +162,7 @@
         return str

     def close(self):
-        pass
+        self.sslobj = None

 def quoteaddr(addr):
     """Quote a subset of the email addresses defined by RFC
821.

-robert

Logged In: YES
user_id=21627

Can you please try the attached patch? It makes sure
_socketobject.close really closes the socket, rather than
relying on reference counting to close it.

Logged In: YES
user_id=21627

This is now fixed in 50844. I won't backport it to 2.4, as
it may cause working code to fail. For example, httplib
would fail since it would already close the connection in
getresponse, when the response object should still be able
to read from the connection.

Martin, I think the rev 50844 is wrong. To start with,
it goes clearly against the documentation for makefile()
which states that both the socket and the pseudo-file
can be closed independently. What httplib.py was doing
was correct.

I could write a whole essay about the twisted history
of socket.py. It would boil down to: in r43746, Georg
removed a comment that was partially out-of-date,
but that was essential in explaining why there was no
self._sock.close() in _socketobject.close(): because
the original purpose of _socketobject was to implement
dup(), so that multiple _socketobjects could refer to
the same underlying _sock. The latter would close
automagically when its reference counter dropped to
zero. (This means that your check-in also made dup()
stop working on all platforms.)

The real OP's problem is that the _ssl object keeps
a reference to the underlying _sock as well, as
kxroberto pointed out. We need somewhere code that
closes the _ssl object...

For reference, PyPy - which doesn't have strong
refcounting guarantees - added the equivalent of an
explicit usage counter in the C socket object, and
socket.py calls methods on its underlying object to
increment and decrement that counter. It looks like
a solution for CPython too - at least, relying on
refcounting is bad, if only because - as you have
just proved - it creates confusion. (Also,
httplib/urllib have their own explicitly-refcounted
wrappers...)

Ok, I reverted this patch in r54543 and r54544.

arigo, what do you think about the alternative patch (fakeclose.diff), which drops the ssl object in FakeSocket.close()? I don't think we can add an explicit close operation for 2.5.x to SSL objects, so we have to continue to rely on reference counting.

File Added: fakeclose.diff

The fakeclose.diff patch looks safe enough.

This patch is now committed as r56588 for 2.6 only.

How to deal with this on python 2.5.x? I assume that msg21288 means that
fakeclose.diff isn't good way for python 2.5.x.

Unfortunately there are web services that refuse to respond to second
query if first wasn't properly ended (rare but I'm dealing with one).

For 2.5, it reverts to the state of 2.4. You'll have to find a
work-around on your own.

Navigation Menu

Repository navigation

SSL-ed sockets don't close correct? #40444

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

SSL-ed sockets don't close correct? #40444

Description

Activity

kxroberto commented on Jun 24, 2004

tinolange commented on Jul 10, 2004

brettcannon commented on Aug 16, 2004

brettcannon commented on Dec 22, 2004

kxroberto commented on Sep 24, 2005

kxroberto commented on Sep 24, 2005

kxroberto commented on May 11, 2006

loewis commented on Jul 3, 2006

loewis commented on Jul 26, 2006

arigo commented on Nov 20, 2006

loewis commented on Mar 23, 2007

arigo commented on Mar 23, 2007

loewis commented on Jul 27, 2007

arekm commented on Sep 5, 2007

loewis commented on Sep 5, 2007

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions