add more sophisticated policies for resumption

Tests are not done yet, this is a draft to get opinions
on this approach from reviewers.

src/urllib3/__init__.py

@@ -4,6 +4,7 @@
 from __future__ import absolute_import
 import warnings
 
+from .connection import SSLSessionResumptionPolicy
 from .connectionpool import HTTPConnectionPool, HTTPSConnectionPool, connection_from_url
 
 from . import exceptions
@@ -28,6 +29,7 @@
 __all__ = (
     "HTTPConnectionPool",
     "HTTPSConnectionPool",
+    "SSLSessionResumptionPolicy",
     "PoolManager",
     "ProxyManager",
     "HTTPResponse",

src/urllib3/connection.py

@@ -239,14 +239,63 @@ def request_chunked(self, method, url, body=None, headers=None):
         self.send(b"0\r\n\r\n")
 
 
+# enum is nice but is only Python 3
+class SSLSessionResumptionPolicy(object):
+    """
+    Enum used by :class:`.HTTPSConnection` and ``HTTPSConnectionPool`` to
+    choose the behavior regarding TLS session resumption.
+
+    .. note::
+        Before Python 3.6, all options will effectively be the same as ``NEVER``,
+        because ``SSLSocket.session`` was only added in that version.
+
+    In order of security (from highest to lowest),
+    and *reverse* order of convenience/performance:
+
+    ``NEVER``
+    Paranoid option.
+    Prevent Forward Secrecy compromise as much as possible
+    by telling the server not to send tickets on TLSv1.2 (``ssl.OP_NO_TICKET``),
+    and by always redoing the whole TLS handshake on reconnection.
+
+    ``BALANCED``
+    Balanced option, default.
+    On TLSv1.3, allow resumption via the new session tickets,
+    see https://tools.ietf.org/html/rfc8446#section-4.6.1
+    On TLSv1.2 and below, only allow session ids (set OP_NO_TICKET).
+    This is a compromise because tickets are sent in the clear before TLSv1.3,
+    which is considered bad (even though the tickets are encrypted themselves).
+
+    ``DEFAULT``
+    Same as ``BALANCED``.
+    May be changed to effectively toggle the default option globally in your python application,
+    to for example ``NEVER``; this will only affect class instances created *after* the change,
+    not ones created before, so you should change this at startup only.
+    Example::
+
+        >> from urllib3 import HTTPSConnectionPool, SSLSessionResumptionPolicy
+        >> # not recommended but possible:
+        >> SSLSessionResumptionPolicy.DEFAULT = SSLSessionResumptionPolicy.NEVER
+        >> pool_with_resumption_disabled = HTTPSConnectionPool('localhost', 443)
+
+    ``ALWAYS``
+    Efficiency/performance option.
+    Request tickets *actively* on TLSv1.2 and below, and try to use them whenever possible.
+    Good choice in case your target server does not have TLSv1.3, nor session IDs on TLSv1.2
+    (which is a very rare feature for servers to support), but you still want
+    TLS session resumption for performance reasons; eg. real-time applications,
+    streaming or high latency connections like 2G/3G/dialups.
+    """
+
+    NEVER = 0
+    BALANCED = 1
+    ALWAYS = 2
+    DEFAULT = BALANCED
+
+
 class HTTPSConnection(HTTPConnection):
     default_port = port_by_scheme["https"]
 
-    # Specifies the default behavior on whether to attempt TLS session resumption.
-    # Can be overridden in HTTPSConnection instances in the constructor,
-    # with kwarg `ssl_session_reuse`.
-    default_ssl_session_reuse_policy = True
-
     cert_reqs = None
     ca_certs = None
     ca_cert_dir = None
@@ -268,8 +317,8 @@ def __init__(
         **kw
     ):
         # do this before calling base class:
-        self.ssl_session_reuse = kw.pop(
-            "ssl_session_reuse", HTTPSConnection.default_ssl_session_reuse_policy
+        self.ssl_session_resumption_policy = kw.pop(
+            "ssl_session_resumption_policy", SSLSessionResumptionPolicy.DEFAULT
         )
         HTTPConnection.__init__(self, host, port, strict=strict, timeout=timeout, **kw)
 
@@ -359,20 +408,16 @@ def connect(self):
                 cert_reqs=resolve_cert_reqs(self.cert_reqs),
             )
             if SSL_OP_NO_TICKET is not None:
-                if self.ssl_session_reuse:
-                    # Try to make it so we request/use a ticket if possible.
+                if (
+                    self.ssl_session_resumption_policy
+                    == SSLSessionResumptionPolicy.ALWAYS
+                ):
+                    # Double negatives are confusing. Unset this ssl flag,
+                    # so that we *do* request a ticket on TLSv1.2 and below.
                     self.ssl_context.options &= ~SSL_OP_NO_TICKET
                 else:
-                    # Indicate to openssl we don't want it to ask for session ticket.
-                    # This potentially saves work for the server by
-                    # allowing it to skip the generation of a useless ticket,
-                    # and a few hundred bytes of received packet size.
-
-                    # It also improves security by some small margin for TLS <1.3.
-                    # This is because session tickets are a risk, however small;
-                    # they contain the master secrets (symmetric keys) in a server-encrypted
-                    # opaque format, and in TLS <1.3 they are sent in the clear before Change Cipher Spec.
-                    # If we don't use them anyway, there's no point letting them go on wire.
+                    # In other cases, set the flag, so we don't
+                    # request a ticket on TLSv1.2 and below.
                     self.ssl_context.options |= SSL_OP_NO_TICKET
 
         context = self.ssl_context
@@ -435,15 +480,65 @@ def connect(self):
             or self.assert_fingerprint is not None
         )
 
