Disallow verify=false

CHANGELOG.rst

@@ -19,6 +19,8 @@ Version 0.5.0
   output.
 - Fix compatibility with iCloud again.
 - Use only one worker if debug mode is activated.
+- ``verify=false`` is now disallowed in vdirsyncer, please use
+  ``verify_fingerprint`` instead.
 
 Version 0.4.4
 =============

docs/ssl-tutorial.rst

@@ -4,57 +4,49 @@
 SSL and certificate validation
 ==============================
 
-Vdirsyncer uses the requests_ library for all its HTTP and SSL interaction.
+All SSL configuration is done per-storage.
 
-All SSL configuration is done per-storage. Storages that have anything to do
+Pinning by fingerprint
-with SSL have two parameters: ``verify`` and ``verify_fingerprint``.
+----------------------
 
-- The ``verify`` parameter determines whether to verify SSL certificates the
+To pin the certificate by SHA1- or MD5-fingerprint::
-  way browsers do: By comparing against a trust store, and by checking the
-  certificate's expiration date.
 
-  1. The default, ``true``, means that certificates will be validated against a
+    [storage foo]
-     set of trusted CAs. See :ref:`ssl-cas`.
+    type = caldav
+    ...
+    verify_fingerprint = "94:FD:7A:CB:50:75:A4:69:82:0A:F8:23:DF:07:FC:69:3E:CD:90:CA"
 
-  2. The value ``false`` will disable both trusted-CA-validation and the
+This disables :ref:`validation against root CAs <ssl-cas>`, since
-     validation of the certificate's expiration date. Unless combined with
+``verify_fingerprint`` is much stricter anyway.
-     ``verify_fingerprint``, **you should not use this value at all, because
-     it's a security risk**.
-
-  3. You can also set ``verify`` to a path of the server's certificate in PEM
-     format, instead of relying on the default root CAs::
-
-         [storage foo]
-         type = caldav
-         ...
-         verify = "/path/to/cert.pem"
-
-- The ``verify_fingerprint`` parameter can be used to compare the SSL
-  fingerprint to a fixed value. The value can be either a SHA1-fingerprint or
-  an MD5 one::
-
-      [storage foo]
-      type = caldav
-      ...
-      verify_fingerprint = "94:FD:7A:CB:50:75:A4:69:82:0A:F8:23:DF:07:FC:69:3E:CD:90:CA"
-
-  Using it will implicitly set ``verify=False``, which means that the pinned
-  certificate doesn't have to be by a trusted CA to be accepted by vdirsyncer.
 
 .. _ssl-cas:
 
-Trusted CAs
+Custom root CAs
------------
+---------------
 
-As said, vdirsyncer uses the requests_ library, which, by default, `uses its
+To point vdirsyncer to a custom set of root CAs::
-own set of trusted CAs
+
+    [storage foo]
+    type = caldav
+    ...
+    verify = "/path/to/cert.pem"
+
+Vdirsyncer uses the requests_ library, which, by default, `uses its own set of
+trusted CAs
 <http://www.python-requests.org/en/latest/user/advanced/#ca-certificates>`_.
 
 However, the actual behavior depends on how you have installed it. Some Linux
 distributions, such as Debian, patch their ``python-requests`` package to use
 the system certificate CAs. Normally these two stores are similar enough for
-you not to care. If the behavior on your system is somehow confusing, your best
+you to not care.
-bet is explicitly setting the SSL options above.
+
+But there are cases where certificate validation fails even though you can
+access the server fine through e.g. your browser. This usually indicates that
+your installation of the ``requests`` library is somehow broken. In such cases,
+it makes sense to explicitly set ``verify`` or ``verify_fingerprint`` as shown
+above.
+
+.. _requests: http://www.python-requests.org/
 
 .. _ssl-client-certs:
 
@@ -75,5 +67,3 @@ If the key and certificate are separate, a list may be used::
    type = caldav
    ...
    auth_cert = ["/path/to/certificate.crt", "/path/to/key.key"]
-
-.. _requests: http://www.python-requests.org/

tests/storage/test_http.py

@@ -111,3 +111,11 @@ def test_prepare_auth_guess(monkeypatch, auth):
         prepare_auth(auth, 'user', 'pwd')
 
     assert 'requests_toolbelt is too old' in str(excinfo.value).lower()
+
+
+def test_verify_false_disallowed():
+    with pytest.raises(ValueError) as excinfo:
+        HttpStorage(url='http://example.com', verify=False)
+
+    assert 'forbidden' in str(excinfo.value).lower()
+    assert 'consider setting verify_fingerprint' in str(excinfo.value).lower()

vdirsyncer/storage/dav.py

@@ -215,11 +215,11 @@ class DavSession(object):
             password = get_password(username, url)
 
         self._settings = {
-            'verify': prepare_verify(verify),
             'auth': prepare_auth(auth, username, password),
-            'verify_fingerprint': verify_fingerprint,
             'cert': prepare_client_cert(auth_cert),
         }
+        self._settings.update(prepare_verify(verify, verify_fingerprint))
+
         self.useragent = useragent
         self.url = url.rstrip('/') + '/'
         self.parsed_url = utils.compat.urlparse.urlparse(self.url)

vdirsyncer/storage/http.py

@@ -36,10 +36,29 @@ def prepare_auth(auth, username, password):
         return None
 
 
-def prepare_verify(verify):
+def prepare_verify(verify, verify_fingerprint):
     if isinstance(verify, (text_type, bytes)):
-        return expand_path(verify)
+        verify = expand_path(verify)
-    return verify
+    elif not isinstance(verify, bool):
+        raise ValueError('Invalid value for verify ({}), '
+                         'must be a path to a PEM-file or boolean.'
+                         .format(verify))
+
+    if verify_fingerprint is not None:
+        if not isinstance(verify_fingerprint, str):
+            raise ValueError('Invalid value for verify_fingerprint ({}), '
+                             'must be a string or null.'
+                             .format(verify_fingerprint))
+        verify = False
+    elif not verify:
+        raise ValueError('verify = false is forbidden. Consider setting '
+                         'verify_fingerprint instead, which also disables '
+                         'validation against CAs.')
+
+    return {
+        'verify': verify,
+        'verify_fingerprint': verify_fingerprint,
+    }
 
 
 def prepare_client_cert(cert):
@@ -105,12 +124,12 @@ class HttpStorage(Storage):
             password = get_password(username, url)
 
         self._settings = {
-            'verify': prepare_verify(verify),
-            'verify_fingerprint': verify_fingerprint,
             'auth': prepare_auth(auth, username, password),
             'cert': prepare_client_cert(auth_cert),
             'latin1_fallback': False,
         }
+        self._settings.update(prepare_verify(verify, verify_fingerprint))
+
         self.username, self.password = username, password
         self.useragent = useragent