From 803510b8f8e0f03801a96fb56992900e26aff829 Mon Sep 17 00:00:00 2001 From: Victor K Date: Wed, 4 Oct 2017 22:06:59 +0300 Subject: [PATCH] ssl.SSLError explicit support (#2297) * Initial ssl.SSLError support * Explicit exc re raise * Added tests for os_error property * Added basic tests for os_error re raise * Added ClientConnectorSSLError to __all__ * Added tests for ClientConnectorSSLError * Fix typo in test_tcp_connector_uses_provided_local_addr * Added myself to CONTRIBUTORS.txt * Added changes. * Fix test_tcp_connector_uses_provided_local_addr * Fix Python versions capability. * Added docs for aiohttp.ClientConnectorSSLError * Refactor connectio_key usage. * Added aiohttp.ClientConnectorCertificateError * Added base ClientSSLError * Happy isort --- CONTRIBUTORS.txt | 1 + aiohttp/client_exceptions.py | 67 +++++++++++++++++++- aiohttp/client_reqrep.py | 8 +++ aiohttp/connector.py | 28 ++++----- changes/2294.feature | 1 + docs/client.rst | 31 +++++++++- docs/client_reference.rst | 22 ++++++- tests/test_connector.py | 115 ++++++++++++++++++++++++++++------- 8 files changed, 234 insertions(+), 39 deletions(-) create mode 100644 changes/2294.feature diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index edafc7fe7f4..ac891b789f0 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -178,6 +178,7 @@ Vaibhav Sagar Vamsi Krishna Avula Vasiliy Faronov Vasyl Baran +Victor Kovtun Vikas Kawadia Vitalik Verhovodov Vitaly Haritonsky diff --git a/aiohttp/client_exceptions.py b/aiohttp/client_exceptions.py index 95fc1d8eb05..ef6abcad42e 100644 --- a/aiohttp/client_exceptions.py +++ b/aiohttp/client_exceptions.py @@ -3,12 +3,21 @@ import asyncio +try: + import ssl +except ImportError: # pragma: no cover + ssl = None + + __all__ = ( 'ClientError', 'ClientConnectionError', 'ClientOSError', 'ClientConnectorError', 'ClientProxyConnectionError', + 'ClientSSLError', + 'ClientConnectorSSLError', 'ClientConnectorCertificateError', + 'ServerConnectionError', 'ServerTimeoutError', 'ServerDisconnectedError', 'ServerFingerprintMismatch', @@ -72,8 +81,13 @@ class ClientConnectorError(ClientOSError): """ def __init__(self, connection_key, os_error): self._conn_key = connection_key + self._os_error = os_error super().__init__(os_error.errno, os_error.strerror) + @property + def os_error(self): + return self._os_error + @property def host(self): return self._conn_key.host @@ -88,7 +102,7 @@ def ssl(self): def __str__(self): return ('Cannot connect to host {0.host}:{0.port} ssl:{0.ssl} [{1}]' - .format(self._conn_key, self.strerror)) + .format(self, self.strerror)) class ClientProxyConnectionError(ClientConnectorError): @@ -150,3 +164,54 @@ def url(self): def __repr__(self): return '<{} {}>'.format(self.__class__.__name__, self.url) + + +class ClientSSLError(ClientConnectorError): + """Base error for ssl.*Errors.""" + + +if ssl is not None: + certificate_errors = (ssl.CertificateError,) + certificate_errors_bases = (ClientSSLError, ssl.CertificateError,) + + ssl_errors = (ssl.SSLError,) + ssl_error_bases = (ClientConnectorError, ssl.SSLError) +else: # pragma: no cover + certificate_errors = tuple() + certificate_errors_bases = (ClientSSLError, ValueError,) + + ssl_errors = tuple() + ssl_error_bases = (ClientConnectorError,) + + +class ClientConnectorSSLError(*ssl_error_bases): + """Response ssl error.""" + + +class ClientConnectorCertificateError(*certificate_errors_bases): + """Response certificate error.""" + + def __init__(self, connection_key, certificate_error): + self._conn_key = connection_key + self._certificate_error = certificate_error + + @property + def certificate_error(self): + return self._certificate_error + + @property + def host(self): + return self._conn_key.host + + @property + def port(self): + return self._conn_key.port + + @property + def ssl(self): + return self._conn_key.ssl + + def __str__(self): + return ('Cannot connect to host {0.host}:{0.port} ssl:{0.ssl} ' + '[{0.certificate_error.__class__.__name__}: ' + '{0.certificate_error.args}]'.format(self)) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 0326c6f6c74..94ed0c6d667 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -6,6 +6,7 @@ import sys import traceback import warnings +from collections import namedtuple from hashlib import md5, sha1, sha256 from http.cookies import CookieError, Morsel @@ -46,6 +47,9 @@ _SSL_OP_NO_COMPRESSION = getattr(ssl, "OP_NO_COMPRESSION", 0) +ConnectionKey = namedtuple('ConnectionKey', ['host', 'port', 'ssl']) + + class ClientRequest: GET_METHODS = {hdrs.METH_GET, hdrs.METH_HEAD, hdrs.METH_OPTIONS} @@ -128,6 +132,10 @@ def __init__(self, method, url, *, self.update_transfer_encoding() self.update_expect_continue(expect100) + @property + def connection_key(self): + return ConnectionKey(self.host, self.port, self.ssl) + @property def host(self): return self.url.host diff --git a/aiohttp/connector.py b/aiohttp/connector.py index b0fe3f43f20..9ea367bb425 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -3,17 +3,20 @@ import sys import traceback import warnings -from collections import defaultdict, namedtuple +from collections import defaultdict from hashlib import md5, sha1, sha256 from itertools import cycle, islice from time import monotonic from types import MappingProxyType from . import hdrs, helpers -from .client_exceptions import (ClientConnectionError, ClientConnectorError, +from .client_exceptions import (ClientConnectionError, + ClientConnectorCertificateError, + ClientConnectorError, ClientConnectorSSLError, ClientHttpProxyError, ClientProxyConnectionError, - ServerFingerprintMismatch) + ServerFingerprintMismatch, certificate_errors, + ssl_errors) from .client_proto import ResponseHandler from .client_reqrep import ClientRequest from .helpers import SimpleCookie, is_ip_address, noop, sentinel @@ -136,9 +139,6 @@ def close(self): pass -ConnectionKey = namedtuple('ConnectionKey', ['host', 'port', 'ssl']) - - class BaseConnector(object): """Base connector class. @@ -349,7 +349,7 @@ def closed(self): @asyncio.coroutine def connect(self, req): """Get from pool or create new connection.""" - key = ConnectionKey(req.host, req.port, req.ssl) + key = req.connection_key if self._limit: # total calc available connections @@ -390,8 +390,6 @@ def connect(self, req): if self._closed: proto.close() raise ClientConnectionError("Connector is closed.") - except OSError as exc: - raise ClientConnectorError(key, exc) from exc finally: if not self._closed: self._acquired.remove(placeholder) @@ -775,7 +773,6 @@ def _create_direct_connection(self, req): fingerprint, hashfunc = self._get_fingerprint_and_hashfunc(req) hosts = yield from self._resolve_host(req.url.raw_host, req.port) - exc = None for hinfo in hosts: try: @@ -807,10 +804,13 @@ def _create_direct_connection(self, req): raise ServerFingerprintMismatch( expected, got, host, port) return transp, proto - except OSError as e: - exc = e - else: - raise ClientConnectorError(req, exc) from exc + except certificate_errors as exc: + raise ClientConnectorCertificateError( + req.connection_key, exc) from exc + except ssl_errors as exc: + raise ClientConnectorSSLError(req.connection_key, exc) from exc + except OSError as exc: + raise ClientConnectorError(req.connection_key, exc) from exc @asyncio.coroutine def _create_proxy_connection(self, req): diff --git a/changes/2294.feature b/changes/2294.feature new file mode 100644 index 00000000000..c1cb49d2ece --- /dev/null +++ b/changes/2294.feature @@ -0,0 +1 @@ +Added `aiohttp.ClientConnectorSSLError` when connection fails due `ssl.SSLError` diff --git a/docs/client.rst b/docs/client.rst index 06642eec2e4..508f55d57ab 100644 --- a/docs/client.rst +++ b/docs/client.rst @@ -542,6 +542,35 @@ same thing as the previous example, but add another call to '/path/to/client/private/device.jey') r = await session.get('https://example.com', ssl_context=sslcontext) +There is explicit errors when ssl verification fails + +:class:`aiohttp.ClientConnectorSSLError`:: + + try: + await session.get('https://expired.badssl.com/') + except aiohttp.ClientConnectorSSLError as e: + assert isinstance(e, ssl.SSLError) + +:class:`aiohttp.ClientConnectorCertificateError`:: + + try: + await session.get('https://wrong.host.badssl.com/') + except aiohttp.ClientConnectorCertificateError as e: + assert isinstance(e, ssl.CertificateError) + +If you need to skip both ssl related errors + +:class:`aiohttp.ClientSSLError`:: + + try: + await session.get('https://expired.badssl.com/') + except aiohttp.ClientSSLError as e: + assert isinstance(e, ssl.SSLError) + + try: + await session.get('https://wrong.host.badssl.com/') + except aiohttp.ClientSSLError as e: + assert isinstance(e, ssl.CertificateError) You may also verify certificates via *SHA256* fingerprint:: @@ -808,7 +837,7 @@ For a ``ClientSession`` with SSL, the application must wait a short duration bef # Wait 250 ms for the underlying SSL connections to close loop.run_until_complete(asyncio.sleep(0.250)) loop.close() - + Note that the appropriate amount of time to wait will vary from application to application. All if this will eventually become obsolete when the asyncio internals are changed so that aiohttp itself can wait on the underlying connection to close. Please follow issue `#1925 `_ for the progress on this. diff --git a/docs/client_reference.rst b/docs/client_reference.rst index 55becacaf59..9b90f69c64c 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -1626,7 +1626,6 @@ Connection errors These exceptions related to low-level connection problems. - Derived from :exc:`ClientError` .. class:: ClientOSError @@ -1650,6 +1649,21 @@ Connection errors Derived from :exc:`ClientConnectonError` +.. class:: ClientSSLError + + Derived from :exc:`ClientConnectonError` + +.. class:: ClientConnectorSSLError + + Response ssl error. + + Derived from :exc:`ClientSSLError` and :exc:`ssl.SSLError` + +.. class:: ClientConnectorCertificateError + + Response certificate error. + + Derived from :exc:`ClientSSLError` and :exc:`ssl.CertificateError` .. class:: ServerDisconnectedError @@ -1692,6 +1706,12 @@ Hierarchy of exceptions * :exc:`ClientConnectorError` + * :exc:`ClientSSLError` + + * :exc:`ClientConnectorCertificateError` + + * :exc:`ClientConnectorSSLError` + * :exc:`ClientProxyConnectionError` * :exc:`ServerConnectionError` diff --git a/tests/test_connector.py b/tests/test_connector.py index 18b1d07c7f1..2cae8b8d768 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -341,6 +341,27 @@ def test_release_close(loop): assert proto.close.called +@asyncio.coroutine +def test_tcp_connector_certificate_error(loop): + req = ClientRequest('GET', URL('https://127.0.0.1:443'), loop=loop) + + @asyncio.coroutine + def certificate_error(*args, **kwargs): + raise ssl.CertificateError + + conn = aiohttp.TCPConnector(loop=loop) + conn._loop.create_connection = certificate_error + + with pytest.raises(aiohttp.ClientConnectorCertificateError) as ctx: + yield from conn.connect(req) + + assert isinstance(ctx.value, ssl.CertificateError) + assert isinstance(ctx.value.certificate_error, ssl.CertificateError) + assert isinstance(ctx.value, aiohttp.ClientSSLError) + assert str(ctx.value) == ('Cannot connect to host 127.0.0.1:443 ssl:True ' + '[CertificateError: ()]') + + @asyncio.coroutine def test_tcp_connector_resolve_host(loop): conn = aiohttp.TCPConnector(loop=loop, use_dns_cache=True) @@ -569,25 +590,6 @@ def test_connect(loop): connection.close() -@asyncio.coroutine -def test_connect_connection_error(loop): - conn = aiohttp.BaseConnector(loop=loop) - conn._create_connection = mock.Mock() - conn._create_connection.return_value = helpers.create_future(loop) - err = OSError(1, 'permission error') - conn._create_connection.return_value.set_exception(err) - - with pytest.raises(aiohttp.ClientConnectorError) as ctx: - req = mock.Mock() - yield from conn.connect(req) - assert 1 == ctx.value.errno - assert str(ctx.value).startswith('Cannot connect to') - assert str(ctx.value).endswith('[permission error]') - assert ctx.value.host == req.host - assert ctx.value.port == req.port - assert ctx.value.ssl == req.ssl - - @asyncio.coroutine def test_close_during_connect(loop): proto = mock.Mock() @@ -1206,15 +1208,16 @@ def tearDown(self): gc.collect() @asyncio.coroutine - def create_server(self, method, path, handler): + def create_server(self, method, path, handler, ssl_context=None): app = web.Application() app.router.add_route(method, path, handler) port = unused_port() self.handler = app.make_handler(loop=self.loop, tcp_keepalive=False) srv = yield from self.loop.create_server( - self.handler, '127.0.0.1', port) - url = "http://127.0.0.1:{}".format(port) + path + self.handler, '127.0.0.1', port, ssl=ssl_context) + scheme = 's' if ssl_context is not None else '' + url = "http{}://127.0.0.1:{}".format(scheme, port) + path self.addCleanup(srv.close) return app, srv, url @@ -1234,6 +1237,74 @@ def create_unix_server(self, method, path, handler): self.addCleanup(srv.close) return app, srv, url, sock_path + def test_tcp_connector_raise_connector_ssl_error(self): + @asyncio.coroutine + def handler(request): + return web.HTTPOk() + + here = os.path.join(os.path.dirname(__file__), '..', 'tests') + keyfile = os.path.join(here, 'sample.key') + certfile = os.path.join(here, 'sample.crt') + sslcontext = ssl.SSLContext(ssl.PROTOCOL_SSLv23) + sslcontext.load_cert_chain(certfile, keyfile) + + app, srv, url = self.loop.run_until_complete( + self.create_server('get', '/', handler, ssl_context=sslcontext) + ) + + port = unused_port() + conn = aiohttp.TCPConnector(loop=self.loop, + local_addr=('127.0.0.1', port)) + + session = aiohttp.ClientSession(connector=conn) + + with pytest.raises(aiohttp.ClientConnectorSSLError) as ctx: + self.loop.run_until_complete(session.request('get', url)) + + self.assertIsInstance(ctx.value.os_error, ssl.SSLError) + self.assertTrue(ctx.value, aiohttp.ClientSSLError) + + session.close() + conn.close() + + def test_tcp_connector_do_not_raise_connector_ssl_error(self): + @asyncio.coroutine + def handler(request): + return web.HTTPOk() + + here = os.path.join(os.path.dirname(__file__), '..', 'tests') + keyfile = os.path.join(here, 'sample.key') + certfile = os.path.join(here, 'sample.crt') + sslcontext = ssl.SSLContext(ssl.PROTOCOL_SSLv23) + sslcontext.load_cert_chain(certfile, keyfile) + + app, srv, url = self.loop.run_until_complete( + self.create_server('get', '/', handler, ssl_context=sslcontext) + ) + + port = unused_port() + conn = aiohttp.TCPConnector(loop=self.loop, + local_addr=('127.0.0.1', port)) + + session = aiohttp.ClientSession(connector=conn) + + r = self.loop.run_until_complete( + session.request('get', url, ssl_context=sslcontext)) + + r.release() + first_conn = next(iter(conn._conns.values()))[0][0] + + try: + _sslcontext = first_conn.transport._ssl_protocol._sslcontext + except AttributeError: + _sslcontext = first_conn.transport._sslcontext + + self.assertIs(_sslcontext, sslcontext) + r.close() + + session.close() + conn.close() + def test_tcp_connector_uses_provided_local_addr(self): @asyncio.coroutine def handler(request):