STINNER Victor added the comment:
I have some comments and suggestions to enhance the new API. I chose to open a
new issue: #22564.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
Geert Jansen added the comment:
Maybe an example is useful on how the Memory BIO stuff can be used to implement
SSL on top of a proactor event loop. I just added support for this to my Gruvi
project in the branch feat-memory-bio:
An SslPipe utility class that uses the memory BIOs:
Antoine Pitrou added the comment:
SSLPipe looks interesting. I wonder if it can be used to reimplement
_SelectorSslTransport in asyncio.selector_events (at least as an experiment).
I'll take a look at the cumulated patch soon, thank you.
--
___
Roundup Robot added the comment:
New changeset a79003f25a41 by Antoine Pitrou in branch 'default':
Issue #21965: Add support for in-memory SSL to the ssl module.
https://hg.python.org/cpython/rev/a79003f25a41
--
nosy: +python-dev
___
Python tracker
Geert Jansen added the comment:
Thanks Antoine for merge!
SSLPipe looks interesting. I wonder if it can be used to reimplement
_SelectorSslTransport in asyncio.selector_events (at least as an experiment).
Yes, it could be done quite easily. SslPipe has no dependency on other parts of
Gruvi
Antoine Pitrou added the comment:
Le 05/10/2014 23:24, Geert Jansen a écrit :
Yes, it could be done quite easily. SslPipe has no dependency on
other
parts of Gruvi and if this is for Python 3.5 only then you don't need
sslcompat either.
Yes, it works. Note that I had to modify SSLPipe to
Roundup Robot added the comment:
New changeset 8da1aa71cd73 by Antoine Pitrou in branch 'default':
Remove unused block argument in SSLObject.do_handshake() (issue #21965)
https://hg.python.org/cpython/rev/8da1aa71cd73
--
___
Python tracker
Antoine Pitrou added the comment:
I'm closing this issue, and will open a new one for asyncio and/or SSLPipe.
Thank you very much, Geert!
--
resolution: - fixed
stage: patch review - resolved
status: open - closed
___
Python tracker
Antoine Pitrou added the comment:
One issue with the owner is that there is now a reference cycle between
SSLSocket and SSLObject (something which the original design is careful to
avoid by using weakrefs in the _ssl module).
--
___
Python tracker
Geert Jansen added the comment:
One issue with the owner is that there is now a reference cycle between
SSLSocket and SSLObject (something which the original design is careful to
avoid by using weakrefs in the _ssl module).
Note that owner is a weakref :) Did you look at the code?
Antoine Pitrou added the comment:
Ahhh. I had forgotten about that. It may be worthwhile to add a comment in
SSLObject.__init__, then. Also, can you provide a cumulated patch?
--
___
Python tracker rep...@bugs.python.org
Geert Jansen added the comment:
Addded the comment about owner being a weakref, and added a new consolidated
patch (ssl-memory-bio-5).
--
Added file: http://bugs.python.org/file36806/ssl-memory-bio-5.patch
___
Python tracker rep...@bugs.python.org
Geert Jansen added the comment:
New patch attached. This patch makes SSLSocket use SSLObject. The big benefit
here is obviously test coverage.
I decided against using SSLObject as a mixin, because all methods need to be
reimplemented anyway because for SSLSocket they need to handle the
Antoine Pitrou added the comment:
Well... I would have expected this approach to yield a bigger reduction in code
size. If it doesn't shrink the code, then I'm not sure it's worthwhile. What do
you think?
(also, why do you have to add an owner attribute?)
--
Geert Jansen added the comment:
Well... I would have expected this approach to yield a bigger reduction in
code size. If it doesn't shrink the code, then I'm not sure it's worthwhile.
What do you think?
I think the improved test coverage might still make it worthwhile. All tests
are now
Changes by Joshua Moore-Oliva chatg...@gmail.com:
--
nosy: +chatgris
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
___
Python-bugs-list
Geert Jansen added the comment:
Antoine, sorry for the delay, we just had a new kid and I changed jobs :)
Let me try if I can create an updated patch that where SSLObject is a mixin for
SSLSocket. I think the argument about writing tests once is important. Be back
in a few days..
--
Antoine Pitrou added the comment:
We can leave these undocumented at the Python level if you prefer.
I'd rather that indeed. If there's a specific need, we can expose them as a
separate issue.
Maybe just SSLInstance, would that be better than SSLObject?
That doesn't sound much better :-)
Geert Jansen added the comment:
Thanks Antoine. See my comments below:
- is it necessary to start exposing server_hostname, server_side and
pending()?
At the C level I need server_hostname and server_side exposed because they are
needed to implement the cert check in do_handshake().
Antoine Pitrou added the comment:
Nice work, thank you! The new API looks mostly good to me. I am wondering about
a couple of things:
- is it necessary to start exposing server_hostname, server_side and pending()?
- SSLObject is a bit vague, should we call it SSLMemoryObject? or do you expect
Geert Jansen added the comment:
Adding small patch (incremental to patch #4) to fix a test failure.
--
Added file: http://bugs.python.org/file36483/ssl-memory-bio-4-incr1.patch
___
Python tracker rep...@bugs.python.org
Geert Jansen added the comment:
Updated patch. Contains:
* An owner attribute on a _ssl.SSLSocket that is used as the first argument
to the SNI servername callback (implemented as a weakref).
* Documentation
I think this covers all outstanding issues that were identified. Antoine,
please
Antoine Pitrou added the comment:
Geert, are you still trying to work on this?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
___
Geert Jansen added the comment:
Antoine, yes, I just got back from holiday. I will have an updated patch
tomorrow.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
Changes by Alex Gaynor alex.gay...@gmail.com:
--
nosy: +alex
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
___
Python-bugs-list mailing
Geert Jansen added the comment:
Thanks to Ben and Glyph for their feedback. The memory BIO should allow
ProactorEventLoop to support SSL. I say should because I have not looked at
it myself. However, my Gruvi project is proactor (libuv) based and I have a
private branch where SSL support is
Changes by Glyph Lefkowitz gl...@twistedmatrix.com:
--
nosy: -glyph
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
___
Python-bugs-list
Jean-Paul Calderone added the comment:
Please do *not* add me to the nosy list of any issues.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
Changes by Jean-Paul Calderone jean-p...@hybridcluster.com:
--
nosy: -exarkun
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
___
Antoine Pitrou added the comment:
Perhaps Glyph wants to chime in :-)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
___
Glyph Lefkowitz added the comment:
I don't have a whole lot to add. I strongly recommended that this be done this
way twice, once when ssl was added to Python and once when ssl was added to
tulip, so I'm glad to see it's happening now. Regarding the specific
implementation I am unlikely to
Ben Darnell added the comment:
Looks good to me. I've added exarkun and glyph to the nosy list since
Twisted's experience with PyOpenSSL may provide useful feedback even though
Twisted will presumably stick with what they've got instead of switching to
this new interface.
--
nosy:
Antoine Pitrou added the comment:
By the way, this would allow ProactorEventLoop to support SSL, since it
decouples the SSL protocol handling from the actual socket I/O.
--
nosy: +sbt
___
Python tracker rep...@bugs.python.org
Geert Jansen added the comment:
New patch with a Python-level API (option #3).
This needs some more tests, and docs.
--
Added file: http://bugs.python.org/file36248/ssl-memory-bio-3.patch
___
Python tracker rep...@bugs.python.org
Antoine Pitrou added the comment:
I think the API choice looks reasonable, thank you (haven't looked at the patch
in detail). A question though: does it support server-side SNI? AFAIR
server-side SNI requires you to be able to change a SSL object's context.
--
Antoine Pitrou added the comment:
Am adding the asyncio maintainers as well as Ben Darnell (Tornado) to the nosy
list, for feedback.
--
nosy: +Ben.Darnell, gvanrossum, yselivanov
___
Python tracker rep...@bugs.python.org
Geert Jansen added the comment:
A question though: does it support server-side SNI? AFAIR server-side SNI
requires you to be able to change a SSL object's context.
Yes, it does. See the following comment in _servername_callback():
/* Pass a PySSLSocket instance when using memory BIOs, but
Antoine Pitrou added the comment:
Le 04/08/2014 11:21, Geert Jansen a écrit :
I realize the above is an abstraction violation between the C and
Python level. Now that we have an SSLObject Python level API, I could
update the code to store a weakref to the SSLObject in the _SSLSocket
(just like
Changes by Geert Jansen gee...@gmail.com:
Added file: http://bugs.python.org/file36189/ssl-memory-bio-2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
Geert Jansen added the comment:
I added a new patch that addresses the comments.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
___
Geert Jansen added the comment:
I've explored a few options for the Python-level API in the attachment
bio_python_options.py.
Me personally I prefer the more light weight option #3. This is both out of
selfish interest (less work for me), but also I believe that memory BIOs are an
API that
Changes by Geert Jansen gee...@gmail.com:
Removed file: http://bugs.python.org/file36190/bio_python_options.py
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
Changes by Geert Jansen gee...@gmail.com:
Added file: http://bugs.python.org/file36191/bio_python_options.py
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
Antoine Pitrou added the comment:
The C part of the patch looks roughly ok to me (modulo a couple of comments).
However, we must now find a way to expose this as a Python-level API.
--
___
Python tracker rep...@bugs.python.org
Geert Jansen added the comment:
Hi all (pitrou, haypo and all others) can I get some feedback on this patch?
Thanks!
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
Changes by STINNER Victor victor.stin...@gmail.com:
--
nosy: +haypo
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
___
Python-bugs-list
Changes by Ezio Melotti ezio.melo...@gmail.com:
--
components: +Extension Modules
nosy: +ezio.melotti
stage: - patch review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
New submission from Geert Jansen:
The attached patch adds a _MemoryBIO type to _ssl, and a _wrap_bio() method to
_SSLContext. The patch also includes tests.
For now I kept _wrap_bio() and _MemoryBIO semi-private. The reason is that it
returns an _SSLSocket instead of an SSLSocket and this
Changes by Antoine Pitrou pit...@free.fr:
--
nosy: +christian.heimes, dstufft, giampaolo.rodola, janssen, pitrou
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21965
___
49 matches
Mail list logo
