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

Add set_post_handshake_auth for TLS 1.3 #78851

Closed
tiran opened this issue Sep 14, 2018 · 8 comments
Closed

Add set_post_handshake_auth for TLS 1.3 #78851

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

Comments

@tiran
Copy link
Member

tiran commented Sep 14, 2018

BPO 34670
Nosy @tiran, @benjaminp, @ned-deily, @njsmith, @xnox, @miss-islington
PRs
  • bpo-34670: Add TLS 1.3 post handshake auth #9460
  • [3.7] bpo-34670: Add TLS 1.3 post handshake auth (GH-9460) #9505
  • [3.6] bpo-34670: Add TLS 1.3 post handshake auth (GH-9460) #9507
    		•

    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 2021-04-19.19:53:59.522>
created_at = <Date 2018-09-14.00:16:08.840>
labels = ['expert-SSL', '3.8', 'type-bug', '3.7']
title = 'Add set_post_handshake_auth for TLS 1.3'
updated_at = <Date 2021-04-19.19:53:59.522>
user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2021-04-19.19:53:59.522>
actor = 'christian.heimes'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2021-04-19.19:53:59.522>
closer = 'christian.heimes'
components = ['SSL']
creation = <Date 2018-09-14.00:16:08.840>
creator = 'christian.heimes'
dependencies = []
files = []
hgrepos = []
issue_num = 34670
keywords = ['patch']
message_count = 8.0
messages = ['325314', '325973', '326138', '326141', '326142', '326458', '327932', '391394']
nosy_count = 6.0
nosy_names = ['christian.heimes', 'benjamin.peterson', 'ned.deily', 'njs', 'xnox', 'miss-islington']
pr_nums = ['9460', '9505', '9507']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue34670'
versions = ['Python 2.7', 'Python 3.6', 'Python 3.7', 'Python 3.8']
    @tiran
    Copy link
    Member Author

    tiran commented Sep 14, 2018

    TLS 1.3 removed renegotiation in favor of rekeying and post handshake authentication (PHA). With PHA, a server can request a client certificate from a client at some point after the handshake. The feature is commonly used by HTTP server for conditional and path specific TLS client auth. For example a server can decide to require a cert based on HTTP method and/or path. A client must announce support for PHA during the handshake

    Apache mod_ssl uses PHA, https://github.com/apache/httpd/blob/trunk/modules/ssl/ssl_engine_kernel.c#L1207

    As of OpenSSL ticket openssl/openssl#6933, TLS 1.3 clients no longer send the PHA TLS extension by default. Nikos and I requested the change, because PHA breaks some assumptions of TLS 1.2 clients. For on-demand auth, PHA extension must be enabled with SSL_CTX_set_post_handshake_auth(), https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_post_handshake_auth.html .

    I propose to add a property on SSLContext to enable PHA and backport the change to Python 2.7, 3.6 and 3.7.

    In order to test the feature, I'd also have to add some flags and a function for the server side: (SSL_VERIFY_CLIENT_ONCE, SSL_VERIFY_POST_HANDSHAKE, SSL_verify_client_post_handshake()).

    @tiran tiran added 3.7 (EOL) end of life 3.8 only security fixes labels Sep 14, 2018
    @tiran tiran self-assigned this Sep 14, 2018
    @tiran tiran added topic-SSL type-bug An unexpected behavior, bug, or error labels Sep 14, 2018
    @tiran
    Copy link
    Member Author

    tiran commented Sep 21, 2018

    Please note that SSL_verify_client_post_handshake() doesn't perform any IO by itself.

    A typical scenario for HTTP looks like this (actual flow may vary):

    • client
      • send HTTP GET /path
    • server
      • recv
      • verify_client_post_handshake
      • send HTTP Connection Upgrade (emits CertRequest message)
    • client
      • recv
      • send upgrade confirmation (emits Certificate, CertificateVerify, Finish message)
    • server
      • recv
      • verify certificate
      • send payload or error (may emit TLS alert for unknown, invalid, or missing cert)
    • client
      • recv (receive TLS alert or server response)

    @miss-islington
    Copy link
    Contributor

    New changeset 9fb051f by Miss Islington (bot) (Christian Heimes) in branch 'master':
    bpo-34670: Add TLS 1.3 post handshake auth (GH-9460)
    9fb051f

    @miss-islington
    Copy link
    Contributor

    New changeset 2756ef3 by Miss Islington (bot) (Christian Heimes) in branch '3.7':
    [3.7] bpo-34670: Add TLS 1.3 post handshake auth (GH-9460) (GH-9505)
    2756ef3

    @miss-islington
    Copy link
    Contributor

    New changeset 94812f7 by Miss Islington (bot) (Christian Heimes) in branch '3.6':
    [3.6] bpo-34670: Add TLS 1.3 post handshake auth (GH-9460) (GH-9507)
    94812f7

    @xnox
    Copy link
    Mannequin

    xnox mannequin commented Sep 26, 2018

    Will this be backported to the 2.7 branch as well? Pretty please =)

    @njsmith
    Copy link
    Contributor

    njsmith commented Oct 18, 2018

    FYI Christian, your "typical scenario for HTTP" doesn't make sense to me... you can't send HTTP Connection Upgrade in the middle of a regular request/response cycle. I feel like the typical scenario ought to be more like:

    • client
      • send HTTP GET /path
    • server
      • recv
      • verify_client_post_handshake (maybe... via calling SSL_do_handshake again?)
    • client
      • recv
      • send upgrade confirmation (emits Certificate, CertificateVerify, Finish message)
    • server
      • recv
      • verify certificate
      • send either the requested response, or a 401 Unauthorized depending

    But I don't really understand the underlying design here, either at the TLS 1.3 level or the openssl level, and haven't found very useful docs yet, so I could be wrong.

    @tiran
    Copy link
    Member Author

    tiran commented Apr 19, 2021

    I don't think is anything left to do here. PHA has been supported for a while and I haven't seen any problems.

    @tiran tiran closed this as completed Apr 19, 2021
    @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 topic-SSL type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @tiran @njsmith @miss-islington