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.

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

SSLContext.post_handshake_auth implicitly enables cert validation #81609

Closed
tiran opened this issue Jun 27, 2019 · 14 comments
Closed

SSLContext.post_handshake_auth implicitly enables cert validation #81609

tiran opened this issue Jun 27, 2019 · 14 comments
Assignees
@tiran
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes topic-SSL type-bug An unexpected behavior, bug, or error

Comments

@tiran
Copy link
Member

tiran commented Jun 27, 2019

BPO 37428
Nosy @tiran, @benjaminp, @ned-deily, @alex, @ambv, @The-Compiler, @miss-islington, @iritkatriel
PRs
  • bpo-37428: Don't set PHA verify flag on client side #14421
  • [3.7] bpo-37428: Don't set PHA verify flag on client side (GH-14421) #14493
  • [3.8] bpo-37428: Don't set PHA verify flag on client side (GH-14494) #14494
    		•

    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/tiran'
closed_at = <Date 2020-10-17.02:23:07.682>
created_at = <Date 2019-06-27.10:33:29.395>
labels = ['expert-SSL', '3.8', 'type-bug', '3.7', '3.9']
title = 'SSLContext.post_handshake_auth implicitly enables cert validation'
updated_at = <Date 2020-10-17.02:23:07.681>
user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2020-10-17.02:23:07.681>
actor = 'benjamin.peterson'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2020-10-17.02:23:07.682>
closer = 'benjamin.peterson'
components = ['SSL']
creation = <Date 2019-06-27.10:33:29.395>
creator = 'christian.heimes'
dependencies = []
files = []
hgrepos = []
issue_num = 37428
keywords = ['patch']
message_count = 14.0
messages = ['346725', '346787', '346818', '346819', '346824', '346880', '346960', '346965', '346971', '347158', '350286', '350656', '350701', '378777']
nosy_count = 8.0
nosy_names = ['christian.heimes', 'benjamin.peterson', 'ned.deily', 'alex', 'lukasz.langa', 'The Compiler', 'miss-islington', 'iritkatriel']
pr_nums = ['14421', '14493', '14494']
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue37428'
versions = ['Python 2.7', 'Python 3.7', 'Python 3.8', 'Python 3.9']
    @tiran
    Copy link
    Member Author

    tiran commented Jun 27, 2019

    Enabling TLS 1.3 post handshake auth also enables cert chain validation. OpenSSL documents SSL_VERIFY_POST_HANDSHAKE as ignored for client side. However tls_process_server_certificate in the client state machine code does not ignore the flag and checks for a correct cert chain.

    see openssl/openssl#9259 and https://github.com/openssl/openssl/blob/743694a6c29e5a6387819523fad5e3b7e613f1ee/ssl/statem/statem_clnt.c#L1899-L1918

    @tiran tiran added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes labels Jun 27, 2019
    @tiran tiran self-assigned this Jun 27, 2019
    @tiran tiran added topic-SSL type-bug An unexpected behavior, bug, or error labels Jun 27, 2019
    @ned-deily
    Copy link
    Member

    Christian, just confirming that, since you have not set this as a "release blocker", 3.7.4 will go out without it.

    @tiran
    Copy link
    Member Author

    tiran commented Jun 28, 2019

    This issue breaks some stuff at work. I would appreciate if we can get the fix into 3.7.4. I wasn't aware that we are so close to cut-off to 3.7.4 release.

    What does the fix do?
    I moved all PHA related flags / options from SSL_CTX* to SSL*. The flags and options now depend on the socket type and existing flags.

    For a server-side socket, the SSL_VERIFY_POST_HANDSHAKE verify flag is now only set when the server socket is configured to verify client certs. Server sockets without SSL_VERIFY_PEER flag don't set the option. The presence of SSL_VERIFY_POST_HANDSHAKE without SSL_VERIFY_PEER sometimes triggers handshake errors like "extension not received". The official documentation says "This flag must be used together with SSL_VERIFY_PEER.". The ssl.CERT_OPTIONAL and ssl.CERT_REQURED both set SSL_VERIFY_PEER. SSL_set_post_handshake_auth() is not enabled for server-side sockets.

    For client side sockets, PHA is only enabled with SSL_set_post_handshake_auth(ssl, 1). The SSL_VERIFY_POST_HANDSHAKE flag is no longer set.

    https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_post_handshake_auth.html

    @ned-deily
    Copy link
    Member

    "Assuming no critical problems are found prior to 2019-06-28, no code changes are planned between these release candidates and the final releases."

    We were planning to start producing the final release artifacts in a couple of hours so we need to make a decision very quickly. If you think this is necessary, we can delay the release a bit. In the worst case, we could do a second release candidate but I *really* do not want to do that unless it is absolutely necessary. Also note that with 3.7.4, we have updated the Windows and macOS installers to use OpenSSL 1.1.1 (c) instead of 1.1.0.

    @tiran
    Copy link
    Member Author

    tiran commented Jun 28, 2019

    There are currently two issues with TLS 1.3 in Python. The issue https://bugs.python.org/issue37440 can be worked around easily with a custom SSLContext. This issue is a bigger problem because there is no possible workaround.

    The bug is going to break applications that verify clients with a certificate but accept untrusted server certificates. It's not a common scenario, but I just happen to run into this issue for a project at work.

    I'm sorry for the mess. :( I noticed the bug a couple of days ago. It took me a while to understand the root cause.

    @ned-deily
    Copy link
    Member

    Christian, do you have an estimate for when these issues will be resolved? We are holding 3.7.4 right now.

    @miss-islington
    Copy link
    Contributor

    New changeset f0f5930 by Miss Islington (bot) (Christian Heimes) in branch 'master':
    bpo-37428: Don't set PHA verify flag on client side (GH-14421)
    f0f5930

    @miss-islington
    Copy link
    Contributor

    New changeset cf76174 by Miss Islington (bot) in branch '3.7':
    [3.7] bpo-37428: Don't set PHA verify flag on client side (GH-14421) (GH-14493)
    cf76174

    @tiran
    Copy link
    Member Author

    tiran commented Jul 1, 2019

    New changeset f22c4cf by Christian Heimes in branch '3.8':
    [3.8] bpo-37428: Don't set PHA verify flag on client side (GH-14494)
    f22c4cf

    @ned-deily
    Copy link
    Member

    New changeset 5b45fb0 by Ned Deily (Miss Islington (bot)) in branch '3.7':
    [3.7] bpo-37428: Don't set PHA verify flag on client side (GH-14421) (GH-14493)
    5b45fb0

    @ambv
    Copy link
    Contributor

    ambv commented Aug 23, 2019

    Should this be closed?

    @tiran
    Copy link
    Member Author

    tiran commented Aug 28, 2019

    3.7 to 3.9 are fixed.

    Benjamin, do you want the fix in 2.7?

    @benjaminp
    Copy link
    Contributor

    Yes, please.

    @iritkatriel
    Copy link
    Member

    Can this be closed? 2.7 is no longer relevant.

    @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

    @tiran tiran

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

    No branches or pull requests
    6 participants
    @ambv @benjaminp @tiran @iritkatriel @ned-deily @miss-islington