Skip to content

Remove command-line password usage #1574

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Remove command-line password usage #1574

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
1 change: 1 addition & 0 deletions charts/redis-operator/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]"` | |
Expand Down
2 changes: 2 additions & 0 deletions charts/redis-operator/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 24 additions & 0 deletions docs/content/en/docs/Advance Configuration/Feature Gates/_index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions internal/cmd/manager/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions internal/features/features.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
84 changes: 31 additions & 53 deletions internal/k8sutils/cluster-scaling.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)...)

Expand Down Expand Up @@ -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",
Expand All @@ -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")...)

Expand Down Expand Up @@ -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")...)

Expand All @@ -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")...)

Expand Down Expand Up @@ -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)
Expand All @@ -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")
}
Expand Down Expand Up @@ -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,
Expand All @@ -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")
Expand Down
96 changes: 75 additions & 21 deletions internal/k8sutils/redis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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")
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
Loading