Add support for password-protected client keyfiles (urllib3#1489)

CHANGES.rst

@@ -10,6 +10,12 @@ dev (master)
 
 * Remove Authorization header regardless of case when redirecting to cross-site. (Issue #1510)
 
+* Added support for ``key_password`` for ``HTTPSConnectionPool`` to use
+  encrypted ``key_file`` without creating your own ``SSLContext`` object. (Pull #1489)
+
+* Fixed issue where OpenSSL would block if an encrypted client private key was
+  given and no password was given. Instead an ``SSLError`` is raised. (Pull #1489)
+
 * ... [Short description of non-trivial change.] (Issue #)
 
 
@@ -18,7 +24,7 @@ dev (master)
 
 * Remove quadratic behavior within ``GzipDecoder.decompress()`` (Issue #1467)
 
-* Restored functionality of `ciphers` parameter for `create_urllib3_context()`. (Issue #1462)
+* Restored functionality of ``ciphers`` parameter for ``create_urllib3_context()``. (Issue #1462)
 
 
 1.24 (2018-10-16)

docs/advanced-usage.rst

@@ -139,8 +139,8 @@ Once PySocks is installed, you can use
 
 .. _ssl_custom:
 
-Custom SSL certificates and client certificates
------------------------------------------------
+Custom SSL certificates
+-----------------------
 
 Instead of using `certifi <https://certifi.io/>`_ you can provide your
 own certificate authority bundle. This is useful for cases where you've
@@ -158,6 +158,11 @@ verified with that bundle will succeed. It's recommended to use a separate
 :class:`~poolmanager.PoolManager` to make requests to URLs that do not need
 the custom certificate.
 
+.. _ssl_client:
+
+Client certificates
+-------------------
+
 You can also specify a client certificate. This is useful when both the server
 and the client need to verify each other's identity. Typically these
 certificates are issued from the same authority. To use a client certificate,
@@ -168,6 +173,17 @@ provide the full path when creating a :class:`~poolmanager.PoolManager`::
     ...     cert_reqs='CERT_REQUIRED',
     ...     ca_certs='/path/to/your/certificate_bundle')
 
+If you have an encrypted client certificate private key you can use
+the ``key_password`` parameter to specify a password to decrypt the key. ::
+
+    >>> http = urllib3.PoolManager(
+    ...     cert_file='/path/to/your/client_cert.pem',
+    ...     cert_reqs='CERT_REQUIRED',
+    ...     key_file='/path/to/your/client.key',
+    ...     key_password='keyfile_password')
+
+If your key isn't encrypted the ``key_password`` parameter isn't required.
+
 .. _ssl_mac:
 
 Certificate validation and Mac OS X

dummyserver/certs/client_password.key

@@ -0,0 +1,18 @@
+-----BEGIN RSA PRIVATE KEY-----
+Proc-Type: 4,ENCRYPTED
+DEK-Info: AES-256-CBC,70C641602D5F366DC5DB70645351993D
+
+/Ijrtw+2Rjc1mQCXWoNCtzjbRoIhBHQu9ZbQoCnC4/lHru2megV0vDQju0yYjs2H
+7Y7tnMe0hlR9F21be6AkoKDF4B5Kg2X47fwG5V9SIbHBkz3KClfnPp/ojrhIWLTo
+grtZoXBFkivDnkuF9NO3qRlskP7u//r3kB5uXIG0ZpfUbRwgm13SqHj1oEB9RdYM
+bGhB3tL6dxdIXEgyc9numKBQ0lQu5yYlOH+1aiJSQQdN59ZunreIq//UM1Qc7Uj/
+ILJusFmnec40ArJ+aykENWkToHSKkpeL6no6ZRCnkAYqtUJ84B6zMv9zYhN5UF3O
+WHP/4FAu4AylJvNx9sYxXdGaBb+YcX46B7wQk2mkmCtK6cgkrNV3/bohUbYt3tSe
+K9dH2xe9orxsGQjoKxylwh7+h8o+BwHpk1naFSzliQV4gvi8yBEzXxM98vNU5B4L
+ex8Q2ARWvfNc7OBqboPP0yBMKP/cV9n+fNMwbP0koHxBt71527fVQLoemMiPRb5M
++rcufc+80AUK4baAA5Nu2sZGRqoiFemQ2vgEAxOzRbt/pHzdheO6OHqLJ5W4IWaW
+Erojm7/ar6gDlIIGwM8IJdbcMG69s7r8u47lD45ONQMq41Io4Svvs0SCgdRhLt/3
+Nb6Smxy7vWFOcrHEJVsv27UD0FViaYHy37DIc6lVvX9s6+VKbdIYuiqxalbaCpKo
+VP8kdQZ4SFBAxV9cgPjFbQKVBXkLBdxJKGPzzK3Jc9khD1uHp5Um8OSM21Kh55N3
+jvDY5h8fQ0cPyJmlZJzRdYi1+8H5TSFvEXd6cqVkYWiJ1ac0gPOoVt7+YAZ6JB2J
+-----END RSA PRIVATE KEY-----

dummyserver/certs/server_password.key

@@ -0,0 +1,18 @@
+-----BEGIN RSA PRIVATE KEY-----
+Proc-Type: 4,ENCRYPTED
+DEK-Info: AES-256-CBC,D1BAE8F8B899FD9C2367B4CCF40917CF
+
+emTIXrQCcOcHtknXXZwK9X4qS8WcT6ozH2TTTDOcz9+G6CnszwvnsLCnO3eiLVbI
+NXiFib7ulQksoHQd2MPC9pjWm1a8vadMYOWgx8jnYkgVE+l1ICGgZVACg55/E6Xj
+qC4hZijQnKhPPyzdebeos2IIk7B3op4kHYJQGpwisAuSmT06c2x/jsmFr+2UMSq5
+Xf+kWuQlHUPcDct6uLJJ4zhFljcxFvk3cgMZIJaqyWCX7+gDCOi2gWrP2l1osllc
+q0egNUdg3RVrbxgxFn4XdHpmTNbIc3NTTR+xYuqHun8UbJrss1Ed25rrK0QzuV0l
+vyKLj1MSOV9VRCujF58I9whDZSwt07Aozmm1JC9F8kyMhbL9C4gmwEKHIQ5N5I+V
+mZKKAbJyQ2B1Oza/yZUnJG6hUyKTVbbCW57OltTDr4KlUzYUJJzTVyMy14AVv3zU
+GzKX5m3AzWMjykpmHjYNcI/zMQem0OQB2U9Pqyyh2GzItnHpnkqb7RDJSIYiOToc
+jA65NhS4sIZWWzwsRRaE2sq1rlssQFkzM3gIHi2C+tJD3PvmYRKW+6fLLNCqikMk
+w4OvHc8U/hIY2YnGAzjE7bbCrkQduhCwBL7bK08HYrluQv6dgVJLA9TtC4jYLYeo
+1uXDNGcY943fwU5h/YwQbvVQ5oo9oHBJuLgUzXlPjc+va4gw0JSG59GgXaTMVTjw
+wybmcNlaFZbK0XrveX3ykJimnuDK29yY4nWSzPFRxvaaWaRAL4IgEXvKCiQhg8NE
+snV2L3uQgJNv6RmE+c4HzQQ71iZuZ+iJglzt/iG4pO88zxjLLfT4qwfPAlEdxRmN
+-----END RSA PRIVATE KEY-----

dummyserver/server.py

@@ -46,6 +46,8 @@
     'certfile': os.path.join(CERTS_PATH, 'client_no_intermediate.pem'),
     'keyfile': os.path.join(CERTS_PATH, 'client_intermediate.key'),
 }
+PASSWORD_KEYFILE = os.path.join(CERTS_PATH, 'server_password.key')
+PASSWORD_CLIENT_KEYFILE = os.path.join(CERTS_PATH, 'client_password.key')
 NO_SAN_CERTS = {
     'certfile': os.path.join(CERTS_PATH, 'server.no_san.crt'),
     'keyfile': DEFAULT_CERTS['keyfile']

src/urllib3/connection.py

@@ -226,14 +226,16 @@ class HTTPSConnection(HTTPConnection):
     ssl_version = None
 
     def __init__(self, host, port=None, key_file=None, cert_file=None,
-                 strict=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT,
+                 key_password=None, strict=None,
+                 timeout=socket._GLOBAL_DEFAULT_TIMEOUT,
                  ssl_context=None, server_hostname=None, **kw):
 
         HTTPConnection.__init__(self, host, port, strict=strict,
                                 timeout=timeout, **kw)
 
         self.key_file = key_file
         self.cert_file = cert_file
+        self.key_password = key_password
         self.ssl_context = ssl_context
         self.server_hostname = server_hostname
 
@@ -255,6 +257,7 @@ def connect(self):
             sock=conn,
             keyfile=self.key_file,
             certfile=self.cert_file,
+            key_password=self.key_password,
             ssl_context=self.ssl_context,
             server_hostname=self.server_hostname
         )
@@ -272,7 +275,7 @@ class VerifiedHTTPSConnection(HTTPSConnection):
     assert_fingerprint = None
 
     def set_cert(self, key_file=None, cert_file=None,
-                 cert_reqs=None, ca_certs=None,
+                 cert_reqs=None, key_password=None, ca_certs=None,
                  assert_hostname=None, assert_fingerprint=None,
                  ca_cert_dir=None):
         """
@@ -291,6 +294,7 @@ def set_cert(self, key_file=None, cert_file=None,
         self.key_file = key_file
         self.cert_file = cert_file
         self.cert_reqs = cert_reqs
+        self.key_password = key_password
         self.assert_hostname = assert_hostname
         self.assert_fingerprint = assert_fingerprint
         self.ca_certs = ca_certs and os.path.expanduser(ca_certs)
@@ -338,6 +342,7 @@ def connect(self):
             sock=conn,
             keyfile=self.key_file,
             certfile=self.cert_file,
+            key_password=self.key_password,
             ca_certs=self.ca_certs,
             ca_cert_dir=self.ca_cert_dir,
             server_hostname=server_hostname,

src/urllib3/connectionpool.py

@@ -746,8 +746,8 @@ class HTTPSConnectionPool(HTTPConnectionPool):
     If ``assert_hostname`` is False, no verification is done.
 
     The ``key_file``, ``cert_file``, ``cert_reqs``, ``ca_certs``,
-    ``ca_cert_dir``, and ``ssl_version`` are only used if :mod:`ssl` is
-    available and are fed into :meth:`urllib3.util.ssl_wrap_socket` to upgrade
+    ``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.
     """
 
@@ -759,7 +759,7 @@ def __init__(self, host, port=None,
                  block=False, headers=None, retries=None,
                  _proxy=None, _proxy_headers=None,
                  key_file=None, cert_file=None, cert_reqs=None,
-                 ca_certs=None, ssl_version=None,
+                 key_password=None, ca_certs=None, ssl_version=None,
                  assert_hostname=None, assert_fingerprint=None,
                  ca_cert_dir=None, **conn_kw):
 
@@ -773,6 +773,7 @@ def __init__(self, host, port=None,
         self.key_file = key_file
         self.cert_file = cert_file
         self.cert_reqs = cert_reqs
+        self.key_password = key_password
         self.ca_certs = ca_certs
         self.ca_cert_dir = ca_cert_dir
         self.ssl_version = ssl_version
@@ -787,6 +788,7 @@ def _prepare_conn(self, conn):
 
         if isinstance(conn, VerifiedHTTPSConnection):
             conn.set_cert(key_file=self.key_file,
+                          key_password=self.key_password,
                           cert_file=self.cert_file,
                           cert_reqs=self.cert_reqs,
                           ca_certs=self.ca_certs,
@@ -824,7 +826,9 @@ def _new_conn(self):
 
         conn = self.ConnectionCls(host=actual_host, port=actual_port,
                                   timeout=self.timeout.connect_timeout,
-                                  strict=self.strict, **self.conn_kw)
+                                  strict=self.strict, cert_file=self.cert_file,
+                                  key_file=self.key_file, key_password=self.key_password,
+                                  **self.conn_kw)
 
         return self._prepare_conn(conn)
 

src/urllib3/contrib/pyopenssl.py

@@ -434,7 +434,9 @@ def load_verify_locations(self, cafile=None, capath=None, cadata=None):
     def load_cert_chain(self, certfile, keyfile=None, password=None):
         self._ctx.use_certificate_chain_file(certfile)
         if password is not None:
-            self._ctx.set_passwd_cb(lambda max_length, prompt_twice, userdata: password)
+            if not isinstance(password, six.binary_type):
+                password = password.encode('utf-8')
+            self._ctx.set_passwd_cb(lambda *_: password)
         self._ctx.use_privatekey_file(keyfile or certfile)
 
     def wrap_socket(self, sock, server_side=False,

src/urllib3/poolmanager.py

@@ -20,7 +20,8 @@
 log = logging.getLogger(__name__)
 
 SSL_KEYWORDS = ('key_file', 'cert_file', 'cert_reqs', 'ca_certs',
-                'ssl_version', 'ca_cert_dir', 'ssl_context')
+                'ssl_version', 'ca_cert_dir', 'ssl_context',
+                'key_password')
 
 # All known keyword arguments that could be provided to the pool manager, its
 # pools, or the underlying connections. This is used to construct a pool key.
@@ -34,6 +35,7 @@
     'key_block',  # bool
     'key_source_address',  # str
     'key_key_file',  # str
+    'key_key_password',  # str
     'key_cert_file',  # str
     'key_cert_reqs',  # str
     'key_ca_certs',  # str

src/urllib3/util/ssl_.py

@@ -281,7 +281,7 @@ def create_urllib3_context(ssl_version=None, cert_reqs=None,
 def ssl_wrap_socket(sock, keyfile=None, certfile=None, cert_reqs=None,
                     ca_certs=None, server_hostname=None,
                     ssl_version=None, ciphers=None, ssl_context=None,
-                    ca_cert_dir=None):
+                    ca_cert_dir=None, key_password=None):
     """
     All arguments except for server_hostname, ssl_context, and ca_cert_dir have
     the same meaning as they do when using :func:`ssl.wrap_socket`.
@@ -297,6 +297,8 @@ def ssl_wrap_socket(sock, keyfile=None, certfile=None, cert_reqs=None,
         A directory containing CA certificates in multiple separate files, as
         supported by OpenSSL's -CApath flag or the capath argument to
         SSLContext.load_verify_locations().
+    :param key_password:
+        Optional password if the keyfile is encrypted.
     """
     context = ssl_context
     if context is None:
@@ -321,8 +323,17 @@ def ssl_wrap_socket(sock, keyfile=None, certfile=None, cert_reqs=None,
         # try to load OS default certs; works well on Windows (require Python3.4+)
         context.load_default_certs()
 
+    # Attempt to detect if we get the goofy behavior of the
+    # keyfile being encrypted and OpenSSL asking for the
+    # passphrase via the terminal and instead error out.
+    if keyfile and key_password is None and _is_key_file_encrypted(keyfile):
+        raise SSLError("Client private key is encrypted, password is required")
+
     if certfile:
-        context.load_cert_chain(certfile, keyfile)
+        if key_password is None:
+            context.load_cert_chain(certfile, keyfile)
+        else:
+            context.load_cert_chain(certfile, keyfile, key_password)
 
     # If we detect server_hostname is an IP address then the SNI
     # extension should not be used according to RFC3546 Section 3.1
@@ -356,5 +367,15 @@ def is_ipaddress(hostname):
     if six.PY3 and isinstance(hostname, bytes):
         # IDN A-label bytes are ASCII compatible.
         hostname = hostname.decode('ascii')
-
     return _IP_ADDRESS_REGEX.match(hostname) is not None
+
+
+def _is_key_file_encrypted(key_file):
+    """Detects if a key file is encrypted or not."""
+    with open(key_file, 'r') as f:
+        for line in f:
+            # Look for Proc-Type: 4,ENCRYPTED
+            if 'ENCRYPTED' in line:
+                return True
+
+    return False

test/__init__.py

@@ -137,6 +137,17 @@ def wrapper(*args, **kwargs):
     return wrapper
 
 
+def requires_ssl_context_keyfile_password(test):
+    @functools.wraps(test)
+    def wrapper(*args, **kwargs):
+        if ((not ssl_.IS_PYOPENSSL and sys.version_info < (2, 7, 9))
+                or ssl_.IS_SECURETRANSPORT):
+            pytest.skip("%s requires password parameter for "
+                        "SSLContext.load_cert_chain()" % test.__name__)
+        return test(*args, **kwargs)
+    return wrapper
+
+
 def fails_on_travis_gce(test):
     """Expect the test to fail on Google Compute Engine instances for Travis.
     Travis uses GCE for its sudo: enabled builds.

test/with_dummyserver/test_https.py

@@ -17,13 +17,14 @@
                                 DEFAULT_CLIENT_NO_INTERMEDIATE_CERTS,
                                 NO_SAN_CERTS, NO_SAN_CA, DEFAULT_CA_DIR,
                                 IPV6_ADDR_CERTS, IPV6_ADDR_CA, HAS_IPV6,
-                                IP_SAN_CERTS)
+                                IP_SAN_CERTS, PASSWORD_CLIENT_KEYFILE)
 
 from test import (
     onlyPy279OrNewer,
     notSecureTransport,
     notOpenSSL098,
     requires_network,
+    requires_ssl_context_keyfile_password,
     fails_on_travis_gce,
     TARPIT_HOST,
 )
@@ -113,6 +114,38 @@ def test_client_no_intermediate(self):
             if not ('An existing connection was forcibly closed by the remote host' in str(e)):
                 raise
 
+    @requires_ssl_context_keyfile_password
+    def test_client_key_password(self):
+        client_cert, client_key = (
+            DEFAULT_CLIENT_CERTS['certfile'],
+            PASSWORD_CLIENT_KEYFILE,
+        )
+        https_pool = HTTPSConnectionPool(self.host, self.port,
+                                         key_file=client_key,
+                                         cert_file=client_cert,
+                                         key_password="letmein")
+        r = https_pool.request('GET', '/certificate')
+        subject = json.loads(r.data.decode('utf-8'))
+        assert subject['organizationalUnitName'].startswith(
+            'Testing server cert')
+
+    @requires_ssl_context_keyfile_password
+    def test_client_encrypted_key_requires_password(self):
+        client_cert, client_key = (
+            DEFAULT_CLIENT_CERTS['certfile'],
+            PASSWORD_CLIENT_KEYFILE,
+        )
+        https_pool = HTTPSConnectionPool(self.host, self.port,
+                                         key_file=client_key,
+                                         cert_file=client_cert,
+                                         key_password=None)
+
+        with pytest.raises(MaxRetryError) as e:
+            https_pool.request('GET', '/certificate')
+
+        assert 'password is required' in str(e.value)
+        assert isinstance(e.value.reason, SSLError)
+
     def test_verified(self):
         https_pool = HTTPSConnectionPool(self.host, self.port,
                                          cert_reqs='CERT_REQUIRED',