Skip to content

Commit 1351460

Browse files
taylorleesetaylorleese
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 d892956 commit 1351460

File tree

6 files changed

+131
-28
lines changed

6 files changed

+131
-28
lines changed

‎.env.example‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@ REDIS_DB=0
44
REDIS_USERNAME=default
55
REDIS_PWD=your_password
66
REDIS_SSL=False
7-
REDIS_CA_PATH=/path/to/ca.pem
7+
REDIS_SSL_CA_PATH=/path/to/ca.pem
88
REDIS_SSL_KEYFILE=/path/to/key.pem
99
REDIS_SSL_CERTFILE=/path/to/cert.pem
10-
REDIS_CERT_REQS=required
11-
REDIS_CA_CERTS=/path/to/ca_certs.pem
10+
REDIS_SSL_CERT_REQS=required
11+
REDIS_SSL_CA_CERTS=/path/to/ca_certs.pem
12+
REDIS_SSL_CHECK_HOSTNAME=true
1213
REDIS_CLUSTER_MODE=False

‎README.md‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ The following example is for Claude Desktop, but the same applies to any other M
195195
"REDIS_PORT": "<your_redis_database_port>",
196196
"REDIS_PWD": "<your_redis_database_password>",
197197
"REDIS_SSL": True|False,
198-
"REDIS_CA_PATH": "<your_redis_ca_path>",
198+
"REDIS_SSL_CA_PATH": "<your_redis_ca_path>",
199199
"REDIS_CLUSTER_MODE": True|False
200200
}
201201
}
@@ -300,20 +300,21 @@ uvx --from redis-mcp-server@latest redis-mcp-server --help
300300

301301
If desired, you can use environment variables. Defaults are provided for all variables.
302302

303-
| Name | Description | Default Value |
304-
|----------------------|-----------------------------------------------------------|---------------|
305-
| `REDIS_HOST` | Redis IP or hostname | `"127.0.0.1"` |
306-
| `REDIS_PORT` | Redis port | `6379` |
307-
| `REDIS_DB` | Database | 0 |
308-
| `REDIS_USERNAME` | Default database username | `"default"` |
309-
| `REDIS_PWD` | Default database password | "" |
310-
| `REDIS_SSL` | Enables or disables SSL/TLS | `False` |
311-
| `REDIS_CA_PATH` | CA certificate for verifying server | None |
312-
| `REDIS_SSL_KEYFILE` | Client's private key file for client authentication | None |
313-
| `REDIS_SSL_CERTFILE` | Client's certificate file for client authentication | None |
314-
| `REDIS_CERT_REQS` | Whether the client should verify the server's certificate | `"required"` |
315-
| `REDIS_CA_CERTS` | Path to the trusted CA certificates file | None |
316-
| `REDIS_CLUSTER_MODE` | Enable Redis Cluster mode | `False` |
303+
| Name | Description | Default Value |
304+
|----------------------------|------------------------------------------------------------------- |---------------|
305+
| `REDIS_HOST` | Redis IP or hostname | `"127.0.0.1"` |
306+
| `REDIS_PORT` | Redis port | `6379` |
307+
| `REDIS_DB` | Database | 0 |
308+
| `REDIS_USERNAME` | Default database username | `"default"` |
309+
| `REDIS_PWD` | Default database password | "" |
310+
| `REDIS_SSL` | Enables or disables SSL/TLS | `False` |
311+
| `REDIS_SSL_CA_PATH` | CA certificate path for verifying server | None |
312+
| `REDIS_SSL_KEYFILE` | Client's private key file for client authentication | None |
313+
| `REDIS_SSL_CERTFILE` | Client's certificate file for client authentication | None |
314+
| `REDIS_SSL_CERT_REQS` | Certificate requirements (none, optional, or required) | `"required"` |
315+
| `REDIS_SSL_CA_CERTS` | Path to the trusted CA certificates file | None |
316+
| `REDIS_SSL_CHECK_HOSTNAME` | Verify SSL certificate hostname (auto-disabled when cert_reqs=none)| `True` |
317+
| `REDIS_CLUSTER_MODE` | Enable Redis Cluster mode | `False` |
317318

318319

319320
There are several ways to set environment variables:

