[issue5238] ssl makefile never closes socket

[Antoine Pitrou]

Antoine Pitrou  added the comment:

The makefile issue is fixed in r80428 (trunk) and r80431 (2.6). Also ported the 
additional test to py3k and 3.1.

The other issue pointed out by Marcin Bachry doesn't seem fixable without 
breaking backwards compatibility, for applications which close() the SSL object 
but expect the underlying socket to still be usable for clear-text 
communications. Therefore I prefer to close this issue.

--
resolution:  -> fixed
stage:  -> committed/rejected
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
nosy: +giampaolo.rodola
versions: +Python 3.2 -Python 3.0

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[Marcin Bachry]

Marcin Bachry  added the comment:

I think there are two bugs spotted here. The first is the mentioned
missing "close" param to _fileobject in makefile() method.

The second one was noticed by Jonathan. He doesn't use makefile() at
all, yet his socket isn't closed either. Here's what I think is going on:

The raw filesystem socket seems to be closed only upon garbage
collection, when reference count of socket._realsocket drops to zero
(ie. it's not closed explicitly anywhere). _realsocket is wrapped by
socket._socketobject and all its close() method does is dropping
reference to _realsocket object it wraps.

The bug goes like this:
1. plain = socket.create_connection(...)
   it creates and wraps a native _realsocket (plain._sock) with
reference count = 1; "plain" is an instance of _socketobject
2. secure = ssl.wrap_socket(plain)
   because SSLSocket inherits from _socketobject, it also wraps
_realsocket plain._sock and the reference count is 2 now
3. secure.close()
   it calls in turn it's parent method close() which simply drops raw
_realsocket reference count to 1
4. native socket is still referenced by plain._sock and filesystem
descriptor isn't closed unless plain.close() is called too and refcount
drops to zero

I attach the simplest possible test to prove the bug.

--
nosy: +marcin.bachry
Added file: http://bugs.python.org/file13963/test-simple.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[Jonathan Hayward]

Jonathan Hayward  added the comment:

Constantine Sapuntzakis wrote:
> import ssl
> 
> # Work around python bug #5328
> def SSLSocket_makefile_fixed(self, mode='r', bufsize=-1):
> from socket import _fileobject
> 
> self._makefile_refs += 1
> return _fileobject(self, mode, bufsize, True)
> 
> ssl.SSLSocket.makefile = SSLSocket_makefile_fixed

Is it possible this workaround has a bug?

In my production code the socket remains open both after this
monkeypatch and after manually closing the underlying socket. In the
attached test case (TLS certificate and keyfile referenced but not
included), a server does the following:

1: listens to a single HTTPS request on port 8443.
2: Serves a "Hello, world!" page.
3: Closes the connection.
4: Sleeps for five seconds.
5: Exits the process.

The server incorporates the quoted patch, and the behavior from within
Firefox is that it serves up a "Hello world!" page via
https://localhost:8443/ and the connection remains open for five seconds
until the server process exits, apparently indicating a connection that
remains open as long as the process is running.

Closing the underlying connection manually seems to work.

--
Added file: http://bugs.python.org/file13958/test.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Bill, can you check if it's a real bug? Unclosed sockets are pretty bad.

--
nosy: +pitrou
priority:  -> high
versions: +Python 2.7, Python 3.0, Python 3.1

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[Antoine Pitrou]

Changes by Antoine Pitrou :


Removed file: http://bugs.python.org/file13935/unnamed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[Constantine Sapuntzakis]

Constantine Sapuntzakis  added the comment:

Here is the workaround I'm using until the code gets fixed:

import ssl

# Work around python bug #5328
def SSLSocket_makefile_fixed(self, mode='r', bufsize=-1):
from socket import _fileobject

self._makefile_refs += 1
return _fileobject(self, mode, bufsize, True)

ssl.SSLSocket.makefile = SSLSocket_makefile_fixed

An alternate way to fix it is to reach in to the _fileobject wrapper and
close the underlying
implementation:

In the do_GET() method of my web server I called:

self.rfile._sock.close()
self.wfile._sock.close()

-Costa

On Fri, May 8, 2009 at 12:51 PM, Jonathan Hayward wrote:

>
> Jonathan Hayward  added the comment:
>
> Is there a workaround to close a TLS socket and its underlying socket?
>
> I was making something to use https for a simple operation, and it the
> browser acted as if the socket never closed. If I followed the close of
> the ssl socket by a close of the underlying socket, I didn't get errors,
> but the browser throbber acted as if the connection was still open.
>
> Jonathan, http://JonathansCorner.com/
>
> --
> nosy: +JonathansCorner.com
>
> ___
> Python tracker 
> 
> ___
>

--
Added file: http://bugs.python.org/file13935/unnamed

___
Python tracker 

___Here is the workaround I'm using until the code gets fixed:import 
ssl# Work around python bug #5328def SSLSocket_makefile_fixed(self, 
mode='r', bufsize=-1):    from socket import _fileobject
        self._makefile_refs += 1    return _fileobject(self, 
mode, bufsize, True)ssl.SSLSocket.makefile = 
SSLSocket_makefile_fixedAn alternate way to fix it is to reach in 
to the _fileobject wrapper and close the underlying
implementation:In the do_GET() method of my web server I 
called:self.rfile._sock.close()self.wfile._sock.close()-CostaOn Fri, May 8, 2009 at 12:51 PM, Jonathan Hayward rep...@bugs.python.org> 
wrote:

