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

Support for the NPN extension to TLS/SSL #58412

Closed
colinmarc mannequin opened this issue Mar 5, 2012 · 18 comments
Closed

Support for the NPN extension to TLS/SSL #58412

colinmarc mannequin opened this issue Mar 5, 2012 · 18 comments
Labels
type-feature A feature request or enhancement

Comments

@colinmarc
Copy link
Mannequin

colinmarc mannequin commented Mar 5, 2012

BPO 14204
Nosy @loewis, @jcea, @pitrou, @Sidnicious
Files
  • npn_patch.diff
  • npn_patch_py3.diff
  • npn_patch_py3.diff
  • npn_openssl_ref.c: relevant openssl code
  • npn_patch_py3.diff
  • npn_patch_py3.diff
  • npn.patch
  • 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 = None
    closed_at = <Date 2012-03-22.01:05:00.082>
    created_at = <Date 2012-03-05.20:21:01.380>
    labels = ['type-feature']
    title = 'Support for the NPN extension to TLS/SSL'
    updated_at = <Date 2012-05-02.19:31:37.027>
    user = 'https://bugs.python.org/colinmarc'

    bugs.python.org fields:

    activity = <Date 2012-05-02.19:31:37.027>
    actor = 'colinmarc'
    assignee = 'none'
    closed = True
    closed_date = <Date 2012-03-22.01:05:00.082>
    closer = 'pitrou'
    components = []
    creation = <Date 2012-03-05.20:21:01.380>
    creator = 'colinmarc'
    dependencies = []
    files = ['24739', '24775', '24777', '24778', '24786', '24797', '24916']
    hgrepos = []
    issue_num = 14204
    keywords = ['patch']
    message_count = 18.0
    messages = ['154973', '154978', '154979', '154980', '154982', '154983', '155326', '155335', '155350', '155408', '155475', '156193', '156198', '156525', '156528', '159815', '159816', '159817']
    nosy_count = 7.0
    nosy_names = ['loewis', 'jcea', 'pitrou', 'marcelo_fernandez', 'python-dev', 'colinmarc', 'ssm']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue14204'
    versions = ['Python 3.3']

    @colinmarc
    Copy link
    Mannequin Author

    colinmarc mannequin commented Mar 5, 2012

    Recent versions of OpenSSL (1.0.1 and greater) support a new extension to SSL/TLS called Next Protocol Negotiation, defined here: http://tools.ietf.org/html/draft-agl-tls-nextprotoneg-02.

    The extension allows servers and clients to advertise which protocols they support (for example, both HTTP and SPDY) and then agree on one during the handshake according to a simple algorithm.

    This patch to 2.7 adds support for the NPN extension via another parameter to ssl.wrap_socket, called 'npn_protocols', and by using the OpenSSL API. It should fail gracefully if the linked version of OpenSSL has no support for NPN, using a macro guard. Once the handshake is completed, SSLSocket.selected_protocol() returns whatever was agreed upon.

    Although I included client/server tests with the patch, testing this functionality in real-life situations proved difficult. Google chrome has SPDY and NPN functionality baked in, so I wrote a simple socket server that advertises SPDY/2 in addition to HTTP/1.1. Chrome, pointed at this server, correctly completed the handshake and started merrily sending SPDY control frames.

    @colinmarc colinmarc mannequin added the type-feature A feature request or enhancement label Mar 5, 2012
    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 5, 2012

    There is zero chance that this can go into 2.7. So if you want to see it included, please port it to Python 3, and it may become part of Python 3.3 or 3.4.

    @colinmarc
    Copy link
    Mannequin Author

    colinmarc mannequin commented Mar 5, 2012

    If I ported it to 3.3 or 3.4, would it then be backported to 2.7? Or is there zero chance of that either? If so, why? I apologize, I'm new to the process.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 5, 2012

    If I ported it to 3.3 or 3.4, would it then be backported to 2.7? Or
    is there zero chance of that either? If so, why? I apologize, I'm new
    to the process.

    It won't be backported. Python 2.7 is in bug-fix mode; no new features
    are allowed it it. In addition, there won't be another 2.x release
    (see PEP-404), so new features can only be added to Python 3.

    If this means that you'll lose interest in this issue - that's fine.
    Let us know whether you then would rather withdraw the patch, or
    leave it open in case someone is motivated to port it. In the latter
    case, please submit a contributor's form to the PSF.

    @pitrou
    Copy link
    Member

    pitrou commented Mar 5, 2012

    Hello Marc,

    Recent versions of OpenSSL (1.0.1 and greater) support a new extension
    to SSL/TLS called Next Protocol Negotiation, defined here:
    http://tools.ietf.org/html/draft-agl-tls-nextprotoneg-02.

    Apparently this is an IETF draft. Do you know if it is stabilized enough that it won't change significantly?

    Also, please notice that the ssl module (starting from Python 3.2) now exposes the notion of an SSL context. The setting of NPN parameters should probably be exposed as a context method and/or a parameter to SSLContext.wrap_socket().
    (see http://docs.python.org/dev/library/ssl.html#ssl-contexts for docs)

    @colinmarc
    Copy link
    Mannequin Author

    colinmarc mannequin commented Mar 5, 2012

    Re the IETF draft: I'm not sure. However, I didn't actually have to implement the specification at all - that was all handled by OpenSSL. My patch just calls the appropriate SSL_CTX_* methods.

    Thanks for the tip. I'm still interested in this getting included, so I'll work on porting it over.

    @colinmarc
    Copy link
    Mannequin Author

    colinmarc mannequin commented Mar 10, 2012

    Here's an updated patch against 3.3.

    @colinmarc
    Copy link
    Mannequin Author

    colinmarc mannequin commented Mar 10, 2012

    Oops, I had my vim configured wrong and left a few tab characters in there. Here's another updated patch =)

    @colinmarc
    Copy link
    Mannequin Author

    colinmarc mannequin commented Mar 10, 2012

    Here's the OpenSSL code I referenced for my implementation. It's an excerpt of ssl/lib_ssl.c, starting at line 1514.

    @colinmarc
    Copy link
    Mannequin Author

    colinmarc mannequin commented Mar 11, 2012

    Updated patch.

    @colinmarc
    Copy link
    Mannequin Author

    colinmarc mannequin commented Mar 12, 2012

    More updates to the patch.

    @pitrou
    Copy link
    Member

    pitrou commented Mar 17, 2012

    Sorry for the delay. I've run the tests (with OpenSSL 1.0.1-beta3) in debug mode and got an error:

    ======================================================================
    ERROR: test_npn_ext (test.test_ssl.ThreadedTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/antoine/cpython/default/Lib/test/test_ssl.py", line 1882, in test_npn_ext
        chatty=True, connectionchatty=True)
      File "/home/antoine/cpython/default/Lib/test/test_ssl.py", line 1210, in server_params_test
        s.connect((HOST, server.port))
      File "/home/antoine/cpython/default/Lib/ssl.py", line 543, in connect
        self._real_connect(addr, False)
      File "/home/antoine/cpython/default/Lib/ssl.py", line 533, in _real_connect
        self.do_handshake()
      File "/home/antoine/cpython/default/Lib/ssl.py", line 513, in do_handshake
        self._sslobj.do_handshake()
    ssl.SSLError: [Errno 1] _ssl.c:434: error:140920E3:SSL routines:SSL3_GET_SERVER_HELLO:parse tlsext

    I've determined that this is because of the use of strlen() on a non-zero terminated string. I'll try to come up with an updated patch.

    @pitrou
    Copy link
    Member

    pitrou commented Mar 17, 2012

    Here is a fixed patch. It also came to me that "selected_protocol" could be ambiguous, so I renamed it to "selected_npn_protocol".

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 21, 2012

    New changeset 2514a4e2b3ce by Antoine Pitrou in branch 'default':
    Issue bpo-14204: The ssl module now has support for the Next Protocol Negotiation extension, if available in the underlying OpenSSL library.
    http://hg.python.org/cpython/rev/2514a4e2b3ce

    @pitrou
    Copy link
    Member

    pitrou commented Mar 22, 2012

    Closing since the buildbots don't seem to show any new failures after the commit. Thank you for your contribution!

    @pitrou pitrou closed this as completed Mar 22, 2012
    @colinmarc
    Copy link
    Mannequin Author

    colinmarc mannequin commented May 2, 2012

    Just noticed this is missing from "What's new in Python 3.3": http://docs.python.org/dev/whatsnew/3.3.html.

    Should I submit a patch for that?

    @pitrou
    Copy link
    Member

    pitrou commented May 2, 2012

    Just noticed this is missing from "What's new in Python 3.3": http://docs.python.org/dev/whatsnew/3.3.html.

    Should I submit a patch for that?

    No need for that, the What's New document usually gets filled later in the release cycle.

    @colinmarc
    Copy link
    Mannequin Author

    colinmarc mannequin commented May 2, 2012

    Ah ok, just curious. Thanks!

    @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
    Labels
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant