1
0
mirror of https://github.com/redis/go-redis.git synced 2025-09-02 22:01:16 +03:00

security: fix remaining CodeQL insecure TLS configuration alerts

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.
This commit is contained in:
ofekshenawa
2025-08-14 11:20:18 +03:00
parent 85cfa2db7b
commit a070b72dfd
4 changed files with 39 additions and 18 deletions

View File

@@ -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")

View File

@@ -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}},

View File

@@ -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")

View File

@@ -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")