Skip to content

feat: add TLS URL parameters for rediss:// URLs #3475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,10 @@
// names will be treated as unknown parameters
// - unknown parameter names will result in an error
// - use "skip_verify=true" to ignore TLS certificate validation
// - for rediss:// URLs, additional TLS parameters are supported:
// - tls_cert_file and tls_key_file: paths to client certificate and key files
// - tls_min_version and tls_max_version: TLS version constraints (minimum TLS 1.2: 771, TLS 1.3: 772)
// - tls_server_name: override server name for certificate validation
//
// Examples:
//
Expand Down Expand Up @@ -575,6 +579,61 @@
} else {
o.ConnMaxLifetime = q.duration("max_conn_age")
}

if u.Scheme == "rediss" {
tlsCertFile := q.string("tls_cert_file")
tlsKeyFile := q.string("tls_key_file")

if (tlsCertFile == "") != (tlsKeyFile == "") {
return nil, fmt.Errorf("redis: tls_cert_file and tls_key_file URL parameters must be both set or both omitted")
}

if tlsCertFile != "" {
cert, certLoadErr := tls.LoadX509KeyPair(tlsCertFile, tlsKeyFile)
if certLoadErr != nil {
return nil, fmt.Errorf("redis: error loading TLS certificate: %w", certLoadErr)
}

o.TLSConfig.Certificates = []tls.Certificate{cert}
}

if q.has("tls_min_version") {
minVer := q.int("tls_min_version")
if minVer < 0 || minVer > 65535 {
return nil, fmt.Errorf("redis: invalid tls_min_version: %d (must be between 0 and 65535)", minVer)
}
// 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)
}
}
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)
}
// 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)
}
}

tlsServerName := q.string("tls_server_name")
if tlsServerName != "" {
// we explicitly check for this query parameter, so we don't overwrite
// the default server name (the hostname of the Redis server) if it's
// not given
o.TLSConfig.ServerName = tlsServerName
}
}
if q.err != nil {
return nil, q.err
}
Expand Down
87 changes: 85 additions & 2 deletions options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,27 @@ import (
)

