asyncio/ssl: Fix AttributeError, increase default handshake timeout

[1st1]

BPO	33734
Nosy	@ned-deily, @asvetlov, @hynek, @1st1
PRs	bpo-33734: asyncio/ssl: Fix AttributeError, increase default handshake timeout #7321 [3.7] bpo-33734: asyncio/ssl: a bunch of bugfixes (GH-7321) #7396

^{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/1st1'
closed_at = <Date 2018-06-04.16:36:40.526>
created_at = <Date 2018-06-01.14:53:53.612>
labels = ['3.8', 'type-bug', '3.7', 'expert-asyncio']
title = 'asyncio/ssl: Fix AttributeError, increase default handshake timeout'
updated_at = <Date 2018-06-04.16:41:56.200>
user = 'https://github.com/1st1'

bugs.python.org fields:

activity = <Date 2018-06-04.16:41:56.200>
actor = 'ned.deily'
assignee = 'yselivanov'
closed = True
closed_date = <Date 2018-06-04.16:36:40.526>
closer = 'ned.deily'
components = ['asyncio']
creation = <Date 2018-06-01.14:53:53.612>
creator = 'yselivanov'
dependencies = []
files = []
hgrepos = []
issue_num = 33734
keywords = ['patch']
message_count = 11.0
messages = ['318422', '318423', '318429', '318430', '318457', '318661', '318665', '318666', '318672', '318673', '318674']
nosy_count = 4.0
nosy_names = ['ned.deily', 'asvetlov', 'hynek', 'yselivanov']
pr_nums = ['7321', '7396']
priority = None
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue33734'
versions = ['Python 3.7', 'Python 3.8']

[1st1]

I've ported asyncio's sslproto.py to uvloop and released a new major version of it yesterday. Hynek discovered that the default SSL handshake timeout (10 seconds currently) is too low, and that there's a critical code path that is broken because it assumes all SSL exceptions have an 'errno' attribute.

The PR changes the default SSL handshake timeout to 60 seconds (as in nginx [1]) and fixes the AttributeError.

IMO this should go into 3.7.0rc1.

[1] https://docs.nginx.com/nginx/admin-guide/security-controls/terminating-ssl-tcp/#speeding-up-secure-tcp-connections

[1st1]

For the reference, we added SSL handshake timeout a while ago in bpo-29970.

[hynek]

For some context: 10s seems to be more common than I liked to believe (seems like Go's http client uses it by default too).

Nevertheless I ran into the 10s after updating uvloop and stopped being able to connect to a server in India. Therefore I'd consider 10s at least a regression that should be fixed.

What was the effective timeout before? Depending on the old value, 60s could be excessive for clients and might lead to self-DoS on the client side…

P.S. I tried to reply on my phone and now I fully support Mariatta’s proposal of moving to GitHub issues.

[1st1]

What was the effective timeout before? Depending on the old value, 60s could be excessive for clients and might lead to self-DoS on the client side…

Previous timeout was effectively infinite.

[hynek]

Previous timeout was effectively infinite.

Oi, well then 60s are an improvement indeed. :)

[1st1]

New changeset 9602643 by Yury Selivanov in branch 'master':
bpo-33734: asyncio/ssl: a bunch of bugfixes (bpo-7321)
9602643

[1st1]

New changeset 87936d0 by Yury Selivanov (Miss Islington (bot)) in branch '3.7':
bpo-33734: asyncio/ssl: a bunch of bugfixes (GH-7321) (GH-7396)
87936d0

[1st1]

Fixes are in master and 3.7 now, so this should be fixed in 3.7.0rc1.

Ned, I'm not sure what's the workflow here, please feel free to close this issue and change its priority.

[ned-deily]

Thanks for the fix! Since it apparently does not apply to 3.6, we should just close it like any other issue, right? So doing!

[1st1]

Since it apparently does not apply to 3.6, we should just close it like any other issue, right? So doing!

Sure. My understanding is that all changes including this one in 3.7 branch will end up in 3.7.0rc1, right?

[ned-deily]

My understanding is that all changes including this one in 3.7 branch will end up in 3.7.0rc1, right?

Yes. Don't worry, the train won't leave without it.