diff --git a/redisconn/client.go b/redisconn/client.go index b0f9d7b..8d07d1c 100644 --- a/redisconn/client.go +++ b/redisconn/client.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "net" + "os" "slices" "time" @@ -51,9 +52,13 @@ func BuildTLSConfig(cfg *TLSConfig) (*tls.Config, error) { MinVersion: tls.VersionTLS12, InsecureSkipVerify: cfg.InsecureSkipVerify, //nolint:gosec // explicitly configurable } - if len(cfg.CACert) > 0 { + caCert, err := loadCACert(cfg) + if err != nil { + return nil, err + } + if len(caCert) > 0 { pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM(cfg.CACert) { + if !pool.AppendCertsFromPEM(caCert) { return nil, errors.New("redis: failed to parse CA certificate PEM data") } tlsCfg.RootCAs = pool @@ -68,6 +73,23 @@ func BuildTLSConfig(cfg *TLSConfig) (*tls.Config, error) { return tlsCfg, nil } +// loadCACert returns the configured CA bundle: CACertFile's contents when set, +// otherwise CACert. An unreadable or empty CACertFile is an error so a +// misconfigured path cannot silently select system roots. +func loadCACert(cfg *TLSConfig) ([]byte, error) { + if cfg.CACertFile == "" { + return cfg.CACert, nil + } + data, err := os.ReadFile(cfg.CACertFile) // #nosec G304 -- path comes from trusted configuration + if err != nil { + return nil, fmt.Errorf("redis: failed to read CA certificate file %q: %w", cfg.CACertFile, err) + } + if len(data) == 0 { + return nil, fmt.Errorf("redis: CA certificate file %q is empty", cfg.CACertFile) + } + return data, nil +} + func buildClient(cfg *Config) (goredis.UniversalClient, error) { switch { case cfg.SentinelConfig != nil: diff --git a/redisconn/client_test.go b/redisconn/client_test.go index 47ad7b8..ff0dec7 100644 --- a/redisconn/client_test.go +++ b/redisconn/client_test.go @@ -14,6 +14,8 @@ import ( "encoding/pem" "math/big" "net" + "os" + "path/filepath" "runtime" "strings" "testing" @@ -294,6 +296,50 @@ func TestBuildTLSConfig(t *testing.T) { } } +func TestBuildTLSConfigCACertFile(t *testing.T) { + t.Parallel() + caPEM, _, _ := testCertificate(t) + dir := t.TempDir() + write := func(name string, data []byte) string { + t.Helper() + path := filepath.Join(dir, name) + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatal(err) + } + return path + } + valid := write("valid.pem", caPEM) + tests := []struct { + name string + cfg *TLSConfig + want string + }{ + {"valid file", &TLSConfig{CACertFile: valid}, ""}, + {"missing file", &TLSConfig{CACertFile: filepath.Join(dir, "missing.pem")}, "failed to read CA certificate file"}, + {"empty file", &TLSConfig{CACertFile: write("empty.pem", nil)}, "is empty"}, + {"malformed file", &TLSConfig{CACertFile: write("bad.pem", []byte("bad"))}, "failed to parse CA"}, + {"file and bytes", &TLSConfig{CACert: caPEM, CACertFile: valid}, "mutually exclusive"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := BuildTLSConfig(tt.cfg) + if tt.want != "" { + if err == nil || !contains(err.Error(), tt.want) { + t.Fatalf("error = %v", err) + } + return + } + if err != nil { + t.Fatal(err) + } + if got.RootCAs == nil { + t.Fatal("CA file did not replace system roots") + } + }) + } +} + func TestSentinelDynamicProviderStaysOnDataNodeOptions(t *testing.T) { t.Parallel() provider := func(context.Context) (string, string, error) { return "user", "token", nil } diff --git a/redisconn/config.go b/redisconn/config.go index d3e76e3..4cfbfd3 100644 --- a/redisconn/config.go +++ b/redisconn/config.go @@ -80,9 +80,16 @@ type SentinelConfig struct { // TLSConfig configures server verification, custom roots, and mutual TLS. type TLSConfig struct { // InsecureSkipVerify disables server certificate verification. - InsecureSkipVerify bool + InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty" yaml:"insecureSkipVerify,omitempty"` // CACert is an optional PEM CA bundle; empty uses system roots. + // It is mutually exclusive with CACertFile. CACert []byte + // CACertFile is an optional path to a PEM CA bundle, read when the TLS + // configuration is built. When set, the bundle replaces the system roots. + // A missing, empty, or malformed file is an error; it never falls back to + // system roots or plaintext. The file is read once, so changes require a + // new client. It is mutually exclusive with CACert. + CACertFile string `json:"caCertFile,omitempty" yaml:"caCertFile,omitempty"` // ClientCert and ClientKey are a PEM mTLS certificate pair. ClientCert []byte ClientKey []byte @@ -153,10 +160,18 @@ func validateDynamicAuth(c *Config) error { return nil } +// validateTLSConfig checks the configuration without touching the filesystem; +// CACertFile is read and checked by BuildTLSConfig. func validateTLSConfig(cfg *TLSConfig) error { - if cfg != nil && (len(cfg.ClientCert) == 0) != (len(cfg.ClientKey) == 0) { + if cfg == nil { + return nil + } + if (len(cfg.ClientCert) == 0) != (len(cfg.ClientKey) == 0) { return errors.New("client certificate and key must be provided together") } + if len(cfg.CACert) > 0 && cfg.CACertFile != "" { + return errors.New("CACert and CACertFile are mutually exclusive") + } return nil }