From 216e6c3b214a3d49ff969ef23e24aed3a392d81d Mon Sep 17 00:00:00 2001
From: Ben Boeckel <mathstuf@gmail.com>
Date: Wed, 5 Aug 2015 22:40:50 -0400
Subject: [PATCH] ssl: use verify and verify_fingerprint

Both have their uses. The latter is very strict in what it will accept,
but it does not catch expired certificates.
---
 CHANGELOG.rst              |  1 +
 docs/ssl-tutorial.rst      |  4 +---
 tests/utils/test_main.py   | 30 ++++++++++++++++++++----------
 vdirsyncer/storage/http.py |  9 +++++----
 vdirsyncer/utils/http.py   |  1 -
 5 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 4759521..af6fc6f 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -19,6 +19,7 @@ Version 0.6.0
 - **Packagers:** Don't use the GitHub tarballs, but the PyPI ones.
 - **Packagers:** ``build.sh`` is gone, and ``Makefile`` is included in
   tarballs. See the content of ``Makefile`` on how to run tests post-packaging.
+- ``verify_fingerprint`` doesn't automatically disable ``verify`` anymore.
 
 Version 0.5.2
 =============
diff --git a/docs/ssl-tutorial.rst b/docs/ssl-tutorial.rst
index d784b36..096d6ce 100644
--- a/docs/ssl-tutorial.rst
+++ b/docs/ssl-tutorial.rst
@@ -15,9 +15,7 @@ To pin the certificate by SHA1- or MD5-fingerprint::
     type = caldav
     ...
     verify_fingerprint = "94:FD:7A:CB:50:75:A4:69:82:0A:F8:23:DF:07:FC:69:3E:CD:90:CA"
-
-This disables :ref:`validation against root CAs <ssl-cas>`, since
-``verify_fingerprint`` is much stricter anyway.
+    #verify = false  # Optional: Disable CA validation, useful for self-signed certs
 
 .. _ssl-cas:
 
diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py
index 73547f8..e65a21b 100644
--- a/tests/utils/test_main.py
+++ b/tests/utils/test_main.py
@@ -221,9 +221,6 @@ def test_get_class_init_args_on_storage():
 
 
 def test_request_ssl(httpsserver):
-    sha1 = '94:FD:7A:CB:50:75:A4:69:82:0A:F8:23:DF:07:FC:69:3E:CD:90:CA'
-    md5 = '19:90:F7:23:94:F2:EF:AB:2B:64:2D:57:3D:25:95:2D'
-
     httpsserver.serve_content('')  # we need to serve something
 
     with pytest.raises(requests.exceptions.SSLError) as excinfo:
@@ -232,19 +229,32 @@ def test_request_ssl(httpsserver):
 
     utils.http.request('GET', httpsserver.url, verify=False)
 
-    # https://github.com/shazow/urllib3/issues/529
+
+def _fingerprints_broken():
     from pkg_resources import parse_version as ver
     tolerant_python = (
         utils.compat.PY2 and platform.python_implementation() != 'PyPy'
     )
     broken_urllib3 = ver(requests.__version__) <= ver('2.5.1')
-    if broken_urllib3 and not tolerant_python:
-        return
+    return broken_urllib3 and not tolerant_python
 
-    utils.http.request('GET', httpsserver.url,
-                       verify_fingerprint=sha1)
-    utils.http.request('GET', httpsserver.url, verify_fingerprint=md5)
+
+@pytest.mark.skipif(_fingerprints_broken(),
+                    reason='https://github.com/shazow/urllib3/issues/529')
+@pytest.mark.parametrize('fingerprint', [
+    '94:FD:7A:CB:50:75:A4:69:82:0A:F8:23:DF:07:FC:69:3E:CD:90:CA',
+    '19:90:F7:23:94:F2:EF:AB:2B:64:2D:57:3D:25:95:2D'
+])
+def test_request_ssl_fingerprints(httpsserver, fingerprint):
+    httpsserver.serve_content('')  # we need to serve something
+
+    utils.http.request('GET', httpsserver.url, verify=False,
+                       verify_fingerprint=fingerprint)
     with pytest.raises(requests.exceptions.SSLError) as excinfo:
         utils.http.request('GET', httpsserver.url,
-                           verify_fingerprint=''.join(reversed(sha1)))
+                           verify_fingerprint=fingerprint)
+
+    with pytest.raises(requests.exceptions.SSLError) as excinfo:
+        utils.http.request('GET', httpsserver.url, verify=False,
+                           verify_fingerprint=''.join(reversed(fingerprint)))
     assert 'Fingerprints did not match' in str(excinfo.value)
diff --git a/vdirsyncer/storage/http.py b/vdirsyncer/storage/http.py
index 8997afb..af86c52 100644
--- a/vdirsyncer/storage/http.py
+++ b/vdirsyncer/storage/http.py
@@ -52,11 +52,12 @@ def prepare_verify(verify, verify_fingerprint):
             raise exceptions.UserError('Invalid value for verify_fingerprint '
                                        '({}), must be a string or null.'
                                        .format(verify_fingerprint))
-        verify = False
     elif not verify:
-        raise exceptions.UserError('verify = false is forbidden. Consider '
-                                   'setting verify_fingerprint instead, which '
-                                   'also disables validation against CAs.')
+        raise exceptions.UserError(
+            'Disabling all SSL validation is forbidden. Consider setting '
+            'verify_fingerprint instead, which also disables validation '
+            'against CAs.'
+        )
 
     return {
         'verify': verify,
diff --git a/vdirsyncer/utils/http.py b/vdirsyncer/utils/http.py
index fba25a4..6b1fb2f 100644
--- a/vdirsyncer/utils/http.py
+++ b/vdirsyncer/utils/http.py
@@ -69,7 +69,6 @@ def request(method, url, session=None, latin1_fallback=True,
             raise RuntimeError('`verify_fingerprint` can only be used with '
                                'requests versions >= 2.4.1')
         _install_fingerprint_adapter(session, verify_fingerprint)
-        kwargs['verify'] = False
 
     func = session.request