Skip to content

Commit fc0e050

Browse files
committed
Applying review comments
1 parent 977c496 commit fc0e050

File tree

4 files changed

+40
-5
lines changed

4 files changed

+40
-5
lines changed

tests/test_asyncio/test_cluster.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3063,6 +3063,7 @@ def create_client(self, request: FixtureRequest) -> Callable[..., RedisCluster]:
30633063
)
30643064

30653065
async def _create_client(mocked: bool = True, **kwargs: Any) -> RedisCluster:
3066+
kwargs.pop("use_localhost", None)
30663067
if mocked:
30673068
with mock.patch.object(
30683069
ClusterNode, "execute_command", autospec=True
@@ -3182,13 +3183,20 @@ async def test_ssl_connection_tls13_custom_ciphers(
31823183
async def test_validating_self_signed_certificate(
31833184
self, create_client: Callable[..., Awaitable[RedisCluster]]
31843185
) -> None:
3186+
# ssl_check_hostname=False is used to avoid hostname verification
3187+
# in the test environment, where the server certificate is self-signed
3188+
# and does not match the hostname that is extracted for the cluster.
3189+
# Cert hostname is 'localhost' in the cluster initialization when using
3190+
# 'localhost' it gets transformed into 127.0.0.1
3191+
# In production code, ssl_check_hostname should be set to True
3192+
# to ensure proper hostname verification.
31853193
async with await create_client(
31863194
ssl=True,
31873195
ssl_ca_certs=self.ca_cert,
31883196
ssl_cert_reqs="required",
3189-
ssl_check_hostname=False,
31903197
ssl_certfile=self.client_cert,
31913198
ssl_keyfile=self.client_key,
3199+
ssl_check_hostname=False,
31923200
) as rc:
31933201
assert await rc.ping()
31943202

@@ -3198,6 +3206,13 @@ async def test_validating_self_signed_string_certificate(
31983206
with open(self.ca_cert) as f:
31993207
cert_data = f.read()
32003208

3209+
# ssl_check_hostname=False is used to avoid hostname verification
3210+
# in the test environment, where the server certificate is self-signed
3211+
# and does not match the hostname that is extracted for the cluster.
3212+
# Cert hostname is 'localhost' in the cluster initialization when using
3213+
# 'localhost' it gets transformed into 127.0.0.1
3214+
# In production code, ssl_check_hostname should be set to True
3215+
# to ensure proper hostname verification.
32013216
async with await create_client(
32023217
ssl=True,
32033218
ssl_ca_data=cert_data,

tests/test_asyncio/test_connect.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,17 @@ async def test_uds_connect(uds_address):
5858
async def test_tcp_ssl_tls12_custom_ciphers(tcp_address, ssl_ciphers):
5959
host, port = tcp_address
6060

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

67+
# ssl_check_hostname=False is used to avoid hostname verification
68+
# in the test environment, where the server certificate is self-signed
69+
# and does not match the hostname.
70+
# In production code, ssl_check_hostname should be set to True
71+
# to ensure proper hostname verification.
6372
conn = SSLConnection(
6473
host=host,
6574
port=port,
@@ -68,7 +77,6 @@ async def test_tcp_ssl_tls12_custom_ciphers(tcp_address, ssl_ciphers):
6877
socket_timeout=10,
6978
ssl_min_version=ssl.TLSVersion.TLSv1_2,
7079
ssl_ciphers=ssl_ciphers,
71-
ssl_check_hostname=False,
7280
)
7381
await _assert_connect(
7482
conn, tcp_address, certfile=server_certs.certfile, keyfile=server_certs.keyfile
@@ -90,13 +98,16 @@ async def test_tcp_ssl_tls12_custom_ciphers(tcp_address, ssl_ciphers):
9098
async def test_tcp_ssl_connect(tcp_address, ssl_min_version):
9199
host, port = tcp_address
92100

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

95107
conn = SSLConnection(
96108
host=host,
97109
port=port,
98110
client_name=_CLIENT_NAME,
99-
ssl_check_hostname=False,
100111
ssl_ca_certs=server_certs.ca_certfile,
101112
socket_timeout=10,
102113
ssl_min_version=ssl_min_version,

tests/test_connect.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,16 @@ def test_uds_connect(uds_address):
5454
)
5555
def test_tcp_ssl_connect(tcp_address, ssl_min_version):
5656
host, port = tcp_address
57+
58+
# in order to have working hostname verification, we need to use "localhost"
59+
# as redis host as the server certificate is self-signed and only valid for "localhost"
60+
host = "localhost"
5761
server_certs = get_tls_certificates(cert_type=CertificateType.server)
62+
5863
conn = SSLConnection(
5964
host=host,
6065
port=port,
61-
ssl_check_hostname=False,
66+
ssl_check_hostname=True,
6267
client_name=_CLIENT_NAME,
6368
ssl_ca_certs=server_certs.ca_certfile,
6469
socket_timeout=10,
@@ -81,6 +86,10 @@ def test_tcp_ssl_connect(tcp_address, ssl_min_version):
8186
def test_tcp_ssl_tls12_custom_ciphers(tcp_address, ssl_ciphers):
8287
host, port = tcp_address
8388

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

8695
conn = SSLConnection(
@@ -91,7 +100,6 @@ def test_tcp_ssl_tls12_custom_ciphers(tcp_address, ssl_ciphers):
91100
socket_timeout=10,
92101
ssl_min_version=ssl.TLSVersion.TLSv1_2,
93102
ssl_ciphers=ssl_ciphers,
94-
ssl_check_hostname=False,
95103
)
96104
_assert_connect(
97105
conn, tcp_address, certfile=server_certs.certfile, keyfile=server_certs.keyfile

tests/test_ssl.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ def test_ssl_with_invalid_cert(self, request):
3737
def test_ssl_connection(self, request):
3838
ssl_url = request.config.option.redis_ssl_url
3939
p = urlparse(ssl_url)[1].split(":")
40+
4041
r = redis.Redis(
4142
host=p[0],
4243
port=p[1],

0 commit comments

Comments
 (0)