From d27e5b832963756a6c48b5067b51d00f46ae4c9b Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Mon, 5 Jan 2015 19:33:04 +0100 Subject: [PATCH 1/9] Add documentation about certificate validation --- docs/tutorial.rst | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/docs/tutorial.rst b/docs/tutorial.rst index 2efaf31..6580238 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -143,3 +143,44 @@ it will not automatically start synchronizing those [2]_. You should run .. [2] Because collections are added rarely, and checking for this case before every synchronization isn't worth the overhead. + +SSL +--- + +Vdirsyncer uses the requests_ library for all its HTTP and SSL interaction. + +All SSL configuration is done per-storage. Storages that have anything to do +with SSL have two parameters: ``verify`` and ``verify_fingerprint``. + +- The ``verify`` parameter determines whether to verify SSL certificates + against a set of trusted root CAs or PEM certificates. + + 1. The default, ``true``, means that certificates will be validated against + `requests' own set of root CAs + `_. + + 2. The value ``false`` will disable any validation. Unless combined with + ``verify_fingerprint``, you should not use this value at all. + + 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 *additionally* 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 *not* turn off the normal CA validation. If you use a + self-signed certificate which you want to pin down, you have to also set + ``verify = False``. + +.. _requests: www.python-requests.org/ From e3bc764515a62bf67bb662e48cd1d93ca26ed2ef Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Mon, 5 Jan 2015 19:36:15 +0100 Subject: [PATCH 2/9] Deduplicate HTTP storage args --- vdirsyncer/storage/dav.py | 16 ++++------------ vdirsyncer/storage/http.py | 15 ++++++++++----- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index 23766c3..69c9092 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -14,7 +14,8 @@ from requests import session as requests_session from requests.exceptions import HTTPError from .base import Item, Storage -from .http import USERAGENT, prepare_auth, prepare_verify +from .http import HTTP_STORAGE_PARAMETERS, USERAGENT, prepare_auth, \ + prepare_verify from .. import exceptions, log, utils @@ -250,18 +251,9 @@ class DavSession(object): class DavStorage(Storage): - ''' + __doc__ = ''' :param url: Base URL or an URL to a collection. - :param username: Username for authentication. - :param password: Password for authentication. - :param verify: Verify SSL certificate, default True. This can also be a - local path to a self-signed SSL certificate. - :param verify_fingerprint: Optional. SHA1 or MD5 fingerprint of the - expected server certificate. - :param auth: Optional. Either ``basic``, ``digest`` or ``guess``. Default - ``guess``. If you know yours, consider setting it explicitly for - performance. - :param useragent: Default ``vdirsyncer``. + ''' + HTTP_STORAGE_PARAMETERS + ''' :param unsafe_href_chars: Replace the given characters when generating hrefs. Defaults to ``'@'``. diff --git a/vdirsyncer/storage/http.py b/vdirsyncer/storage/http.py index 190c1e8..fa37ded 100644 --- a/vdirsyncer/storage/http.py +++ b/vdirsyncer/storage/http.py @@ -44,11 +44,7 @@ def prepare_verify(verify): return verify -class HttpStorage(Storage): - ''' - Use a simple ``.ics`` file (or similar) from the web. - - :param url: URL to the ``.ics`` file. +HTTP_STORAGE_PARAMETERS = ''' :param username: Username for authentication. :param password: Password for authentication. :param verify: Verify SSL certificate, default True. This can also be a @@ -59,6 +55,15 @@ class HttpStorage(Storage): ``guess``. If you know yours, consider setting it explicitly for performance. :param useragent: Default ``vdirsyncer``. +''' + + +class HttpStorage(Storage): + __doc__ = ''' + Use a simple ``.ics`` file (or similar) from the web. + + :param url: URL to the ``.ics`` file. + ''' + HTTP_STORAGE_PARAMETERS + ''' A simple example:: From ebedca1e8561d19b86a3155b228b8e7a71e913aa Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Mon, 5 Jan 2015 19:36:39 +0100 Subject: [PATCH 3/9] add testcase --- tests/utils/test_main.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py index cf84838..34f650e 100644 --- a/tests/utils/test_main.py +++ b/tests/utils/test_main.py @@ -208,6 +208,8 @@ def test_get_class_init_args_on_storage(): assert not required +@pytest.mark.skipif(not utils.compat.PY2, + reason='https://github.com/shazow/urllib3/issues/529') 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' @@ -221,3 +223,7 @@ def test_request_ssl(httpsserver): utils.request('GET', httpsserver.url, verify=False, verify_fingerprint=sha1) utils.request('GET', httpsserver.url, verify=False, verify_fingerprint=md5) + with pytest.raises(requests.exceptions.SSLError) as excinfo: + utils.request('GET', httpsserver.url, verify=False, + verify_fingerprint=''.join(reversed(sha1))) + assert 'Fingerprints did not match' in str(excinfo.value) From a1bf00837d9074fe418e0898acf2e014a315b4f6 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Mon, 5 Jan 2015 19:45:36 +0100 Subject: [PATCH 4/9] Refer to SSL tutorial from config docs --- docs/tutorial.rst | 2 ++ vdirsyncer/storage/http.py | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/tutorial.rst b/docs/tutorial.rst index 6580238..35ecca2 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -144,6 +144,8 @@ it will not automatically start synchronizing those [2]_. You should run .. [2] Because collections are added rarely, and checking for this case before every synchronization isn't worth the overhead. +.. _ssl-tutorial: + SSL --- diff --git a/vdirsyncer/storage/http.py b/vdirsyncer/storage/http.py index fa37ded..d3e7498 100644 --- a/vdirsyncer/storage/http.py +++ b/vdirsyncer/storage/http.py @@ -48,9 +48,11 @@ HTTP_STORAGE_PARAMETERS = ''' :param username: Username for authentication. :param password: Password for authentication. :param verify: Verify SSL certificate, default True. This can also be a - local path to a self-signed SSL certificate. + local path to a self-signed SSL certificate. See :ref:`ssl-tutorial` + for more information. :param verify_fingerprint: Optional. SHA1 or MD5 fingerprint of the - expected server certificate. + expected server certificate. See :ref:`ssl-tutorial` for more + information. :param auth: Optional. Either ``basic``, ``digest`` or ``guess``. Default ``guess``. If you know yours, consider setting it explicitly for performance. From 7ddd3a5fdd57410a5135a012f37155103a4e129d Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Mon, 5 Jan 2015 20:11:16 +0100 Subject: [PATCH 5/9] Raise requests dep version --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 35ad5ae..299940c 100644 --- a/setup.py +++ b/setup.py @@ -39,7 +39,7 @@ setup( }, install_requires=[ 'click>=3.1', - 'requests>=2.1', + 'requests>=2.4.1', 'lxml>=3.0', 'icalendar>=3.6', 'requests_toolbelt>=0.3.0' From 3d62c19f42c32b760c4d3a2b28b6d788d7627573 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Mon, 5 Jan 2015 20:13:47 +0100 Subject: [PATCH 6/9] Add comments on dependencies --- setup.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/setup.py b/setup.py index 299940c..dc91380 100644 --- a/setup.py +++ b/setup.py @@ -38,10 +38,13 @@ setup( 'console_scripts': ['vdirsyncer = vdirsyncer.cli:main'] }, install_requires=[ + # https://github.com/mitsuhiko/click/issues/200 'click>=3.1', + # https://github.com/shazow/urllib3/pull/444 'requests>=2.4.1', 'lxml>=3.0', 'icalendar>=3.6', + # https://github.com/sigmavirus24/requests-toolbelt/pull/28 'requests_toolbelt>=0.3.0' ], extras_require={'keyring': ['keyring']} From f76aeba9bc5ed08e87091a73deae1ff2d5ab5c2d Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 6 Jan 2015 13:54:46 +0100 Subject: [PATCH 7/9] Some clarifications --- docs/tutorial.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/tutorial.rst b/docs/tutorial.rst index 35ecca2..f06cf24 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -154,14 +154,14 @@ Vdirsyncer uses the requests_ library for all its HTTP and SSL interaction. All SSL configuration is done per-storage. Storages that have anything to do with SSL have two parameters: ``verify`` and ``verify_fingerprint``. -- The ``verify`` parameter determines whether to verify SSL certificates - against a set of trusted root CAs or PEM certificates. +- The ``verify`` parameter determines whether to verify SSL certificates. 1. The default, ``true``, means that certificates will be validated against `requests' own set of root CAs `_. - 2. The value ``false`` will disable any validation. Unless combined with + 2. The value ``false`` will disable both trusted-CA-validation and the + validation of the certificate's expiration date. Unless combined with ``verify_fingerprint``, you should not use this value at all. 3. You can also set ``verify`` to a path of the server's certificate in PEM @@ -170,7 +170,7 @@ with SSL have two parameters: ``verify`` and ``verify_fingerprint``. [storage foo] type = caldav ... - verify = /path/to/cert.pem + verify = "/path/to/cert.pem" - The ``verify_fingerprint`` parameter can be used to *additionally* compare the SSL fingerprint to a fixed value. The value can be either a @@ -183,6 +183,6 @@ with SSL have two parameters: ``verify`` and ``verify_fingerprint``. Using it will *not* turn off the normal CA validation. If you use a self-signed certificate which you want to pin down, you have to also set - ``verify = False``. + ``verify = false``. .. _requests: www.python-requests.org/ From 11bd19febf8b0d580df30914a13e6d0cb425c2bd Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 6 Jan 2015 14:41:16 +0100 Subject: [PATCH 8/9] Make the SSL stuff into a new page --- docs/index.rst | 3 ++- docs/ssl-tutorial.rst | 56 +++++++++++++++++++++++++++++++++++++++++++ docs/tutorial.rst | 43 --------------------------------- 3 files changed, 58 insertions(+), 44 deletions(-) create mode 100644 docs/ssl-tutorial.rst diff --git a/docs/index.rst b/docs/index.rst index a90cf65..3e912d6 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -18,8 +18,9 @@ Table of Contents :maxdepth: 1 tutorial - config + ssl-tutorial keyring + config supported problems vdir diff --git a/docs/ssl-tutorial.rst b/docs/ssl-tutorial.rst new file mode 100644 index 0000000..b2d59c5 --- /dev/null +++ b/docs/ssl-tutorial.rst @@ -0,0 +1,56 @@ +.. _ssl-tutorial: + +============================== +SSL and certificate validation +============================== + +Vdirsyncer uses the requests_ library for all its HTTP and SSL interaction. + +All SSL configuration is done per-storage. Storages that have anything to do +with SSL have two parameters: ``verify`` and ``verify_fingerprint``. + +- The ``verify`` parameter determines whether to verify SSL certificates. + + 1. The default, ``true``, means that certificates will be validated against a + set of trusted CAs. See :ref:`ssl-cas`. + + 2. The value ``false`` will disable both trusted-CA-validation and the + validation of the certificate's expiration date. Unless combined with + ``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 effectively set ``verify=False``. + +.. _ssl-cas: + +Trusted CAs +----------- + +As said, vdirsyncer uses the requests_ library for such parts, which, by +default, `uses its own set of trusted CAs +`_. + +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 +bet is explicitly setting the SSL options above. + +.. _requests: www.python-requests.org/ diff --git a/docs/tutorial.rst b/docs/tutorial.rst index f06cf24..2efaf31 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -143,46 +143,3 @@ it will not automatically start synchronizing those [2]_. You should run .. [2] Because collections are added rarely, and checking for this case before every synchronization isn't worth the overhead. - -.. _ssl-tutorial: - -SSL ---- - -Vdirsyncer uses the requests_ library for all its HTTP and SSL interaction. - -All SSL configuration is done per-storage. Storages that have anything to do -with SSL have two parameters: ``verify`` and ``verify_fingerprint``. - -- The ``verify`` parameter determines whether to verify SSL certificates. - - 1. The default, ``true``, means that certificates will be validated against - `requests' own set of root CAs - `_. - - 2. The value ``false`` will disable both trusted-CA-validation and the - validation of the certificate's expiration date. Unless combined with - ``verify_fingerprint``, you should not use this value at all. - - 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 *additionally* 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 *not* turn off the normal CA validation. If you use a - self-signed certificate which you want to pin down, you have to also set - ``verify = false``. - -.. _requests: www.python-requests.org/ From cda763fcc5bc4abed6061878a90f7c496fe8c28e Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sat, 10 Jan 2015 23:30:35 +0100 Subject: [PATCH 9/9] Set verify=False if verify_fingerprint is given --- tests/utils/test_main.py | 6 +++--- vdirsyncer/utils/__init__.py | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py index 34f650e..ad3d984 100644 --- a/tests/utils/test_main.py +++ b/tests/utils/test_main.py @@ -220,10 +220,10 @@ def test_request_ssl(httpsserver): utils.request('GET', httpsserver.url) assert 'certificate verify failed' in str(excinfo.value) utils.request('GET', httpsserver.url, verify=False) - utils.request('GET', httpsserver.url, verify=False, + utils.request('GET', httpsserver.url, verify_fingerprint=sha1) - utils.request('GET', httpsserver.url, verify=False, verify_fingerprint=md5) + utils.request('GET', httpsserver.url, verify_fingerprint=md5) with pytest.raises(requests.exceptions.SSLError) as excinfo: - utils.request('GET', httpsserver.url, verify=False, + utils.request('GET', httpsserver.url, verify_fingerprint=''.join(reversed(sha1))) assert 'Fingerprints did not match' in str(excinfo.value) diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py index 90f5f8d..18c1410 100644 --- a/vdirsyncer/utils/__init__.py +++ b/vdirsyncer/utils/__init__.py @@ -205,6 +205,7 @@ def request(method, url, session=None, latin1_fallback=True, session = requests.Session() if verify_fingerprint is not None: + kwargs['verify'] = False https_prefix = 'https://' if not isinstance(session.adapters[https_prefix], _FingerprintAdapter):