diff --git a/CHANGES b/CHANGES index 7ce774621d..c84d521d79 100644 --- a/CHANGES +++ b/CHANGES @@ -70,6 +70,7 @@ * Close Unix sockets if the connection attempt fails. This prevents `ResourceWarning`s. (#3314) * Close SSL sockets if the connection attempt fails, or if validations fail. (#3317) * Eliminate mutable default arguments in the `redis.commands.core.Script` class. (#3332) + * Fix SSL verification with `ssl_cert_reqs="none"` and `ssl_check_hostname=True` by automatically setting `check_hostname=False` when `verify_mode=ssl.CERT_NONE` (#3635) * 4.1.3 (Feb 8, 2022) * Fix flushdb and flushall (#1926) diff --git a/redis/asyncio/connection.py b/redis/asyncio/connection.py index 77131ab951..9a6dd463fb 100644 --- a/redis/asyncio/connection.py +++ b/redis/asyncio/connection.py @@ -868,7 +868,7 @@ def __init__( cert_reqs: Optional[Union[str, ssl.VerifyMode]] = None, ca_certs: Optional[str] = None, ca_data: Optional[str] = None, - check_hostname: bool = False, + check_hostname: bool = True, min_version: Optional[TLSVersion] = None, ciphers: Optional[str] = None, ): @@ -901,7 +901,10 @@ def __init__( def get(self) -> SSLContext: if not self.context: context = ssl.create_default_context() - context.check_hostname = self.check_hostname + if self.cert_reqs == ssl.CERT_NONE: + context.check_hostname = False + else: + context.check_hostname = self.check_hostname context.verify_mode = self.cert_reqs if self.certfile and self.keyfile: context.load_cert_chain(certfile=self.certfile, keyfile=self.keyfile) diff --git a/redis/cluster.py b/redis/cluster.py index 99e5c37f9d..17bb1f3c27 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -174,6 +174,7 @@ def parse_cluster_myshardid(resp, **options): "ssl_cert_reqs", "ssl_keyfile", "ssl_password", + "ssl_check_hostname", "unix_socket_path", "username", "cache", diff --git a/redis/connection.py b/redis/connection.py index dab45906d2..db58504d07 100644 --- a/redis/connection.py +++ b/redis/connection.py @@ -1115,7 +1115,10 @@ def _wrap_socket_with_ssl(self, sock): An SSL wrapped socket. """ context = ssl.create_default_context() - context.check_hostname = self.check_hostname + if self.cert_reqs == ssl.CERT_NONE: + context.check_hostname = False + else: + context.check_hostname = self.check_hostname context.verify_mode = self.cert_reqs if self.certfile or self.keyfile: context.load_cert_chain( diff --git a/tests/test_asyncio/test_ssl.py b/tests/test_asyncio/test_ssl.py new file mode 100644 index 0000000000..3502780b0c --- /dev/null +++ b/tests/test_asyncio/test_ssl.py @@ -0,0 +1,56 @@ +import asyncio +import ssl +import socket +from urllib.parse import urlparse +import pytest +import pytest_asyncio +import redis.asyncio as redis +from redis.exceptions import RedisError, ConnectionError +import ssl + +# Skip test or not based on cryptography installation +try: + import cryptography # noqa + skip_if_cryptography = pytest.mark.skipif(False, reason="") + skip_if_nocryptography = pytest.mark.skipif(False, reason="") +except ImportError: + skip_if_cryptography = pytest.mark.skipif(True, reason="cryptography not installed") + skip_if_nocryptography = pytest.mark.skipif(True, reason="cryptography not installed") + +@pytest.mark.ssl +class TestSSL: + """Tests for SSL connections in asyncio.""" + + @pytest_asyncio.fixture() + async def _get_client(self, request): + ssl_url = request.config.option.redis_ssl_url + p = urlparse(ssl_url)[1].split(":") + client = redis.Redis(host=p[0], port=p[1], ssl=True) + yield client + await client.aclose() + + async def test_ssl_with_invalid_cert(self, _get_client): + """Test SSL connection with invalid certificate.""" + pass + + async def test_cert_reqs_none_with_check_hostname(self, request): + """Test that when ssl_cert_reqs=none is used with ssl_check_hostname=True, + the connection is created successfully with check_hostname internally set to False""" + ssl_url = request.config.option.redis_ssl_url + p = urlparse(ssl_url)[1].split(":") + r = redis.Redis( + host=p[0], + port=p[1], + ssl=True, + ssl_cert_reqs="none", + ssl_check_hostname=True, # This should work now because we handle the incompatibility + ) + try: + # Connection should be successful + assert await r.ping() + # check_hostname should have been automatically set to False + assert r.connection_pool.connection_class == redis.SSLConnection + conn = r.connection_pool.make_connection() + assert conn.check_hostname is False + finally: + await r.aclose() \ No newline at end of file diff --git a/tests/test_ssl.py b/tests/test_ssl.py index 5aa33353a8..439b0beb8c 100644 --- a/tests/test_ssl.py +++ b/tests/test_ssl.py @@ -309,3 +309,25 @@ def test_mock_ocsp_staple(self, request): r.ping() assert "no ocsp response present" in str(e) r.close() + + def test_cert_reqs_none_with_check_hostname(self, request): + """Test that when ssl_cert_reqs=none is used with ssl_check_hostname=True, + the connection is created successfully with check_hostname internally set to False""" + ssl_url = request.config.option.redis_ssl_url + p = urlparse(ssl_url)[1].split(":") + r = redis.Redis( + host=p[0], + port=p[1], + ssl=True, + ssl_cert_reqs="none", + ssl_check_hostname=True, # This should work now because we handle the incompatibility + ) + try: + # Connection should be successful + assert r.ping() + # check_hostname should have been automatically set to False + assert r.connection_pool.connection_kwargs["connection_class"] == redis.SSLConnection + conn = r.connection_pool.make_connection() + assert conn.check_hostname is False + finally: + r.close()