From 18c6809b761bc6755349e1d7e08e74e857ec2c65 Mon Sep 17 00:00:00 2001 From: Chayim Date: Thu, 16 Dec 2021 09:36:56 +0200 Subject: Support for password-encrypted SSL private keys (#1782) Adding support for SSL private keys with a password. This PR also adds support for future SSL tests. --- .github/workflows/install_and_test.sh | 2 +- .github/workflows/integration.yaml | 1 + .gitignore | 1 + CONTRIBUTING.md | 3 +- 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 | 46 ++++++++++++++++++++++++++ redis/client.py | 3 ++ redis/connection.py | 35 +++++++++++++++++--- tasks.py | 13 ++++++++ tests/conftest.py | 17 +++++++++- tests/test_ssl.py | 61 +++++++++++++++++++++++++++++++++++ tox.ini | 27 ++++++++++------ 17 files changed, 215 insertions(+), 17 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/.github/workflows/install_and_test.sh b/.github/workflows/install_and_test.sh index 7a8cd67..33a1edb 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/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index bd0fb2d..e81cf33 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/.gitignore b/.gitignore index 08138d7..96fbdd5 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ coverage.xml .venv *.xml .coverage* +docker/stunnel/keys diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fe37ff9..04f989a 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/dev_requirements.txt b/dev_requirements.txt index 2a4f377..1d33b98 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 60be374..c76d15d 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 70e8013..70df5ba 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 93c16a7..ef659e3 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 0000000..bf45109 --- /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 0000000..84f6d40 --- /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 0000000..f3bcea6 --- /dev/null +++ b/docker/stunnel/create_certs.sh @@ -0,0 +1,46 @@ +#!/bin/bash + +set -e + +DESTDIR=`dirname "$0"`/keys +test -d ${DESTDIR} || mkdir ${DESTDIR} +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 &>/dev/null + +openssl req -new -x509 -nodes -days 365000 \ + -key ca-key.pem \ + -out ca-cert.pem \ + -subj "${SSL_SUBJECT}" &>/dev/null + +openssl req -newkey rsa:2048 -nodes -days 365000 \ + -keyout server-key.pem \ + -out server-req.pem \ + -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 &>/dev/null + +openssl req -newkey rsa:2048 -nodes -days 365000 \ + -keyout client-key.pem \ + -out client-req.pem \ + -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 &>/dev/null + +echo "Keys generated in ${DESTDIR}:" +ls diff --git a/redis/client.py b/redis/client.py index c02bc3a..3ae133c 100755 --- a/redis/client.py +++ b/redis/client.py @@ -873,7 +873,9 @@ class Redis(RedisModuleCommands, CoreCommands, SentinelCommands): 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, single_connection_client=False, health_check_interval=0, @@ -947,6 +949,7 @@ class Redis(RedisModuleCommands, CoreCommands, SentinelCommands): "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 1bb8eb5..3fe8543 100755 --- a/redis/connection.py +++ b/redis/connection.py @@ -884,6 +884,11 @@ class Connection: 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, @@ -891,8 +896,24 @@ class SSLConnection(Connection): 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") @@ -915,7 +936,9 @@ class SSLConnection(Connection): 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" @@ -923,10 +946,14 @@ class SSLConnection(Connection): context = ssl.create_default_context() 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) + if self.certfile or self.keyfile: + 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) diff --git a/tasks.py b/tasks.py index 880e70d..98ed483 100644 --- a/tasks.py +++ b/tasks.py @@ -3,6 +3,11 @@ import shutil 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}" @@ -29,6 +35,7 @@ def build_docs(c): @task def linters(c): """Run code linters""" + _generate_keys() run("tox -e linters") @@ -37,6 +44,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 +55,7 @@ def tests(c): with and without hiredis. """ print("Starting Redis tests") + _generate_keys() run("tox -e '{standalone,cluster}'-'{plain,hiredis}'") @@ -55,6 +64,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 +73,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 +85,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 4b5f6cb..0149166 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,8 +14,10 @@ from redis.retry import Retry REDIS_INFO = {} default_redis_url = "redis://localhost:6379/9" - 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,6 +38,13 @@ def pytest_addoption(parser): " 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", default=default_cluster_nodes, @@ -248,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 new file mode 100644 index 0000000..70f9e58 --- /dev/null +++ b/tests/test_ssl.py @@ -0,0 +1,61 @@ +import os +from urllib.parse import urlparse + +import pytest + +import redis +from redis.exceptions import ConnectionError + + +@pytest.mark.ssl +class TestSSL: + """Tests for SSL connections + + This relies on the --redis-ssl-url purely for rebuilding the client + and connecting to the appropriate port. + """ + + 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 + 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(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) + + 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() diff --git a/tox.ini b/tox.ini index 32d1680..b574d17 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 @@ -31,6 +32,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 +93,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 +121,12 @@ 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,16 +135,9 @@ 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 +run_before = {[testenv]run_before} [testenv:linters] deps_files = dev_requirements.txt -- cgit v1.2.1