Skip to content

bpo-23749: Implement loop.start_tls() #5039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions Doc/library/asyncio-eventloop.rst
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,38 @@ Creating listening connections
.. versionadded:: 3.5.3


TLS Upgrade
-----------

.. coroutinemethod:: AbstractEventLoop.start_tls(transport, protocol, sslcontext, \*, server_side=False, server_hostname=None, ssl_handshake_timeout=None)

Upgrades an existing connection to TLS.

Returns a new transport instance, that the *protocol* must start using
immediately after the *await*. The *transport* instance passed to
the *start_tls* method should never be used again.

Parameters:

* *transport* and *protocol* instances that methods like
:meth:`~AbstractEventLoop.create_server` and
:meth:`~AbstractEventLoop.create_connection` return.

* *sslcontext*: a configured instance of :class:`~ssl.SSLContext`.

* *server_side* pass ``True`` when a server-side connection is being
upgraded (like the one created by :meth:`~AbstractEventLoop.create_server`).

* *server_hostname*: sets or overrides the host name that the target
server's certificate will be matched against.

* *ssl_handshake_timeout* is (for an SSL connection) the time in seconds to
wait for the SSL handshake to complete before aborting the connection.
``10.0`` seconds if ``None`` (default).

.. versionadded:: 3.7


Watch file descriptors
----------------------

Expand Down
45 changes: 44 additions & 1 deletion Lib/asyncio/base_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,15 @@
import warnings
import weakref

try:
import ssl
except ImportError: # pragma: no cover
ssl = None

from . import coroutines
from . import events
from . import futures
from . import sslproto
from . import tasks
from .log import logger

Expand Down Expand Up @@ -279,7 +285,8 @@ def _make_ssl_transport(
self, rawsock, protocol, sslcontext, waiter=None,
*, server_side=False, server_hostname=None,
extra=None, server=None,
ssl_handshake_timeout=None):
ssl_handshake_timeout=None,
call_connection_made=True):
"""Create SSL transport."""
raise NotImplementedError

Expand Down Expand Up @@ -795,6 +802,42 @@ async def _create_connection_transport(

return transport, protocol

async def start_tls(self, transport, protocol, sslcontext, *,
server_side=False,
server_hostname=None,
ssl_handshake_timeout=None):
"""Upgrade transport to TLS.

Return a new transport that *protocol* should start using
immediately.
"""
if ssl is None:
raise RuntimeError('Python ssl module is not available')

if not isinstance(sslcontext, ssl.SSLContext):
raise TypeError(
f'sslcontext is expected to be an instance of ssl.SSLContext, '
f'got {sslcontext!r}')

if not getattr(transport, '_start_tls_compatible', False):
raise TypeError(
f'transport {self!r} is not supported by start_tls()')

waiter = self.create_future()
ssl_protocol = sslproto.SSLProtocol(
self, protocol, sslcontext, waiter,
server_side, server_hostname,
ssl_handshake_timeout=ssl_handshake_timeout,
call_connection_made=False)

transport.set_protocol(ssl_protocol)
self.call_soon(ssl_protocol.connection_made, transport)
if not transport.is_reading():
self.call_soon(transport.resume_reading)

await waiter
return ssl_protocol._app_transport

async def create_datagram_endpoint(self, protocol_factory,
local_addr=None, remote_addr=None, *,
family=0, proto=0, flags=0,
Expand Down
11 changes: 11 additions & 0 deletions Lib/asyncio/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,17 @@ async def create_server(
"""
raise NotImplementedError

async def start_tls(self, transport, protocol, sslcontext, *,
server_side=False,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add server_hostname parameter maybe?
At least create_connection has it, checking hostname match increases security.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, server_hostname should be added. Or perhaps a bunch of kwargs to allow passing arbitrary arguments to wrap_socket or wrap_bio.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will add it.

Or perhaps a bunch of kwargs to allow passing arbitrary arguments to wrap_socket or wrap_bio.

We don't do that in loop.create_connection or in loop.create_server, we explicitly pass params there. Most of the stuff can be configured through the SSLContext anyways.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... I guess that could be deferred to another PR. However, the ssl module may add some other parameters along the way, and it would be nicer to inherit them automatically. For example, Python 3.6 got the session parameter: https://docs.python.org/3/library/ssl.html#ssl.SSLContext.wrap_socket

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Can you make a PR after this one lands?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarification: asyncio doesn't use ssl.wrap_socket since Python 3.5

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it uses ssl.wrap_bio which accepts similar arguments.

server_hostname=None,
ssl_handshake_timeout=None):
"""Upgrade a transport to TLS.

Return a new transport that *protocol* should start using
immediately.
"""
raise NotImplementedError

async def create_unix_connection(
self, protocol_factory, path=None, *,
ssl=None, sock=None,
Expand Down
2 changes: 2 additions & 0 deletions Lib/asyncio/proactor_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ class _ProactorBaseWritePipeTransport(_ProactorBasePipeTransport,
transports.WriteTransport):
"""Transport for write pipes."""

_start_tls_compatible = True

def write(self, data):
if not isinstance(data, (bytes, bytearray, memoryview)):
raise TypeError(
Expand Down
2 changes: 2 additions & 0 deletions Lib/asyncio/selector_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,8 @@ def get_write_buffer_size(self):

class _SelectorSocketTransport(_SelectorTransport):

_start_tls_compatible = True

def __init__(self, loop, sock, protocol, waiter=None,
extra=None, server=None):
super().__init__(loop, sock, protocol, extra, server)
Expand Down
Loading