cdent / wsgi-intercept

Intercept socket connection to wsgi applications for testing
MIT License
42 stars 21 forks source link

HTTPS connections with Requests library don't support verify=False to outside domains #59

Closed fuzzyrichie closed 5 years ago

fuzzyrichie commented 5 years ago

Was working on some local tests that use verify=False in the code to do calls using the Requests library, and any calls to external resources with this flag were getting SSL verification issues regardless.

In WSGI_HTTPSConnection, it doesn't handle the change in the SSL context when calling it for external resources. I've got a patch locally that works for this + with tests. Example test:

def test_https_no_ssl_verification_not_intercepted():
    with InstalledApp(wsgi_app.simple_app, host=HOST, port=443) as app:
        resp = requests.get('https://self-signed.badssl.com/', verify=False)
        assert resp.status_code >= 200 and resp.status_code < 300
        assert not app.success()
fuzzyrichie commented 5 years ago

Patch that I created locally:

diff --git a/wsgi_intercept/__init__.py b/wsgi_intercept/__init__.py
index 81de790..d2cb2c2 100644
--- a/wsgi_intercept/__init__.py
+++ b/wsgi_intercept/__init__.py
@@ -623,6 +623,13 @@ class WSGI_HTTPSConnection(HTTPSConnection, WSGI_HTTPConnection):
             else:
                 try:
                     import ssl
+                    if hasattr(self, '_context'):
+                        self._context.check_hostname = self.assert_hostname
+                        if isinstance(self.cert_reqs, ssl.VerifyMode):
+                            self._context.verify_mode = self.cert_reqs
+                        else:
+                            self._context.verify_mode = ssl.VerifyMode[self.cert_reqs]
+
                     if not hasattr(self, 'key_file'):
                         self.key_file = None
                     if not hasattr(self, 'cert_file'):
diff --git a/wsgi_intercept/tests/test_requests.py b/wsgi_intercept/tests/test_requests.py
index 4d69ba9..c8eb663 100644
--- a/wsgi_intercept/tests/test_requests.py
+++ b/wsgi_intercept/tests/test_requests.py
@@ -59,6 +59,20 @@ def test_https():
         assert app.success()

+def test_https_no_ssl_verification_intercepted():
+    with InstalledApp(wsgi_app.simple_app, host=HOST, port=443) as app:
+        resp = requests.get('https://some_hopefully_nonexistant_domain:443/', verify=False)
+        assert resp.content == b'WSGI intercept successful!\n'
+        assert app.success()
+
+
+def test_https_no_ssl_verification_not_intercepted():
+    with InstalledApp(wsgi_app.simple_app, host=HOST, port=443) as app:
+        resp = requests.get('https://self-signed.badssl.com/', verify=False)
+        assert resp.status_code >= 200 and resp.status_code < 300
+        assert not app.success()
+
+
 def test_https_default_port():
     with InstalledApp(wsgi_app.simple_app, host=HOST, port=443) as app:
         resp = requests.get('https://some_hopefully_nonexistant_domain/')
cdent commented 5 years ago

If you can turn this into a pull request I'll happily merge it. Thanks for noticing (and figuring out how to fix) the issue.

fuzzyrichie commented 5 years ago

I can't push a fix unfortunately - getting permission denied in doing so. Any permissions you need to set on your end?

cdent commented 5 years ago

You need to fork the repo into your own user and then create a pull request, which I can then merge. If that's not something you know how to do, let me know and I'll incorporate the change myself.

fuzzyrichie commented 5 years ago

Didn't see that workflow, but makes sense - https://github.com/cdent/wsgi-intercept/pull/60

fuzzyrichie commented 5 years ago

Thanks for all your help @cdent - are you OK if I close this ticket as resolved?

cdent commented 5 years ago

are you OK if I close this ticket as resolved?

Since I'm here, I'll get it. Thanks again for fixing this.