‎smithery.yaml‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ startCommand:
2727
type: boolean
2828
default: false
2929
description: Enable SSL for Redis connection
30-
redisCAPath:
30+
redisSSLCAPath:
3131
type: string
3232
default: ""
3333
description: CA certificate path for verifying server
@@ -39,14 +39,18 @@ startCommand:
3939
type: string
4040
default: ""
4141
description: Client certificate file for authentication
42-
redisCertReqs:
42+
redisSSLCertReqs:
4343
type: string
4444
default: required
45-
description: Certificate requirements
46-
redisCACerts:
45+
description: Certificate requirements (none, optional, or required)
46+
redisSSLCACerts:
4747
type: string
4848
default: ""
4949
description: Path to trusted CA certificates file
50+
redisSSLCheckHostname:
51+
type: boolean
52+
default: true
53+
description: Verify SSL certificate hostname (auto-disabled when cert_reqs=none)
5054
commandFunction:
5155
# A JS function that produces the CLI command based on the given config to start the MCP on stdio.
5256
|-
@@ -59,11 +63,12 @@ startCommand:
5963
REDIS_USERNAME: config.redisUsername,
6064
REDIS_PWD: config.redisPwd,
6165
REDIS_SSL: String(config.redisSSL),
62-
REDIS_CA_PATH: config.redisCAPath,
66+
REDIS_SSL_CA_PATH: config.redisSSLCAPath,
6367
REDIS_SSL_KEYFILE: config.redisSSLKeyfile,
6468
REDIS_SSL_CERTFILE: config.redisSSLCertfile,
65-
REDIS_CERT_REQS: config.redisCertReqs,
66-
REDIS_CA_CERTS: config.redisCACerts
69+
REDIS_SSL_CERT_REQS: config.redisSSLCertReqs,
70+
REDIS_SSL_CA_CERTS: config.redisSSLCACerts,
71+
REDIS_SSL_CHECK_HOSTNAME: String(config.redisSSLCheckHostname)
6772
}
6873
})
6974
exampleConfig:
@@ -72,8 +77,9 @@ startCommand:
7277
redisUsername: default
7378
redisPwd: ""
7479
redisSSL: false
75-
redisCAPath: ""
80+
redisSSLCAPath: ""
7681
redisSSLKeyfile: ""
7782
redisSSLCertfile: ""
78-
redisCertReqs: required
79-
redisCACerts: ""
83+
redisSSLCertReqs: required
84+
redisSSLCACerts: ""
85+
redisSSLCheckHostname: true

‎src/common/config.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@
2020
"db": int(os.getenv("REDIS_DB", 0)),
2121
}
2222

23+
# When ssl_cert_reqs is "none", we should disable hostname checking by default
24+
# This matches the behavior of redis-cli --insecure flag
25+
default_check_hostname = "false" if REDIS_CFG["ssl_cert_reqs"] == "none" else "true"
26+
REDIS_CFG["ssl_check_hostname"] = os.getenv("REDIS_SSL_CHECK_HOSTNAME", default_check_hostname) in ("true", "1", "t")
27+
2328

2429
def parse_redis_uri(uri: str) -> dict:
2530
"""Parse a Redis URI and return connection parameters."""
@@ -69,6 +74,8 @@ def parse_redis_uri(uri: str) -> dict:
6974
config["ssl_keyfile"] = query_params["ssl_keyfile"][0]
7075
if "ssl_certfile" in query_params:
7176
config["ssl_certfile"] = query_params["ssl_certfile"][0]
77+
if "ssl_check_hostname" in query_params:
78+
config["ssl_check_hostname"] = query_params["ssl_check_hostname"][0] in ("true", "1", "t")
7279

7380
# Handle other parameters. According to https://www.iana.org/assignments/uri-schemes/prov/redis,
7481
# 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
@@ -31,6 +31,7 @@ def get_connection(cls, decode_responses=True) -> Redis:
3131
"ssl_certfile": REDIS_CFG["ssl_certfile"],
3232
"ssl_cert_reqs": REDIS_CFG["ssl_cert_reqs"],
3333
"ssl_ca_certs": REDIS_CFG["ssl_ca_certs"],
34+
"ssl_check_hostname": REDIS_CFG["ssl_check_hostname"],
3435
"decode_responses": decode_responses,
3536
"lib_name": f"redis-py(mcp-server_v{__version__})",
3637
"max_connections_per_node": 10,
@@ -49,6 +50,7 @@ def get_connection(cls, decode_responses=True) -> Redis:
4950
"ssl_certfile": REDIS_CFG["ssl_certfile"],
5051
"ssl_cert_reqs": REDIS_CFG["ssl_cert_reqs"],
5152
"ssl_ca_certs": REDIS_CFG["ssl_ca_certs"],
53+
"ssl_check_hostname": REDIS_CFG["ssl_check_hostname"],
5254
"decode_responses": decode_responses,
5355
"lib_name": f"redis-py(mcp-server_v{__version__})",
5456
"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)