Browse Source

Enable SSL verification for `urllib3`

By default, `urllib3` does not verify HTTPS requests.

As recommended in the `urllib3` documentation [0], SSL verification is
enabled by including the `certifi` package which comes with Mozilla's
root certificate bundle. This silences an `InsecureRequestWarning`
otherwise issued by `urllib3`.

Before Python 2.7.9, the `ssl` module lacks SNI support and may lag
behind in security updates [1, 2, 3, 4]. The recommended approach by
`urllib3` is to use pyOpenSSL as a replacement for those versions. This
silences `InsecurePlatformWarning` and `SNIMissingWarning` otherwise
issued by `urllib3` when run on these versions.

[0]: https://urllib3.readthedocs.io/en/latest/user-guide.html#certificate-verification
[1]: https://urllib3.readthedocs.io/en/latest/user-guide.html#certificate-verification-in-python-2
[2]: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-9365
[3]: https://www.python.org/dev/peps/pep-0476/
[4]: https://www.python.org/dev/peps/pep-0493/
Daniel Andersson 7 years ago
parent
commit
48e2ef8c2f
4 changed files with 45 additions and 7 deletions
  1. 26 3
      dulwich/client.py
  2. 17 2
      dulwich/tests/test_client.py
  3. 1 1
      requirements.txt
  4. 1 1
      setup.py

+ 26 - 3
dulwich/client.py

@@ -60,6 +60,7 @@ except ImportError:
     import urllib.request as urllib2
     import urllib.parse as urlparse
 
+import certifi
 import urllib3
 import urllib3.util
 
@@ -111,6 +112,13 @@ from dulwich.refs import (
     )
 
 
+if sys.version_info < (2, 7, 9):
+    # Before Python 2.7.9 the `ssl` module lacks SNI support and lags behind in
+    # security updates. Use pyOpenSSL instead.
+    import urllib3.contrib.pyopenssl
+    urllib3.contrib.pyopenssl.inject_into_urllib3()
+
+
 def _fileno_can_read(fileno):
     """Check if a file descriptor is readable."""
     return len(select.select([fileno], [], [], 0)[0]) > 0
@@ -1179,7 +1187,16 @@ def default_user_agent_string():
     return "git/dulwich/%s" % ".".join([str(x) for x in dulwich.__version__])
 
 
-def default_urllib3_manager(config):
+def default_urllib3_manager(config, verify_ssl=True):
+    """Return `urllib3` connection pool manager.
+
+    Honour detected proxy configurations.
+
+    :param config: `dulwich.config.ConfigDict` instance with Git configuration.
+    :param verify_ssl: Whether SSL verification is enabled.
+    :return: `urllib3.ProxyManager` instance for proxy configurations,
+        `urllib3.PoolManager` otherwise.
+    """
     proxy_server = user_agent = None
 
     if config is not None:
@@ -1192,18 +1209,24 @@ def default_urllib3_manager(config):
         except KeyError:
             pass
 
+    ssl_kwargs = {}
+    if verify_ssl:
+        ssl_kwargs.update(cert_reqs="CERT_REQUIRED", ca_certs=certifi.where())
+
     if user_agent is None:
         user_agent = default_user_agent_string()
 
     headers = {"User-agent": user_agent}
+
     if proxy_server is not None:
         # `urllib3` requires a `str` object in both Python 2 and 3, while
         # `ConfigDict` coerces entries to `bytes` on Python 3. Compensate.
         if not isinstance(proxy_server, str):
             proxy_server = proxy_server.decode()
-        manager = urllib3.ProxyManager(proxy_server, headers=headers)
+        manager = urllib3.ProxyManager(proxy_server, headers=headers,
+                                       **ssl_kwargs)
     else:
-        manager = urllib3.PoolManager(headers=headers)
+        manager = urllib3.PoolManager(headers=headers, **ssl_kwargs)
 
     return manager
 

+ 17 - 2
dulwich/tests/test_client.py

@@ -34,6 +34,7 @@ try:
 except ImportError:
     import urllib.parse as urlparse
 
+import certifi
 import urllib3
 
 import dulwich
@@ -935,21 +936,35 @@ class TCPGitClientTests(TestCase):
 
 class DefaultUrllib3ManagerTest(TestCase):
 
+    def assert_verify_ssl(self, manager, assertion=True):
+        pool_keywords = tuple(manager.connection_pool_kw.items())
+        assert_method = self.assertIn if assertion else self.assertNotIn
+        assert_method(('cert_reqs', 'CERT_REQUIRED'), pool_keywords)
+        assert_method(('ca_certs', certifi.where()), pool_keywords)
+
     def test_no_config(self):
-        default_urllib3_manager(config=None)
+        manager = default_urllib3_manager(config=None)
+        self.assert_verify_ssl(manager)
 
     def test_config_no_proxy(self):
-        default_urllib3_manager(config=ConfigDict())
+        manager = default_urllib3_manager(config=ConfigDict())
+        self.assert_verify_ssl(manager)
 
     def test_config_proxy(self):
         config = ConfigDict()
         config.set(b'http', b'proxy', b'http://localhost:3128/')
         manager = default_urllib3_manager(config=config)
+
         self.assertIsInstance(manager, urllib3.ProxyManager)
         self.assertTrue(hasattr(manager, 'proxy'))
         self.assertEqual(manager.proxy.scheme, 'http')
         self.assertEqual(manager.proxy.host, 'localhost')
         self.assertEqual(manager.proxy.port, 3128)
+        self.assert_verify_ssl(manager)
+
+    def test_config_no_verify_ssl(self):
+        manager = default_urllib3_manager(config=None, verify_ssl=False)
+        self.assert_verify_ssl(manager, assertion=False)
 
 
 class SubprocessSSHVendorTests(TestCase):

+ 1 - 1
requirements.txt

@@ -1 +1 @@
-urllib3==1.22
+urllib3[secure]==1.22

+ 1 - 1
setup.py

@@ -113,7 +113,7 @@ setup(name='dulwich',
           'fastimport': ['fastimport'],
       },
       ext_modules=ext_modules,
-      install_requires=['urllib3>=1.21'],
+      install_requires=['urllib3[secure]>=1.21'],
       test_suite='dulwich.tests.test_suite',
       tests_require=tests_require,
       distclass=DulwichDistribution,