Skip to content

Commit a070b72

Browse files
ofekshenawaofekshenawa
committed
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.
1 parent 85cfa2d commit a070b72

File tree

4 files changed

+39
-18
lines changed

4 files changed

+39
-18
lines changed

‎options.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -602,22 +602,28 @@ func setupConnParams(u *url.URL, o *Options) (*Options, error) {
602602
if minVer < 0 || minVer > 65535 {
603603
return nil, fmt.Errorf("redis: invalid tls_min_version: %d (must be between 0 and 65535)", minVer)
604604
}
605-
// Enforce minimum TLS 1.2 for security
606-
if minVer > 0 && minVer < int(tls.VersionTLS12) {
605+
// Handle TLS version setting securely
606+
if minVer == 0 {
607+
// Don't set MinVersion, let Go use its secure default
608+
} else if minVer < int(tls.VersionTLS12) {
607609
return nil, fmt.Errorf("redis: tls_min_version %d is insecure (minimum allowed is TLS 1.2: %d)", minVer, tls.VersionTLS12)
610+
} else {
611+
o.TLSConfig.MinVersion = uint16(minVer)
608612
}
609-
o.TLSConfig.MinVersion = uint16(minVer)
610613
}
611614
if q.has("tls_max_version") {
612615
maxVer := q.int("tls_max_version")
613616
if maxVer < 0 || maxVer > 65535 {
614617
return nil, fmt.Errorf("redis: invalid tls_max_version: %d (must be between 0 and 65535)", maxVer)
615618
}
616-
// Ensure max version is not lower than TLS 1.2
617-
if maxVer > 0 && maxVer < int(tls.VersionTLS12) {
619+
// Handle TLS max version setting securely
620+
if maxVer == 0 {
621+
// Don't set MaxVersion, let Go use its secure default
622+
} else if maxVer < int(tls.VersionTLS12) {
618623
return nil, fmt.Errorf("redis: tls_max_version %d is insecure (minimum allowed is TLS 1.2: %d)", maxVer, tls.VersionTLS12)
624+
} else {
625+
o.TLSConfig.MaxVersion = uint16(maxVer)
619626
}
620-
o.TLSConfig.MaxVersion = uint16(maxVer)
621627
}
622628

623629
tlsServerName := q.string("tls_server_name")

‎options_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA==
7777
}, {
7878
url: "rediss://localhost:123?tls_min_version=70000",
7979
err: errors.New("redis: invalid tls_min_version: 70000 (must be between 0 and 65535)"),
80+
}, {
81+
url: "rediss://localhost:123?tls_min_version=0",
82+
o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12}},
8083
}, {
8184
url: "rediss://localhost:123/?skip_verify=true",
8285
o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12, InsecureSkipVerify: true}},

‎osscluster.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -329,22 +329,28 @@ func setupClusterQueryParams(u *url.URL, o *ClusterOptions) (*ClusterOptions, er
329329
if minVer < 0 || minVer > 65535 {
330330
return nil, fmt.Errorf("redis: invalid tls_min_version: %d (must be between 0 and 65535)", minVer)
331331
}
332-
// Enforce minimum TLS 1.2 for security
333-
if minVer > 0 && minVer < int(tls.VersionTLS12) {
332+
// Handle TLS version setting securely
333+
if minVer == 0 {
334+
// Don't set MinVersion, let Go use its secure default
335+
} else if minVer < int(tls.VersionTLS12) {
334336
return nil, fmt.Errorf("redis: tls_min_version %d is insecure (minimum allowed is TLS 1.2: %d)", minVer, tls.VersionTLS12)
337+
} else {
338+
o.TLSConfig.MinVersion = uint16(minVer)
335339
}
336-
o.TLSConfig.MinVersion = uint16(minVer)
337340
}
338341
if q.has("tls_max_version") {
339342
maxVer := q.int("tls_max_version")
340343
if maxVer < 0 || maxVer > 65535 {
341344
return nil, fmt.Errorf("redis: invalid tls_max_version: %d (must be between 0 and 65535)", maxVer)
342345
}
343-
// Ensure max version is not lower than TLS 1.2
344-
if maxVer > 0 && maxVer < int(tls.VersionTLS12) {
346+
// Handle TLS max version setting securely
347+
if maxVer == 0 {
348+
// Don't set MaxVersion, let Go use its secure default
349+
} else if maxVer < int(tls.VersionTLS12) {
345350
return nil, fmt.Errorf("redis: tls_max_version %d is insecure (minimum allowed is TLS 1.2: %d)", maxVer, tls.VersionTLS12)
351+
} else {
352+
o.TLSConfig.MaxVersion = uint16(maxVer)
346353
}
347-
o.TLSConfig.MaxVersion = uint16(maxVer)
348354
}
349355

350356
tlsServerName := q.string("tls_server_name")

‎sentinel.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -439,22 +439,28 @@ func setupFailoverConnParams(u *url.URL, o *FailoverOptions) (*FailoverOptions,
439439
if minVer < 0 || minVer > 65535 {
440440
return nil, fmt.Errorf("redis: invalid tls_min_version: %d (must be between 0 and 65535)", minVer)
441441
}
442-
// Enforce minimum TLS 1.2 for security
443-
if minVer > 0 && minVer < int(tls.VersionTLS12) {
442+
// Handle TLS version setting securely
443+
if minVer == 0 {
444+
// Don't set MinVersion, let Go use its secure default
445+
} else if minVer < int(tls.VersionTLS12) {
444446
return nil, fmt.Errorf("redis: tls_min_version %d is insecure (minimum allowed is TLS 1.2: %d)", minVer, tls.VersionTLS12)
447+
} else {
448+
o.TLSConfig.MinVersion = uint16(minVer)
445449
}
446-
o.TLSConfig.MinVersion = uint16(minVer)
447450
}
448451
if q.has("tls_max_version") {
449452
maxVer := q.int("tls_max_version")
450453
if maxVer < 0 || maxVer > 65535 {
451454
return nil, fmt.Errorf("redis: invalid tls_max_version: %d (must be between 0 and 65535)", maxVer)
452455
}
453-
// Ensure max version is not lower than TLS 1.2
454-
if maxVer > 0 && maxVer < int(tls.VersionTLS12) {
456+
// Handle TLS max version setting securely
457+
if maxVer == 0 {
458+
// Don't set MaxVersion, let Go use its secure default
459+
} else if maxVer < int(tls.VersionTLS12) {
455460
return nil, fmt.Errorf("redis: tls_max_version %d is insecure (minimum allowed is TLS 1.2: %d)", maxVer, tls.VersionTLS12)
461+
} else {
462+
o.TLSConfig.MaxVersion = uint16(maxVer)
456463
}
457-
o.TLSConfig.MaxVersion = uint16(maxVer)
458464
}
459465

460466
tlsServerName := q.string("tls_server_name")

0 commit comments

Comments
 (0)