Skip to content

Commit

Permalink
Add support for TLS session resumption in HTTPSConnection
Browse files Browse the repository at this point in the history
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
  • Loading branch information
PleasantMachine9 committed Jun 29, 2020
1 parent fd1ad73 commit af4be67
Show file tree
Hide file tree
Showing 5 changed files with 200 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CONTRIBUTORS.txt
Expand Up @@ -303,5 +303,8 @@ In chronological order:
* [Bastiaan Bakker] <https://github.com/bastiaanb>
* Support for logging session keys via environment variable ``SSLKEYLOGFILE`` (Python 3.8+)

* PleasantMachine9 <https://github.com/PleasantMachine9>
* Reuse previous TLS session (ticket) in HTTPSConnection if possible

* [Your name or handle] <[email or website]>
* [Brief summary of your changes]
38 changes: 37 additions & 1 deletion src/urllib3/connection.py
Expand Up @@ -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."""
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
28 changes: 28 additions & 0 deletions src/urllib3/connectionpool.py
Expand Up @@ -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"
Expand Down Expand Up @@ -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::
Expand Down
31 changes: 28 additions & 3 deletions src/urllib3/util/ssl_.py
Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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 "
Expand All @@ -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):
Expand Down
105 changes: 104 additions & 1 deletion test/with_dummyserver/test_https.py
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit af4be67

Please sign in to comment.