From a070b72dfd915878c4bf221a5ed26becf15add2c Mon Sep 17 00:00:00 2001 From: ofekshenawa Date: Thu, 14 Aug 2025 11:20:18 +0300 Subject: [PATCH] security: fix remaining CodeQL insecure TLS configuration alerts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the final 3 CodeQL security alerts for 'Insecure TLS configuration': **Root Cause**: CodeQL detected that setting or would result in , which is insecure (TLS version 0). **Security Fix**: - When or is specified, don't set the TLS version at all - let Go use its secure defaults - Only set explicit TLS versions when they are >= TLS 1.2 (secure) - Applied fix consistently across all client types **Files Fixed**: - options.go (lines 609, 620) - Single client - osscluster.go (lines 336, 350) - Cluster client - sentinel.go (lines 446, 460) - Sentinel client **Security Behavior**: - → Don't set MinVersion (Go default: secure) - → Error: insecure, minimum TLS 1.2 required - → Set explicit secure version - Same logic applies to **Test Coverage**: - Added test case for behavior - Verified all security validation tests pass - Confirmed no regression in functionality This resolves all remaining CodeQL security alerts while maintaining secure defaults and clear error messages for insecure configurations. --- options.go | 18 ++++++++++++------ options_test.go | 3 +++ osscluster.go | 18 ++++++++++++------ sentinel.go | 18 ++++++++++++------ 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/options.go b/options.go index fc380638..f6982c28 100644 --- a/options.go +++ b/options.go @@ -602,22 +602,28 @@ func setupConnParams(u *url.URL, o *Options) (*Options, error) { if minVer < 0 || minVer > 65535 { return nil, fmt.Errorf("redis: invalid tls_min_version: %d (must be between 0 and 65535)", minVer) } - // Enforce minimum TLS 1.2 for security - if minVer > 0 && minVer < int(tls.VersionTLS12) { + // Handle TLS version setting securely + if minVer == 0 { + // Don't set MinVersion, let Go use its secure default + } else if minVer < int(tls.VersionTLS12) { return nil, fmt.Errorf("redis: tls_min_version %d is insecure (minimum allowed is TLS 1.2: %d)", minVer, tls.VersionTLS12) + } else { + o.TLSConfig.MinVersion = uint16(minVer) } - o.TLSConfig.MinVersion = uint16(minVer) } if q.has("tls_max_version") { maxVer := q.int("tls_max_version") if maxVer < 0 || maxVer > 65535 { return nil, fmt.Errorf("redis: invalid tls_max_version: %d (must be between 0 and 65535)", maxVer) } - // Ensure max version is not lower than TLS 1.2 - if maxVer > 0 && maxVer < int(tls.VersionTLS12) { + // Handle TLS max version setting securely + if maxVer == 0 { + // Don't set MaxVersion, let Go use its secure default + } else if maxVer < int(tls.VersionTLS12) { return nil, fmt.Errorf("redis: tls_max_version %d is insecure (minimum allowed is TLS 1.2: %d)", maxVer, tls.VersionTLS12) + } else { + o.TLSConfig.MaxVersion = uint16(maxVer) } - o.TLSConfig.MaxVersion = uint16(maxVer) } tlsServerName := q.string("tls_server_name") diff --git a/options_test.go b/options_test.go index 97a8f700..5dff8205 100644 --- a/options_test.go +++ b/options_test.go @@ -77,6 +77,9 @@ EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA== }, { url: "rediss://localhost:123?tls_min_version=70000", err: errors.New("redis: invalid tls_min_version: 70000 (must be between 0 and 65535)"), + }, { + url: "rediss://localhost:123?tls_min_version=0", + o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12}}, }, { url: "rediss://localhost:123/?skip_verify=true", o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12, InsecureSkipVerify: true}}, diff --git a/osscluster.go b/osscluster.go index c560d411..6e57bef8 100644 --- a/osscluster.go +++ b/osscluster.go @@ -329,22 +329,28 @@ func setupClusterQueryParams(u *url.URL, o *ClusterOptions) (*ClusterOptions, er if minVer < 0 || minVer > 65535 { return nil, fmt.Errorf("redis: invalid tls_min_version: %d (must be between 0 and 65535)", minVer) } - // Enforce minimum TLS 1.2 for security - if minVer > 0 && minVer < int(tls.VersionTLS12) { + // Handle TLS version setting securely + if minVer == 0 { + // Don't set MinVersion, let Go use its secure default + } else if minVer < int(tls.VersionTLS12) { return nil, fmt.Errorf("redis: tls_min_version %d is insecure (minimum allowed is TLS 1.2: %d)", minVer, tls.VersionTLS12) + } else { + o.TLSConfig.MinVersion = uint16(minVer) } - o.TLSConfig.MinVersion = uint16(minVer) } if q.has("tls_max_version") { maxVer := q.int("tls_max_version") if maxVer < 0 || maxVer > 65535 { return nil, fmt.Errorf("redis: invalid tls_max_version: %d (must be between 0 and 65535)", maxVer) } - // Ensure max version is not lower than TLS 1.2 - if maxVer > 0 && maxVer < int(tls.VersionTLS12) { + // Handle TLS max version setting securely + if maxVer == 0 { + // Don't set MaxVersion, let Go use its secure default + } else if maxVer < int(tls.VersionTLS12) { return nil, fmt.Errorf("redis: tls_max_version %d is insecure (minimum allowed is TLS 1.2: %d)", maxVer, tls.VersionTLS12) + } else { + o.TLSConfig.MaxVersion = uint16(maxVer) } - o.TLSConfig.MaxVersion = uint16(maxVer) } tlsServerName := q.string("tls_server_name") diff --git a/sentinel.go b/sentinel.go index e7005ded..e672d9c8 100644 --- a/sentinel.go +++ b/sentinel.go @@ -439,22 +439,28 @@ func setupFailoverConnParams(u *url.URL, o *FailoverOptions) (*FailoverOptions, if minVer < 0 || minVer > 65535 { return nil, fmt.Errorf("redis: invalid tls_min_version: %d (must be between 0 and 65535)", minVer) } - // Enforce minimum TLS 1.2 for security - if minVer > 0 && minVer < int(tls.VersionTLS12) { + // Handle TLS version setting securely + if minVer == 0 { + // Don't set MinVersion, let Go use its secure default + } else if minVer < int(tls.VersionTLS12) { return nil, fmt.Errorf("redis: tls_min_version %d is insecure (minimum allowed is TLS 1.2: %d)", minVer, tls.VersionTLS12) + } else { + o.TLSConfig.MinVersion = uint16(minVer) } - o.TLSConfig.MinVersion = uint16(minVer) } if q.has("tls_max_version") { maxVer := q.int("tls_max_version") if maxVer < 0 || maxVer > 65535 { return nil, fmt.Errorf("redis: invalid tls_max_version: %d (must be between 0 and 65535)", maxVer) } - // Ensure max version is not lower than TLS 1.2 - if maxVer > 0 && maxVer < int(tls.VersionTLS12) { + // Handle TLS max version setting securely + if maxVer == 0 { + // Don't set MaxVersion, let Go use its secure default + } else if maxVer < int(tls.VersionTLS12) { return nil, fmt.Errorf("redis: tls_max_version %d is insecure (minimum allowed is TLS 1.2: %d)", maxVer, tls.VersionTLS12) + } else { + o.TLSConfig.MaxVersion = uint16(maxVer) } - o.TLSConfig.MaxVersion = uint16(maxVer) } tlsServerName := q.string("tls_server_name")