Remove command-line password usage

Signed-off-by: Michael Mulvanny <[email protected]>

Author: Michael Mulvanny

charts/redis-operator/README.md

@@ -92,6 +92,7 @@ kubectl create secret tls <webhook-server-cert> --key tls.key --cert tls.crt -n
 | certificate.secretName | string | `"webhook-server-cert"` |  |
 | certmanager.apiVersion | string | `"cert-manager.io/v1"` |  |
 | certmanager.enabled | bool | `false` |  |
+| featureGates.AvoidCommandLinePassword | bool | `false` |  |
 | featureGates.GenerateConfigInInitContainer | bool | `false` |  |
 | issuer.create | bool | `true` |  |
 | issuer.email | string | `"[email protected]"` |  |

charts/redis-operator/values.yaml

@@ -106,6 +106,8 @@ securityContext: {}
 
 # Feature gates for alpha/experimental features
 featureGates:
+  # Never execute redis-cli -a <password>, even if authentication cannot succeed without it
+  AvoidCommandLinePassword: false
   # Enable generating Redis configuration using an init container instead of a regular container
   GenerateConfigInInitContainer: false
 

docs/content/en/docs/Advance Configuration/Feature Gates/_index.md

@@ -17,10 +17,34 @@ Feature gates can be configured in the Helm chart values:
 featureGates:
   # Enable generating Redis configuration using an init container instead of a regular container
   GenerateConfigInInitContainer: false
