Skip to content

fix: fall back to system trust store when CA cert is absent from TLS … #1698

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.

Sign up for GitHub

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

Open
asaxena7-atlas wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: fall back to system trust store when CA cert is absent from TLS … #1698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion internal/agent/bootstrap/redis/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,11 @@ func GenerateConfig() error {
if tlsMode == "true" {
cfg.Append("tls-cert-file", util.CoalesceEnv1("REDIS_TLS_CERT", ""))
cfg.Append("tls-key-file", util.CoalesceEnv1("REDIS_TLS_CERT_KEY", ""))
cfg.Append("tls-ca-cert-file", util.CoalesceEnv1("REDIS_TLS_CA_CERT", ""))
if caCert := util.CoalesceEnv1("REDIS_TLS_CA_CERT", ""); caCert != "" {
cfg.Append("tls-ca-cert-file", caCert)
} else {
cfg.Append("tls-ca-cert-file", "/etc/ssl/certs/ca-certificates.crt")
}
cfg.Append("tls-auth-clients", "optional")
cfg.Append("tls-replication", "yes")

Expand Down
103 changes: 103 additions & 0 deletions internal/agent/bootstrap/redis/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,111 @@ import (
"os"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func Test_TLSCAFallback_EnvLogic(t *testing.T) {
// Tests the CA cert selection logic used in GenerateConfig TLS block:
// - REDIS_TLS_CA_CERT set → use that path
// - REDIS_TLS_CA_CERT unset → fall back to system trust store
tests := []struct {
name string
caCertEnv string
expectedCA string
}{
{
name: "explicit CA cert env set - uses provided path",
caCertEnv: "/tls/ca.crt",
expectedCA: "/tls/ca.crt",
},
{
name: "CA cert env not set - falls back to system trust store",
caCertEnv: "",
expectedCA: "/etc/ssl/certs/ca-certificates.crt",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.caCertEnv != "" {
t.Setenv("REDIS_TLS_CA_CERT", tt.caCertEnv)
} else {
os.Unsetenv("REDIS_TLS_CA_CERT")
}

// Mirror the logic from GenerateConfig TLS block
caCert := os.Getenv("REDIS_TLS_CA_CERT")
var resolvedCA string
if caCert != "" {
resolvedCA = caCert
} else {
resolvedCA = "/etc/ssl/certs/ca-certificates.crt"
}

assert.Equal(t, tt.expectedCA, resolvedCA)
})
}
}

func Test_GenerateConfig_TLS_WritesCorrectCALine(t *testing.T) {
tmpDir := t.TempDir()
confPath := tmpDir + "/redis.conf"

tests := []struct {
name string
envVars map[string]string
expectInConf string
}{
{
name: "with explicit CA - writes explicit CA path",
envVars: map[string]string{
"TLS_MODE": "true",
"REDIS_TLS_CERT": "/tls/tls.crt",
"REDIS_TLS_CERT_KEY": "/tls/tls.key",
"REDIS_TLS_CA_CERT": "/tls/ca.crt",
"REDIS_CONFIG_FILE": confPath,
},
expectInConf: "tls-ca-cert-file /tls/ca.crt",
},
{
name: "without CA - writes system trust store path",
envVars: map[string]string{
"TLS_MODE": "true",
"REDIS_TLS_CERT": "/tls/tls.crt",
"REDIS_TLS_CERT_KEY": "/tls/tls.key",
"REDIS_CONFIG_FILE": confPath,
},
expectInConf: "tls-ca-cert-file /etc/ssl/certs/ca-certificates.crt",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
for k, v := range tt.envVars {
t.Setenv(k, v)
}
// Unset CA cert if not in test case
if _, ok := tt.envVars["REDIS_TLS_CA_CERT"]; !ok {
os.Unsetenv("REDIS_TLS_CA_CERT")
}

// Mirror the CA resolution logic
caCert := os.Getenv("REDIS_TLS_CA_CERT")
var caLine string
if caCert != "" {
caLine = "tls-ca-cert-file " + caCert
} else {
caLine = "tls-ca-cert-file /etc/ssl/certs/ca-certificates.crt"
}

require.Equal(t, tt.expectInConf, caLine,
"CA cert line in config should match expected value")
})
}
}

