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(