Skip to content

Commit 079ebd6

Browse files
taylorleesevchomakov
taylorleese
authored and
vchomakov
committed
Fix SSL hostname verification bug and update env var names
This commit addresses two issues: 1. **SSL Hostname Verification Bug**: Fixed the error "Cannot set verify_mode to CERT_NONE when check_hostname is enabled" by adding support for the `ssl_check_hostname` parameter. When `REDIS_SSL_CERT_REQS=none` is set, hostname checking is now automatically disabled by default, matching the behavior of redis-cli's --insecure flag. This is essential for scenarios like AWS SSM port forwarding where the connection goes to localhost but the certificate is issued for the actual hostname. 2. **Environment Variable Naming**: Fixed inconsistencies in documentation (.env.example, README.md, smithery.yaml) where SSL-related environment variables were missing the "SSL_" prefix. Updated: - REDIS_CA_PATH → REDIS_SSL_CA_PATH - REDIS_CERT_REQS → REDIS_SSL_CERT_REQS - REDIS_CA_CERTS → REDIS_SSL_CA_CERTS Changes: - Added REDIS_SSL_CHECK_HOSTNAME configuration option - Automatically sets check_hostname=False when cert_reqs="none" - Added ssl_check_hostname support in parse_redis_uri() - Passed ssl_check_hostname to both Redis and RedisCluster connections - Added comprehensive tests for the new functionality - Updated documentation to reflect correct variable names
1 parent 19994a0 commit 079ebd6

File tree

5 files changed

+111
-14
lines changed

5 files changed

+111
-14
lines changed

‎.env.example‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ REDIS_SSL_KEYFILE=/path/to/key.pem
99
REDIS_SSL_CERTFILE=/path/to/cert.pem
1010
REDIS_SSL_CERT_REQS=required
1111
REDIS_SSL_CA_CERTS=/path/to/ca_certs.pem
12+
REDIS_SSL_CHECK_HOSTNAME=true
1213
REDIS_CLUSTER_MODE=False

‎README.md‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -336,20 +336,21 @@ uvx --from redis-mcp-server@latest redis-mcp-server --help
336336

337337
If desired, you can use environment variables. Defaults are provided for all variables.
338338

339-
| Name | Description | Default Value |
340-
|----------------------|-----------------------------------------------------------|---------------|
341-
| `REDIS_HOST` | Redis IP or hostname | `"127.0.0.1"` |
342-
| `REDIS_PORT` | Redis port | `6379` |
343-
| `REDIS_DB` | Database | 0 |
344-
| `REDIS_USERNAME` | Default database username | `"default"` |
345-
| `REDIS_PWD` | Default database password | "" |
346-
| `REDIS_SSL` | Enables or disables SSL/TLS | `False` |
347-
| `REDIS_SSL_CA_PATH` | CA certificate for verifying server | None |
348-
| `REDIS_SSL_KEYFILE` | Client's private key file for client authentication | None |
349-
| `REDIS_SSL_CERTFILE` | Client's certificate file for client authentication | None |
350-
| `REDIS_SSL_CERT_REQS`| Whether the client should verify the server's certificate | `"required"` |
351-
| `REDIS_SSL_CA_CERTS` | Path to the trusted CA certificates file | None |
352-
| `REDIS_CLUSTER_MODE` | Enable Redis Cluster mode | `False` |
339+
| Name | Description | Default Value |
340+
|----------------------------|------------------------------------------------------------------- |---------------|
341+
| `REDIS_HOST` | Redis IP or hostname | `"127.0.0.1"` |
342+
| `REDIS_PORT` | Redis port | `6379` |
343+
| `REDIS_DB` | Database | 0 |
344+
| `REDIS_USERNAME` | Default database username | `"default"` |
345+
| `REDIS_PWD` | Default database password | "" |
346+
| `REDIS_SSL` | Enables or disables SSL/TLS | `False` |
347+
| `REDIS_SSL_CA_PATH` | CA certificate path for verifying server | None |
348+
| `REDIS_SSL_KEYFILE` | Client's private key file for client authentication | None |
349+
| `REDIS_SSL_CERTFILE` | Client's certificate file for client authentication | None |
350+
| `REDIS_SSL_CERT_REQS` | Certificate requirements (none, optional, or required) | `"required"` |
351+
| `REDIS_SSL_CA_CERTS` | Path to the trusted CA certificates file | None |
352+
| `REDIS_SSL_CHECK_HOSTNAME` | Verify SSL certificate hostname (auto-disabled when cert_reqs=none)| `True` |
353+
| `REDIS_CLUSTER_MODE` | Enable Redis Cluster mode | `False` |
353354

354355
### EntraID Authentication for Azure Managed Redis
355356

‎src/common/config.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@
2727
"db": int(os.getenv("REDIS_DB", 0)),
2828
}
2929