func TestParseURL(t *testing.T) {
certPem := []byte(`-----BEGIN CERTIFICATE-----
MIIBhTCCASugAwIBAgIQIRi6zePL6mKjOipn+dNuaTAKBggqhkjOPQQDAjASMRAw
DgYDVQQKEwdBY21lIENvMB4XDTE3MTAyMDE5NDMwNloXDTE4MTAyMDE5NDMwNlow
EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABD0d
7VNhbWvZLWPuj/RtHFjvtJBEwOkhbN/BnnE8rnZR8+sbwnc/KhCk3FhnpHZnQz7B
5aETbbIgmuvewdjvSBSjYzBhMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggr
BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdEQQiMCCCDmxvY2FsaG9zdDo1
NDUzgg4xMjcuMC4wLjE6NTQ1MzAKBggqhkjOPQQDAgNIADBFAiEA2zpJEPQyz6/l
Wf86aX6PepsntZv2GYlA5UpabfT2EZICICpJ5h/iI+i341gBmLiAFQOyTDT+/wQc
6MF9+Yw1Yy0t
-----END CERTIFICATE-----`)
keyPem := []byte(`-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIIrYSSNQFaA2Hwf1duRSxKtLYX5CB04fSeQ6tF1aY/PuoAoGCCqGSM49
AwEHoUQDQgAEPR3tU2Fta9ktY+6P9G0cWO+0kETA6SFs38GecTyudlHz6xvCdz8q
EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA==
-----END EC PRIVATE KEY-----`)
testCert, err := tls.X509KeyPair(certPem, keyPem)
if err != nil {
t.Fatal(err)
}

cases := []struct {
url string
o *Options // expected value
Expand All @@ -29,10 +50,39 @@ func TestParseURL(t *testing.T) {
o: &Options{Addr: "12345:6379"},
}, {
url: "rediss://localhost:123",
o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ /* no deep comparison */ }},
o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12}},
}, {
url: "rediss://localhost:123?tls_server_name=abc&tls_min_version=771&tls_max_version=772&skip_verify=true",
o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "abc", MinVersion: 771, MaxVersion: 772, InsecureSkipVerify: true}},
}, {
url: "rediss://localhost:123?tls_cert_file=./testdata/testcert.pem&tls_key_file=./testdata/testkey.pem",
o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12, Certificates: []tls.Certificate{testCert}}},
}, {
url: "rediss://localhost:123?tls_cert_file=./testdata/doesnotexist.pem&tls_key_file=./testdata/testkey.pem",
o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "abc"}},
err: errors.New("redis: error loading TLS certificate: open ./testdata/doesnotexist.pem: no such file or directory"),
}, {
url: "rediss://localhost:123?tls_cert_file=./testdata/testcert.pem",
o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "abc"}},
err: errors.New("redis: tls_cert_file and tls_key_file URL parameters must be both set or both omitted"),
}, {
url: "rediss://localhost:123?tls_key_file=./testdata/testkey.pem",
err: errors.New("redis: tls_cert_file and tls_key_file URL parameters must be both set or both omitted"),
}, {
url: "rediss://localhost:123?tls_min_version=1",
err: errors.New("redis: tls_min_version 1 is insecure (minimum allowed is TLS 1.2: 771)"),
}, {
url: "rediss://localhost:123?tls_max_version=1",
err: errors.New("redis: tls_max_version 1 is insecure (minimum allowed is TLS 1.2: 771)"),
}, {
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{InsecureSkipVerify: true}},
o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12, InsecureSkipVerify: true}},
}, {
url: "redis://:bar@localhost:123",
o: &Options{Addr: "localhost:123", Password: "bar"},
Expand Down Expand Up @@ -197,6 +247,39 @@ func comprareOptions(t *testing.T, actual, expected *Options) {
if actual.ConnMaxLifetime != expected.ConnMaxLifetime {
t.Errorf("ConnMaxLifetime: got %v, expected %v", actual.ConnMaxLifetime, expected.ConnMaxLifetime)
}

if (actual.TLSConfig == nil) != (expected.TLSConfig == nil) {
t.Errorf("TLSConfig nil: got %v, expected %v", actual.TLSConfig == nil, expected.TLSConfig == nil)
}

if (actual.TLSConfig != nil) && (expected.TLSConfig != nil) {
if actual.TLSConfig.MinVersion != expected.TLSConfig.MinVersion {
t.Errorf("TLSConfig.MinVersion: got %v, expected %v", actual.TLSConfig.MinVersion, expected.TLSConfig.MinVersion)
}

if actual.TLSConfig.MaxVersion != expected.TLSConfig.MaxVersion {
t.Errorf("TLSConfig.MaxVersion: got %v, expected %v", actual.TLSConfig.MaxVersion, expected.TLSConfig.MaxVersion)
}

if actual.TLSConfig.ServerName != expected.TLSConfig.ServerName {
t.Errorf("TLSConfig.ServerName: got %v, expected %v", actual.TLSConfig.ServerName, expected.TLSConfig.ServerName)
}

if actual.TLSConfig.InsecureSkipVerify != expected.TLSConfig.InsecureSkipVerify {
t.Errorf("TLSConfig.InsecureSkipVerify: got %v, expected %v", actual.TLSConfig.InsecureSkipVerify, expected.TLSConfig.InsecureSkipVerify)
}

if len(actual.TLSConfig.Certificates) != len(expected.TLSConfig.Certificates) {
t.Errorf("TLSConfig.Certificates: got %v, expected %v", actual.TLSConfig.Certificates, expected.TLSConfig.Certificates)
}

for i, actualCert := range actual.TLSConfig.Certificates {
expectedCert := expected.TLSConfig.Certificates[i]
if !actualCert.Leaf.Equal(expectedCert.Leaf) {
t.Errorf("TLSConfig.Certificates[%d].Leaf: got %v, expected %v", i, actual.TLSConfig.Certificates, expected.TLSConfig.Certificates)
}
}
}
}

// Test ReadTimeout option initialization, including special values -1 and 0.
Expand Down
63 changes: 63 additions & 0 deletions osscluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,11 @@
// URL attributes (scheme, host, userinfo, resp.), query parameters using these
// names will be treated as unknown parameters
// - unknown parameter names will result in an error
// - use "skip_verify=true" to ignore TLS certificate validation
// - for rediss:// URLs, additional TLS parameters are supported:
// - tls_cert_file and tls_key_file: paths to client certificate and key files
// - tls_min_version and tls_max_version: TLS version constraints (minimum TLS 1.2: 771, TLS 1.3: 772)
// - tls_server_name: override server name for certificate validation
//
// Example:
//
Expand Down Expand Up @@ -298,6 +303,64 @@
if q.err != nil {
return nil, q.err
}
if o.TLSConfig != nil && q.has("skip_verify") {
o.TLSConfig.InsecureSkipVerify = q.bool("skip_verify")
}

if u.Scheme == "rediss" {
tlsCertFile := q.string("tls_cert_file")
tlsKeyFile := q.string("tls_key_file")

if (tlsCertFile == "") != (tlsKeyFile == "") {
return nil, fmt.Errorf("redis: tls_cert_file and tls_key_file URL parameters must be both set or both omitted")
}

if tlsCertFile != "" {
cert, certLoadErr := tls.LoadX509KeyPair(tlsCertFile, tlsKeyFile)
if certLoadErr != nil {
return nil, fmt.Errorf("redis: error loading TLS certificate: %w", certLoadErr)
}

o.TLSConfig.Certificates = []tls.Certificate{cert}
}

if q.has("tls_min_version") {
minVer := q.int("tls_min_version")
if minVer < 0 || minVer > 65535 {
return nil, fmt.Errorf("redis: invalid tls_min_version: %d (must be between 0 and 65535)", minVer)
}
// 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)
}
}
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)
}
// 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)
}
}

