From 079ebd611c652d60ccfcd733614dc4f0c5122357 Mon Sep 17 00:00:00 2001 From: Taylor Leese Date: Mon, 3 Nov 2025 21:10:56 -0800 Subject: [PATCH 1/3] Fix SSL hostname verification bug and update env var names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .env.example | 1 + README.md | 29 +++++++------- src/common/config.py | 7 ++++ src/common/connection.py | 2 + tests/test_config.py | 86 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 111 insertions(+), 14 deletions(-) diff --git a/.env.example b/.env.example index eff314f..9416ec6 100644 --- a/.env.example +++ b/.env.example @@ -9,4 +9,5 @@ REDIS_SSL_KEYFILE=/path/to/key.pem REDIS_SSL_CERTFILE=/path/to/cert.pem REDIS_SSL_CERT_REQS=required REDIS_SSL_CA_CERTS=/path/to/ca_certs.pem +REDIS_SSL_CHECK_HOSTNAME=true REDIS_CLUSTER_MODE=False \ No newline at end of file diff --git a/README.md b/README.md index d66d4b6..7fa3afc 100644 --- a/README.md +++ b/README.md @@ -336,20 +336,21 @@ uvx --from redis-mcp-server@latest redis-mcp-server --help If desired, you can use environment variables. Defaults are provided for all variables. -| Name | Description | Default Value | -|----------------------|-----------------------------------------------------------|---------------| -| `REDIS_HOST` | Redis IP or hostname | `"127.0.0.1"` | -| `REDIS_PORT` | Redis port | `6379` | -| `REDIS_DB` | Database | 0 | -| `REDIS_USERNAME` | Default database username | `"default"` | -| `REDIS_PWD` | Default database password | "" | -| `REDIS_SSL` | Enables or disables SSL/TLS | `False` | -| `REDIS_SSL_CA_PATH` | CA certificate for verifying server | None | -| `REDIS_SSL_KEYFILE` | Client's private key file for client authentication | None | -| `REDIS_SSL_CERTFILE` | Client's certificate file for client authentication | None | -| `REDIS_SSL_CERT_REQS`| Whether the client should verify the server's certificate | `"required"` | -| `REDIS_SSL_CA_CERTS` | Path to the trusted CA certificates file | None | -| `REDIS_CLUSTER_MODE` | Enable Redis Cluster mode | `False` | +| Name | Description | Default Value | +|----------------------------|------------------------------------------------------------------- |---------------| +| `REDIS_HOST` | Redis IP or hostname | `"127.0.0.1"` | +| `REDIS_PORT` | Redis port | `6379` | +| `REDIS_DB` | Database | 0 | +| `REDIS_USERNAME` | Default database username | `"default"` | +| `REDIS_PWD` | Default database password | "" | +| `REDIS_SSL` | Enables or disables SSL/TLS | `False` | +| `REDIS_SSL_CA_PATH` | CA certificate path for verifying server | None | +| `REDIS_SSL_KEYFILE` | Client's private key file for client authentication | None | +| `REDIS_SSL_CERTFILE` | Client's certificate file for client authentication | None | +| `REDIS_SSL_CERT_REQS` | Certificate requirements (none, optional, or required) | `"required"` | +| `REDIS_SSL_CA_CERTS` | Path to the trusted CA certificates file | None | +| `REDIS_SSL_CHECK_HOSTNAME` | Verify SSL certificate hostname (auto-disabled when cert_reqs=none)| `True` | +| `REDIS_CLUSTER_MODE` | Enable Redis Cluster mode | `False` | ### EntraID Authentication for Azure Managed Redis diff --git a/src/common/config.py b/src/common/config.py index 790bd2a..41a889c 100644 --- a/src/common/config.py +++ b/src/common/config.py @@ -27,6 +27,11 @@ "db": int(os.getenv("REDIS_DB", 0)), } +# When ssl_cert_reqs is "none", we should disable hostname checking by default +# This matches the behavior of redis-cli --insecure flag +default_check_hostname = "false" if REDIS_CFG["ssl_cert_reqs"] == "none" else "true" +REDIS_CFG["ssl_check_hostname"] = os.getenv("REDIS_SSL_CHECK_HOSTNAME", default_check_hostname) in ("true", "1", "t") + # Entra ID Authentication Configuration ENTRAID_CFG = { # Authentication flow selection @@ -125,6 +130,8 @@ def parse_redis_uri(uri: str) -> dict: config["ssl_keyfile"] = query_params["ssl_keyfile"][0] if "ssl_certfile" in query_params: config["ssl_certfile"] = query_params["ssl_certfile"][0] + if "ssl_check_hostname" in query_params: + config["ssl_check_hostname"] = query_params["ssl_check_hostname"][0] in ("true", "1", "t") # Handle other parameters. According to https://www.iana.org/assignments/uri-schemes/prov/redis, # The database number to use for the Redis SELECT command comes from diff --git a/src/common/connection.py b/src/common/connection.py index 176a096..9078625 100644 --- a/src/common/connection.py +++ b/src/common/connection.py @@ -48,6 +48,7 @@ def get_connection(cls, decode_responses=True) -> Redis: "ssl_certfile": REDIS_CFG["ssl_certfile"], "ssl_cert_reqs": REDIS_CFG["ssl_cert_reqs"], "ssl_ca_certs": REDIS_CFG["ssl_ca_certs"], + "ssl_check_hostname": REDIS_CFG["ssl_check_hostname"], "decode_responses": decode_responses, "lib_name": f"redis-py(mcp-server_v{__version__})", "max_connections_per_node": 10, @@ -72,6 +73,7 @@ def get_connection(cls, decode_responses=True) -> Redis: "ssl_certfile": REDIS_CFG["ssl_certfile"], "ssl_cert_reqs": REDIS_CFG["ssl_cert_reqs"], "ssl_ca_certs": REDIS_CFG["ssl_ca_certs"], + "ssl_check_hostname": REDIS_CFG["ssl_check_hostname"], "decode_responses": decode_responses, "lib_name": f"redis-py(mcp-server_v{__version__})", "max_connections": 10, diff --git a/tests/test_config.py b/tests/test_config.py index 067253b..9b0d689 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -80,6 +80,22 @@ def test_parse_uri_with_ssl_parameters(self): assert result["ssl_certfile"] == "/cert.pem" assert result["ssl_ca_path"] == "/ca.pem" + def test_parse_uri_with_ssl_check_hostname(self): + """Test parsing URI with ssl_check_hostname query parameter.""" + uri = "rediss://localhost:6379/0?ssl_check_hostname=false" + result = parse_redis_uri(uri) + + assert result["ssl"] is True + assert result["ssl_check_hostname"] is False + + def test_parse_uri_with_ssl_check_hostname_true(self): + """Test parsing URI with ssl_check_hostname set to true.""" + uri = "rediss://localhost:6379/0?ssl_check_hostname=true" + result = parse_redis_uri(uri) + + assert result["ssl"] is True + assert result["ssl_check_hostname"] is True + def test_parse_uri_defaults(self): """Test parsing URI with default values.""" uri = "redis://example.com" @@ -286,3 +302,73 @@ def test_config_from_environment(self, mock_load_dotenv): assert config["port"] == 6380 assert config["ssl"] is True assert config["cluster_mode"] is True + + @patch.dict( + os.environ, + { + "REDIS_SSL": "true", + "REDIS_SSL_CERT_REQS": "none", + }, + ) + @patch("src.common.config.load_dotenv") + def test_ssl_check_hostname_disabled_with_cert_reqs_none(self, mock_load_dotenv): + """Test that ssl_check_hostname is disabled by default when ssl_cert_reqs is none.""" + # Re-import to get fresh config + import importlib + + import src.common.config + + importlib.reload(src.common.config) + + config = src.common.config.REDIS_CFG + + assert config["ssl"] is True + assert config["ssl_cert_reqs"] == "none" + assert config["ssl_check_hostname"] is False + + @patch.dict( + os.environ, + { + "REDIS_SSL": "true", + "REDIS_SSL_CERT_REQS": "required", + }, + ) + @patch("src.common.config.load_dotenv") + def test_ssl_check_hostname_enabled_with_cert_reqs_required(self, mock_load_dotenv): + """Test that ssl_check_hostname is enabled by default when ssl_cert_reqs is required.""" + # Re-import to get fresh config + import importlib + + import src.common.config + + importlib.reload(src.common.config) + + config = src.common.config.REDIS_CFG + + assert config["ssl"] is True + assert config["ssl_cert_reqs"] == "required" + assert config["ssl_check_hostname"] is True + + @patch.dict( + os.environ, + { + "REDIS_SSL": "true", + "REDIS_SSL_CERT_REQS": "none", + "REDIS_SSL_CHECK_HOSTNAME": "true", + }, + ) + @patch("src.common.config.load_dotenv") + def test_ssl_check_hostname_override(self, mock_load_dotenv): + """Test that ssl_check_hostname can be explicitly overridden.""" + # Re-import to get fresh config + import importlib + + import src.common.config + + importlib.reload(src.common.config) + + config = src.common.config.REDIS_CFG + + assert config["ssl"] is True + assert config["ssl_cert_reqs"] == "none" + assert config["ssl_check_hostname"] is True From c4b84f7d098f156371f37c3c739e7f72752d9954 Mon Sep 17 00:00:00 2001 From: Vasil Chomakov Date: Tue, 4 Nov 2025 19:50:55 +0200 Subject: [PATCH 2/3] style: format src/common/config.py --- src/common/config.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/common/config.py b/src/common/config.py index 41a889c..27f6194 100644 --- a/src/common/config.py +++ b/src/common/config.py @@ -30,7 +30,9 @@ # When ssl_cert_reqs is "none", we should disable hostname checking by default # This matches the behavior of redis-cli --insecure flag default_check_hostname = "false" if REDIS_CFG["ssl_cert_reqs"] == "none" else "true" -REDIS_CFG["ssl_check_hostname"] = os.getenv("REDIS_SSL_CHECK_HOSTNAME", default_check_hostname) in ("true", "1", "t") +REDIS_CFG["ssl_check_hostname"] = os.getenv( + "REDIS_SSL_CHECK_HOSTNAME", default_check_hostname +) in ("true", "1", "t") # Entra ID Authentication Configuration ENTRAID_CFG = { @@ -131,7 +133,11 @@ def parse_redis_uri(uri: str) -> dict: if "ssl_certfile" in query_params: config["ssl_certfile"] = query_params["ssl_certfile"][0] if "ssl_check_hostname" in query_params: - config["ssl_check_hostname"] = query_params["ssl_check_hostname"][0] in ("true", "1", "t") + config["ssl_check_hostname"] = query_params["ssl_check_hostname"][0] in ( + "true", + "1", + "t", + ) # Handle other parameters. According to https://www.iana.org/assignments/uri-schemes/prov/redis, # The database number to use for the Redis SELECT command comes from From c7bd0ad98d1758d28755597b330eed8ae25b7cb1 Mon Sep 17 00:00:00 2001 From: Vasil Chomakov Date: Wed, 5 Nov 2025 13:53:48 +0200 Subject: [PATCH 3/3] fix: add ssl_check_hostname to connection test mocks --- tests/test_connection.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_connection.py b/tests/test_connection.py index 14fd936..5b53001 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -40,6 +40,7 @@ def test_get_connection_standalone_mode(self, mock_config, mock_redis_class): "ssl_certfile": None, "ssl_cert_reqs": "required", "ssl_ca_certs": None, + "ssl_check_hostname": True, }[key] mock_redis_instance = Mock() @@ -75,6 +76,7 @@ def test_get_connection_cluster_mode(self, mock_config, mock_cluster_class): "ssl_certfile": "/path/to/cert.pem", "ssl_cert_reqs": "required", "ssl_ca_certs": "/path/to/ca-bundle.pem", + "ssl_check_hostname": True, }[key] mock_cluster_instance = Mock() @@ -114,6 +116,7 @@ def test_get_connection_singleton_behavior(self, mock_config, mock_redis_class): "ssl_certfile": None, "ssl_cert_reqs": "required", "ssl_ca_certs": None, + "ssl_check_hostname": True, }[key] mock_redis_instance = Mock() @@ -148,6 +151,7 @@ def test_get_connection_with_decode_responses_false( "ssl_certfile": None, "ssl_cert_reqs": "required", "ssl_ca_certs": None, + "ssl_check_hostname": True, }[key] mock_redis_instance = Mock() @@ -176,6 +180,7 @@ def test_get_connection_with_ssl_configuration(self, mock_config, mock_redis_cla "ssl_certfile": "/path/to/cert.pem", "ssl_cert_reqs": "optional", "ssl_ca_certs": "/path/to/ca-bundle.pem", + "ssl_check_hostname": True, }[key] mock_redis_instance = Mock() @@ -211,6 +216,7 @@ def test_get_connection_includes_version_in_lib_name( "ssl_certfile": None, "ssl_cert_reqs": "required", "ssl_ca_certs": None, + "ssl_check_hostname": True, }[key] mock_redis_instance = Mock() @@ -241,6 +247,7 @@ def test_connection_error_handling(self, mock_config, mock_redis_class): "ssl_certfile": None, "ssl_cert_reqs": "required", "ssl_ca_certs": None, + "ssl_check_hostname": True, }[key] # Mock Redis constructor to raise ConnectionError @@ -265,6 +272,7 @@ def test_cluster_connection_error_handling(self, mock_config, mock_cluster_class "ssl_certfile": None, "ssl_cert_reqs": "required", "ssl_ca_certs": None, + "ssl_check_hostname": True, }[key] # Mock RedisCluster constructor to raise ConnectionError @@ -305,6 +313,7 @@ def test_connection_parameters_filtering(self, mock_config, mock_redis_class): "ssl_certfile": None, "ssl_cert_reqs": "required", "ssl_ca_certs": None, + "ssl_check_hostname": True, }[key] mock_redis_instance = Mock()