classification
Title: asyncio/ssl: Fix AttributeError, increase default handshake timeout
Type: behavior Stage: resolved
Components: asyncio Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: yselivanov Nosy List: asvetlov, hynek, ned.deily, yselivanov
Priority: Keywords: patch

Created on 2018-06-01 14:53 by yselivanov, last changed 2018-06-04 16:41 by ned.deily. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 7321 merged yselivanov, 2018-06-01 14:56
PR 7396 merged miss-islington, 2018-06-04 15:33
Messages (11)
msg318422 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-06-01 14:53
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
msg318423 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-06-01 14:54
For the reference, we added SSL handshake timeout a while ago in bpo-29970.
msg318429 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2018-06-01 16:30
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.
msg318430 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-06-01 16:35
> 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.
msg318457 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2018-06-01 20:20
> Previous timeout was effectively infinite.

Oi, well then 60s are an improvement indeed. :)
msg318661 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-06-04 15:32
New changeset 9602643120a509858d0bee4215d7f150e6125468 by Yury Selivanov in branch 'master':
bpo-33734: asyncio/ssl: a bunch of bugfixes (#7321)
https://github.com/python/cpython/commit/9602643120a509858d0bee4215d7f150e6125468
msg318665 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-06-04 16:05
New changeset 87936d03cb29ca039c5799190e8da764e62b7882 by Yury Selivanov (Miss Islington (bot)) in branch '3.7':
bpo-33734: asyncio/ssl: a bunch of bugfixes (GH-7321) (GH-7396)
https://github.com/python/cpython/commit/87936d03cb29ca039c5799190e8da764e62b7882
msg318666 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-06-04 16:07
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.
msg318672 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-06-04 16:36
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!
msg318673 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-06-04 16:41
> 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?
msg318674 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-06-04 16:41
> 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.
History
Date User Action Args
2018-06-04 16:41:56ned.deilysetmessages: + msg318674
2018-06-04 16:41:06yselivanovsetmessages: + msg318673
2018-06-04 16:36:40ned.deilysetstatus: open -> closed
priority: release blocker ->
messages: + msg318672

resolution: fixed
stage: patch review -> resolved
2018-06-04 16:07:38yselivanovsetmessages: + msg318666
2018-06-04 16:05:48yselivanovsetmessages: + msg318665
2018-06-04 15:33:48miss-islingtonsetpull_requests: + pull_request7022
2018-06-04 15:32:40yselivanovsetmessages: + msg318661
2018-06-01 20:20:21hyneksetmessages: + msg318457
2018-06-01 16:35:28yselivanovsetmessages: + msg318430
2018-06-01 16:30:15hyneksetmessages: + msg318429
2018-06-01 14:56:32yselivanovsetkeywords: + patch
stage: patch review
pull_requests: + pull_request6950
2018-06-01 14:54:53yselivanovsetmessages: + msg318423
2018-06-01 14:53:53yselivanovcreate