From f18ad391427f580d068c86e66829e3cbca28bbc9 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 24 Jul 2026 10:26:38 -0700 Subject: [PATCH] filer: honor the documented TLS options in every redis store (#10425) The scaffold advertises enable_tls, ca_cert_path, client_cert_path and client_key_path under redis2, redis2_sentinel and redis_cluster2, but only the plain redis2 and redis3 stores ever read them, and under a different name, enable_mtls. Sentinel and cluster setups quietly connected in plaintext. Build the TLS config in one place and use it from all six stores. enable_mtls still works. The CA and the client key pair are optional now, so enable_tls alone verifies against the system roots, and ServerName is left unset so go-redis validates each address the sentinel and cluster clients dial. --- weed/filer/redis2/redis_cluster_store.go | 12 +- weed/filer/redis2/redis_sentinel_store.go | 10 +- weed/filer/redis2/redis_store.go | 58 ++------ weed/filer/redis3/redis_cluster_store.go | 12 +- weed/filer/redis3/redis_sentinel_store.go | 10 +- weed/filer/redis3/redis_store.go | 62 ++------ weed/filer/redis_tls/redis_tls.go | 50 +++++++ weed/filer/redis_tls/redis_tls_test.go | 167 ++++++++++++++++++++++ 8 files changed, 284 insertions(+), 97 deletions(-) create mode 100644 weed/filer/redis_tls/redis_tls.go create mode 100644 weed/filer/redis_tls/redis_tls_test.go diff --git a/weed/filer/redis2/redis_cluster_store.go b/weed/filer/redis2/redis_cluster_store.go index 5e1593e9e..19bc6c6aa 100644 --- a/weed/filer/redis2/redis_cluster_store.go +++ b/weed/filer/redis2/redis_cluster_store.go @@ -1,8 +1,11 @@ package redis2 import ( + "crypto/tls" + "github.com/redis/go-redis/v9" "github.com/seaweedfs/seaweedfs/weed/filer" + "github.com/seaweedfs/seaweedfs/weed/filer/redis_tls" "github.com/seaweedfs/seaweedfs/weed/util" ) @@ -23,6 +26,11 @@ func (store *RedisCluster2Store) Initialize(configuration util.Configuration, pr configuration.SetDefault(prefix+"useReadOnly", false) configuration.SetDefault(prefix+"routeByLatency", false) + tlsConfig, err := redis_tls.Config(configuration, prefix) + if err != nil { + return err + } + return store.initialize( configuration.GetStringSlice(prefix+"addresses"), configuration.GetString(prefix+"username"), @@ -31,16 +39,18 @@ func (store *RedisCluster2Store) Initialize(configuration util.Configuration, pr configuration.GetBool(prefix+"useReadOnly"), configuration.GetBool(prefix+"routeByLatency"), configuration.GetStringSlice(prefix+"superLargeDirectories"), + tlsConfig, ) } -func (store *RedisCluster2Store) initialize(addresses []string, username string, password string, keyPrefix string, readOnly, routeByLatency bool, superLargeDirectories []string) (err error) { +func (store *RedisCluster2Store) initialize(addresses []string, username string, password string, keyPrefix string, readOnly, routeByLatency bool, superLargeDirectories []string, tlsConfig *tls.Config) (err error) { store.Client = redis.NewClusterClient(&redis.ClusterOptions{ Addrs: addresses, Username: username, Password: password, ReadOnly: readOnly, RouteByLatency: routeByLatency, + TLSConfig: tlsConfig, }) store.keyPrefix = keyPrefix store.loadSuperLargeDirectories(superLargeDirectories) diff --git a/weed/filer/redis2/redis_sentinel_store.go b/weed/filer/redis2/redis_sentinel_store.go index 5cadf6a37..f0d1c20fe 100644 --- a/weed/filer/redis2/redis_sentinel_store.go +++ b/weed/filer/redis2/redis_sentinel_store.go @@ -1,10 +1,12 @@ package redis2 import ( + "crypto/tls" "time" "github.com/redis/go-redis/v9" "github.com/seaweedfs/seaweedfs/weed/filer" + "github.com/seaweedfs/seaweedfs/weed/filer/redis_tls" "github.com/seaweedfs/seaweedfs/weed/util" ) @@ -21,6 +23,10 @@ func (store *Redis2SentinelStore) GetName() string { } func (store *Redis2SentinelStore) Initialize(configuration util.Configuration, prefix string) (err error) { + tlsConfig, err := redis_tls.Config(configuration, prefix) + if err != nil { + return err + } return store.initialize( configuration.GetStringSlice(prefix+"addresses"), configuration.GetString(prefix+"masterName"), @@ -30,10 +36,11 @@ func (store *Redis2SentinelStore) Initialize(configuration util.Configuration, p configuration.GetString(prefix+"sentinel_password"), configuration.GetInt(prefix+"database"), configuration.GetString(prefix+"keyPrefix"), + tlsConfig, ) } -func (store *Redis2SentinelStore) initialize(addresses []string, masterName string, username string, password string, sentinelUsername string, sentinelPassword string, database int, keyPrefix string) (err error) { +func (store *Redis2SentinelStore) initialize(addresses []string, masterName string, username string, password string, sentinelUsername string, sentinelPassword string, database int, keyPrefix string, tlsConfig *tls.Config) (err error) { store.Client = redis.NewFailoverClient(&redis.FailoverOptions{ MasterName: masterName, SentinelAddrs: addresses, @@ -42,6 +49,7 @@ func (store *Redis2SentinelStore) initialize(addresses []string, masterName stri SentinelUsername: sentinelUsername, SentinelPassword: sentinelPassword, DB: database, + TLSConfig: tlsConfig, MinRetryBackoff: time.Millisecond * 100, MaxRetryBackoff: time.Minute * 1, ReadTimeout: time.Second * 30, diff --git a/weed/filer/redis2/redis_store.go b/weed/filer/redis2/redis_store.go index 7193699f9..fbbd02d3f 100644 --- a/weed/filer/redis2/redis_store.go +++ b/weed/filer/redis2/redis_store.go @@ -2,13 +2,10 @@ package redis2 import ( "crypto/tls" - "crypto/x509" - "net" - "os" "github.com/redis/go-redis/v9" "github.com/seaweedfs/seaweedfs/weed/filer" - "github.com/seaweedfs/seaweedfs/weed/glog" + "github.com/seaweedfs/seaweedfs/weed/filer/redis_tls" "github.com/seaweedfs/seaweedfs/weed/util" ) @@ -25,6 +22,10 @@ func (store *Redis2Store) GetName() string { } func (store *Redis2Store) Initialize(configuration util.Configuration, prefix string) (err error) { + tlsConfig, err := redis_tls.Config(configuration, prefix) + if err != nil { + return err + } return store.initialize( configuration.GetString(prefix+"address"), configuration.GetString(prefix+"username"), @@ -32,49 +33,18 @@ func (store *Redis2Store) Initialize(configuration util.Configuration, prefix st configuration.GetInt(prefix+"database"), configuration.GetString(prefix+"keyPrefix"), configuration.GetStringSlice(prefix+"superLargeDirectories"), - configuration.GetBool(prefix+"enable_mtls"), - configuration.GetString(prefix+"ca_cert_path"), - configuration.GetString(prefix+"client_cert_path"), - configuration.GetString(prefix+"client_key_path"), + tlsConfig, ) } -func (store *Redis2Store) initialize(hostPort string, username string, password string, database int, keyPrefix string, superLargeDirectories []string, enableMtls bool, caCertPath string, clientCertPath string, clientKeyPath string) (err error) { - opt := &redis.Options{ - Addr: hostPort, - Username: username, - Password: password, - DB: database, - } - if enableMtls { - clientCert, err := tls.LoadX509KeyPair(clientCertPath, clientKeyPath) - if err != nil { - glog.Fatalf("Error loading client certificate and key pair: %v", err) - } - - caCertBytes, err := os.ReadFile(caCertPath) - if err != nil { - glog.Fatalf("Error reading CA certificate file: %v", err) - } - - caCertPool := x509.NewCertPool() - if ok := caCertPool.AppendCertsFromPEM(caCertBytes); !ok { - glog.Fatalf("Error appending CA certificate to pool") - } - - redisHost, _, err := net.SplitHostPort(hostPort) - if err != nil { - glog.Fatalf("Error parsing redis host and port from %s: %v", hostPort, err) - } - - opt.TLSConfig = &tls.Config{ - Certificates: []tls.Certificate{clientCert}, - RootCAs: caCertPool, - ServerName: redisHost, - MinVersion: tls.VersionTLS12, - } - } - store.Client = redis.NewClient(opt) +func (store *Redis2Store) initialize(hostPort string, username string, password string, database int, keyPrefix string, superLargeDirectories []string, tlsConfig *tls.Config) (err error) { + store.Client = redis.NewClient(&redis.Options{ + Addr: hostPort, + Username: username, + Password: password, + DB: database, + TLSConfig: tlsConfig, + }) store.keyPrefix = keyPrefix store.loadSuperLargeDirectories(superLargeDirectories) return diff --git a/weed/filer/redis3/redis_cluster_store.go b/weed/filer/redis3/redis_cluster_store.go index cb8c1896d..0bd6c1ddf 100644 --- a/weed/filer/redis3/redis_cluster_store.go +++ b/weed/filer/redis3/redis_cluster_store.go @@ -1,10 +1,13 @@ package redis3 import ( + "crypto/tls" + "github.com/go-redsync/redsync/v4" "github.com/go-redsync/redsync/v4/redis/goredis/v9" "github.com/redis/go-redis/v9" "github.com/seaweedfs/seaweedfs/weed/filer" + "github.com/seaweedfs/seaweedfs/weed/filer/redis_tls" "github.com/seaweedfs/seaweedfs/weed/util" ) @@ -25,20 +28,27 @@ func (store *RedisCluster3Store) Initialize(configuration util.Configuration, pr configuration.SetDefault(prefix+"useReadOnly", false) configuration.SetDefault(prefix+"routeByLatency", false) + tlsConfig, err := redis_tls.Config(configuration, prefix) + if err != nil { + return err + } + return store.initialize( configuration.GetStringSlice(prefix+"addresses"), configuration.GetString(prefix+"password"), configuration.GetBool(prefix+"useReadOnly"), configuration.GetBool(prefix+"routeByLatency"), + tlsConfig, ) } -func (store *RedisCluster3Store) initialize(addresses []string, password string, readOnly, routeByLatency bool) (err error) { +func (store *RedisCluster3Store) initialize(addresses []string, password string, readOnly, routeByLatency bool, tlsConfig *tls.Config) (err error) { store.Client = redis.NewClusterClient(&redis.ClusterOptions{ Addrs: addresses, Password: password, ReadOnly: readOnly, RouteByLatency: routeByLatency, + TLSConfig: tlsConfig, }) store.redsync = redsync.New(goredis.NewPool(store.Client)) return diff --git a/weed/filer/redis3/redis_sentinel_store.go b/weed/filer/redis3/redis_sentinel_store.go index d066342f7..7dc33599f 100644 --- a/weed/filer/redis3/redis_sentinel_store.go +++ b/weed/filer/redis3/redis_sentinel_store.go @@ -1,12 +1,14 @@ package redis3 import ( + "crypto/tls" "time" "github.com/go-redsync/redsync/v4" "github.com/go-redsync/redsync/v4/redis/goredis/v9" "github.com/redis/go-redis/v9" "github.com/seaweedfs/seaweedfs/weed/filer" + "github.com/seaweedfs/seaweedfs/weed/filer/redis_tls" "github.com/seaweedfs/seaweedfs/weed/util" ) @@ -23,6 +25,10 @@ func (store *Redis3SentinelStore) GetName() string { } func (store *Redis3SentinelStore) Initialize(configuration util.Configuration, prefix string) (err error) { + tlsConfig, err := redis_tls.Config(configuration, prefix) + if err != nil { + return err + } return store.initialize( configuration.GetStringSlice(prefix+"addresses"), configuration.GetString(prefix+"masterName"), @@ -31,10 +37,11 @@ func (store *Redis3SentinelStore) Initialize(configuration util.Configuration, p configuration.GetString(prefix+"sentinel_username"), configuration.GetString(prefix+"sentinel_password"), configuration.GetInt(prefix+"database"), + tlsConfig, ) } -func (store *Redis3SentinelStore) initialize(addresses []string, masterName string, username string, password string, sentinelUsername string, sentinelPassword string, database int) (err error) { +func (store *Redis3SentinelStore) initialize(addresses []string, masterName string, username string, password string, sentinelUsername string, sentinelPassword string, database int, tlsConfig *tls.Config) (err error) { store.Client = redis.NewFailoverClient(&redis.FailoverOptions{ MasterName: masterName, SentinelAddrs: addresses, @@ -43,6 +50,7 @@ func (store *Redis3SentinelStore) initialize(addresses []string, masterName stri SentinelUsername: sentinelUsername, SentinelPassword: sentinelPassword, DB: database, + TLSConfig: tlsConfig, MinRetryBackoff: time.Millisecond * 100, MaxRetryBackoff: time.Minute * 1, ReadTimeout: time.Second * 30, diff --git a/weed/filer/redis3/redis_store.go b/weed/filer/redis3/redis_store.go index 3bb0ce46f..91c8dee18 100644 --- a/weed/filer/redis3/redis_store.go +++ b/weed/filer/redis3/redis_store.go @@ -2,15 +2,12 @@ package redis3 import ( "crypto/tls" - "crypto/x509" - "net" - "os" "github.com/go-redsync/redsync/v4" "github.com/go-redsync/redsync/v4/redis/goredis/v9" "github.com/redis/go-redis/v9" "github.com/seaweedfs/seaweedfs/weed/filer" - "github.com/seaweedfs/seaweedfs/weed/glog" + "github.com/seaweedfs/seaweedfs/weed/filer/redis_tls" "github.com/seaweedfs/seaweedfs/weed/util" ) @@ -27,58 +24,25 @@ func (store *Redis3Store) GetName() string { } func (store *Redis3Store) Initialize(configuration util.Configuration, prefix string) (err error) { + tlsConfig, err := redis_tls.Config(configuration, prefix) + if err != nil { + return err + } return store.initialize( configuration.GetString(prefix+"address"), configuration.GetString(prefix+"password"), configuration.GetInt(prefix+"database"), - configuration.GetBool(prefix+"enable_mtls"), - configuration.GetString(prefix+"ca_cert_path"), - configuration.GetString(prefix+"client_cert_path"), - configuration.GetString(prefix+"client_key_path"), + tlsConfig, ) } -func (store *Redis3Store) initialize(hostPort string, password string, database int, enableMtls bool, caCertPath string, clientCertPath string, clientKeyPath string) (err error) { - if enableMtls { - clientCert, err := tls.LoadX509KeyPair(clientCertPath, clientKeyPath) - if err != nil { - glog.Fatalf("Error loading client certificate and key pair: %v", err) - } - - caCertBytes, err := os.ReadFile(caCertPath) - if err != nil { - glog.Fatalf("Error reading CA certificate file: %v", err) - } - - caCertPool := x509.NewCertPool() - if ok := caCertPool.AppendCertsFromPEM(caCertBytes); !ok { - glog.Fatalf("Error appending CA certificate to pool") - } - - redisHost, _, err := net.SplitHostPort(hostPort) - if err != nil { - glog.Fatalf("Error parsing redis host and port from %s: %v", hostPort, err) - } - - tlsConfig := &tls.Config{ - Certificates: []tls.Certificate{clientCert}, - RootCAs: caCertPool, - ServerName: redisHost, - MinVersion: tls.VersionTLS12, - } - store.Client = redis.NewClient(&redis.Options{ - Addr: hostPort, - Password: password, - DB: database, - TLSConfig: tlsConfig, - }) - } else { - store.Client = redis.NewClient(&redis.Options{ - Addr: hostPort, - Password: password, - DB: database, - }) - } +func (store *Redis3Store) initialize(hostPort string, password string, database int, tlsConfig *tls.Config) (err error) { + store.Client = redis.NewClient(&redis.Options{ + Addr: hostPort, + Password: password, + DB: database, + TLSConfig: tlsConfig, + }) store.redsync = redsync.New(goredis.NewPool(store.Client)) return } diff --git a/weed/filer/redis_tls/redis_tls.go b/weed/filer/redis_tls/redis_tls.go new file mode 100644 index 000000000..4f89f3ab0 --- /dev/null +++ b/weed/filer/redis_tls/redis_tls.go @@ -0,0 +1,50 @@ +// Package redis_tls builds the TLS configuration shared by the redis filer stores. +package redis_tls + +import ( + "crypto/tls" + "crypto/x509" + "fmt" + "os" + + "github.com/seaweedfs/seaweedfs/weed/util" +) + +// Config reads the TLS options of a redis filer store section and returns nil when TLS is off. +// ServerName is left unset so go-redis derives it from each dialed address, which the sentinel +// and cluster stores need since they talk to more than one host. +func Config(configuration util.Configuration, prefix string) (*tls.Config, error) { + + // enable_mtls is the name the redis2 and redis3 stores shipped with + if !configuration.GetBool(prefix+"enable_tls") && !configuration.GetBool(prefix+"enable_mtls") { + return nil, nil + } + + tlsConfig := &tls.Config{ + MinVersion: tls.VersionTLS12, + } + + if caCertPath := configuration.GetString(prefix + "ca_cert_path"); caCertPath != "" { + caCertBytes, err := os.ReadFile(caCertPath) + if err != nil { + return nil, fmt.Errorf("read CA certificate %s: %w", caCertPath, err) + } + caCertPool := x509.NewCertPool() + if !caCertPool.AppendCertsFromPEM(caCertBytes) { + return nil, fmt.Errorf("no CA certificate found in %s", caCertPath) + } + tlsConfig.RootCAs = caCertPool + } + + clientCertPath := configuration.GetString(prefix + "client_cert_path") + clientKeyPath := configuration.GetString(prefix + "client_key_path") + if clientCertPath != "" || clientKeyPath != "" { + clientCert, err := tls.LoadX509KeyPair(clientCertPath, clientKeyPath) + if err != nil { + return nil, fmt.Errorf("load client certificate %s and key %s: %w", clientCertPath, clientKeyPath, err) + } + tlsConfig.Certificates = []tls.Certificate{clientCert} + } + + return tlsConfig, nil +} diff --git a/weed/filer/redis_tls/redis_tls_test.go b/weed/filer/redis_tls/redis_tls_test.go new file mode 100644 index 000000000..75b073883 --- /dev/null +++ b/weed/filer/redis_tls/redis_tls_test.go @@ -0,0 +1,167 @@ +package redis_tls + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "os" + "path/filepath" + "testing" + "time" +) + +type fakeConfiguration map[string]interface{} + +func (c fakeConfiguration) GetString(key string) string { + value, _ := c[key].(string) + return value +} + +func (c fakeConfiguration) GetBool(key string) bool { + value, _ := c[key].(bool) + return value +} + +func (c fakeConfiguration) GetInt(key string) int { + value, _ := c[key].(int) + return value +} + +func (c fakeConfiguration) GetStringSlice(key string) []string { + value, _ := c[key].([]string) + return value +} + +func (c fakeConfiguration) SetDefault(key string, value interface{}) { + if _, found := c[key]; !found { + c[key] = value + } +} + +func TestDisabled(t *testing.T) { + tlsConfig, err := Config(fakeConfiguration{"redis2.ca_cert_path": "/does/not/exist"}, "redis2.") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tlsConfig != nil { + t.Fatalf("expected no TLS config when neither enable_tls nor enable_mtls is set") + } +} + +func TestEnabledWithoutCertificates(t *testing.T) { + for _, key := range []string{"enable_tls", "enable_mtls"} { + tlsConfig, err := Config(fakeConfiguration{"redis2." + key: true}, "redis2.") + if err != nil { + t.Fatalf("%s: unexpected error: %v", key, err) + } + if tlsConfig == nil { + t.Fatalf("%s: expected a TLS config", key) + } + if tlsConfig.MinVersion != tls.VersionTLS12 { + t.Errorf("%s: expected TLS 1.2 minimum, got %x", key, tlsConfig.MinVersion) + } + if tlsConfig.RootCAs != nil { + t.Errorf("%s: expected the system root pool", key) + } + if len(tlsConfig.Certificates) != 0 { + t.Errorf("%s: expected no client certificate", key) + } + if tlsConfig.ServerName != "" { + t.Errorf("%s: expected the server name to come from the dialed address, got %s", key, tlsConfig.ServerName) + } + } +} + +func TestMutualTls(t *testing.T) { + dir := t.TempDir() + certPath, keyPath := writeCertificate(t, dir, "client") + + tlsConfig, err := Config(fakeConfiguration{ + "redis2.enable_tls": true, + "redis2.ca_cert_path": certPath, + "redis2.client_cert_path": certPath, + "redis2.client_key_path": keyPath, + }, "redis2.") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tlsConfig.RootCAs == nil { + t.Errorf("expected the CA certificate to be loaded") + } + if len(tlsConfig.Certificates) != 1 { + t.Errorf("expected the client certificate to be loaded, got %d", len(tlsConfig.Certificates)) + } +} + +func TestUnreadableCaCertificate(t *testing.T) { + if _, err := Config(fakeConfiguration{ + "redis2.enable_tls": true, + "redis2.ca_cert_path": filepath.Join(t.TempDir(), "missing.pem"), + }, "redis2."); err == nil { + t.Fatalf("expected an error for a missing CA certificate") + } +} + +func TestCaCertificateWithoutPem(t *testing.T) { + path := filepath.Join(t.TempDir(), "ca.pem") + if err := os.WriteFile(path, []byte("not a certificate"), 0600); err != nil { + t.Fatal(err) + } + if _, err := Config(fakeConfiguration{ + "redis2.enable_tls": true, + "redis2.ca_cert_path": path, + }, "redis2."); err == nil { + t.Fatalf("expected an error for a CA file without any certificate") + } +} + +func TestClientCertificateWithoutKey(t *testing.T) { + certPath, _ := writeCertificate(t, t.TempDir(), "client") + if _, err := Config(fakeConfiguration{ + "redis2.enable_tls": true, + "redis2.client_cert_path": certPath, + }, "redis2."); err == nil { + t.Fatalf("expected an error for a client certificate without a key") + } +} + +func writeCertificate(t *testing.T, dir, name string) (certPath, keyPath string) { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: name}, + NotBefore: time.Unix(0, 0), + NotAfter: time.Unix(1<<31-1, 0), + IsCA: true, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature, + BasicConstraintsValid: true, + } + certBytes, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + keyBytes, err := x509.MarshalECPrivateKey(key) + if err != nil { + t.Fatal(err) + } + + certPath = filepath.Join(dir, name+".crt") + keyPath = filepath.Join(dir, name+".key") + if err := os.WriteFile(certPath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certBytes}), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(keyPath, pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyBytes}), 0600); err != nil { + t.Fatal(err) + } + return certPath, keyPath +}