Skip to content

Support for password-encrypted SSL private keys #1782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Dec 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/install_and_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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}
1 change: 1 addition & 0 deletions .github/workflows/integration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ coverage.xml
.venv
*.xml
.coverage*
docker/stunnel/keys
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions dev_requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions docker/base/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# produces redisfab/redis-py:6.2.6
FROM redis:6.2.6-buster

CMD ["redis-server", "/redis.conf"]
3 changes: 2 additions & 1 deletion docker/base/Dockerfile.cluster
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# produces redisfab/redis-py-cluster:6.2.6
FROM redis:6.2.6-buster

COPY create_cluster.sh /create_cluster.sh
RUN chmod +x /create_cluster.sh

EXPOSE 16379 16380 16381 16382 16383 16384

CMD [ "/create_cluster.sh"]
CMD [ "/create_cluster.sh"]
1 change: 1 addition & 0 deletions docker/base/Dockerfile.sentinel
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# produces redisfab/redis-py-sentinel:6.2.6
FROM redis:6.2.6-buster

CMD ["redis-sentinel", "/sentinel.conf"]
11 changes: 11 additions & 0 deletions docker/base/Dockerfile.stunnel
Original file line number Diff line number Diff line change
@@ -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"]
6 changes: 6 additions & 0 deletions docker/stunnel/conf/redis.conf
Original file line number Diff line number Diff line change
@@ -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
46 changes: 46 additions & 0 deletions docker/stunnel/create_certs.sh
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions redis/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,9 @@ 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,
single_connection_client=False,
health_check_interval=0,
Expand Down Expand Up @@ -947,6 +949,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)
Expand Down
35 changes: 31 additions & 4 deletions redis/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -879,15 +879,36 @@ 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,
ssl_certfile=None,
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")

Expand All @@ -910,18 +931,24 @@ 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"
sock = super()._connect()
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)


Expand Down
13 changes: 13 additions & 0 deletions tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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}"
Expand All @@ -29,6 +35,7 @@ def build_docs(c):
@task
def linters(c):
"""Run code linters"""
_generate_keys()
run("tox -e linters")


Expand All @@ -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)

Expand All @@ -47,6 +55,7 @@ def tests(c):
with and without hiredis.
"""
print("Starting Redis tests")
_generate_keys()
run("tox -e '{standalone,cluster}'-'{plain,hiredis}'")


Expand All @@ -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}'")


Expand All @@ -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}'")


Expand All @@ -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
Expand Down
17 changes: 16 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@

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


Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
61 changes: 61 additions & 0 deletions tests/test_ssl.py
Original file line number Diff line number Diff line change
@@ -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()
Loading