diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 318c397..c6e34ed 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -26,8 +26,6 @@ Version 0.19.0 - A new ``asyncio`` backend is now used. So far, this shows substantial speed improvements in ``discovery`` and ``metasync``, but little change in `sync`. This will likely continue improving over time. :gh:`906` -- Support for `md5` and `sha1` certificate fingerprints has been dropped. If - you're validating certificate fingerprints, use `sha256` instead. - The ``google`` storage types no longer require ``requests-oauthlib``, but require ``python-aiohttp-oauthlib`` instead. - Vdirsyncer no longer includes experimental support for `EteSync @@ -41,6 +39,22 @@ Version 0.19.0 .. _etesync-dav: https://github.com/etesync/etesync-dav +Changes to SSL configuration +---------------------------- + +Support for `md5` and `sha1` certificate fingerprints has been dropped. If +you're validating certificate fingerprints, use `sha256` instead. + +# XXX: just make it one arg + +When using a custom `verify_fingerprint`, CA validation is always disabled. + +If `verify_fingerprint` is unset, CA verification is always active. Disabling +both features is insecure and no longer supported. + +The `verify` parameter no longer takes boolean values, it is now optional and +only takes a string to a custom CA for verification. + Version 0.18.0 ============== diff --git a/docs/config.rst b/docs/config.rst index 3b85ecf..080420c 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -175,7 +175,7 @@ CalDAV and CardDAV url = "..." #username = "" #password = "" - #verify = true + #verify = /path/to/custom_ca.pem #auth = null #useragent = "vdirsyncer/0.16.4" #verify_fingerprint = null @@ -208,12 +208,10 @@ CalDAV and CardDAV :param url: Base URL or an URL to a calendar. :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. See :ref:`ssl-tutorial` - for more information. - :param verify_fingerprint: Optional. SHA1 or MD5 fingerprint of the - expected server certificate. See :ref:`ssl-tutorial` for more - information. + :param verify: Optional. Local path to a self-signed SSL certificate. + See :ref:`ssl-tutorial` for more information. + :param verify_fingerprint: Optional. SHA256 fingerprint of the expected + server certificate. See :ref:`ssl-tutorial` for more information. :param auth: Optional. Either ``basic``, ``digest`` or ``guess``. The default is preemptive Basic auth, sending credentials even if server didn't request them. This saves from an additional roundtrip per @@ -235,7 +233,7 @@ CalDAV and CardDAV url = "..." #username = "" #password = "" - #verify = true + #verify = /path/to/custom_ca.pem #auth = null #useragent = "vdirsyncer/0.16.4" #verify_fingerprint = null @@ -244,12 +242,10 @@ CalDAV and CardDAV :param url: Base URL or an URL to an addressbook. :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. See - :ref:`ssl-tutorial` for more information. - :param verify_fingerprint: Optional. SHA1 or MD5 fingerprint of the expected - server certificate. See :ref:`ssl-tutorial` for - more information. + :param verify: Optional. Local path to a self-signed SSL certificate. + See :ref:`ssl-tutorial` for more information. + :param verify_fingerprint: Optional. SHA256 fingerprint of the expected + server certificate. See :ref:`ssl-tutorial` for more information. :param auth: Optional. Either ``basic``, ``digest`` or ``guess``. The default is preemptive Basic auth, sending credentials even if server didn't request them. This saves from an additional @@ -478,12 +474,10 @@ leads to an error. :param url: URL to the ``.ics`` file. :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. See :ref:`ssl-tutorial` - for more information. - :param verify_fingerprint: Optional. SHA1 or MD5 fingerprint of the - expected server certificate. See :ref:`ssl-tutorial` for more - information. + :param verify: Optional. Local path to a self-signed SSL certificate. + See :ref:`ssl-tutorial` for more information. + :param verify_fingerprint: Optional. SHA256 fingerprint of the expected + server certificate. See :ref:`ssl-tutorial` for more information. :param auth: Optional. Either ``basic``, ``digest`` or ``guess``. The default is preemptive Basic auth, sending credentials even if server didn't request them. This saves from an additional roundtrip per diff --git a/docs/ssl-tutorial.rst b/docs/ssl-tutorial.rst index fe7342e..5fba013 100644 --- a/docs/ssl-tutorial.rst +++ b/docs/ssl-tutorial.rst @@ -15,21 +15,14 @@ To pin the certificate by 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" - #verify = false # Optional: Disable CA validation, useful for self-signed certs -SHA1-, SHA256- or MD5-Fingerprints can be used. They're detected by their -length. +SHA256-Fingerprints can be used. CA validation is disabled when pinning a +fingerprint. You can use the following command for obtaining a SHA-1 fingerprint:: echo -n | openssl s_client -connect unterwaditzer.net:443 | openssl x509 -noout -fingerprint -Note that ``verify_fingerprint`` doesn't suffice for vdirsyncer to work with -self-signed certificates (or certificates that are not in your trust store). You -most likely need to set ``verify = false`` as well. This disables verification -of the SSL certificate's expiration time and the existence of it in your trust -store, all that's verified now is the fingerprint. - However, please consider using `Let's Encrypt `_ such that you can forget about all of that. It is easier to deploy a free certificate from them than configuring all of your clients to accept the diff --git a/tests/storage/test_http.py b/tests/storage/test_http.py index 202bda2..0e0247f 100644 --- a/tests/storage/test_http.py +++ b/tests/storage/test_http.py @@ -126,5 +126,4 @@ def test_verify_false_disallowed(aio_connector): with pytest.raises(ValueError) as excinfo: HttpStorage(url="http://example.com", verify=False, connector=aio_connector) - assert "forbidden" in str(excinfo.value).lower() - assert "consider setting verify_fingerprint" in str(excinfo.value).lower() + assert "must be a path to a pem-file." in str(excinfo.value).lower() diff --git a/tests/system/utils/test_main.py b/tests/system/utils/test_main.py index d7c25fd..55ffa3a 100644 --- a/tests/system/utils/test_main.py +++ b/tests/system/utils/test_main.py @@ -35,12 +35,15 @@ async def test_request_ssl(): ) assert "certificate verify failed" in str(excinfo.value) - await http.request( - "GET", - "https://self-signed.badssl.com/", - verify=False, - session=session, - ) + # XXX FIXME + + with pytest.raises(Exception): + await http.request( + "GET", + "https://self-signed.badssl.com/", + verify=False, + session=session, + ) def fingerprint_of_cert(cert, hash=hashes.SHA256) -> str: @@ -62,10 +65,12 @@ async def test_request_ssl_leaf_fingerprint( httpserver.expect_request("/").respond_with_data("OK") url = f"https://127.0.0.1:{httpserver.port}/" - await http.request("GET", url, verify_fingerprint=fingerprint, session=aio_session) + ssl = http.prepare_verify(None, fingerprint) + await http.request("GET", url, ssl=ssl, session=aio_session) + ssl = http.prepare_verify(None, bogus) with pytest.raises(aiohttp.ServerFingerprintMismatch): - await http.request("GET", url, verify_fingerprint=bogus, session=aio_session) + await http.request("GET", url, ssl=ssl, session=aio_session) @pytest.mark.xfail(reason="Not implemented") diff --git a/vdirsyncer/http.py b/vdirsyncer/http.py index 4947291..b35035b 100644 --- a/vdirsyncer/http.py +++ b/vdirsyncer/http.py @@ -1,4 +1,5 @@ import logging +from ssl import create_default_context import aiohttp @@ -64,30 +65,23 @@ def prepare_auth(auth, username, password): def prepare_verify(verify, verify_fingerprint): - if isinstance(verify, (str, bytes)): - verify = expand_path(verify) - elif not isinstance(verify, bool): + if isinstance(verify, str): + return create_default_context(cafile=expand_path(verify)) + elif verify is not None: raise exceptions.UserError( - "Invalid value for verify ({}), " - "must be a path to a PEM-file or boolean.".format(verify) + f"Invalid value for verify ({verify}), must be a path to a PEM-file." ) if verify_fingerprint is not None: - if not isinstance(verify_fingerprint, (bytes, str)): + if not isinstance(verify_fingerprint, str): raise exceptions.UserError( "Invalid value for verify_fingerprint " - "({}), must be a string or null.".format(verify_fingerprint) + f"({verify_fingerprint}), must be a string." ) - elif not verify: - raise exceptions.UserError( - "Disabling all SSL validation is forbidden. Consider setting " - "verify_fingerprint if you have a broken or self-signed cert." - ) - return { - "verify": verify, - "verify_fingerprint": verify_fingerprint, - } + return aiohttp.Fingerprint(bytes.fromhex(verify_fingerprint.replace(":", ""))) + + return None def prepare_client_cert(cert): @@ -99,15 +93,18 @@ def prepare_client_cert(cert): async def request( - method, url, session, latin1_fallback=True, verify_fingerprint=None, **kwargs + method, + url, + session, + latin1_fallback=True, + **kwargs, ): - """ - Wrapper method for requests, to ease logging and mocking. Parameters should - be the same as for ``requests.request``, except: + """Wrapper method for requests, to ease logging and mocking. + + Parameters should be the same as for ``aiohttp.request``, as well as: :param session: A requests session object to use. - :param verify_fingerprint: Optional. SHA1 or MD5 fingerprint of the - expected server certificate. + :param verify_fingerprint: Optional. SHA256 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 @@ -116,13 +113,7 @@ async def request( https://github.com/kennethreitz/requests/issues/2042 """ - if verify_fingerprint is not None: - ssl = aiohttp.Fingerprint(bytes.fromhex(verify_fingerprint.replace(":", ""))) - kwargs.pop("verify", None) - elif kwargs.pop("verify", None) is False: - ssl = False - else: - ssl = None # TODO XXX: Check all possible values for this + # TODO: Support for client-side certifications. session.hooks = {"response": _fix_redirects} @@ -138,29 +129,29 @@ async def request( kwargs.pop("cert", None) # TODO XXX FIXME! - r = await session.request(method, url, ssl=ssl, **kwargs) + response = await session.request(method, url, **kwargs) # See https://github.com/kennethreitz/requests/issues/2042 - content_type = r.headers.get("Content-Type", "") + content_type = response.headers.get("Content-Type", "") if ( not latin1_fallback and "charset" not in content_type and content_type.startswith("text/") ): logger.debug("Removing latin1 fallback") - r.encoding = None + response.encoding = None - logger.debug(r.status) - logger.debug(r.headers) - logger.debug(r.content) + logger.debug(response.status) + logger.debug(response.headers) + logger.debug(response.content) - if r.status == 412: - raise exceptions.PreconditionFailed(r.reason) - if r.status in (404, 410): - raise exceptions.NotFoundError(r.reason) + if response.status == 412: + raise exceptions.PreconditionFailed(response.reason) + if response.status in (404, 410): + raise exceptions.NotFoundError(response.reason) - r.raise_for_status() - return r + response.raise_for_status() + return response def _fix_redirects(r, *args, **kwargs): diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index 363e0f9..727f685 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -375,7 +375,7 @@ class DAVSession: url, username="", password="", - verify=True, + verify=None, auth=None, useragent=USERAGENT, verify_fingerprint=None, @@ -389,7 +389,10 @@ class DAVSession: auth = prepare_auth(auth, username, password) if auth: self._settings["auth"] = auth - self._settings.update(prepare_verify(verify, verify_fingerprint)) + + ssl = prepare_verify(verify, verify_fingerprint) + if ssl: + self._settings["ssl"] = ssl self.useragent = useragent self.url = url.rstrip("/") + "/" diff --git a/vdirsyncer/storage/http.py b/vdirsyncer/storage/http.py index dcbc002..5b12098 100644 --- a/vdirsyncer/storage/http.py +++ b/vdirsyncer/storage/http.py @@ -27,7 +27,7 @@ class HttpStorage(Storage): url, username="", password="", - verify=True, + verify=None, auth=None, useragent=USERAGENT, verify_fingerprint=None, @@ -45,7 +45,10 @@ class HttpStorage(Storage): auth = prepare_auth(auth, username, password) if auth: self._settings["auth"] = auth - self._settings.update(prepare_verify(verify, verify_fingerprint)) + + ssl = prepare_verify(verify, verify_fingerprint) + if ssl: + self._settings["ssl"] = ssl self.username, self.password = username, password self.useragent = useragent