diff --git a/src/urllib3/connection.py b/src/urllib3/connection.py index f3336acff1..afc8d787e7 100644 --- a/src/urllib3/connection.py +++ b/src/urllib3/connection.py @@ -71,6 +71,9 @@ class BrokenPipeError(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 HTTPConnection(_HTTPConnection, object): """ @@ -269,6 +272,7 @@ class HTTPSConnection(HTTPConnection): """ default_port = port_by_scheme["https"] + default_resume_ssl_sessions = True cert_reqs = None ca_certs = None @@ -291,7 +295,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 @@ -300,6 +307,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" @@ -382,6 +391,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) @@ -397,6 +418,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, @@ -407,6 +431,7 @@ def connect(self): ca_cert_data=self.ca_cert_data, server_hostname=server_hostname, ssl_context=context, + ssl_session=prev_ssl_session, tls_in_tls=tls_in_tls, ) @@ -440,6 +465,17 @@ def connect(self): or self.assert_fingerprint is not None ) + if prev_ssl_session is not None: + log.debug( + "For %s: new socket created, socket.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 _connect_tls_proxy(self, hostname, conn): """ Establish a TLS connection to the proxy using the provided SSL context. diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py index da1931eeb3..75b6b3716e 100644 --- a/src/urllib3/connectionpool.py +++ b/src/urllib3/connectionpool.py @@ -872,6 +872,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" @@ -1033,6 +1059,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 8773334067..b8ed2a64e9 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -21,6 +21,8 @@ HAS_SNI = False IS_PYOPENSSL = False IS_SECURETRANSPORT = False +# SSLSocket.session -> version 3.6 +IS_SSL_SESSION_SUPPORTED = sys.version_info >= (3, 6) ALPN_PROTOCOLS = ["http/1.1"] # Maps the length of a digest to a possible hash function producing this digest @@ -116,6 +118,7 @@ def _const_compare_digest_backport(a, b): try: from ssl import SSLContext # Modern SSL? except ImportError: + IS_SSL_SESSION_SUPPORTED = False class SSLContext(object): # Platform-specific: Python 2 def __init__(self, protocol_version): @@ -334,10 +337,11 @@ def ssl_wrap_socket( ca_cert_dir=None, key_password=None, ca_cert_data=None, + ssl_session=None, tls_in_tls=False, ): """ - 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: @@ -356,8 +360,20 @@ 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. :param tls_in_tls: - Use SSLTransport to wrap the existing socket. + Use :class:`SSLTransport` to wrap the existing socket if available. """ context = ssl_context if context is None: @@ -414,13 +430,30 @@ def ssl_wrap_socket( SNIMissingWarning, ) + wrap_sock_kwargs = {} + # do not pass it if not sending SNI if send_sni: - ssl_sock = _ssl_wrap_socket_impl( - sock, context, tls_in_tls, server_hostname=server_hostname - ) - else: - ssl_sock = _ssl_wrap_socket_impl(sock, context, tls_in_tls) - return ssl_sock + wrap_sock_kwargs["server_hostname"] = server_hostname + # Depending on the underlying wrap_socket, + # it may not accept the `session` kwarg even if it is provided by caller + if IS_SSL_SESSION_SUPPORTED and ssl_session is not None: + wrap_sock_kwargs["session"] = ssl_session + + return _ssl_wrap_socket_impl(sock, context, tls_in_tls, wrap_sock_kwargs) + + +def _ssl_wrap_socket_impl(sock, ssl_context, tls_in_tls, wrap_sock_kwargs): + if tls_in_tls: + if not SSLTransport: + # Import error, ssl is not available. + raise ProxySchemeUnsupported( + "TLS in TLS requires support for the 'ssl' module" + ) + + SSLTransport._validate_ssl_context_for_tls_in_tls(ssl_context) + return SSLTransport(sock, ssl_context, **wrap_sock_kwargs) + + return ssl_context.wrap_socket(sock, **wrap_sock_kwargs) def is_ipaddress(hostname): @@ -445,20 +478,3 @@ def _is_key_file_encrypted(key_file): return True return False - - -def _ssl_wrap_socket_impl(sock, ssl_context, tls_in_tls, server_hostname=None): - if tls_in_tls: - if not SSLTransport: - # Import error, ssl is not available. - raise ProxySchemeUnsupported( - "TLS in TLS requires support for the 'ssl' module" - ) - - SSLTransport._validate_ssl_context_for_tls_in_tls(ssl_context) - return SSLTransport(sock, ssl_context, server_hostname) - - if server_hostname: - return ssl_context.wrap_socket(sock, server_hostname=server_hostname) - else: - return ssl_context.wrap_socket(sock) diff --git a/src/urllib3/util/ssltransport.py b/src/urllib3/util/ssltransport.py index d23e518de7..24731a96d5 100644 --- a/src/urllib3/util/ssltransport.py +++ b/src/urllib3/util/ssltransport.py @@ -42,10 +42,19 @@ def _validate_ssl_context_for_tls_in_tls(ssl_context): ) def __init__( - self, socket, ssl_context, suppress_ragged_eofs=True, server_hostname=None + self, socket, ssl_context, suppress_ragged_eofs=True, **wrap_bio_kwargs ): """ Create an SSLTransport around socket using the provided ssl_context. + + :param socket: + The socket object to wrap. + :param ssl_context: + :class:`ssl.SSLContext` to use call :func:`ssl.SSLContext.wrap_bio` on. + :param suppress_ragged_eofs: + Suppress ``ssl.SSL_ERROR_EOF`` read errors. + :param wrap_bio_kwargs: + These args will be passed to :func:`ssl.SSLContext.wrap_bio` """ self.incoming = ssl.MemoryBIO() self.outgoing = ssl.MemoryBIO() @@ -54,7 +63,7 @@ def __init__( self.socket = socket self.sslobj = ssl_context.wrap_bio( - self.incoming, self.outgoing, server_hostname=server_hostname + self.incoming, self.outgoing, **wrap_bio_kwargs ) # Perform initial handshake. diff --git a/test/test_retry.py b/test/test_retry.py index cc36089796..2f6e050015 100644 --- a/test/test_retry.py +++ b/test/test_retry.py @@ -343,11 +343,7 @@ def test_respect_retry_after_header_propagated(self, respect_retry_after_header) ) @pytest.mark.parametrize( "stub_timezone", - [ - "UTC", - "Asia/Jerusalem", - None, - ], + ["UTC", "Asia/Jerusalem", None], indirect=True, ) @pytest.mark.usefixtures("stub_timezone") diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 4c587d3432..c21fdfb888 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -36,7 +36,12 @@ ) from dummyserver.testcase import HTTPSDummyServerTestCase from urllib3 import HTTPSConnectionPool -from urllib3.connection import RECENT_DATE, VerifiedHTTPSConnection +from urllib3.connection import ( + RECENT_DATE, + SSL_OP_NO_TICKET, + HTTPSConnection, + VerifiedHTTPSConnection, +) from urllib3.exceptions import ( ConnectTimeoutError, InsecurePlatformWarning, @@ -134,6 +139,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(