tlsServerName := q.string("tls_server_name")
if tlsServerName != "" {
// we explicitly check for this query parameter, so we don't overwrite
// the default server name (the hostname of the Redis server) if it's
// not given
o.TLSConfig.ServerName = tlsServerName
}
}

// addr can be specified as many times as needed
addrs := q.strings("addr")
Expand Down
33 changes: 33 additions & 0 deletions osscluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1603,6 +1603,27 @@ var _ = Describe("ClusterClient timeout", func() {
})

var _ = Describe("ClusterClient ParseURL", func() {
certPem := []byte(`-----BEGIN CERTIFICATE-----
MIIBhTCCASugAwIBAgIQIRi6zePL6mKjOipn+dNuaTAKBggqhkjOPQQDAjASMRAw
DgYDVQQKEwdBY21lIENvMB4XDTE3MTAyMDE5NDMwNloXDTE4MTAyMDE5NDMwNlow
EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABD0d
7VNhbWvZLWPuj/RtHFjvtJBEwOkhbN/BnnE8rnZR8+sbwnc/KhCk3FhnpHZnQz7B
5aETbbIgmuvewdjvSBSjYzBhMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggr
BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdEQQiMCCCDmxvY2FsaG9zdDo1
NDUzgg4xMjcuMC4wLjE6NTQ1MzAKBggqhkjOPQQDAgNIADBFAiEA2zpJEPQyz6/l
Wf86aX6PepsntZv2GYlA5UpabfT2EZICICpJ5h/iI+i341gBmLiAFQOyTDT+/wQc
6MF9+Yw1Yy0t
-----END CERTIFICATE-----`)
keyPem := []byte(`-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIIrYSSNQFaA2Hwf1duRSxKtLYX5CB04fSeQ6tF1aY/PuoAoGCCqGSM49
AwEHoUQDQgAEPR3tU2Fta9ktY+6P9G0cWO+0kETA6SFs38GecTyudlHz6xvCdz8q
EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA==
-----END EC PRIVATE KEY-----`)
testCert, err := tls.X509KeyPair(certPem, keyPem)
if err != nil {
panic(err)
}

cases := []struct {
test string
url string
Expand Down Expand Up @@ -1633,6 +1654,18 @@ var _ = Describe("ClusterClient ParseURL", func() {
test: "MultipleRedissURLs",
url: "rediss://localhost:123?addr=localhost:1234&addr=localhost:12345",
o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234", "localhost:12345"}, TLSConfig: &tls.Config{ServerName: "localhost"}},
}, {
test: "RedissTLSParams",
url: "rediss://localhost:123?tls_server_name=abc&tls_min_version=771&tls_max_version=772&skip_verify=true",
o: &redis.ClusterOptions{Addrs: []string{"localhost:123"}, TLSConfig: &tls.Config{ServerName: "abc", MinVersion: 771, MaxVersion: 772, InsecureSkipVerify: true}},
}, {
test: "RedissTLSCert",
url: "rediss://localhost:123?tls_cert_file=./testdata/testcert.pem&tls_key_file=./testdata/testkey.pem",
o: &redis.ClusterOptions{Addrs: []string{"localhost:123"}, TLSConfig: &tls.Config{ServerName: "localhost", Certificates: []tls.Certificate{testCert}}},
}, {
test: "RedissSkipVerify",
url: "rediss://localhost:123?skip_verify=true",
o: &redis.ClusterOptions{Addrs: []string{"localhost:123"}, TLSConfig: &tls.Config{ServerName: "localhost", InsecureSkipVerify: true}},
}, {
test: "OnlyPassword",
url: "redis://:bar@localhost:123",
Expand Down
59 changes: 59 additions & 0 deletions sentinel.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@
// names will be treated as unknown parameters
// - unknown parameter names will result in an error
// - use "skip_verify=true" to ignore TLS certificate validation
// - for rediss:// URLs, additional TLS parameters are supported:
// - tls_cert_file and tls_key_file: paths to client certificate and key files
// - tls_min_version and tls_max_version: TLS version constraints (minimum TLS 1.2: 771, TLS 1.3: 772)
// - tls_server_name: override server name for certificate validation
//
// Example:
//
Expand Down Expand Up @@ -413,6 +417,61 @@
o.TLSConfig.InsecureSkipVerify = q.bool("skip_verify")
}

if u.Scheme == "rediss" {
tlsCertFile := q.string("tls_cert_file")
tlsKeyFile := q.string("tls_key_file")

if (tlsCertFile == "") != (tlsKeyFile == "") {
return nil, fmt.Errorf("redis: tls_cert_file and tls_key_file URL parameters must be both set or both omitted")
}

if tlsCertFile != "" {
cert, certLoadErr := tls.LoadX509KeyPair(tlsCertFile, tlsKeyFile)
if certLoadErr != nil {
return nil, fmt.Errorf("redis: error loading TLS certificate: %w", certLoadErr)
}

o.TLSConfig.Certificates = []tls.Certificate{cert}
}

if q.has("tls_min_version") {
minVer := q.int("tls_min_version")
if minVer < 0 || minVer > 65535 {
return nil, fmt.Errorf("redis: invalid tls_min_version: %d (must be between 0 and 65535)", minVer)
}
// 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)
}
}
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)
}
// 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)
}
}

