Skip to content
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.

Sign up for GitHub

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

Jump to bottom

asyncio/ssl: Fix AttributeError, increase default handshake timeout #77915

Closed
1st1 opened this issue Jun 1, 2018 · 11 comments
Closed

asyncio/ssl: Fix AttributeError, increase default handshake timeout #77915

1st1 opened this issue Jun 1, 2018 · 11 comments
Assignees
@1st1
Labels
3.7 (EOL) end of life 3.8 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@1st1
Copy link
Member

1st1 commented Jun 1, 2018

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
    Copy link
    Member Author

    1st1 commented Jun 1, 2018

    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 1st1 added release-blocker 3.7 (EOL) end of life 3.8 only security fixes labels Jun 1, 2018
    @1st1 1st1 self-assigned this Jun 1, 2018
    @1st1 1st1 added topic-asyncio type-bug An unexpected behavior, bug, or error labels Jun 1, 2018
    @1st1
    Copy link
    Member Author

    1st1 commented Jun 1, 2018

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

    @hynek
    Copy link
    Member

    hynek commented Jun 1, 2018

    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
    Copy link
    Member Author

    1st1 commented Jun 1, 2018

    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
    Copy link
    Member

    hynek commented Jun 1, 2018

    Previous timeout was effectively infinite.

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

    @1st1
    Copy link
    Member Author

    1st1 commented Jun 4, 2018

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

    @1st1
    Copy link
    Member Author

    1st1 commented Jun 4, 2018

    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
    Copy link
    Member Author

    1st1 commented Jun 4, 2018

    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
    Copy link
    Member

    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
    Copy link
    Member Author

    1st1 commented Jun 4, 2018

    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
    Copy link
    Member

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @1st1 1st1

    Labels
    3.7 (EOL) end of life 3.8 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @hynek @1st1 @ned-deily