Attached patch is a first-cut at a backport patch. Note that it is not quite a 1-1 with the original:

The SSL module backport added a new field for the Python-level "SSLSocket" reference (ssl_sock), which was a different object from the _socket.socket (PySocketObject *) object. This is all effectively supplanted by the new owner field, so this patch does that as well.

The patch looks basically complete to me, but the tests hang for some reason: in a test where the client side hangs up (test_check_hostname) the server side of the socket blocks in a call to read(). I'm not sure why.

Antoine, or anyone else who worked on the original patch, did you run into any issues like this, or have any other insights into what might be causing it?

It seems that SSLSocket.close() doesn't actually close the socket, and that's why the server side read() blocks.

It's a bit of a mystery to me how socket.close(), which is called by SSLSocket to do the actual close, is supposed to work. I don't see any calls to _sock.close() in there..

If I add the following 3 lines to socket.py then it works (but there's a few unexpected EOF errors in return).

diff -r ae64614b66b7 Lib/socket.py
--- a/Lib/socket.py     Sat Oct 04 18:24:32 2014 -0400
+++ b/Lib/socket.py     Sun Oct 05 18:16:51 2014 -0400
@@ -192,6 +192,9 @@
     def close(self, _closedsocket=_closedsocket,
               _delegate_methods=_delegate_methods, setattr=setattr):
         # This function should not reference any globals. See issue python/cpython#39247.
+        if hasattr(self._sock, '_dummy'):
+            return
+        self._sock.close()
         self._sock = _closedsocket()
         dummy = self._sock._dummy
         for method in _delegate_methods:

I'm probably overlooking something b/c I can't imagine socket.close() being a no-op.

Right, socket._socketobject mearly nulls out the reference to _socket.socket, and lets reference counting take care of the rest.

I've more of less got this figured out:

• When do_handshake() raises an exception (say, a CertificateError), then a reference to a traceback is stored for sys.exc_info()
• This traceback holds a reference to a frame where ssl.SSLObject is self
• ssl.SSLObject holds a reference to _ssl._SSLSocket
• Which holds a reference to _socket.socket

This is avoided on Python3 because exceptions don't stick around, adding a sys.exc_clear() to that test causes it to not hang.

It seems like ssl.SSLSocket.close() should probably explicitly close the SSLObject somehow? I think this problem would appear on Python3 if you caught the exception manually and kept a reference to it?

Right, socket._socketobject mearly nulls out the reference to _socket.socket, and lets reference counting take care of the rest.

Ugh this is bad... I thought close() was exactly there when you don't want to depend on refcounting for cleanup.

• When do_handshake() raises an exception (say, a CertificateError), then a reference to a traceback is stored for sys.exc_info()
• This traceback holds a reference to a frame where ssl.SSLObject is self
• ssl.SSLObject holds a reference to _ssl._SSLSocket
• Which holds a reference to _socket.socket

On Python 3.x the last one above is a weakref.

It seems like ssl.SSLSocket.close() should probably explicitly close the SSLObject somehow? I think this problem would appear on Python3 if you caught the exception manually and kept a reference to it?

On Python 3.x socket.close() does a real close() on the socket, it seems. (thought it appears to have an app-level refcount for makefile()). I agree this is the best way but it seems very scary to make that change for 2.7.

I think that closing the socket in SSLSocket.close(), as you suggest, would work (using socket._sock.close()), or or maybe you can make the "Socket" member in _SSLSocket a weakref?

Unfortunately it can't be a weakref in python2 :-(

In Python3 socket._socketobject *subclasses* _socket.socket, so when we pass "self" to stuff, it's has the right C-level fields but it's also a Python-levle object so it can have a weakref.

In Python2 socket._socketobject composes with _socket.socket, so we pass "self._sock", that way it has teh right C-level fields. Unfortunately taking a weakref of _socket.socket is not allowed.

In Python2 socket._socketobject composes with _socket.socket, so we pass "self._sock", that way it has teh right C-level fields. Unfortunately taking a weakref of _socket.socket is not allowed.

I see, and agree that making it weakref-able would be a bad idea for a minor release.

As suggested by Benjamin, I've filed bpo-22569 to add weakref support to _socket.socket; that will address this and further reduce teh diff with Python3.

New patch works and passes all tests. It's on top of bpo-22569.

New patch is the same, it just rebases the socket changes out since Benjamin landed that (thanks!)

Updated patch cherry-picks in some of the documentation updates that were pushed by Victor.

Since this is such a new feature (not even released in 3.x), I don't think we should put it in 2.7.9.

Would you be ok with it going into 2.7.10? The biggest argument in favor of this is that it significantly reduces the diff between 2.x and 3.x's SSL module, specifically it removes the one major difference between the two of them.

We can reevaluate when we know when 2.7.10 will be released.

While I agree with the idea of backporting this at some point after it has been published in a 3.x release, Guido also specifically requested that we *not* treat PEP-466 as blanket permission to backport other network security features to Python 2.7. (I originally had that kind of wording in the PEP, and he convinced me to take it out to ensure we considered each future feature backport request on its own merits)

That means 2.7.x should track the 3.4.x maintenance branch until after 3.5 is released, and then it should start tracking the 3.5.x maintenance branch.

However, any new 3.5 ssl features will need their own backporting PEP, rather than relying on the previous approval in PEP-466 to backport ssl features from 3.4.

Since this is such a new feature (not even released in 3.x), I don't think we should put it in 2.7.9.

While ssl.MemoryBIO would be very useful on Windows for Trollius (to support SSL with the IOCP event loop), I also consider it as a new feature. It's a little bit borderline between feature and security fix.

Maybe it should be discussed on python-dev?

Note: MemoryBIO was added by issue bpo-21965 (first commit: a79003f25a41).

Now that 3.5 is out, does that mean we can sync 2.7 with the 3.5 ssl again and land this patch?

I don't know or remember what the whole PEP says. In doubt, I suggest floating it by python-dev to get a more informed answer.

Guido specifically asked for a new PEP for each resync that explicitly enumerated the changes being backported, after earlier drafts of PEP-466 requested blanket future permission for network security related backports.

That approach actually turns out to be better from a documentation and backport management perspective - I organised the "New features in maintenance releases" section of the Python 2.7 What's New by PEP number, and "we backported PEP-466" nicely describes the changes that were backport to RHEL 7.2.

A new PEP for resyncing with Python 3.5 can be a lot simpler than PEP-466 was, though - the basic rationale doesn't need to be repeated, and the PEP-466 backport already brought the implementations much closer together.

For what it's worth, it would still be great if this could be merged. It could help a lot with 2/3 migrations of async code that uses Twisted and wants to adopt some asyncio parts. Right now the stdlib SSL module is de-facto useless in those scenarios and depending on pyOpenSSL is pretty much the only choice, and the lack of availability of a memory BIO on 2.x is the biggest blocker.

Glyph, if this is just for Twisted you could ship the "sslcompat" module that I'm shipping with Gruvi. It backports the async IO stuff and a few other pieces through an extension module.

https://github.com/geertj/gruvi/blob/master/src/sslcompat.c

and

https://github.com/geertj/gruvi/blob/master/lib/gruvi/sslcompat.py

But having this in the stdlib itself is much cleaner of course.

I converted bpo-22559.diff to a pull request. I only had one conflict!

--- Lib/ssl.py
+++ Lib/ssl.py
@@ -727,11 +857,7 @@ class SSLSocket(socket):
         if not self._sslobj:
             raise ValueError("Read on closed or unwrapped SSL socket.")
         try:
-            if buffer is not None:
-                v = self._sslobj.read(len, buffer)
-            else:
-                v = self._sslobj.read(len or 1024)
-            return v
+            return self._sslobj.read(len, buffer)
         except SSLError as x:
             if x.args[0] == SSL_ERROR_EOF and self.suppress_ragged_eofs:
                 if buffer is not None:

I fixed it by using "return self._sslobj.read(len, buffer)".

I added a second commit to synchronize Lib/ssl.py with master:

• add selected_alpn_protocol() method, needed by test_ssl
• change how the default size of read() is defined
• change how send()/write result is reported: handle SSL_ERROR_WANT_READ/SSL_ERROR_WANT_WRITE

I tagged my PR as [WIP] since I'm not sure if I backported bpo-20951 "SSLSocket.send() returns 0 for non-blocking socket" by mistake or not :-)

Please review the PR carefully!

You have to include the changes from bpo-29334, too.

Christian Heimes: "You have to include the changes from bpo-29334, too."

Right now, we are only allow to squash a patch serie into an unique patch. I would prefer to keep the fix of bpo-29334 as a separated commit, so merge it after MemoryBIO is merged. The change seems to change different lines, so it will be fine.

Sounds good to me! I'll take care of the other patch after you have committed your PR.

bpo-29781 must be backported to 2.7 to fix SSLSocket.version()

ssl_io_loop() currently ignores the timeout parameter: #3500 fixes it in the master branch.

With PEP-546 withdrawn/rejected and 2.7 at EOL, I'm closing the issue.