start_tls() difficult when using asyncio.start_server()

[icgood]

BPO	34975
Nosy	@gpshead, @asvetlov, @1st1, @icgood
PRs	bpo-34975: Add start_tls() method to streams API #13143
Superseder	bpo-36889: Merge StreamWriter and StreamReader into just asyncio.Stream

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/asvetlov'
closed_at = None
created_at = <Date 2018-10-13.20:03:13.550>
labels = ['3.11', 'type-feature', 'expert-asyncio']
title = 'start_tls() difficult when using asyncio.start_server()'
updated_at = <Date 2022-04-08.00:05:00.270>
user = 'https://github.com/icgood'

bugs.python.org fields:

activity = <Date 2022-04-08.00:05:00.270>
actor = 'gregory.p.smith'
assignee = 'asvetlov'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2018-10-13.20:03:13.550>
creator = 'icgood'
dependencies = []
files = []
hgrepos = []
issue_num = 34975
keywords = ['patch']
message_count = 9.0
messages = ['327665', '327674', '327678', '342012', '342215', '343679', '355586', '355587', '416953']
nosy_count = 4.0
nosy_names = ['gregory.p.smith', 'asvetlov', 'yselivanov', 'icgood']
pr_nums = ['13143']
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = '36889'
type = 'enhancement'
url = 'https://bugs.python.org/issue34975'
versions = ['Python 3.11']

[icgood]

There does not seem to be a public API for replacing the transport of the StreamReader / StreamWriter provided to the callback of a call to asyncio.start_server().

The only way I have found to use the new SSL transport is to update protected members of the StreamReaderProtocol object, e.g.:

    async def callback(reader, writer):
        loop = asyncio.get_event_loop()
        transport = writer.transport
        protocol = transport.get_protocol()
        new_transport = await loop.start_tls(
            transport, protocol, ssl_context, server_side=True)
        protocol._stream_reader = StreamReader(loop=loop)
        protocol._client_connected_cb = do_stuff_after_start_tls
        protocol.connection_made(new_transport)

    async def do_stuff_after_start_tls(ssl_reader, ssl_writer): ...

[asvetlov]

Thanks for raising the problem.
I'm in the middle of streams API refactoring.

https://github.com/asvetlov/cpython/blob/async-streams/Lib/asyncio/streams.py#L801-L812 is a draft.

A help is very appreciated.
Would you pick up this snippet and make a pull request with tests and documentation update?

[1st1]

One thing: I'm -1 on adding starttls to current stream api; let's add it only to the new one (same for sendfile)

[icgood]

I added start_tls() to StreamWriter. My implementation returns a new StreamWriter that should be used from then on, but it could be adapted to modify the current writer in-place (let me know).

I've added docs, an integration test, and done some additional "real-world" testing with an IMAP server I work on. Specifically, "openssl s_client -connect xxx -starttls imap" works like a charm, and no errors/warnings are logged on disconnect.

[asvetlov]

I believe stream transport should be replaced.
If you have time please do this change.

Also please take a look at bpo-36889 for merging StreamWriter and StreamReader.
It can simplify the replacement strategy.

Anyway, please keep your PR open at least.
The plan is: either merge your PR before writer/reader merging or land unified streams first and adopt the PR to new API.

Another thing that should be done before 3.8 feature freeze is support for sendfile in streams API. Hopefully, it is much simpler: no need for transport changing etc.

[asvetlov]

Fixed by bpo-36889

[icgood]

bpo-36889 was reverted, so this is not resolved.

I'm guessing this needs to be moved to 3.9 now too. Is my original PR worth revisiting? https://github.com/python/cpython/pull/13143/files

[asvetlov]

I think it should be closed; Yuri thinks that current streams API is frozen for the sake of shiny brand new streams (don't exist yet).

[gpshead]

https://discuss.python.org/t/need-reconsideration-of-bpo-34975-add-start-tls-method-to-streams-api/14720 would like to see this reconsidered. reopening.

[kumaraditya303]

Fixed by #91453