Support for TLS server_mode #5
Conversation
| @@ -632,6 +641,10 @@ def starttls(self, ssl_context=None, | |||
| if post_handshake_callback is not None: | |||
| self._tls_post_handshake_callback = post_handshake_callback | |||
|
|
|||
| # Drain before initializing TLS | |||
| while self._buffer: | |||
| yield from asyncio.sleep(0) | |||
There was a problem hiding this comment.
I haven’t looked at this into detail yet, so this may be stupid, but yield from self.drain() is no option?
Also, this makes concurrent writes undefined, which should at least be documented.
In addition, a very simple server-mode test (possibly without starttls) should be added. I’m trying to get some unit-test coverage into aioopenssl.
|
There is no self.drain, I looked into copying the code over but what is
really needed is ensuring self._buffer is empty
It might be necessary to freeze write() and read() prior to going into this
loop. The idea is to ensure writes made before the call guarenteed to be
sent prior to TLS handshake.
…On Sat, May 27, 2017 at 1:57 AM, Jonas Wielicki ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In aioopenssl/__init__.py
<#5 (comment)>:
> @@ -632,6 +641,10 @@ def starttls(self, ssl_context=None,
if post_handshake_callback is not None:
self._tls_post_handshake_callback = post_handshake_callback
+ # Drain before initializing TLS
+ while self._buffer:
+ yield from asyncio.sleep(0)
I haven’t looked at this into detail yet, so this may be stupid, but yield
from self.drain() is no option?
Also, this makes concurrent writes undefined, which should at least be
documented.
In addition, a very simple server-mode test (possibly without starttls)
should be added. I’m trying to get some unit-test coverage into aioopenssl.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#5 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB1A8eDBh0VJtIfsWULV3vkgS39yEqlYks5r9-WFgaJpZM4Nn9YI>
.
|
|
Ah, I assumed that drain was provided by the base class somehow; which of course doesn’t make sense because the base class doesn’t do any buffer management. I’ll have a look at this this weekend. |
|
I tried to write tests for this, and I’m not entirely convinced of the design yet. How is that supposed to be used? One cannot meaningfully use So the I really would like to avoid to re-create the Thoughts? |
|
As per asyncio devs' advice, this is how I did this:
async def starttls_server(loop, prototype, addr, port, family):
sock = socket.socket(family, socket.SOCK_STREAM)
sock.setblocking(False)
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
sock.bind((addr, port))
sock.listen(128)
while True:
conn, remote = await loop.sock_accept(sock)
print("accepted", conn, remote)
transport, proto = await aiossl.create_starttls_connection(
loop, prototype, sock=conn, ssl_context_factory=TlsContext,
use_starttls=True, server_mode=True)
proto.loop = loop
print("connected", transport, proto,
transport._extra['server_mode'])
#TODO record remote address, etc into proto
class MucProto(aio.Protocol):
def __init__(self):
self.loop = None
def connection_made(self, transport):
self.transport = transport
self.parser = XmppParser(transport, self)
def connection_lost(self, e):
return
def data_received(self, data):
self.parser.parse(data)
loop = aio.get_event_loop()
coro = starttls_server(loop, MucProto, '127.0.0.1', 5272, socket.AF_INET)
server = loop.run_until_complete(coro)
…On Mon, May 29, 2017 at 12:12 PM, Jonas Wielicki ***@***.***> wrote:
I tried to write tests for this, and I’m not entirely convinced of the
design yet. How is that supposed to be used?
One cannot meaningfully use server_mode with host and port, so sock is
the only option. However, with BaseEventLoop.create_server, one cannot
replace the transport; instead, one has to provide a Protocol (one could
of course create a "Protocol" which .
So the create_starttls_connection function is not really useful for this
kind of scenario.
I really would like to avoid to re-create the create_server logic of
asyncio. We might need a completely different approach, possibly layering
the STARTTLS layer as a Protocol which also exposes a Transport interface.
Thoughts?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#5 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB1A8R8xQ-LC9TL-HSXSId6-TZlZvaaNks5r-xipgaJpZM4Nn9YI>
.
|
|
Okay, in that case, that |
|
I'm fine with that. I can look into a cleaner way to do it, but this
strategy was given by an asyncio developer as the current best choice
supported by the current asyncio API - at least until STARTTLS support is
added in Python 3.8
…On Mon, May 29, 2017 at 10:31 PM, Jonas Wielicki ***@***.***> wrote:
Okay, in that case, that starttls_server function should probably be
included in aioopenssl (even though I don’t like it). It should also use
the STARTTLSTransport constructor directly then, which should be extended
by an argument which allows to initialise the extra-dict.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#5 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB1A8bFhgAmB6Y0TE65gVF-ThxoUSinoks5r-6nOgaJpZM4Nn9YI>
.
|
|
Hey ho! It’s been a while! Do you have any intention on persuing this or can we close it? |
No description provided.