+  # Never execute redis-cli -a <password>, even if authentication cannot succeed without it
+  AvoidCommandLinePassword: false
 ```
 
 ## Available Feature Gates
 
+### AvoidCommandLinePassword
+
+When enabled, Redis Operator will never execute `redis-cli -a <password>`, which can leak passwords. The Operator sets the
+`REDISCLI_AUTH` variable on all Redis pods, so the password does not need to be provided on the command line and it is normally
+safe to turn this on unless you are simultaneously upgrading the operator. This is an alpha feature and may change in future releases.
+
+However, if you upgrade from a version that does not add `REDISCLI_AUTH` to the pods (a behavior introduced in the same version that
+added `AvoidCommandLinePassword`), simultaneously enabling `AvoidCommandLinePassword` will make Redis Operator unable to manage
+your current pods, since `-a <password>` is still needed on them. Hence, to guarantee that the Redis password will never be included
+on a command line, you must either risk an operator downtime or upgrade in two steps:
+
+1. Upgrade to a version that adds `REDISCLI_AUTH` to the pods (which was introduced at the same time as `AvoidCommandLinePassword`).
+2. Turn on `AvoidCommandLinePassword`.
+
+**Default**: `false`
+
+**Usage**:
+```yaml
+featureGates:
+  AvoidCommandLinePassword: true
+```
+
 ### GenerateConfigInInitContainer
 
 When enabled, Redis configuration will be generated using an init container instead of a regular container. This is an alpha feature and may change in future releases.

internal/cmd/manager/cmd.go

@@ -97,8 +97,9 @@ func addFlags(cmd *cobra.Command, opts *managerOptions) {
 	cmd.Flags().BoolVar(&opts.enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.")
 	cmd.Flags().BoolVar(&opts.enableWebhooks, "enable-webhooks", internalenv.IsWebhookEnabled(), "Enable webhooks")
 	cmd.Flags().IntVar(&opts.maxConcurrentReconciles, "max-concurrent-reconciles", 1, "Max concurrent reconciles")
-	cmd.Flags().StringVar(&opts.featureGatesString, "feature-gates", internalenv.GetFeatureGates(), "A set of key=value pairs that describe feature gates for alpha/experimental features. "+
-		"Options are:\n  GenerateConfigInInitContainer=true|false: enables using init container for config generation")
+	cmd.Flags().StringVar(&opts.featureGatesString, "feature-gates", internalenv.GetFeatureGates(), "A set of key=value pairs that describe feature gates for alpha/experimental features. Options are:"+
+		"\n  GenerateConfigInInitContainer=true|false: enables using init container for config generation"+
+		"\n  AvoidCommandLinePassword=true|false: prevents using -a <password> in redis-cli commands")
 	cmd.Flags().Duration(
 		operator.KubeClientTimeoutMGRFlag,
 		60*time.Second,

internal/features/features.go

@@ -9,12 +9,14 @@ const (
 	// GenerateConfigInInitContainer enables generating Redis configuration using an init container
 	// instead of a regular container
 	GenerateConfigInInitContainer featuregate.Feature = "GenerateConfigInInitContainer"
+	AvoidCommandLinePassword      featuregate.Feature = "AvoidCommandLinePassword"
 )
 
 // DefaultRedisOperatorFeatureGates consists of all known Redis operator feature gates.
 // To add a new feature, define a key for it above and add it here.
 var DefaultRedisOperatorFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	GenerateConfigInInitContainer: {Default: false, PreRelease: featuregate.Alpha},
+	AvoidCommandLinePassword:      {Default: false, PreRelease: featuregate.Alpha},
 }
 
 // MutableFeatureGate is a feature gate that can be dynamically set

internal/k8sutils/cluster-scaling.go

@@ -35,15 +35,11 @@ func ReshardRedisCluster(ctx context.Context, client kubernetes.Interface, cr *r
 	}
 	cmd = []string{"redis-cli", "--cluster", "reshard"}
 	cmd = append(cmd, getEndpoint(ctx, client, cr, transferPOD))
-	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
-		pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
-		if err != nil {
-			log.FromContext(ctx).Error(err, "error in getting redis password")
-			return
-		}
-		cmd = append(cmd, "-a")
-		cmd = append(cmd, pass)
+	authArgs, err := getRedisClusterAuthArgs(ctx, client, cr, transferNodeName)
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Failed to get password authentication arguments")
 	}
+	cmd = append(cmd, authArgs...)
 
 	cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, transferNodeName)...)
 
@@ -129,7 +125,7 @@ func getRedisNodeID(ctx context.Context, client kubernetes.Interface, cr *rcvb2.
 
 // Rebalance the Redis CLuster using the Empty Master Nodes
 func RebalanceRedisClusterEmptyMasters(ctx context.Context, client kubernetes.Interface, cr *rcvb2.RedisCluster) {
-	// cmd = redis-cli --cluster rebalance <redis>:<port> --cluster-use-empty-masters -a <pass>
+	// cmd = redis-cli --cluster rebalance <redis>:<port> --cluster-use-empty-masters
 	var cmd []string
 	pod := RedisDetails{
 		PodName:   cr.Name + "-leader-1",
@@ -138,14 +134,11 @@ func RebalanceRedisClusterEmptyMasters(ctx context.Context, client kubernetes.In
 	cmd = []string{"redis-cli", "--cluster", "rebalance"}
 	cmd = append(cmd, getEndpoint(ctx, client, cr, pod))
 	cmd = append(cmd, "--cluster-use-empty-masters")
-	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
-		pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
-		if err != nil {
-			log.FromContext(ctx).Error(err, "Error in getting redis password")
-		}
-		cmd = append(cmd, "-a")
-		cmd = append(cmd, pass)
+	authArgs, err := getRedisClusterAuthArgs(ctx, client, cr, cr.Name+"-leader-0")
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Failed to get password authentication arguments")
 	}
+	cmd = append(cmd, authArgs...)
 
 	cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, cr.Name+"-leader-0")...)
 
@@ -175,22 +168,19 @@ func CheckIfEmptyMasters(ctx context.Context, client kubernetes.Interface, cr *r
 
 // Rebalance Redis Cluster Would Rebalance the Redis Cluster without using the empty masters
 func RebalanceRedisCluster(ctx context.Context, client kubernetes.Interface, cr *rcvb2.RedisCluster) {
-	// cmd = redis-cli --cluster rebalance <redis>:<port> -a <pass>
+	// cmd = redis-cli --cluster rebalance <redis>:<port>
 	var cmd []string
 	pod := RedisDetails{
 		PodName:   cr.Name + "-leader-1",
 		Namespace: cr.Namespace,
 	}
 	cmd = []string{"redis-cli", "--cluster", "rebalance"}
 	cmd = append(cmd, getEndpoint(ctx, client, cr, pod))
-	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
-		pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
-		if err != nil {
-			log.FromContext(ctx).Error(err, "Error in getting redis password")
-		}
-		cmd = append(cmd, "-a")
-		cmd = append(cmd, pass)
+	authArgs, err := getRedisClusterAuthArgs(ctx, client, cr, cr.Name+"-leader-0")
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Failed to get password authentication arguments")
 	}
+	cmd = append(cmd, authArgs...)
 
 	cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, cr.Name+"-leader-0")...)
 
@@ -211,14 +201,11 @@ func AddRedisNodeToCluster(ctx context.Context, client kubernetes.Interface, cr
 	}
 	cmd = append(cmd, getEndpoint(ctx, client, cr, newPod))
 	cmd = append(cmd, getEndpoint(ctx, client, cr, existingPod))
-	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
-		pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
-		if err != nil {
-			log.FromContext(ctx).Error(err, "Error in getting redis password")
-		}
-		cmd = append(cmd, "-a")
-		cmd = append(cmd, pass)
+	authArgs, err := getRedisClusterAuthArgs(ctx, client, cr, cr.Name+"-leader-0")
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Failed to get password authentication arguments")
 	}
+	cmd = append(cmd, authArgs...)
 
 	cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, cr.Name+"-leader-0")...)
 
@@ -259,14 +246,11 @@ func RemoveRedisFollowerNodesFromCluster(ctx context.Context, client kubernetes.
 
 	cmd = []string{"redis-cli"}
 
-	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
-		pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
-		if err != nil {
-			log.FromContext(ctx).Error(err, "Error in getting redis password")
-		}
-		cmd = append(cmd, "-a")
-		cmd = append(cmd, pass)
+	authArgs, err := getRedisClusterAuthArgs(ctx, client, cr, cr.Name+"-leader-0")
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Failed to get password authentication arguments")
 	}
+	cmd = append(cmd, authArgs...)
 	cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, cr.Name+"-leader-0")...)
 
 	lastLeaderPodNodeID := getRedisNodeID(ctx, client, cr, lastLeaderPod)
@@ -292,14 +276,11 @@ func RemoveRedisNodeFromCluster(ctx context.Context, client kubernetes.Interface
 	cmd := []string{"redis-cli", "--cluster", "del-node"}
 	cmd = append(cmd, getEndpoint(ctx, client, cr, existingPod))
 	cmd = append(cmd, getRedisNodeID(ctx, client, cr, removePod))
-	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
-		pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
-		if err != nil {
-			log.FromContext(ctx).Error(err, "Error in getting redis password")
-		}
-		cmd = append(cmd, "-a")
-		cmd = append(cmd, pass)
+	authArgs, err := getRedisClusterAuthArgs(ctx, client, cr, cr.Name+"-leader-0")
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Failed to get password authentication arguments")
 	}
+	cmd = append(cmd, authArgs...)
 	cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, cr.Name+"-leader-0")...)
 	executeCommand(ctx, client, cr, cmd, cr.Name+"-leader-0")
 }
@@ -333,7 +314,7 @@ func verifyLeaderPodInfo(ctx context.Context, redisClient *redis.Client, podName
 
 func ClusterFailover(ctx context.Context, client kubernetes.Interface, cr *rcvb2.RedisCluster, shardIdx int32) error {
 	slavePodName := cr.Name + "-leader-" + strconv.Itoa(int(shardIdx))
-	// cmd = redis-cli cluster failover  -a <pass>
+	// cmd = redis-cli cluster failover
 	var cmd []string
 	pod := RedisDetails{
 		PodName:   slavePodName,
@@ -344,14 +325,11 @@ func ClusterFailover(ctx context.Context, client kubernetes.Interface, cr *rcvb2
 		return err
 	}
 	cmd = []string{"redis-cli", "-h", host, "-p", port}
-	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
-		pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
-		if err != nil {
-			log.FromContext(ctx).Error(err, "Error in getting redis password")
-		}
-		cmd = append(cmd, "-a")
-		cmd = append(cmd, pass)
+	authArgs, err := getRedisClusterAuthArgs(ctx, client, cr, slavePodName)
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Failed to get password authentication arguments")
 	}
+	cmd = append(cmd, authArgs...)
 
 	cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, slavePodName)...)
 	cmd = append(cmd, "cluster", "failover")

internal/k8sutils/redis.go

@@ -14,6 +14,7 @@ import (
 	rcvb2 "github.com/OT-CONTAINER-KIT/redis-operator/api/rediscluster/v1beta2"
 	rrvb2 "github.com/OT-CONTAINER-KIT/redis-operator/api/redisreplication/v1beta2"
 	common "github.com/OT-CONTAINER-KIT/redis-operator/internal/controller/common"
+	"github.com/OT-CONTAINER-KIT/redis-operator/internal/features"
 	redis "github.com/redis/go-redis/v9"
 	"github.com/samber/lo"
 	corev1 "k8s.io/api/core/v1"
@@ -122,6 +123,66 @@ func getEndpoint(ctx context.Context, client kubernetes.Interface, cr *rcvb2.Red
 	return host + ":" + strconv.Itoa(port)
 }
 
+// checkRedisCLIAuthInEnv returns true if we can use the pod's REDISCLI_AUTH variable instead of sending redis-cli -a <password>.
+// It checks only variables specified via env[].valueFrom since this is what the operator sets; it does not look at envFrom.
+func checkRedisCLIAuthInEnv(ctx context.Context, client kubernetes.Interface, cr *rcvb2.RedisCluster, podName, secretName, secretKey string) (bool, error) {
+	redisPod, err := client.CoreV1().Pods(cr.Namespace).Get(context.TODO(), podName, metav1.GetOptions{})
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Error checking Redis pod's REDISCLI_AUTH variable", "namespace", cr.Namespace, "podName", podName)
+		return false, err
+	}
+
+	for _, tr := range redisPod.Spec.Containers {
+		if tr.Name == cr.Name+"-leader" {
+			for _, e := range tr.Env {
+				if e.Name != "REDISCLI_AUTH" {
+					continue
+				}
+
+				if e.ValueFrom == nil || e.ValueFrom.SecretKeyRef == nil {
+					continue
+				}
+
+				if e.ValueFrom.SecretKeyRef.Name != secretName || e.ValueFrom.SecretKeyRef.Key != secretKey {
+					return false, nil
+				}
+
+				return true, nil
+			}
+
+			log.FromContext(ctx).V(1).Info("Leader container not configured with REDISCLI_AUTH", "podName", podName)
+			return false, nil
+		}
+	}
+
+	log.FromContext(ctx).V(1).Info("Leader container not found in pod", "podName", podName)
+	return false, nil
+}
+
+func getRedisClusterAuthArgs(ctx context.Context, client kubernetes.Interface, cr *rcvb2.RedisCluster, podName string) ([]string, error) {
+	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
+		passwordInEnv, err := checkRedisCLIAuthInEnv(ctx, client, cr, podName, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
+		if err != nil {
+			return []string{}, fmt.Errorf("error checking pod authentication config: %w", err)
+		}
+
+		if !passwordInEnv {
+			if features.Enabled(features.AvoidCommandLinePassword) {
+				return []string{}, errors.New("refusing to use command-line authentication because AvoidCommandLinePassword is set")
+			}
+
+			pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
+			if err != nil {
+				return []string{}, fmt.Errorf("error getting Redis password: %w", err)
+			}
+
+			return []string{"-a", pass}, nil
+		}
+	}
+
+	return []string{}, nil
+}
+
 // CreateSingleLeaderRedisCommand will create command for single leader cluster creation
 func CreateSingleLeaderRedisCommand(ctx context.Context, cr *rcvb2.RedisCluster) RedisInvocation {
 	cmd := RedisInvocation{
@@ -245,13 +306,12 @@ func ExecuteRedisClusterCommand(ctx context.Context, client kubernetes.Interface
 		cmd = CreateMultipleLeaderRedisCommand(ctx, client, cr)
 	}
 
-	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
-		pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
-		if err != nil {
-			log.FromContext(ctx).Error(err, "Error in getting redis password")
-		}
-		cmd.AddFlag("-a")
-		cmd.AddFlag(pass)
+	authArgs, err := getRedisClusterAuthArgs(ctx, client, cr, cr.Name+"-leader-0")
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Failed to get password authentication arguments")
+	}
+	for _, arg := range authArgs {
+		cmd.AddFlag(arg)
 	}
 	cmd.AddFlag(getRedisTLSArgs(cr.Spec.TLS, cr.Name+"-leader-0")...)
 	executeCommand(ctx, client, cr, cmd.Args(), cr.Name+"-leader-0")
@@ -274,14 +334,11 @@ func createRedisReplicationCommand(ctx context.Context, client kubernetes.Interf
 	cmd = append(cmd, getEndpoint(ctx, client, cr, followerPod))
 	cmd = append(cmd, getEndpoint(ctx, client, cr, leaderPod))
 	cmd = append(cmd, "--cluster-slave")
-	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
-		pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
-		if err != nil {
-			log.FromContext(ctx).Error(err, "Failed to retrieve Redis password", "Secret", *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name)
-		} else {
-			cmd = append(cmd, "-a", pass)
-		}
+	authArgs, err := getRedisClusterAuthArgs(ctx, client, cr, leaderPod.PodName)
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Failed to get password authentication arguments")
 	}
+	cmd = append(cmd, authArgs...)
 	cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, leaderPod.PodName)...)
 	return cmd
 }
@@ -444,14 +501,11 @@ func RedisClusterStatusHealth(ctx context.Context, client kubernetes.Interface,
 	defer redisClient.Close()
 
 	cmd := []string{"redis-cli", "--cluster", "check", fmt.Sprintf("127.0.0.1:%d", *cr.Spec.Port)}
-	if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
-		pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
-		if err != nil {
-			log.FromContext(ctx).Error(err, "Error in getting redis password")
-		}
-		cmd = append(cmd, "-a")
-		cmd = append(cmd, pass)
+	authArgs, err := getRedisClusterAuthArgs(ctx, client, cr, cr.Name+"-leader-0")
+	if err != nil {
+		log.FromContext(ctx).Error(err, "Failed to get password authentication arguments")
 	}
+	cmd = append(cmd, authArgs...)
 	cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, cr.Name+"-leader-0")...)
 	out, err := executeCommand1(ctx, client, cr, cmd, cr.Name+"-leader-0")
 	if err != nil {