Skip to content

Changing the default value for ssl_check_hostname to True, to ensure security validations are not skipped by default #3626

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
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
19 changes: 11 additions & 8 deletions docs/examples/ssl_connection_examples.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@
"import redis\n",
"\n",
"r = redis.Redis(\n",
" host='localhost', \n",
" port=6666, \n",
" ssl=True, \n",
" host='localhost',\n",
" port=6666,\n",
" ssl=True,\n",
" ssl_check_hostname=False,\n",
" ssl_cert_reqs=\"none\",\n",
")\n",
"r.ping()"
Expand Down Expand Up @@ -68,7 +69,7 @@
"source": [
"import redis\n",
"\n",
"r = redis.from_url(\"rediss://localhost:6666?ssl_cert_reqs=none&decode_responses=True&health_check_interval=2\")\n",
"r = redis.from_url(\"rediss://localhost:6666?ssl_cert_reqs=none&ssl_check_hostname=False&decode_responses=True&health_check_interval=2\")\n",
"r.ping()"
]
},
Expand Down Expand Up @@ -99,13 +100,14 @@
"import redis\n",
"\n",
"redis_pool = redis.ConnectionPool(\n",
" host=\"localhost\", \n",
" port=6666, \n",
" connection_class=redis.SSLConnection, \n",
" host=\"localhost\",\n",
" port=6666,\n",
" connection_class=redis.SSLConnection,\n",
" ssl_check_hostname=False,\n",
" ssl_cert_reqs=\"none\",\n",
")\n",
"\n",
"r = redis.StrictRedis(connection_pool=redis_pool) \n",
"r = redis.StrictRedis(connection_pool=redis_pool)\n",
"r.ping()"
]
},
Expand Down Expand Up @@ -141,6 +143,7 @@
" port=6666,\n",
" ssl=True,\n",
" ssl_min_version=ssl.TLSVersion.TLSv1_3,\n",
" ssl_check_hostname=False,\n",
" ssl_cert_reqs=\"none\",\n",
")\n",
"r.ping()"
Expand Down
2 changes: 1 addition & 1 deletion redis/asyncio/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def __init__(
ssl_cert_reqs: Union[str, VerifyMode] = "required",
ssl_ca_certs: Optional[str] = None,
ssl_ca_data: Optional[str] = None,
ssl_check_hostname: bool = False,
ssl_check_hostname: bool = True,
ssl_min_version: Optional[TLSVersion] = None,
ssl_ciphers: Optional[str] = None,
max_connections: Optional[int] = None,
Expand Down
2 changes: 1 addition & 1 deletion redis/asyncio/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ def __init__(
ssl_ca_data: Optional[str] = None,
ssl_cert_reqs: Union[str, VerifyMode] = "required",
ssl_certfile: Optional[str] = None,
ssl_check_hostname: bool = False,
ssl_check_hostname: bool = True,
ssl_keyfile: Optional[str] = None,
ssl_min_version: Optional[TLSVersion] = None,
ssl_ciphers: Optional[str] = None,
Expand Down
2 changes: 1 addition & 1 deletion redis/asyncio/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ def __init__(
ssl_cert_reqs: Union[str, ssl.VerifyMode] = "required",
ssl_ca_certs: Optional[str] = None,
ssl_ca_data: Optional[str] = None,
ssl_check_hostname: bool = False,
ssl_check_hostname: bool = True,
ssl_min_version: Optional[TLSVersion] = None,
ssl_ciphers: Optional[str] = None,
**kwargs,
Expand Down
2 changes: 1 addition & 1 deletion redis/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ def __init__(
ssl_ca_certs: Optional[str] = None,
ssl_ca_path: Optional[str] = None,
ssl_ca_data: Optional[str] = None,
ssl_check_hostname: bool = False,
ssl_check_hostname: bool = True,
ssl_password: Optional[str] = None,
ssl_validate_ocsp: bool = False,
ssl_validate_ocsp_stapled: bool = False,
Expand Down
2 changes: 1 addition & 1 deletion redis/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ def __init__(
ssl_cert_reqs="required",
ssl_ca_certs=None,
ssl_ca_data=None,
ssl_check_hostname=False,
ssl_check_hostname=True,
ssl_ca_path=None,
ssl_password=None,
ssl_validate_ocsp=False,
Expand Down
23 changes: 22 additions & 1 deletion tests/test_asyncio/test_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -3118,7 +3118,9 @@ async def test_ssl_with_invalid_cert(
async def test_ssl_connection(
self, create_client: Callable[..., Awaitable[RedisCluster]]
) -> None:
async with await create_client(ssl=True, ssl_cert_reqs="none") as rc:
async with await create_client(
ssl=True, ssl_check_hostname=False, ssl_cert_reqs="none"
) as rc:
assert await rc.ping()

@pytest.mark.parametrize(
Expand All @@ -3134,6 +3136,7 @@ async def test_ssl_connection_tls12_custom_ciphers(
) -> None:
async with await create_client(
ssl=True,
ssl_check_hostname=False,
ssl_cert_reqs="none",
ssl_min_version=ssl.TLSVersion.TLSv1_2,
ssl_ciphers=ssl_ciphers,
Expand All @@ -3145,6 +3148,7 @@ async def test_ssl_connection_tls12_custom_ciphers_invalid(
) -> None:
async with await create_client(
ssl=True,
ssl_check_hostname=False,
ssl_cert_reqs="none",
ssl_min_version=ssl.TLSVersion.TLSv1_2,
ssl_ciphers="foo:bar",
Expand All @@ -3166,6 +3170,7 @@ async def test_ssl_connection_tls13_custom_ciphers(
# TLSv1.3 does not support changing the ciphers
async with await create_client(
ssl=True,
ssl_check_hostname=False,
ssl_cert_reqs="none",
ssl_min_version=ssl.TLSVersion.TLSv1_2,
ssl_ciphers=ssl_ciphers,
Expand All @@ -3177,12 +3182,20 @@ async def test_ssl_connection_tls13_custom_ciphers(
async def test_validating_self_signed_certificate(
self, create_client: Callable[..., Awaitable[RedisCluster]]
) -> None:
# ssl_check_hostname=False is used to avoid hostname verification
# in the test environment, where the server certificate is self-signed
# and does not match the hostname that is extracted for the cluster.
# Cert hostname is 'localhost' in the cluster initialization when using
# 'localhost' it gets transformed into 127.0.0.1
# In production code, ssl_check_hostname should be set to True
# to ensure proper hostname verification.
async with await create_client(
ssl=True,
ssl_ca_certs=self.ca_cert,
ssl_cert_reqs="required",
ssl_certfile=self.client_cert,
ssl_keyfile=self.client_key,
ssl_check_hostname=False,
) as rc:
assert await rc.ping()

Expand All @@ -3192,10 +3205,18 @@ async def test_validating_self_signed_string_certificate(
with open(self.ca_cert) as f:
cert_data = f.read()

# ssl_check_hostname=False is used to avoid hostname verification
# in the test environment, where the server certificate is self-signed
# and does not match the hostname that is extracted for the cluster.
# Cert hostname is 'localhost' in the cluster initialization when using
# 'localhost' it gets transformed into 127.0.0.1
# In production code, ssl_check_hostname should be set to True
# to ensure proper hostname verification.
async with await create_client(
ssl=True,
ssl_ca_data=cert_data,
ssl_cert_reqs="required",
ssl_check_hostname=False,
ssl_certfile=self.client_cert,
ssl_keyfile=self.client_key,
) as rc:
Expand Down
13 changes: 12 additions & 1 deletion tests/test_asyncio/test_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ async def test_uds_connect(uds_address):
async def test_tcp_ssl_tls12_custom_ciphers(tcp_address, ssl_ciphers):
host, port = tcp_address

# in order to have working hostname verification, we need to use "localhost"
# as redis host as the server certificate is self-signed and only valid for "localhost"
host = "localhost"

server_certs = get_tls_certificates(cert_type=CertificateType.server)

conn = SSLConnection(
Expand Down Expand Up @@ -89,6 +93,10 @@ async def test_tcp_ssl_tls12_custom_ciphers(tcp_address, ssl_ciphers):
async def test_tcp_ssl_connect(tcp_address, ssl_min_version):
host, port = tcp_address

# in order to have working hostname verification, we need to use "localhost"
# as redis host as the server certificate is self-signed and only valid for "localhost"
host = "localhost"

server_certs = get_tls_certificates(cert_type=CertificateType.server)

conn = SSLConnection(
Expand All @@ -100,7 +108,10 @@ async def test_tcp_ssl_connect(tcp_address, ssl_min_version):
ssl_min_version=ssl_min_version,
)
await _assert_connect(
conn, tcp_address, certfile=server_certs.certfile, keyfile=server_certs.keyfile
conn,
tcp_address,
certfile=server_certs.certfile,
keyfile=server_certs.keyfile,
)
await conn.disconnect()

Expand Down
10 changes: 10 additions & 0 deletions tests/test_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,16 @@ def test_uds_connect(uds_address):
)
def test_tcp_ssl_connect(tcp_address, ssl_min_version):
host, port = tcp_address

# in order to have working hostname verification, we need to use "localhost"
# as redis host as the server certificate is self-signed and only valid for "localhost"
host = "localhost"
server_certs = get_tls_certificates(cert_type=CertificateType.server)

conn = SSLConnection(
host=host,
port=port,
ssl_check_hostname=True,
client_name=_CLIENT_NAME,
ssl_ca_certs=server_certs.ca_certfile,
socket_timeout=10,
Expand All @@ -80,6 +86,10 @@ def test_tcp_ssl_connect(tcp_address, ssl_min_version):
def test_tcp_ssl_tls12_custom_ciphers(tcp_address, ssl_ciphers):
host, port = tcp_address

# in order to have working hostname verification, we need to use "localhost"
# as redis host as the server certificate is self-signed and only valid for "localhost"
host = "localhost"

server_certs = get_tls_certificates(cert_type=CertificateType.server)

conn = SSLConnection(
Expand Down
12 changes: 11 additions & 1 deletion tests/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,14 @@ def test_ssl_with_invalid_cert(self, request):
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")

r = redis.Redis(
host=p[0],
port=p[1],
ssl=True,
ssl_check_hostname=False,
ssl_cert_reqs="none",
)
assert r.ping()
r.close()

Expand Down Expand Up @@ -98,6 +105,7 @@ def test_ssl_connection_tls12_custom_ciphers(self, request, ssl_ciphers):
host=p[0],
port=p[1],
ssl=True,
ssl_check_hostname=False,
ssl_cert_reqs="none",
ssl_min_version=ssl.TLSVersion.TLSv1_3,
ssl_ciphers=ssl_ciphers,
Expand All @@ -112,6 +120,7 @@ def test_ssl_connection_tls12_custom_ciphers_invalid(self, request):
host=p[0],
port=p[1],
ssl=True,
ssl_check_hostname=False,
ssl_cert_reqs="none",
ssl_min_version=ssl.TLSVersion.TLSv1_2,
ssl_ciphers="foo:bar",
Expand All @@ -136,6 +145,7 @@ def test_ssl_connection_tls13_custom_ciphers(self, request, ssl_ciphers):
host=p[0],
port=p[1],
ssl=True,
ssl_check_hostname=False,
ssl_cert_reqs="none",
ssl_min_version=ssl.TLSVersion.TLSv1_2,
ssl_ciphers=ssl_ciphers,
Expand Down
Loading