-        # Saving & later reusing the same _ssl.Session in future .connect() calls will allow
-        # the TLS layer to attempt session resumption via session ticket/id if there are any.
-        if self.ssl_session_reuse:
-            curr_ssl_session = getattr(self.sock, "session", None)
+        if prev_ssl_session is not None:
             log.debug(
-                "For %s: session_reused=%s, new ssl_session=%s",
+                "For %s: session_reused=%s",
                 self,
                 getattr(self.sock, "session_reused", None),
+            )
+
+        self._save_ssl_session_if_needed()
+
+    def _save_ssl_session_if_needed(self):
+        """
+        Check the self.ssl_session_resumption_policy and decide whether to save the
+        sock.session reference into our class attrib or not, based on the policy.
+        """
+        curr_ssl_session = getattr(self.sock, "session", None)
+        if curr_ssl_session is None:
+            log.debug("For %s: not saving ssl session as as curr_ssl_session is None")
+            return
+
+        if self.ssl_session_resumption_policy == SSLSessionResumptionPolicy.NEVER:
+            log.debug(
+                "For %s: not saving ssl session as SSLSessionResumptionPolicy is NEVER"
+            )
+            return
+
+        if self.ssl_session_resumption_policy == SSLSessionResumptionPolicy.ALWAYS:
+            log.debug(
+                "For %s: new ssl_session=%s (SSLSessionResumptionPolicy is ALWAYS)",
+                self,
+                curr_ssl_session,
+            )
+            self.ssl_session = curr_ssl_session
+            return
+
+        elif self.ssl_session_resumption_policy == SSLSessionResumptionPolicy.BALANCED:
+            # .version() is a 0-arg method on Python 3.5+ ssl:
+            curr_ssl_version = (
+                self.sock.version() if hasattr(self.sock, "version") else None
+            )
+            if curr_ssl_version != "TLSv1.3" and curr_ssl_session.has_ticket:
+                log.debug(
+                    (
+                        "For %s: not saving ssl session as SSLSessionResumptionPolicy"
+                        " is BALANCED and curr_ssl_version=%s != v1.3"
+                        " and curr_ssl_session.has_ticket=True"
+                    ),
+                    self,
+                    curr_ssl_version,
+                )
+                return
+
+            log.debug(
+                (
+                    "For %s: new ssl_session=%s curr_ssl_version=%s "
+                    "(SSLSessionResumptionPolicy is BALANCED)"
+                ),
+                self,
                 curr_ssl_session,
+                curr_ssl_version,
             )
             self.ssl_session = curr_ssl_session
 

src/urllib3/connectionpool.py

@@ -849,6 +849,25 @@ 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 Python is 3.6 or higher and compiled with the :mod:`ssl` module,
+    it is possible to resume TLS sessions in case the TCP socket level was dropped/closed.
+    For resumption behavior options, see the enum :class:`.SSLSessionResumptionPolicy`.
+    In most reasonable cases, it's fine to leave the default and ignore this section.
+    Should you want to override the default for all HTTPSConnections in this pool,
+    you can use the kwarg ``ssl_session_resumption_policy`` when initializing,
+    and pass it a valid enum value from :class:`.SSLSessionResumptionPolicy`.
+    Example usage::
+
+        >> from urllib3 import HTTPSConnectionPool, SSLSessionResumptionPolicy
+        >> pool_with_full_resumption_enabled = HTTPSConnectionPool('localhost', 443, ssl_session_resumption_policy=SSLSessionResumptionPolicy.ALWAYS)
+        >> pool_with_resumption_disabled = HTTPSConnectionPool('localhost', 443, ssl_session_resumption_policy=SSLSessionResumptionPolicy.NEVER)
+
+    .. note::
+        In most cases the ``Connection: keep-alive`` directive of the HTTP layer solves
+        largely the same issue more efficiently. However there are some cases where ``keep-alive``
+        does not work or has expired. In such cases reconnection will be accelerated by TLS resumption,
+        which will save some CPU work and network traffic for both sides.
     """
 
     scheme = "https"
@@ -1008,6 +1027,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::