tlsServerName := q.string("tls_server_name")
if tlsServerName != "" {
// we explicitly check for this query parameter, so we don't overwrite
// the default server name (the hostname of the Redis server) if it's
// not given
o.TLSConfig.ServerName = tlsServerName
}
}

// any parameters left?
if r := q.remaining(); len(r) > 0 {
return nil, fmt.Errorf("redis: unexpected option: %s", strings.Join(r, ", "))
Expand Down
11 changes: 11 additions & 0 deletions testdata/testcert.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-----BEGIN CERTIFICATE-----
MIIBhTCCASugAwIBAgIQIRi6zePL6mKjOipn+dNuaTAKBggqhkjOPQQDAjASMRAw
DgYDVQQKEwdBY21lIENvMB4XDTE3MTAyMDE5NDMwNloXDTE4MTAyMDE5NDMwNlow
EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABD0d
7VNhbWvZLWPuj/RtHFjvtJBEwOkhbN/BnnE8rnZR8+sbwnc/KhCk3FhnpHZnQz7B
5aETbbIgmuvewdjvSBSjYzBhMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggr
BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdEQQiMCCCDmxvY2FsaG9zdDo1
NDUzgg4xMjcuMC4wLjE6NTQ1MzAKBggqhkjOPQQDAgNIADBFAiEA2zpJEPQyz6/l
Wf86aX6PepsntZv2GYlA5UpabfT2EZICICpJ5h/iI+i341gBmLiAFQOyTDT+/wQc
6MF9+Yw1Yy0t
-----END CERTIFICATE-----
5 changes: 5 additions & 0 deletions testdata/testkey.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIIrYSSNQFaA2Hwf1duRSxKtLYX5CB04fSeQ6tF1aY/PuoAoGCCqGSM49
AwEHoUQDQgAEPR3tU2Fta9ktY+6P9G0cWO+0kETA6SFs38GecTyudlHz6xvCdz8q
EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA==
-----END EC PRIVATE KEY-----
Loading