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

add support for tls/ssl sessions in asyncio #79152

Closed
RemiCardona mannequin opened this issue Oct 13, 2018 · 15 comments
Closed

add support for tls/ssl sessions in asyncio #79152

RemiCardona mannequin opened this issue Oct 13, 2018 · 15 comments
Labels
topic-asyncio type-feature A feature request or enhancement

Comments

@RemiCardona
Copy link
Mannequin

RemiCardona mannequin commented Oct 13, 2018

BPO 34971
Nosy @tiran, @asvetlov, @1st1, @RemiCardona, @cooperlees
PRs
  • bpo-34971: add support for TLS sessions from asyncio #9840
  • 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 = None
    created_at = <Date 2018-10-13.08:47:40.895>
    labels = ['type-feature', '3.8', 'expert-asyncio']
    title = 'add support for tls/ssl sessions in asyncio'
    updated_at = <Date 2019-06-06.16:18:48.945>
    user = 'https://github.com/RemiCardona'

    bugs.python.org fields:

    activity = <Date 2019-06-06.16:18:48.945>
    actor = 'cooperlees'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['asyncio']
    creation = <Date 2018-10-13.08:47:40.895>
    creator = 'RemiCardona'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34971
    keywords = ['patch']
    message_count = 9.0
    messages = ['327638', '327641', '328243', '328244', '329407', '329410', '329422', '335558', '335676']
    nosy_count = 5.0
    nosy_names = ['christian.heimes', 'asvetlov', 'yselivanov', 'RemiCardona', 'cooperlees']
    pr_nums = ['9840']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue34971'
    versions = ['Python 3.8']

    @RemiCardona
    Copy link
    Mannequin Author

    RemiCardona mannequin commented Oct 13, 2018

    cpython has had TLS session support since 3.6, using the SSLContext.wrap_* methods. Unfortunately, this support is not available when using asyncio's create_connection.

    While I've managed to monkeypatch asyncio.sslproto._SSLPipe from my own code (it's a filthy hack but it's short and it gets the job done) running on 3.6.6, I feel this should be properly supported out of the box.

    A patch is ready (tests work), a github PR will be created shortly.

    Notes in no particular order:

    • argument and attribute naming is all over the place, but I could not decide between "sslsession" (matching "sslcontext") and "ssl_session" (matching "ssl_handshake_timeout") so I just picked one
    • tested on jessie (with openssl 1.0.2 from jessie-backports) and on gentoo
    • the new asyncio tests added in the patch are adapted from test_ssl.py's test_session, with the server-side stats left out. I felt they were not useful if one assumes that the hard work is done by SSLContext.wrap_*.
    • I did not reuse test_asyncio.utils.run_test_server which AIUI creates a new server-side context for each incoming connection, thus breaking sessions completely

    TIA for considering this bug and patch

    @RemiCardona RemiCardona mannequin added topic-asyncio type-feature A feature request or enhancement labels Oct 13, 2018
    @asvetlov
    Copy link
    Contributor

    TLS session support is awesome.

    IFAIK ssl_proto.py is under heavy reconstruction now.
    Please coordinate your work with Yuri.

    @RemiCardona
    Copy link
    Mannequin Author

    RemiCardona mannequin commented Oct 22, 2018

    Hi Andrew,

    How should I proceed? What's the best avenue to get in touch with Yuri?

    Thanks

    @tiran
    Copy link
    Member

    tiran commented Oct 22, 2018

    Don't use the existing session feature, yet. It only works for TLS 1.2 connections. TLS 1.3 behaves differently. There are multiple session tickets (usually two) and tickets are sent after handshake. Further more, Python lacks clear shutdown of a connection, which causes further problems with session handling. See https://www.openssl.org/docs/manmaster/man3/SSL_get_session.html

    @tiran tiran added the 3.8 (EOL) end of life label Oct 22, 2018
    @RemiCardona
    Copy link
    Mannequin Author

    RemiCardona mannequin commented Nov 7, 2018

    Hi Christian,

    Could you tell me more about this new openssl API? Right now my patch works with whatever the ssl module provides. Are you suggesting the ssl module is in some way incomplete? Would supporting TLS 1.3 sessions be incompatible with the current session API?

    I'd like to help wherever possible, but I'm probably missing some context and/or knowledge around all things TLS in cpython.

    Thanks

    @tiran
    Copy link
    Member

    tiran commented Nov 7, 2018

    The session code of the ssl is not compatible with TLS 1.3. Actually the whole API doesn't work with TLS 1.3. In TLS 1.2 and before, sessions had multiple security implications. For example they break PFS.

    TLS 1.3 changed when sessions are exchanged and how session are resumed. Session data is no longer part of the handshake. Instead the server can send session tickets at any point after the handshake. A server can send multiple tickets (usually two) and tickets must only be reused once.

    @RemiCardona
    Copy link
    Mannequin Author

    RemiCardona mannequin commented Nov 7, 2018

    So, IOW, the ssl module needs a good shakeup wrt TLS 1.3 sessions before any asyncio work can be merged. Am I getting this right?

    In which case, a whole other issue/PR is needed and possibly better folks than me. I try to stay clear of low-level crypto APIs because I don't trust myself to get things right. Well… I certainly can look at it, but I fear I may be punching above my weight with this.

    @1st1
    Copy link
    Member

    1st1 commented Feb 14, 2019

    Christian, do you think the sessions support shouldn't be added to asyncio in 3.8?

    @RemiCardona
    Copy link
    Mannequin Author

    RemiCardona mannequin commented Feb 16, 2019

    Anything I can do to get the ball rolling? Let me know who to get in touch with and *how*, and I will. Thanks

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @ezio-melotti ezio-melotti moved this to Todo in asyncio Jul 17, 2022
    @HMaker
    Copy link

    HMaker commented Oct 3, 2022

    A quick solution while SSL sessions aren't officially supported

    import ssl
    import socket
    import asyncio
    
    
    class SSLSessionBoundContext:
        """ssl.SSLContext bound to an existing SSL session.
        
        Actually asyncio doesn't support TLS session resumption, the loop.create_connection() API
        does not take any TLS session related argument. There is ongoing work to add support for this
        at https://github.com/python/cpython/issues/79152.
    
        The loop.create_connection() API takes a SSL context argument though, the SSLSessionBoundContext
        is used to wrap a SSL context and inject a SSL session on calls to
            - SSLSessionBoundContext.wrap_socket()
            - SSLSessionBoundContext.wrap_bio()
    
        This wrapper is compatible with any TLS application which calls only the methods above when
        making new TLS connections. This class is NOT a subclass of ssl.SSLContext, so it will be
        rejected by applications which ensure the SSL context is an instance of ssl.SSLContext. Not being
        a subclass of ssl.SSLContext makes this wrapper lightweight.
        """
        __slots__ = ('_context', '_session')
    
        def __init__(self, context: ssl.SSLContext, session: ssl.SSLSession):
            self._context = context
            self._session = session
    
        @property
        def session(self):
            return self._session
    
        def wrap_socket(
            self,
            sock: socket.socket,
            server_side: bool=False,
            do_handshake_on_connect: bool=True,
            suppress_ragged_eofs: bool=True,
            server_hostname: bool=None,
            session: ssl.SSLSession=None
        ) -> ssl.SSLSocket:
            if session is not None:
                raise ValueError('expected session to be None')
            return self._context.wrap_socket(
                sock=sock,
                server_hostname=server_hostname,
                server_side=server_side,
                do_handshake_on_connect=do_handshake_on_connect,
                suppress_ragged_eofs=suppress_ragged_eofs,
                session=self._session
            )
    
        def wrap_bio(
            self,
            incoming: ssl.MemoryBIO,
            outgoing: ssl.MemoryBIO,
            server_side: bool=False,
            server_hostname: bool=None,
            session: ssl.SSLSession=None
        ) -> ssl.SSLObject:
            if session is not None:
                raise ValueError('expected session to be None')
            return self._context.wrap_bio(
                incoming=incoming,
                outgoing=outgoing,
                server_hostname=server_hostname,
                server_side=server_side,
                session=self._session
            )
    
    
    async def test():
        context = ssl.create_default_context(ssl.Purpose.SERVER_AUTH)
        _, writer = await asyncio.open_connection('docs.python.org', 443, ssl=context)
        sslobj = writer.transport.get_extra_info('ssl_object')
        session = sslobj.session
        writer.close()
        await writer.wait_closed()
        # try to reuse session
        _, writer = await asyncio.open_connection(
            'docs.python.org',
            443,
            ssl=SSLSessionBoundContext(context, session)
        )
        sslobj = writer.transport.get_extra_info('ssl_object')
        print('Session reused:', sslobj.session_reused)
        writer.close()
        await writer.wait_closed()
    
    
    asyncio.run(test())

    Note that it just makes asyncio use an existing SSL session, this solution doesn't show how to correctly handle sessions as by the TLS spec. The standard SSL module still lacks needed bindings to handle TLS1.3 sessions, but from what I have seen SSLObject.session getter returns the last created session on the connection, so session reuse on TLS1.3 is still possible.

    @arhadthedev
    Copy link
    Member

    Can this issue and the associated PR be considered superseded with gh-79156 and gh-91453 respectively?

    @gvanrossum
    Copy link
    Member

    I think so. Closing everything in sight.

    Repository owner moved this from Todo to Done in asyncio Oct 4, 2022
    @HMaker
    Copy link

    HMaker commented Oct 4, 2022

    Sorry, but I can't see how gh-79156 supersedes this issue. From what I understood that other issue is about TLS upgrade on a stream, not about TLS session resumption.

    Under the hood loop.start_tls() is still called, and I see no support for TLS sessions. Maybe you mean the TLS session support was deferred?

    @gvanrossum
    Copy link
    Member

    Okay, I admit it -- I don't understand anything about TLS, and I have no idea what "TLS session" means (nor do I have any desire to learn about all of this). I'll reopen the issue. Sorry!

    @gvanrossum gvanrossum reopened this Oct 4, 2022
    Repository owner moved this from Done to In Progress in asyncio Oct 4, 2022
    @kumaraditya303 kumaraditya303 removed the 3.8 (EOL) end of life label Oct 15, 2022
    @kumaraditya303 kumaraditya303 moved this from In Progress to Todo in asyncio Jan 9, 2023
    @kumaraditya303
    Copy link
    Contributor

    kumaraditya303 commented Apr 19, 2023

    Let's not extend our tls APIs of asyncio anymore. This can be implemented as a third party package if deemed desired but not best fit for stdlib.

    @kumaraditya303 kumaraditya303 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 19, 2023
    @github-project-automation github-project-automation bot moved this from Todo to Done in asyncio Apr 19, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-asyncio type-feature A feature request or enhancement
    Projects
    Status: Done
    Development

    No branches or pull requests

    7 participants