Jonathan Hayward jonathan.hayw...@pobox.com> 
added the comment:

Is there a workaround to close a TLS socket and its underlying socket?

I was making something to use https for a simple operation, and it the
browser acted as if the socket never closed. If I followed the close of
the ssl socket by a close of the underlying socket, I didn't get errors,
but the browser throbber acted as if the connection was still open.

Jonathan, http://JonathansCorner.com/"; 
target="_blank">http://JonathansCorner.com/

--
nosy: +JonathansCorner.com

___
Python tracker rep...@bugs.python.org>
http://bugs.python.org/issue5238>
___

___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[Jonathan Hayward]

Jonathan Hayward  added the comment:

Is there a workaround to close a TLS socket and its underlying socket?

I was making something to use https for a simple operation, and it the
browser acted as if the socket never closed. If I followed the close of
the ssl socket by a close of the underlying socket, I didn't get errors,
but the browser throbber acted as if the connection was still open.

Jonathan, http://JonathansCorner.com/

--
nosy: +JonathansCorner.com

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[Constantine Sapuntzakis]

Constantine Sapuntzakis  added the comment:

I ran into this problem when trying to use wrapsocket with httplib.py
and came up with the same fix.

The problem turns out to be even simpler than a ref counting issue.

In the current tree, the _fileobject constructor is called without the
close = True argument, As a result, _fileobject._close gets set to False
and _fileobject.close() method never propagates the close to
SSLSocket.close(). See line 269 of socket.py.

--
nosy: +csapuntz

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[David Christian]

David Christian  added the comment:

I actually discovered this issue when using httplib over ssl. Closing
the httplib connection was not closing the socket - the socket would
only be closed after garbage collection, due to this bug.  That's what
caused me to investigate and find this flaw.

I ran the regression tests and didn't run into any issues.

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[Bill Janssen]

Bill Janssen  added the comment:

I'd recommend running the whole suite of tests here.  The issue is
mainly with httplib, as I recall it, which "closes" the socket before it
finishes reading from it.

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[Benjamin Peterson]

Changes by Benjamin Peterson :


--
assignee:  -> janssen
nosy: +janssen

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[David Christian]

Changes by David Christian :


--
components: +Library (Lib)
keywords: +patch
type:  -> resource usage
Added file: http://bugs.python.org/file13061/ssl.py.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5238] ssl makefile never closes socket

[David Christian]

New submission from David Christian :

The ssl.py makefile function returns a socket._fileobject object with a
reference to itself, and also increments the makefile_refs variable.

However, the _fileobject is created with the parameter close=False,
which means that when you call _fileobject.close, it does not call close
on the ssl socket!  

>>> import socket, ssl
>>> s = socket.create_connection(('www.rpath.com', 443))
>>> sslSocket = ssl.wrap_socket(s)
>>> f1 = sslSocket.makefile()
>>> f2 = sslSocket.makefile()
>>> f3 = sslSocket.makefile()
>>> sslSocket._makefile_refs
3
>>> sslSocket._sock

>>> sslSocket.close()
>>> f1.close()
>>> f2.close()
>>> f3.close()
>>> sslSocket._makefile_refs
2

The quick fix is to add close=True on the _fileobject call in ssl.py. 
Note that this close=True is _not_ needed in the socket.py makefile call
as that makefile does not do reference counting.

--
messages: 81842
nosy: dugan
severity: normal
status: open
title: ssl makefile never closes socket
versions: Python 2.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com