[issue22559] [backport] ssl.MemoryBIO

[STINNER Victor]

STINNER Victor added the comment:

Christian Heimes: "You have to include the changes from #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.

--

___
Python tracker 

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

[issue22559] [backport] ssl.MemoryBIO

[STINNER Victor]

STINNER Victor added the comment:

I converted issue22559.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!

--

___
Python tracker 

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

[issue22559] [backport] ssl.MemoryBIO

[Christian Heimes]

Christian Heimes added the comment:

You have to include the changes from #29334, too.

--

___
Python tracker 

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

[issue22559] [backport] ssl.MemoryBIO

[STINNER Victor]

Changes by STINNER Victor :


--
pull_requests: +2187

___
Python tracker 

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

[issue22559] [backport] ssl.MemoryBIO

[Geert Jansen]

Geert Jansen added the comment:

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.

--

___
Python tracker 

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

[issue22559] [backport] ssl.MemoryBIO

[Glyph Lefkowitz]

Glyph Lefkowitz added the comment:

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.

--
nosy: +glyph

___
Python tracker 

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

[issue22559] [backport] ssl.MemoryBIO

[Christian Heimes]

Changes by Christian Heimes :


--
assignee:  -> christian.heimes
components: +Extension Modules, SSL
type:  -> enhancement

___
Python tracker 

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

[issue22559] [backport] ssl.MemoryBIO

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--

___
Python tracker 

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

[issue22559] [backport] ssl.MemoryBIO

[Donald Stufft]

Donald Stufft added the comment:

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

--

___
Python tracker 

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

[issue22559] [backport] ssl.MemoryBIO

[Antoine Pitrou]

Antoine Pitrou added the comment:

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.

--

___
Python tracker 

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

[issue22559] [backport] ssl.MemoryBIO

[STINNER Victor]

STINNER Victor added the comment:

 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 #21965 (first commit: a79003f25a41).

--
nosy: +haypo

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

[issue22559] [backport] ssl.MemoryBIO

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--
nosy: +ncoghlan

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

[issue22559] [backport] ssl.MemoryBIO

[Benjamin Peterson]

Benjamin Peterson added the comment:

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

--

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

[issue22559] [backport] ssl.MemoryBIO

[Arfrever Frehtes Taifersar Arahesis]

Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com:


--
nosy: +Arfrever

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

[issue22559] [backport] ssl.MemoryBIO

[Alex Gaynor]

Alex Gaynor added the comment:

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.

--

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

[issue22559] [backport] ssl.MemoryBIO

[Alex Gaynor]

Alex Gaynor added the comment:

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

--
Added file: http://bugs.python.org/file36868/issue22559.diff

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

[issue22559] [backport] ssl.MemoryBIO

[Benjamin Peterson]

Benjamin Peterson added the comment:

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.

--
nosy: +benjamin.peterson

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

[issue22559] [backport] ssl.MemoryBIO

[Alex Gaynor]

Alex Gaynor added the comment:

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

--
dependencies: +Add support for weakrefs to _socket.socket

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

[issue22559] [backport] ssl.MemoryBIO

[Alex Gaynor]

Alex Gaynor added the comment:

New patch works and passes all tests. It's on top of issue22569.

--
Added file: http://bugs.python.org/file36827/issue225593.diff

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

[issue22559] [backport] ssl.MemoryBIO

[Alex Gaynor]

Alex Gaynor added the comment:

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

--
Added file: http://bugs.python.org/file36829/issue22559.diff

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

[issue22559] [backport] ssl.MemoryBIO

[Alex Gaynor]

New submission from Alex Gaynor:

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?

--
files: memory-bio.diff
keywords: patch
messages: 228618
nosy: alex, christian.heimes, dstufft, giampaolo.rodola, janssen, pitrou
priority: normal
severity: normal
status: open
title: [backport] ssl.MemoryBIO
versions: Python 2.7
Added file: http://bugs.python.org/file36820/memory-bio.diff

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

[issue22559] [backport] ssl.MemoryBIO

[Geert Jansen]

Changes by Geert Jansen gee...@gmail.com:


--
nosy: +geertj

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

[issue22559] [backport] ssl.MemoryBIO

[Geert Jansen]

Geert Jansen added the comment:

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 #808164.
+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.

--

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

[issue22559] [backport] ssl.MemoryBIO

[Alex Gaynor]

Alex Gaynor added the comment:

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?

--

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

[issue22559] [backport] ssl.MemoryBIO

[Geert Jansen]

Geert Jansen added the comment:

 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?

--

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

[issue22559] [backport] ssl.MemoryBIO

[Alex Gaynor]

Alex Gaynor added the comment:

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.

--

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

[issue22559] [backport] ssl.MemoryBIO

[Geert Jansen]

Geert Jansen added the comment:

 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.

--

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