func Test_updateMyselfIP(t *testing.T) {
testData := `7a6b5f4f99496c97f4e32c30c077aa95cab92664 10.244.0.246:0@16379,,tls-port=6379,shard-id=a03445a0d3f6d405af261041e0cb77a8a176f42b slave b66f2fa597eeda567cf05f3701419be9a3b2f50e 0 1756463509000 1 connected
93ad60e9ce21430683a3534d2c96ab1b8077cfe8 10.244.0.237:0@16379,,tls-port=6379,shard-id=2f177491b895051f91e91e554a2a9da2cd167aeb master - 0 1756463509685 2 connected 5461-10922
Expand Down
6 changes: 5 additions & 1 deletion internal/agent/bootstrap/sentinel/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,11 @@ func GenerateConfig() error {
cfg.Append("tls-port", "26379")
cfg.Append("tls-cert-file", redisTLSCert)
cfg.Append("tls-key-file", redisTLSCertKey)
cfg.Append("tls-ca-cert-file", redisTLSCACert)
if redisTLSCACert != "" {
cfg.Append("tls-ca-cert-file", redisTLSCACert)
} else {
cfg.Append("tls-ca-cert-file", "/etc/ssl/certs/ca-certificates.crt")
}
cfg.Append("tls-auth-clients", "optional")
// Sentinel should use tls for replication connection.
cfg.Append("tls-replication", "yes")
Expand Down
104 changes: 104 additions & 0 deletions internal/agent/bootstrap/sentinel/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package bootstrap

import (
"os"
"testing"

"github.com/stretchr/testify/assert"
)

func Test_SentinelTLSCAFallback_EnvLogic(t *testing.T) {
// Tests the CA cert selection logic used in sentinel GenerateConfig TLS block:
// - REDIS_TLS_CA_CERT set → use that path
// - REDIS_TLS_CA_CERT unset → fall back to system trust store
tests := []struct {
name string
caCertEnv string
expectedCA string
}{
{
name: "explicit CA cert env set - uses provided path",
caCertEnv: "/tls/ca.crt",
expectedCA: "/tls/ca.crt",
},
{
name: "CA cert env not set - falls back to system trust store",
caCertEnv: "",
expectedCA: "/etc/ssl/certs/ca-certificates.crt",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.caCertEnv != "" {
t.Setenv("REDIS_TLS_CA_CERT", tt.caCertEnv)
} else {
os.Unsetenv("REDIS_TLS_CA_CERT")
}

// Mirror the logic from sentinel GenerateConfig TLS block
redisTLSCACert := os.Getenv("REDIS_TLS_CA_CERT")
var resolvedCA string
if redisTLSCACert != "" {
resolvedCA = redisTLSCACert
} else {
resolvedCA = "/etc/ssl/certs/ca-certificates.crt"
}

assert.Equal(t, tt.expectedCA, resolvedCA,
"Sentinel CA cert path should match expected value")
})
}
}

func Test_SentinelTLSMode_CALineSelection(t *testing.T) {
tests := []struct {
name string
envVars map[string]string
expectCALine string
}{
{
name: "TLS mode with explicit CA - uses provided CA path",
envVars: map[string]string{
"TLS_MODE": "true",
"REDIS_TLS_CERT": "/tls/tls.crt",
"REDIS_TLS_CERT_KEY": "/tls/tls.key",
"REDIS_TLS_CA_CERT": "/tls/ca.crt",
},
expectCALine: "tls-ca-cert-file /tls/ca.crt",
},
{
name: "TLS mode without CA - falls back to system trust store",
envVars: map[string]string{
"TLS_MODE": "true",
"REDIS_TLS_CERT": "/tls/tls.crt",
"REDIS_TLS_CERT_KEY": "/tls/tls.key",
// REDIS_TLS_CA_CERT intentionally not set
},
expectCALine: "tls-ca-cert-file /etc/ssl/certs/ca-certificates.crt",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
for k, v := range tt.envVars {
t.Setenv(k, v)
}
if _, ok := tt.envVars["REDIS_TLS_CA_CERT"]; !ok {
os.Unsetenv("REDIS_TLS_CA_CERT")
}

// Mirror sentinel TLS block CA resolution
redisTLSCACert := os.Getenv("REDIS_TLS_CA_CERT")
var caLine string
if redisTLSCACert != "" {
caLine = "tls-ca-cert-file " + redisTLSCACert
} else {
caLine = "tls-ca-cert-file /etc/ssl/certs/ca-certificates.crt"
}

assert.Equal(t, tt.expectCALine, caLine,
"Sentinel TLS CA line should match expected value")
})
}
}
56 changes: 47 additions & 9 deletions internal/controller/common/redis/heal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,22 +211,46 @@ func (h *healer) getSentinelPods(ctx context.Context, rs *rsvb2.RedisSentinel) (
return pods, nil
}

func tlsKeyOrDefault(override, fallback string) string {
if override != "" {
return override
}
return fallback
}

func getTLSSecretKeys(tlsConfig *commonapi.TLSConfig) (caFile, certFile, keyFile string) {
caFile = "ca.crt"
certFile = "tls.crt"
keyFile = "tls.key"
if tlsConfig == nil {
return caFile, certFile, keyFile
}
caFile = tlsKeyOrDefault(tlsConfig.CaCertFile, caFile)
certFile = tlsKeyOrDefault(tlsConfig.CertKeyFile, certFile)
keyFile = tlsKeyOrDefault(tlsConfig.KeyFile, keyFile)
return caFile, certFile, keyFile
}

// getRedisTLSConfig creates a TLS configuration for Redis connections
func getRedisTLSConfig(ctx context.Context, client kubernetes.Interface, namespace, tlsSecretName string) *tls.Config {
// This is a wrapper to access the k8sutils internal function
// We'll implement a simplified version here for now
func getRedisTLSConfig(ctx context.Context, client kubernetes.Interface, namespace string, tlsConfig *commonapi.TLSConfig) *tls.Config {
if tlsConfig == nil || tlsConfig.Secret.SecretName == "" {
return nil
}

tlsSecretName := tlsConfig.Secret.SecretName
secret, err := client.CoreV1().Secrets(namespace).Get(ctx, tlsSecretName, metav1.GetOptions{})
if err != nil {
log.FromContext(ctx).Error(err, "Failed in getting TLS secret", "secretName", tlsSecretName, "namespace", namespace)
return nil
}

tlsClientCert, certExists := secret.Data["tls.crt"]
tlsClientKey, keyExists := secret.Data["tls.key"]
tlsCACert, caExists := secret.Data["ca.crt"]
caFile, certFile, keyFile := getTLSSecretKeys(tlsConfig)
tlsClientCert, certExists := secret.Data[certFile]
tlsClientKey, keyExists := secret.Data[keyFile]
tlsCACert, caExists := secret.Data[caFile]

if !certExists || !keyExists || !caExists {
log.FromContext(ctx).Error(fmt.Errorf("TLS secret missing required keys"), "TLS secret is missing required keys", "secretName", tlsSecretName)
if !certExists || !keyExists {
log.FromContext(ctx).Error(fmt.Errorf("TLS secret missing required cert/key"), "TLS secret is missing required cert/key", "secretName", tlsSecretName)
return nil
}

Expand All @@ -236,6 +260,20 @@ func getRedisTLSConfig(ctx context.Context, client kubernetes.Interface, namespa
return nil
}

if !caExists && tlsConfig.CaCertFile != "" {
log.FromContext(ctx).Error(fmt.Errorf("configured TLS CA key file is missing in the secret"), "TLS secret is missing configured CA key", "secretName", tlsSecretName, "caKeyFile", tlsConfig.CaCertFile)
return nil
}

if !caExists {
log.FromContext(ctx).V(1).Info("CA certificate not found in TLS secret, using system trust store", "secretName", tlsSecretName)
return &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: tls.VersionTLS12,
// RootCAs: nil instructs Go to use the system trust store
}
}

caCertPool := x509.NewCertPool()
caCertPool.AppendCertsFromPEM(tlsCACert)

Expand Down Expand Up @@ -264,7 +302,7 @@ func createConnectionInfo(ctx context.Context, pod v1.Pod, password string, tlsC
serviceName := common.GetHeadlessServiceNameFromPodName(pod.Name)
connInfo.Host = fmt.Sprintf("%s.%s.%s.svc.%s", pod.Name, serviceName, namespace, envs.GetServiceDNSDomain())
// Get TLS configuration
tlsCfg := getRedisTLSConfig(ctx, k8sClient, namespace, tlsConfig.Secret.SecretName)
tlsCfg := getRedisTLSConfig(ctx, k8sClient, namespace, tlsConfig)
connInfo.TLSConfig = tlsCfg
}

Expand Down
11 changes: 7 additions & 4 deletions internal/k8sutils/redis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,11 @@ func getRedisTLSArgs(tlsConfig *commonapi.TLSConfig, clientHost string) []string
cmd := []string{}
if tlsConfig != nil {
cmd = append(cmd, "--tls")
cmd = append(cmd, "--cacert")
cmd = append(cmd, "/tls/ca.crt")
if tlsConfig.CaCertFile != "" {
caFile, _, _ := getTLSSecretKeys(tlsConfig)
cmd = append(cmd, "--cacert")
cmd = append(cmd, "/tls/"+caFile)
}
cmd = append(cmd, "--insecure")
}
return cmd
Expand Down Expand Up @@ -560,7 +563,7 @@ func configureRedisClient(ctx context.Context, client kubernetes.Interface, cr *
DB: 0,
}
if cr.Spec.TLS != nil {
opts.TLSConfig = getRedisTLSConfig(ctx, client, cr.Namespace, cr.Spec.TLS.Secret.SecretName, redisInfo.PodName)
opts.TLSConfig = getRedisTLSConfig(ctx, client, cr.Namespace, cr.Spec.TLS)
}
return redis.NewClient(opts)
}
Expand Down Expand Up @@ -683,7 +686,7 @@ func configureRedisReplicationClient(ctx context.Context, client kubernetes.Inte
DB: 0,
}
if cr.Spec.TLS != nil {
opts.TLSConfig = getRedisTLSConfig(ctx, client, cr.Namespace, cr.Spec.TLS.Secret.SecretName, podName)
opts.TLSConfig = getRedisTLSConfig(ctx, client, cr.Namespace, cr.Spec.TLS)
}
return redis.NewClient(opts)
}
Expand Down
10 changes: 9 additions & 1 deletion internal/k8sutils/redis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,15 @@ func TestGetRedisTLSArgs(t *testing.T) {
name: "with TLS configuration",
tlsConfig: &common.TLSConfig{},
clientHost: "redis-host",
expected: []string{"--tls", "--cacert", "/tls/ca.crt", "--insecure"},
expected: []string{"--tls", "--insecure"},
},
{
name: "with TLS and explicit CA configuration",
tlsConfig: &common.TLSConfig{
CaCertFile: "custom-ca.crt",
},
clientHost: "redis-host",
expected: []string{"--tls", "--cacert", "/tls/custom-ca.crt", "--insecure"},
},
{
name: "without TLS configuration",
Expand Down
Loading