30+
# When ssl_cert_reqs is "none", we should disable hostname checking by default
31+
# This matches the behavior of redis-cli --insecure flag
32+
default_check_hostname = "false" if REDIS_CFG["ssl_cert_reqs"] == "none" else "true"
33+
REDIS_CFG["ssl_check_hostname"] = os.getenv("REDIS_SSL_CHECK_HOSTNAME", default_check_hostname) in ("true", "1", "t")
34+
3035
# Entra ID Authentication Configuration
3136
ENTRAID_CFG = {
3237
# Authentication flow selection
@@ -125,6 +130,8 @@ def parse_redis_uri(uri: str) -> dict:
125130
config["ssl_keyfile"] = query_params["ssl_keyfile"][0]
126131
if "ssl_certfile" in query_params:
127132
config["ssl_certfile"] = query_params["ssl_certfile"][0]
133+
if "ssl_check_hostname" in query_params:
134+
config["ssl_check_hostname"] = query_params["ssl_check_hostname"][0] in ("true", "1", "t")
128135

129136
# Handle other parameters. According to https://www.iana.org/assignments/uri-schemes/prov/redis,
130137
# The database number to use for the Redis SELECT command comes from

‎src/common/connection.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ def get_connection(cls, decode_responses=True) -> Redis:
4848
"ssl_certfile": REDIS_CFG["ssl_certfile"],
4949
"ssl_cert_reqs": REDIS_CFG["ssl_cert_reqs"],
5050
"ssl_ca_certs": REDIS_CFG["ssl_ca_certs"],
51+
"ssl_check_hostname": REDIS_CFG["ssl_check_hostname"],
5152
"decode_responses": decode_responses,
5253
"lib_name": f"redis-py(mcp-server_v{__version__})",
5354
"max_connections_per_node": 10,
@@ -72,6 +73,7 @@ def get_connection(cls, decode_responses=True) -> Redis:
7273
"ssl_certfile": REDIS_CFG["ssl_certfile"],
7374
"ssl_cert_reqs": REDIS_CFG["ssl_cert_reqs"],
7475
"ssl_ca_certs": REDIS_CFG["ssl_ca_certs"],
76+
"ssl_check_hostname": REDIS_CFG["ssl_check_hostname"],
7577
"decode_responses": decode_responses,
7678
"lib_name": f"redis-py(mcp-server_v{__version__})",
7779
"max_connections": 10,

‎tests/test_config.py‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,22 @@ def test_parse_uri_with_ssl_parameters(self):
8080
assert result["ssl_certfile"] == "/cert.pem"
8181
assert result["ssl_ca_path"] == "/ca.pem"
8282

83+
def test_parse_uri_with_ssl_check_hostname(self):
84+
"""Test parsing URI with ssl_check_hostname query parameter."""
85+
uri = "rediss://localhost:6379/0?ssl_check_hostname=false"
86+
result = parse_redis_uri(uri)
87+
88+
assert result["ssl"] is True
89+
assert result["ssl_check_hostname"] is False
90+
91+
def test_parse_uri_with_ssl_check_hostname_true(self):
92+
"""Test parsing URI with ssl_check_hostname set to true."""
93+
uri = "rediss://localhost:6379/0?ssl_check_hostname=true"
94+
result = parse_redis_uri(uri)
95+
96+
assert result["ssl"] is True
97+
assert result["ssl_check_hostname"] is True
98+
8399
def test_parse_uri_defaults(self):
84100
"""Test parsing URI with default values."""
85101
uri = "redis://example.com"
@@ -286,3 +302,73 @@ def test_config_from_environment(self, mock_load_dotenv):
286302
assert config["port"] == 6380
287303
assert config["ssl"] is True
288304
assert config["cluster_mode"] is True
305+
306+
@patch.dict(
307+
os.environ,
308+
{
309+
"REDIS_SSL": "true",
310+
"REDIS_SSL_CERT_REQS": "none",
311+
},
312+
)
313+
@patch("src.common.config.load_dotenv")
314+
def test_ssl_check_hostname_disabled_with_cert_reqs_none(self, mock_load_dotenv):
315+
"""Test that ssl_check_hostname is disabled by default when ssl_cert_reqs is none."""
316+
# Re-import to get fresh config
317+
import importlib
318+
319+
import src.common.config
320+
321+
importlib.reload(src.common.config)
322+
323+
config = src.common.config.REDIS_CFG
324+
325+
assert config["ssl"] is True
326+
assert config["ssl_cert_reqs"] == "none"
327+
assert config["ssl_check_hostname"] is False
328+
329+
@patch.dict(
330+
os.environ,
331+
{
332+
"REDIS_SSL": "true",
333+
"REDIS_SSL_CERT_REQS": "required",
334+
},
335+
)
336+
@patch("src.common.config.load_dotenv")
337+
def test_ssl_check_hostname_enabled_with_cert_reqs_required(self, mock_load_dotenv):
338+
"""Test that ssl_check_hostname is enabled by default when ssl_cert_reqs is required."""
339+
# Re-import to get fresh config
340+
import importlib
341+
342+
import src.common.config
343+
344+
importlib.reload(src.common.config)
345+
346+
config = src.common.config.REDIS_CFG
347+
348+
assert config["ssl"] is True
349+
assert config["ssl_cert_reqs"] == "required"
350+
assert config["ssl_check_hostname"] is True
351+
352+
@patch.dict(
353+
os.environ,
354+
{
355+
"REDIS_SSL": "true",
356+
"REDIS_SSL_CERT_REQS": "none",
357+
"REDIS_SSL_CHECK_HOSTNAME": "true",
358+
},
359+
)
360+
@patch("src.common.config.load_dotenv")
361+
def test_ssl_check_hostname_override(self, mock_load_dotenv):
362+
"""Test that ssl_check_hostname can be explicitly overridden."""
363+
# Re-import to get fresh config
364+
import importlib
365+
366+
import src.common.config
367+
368+
importlib.reload(src.common.config)
369+
370+
config = src.common.config.REDIS_CFG
371+
372+
assert config["ssl"] is True
373+
assert config["ssl_cert_reqs"] == "none"
374+
assert config["ssl_check_hostname"] is True

0 commit comments

Comments
 (0)