From 550986895891153d2aa17161f6e0296f64f6ecb9 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 20 Aug 2014 18:43:34 +0200 Subject: [PATCH 01/10] Revert "Revert "Tls fingerprints"" --- AUTHORS.rst | 1 + CONTRIBUTING.rst | 2 +- tests/storage/test_http.py | 4 ++-- vdirsyncer/storage/dav.py | 15 ++++++++++----- vdirsyncer/storage/http.py | 6 +++++- vdirsyncer/utils/__init__.py | 31 +++++++++++++++++++++++++++---- 6 files changed, 46 insertions(+), 13 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index c4b63ec..89114c3 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -7,3 +7,4 @@ In alphabetical order: - Clément Mondon - Julian Mehne - Markus Unterwaditzer +- Thomas Weißschuh diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 2da2c5b..f5b83f4 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -20,6 +20,6 @@ * But not because you wrote too few tests. - * Add yourself to ``CONTRIBUTORS.rst``. Don't add anything to + * Add yourself to ``AUTHORS.rst``. Don't add anything to ``CHANGELOG.rst``, I do that myself shortly before the release. You can help by writing meaningful commit messages. diff --git a/tests/storage/test_http.py b/tests/storage/test_http.py index 648f98a..d3d74cf 100644 --- a/tests/storage/test_http.py +++ b/tests/storage/test_http.py @@ -41,7 +41,7 @@ def test_list(monkeypatch): u'\n'.join([u'BEGIN:VCALENDAR'] + items + [u'END:VCALENDAR']) ] * 2 - def get(method, url, *a, **kw): + def get(self, method, url, *a, **kw): assert method == 'GET' assert url == collection_url r = Response() @@ -52,7 +52,7 @@ def test_list(monkeypatch): r.encoding = 'ISO-8859-1' return r - monkeypatch.setattr('requests.request', get) + monkeypatch.setattr('requests.sessions.Session.request', get) s = HttpStorage(url=collection_url) diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index e224b8f..a2b7b46 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -162,13 +162,14 @@ class DavSession(object): ''' def __init__(self, url, username='', password='', verify=True, auth=None, - useragent=USERAGENT, dav_header=None): + useragent=USERAGENT, tls_fingerprint=None, dav_header=None): if username and not password: password = utils.get_password(username, url) self._settings = { 'verify': prepare_verify(verify), - 'auth': prepare_auth(auth, username, password) + 'auth': prepare_auth(auth, username, password), + 'tls_fingerprint': tls_fingerprint, } self.useragent = useragent self.url = url.rstrip('/') + '/' @@ -222,6 +223,8 @@ class DavStorage(Storage): :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 tls_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. @@ -248,14 +251,15 @@ class DavStorage(Storage): def __init__(self, url, username='', password='', collection=None, verify=True, auth=None, useragent=USERAGENT, - unsafe_href_chars='@', **kwargs): + unsafe_href_chars='@', tls_fingerprint=None, **kwargs): super(DavStorage, self).__init__(**kwargs) url = url.rstrip('/') + '/' if collection is not None: url = utils.urlparse.urljoin(url, collection) self.session = DavSession(url, username, password, verify, auth, - useragent, dav_header=self.dav_header) + useragent, tls_fingerprint, + dav_header=self.dav_header) self.collection = collection self.unsafe_href_chars = unsafe_href_chars @@ -268,7 +272,8 @@ class DavStorage(Storage): if kwargs.pop('collection', None) is not None: raise TypeError('collection argument must not be given.') discover_args, _ = utils.split_dict(kwargs, lambda key: key in ( - 'username', 'password', 'verify', 'auth', 'useragent' + 'username', 'password', 'verify', 'auth', 'useragent', + 'tls_fingerprint', )) d = cls.discovery_class(DavSession( url=url, dav_header=cls.dav_header, **discover_args)) diff --git a/vdirsyncer/storage/http.py b/vdirsyncer/storage/http.py index ad918e6..4e0ef11 100644 --- a/vdirsyncer/storage/http.py +++ b/vdirsyncer/storage/http.py @@ -54,6 +54,8 @@ class HttpStorage(Storage): :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 tls_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. @@ -81,7 +83,8 @@ class HttpStorage(Storage): _items = None def __init__(self, url, username='', password='', collection=None, - verify=True, auth=None, useragent=USERAGENT, **kwargs): + verify=True, auth=None, useragent=USERAGENT, + tls_fingerprint=None, **kwargs): super(HttpStorage, self).__init__(**kwargs) if username and not password: @@ -89,6 +92,7 @@ class HttpStorage(Storage): self._settings = { 'verify': prepare_verify(verify), + 'tls_fingerprint': tls_fingerprint, 'auth': prepare_auth(auth, username, password), 'latin1_fallback': False } diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py index 75bc384..72b3067 100644 --- a/vdirsyncer/utils/__init__.py +++ b/vdirsyncer/utils/__init__.py @@ -10,6 +10,7 @@ import os import requests +from requests.packages.urllib3.poolmanager import PoolManager from .. import exceptions, log from ..doubleclick import click @@ -177,8 +178,23 @@ def _password_from_keyring(username, resource): key = new_key +class _FingerprintAdapter(requests.adapters.HTTPAdapter): + def __init__(self, fingerprint=None, **kwargs): + self.fingerprint = str(fingerprint) + super(_FingerprintAdapter, self).__init__(**kwargs) + + def send(self, *args, **kwargs): + return super(_FingerprintAdapter, self).send(*args, **kwargs) + + def init_poolmanager(self, connections, maxsize, block=False): + self.poolmanager = PoolManager(num_pools=connections, + maxsize=maxsize, + block=block, + assert_fingerprint=self.fingerprint) + + def request(method, url, data=None, headers=None, auth=None, verify=None, - session=None, latin1_fallback=True): + session=None, latin1_fallback=True, tls_fingerprint=None): ''' Wrapper method for requests, to ease logging and mocking. Parameters should be the same as for ``requests.request``, except: @@ -193,9 +209,16 @@ def request(method, url, data=None, headers=None, auth=None, verify=None, ''' if session is None: - func = requests.request - else: - func = session.request + session = requests.Session() + + if tls_fingerprint is not None: + https_prefix = 'https://' + + if not isinstance(session.adapters[https_prefix], _FingerprintAdapter): + fingerprint_adapter = _FingerprintAdapter(tls_fingerprint) + session.mount(https_prefix, fingerprint_adapter) + + func = session.request logger.debug(u'{} {}'.format(method, url)) logger.debug(headers) From c9c2a43f43bbfa18bad4c522bc447feb44bf6b2b Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 20 Aug 2014 23:39:26 +0200 Subject: [PATCH 02/10] Rename tls_fingerprint to verify_fingerprint --- vdirsyncer/storage/dav.py | 12 ++++++------ vdirsyncer/storage/http.py | 6 +++--- vdirsyncer/utils/__init__.py | 8 +++++--- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index a2b7b46..803d059 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -162,14 +162,14 @@ class DavSession(object): ''' def __init__(self, url, username='', password='', verify=True, auth=None, - useragent=USERAGENT, tls_fingerprint=None, dav_header=None): + useragent=USERAGENT, verify_fingerprint=None, dav_header=None): if username and not password: password = utils.get_password(username, url) self._settings = { 'verify': prepare_verify(verify), 'auth': prepare_auth(auth, username, password), - 'tls_fingerprint': tls_fingerprint, + 'verify_fingerprint': verify_fingerprint, } self.useragent = useragent self.url = url.rstrip('/') + '/' @@ -223,7 +223,7 @@ class DavStorage(Storage): :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 tls_fingerprint: Optional. SHA1 or MD5 fingerprint of the + :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 @@ -251,14 +251,14 @@ class DavStorage(Storage): def __init__(self, url, username='', password='', collection=None, verify=True, auth=None, useragent=USERAGENT, - unsafe_href_chars='@', tls_fingerprint=None, **kwargs): + unsafe_href_chars='@', verify_fingerprint=None, **kwargs): super(DavStorage, self).__init__(**kwargs) url = url.rstrip('/') + '/' if collection is not None: url = utils.urlparse.urljoin(url, collection) self.session = DavSession(url, username, password, verify, auth, - useragent, tls_fingerprint, + useragent, verify_fingerprint, dav_header=self.dav_header) self.collection = collection self.unsafe_href_chars = unsafe_href_chars @@ -273,7 +273,7 @@ class DavStorage(Storage): raise TypeError('collection argument must not be given.') discover_args, _ = utils.split_dict(kwargs, lambda key: key in ( 'username', 'password', 'verify', 'auth', 'useragent', - 'tls_fingerprint', + 'verify_fingerprint', )) d = cls.discovery_class(DavSession( url=url, dav_header=cls.dav_header, **discover_args)) diff --git a/vdirsyncer/storage/http.py b/vdirsyncer/storage/http.py index 4e0ef11..bb37ac2 100644 --- a/vdirsyncer/storage/http.py +++ b/vdirsyncer/storage/http.py @@ -54,7 +54,7 @@ class HttpStorage(Storage): :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 tls_fingerprint: Optional. SHA1 or MD5 fingerprint of the + :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 @@ -84,7 +84,7 @@ class HttpStorage(Storage): def __init__(self, url, username='', password='', collection=None, verify=True, auth=None, useragent=USERAGENT, - tls_fingerprint=None, **kwargs): + verify_fingerprint=None, **kwargs): super(HttpStorage, self).__init__(**kwargs) if username and not password: @@ -92,7 +92,7 @@ class HttpStorage(Storage): self._settings = { 'verify': prepare_verify(verify), - 'tls_fingerprint': tls_fingerprint, + 'verify_fingerprint': verify_fingerprint, 'auth': prepare_auth(auth, username, password), 'latin1_fallback': False } diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py index 72b3067..e8df1ff 100644 --- a/vdirsyncer/utils/__init__.py +++ b/vdirsyncer/utils/__init__.py @@ -194,12 +194,14 @@ class _FingerprintAdapter(requests.adapters.HTTPAdapter): def request(method, url, data=None, headers=None, auth=None, verify=None, - session=None, latin1_fallback=True, tls_fingerprint=None): + session=None, latin1_fallback=True, verify_fingerprint=None): ''' Wrapper method for requests, to ease logging and mocking. Parameters should be the same as for ``requests.request``, except: :param session: A requests session object to use. + :param verify_fingerprint: Optional. SHA1 or MD5 fingerprint of the + expected server certificate. :param latin1_fallback: RFC-2616 specifies the default Content-Type of text/* to be latin1, which is not always correct, but exactly what requests is doing. Setting this parameter to False will use charset @@ -211,11 +213,11 @@ def request(method, url, data=None, headers=None, auth=None, verify=None, if session is None: session = requests.Session() - if tls_fingerprint is not None: + if verify_fingerprint is not None: https_prefix = 'https://' if not isinstance(session.adapters[https_prefix], _FingerprintAdapter): - fingerprint_adapter = _FingerprintAdapter(tls_fingerprint) + fingerprint_adapter = _FingerprintAdapter(verify_fingerprint) session.mount(https_prefix, fingerprint_adapter) func = session.request From 44c1d84a7efad1592910ceb7e6d0a583a23c5784 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Thu, 21 Aug 2014 00:01:17 +0200 Subject: [PATCH 03/10] Fix style errors --- vdirsyncer/storage/dav.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index 803d059..19d8069 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -162,7 +162,8 @@ class DavSession(object): ''' def __init__(self, url, username='', password='', verify=True, auth=None, - useragent=USERAGENT, verify_fingerprint=None, dav_header=None): + useragent=USERAGENT, verify_fingerprint=None, + dav_header=None): if username and not password: password = utils.get_password(username, url) From f61ef5318dc3d72a053a022c784421f478372058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Wed, 20 Aug 2014 22:04:35 +0000 Subject: [PATCH 04/10] remove superfluous method override --- vdirsyncer/utils/__init__.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py index e8df1ff..0494ae8 100644 --- a/vdirsyncer/utils/__init__.py +++ b/vdirsyncer/utils/__init__.py @@ -183,9 +183,6 @@ class _FingerprintAdapter(requests.adapters.HTTPAdapter): self.fingerprint = str(fingerprint) super(_FingerprintAdapter, self).__init__(**kwargs) - def send(self, *args, **kwargs): - return super(_FingerprintAdapter, self).send(*args, **kwargs) - def init_poolmanager(self, connections, maxsize, block=False): self.poolmanager = PoolManager(num_pools=connections, maxsize=maxsize, From e8e55de165eafdf302e6fde28cce1d866f3fe04e Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Thu, 21 Aug 2014 00:26:39 +0200 Subject: [PATCH 05/10] Add some tests --- build.sh | 2 +- tests/utils/test_main.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/build.sh b/build.sh index 3203d39..54fdef1 100755 --- a/build.sh +++ b/build.sh @@ -33,7 +33,7 @@ _davserver() { } command__install_tests() { - $PIP_INSTALL pytest pytest-xprocess + $PIP_INSTALL pytest pytest-xprocess pytest-localserver _optimize_pip _davserver $DAV_SERVER [ "$TRAVIS" != "true" ] || $PIP_INSTALL coverage coveralls diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py index 929a48a..42b52d8 100644 --- a/tests/utils/test_main.py +++ b/tests/utils/test_main.py @@ -10,6 +10,8 @@ import click from click.testing import CliRunner import pytest +import requests + import vdirsyncer.utils as utils from vdirsyncer.utils.vobject import split_collection @@ -159,3 +161,14 @@ def test_get_class_init_args_on_storage(): all, required = utils.get_class_init_args(MemoryStorage) assert all == set(['collection', 'read_only', 'instance_name']) assert not required + + +def test_request_verify_fingerprint(httpsserver): + httpsserver.serve_content(content='hello', code=200, headers=None) + with pytest.raises(requests.exceptions.SSLError) as excinfo: + utils.request('GET', httpsserver.url) + assert 'certificate verify failed' in str(excinfo.value) + utils.request('GET', httpsserver.url, verify=False) + with pytest.raises(requests.exceptions.SSLError) as excinfo: + utils.request('GET', httpsserver.url, verify=False, + verify_fingerprint='ABCD') From b909d525f8f9ba93bed0ef7955802554769b0c69 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Thu, 21 Aug 2014 00:51:25 +0200 Subject: [PATCH 06/10] Fix broken testcase --- tests/utils/test_main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py index 42b52d8..657c11f 100644 --- a/tests/utils/test_main.py +++ b/tests/utils/test_main.py @@ -170,5 +170,5 @@ def test_request_verify_fingerprint(httpsserver): assert 'certificate verify failed' in str(excinfo.value) utils.request('GET', httpsserver.url, verify=False) with pytest.raises(requests.exceptions.SSLError) as excinfo: - utils.request('GET', httpsserver.url, verify=False, + utils.request('GET', httpsserver.url, verify=None, verify_fingerprint='ABCD') From 63c990a320974ab925c77e8fc3370d37daa68420 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Thu, 21 Aug 2014 00:56:13 +0200 Subject: [PATCH 07/10] Properly remove monkeypatching in radicale tests --- tests/storage/dav/servers/radicale/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/storage/dav/servers/radicale/__init__.py b/tests/storage/dav/servers/radicale/__init__.py index 3ba1dd8..9be6748 100644 --- a/tests/storage/dav/servers/radicale/__init__.py +++ b/tests/storage/dav/servers/radicale/__init__.py @@ -8,8 +8,6 @@ import pytest import wsgi_intercept import wsgi_intercept.requests_intercept -wsgi_intercept.requests_intercept.install() - RADICALE_SCHEMA = ''' create table collection ( @@ -93,10 +91,12 @@ class ServerMixin(object): do_the_radicale_dance(str(tmpdir)) from radicale import Application + wsgi_intercept.requests_intercept.install() wsgi_intercept.add_wsgi_intercept('127.0.0.1', 80, Application) def teardown(): wsgi_intercept.remove_wsgi_intercept('127.0.0.1', 80) + wsgi_intercept.requests_intercept.uninstall() request.addfinalizer(teardown) def get_storage_args(self, collection='test'): From b0939892205562f208c7291bc557941aed57d556 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Thu, 21 Aug 2014 01:26:00 +0200 Subject: [PATCH 08/10] Refine testcase again --- tests/utils/test_main.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py index 657c11f..a1b236e 100644 --- a/tests/utils/test_main.py +++ b/tests/utils/test_main.py @@ -163,12 +163,14 @@ def test_get_class_init_args_on_storage(): assert not required -def test_request_verify_fingerprint(httpsserver): - httpsserver.serve_content(content='hello', code=200, headers=None) +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' + with pytest.raises(requests.exceptions.SSLError) as excinfo: utils.request('GET', httpsserver.url) assert 'certificate verify failed' in str(excinfo.value) utils.request('GET', httpsserver.url, verify=False) - with pytest.raises(requests.exceptions.SSLError) as excinfo: - utils.request('GET', httpsserver.url, verify=None, - verify_fingerprint='ABCD') + utils.request('GET', httpsserver.url, verify=False, + verify_fingerprint=sha1) + utils.request('GET', httpsserver.url, verify=False, verify_fingerprint=md5) From 23ae6eb03f3053f650a20f93a7d88f214fd7eefd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Fri, 29 Aug 2014 17:24:27 +0000 Subject: [PATCH 09/10] we need the git version of werkzeug for ssl support on py3 --- build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sh b/build.sh index 54fdef1..5318f07 100755 --- a/build.sh +++ b/build.sh @@ -33,7 +33,7 @@ _davserver() { } command__install_tests() { - $PIP_INSTALL pytest pytest-xprocess pytest-localserver + $PIP_INSTALL pytest pytest-xprocess git+https://github.com/mitsuhiko/werkzeug/ pytest-localserver _optimize_pip _davserver $DAV_SERVER [ "$TRAVIS" != "true" ] || $PIP_INSTALL coverage coveralls From fa2f7ca5406ad033019bba45bf9660c82e9e0044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Fri, 29 Aug 2014 18:14:58 +0000 Subject: [PATCH 10/10] always serve something, else werkzeug blows up --- tests/utils/test_main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py index a1b236e..309b7ba 100644 --- a/tests/utils/test_main.py +++ b/tests/utils/test_main.py @@ -167,6 +167,8 @@ 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: utils.request('GET', httpsserver.url) assert 'certificate verify failed' in str(excinfo.value)