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: SSL-ed sockets don't close correct?
Type: Stage:
Components: Library (Lib) Versions: Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: loewis Nosy List: arekm, arigo, brett.cannon, kxroberto, loewis, tinolange
Priority: release blocker Keywords:

Created on 2004-06-24 09:57 by kxroberto, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
test_ftpslib.py kxroberto, 2005-09-24 19:06 FTPS client with simple test function
httplib.patch kxroberto, 2005-09-24 19:45 httplib patch for full shutdown of ssl fake sockets
httplib_sslsockclose.patch kxroberto, 2006-05-11 12:05
smptlib_sslclose.patch kxroberto, 2006-05-11 12:07
socket.diff loewis, 2006-07-03 12:03
fakeclose.diff loewis, 2007-03-23 13:35
Messages (15)
msg21278 - (view) Author: kxroberto (kxroberto) Date: 2004-06-24 09:57
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.)
msg21279 - (view) Author: Tino Lange (tinolange) Date: 2004-07-10 22:30
Logged In: YES 
user_id=212920

Hi Roberto!

Today a patch for _ssl.c was checked in (see #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
msg21280 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2004-08-16 23:18
Logged In: YES 
user_id=357491

Is this still a problem for you, Roberto, with Python 2.4a2?
msg21281 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2004-12-22 05:14
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.
msg21282 - (view) Author: kxroberto (kxroberto) Date: 2005-09-24 19:06
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
msg21283 - (view) Author: kxroberto (kxroberto) Date: 2005-09-24 19:45
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):
msg21284 - (view) Author: kxroberto (kxroberto) Date: 2006-05-11 12:05
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
msg21285 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2006-07-03 12:03
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.
msg21286 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2006-07-26 12:14
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.
msg21287 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2006-11-20 11:33
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...)
msg21288 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2007-03-23 13:35
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
msg21289 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2007-03-23 20:37
The fakeclose.diff patch looks safe enough.
msg21290 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2007-07-27 18:28
This patch is now committed as r56588 for 2.6 only.
msg55667 - (view) Author: Arkadiusz Miśkiewicz (arekm) Date: 2007-09-05 19:05
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).
msg55668 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2007-09-05 19:12
For 2.5, it reverts to the state of 2.4. You'll have to find a
work-around on your own.
History
Date User Action Args
2022-04-11 14:56:05adminsetgithub: 40444
2007-09-05 19:12:27loewissetmessages: + msg55668
2007-09-05 19:05:05arekmsetnosy: + arekm
messages: + msg55667
2004-06-24 09:57:53kxrobertocreate