From af4be6713dc7dde41e817f76ec79e8e5566a09d1 Mon Sep 17 00:00:00 2001 From: PleasantMachine9 <65126927+PleasantMachine9@users.noreply.github.com> Date: Mon, 29 Jun 2020 23:28:18 +0200 Subject: [PATCH] Add support for TLS session resumption in HTTPSConnection Indicate with a simple flag kwarg whether this should be enabled. To test it in the wild, CPython 3.6+ and an SSL ticket/id issuing server is needed. This cannot be tested with current libraries in the repo, as tornado (which is the mock https server) does not seem to work properly with SSL tickets/ids. Below is a way to test it in the real world: import logging from urllib3.connectionpool import connection_from_url logging.basicConfig(level=logging.DEBUG) # example sites that support SSL resumption, at time of writing: urls = [ 'https://www.reddit.com/', 'https://www.twitch.tv/', 'https://twitter.com/', ] for url in urls: conn = connection_from_url(url) # by default, connection is kept alive, # for testing this, it's easier if it isn't: conn.request('GET', '/', headers={'Connection':'close'}) conn.request('GET', '/', headers={'Connection':'close'}) # should see: # DEBUG:urllib3.connection:For <...>: session_reused=True --- CONTRIBUTORS.txt | 3 + src/urllib3/connection.py | 38 +++++++++- src/urllib3/connectionpool.py | 28 ++++++++ src/urllib3/util/ssl_.py | 31 +++++++- test/with_dummyserver/test_https.py | 105 +++++++++++++++++++++++++++- 5 files changed, 200 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 68caa5defe..3094e2bf41 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -303,5 +303,8 @@ In chronological order: * [Bastiaan Bakker] * Support for logging session keys via environment variable ``SSLKEYLOGFILE`` (Python 3.8+) +* PleasantMachine9 + * Reuse previous TLS session (ticket) in HTTPSConnection if possible + * [Your name or handle] <[email or website]> * [Brief summary of your changes] diff --git a/src/urllib3/connection.py b/src/urllib3/connection.py index ce94b25640..081314e5a7 100644 --- a/src/urllib3/connection.py +++ b/src/urllib3/connection.py @@ -61,6 +61,9 @@ class ConnectionError(Exception): _CONTAINS_CONTROL_CHAR_RE = re.compile(r"[^-!#$%&'*+.^_`|~0-9a-zA-Z]") +# OP_NO_TICKET is only available in Python 3.6+ +SSL_OP_NO_TICKET = getattr(ssl, "OP_NO_TICKET", None) + class DummyConnection(object): """Used to detect a failed ConnectionCls import.""" @@ -238,6 +241,7 @@ def request_chunked(self, method, url, body=None, headers=None): class HTTPSConnection(HTTPConnection): default_port = port_by_scheme["https"] + default_resume_ssl_sessions = True cert_reqs = None ca_certs = None @@ -259,7 +263,10 @@ def __init__( server_hostname=None, **kw ): - + # do this before calling base class: + self.resume_ssl_sessions = kw.pop( + "resume_ssl_sessions", HTTPSConnection.default_resume_ssl_sessions + ) HTTPConnection.__init__(self, host, port, strict=strict, timeout=timeout, **kw) self.key_file = key_file @@ -268,6 +275,8 @@ def __init__( self.ssl_context = ssl_context self.server_hostname = server_hostname + # This may get a value after the first call to .connect(): + self._ssl_session = None # Required property for Google AppEngine 1.9.0 which otherwise causes # HTTPS requests to go out as HTTP. (See Issue #356) self._protocol = "https" @@ -345,6 +354,18 @@ def connect(self): ssl_version=resolve_ssl_version(self.ssl_version), cert_reqs=resolve_cert_reqs(self.cert_reqs), ) + if SSL_OP_NO_TICKET is not None: + # Unset this sslctx flag, so that we express + # to the server we support tickets on TLSv1.2 and below. + # On TLSv1.3, this flag does nothing, and is ignored + # by openssl, as the TLS client no longer states + # it supports tickets to the server explicitly: + if self.resume_ssl_sessions: + self.ssl_context.options &= ~SSL_OP_NO_TICKET + # In other cases, set the flag, expressing + # to the server we don't want a ticket: + else: + self.ssl_context.options |= SSL_OP_NO_TICKET context = self.ssl_context context.verify_mode = resolve_cert_reqs(self.cert_reqs) @@ -360,6 +381,9 @@ def connect(self): ): context.load_default_certs() + # If this is None, it will be the same as if it were ommitted in kwargs: + prev_ssl_session = self._ssl_session + self.sock = ssl_wrap_socket( sock=conn, keyfile=self.key_file, @@ -370,6 +394,7 @@ def connect(self): ca_cert_data=self.ca_cert_data, server_hostname=server_hostname, ssl_context=context, + ssl_session=prev_ssl_session, ) if self.assert_fingerprint: @@ -402,6 +427,17 @@ def connect(self): or self.assert_fingerprint is not None ) + if prev_ssl_session is not None: + log.debug( + "For %s: session_reused=%s", + self, + getattr(self.sock, "session_reused", None), + ) + + curr_ssl_session = getattr(self.sock, "session", None) + if curr_ssl_session is not None and self.resume_ssl_sessions: + self._ssl_session = curr_ssl_session + def _match_hostname(cert, asserted_hostname): try: diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py index 492590fb9e..d4d97faffc 100644 --- a/src/urllib3/connectionpool.py +++ b/src/urllib3/connectionpool.py @@ -849,6 +849,32 @@ class HTTPSConnectionPool(HTTPConnectionPool): ``ca_cert_dir``, ``ssl_version``, ``key_password`` are only used if :mod:`ssl` is available and are fed into :meth:`urllib3.util.ssl_wrap_socket` to upgrade the connection socket into an SSL socket. + + When running on CPython >=3.6 compiled with :mod:`ssl` (openssl binding), + it is possible to resume TLS sessions in case the TCP socket level was dropped/closed. + This feature is enabled by default when supported by the underlying openssl bindings. + To disable it for all connections in this pool, set the kwarg ``resume_ssl_sessions`` + to ``False``. + + >> from urllib3 import HTTPSConnectionPool + >> pool_with_resumption_enabled = HTTPSConnectionPool('localhost', 443) + >> pool_with_resumption_disabled = HTTPSConnectionPool('localhost', 443, resume_ssl_sessions=False) + + To globally change the default behavior in your application + without this explicit kwarg, it is also possible to the following: + + >> from urllib3 import HTTPSConnectionPool + >> from urllib3.connection import HTTPSConnection + >> HTTPSConnection.default_resume_ssl_sessions = False + >> pool_with_resumption_disabled = HTTPSConnectionPool('localhost', 443) + + .. note:: + In most cases the ``Connection: keep-alive`` directive of the HTTP layer solves + largely the same issue and does it better. + However, there are some cases where ``keep-alive`` does not work, + or a middlebox does not agree with too long-lived TCP connections. + In such cases reconnection may be accelerated by TLS resumption, + which will save some CPU work and network traffic for both sides. """ scheme = "https" @@ -1008,6 +1034,8 @@ def connection_from_url(url, **kw): Passes additional parameters to the constructor of the appropriate :class:`.ConnectionPool`. Useful for specifying things like timeout, maxsize, headers, etc. + For the keyword args available, see :class:`.HTTPConnectionPool` + or :class:`.HTTPSConnectionPool`, depending on your url. Example:: diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index 3d89a56c08..4cd63a6f6e 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -17,6 +17,7 @@ HAS_SNI = False IS_PYOPENSSL = False IS_SECURETRANSPORT = False +IS_MODERN_SSL_CONTEXT = False # Maps the length of a digest to a possible hash function producing this digest HASHFUNC_MAP = {32: md5, 40: sha1, 64: sha256} @@ -102,6 +103,8 @@ def _const_compare_digest_backport(a, b): try: from ssl import SSLContext # Modern SSL? + + IS_MODERN_SSL_CONTEXT = True except ImportError: class SSLContext(object): # Platform-specific: Python 2 @@ -316,9 +319,10 @@ def ssl_wrap_socket( ca_cert_dir=None, key_password=None, ca_cert_data=None, + ssl_session=None, ): """ - All arguments except for server_hostname, ssl_context, and ca_cert_dir have + All arguments except for server_hostname, ssl_context, ssl_session, and ca_cert_dir have the same meaning as they do when using :func:`ssl.wrap_socket`. :param server_hostname: @@ -337,6 +341,18 @@ def ssl_wrap_socket( :param ca_cert_data: Optional string containing CA certificates in PEM format suitable for passing as the cadata parameter to SSLContext.load_verify_locations() + :param ssl_session: + If not ``None``, will be passed to :func:`ssl.wrap_socket` + as the `session` keyword argument. + An `_ssl.Session` object is obtained from a previously created + ``SSLSocket`` object, that had a completed handshake. + If provided, the ssl layer may attempt session resumption + depending on the ssl bindings in use and whether a ticket/id + to use for resumption was sent by the SSL server, but there is + no guarantee that a resumption will be attempted, nor that it will succeed. + In case the resumption fails or does not happen, the underlying ssl + layer will fall back to a full handshake - ie. same as when establishing + a new connection, so this failure is transparent to a caller. """ context = ssl_context if context is None: @@ -381,7 +397,13 @@ def ssl_wrap_socket( server_hostname is not None and not is_ipaddress(server_hostname) ) or IS_SECURETRANSPORT: if HAS_SNI and server_hostname is not None: - return context.wrap_socket(sock, server_hostname=server_hostname) + + if ssl_session is not None and IS_MODERN_SSL_CONTEXT: + return context.wrap_socket( + sock, server_hostname=server_hostname, session=ssl_session + ) + else: + return context.wrap_socket(sock, server_hostname=server_hostname) warnings.warn( "An HTTPS request has been made, but the SNI (Server Name " @@ -394,7 +416,10 @@ def ssl_wrap_socket( SNIMissingWarning, ) - return context.wrap_socket(sock) + if ssl_session is not None and IS_MODERN_SSL_CONTEXT: + return context.wrap_socket(sock, session=ssl_session) + else: + return context.wrap_socket(sock) def is_ipaddress(hostname): diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 414aea49f0..00a99cb235 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -36,7 +36,12 @@ resolvesLocalhostFQDN, ) from urllib3 import HTTPSConnectionPool -from urllib3.connection import VerifiedHTTPSConnection, RECENT_DATE +from urllib3.connection import ( + HTTPSConnection, + VerifiedHTTPSConnection, + RECENT_DATE, + SSL_OP_NO_TICKET, +) from urllib3.exceptions import ( SSLError, ConnectTimeoutError, @@ -133,6 +138,104 @@ def test_simple(self): r = https_pool.request("GET", "/") assert r.status == 200, r.data + def __ssl_resumption_assertions(self, https_pool): + def is_sock_ssl_session_capable(sock): + return isinstance(sock, ssl.SSLSocket) and hasattr(sock, "session") + + def https_conn_assertions(https_conn, resume_ssl_sessions): + if SSL_OP_NO_TICKET is not None: + if resume_ssl_sessions: + assert (SSL_OP_NO_TICKET & https_conn.ssl_context.options) == 0 + else: + assert (SSL_OP_NO_TICKET & https_conn.ssl_context.options) != 0 + + if resume_ssl_sessions and is_sock_ssl_session_capable(https_conn.sock): + assert https_conn._ssl_session is not None + else: + assert https_conn._ssl_session is None + + # in case it's none, get the default that it will be + resume_ssl_sessions = https_pool.conn_kw.get( + "resume_ssl_sessions", HTTPSConnection.default_resume_ssl_sessions + ) + r = https_pool.request("GET", "/") + assert r.status == 200, r.data + https_conn = https_pool.pool.get_nowait() + assert https_conn is not None + https_pool.pool.put( + https_conn + ) # put https_conn back so next request can use it + + https_conn_assertions(https_conn, resume_ssl_sessions) + + # force next call to reopen TCP layer by closing it + https_conn.sock.close() + first_req_sock = https_conn.sock + https_conn.sock = None + + r = https_pool.request("GET", "/") + assert r.status == 200, r.data + assert first_req_sock is not https_conn.sock + + https_conn_assertions(https_conn, resume_ssl_sessions) + # FIXME: For some reason the Tornado HTTPS server ignores the + # session ticket that itself sent in `first_req_sock.session`, + # even though with pcap tools it can be observed that + # the 2nd ClientHello contains the session key as expected. + # Is Tornado messing up, or ssl.wrap_socket/do_handshake? + # Uncomment this when the HTTPS server handles session tickets properly: + # assert ( + # not is_sock_ssl_session_capable(https_conn.sock) + # or https_conn.sock.session_reused == resume_ssl_sessions + # ) + + def test_resume_ssl_sessions_enabled(self): + """ + Test the cases where HTTPSConnection objects are implicitly + or explitictly told to try to reuse their previous ssl_sessions if possible. + Test both the SNI and IP address requests to cover both branches in ssl_.py. + """ + for host in ("127.0.0.1", self.host): + with mock.patch("warnings.warn"): + # implicitly: + with HTTPSConnectionPool( + host, + self.port, + ca_certs=DEFAULT_CA, + cert_reqs=ssl.CERT_NONE, + # resume_ssl_sessions=True, + ) as https_pool: + self.__ssl_resumption_assertions(https_pool) + + # explicitly: + with HTTPSConnectionPool( + host, + self.port, + ca_certs=DEFAULT_CA, + cert_reqs=ssl.CERT_NONE, + resume_ssl_sessions=True, + ) as https_pool: + self.__ssl_resumption_assertions(https_pool) + + def test_resume_ssl_sessions_disabled(self): + """ + Test the cases where HTTPSConnection objects are explicitly + told not to reuse their previous ssl_sessions. + In this case, OP_NO_TICKET should be set in the default sslctx of + the connection objects as well. + Test both the SNI and IP address requests to cover both branches in ssl_.py. + """ + for host in ("127.0.0.1", self.host): + with mock.patch("warnings.warn"): + with HTTPSConnectionPool( + host, + self.port, + ca_certs=DEFAULT_CA, + cert_reqs=ssl.CERT_NONE, + resume_ssl_sessions=False, + ) as https_pool: + self.__ssl_resumption_assertions(https_pool) + @resolvesLocalhostFQDN def test_dotted_fqdn(self): with HTTPSConnectionPool(