From 72addeca0933a23bffa49637a90ffadf26c4eb0e Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Thu, 9 Dec 2021 14:40:09 +0200 Subject: [PATCH 1/8] ssl password support --- redis/client.py | 2 ++ redis/connection.py | 32 +++++++++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/redis/client.py b/redis/client.py index c02bc3a4d5..db3f036b64 100755 --- a/redis/client.py +++ b/redis/client.py @@ -874,6 +874,7 @@ def __init__( ssl_cert_reqs="required", ssl_ca_certs=None, ssl_check_hostname=False, + ssl_password=None, max_connections=None, single_connection_client=False, health_check_interval=0, @@ -947,6 +948,7 @@ def __init__( "ssl_cert_reqs": ssl_cert_reqs, "ssl_ca_certs": ssl_ca_certs, "ssl_check_hostname": ssl_check_hostname, + "ssl_password": ssl_password, } ) connection_pool = ConnectionPool(**kwargs) diff --git a/redis/connection.py b/redis/connection.py index 2001c6447b..051dc85f8d 100755 --- a/redis/connection.py +++ b/redis/connection.py @@ -879,6 +879,11 @@ def pack_commands(self, commands): class SSLConnection(Connection): + """Manages SSL connections to and from the Redis server(s). + This class extends the Connection class, adding SSL functionality, and making + use of ssl.SSLContext (https://docs.python.org/3/library/ssl.html#ssl.SSLContext) + """ # noqa + def __init__( self, ssl_keyfile=None, @@ -886,8 +891,24 @@ def __init__( ssl_cert_reqs="required", ssl_ca_certs=None, ssl_check_hostname=False, + ssl_ca_path=None, + ssl_password=None, **kwargs, ): + """Constructor + + Args: + ssl_keyfile: Path to an ssl private key. Defaults to None. + ssl_certfile: Path to an ssl certificate. Defaults to None. + ssl_cert_reqs: The string value for the SSLContext.verify_mode (none, optional, required). Defaults to "required". + ssl_ca_certs: The path to a file of concatenated CA certificates in PEM format. Defaults to None. + ssl_check_hostname: If set, match the hostname during the SSL handshake. Defaults to False. + ssl_ca_path: The path to a directory containing several CA certificates in PEM format. Defaults to None. + ssl_password: Password for unlocking an encrypted private key. Defaults to None. + + Raises: + RedisError + """ # noqa if not ssl_available: raise RedisError("Python wasn't built with SSL support") @@ -910,7 +931,9 @@ def __init__( ssl_cert_reqs = CERT_REQS[ssl_cert_reqs] self.cert_reqs = ssl_cert_reqs self.ca_certs = ssl_ca_certs + self.ca_path = ssl_ca_path self.check_hostname = ssl_check_hostname + self.certificate_password = ssl_password def _connect(self): "Wrap the socket with SSL support" @@ -919,9 +942,12 @@ def _connect(self): 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) - if self.ca_certs: - context.load_verify_locations(self.ca_certs) + context.load_cert_chain(certfile=self.certfile, + keyfile=self.keyfile, + password=self.certificate_password) + if self.ca_certs is not None or self.ca_path is not None: + context.load_verify_locations(cafile=self.ca_certs, + capath=self.ca_path) return context.wrap_socket(sock, server_hostname=self.host) From 7365b267e2ac1ad6fd323c9bdb121a560b101337 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Thu, 9 Dec 2021 18:25:10 +0200 Subject: [PATCH 2/8] first ssl test - validates an invalid certificate --- .gitignore | 1 + dev_requirements.txt | 1 + docker/base/Dockerfile | 1 + docker/base/Dockerfile.cluster | 3 ++- docker/base/Dockerfile.sentinel | 1 + docker/base/Dockerfile.stunnel | 11 +++++++++ docker/stunnel/conf/redis.conf | 6 +++++ docker/stunnel/create_certs.sh | 43 +++++++++++++++++++++++++++++++++ tests/conftest.py | 16 ++++++++++++ tests/test_ssl.py | 17 +++++++++++++ tox.ini | 26 +++++++++++++------- 11 files changed, 116 insertions(+), 10 deletions(-) create mode 100644 docker/base/Dockerfile.stunnel create mode 100644 docker/stunnel/conf/redis.conf create mode 100755 docker/stunnel/create_certs.sh create mode 100644 tests/test_ssl.py diff --git a/.gitignore b/.gitignore index 08138d7c8c..96fbdd5646 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ coverage.xml .venv *.xml .coverage* +docker/stunnel/keys diff --git a/dev_requirements.txt b/dev_requirements.txt index 2a4f37762f..1d33b9875b 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -6,6 +6,7 @@ pytest==6.2.5 pytest-timeout==2.0.1 tox==3.24.4 tox-docker==3.1.0 +tox-run-before==0.1 invoke==1.6.0 pytest-cov>=3.0.0 vulture>=2.3.0 diff --git a/docker/base/Dockerfile b/docker/base/Dockerfile index 60be37483b..c76d15db36 100644 --- a/docker/base/Dockerfile +++ b/docker/base/Dockerfile @@ -1,3 +1,4 @@ +# produces redisfab/redis-py:6.2.6 FROM redis:6.2.6-buster CMD ["redis-server", "/redis.conf"] diff --git a/docker/base/Dockerfile.cluster b/docker/base/Dockerfile.cluster index 70e8013631..70df5babf0 100644 --- a/docker/base/Dockerfile.cluster +++ b/docker/base/Dockerfile.cluster @@ -1,3 +1,4 @@ +# produces redisfab/redis-py-cluster:6.2.6 FROM redis:6.2.6-buster COPY create_cluster.sh /create_cluster.sh @@ -5,4 +6,4 @@ RUN chmod +x /create_cluster.sh EXPOSE 16379 16380 16381 16382 16383 16384 -CMD [ "/create_cluster.sh"] \ No newline at end of file +CMD [ "/create_cluster.sh"] diff --git a/docker/base/Dockerfile.sentinel b/docker/base/Dockerfile.sentinel index 93c16a71ab..ef659e3004 100644 --- a/docker/base/Dockerfile.sentinel +++ b/docker/base/Dockerfile.sentinel @@ -1,3 +1,4 @@ +# produces redisfab/redis-py-sentinel:6.2.6 FROM redis:6.2.6-buster CMD ["redis-sentinel", "/sentinel.conf"] diff --git a/docker/base/Dockerfile.stunnel b/docker/base/Dockerfile.stunnel new file mode 100644 index 0000000000..bf4510907c --- /dev/null +++ b/docker/base/Dockerfile.stunnel @@ -0,0 +1,11 @@ +# produces redisfab/stunnel:latest +FROM ubuntu:18.04 + +RUN apt-get update -qq --fix-missing +RUN apt-get upgrade -qqy +RUN apt install -qqy stunnel +RUN mkdir -p /etc/stunnel/conf.d +RUN echo "foreground = yes\ninclude = /etc/stunnel/conf.d" > /etc/stunnel/stunnel.conf +RUN chown -R root:root /etc/stunnel/ + +CMD ["/usr/bin/stunnel"] diff --git a/docker/stunnel/conf/redis.conf b/docker/stunnel/conf/redis.conf new file mode 100644 index 0000000000..84f6d40133 --- /dev/null +++ b/docker/stunnel/conf/redis.conf @@ -0,0 +1,6 @@ +[redis] +accept = 6666 +connect = master:6379 +cert = /etc/stunnel/keys/server-cert.pem +key = /etc/stunnel/keys/server-key.pem +verify = 0 diff --git a/docker/stunnel/create_certs.sh b/docker/stunnel/create_certs.sh new file mode 100755 index 0000000000..b94c56013e --- /dev/null +++ b/docker/stunnel/create_certs.sh @@ -0,0 +1,43 @@ +#!/bin/bash + +DESTDIR=`dirname "$0"`/keys +if [ ! -d ${DESTDIR} ]; then + mkdir `dirname "$0"`/keys +fi +cd ${DESTDIR} + +SSL_SUBJECT="/C=CA/ST=Winnipeg/L=Manitoba/O=Some Corp/OU=IT Department/CN=example.com" +which openssl &>/dev/null +if [ $? -ne 0 ]; then + echo "No openssl binary present, exiting." + exit 1 +fi + +openssl genrsa -out ca-key.pem 2048 + +openssl req -new -x509 -nodes -days 365000 \ + -key ca-key.pem \ + -out ca-cert.pem \ + -subj "${SSL_SUBJECT}" + +openssl req -newkey rsa:2048 -nodes -days 365000 \ + -keyout server-key.pem \ + -out server-req.pem \ + -subj "${SSL_SUBJECT}" + +openssl x509 -req -days 365000 -set_serial 01 \ + -in server-req.pem \ + -out server-cert.pem \ + -CA ca-cert.pem \ + -CAkey ca-key.pem + +openssl req -newkey rsa:2048 -nodes -days 365000 \ + -keyout client-key.pem \ + -out client-req.pem \ + -subj "${SSL_SUBJECT}" + +openssl x509 -req -days 365000 -set_serial 01 \ + -in client-req.pem \ + -out client-cert.pem \ + -CA ca-cert.pem \ + -CAkey ca-key.pem diff --git a/tests/conftest.py b/tests/conftest.py index 4b5f6cbd5c..65e5369f92 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,6 +14,7 @@ REDIS_INFO = {} default_redis_url = "redis://localhost:6379/9" +default_redis_ssl_url = "rediss://localhost:6666" default_redismod_url = "redis://localhost:36379" default_cluster_nodes = 6 @@ -35,6 +36,13 @@ def pytest_addoption(parser): " with loaded modules," " defaults to `%(default)s`", ) + + parser.addoption( + "--redis-ssl-url", + default=default_redis_ssl_url, + action="store", + help="Redis SSL connection string," " defaults to `%(default)s`", + ) parser.addoption( "--redis-cluster-nodes", @@ -227,6 +235,14 @@ def modclient(request, **kwargs): redis.Redis, request, from_url=rmurl, decode_responses=True, **kwargs ) as client: yield client + +# @pytest.fixture() +# def sslclient(request, **kwargs): +# ssl_url = request.config.getoption("--redis-ssl-url") +# with _get_client( +# redis.Redis, request, from_url=ssl_url, decode_responses=True, **kwargs +# ) as client: +# yield client @pytest.fixture() diff --git a/tests/test_ssl.py b/tests/test_ssl.py new file mode 100644 index 0000000000..4064ae3c8f --- /dev/null +++ b/tests/test_ssl.py @@ -0,0 +1,17 @@ +import pytest +import redis +from redis.exceptions import ConnectionError + + +class TestSSL: + """Tests for SSL connections""" + + def test_ssl_with_invalid_cert(self, request): + ssl_url = request.config.option.redis_ssl_url + sslclient = redis.from_url(ssl_url) + with pytest.raises(ConnectionError) as e: + sslclient.ping() + assert 'SSL: CERTIFICATE_VERIFY_FAILED' in str(e) + + def test_ssl_connection_creation(self, sslclient): + assert sslclient.ping() \ No newline at end of file diff --git a/tox.ini b/tox.ini index 32d1680a2a..f840f83be6 100644 --- a/tox.ini +++ b/tox.ini @@ -31,6 +31,7 @@ healtcheck_cmd = python -c "import socket;print(True) if 0 == socket.socket(sock volumes = bind:rw:{toxinidir}/docker/replica/redis.conf:/redis.conf + [docker:sentinel_1] name = sentinel_1 image = redisfab/redis-py-sentinel:6.2.6-buster @@ -91,6 +92,18 @@ healtcheck_cmd = python -c "import socket;print(True) if all([0 == socket.socket volumes = bind:rw:{toxinidir}/docker/cluster/redis.conf:/redis.conf +[docker:stunnel] +name = stunnel +image = redisfab/stunnel:latest +healtcheck_cmd = python -c "import socket;print(True) if 0 == socket.socket(socket.AF_INET, socket.SOCK_STREAM).connect_ex(('127.0.0.1',6666)) else False" +links = + master:master +ports = + 6666:6666/tcp +volumes = + bind:ro:{toxinidir}/docker/stunnel/conf:/etc/stunnel/conf.d + bind:ro:{toxinidir}/docker/stunnel/keys:/etc/stunnel/keys + [isort] profile = black multi_line_output = 3 @@ -107,10 +120,13 @@ docker = sentinel_3 redis_cluster redismod + stunnel extras = hiredis: hiredis setenv = CLUSTER_URL = "redis://localhost:16379/0" +run_before = + {toxinidir}/docker/stunnel/create_certs.sh commands = standalone: pytest --cov=./ --cov-report=xml:coverage_redis.xml -W always -m 'not onlycluster' {posargs} cluster: pytest --cov=./ --cov-report=xml:coverage_cluster.xml -W always -m 'not onlynoncluster and not redismod' --redis-url={env:CLUSTER_URL:} {posargs} @@ -119,15 +135,7 @@ commands = skipsdist = true skip_install = true deps = -r {toxinidir}/dev_requirements.txt -docker = - master - replica - sentinel_1 - sentinel_2 - sentinel_3 - redis_cluster - redismod - lots-of-pythons +docker = {[testenv]docker} commands = /usr/bin/echo [testenv:linters] From 0cfffa08300c0f42e8b5f83e1c8522678a53669e Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Sat, 11 Dec 2021 22:42:47 +0200 Subject: [PATCH 3/8] linter fixes --- redis/connection.py | 11 ++++++----- tests/conftest.py | 5 +++-- tests/test_ssl.py | 12 +++++++----- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/redis/connection.py b/redis/connection.py index 051dc85f8d..250d76487e 100755 --- a/redis/connection.py +++ b/redis/connection.py @@ -942,12 +942,13 @@ def _connect(self): 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, - password=self.certificate_password) + context.load_cert_chain( + certfile=self.certfile, + keyfile=self.keyfile, + password=self.certificate_password, + ) if self.ca_certs is not None or self.ca_path is not None: - context.load_verify_locations(cafile=self.ca_certs, - capath=self.ca_path) + context.load_verify_locations(cafile=self.ca_certs, capath=self.ca_path) return context.wrap_socket(sock, server_hostname=self.host) diff --git a/tests/conftest.py b/tests/conftest.py index 65e5369f92..9c848832b6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -36,7 +36,7 @@ def pytest_addoption(parser): " with loaded modules," " defaults to `%(default)s`", ) - + parser.addoption( "--redis-ssl-url", default=default_redis_ssl_url, @@ -235,7 +235,8 @@ def modclient(request, **kwargs): redis.Redis, request, from_url=rmurl, decode_responses=True, **kwargs ) as client: yield client - + + # @pytest.fixture() # def sslclient(request, **kwargs): # ssl_url = request.config.getoption("--redis-ssl-url") diff --git a/tests/test_ssl.py b/tests/test_ssl.py index 4064ae3c8f..90c2f79b1a 100644 --- a/tests/test_ssl.py +++ b/tests/test_ssl.py @@ -1,17 +1,19 @@ import pytest + import redis from redis.exceptions import ConnectionError class TestSSL: """Tests for SSL connections""" - + def test_ssl_with_invalid_cert(self, request): ssl_url = request.config.option.redis_ssl_url sslclient = redis.from_url(ssl_url) with pytest.raises(ConnectionError) as e: sslclient.ping() - assert 'SSL: CERTIFICATE_VERIFY_FAILED' in str(e) - - def test_ssl_connection_creation(self, sslclient): - assert sslclient.ping() \ No newline at end of file + assert "SSL: CERTIFICATE_VERIFY_FAILED" in str(e) + + +# def test_ssl_connection_creation(self, sslclient): +# assert sslclient.ping() From d127ce526b6c8e59a93cdd744ff0b42fef487022 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Sun, 12 Dec 2021 15:14:26 +0200 Subject: [PATCH 4/8] adding two ssl tests --- .github/workflows/integration.yaml | 1 + docker/stunnel/create_certs.sh | 21 +++++++++++--------- redis/connection.py | 11 ++++++----- tasks.py | 12 ++++++++++++ tests/conftest.py | 21 ++++++++++---------- tests/test_ssl.py | 31 ++++++++++++++++++++++++------ tox.ini | 4 ++-- 7 files changed, 68 insertions(+), 33 deletions(-) diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index 10ab3ed03e..9d0c56d405 100644 --- a/.github/workflows/integration.yaml +++ b/.github/workflows/integration.yaml @@ -48,6 +48,7 @@ jobs: - name: run tests run: | pip install -r dev_requirements.txt + bash docker/stunnel/create_certs.sh tox -e ${{matrix.test-type}}-${{matrix.connection-type}} - name: Upload codecov coverage uses: codecov/codecov-action@v2 diff --git a/docker/stunnel/create_certs.sh b/docker/stunnel/create_certs.sh index b94c56013e..f3bcea6f5d 100755 --- a/docker/stunnel/create_certs.sh +++ b/docker/stunnel/create_certs.sh @@ -1,9 +1,9 @@ #!/bin/bash +set -e + DESTDIR=`dirname "$0"`/keys -if [ ! -d ${DESTDIR} ]; then - mkdir `dirname "$0"`/keys -fi +test -d ${DESTDIR} || mkdir ${DESTDIR} cd ${DESTDIR} SSL_SUBJECT="/C=CA/ST=Winnipeg/L=Manitoba/O=Some Corp/OU=IT Department/CN=example.com" @@ -13,31 +13,34 @@ if [ $? -ne 0 ]; then exit 1 fi -openssl genrsa -out ca-key.pem 2048 +openssl genrsa -out ca-key.pem 2048 &>/dev/null openssl req -new -x509 -nodes -days 365000 \ -key ca-key.pem \ -out ca-cert.pem \ - -subj "${SSL_SUBJECT}" + -subj "${SSL_SUBJECT}" &>/dev/null openssl req -newkey rsa:2048 -nodes -days 365000 \ -keyout server-key.pem \ -out server-req.pem \ - -subj "${SSL_SUBJECT}" + -subj "${SSL_SUBJECT}" &>/dev/null openssl x509 -req -days 365000 -set_serial 01 \ -in server-req.pem \ -out server-cert.pem \ -CA ca-cert.pem \ - -CAkey ca-key.pem + -CAkey ca-key.pem &>/dev/null openssl req -newkey rsa:2048 -nodes -days 365000 \ -keyout client-key.pem \ -out client-req.pem \ - -subj "${SSL_SUBJECT}" + -subj "${SSL_SUBJECT}" &>/dev/null openssl x509 -req -days 365000 -set_serial 01 \ -in client-req.pem \ -out client-cert.pem \ -CA ca-cert.pem \ - -CAkey ca-key.pem + -CAkey ca-key.pem &>/dev/null + +echo "Keys generated in ${DESTDIR}:" +ls diff --git a/redis/connection.py b/redis/connection.py index 051dc85f8d..250d76487e 100755 --- a/redis/connection.py +++ b/redis/connection.py @@ -942,12 +942,13 @@ def _connect(self): 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, - password=self.certificate_password) + context.load_cert_chain( + certfile=self.certfile, + keyfile=self.keyfile, + password=self.certificate_password, + ) if self.ca_certs is not None or self.ca_path is not None: - context.load_verify_locations(cafile=self.ca_certs, - capath=self.ca_path) + context.load_verify_locations(cafile=self.ca_certs, capath=self.ca_path) return context.wrap_socket(sock, server_hostname=self.host) diff --git a/tasks.py b/tasks.py index 880e70dcf0..8af101c993 100644 --- a/tasks.py +++ b/tasks.py @@ -3,6 +3,11 @@ from invoke import run, task + +def _generate_keys(): + run("bash docker/stunnel/create_certs.sh") + + with open("tox.ini") as fp: lines = fp.read().split("\n") dockers = [line.split("=")[1].strip() for line in lines if line.find("name") != -1] @@ -14,6 +19,7 @@ def devenv(c): specified in the tox.ini file. """ clean(c) + _generate_keys() cmd = "tox -e devenv" for d in dockers: cmd += f" --docker-dont-stop={d}" @@ -37,6 +43,7 @@ def all_tests(c): """Run all linters, and tests in redis-py. This assumes you have all the python versions specified in the tox.ini file. """ + _generate_keys() linters(c) tests(c) @@ -47,6 +54,7 @@ def tests(c): with and without hiredis. """ print("Starting Redis tests") + _generate_keys() run("tox -e '{standalone,cluster}'-'{plain,hiredis}'") @@ -55,6 +63,7 @@ def standalone_tests(c): """Run all Redis tests against the current python, with and without hiredis.""" print("Starting Redis tests") + _generate_keys() run("tox -e standalone-'{plain,hiredis}'") @@ -63,6 +72,7 @@ def cluster_tests(c): """Run all Redis Cluster tests against the current python, with and without hiredis.""" print("Starting RedisCluster tests") + _generate_keys() run("tox -e cluster-'{plain,hiredis}'") @@ -74,6 +84,8 @@ def clean(c): if os.path.isdir("dist"): shutil.rmtree("dist") run(f"docker rm -f {' '.join(dockers)}") + if os.path.isdir("docker/stunnel/keys"): + shutil.rmtree("docker/stunnel/keys") @task diff --git a/tests/conftest.py b/tests/conftest.py index 65e5369f92..0149166d65 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,9 +14,10 @@ REDIS_INFO = {} default_redis_url = "redis://localhost:6379/9" -default_redis_ssl_url = "rediss://localhost:6666" - default_redismod_url = "redis://localhost:36379" + +# default ssl client ignores verification for the purpose of testing +default_redis_ssl_url = "rediss://localhost:6666" default_cluster_nodes = 6 @@ -36,7 +37,7 @@ def pytest_addoption(parser): " with loaded modules," " defaults to `%(default)s`", ) - + parser.addoption( "--redis-ssl-url", default=default_redis_ssl_url, @@ -235,14 +236,6 @@ def modclient(request, **kwargs): redis.Redis, request, from_url=rmurl, decode_responses=True, **kwargs ) as client: yield client - -# @pytest.fixture() -# def sslclient(request, **kwargs): -# ssl_url = request.config.getoption("--redis-ssl-url") -# with _get_client( -# redis.Redis, request, from_url=ssl_url, decode_responses=True, **kwargs -# ) as client: -# yield client @pytest.fixture() @@ -264,6 +257,12 @@ def r2(request): yield client +@pytest.fixture() +def sslclient(request): + with _get_client(redis.Redis, request, ssl=True) as client: + yield client + + def _gen_cluster_mock_resp(r, response): connection = Mock() connection.retry = Retry(NoBackoff(), 0) diff --git a/tests/test_ssl.py b/tests/test_ssl.py index 4064ae3c8f..453efe6f33 100644 --- a/tests/test_ssl.py +++ b/tests/test_ssl.py @@ -1,17 +1,36 @@ +from urllib.parse import urlparse + import pytest + import redis from redis.exceptions import ConnectionError class TestSSL: - """Tests for SSL connections""" - + """Tests for SSL connections + + This relies on the --redis-ssl-url purely for rebuilding the client + and connecting to the appropriate port. + """ + def test_ssl_with_invalid_cert(self, request): ssl_url = request.config.option.redis_ssl_url sslclient = redis.from_url(ssl_url) with pytest.raises(ConnectionError) as e: sslclient.ping() - assert 'SSL: CERTIFICATE_VERIFY_FAILED' in str(e) - - def test_ssl_connection_creation(self, sslclient): - assert sslclient.ping() \ No newline at end of file + assert "SSL: CERTIFICATE_VERIFY_FAILED" in str(e) + + def test_ssl_connection(self, request): + 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") + assert r.ping() + + def test_ssl_connection_without_ssl(self, request): + ssl_url = request.config.option.redis_ssl_url + p = urlparse(ssl_url)[1].split(":") + r = redis.Redis(host=p[0], port=p[1], ssl=False) + + with pytest.raises(ConnectionError) as e: + r.ping() + assert "Connection closed by server" in str(e) diff --git a/tox.ini b/tox.ini index f840f83be6..df6712a18c 100644 --- a/tox.ini +++ b/tox.ini @@ -125,8 +125,7 @@ extras = hiredis: hiredis setenv = CLUSTER_URL = "redis://localhost:16379/0" -run_before = - {toxinidir}/docker/stunnel/create_certs.sh +run_before = {toxinidir}/docker/stunnel/create_certs.sh commands = standalone: pytest --cov=./ --cov-report=xml:coverage_redis.xml -W always -m 'not onlycluster' {posargs} cluster: pytest --cov=./ --cov-report=xml:coverage_cluster.xml -W always -m 'not onlynoncluster and not redismod' --redis-url={env:CLUSTER_URL:} {posargs} @@ -137,6 +136,7 @@ skip_install = true deps = -r {toxinidir}/dev_requirements.txt docker = {[testenv]docker} commands = /usr/bin/echo +run_before = {[testenv]run_before} [testenv:linters] deps_files = dev_requirements.txt From bec40574be60b0535a802d2d6ed0cbb1da6445e9 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Sun, 12 Dec 2021 16:31:26 +0200 Subject: [PATCH 5/8] more ssl tests --- CONTRIBUTING.md | 3 ++- redis/client.py | 1 + redis/connection.py | 2 +- tests/test_ssl.py | 18 ++++++++++++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fe37ff9abe..04f989a308 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -58,6 +58,7 @@ can execute docker and its various commands. - A Redis replica node - Three sentinel Redis nodes - A multi-python docker, with your source code mounted in /data +- An stunnel docker, fronting the master Redis node The replica node, is a replica of the master node, using the [leader-follower replication](https://redis.io/topics/replication) @@ -73,7 +74,7 @@ tests as well. With the 'tests' and 'all-tests' targets, all Redis and RedisCluster tests will be run. It is possible to run only Redis client tests (with cluster mode disabled) by -using `invoke redis-tests`; similarly, RedisCluster tests can be run by using +using `invoke standalone-tests`; similarly, RedisCluster tests can be run by using `invoke cluster-tests`. Each run of tox starts and stops the various dockers required. Sometimes diff --git a/redis/client.py b/redis/client.py index db3f036b64..3ae133c300 100755 --- a/redis/client.py +++ b/redis/client.py @@ -873,6 +873,7 @@ def __init__( ssl_certfile=None, ssl_cert_reqs="required", ssl_ca_certs=None, + ssl_ca_path=None, ssl_check_hostname=False, ssl_password=None, max_connections=None, diff --git a/redis/connection.py b/redis/connection.py index 250d76487e..357660cc21 100755 --- a/redis/connection.py +++ b/redis/connection.py @@ -941,7 +941,7 @@ def _connect(self): context = ssl.create_default_context() context.check_hostname = self.check_hostname context.verify_mode = self.cert_reqs - if self.certfile and self.keyfile: + if self.certfile or self.keyfile: context.load_cert_chain( certfile=self.certfile, keyfile=self.keyfile, diff --git a/tests/test_ssl.py b/tests/test_ssl.py index 453efe6f33..3e82645369 100644 --- a/tests/test_ssl.py +++ b/tests/test_ssl.py @@ -1,3 +1,4 @@ +import os from urllib.parse import urlparse import pytest @@ -13,6 +14,9 @@ class TestSSL: and connecting to the appropriate port. """ + ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) + CERT_DIR = os.path.join(ROOT, "docker", "stunnel", "keys") + def test_ssl_with_invalid_cert(self, request): ssl_url = request.config.option.redis_ssl_url sslclient = redis.from_url(ssl_url) @@ -34,3 +38,17 @@ def test_ssl_connection_without_ssl(self, request): with pytest.raises(ConnectionError) as e: r.ping() assert "Connection closed by server" in str(e) + + def test_validating_self_signed_certificate(self, request): + 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_certfile=os.path.join(self.CERT_DIR, "server-cert.pem"), + ssl_keyfile=os.path.join(self.CERT_DIR, "server-key.pem"), + ssl_cert_reqs="required", + ssl_ca_certs=os.path.join(self.CERT_DIR, "server-cert.pem"), + ) + assert r.ping() From 696411715bcb0761f2db1c1b3f3f1f62f9a68328 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Sun, 12 Dec 2021 16:34:54 +0200 Subject: [PATCH 6/8] added certgen to linters --- tasks.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tasks.py b/tasks.py index 8af101c993..98ed483fb1 100644 --- a/tasks.py +++ b/tasks.py @@ -35,6 +35,7 @@ def build_docs(c): @task def linters(c): """Run code linters""" + _generate_keys() run("tox -e linters") From 59c44cc0722b348e734de5a7e28eda9b68b1b2db Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 15 Dec 2021 11:58:14 +0200 Subject: [PATCH 7/8] removing ssl tests from the package run --- .github/workflows/install_and_test.sh | 2 +- tests/test_ssl.py | 1 + tox.ini | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/install_and_test.sh b/.github/workflows/install_and_test.sh index 7a8cd672fd..33a1edb1e7 100755 --- a/.github/workflows/install_and_test.sh +++ b/.github/workflows/install_and_test.sh @@ -42,4 +42,4 @@ pip install ${PKG} pytest -m 'not onlycluster' # RedisCluster tests CLUSTER_URL="redis://localhost:16379/0" -pytest -m 'not onlynoncluster and not redismod' --redis-url=${CLUSTER_URL} +pytest -m 'not onlynoncluster and not redismod and not ssl' --redis-url=${CLUSTER_URL} diff --git a/tests/test_ssl.py b/tests/test_ssl.py index 3e82645369..811842d84c 100644 --- a/tests/test_ssl.py +++ b/tests/test_ssl.py @@ -7,6 +7,7 @@ from redis.exceptions import ConnectionError +@pytest.mark.ssl class TestSSL: """Tests for SSL connections diff --git a/tox.ini b/tox.ini index df6712a18c..b574d17b57 100644 --- a/tox.ini +++ b/tox.ini @@ -5,6 +5,7 @@ markers = pipeline: pipeline tests onlycluster: marks tests to be run only with cluster mode redis onlynoncluster: marks tests to be run only with standalone redis + ssl: marker for only the ssl tests [tox] minversion = 3.2.0 @@ -98,7 +99,7 @@ image = redisfab/stunnel:latest healtcheck_cmd = python -c "import socket;print(True) if 0 == socket.socket(socket.AF_INET, socket.SOCK_STREAM).connect_ex(('127.0.0.1',6666)) else False" links = master:master -ports = +ports = 6666:6666/tcp volumes = bind:ro:{toxinidir}/docker/stunnel/conf:/etc/stunnel/conf.d From 1cff383bd4b42e578602dd3aab0e5523f6c5acd2 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 15 Dec 2021 17:39:35 +0200 Subject: [PATCH 8/8] fixing packaging test case --- tests/test_ssl.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test_ssl.py b/tests/test_ssl.py index 811842d84c..70f9e58b76 100644 --- a/tests/test_ssl.py +++ b/tests/test_ssl.py @@ -15,8 +15,14 @@ class TestSSL: and connecting to the appropriate port. """ - ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) - CERT_DIR = os.path.join(ROOT, "docker", "stunnel", "keys") + ROOT = os.path.join(os.path.dirname(__file__), "..") + CERT_DIR = os.path.abspath(os.path.join(ROOT, "docker", "stunnel", "keys")) + if not os.path.isdir(CERT_DIR): # github actions package validation case + CERT_DIR = os.path.abspath( + os.path.join(ROOT, "..", "docker", "stunnel", "keys") + ) + if not os.path.isdir(CERT_DIR): + raise IOError(f"No SSL certificates found. They should be in {CERT_DIR}") def test_ssl_with_invalid_cert(self, request): ssl_url = request.config.option.redis_ssl_url