feat: secure redis authentication [CVE-2024-31989] (#1364)

* feat: secure redis authentication Signed-off-by: iam-veeramalla <abhishek.veeramalla@gmail.com> * fix: failing unit tests Signed-off-by: iam-veeramalla <abhishek.veeramalla@gmail.com> * fix: code review comments and liniting issue Signed-off-by: iam-veeramalla <abhishek.veeramalla@gmail.com> * fix: issues with HA pod crashing Signed-off-by: iam-veeramalla <abhishek.veeramalla@gmail.com> * fix: mount password env Signed-off-by: iam-veeramalla <abhishek.veeramalla@gmail.com> --------- Signed-off-by: iam-veeramalla <abhishek.veeramalla@gmail.com>
argoproj-labs · May 22, 2024 · 83f97da · 83f97da
1 parent 5dc1abe
commit 83f97da
Show file tree

Hide file tree

Showing 21 changed files with 650 additions and 29 deletions.
diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
@@ -16,7 +16,7 @@ jobs:
       - name: Run golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.52.2
+          version: v1.56.2
           args: --timeout 5m --exclude SA5011
           only-new-issues: true
 
diff --git a/.golangci.yml b/.golangci.yml
@@ -18,4 +18,4 @@ linters-settings:
   goimports:
     local-prefixes: github.com/argoproj-labs/argocd-operator
 service:
-  golangci-lint-version: 1.52.2
+  golangci-lint-version: 1.56.2
diff --git a/build/redis/haproxy.cfg.tpl b/build/redis/haproxy.cfg.tpl
@@ -24,6 +24,8 @@ backend check_if_redis_is_master_0
 {{- else}}
     tcp-check connect ssl
 {{- end}}
+    tcp-check send "AUTH replace-with-redis-auth"\r\n
+    tcp-check expect string +OK
     tcp-check send PING\r\n
     tcp-check expect string +PONG
     tcp-check send SENTINEL\ get-master-addr-by-name\ argocd\r\n
@@ -48,6 +50,8 @@ backend check_if_redis_is_master_1
 {{- else}}
     tcp-check connect ssl
 {{- end}}
+    tcp-check send "AUTH replace-with-redis-auth"\r\n
+    tcp-check expect string +OK
     tcp-check send PING\r\n
     tcp-check expect string +PONG
     tcp-check send SENTINEL\ get-master-addr-by-name\ argocd\r\n
@@ -72,6 +76,8 @@ backend check_if_redis_is_master_2
 {{- else}}
     tcp-check connect ssl
 {{- end}}
+    tcp-check send "AUTH replace-with-redis-auth"\r\n
+    tcp-check expect string +OK
     tcp-check send PING\r\n
     tcp-check expect string +PONG
     tcp-check send SENTINEL\ get-master-addr-by-name\ argocd\r\n
@@ -102,6 +108,8 @@ backend bk_redis_master
 {{- else}}
     tcp-check connect ssl
 {{- end}}
+    tcp-check send "AUTH replace-with-redis-auth"\r\n
+    tcp-check expect string +OK
     tcp-check send PING\r\n
     tcp-check expect string +PONG
     tcp-check send info\ replication\r\n

diff --git a/build/redis/haproxy_init.sh.tpl b/build/redis/haproxy_init.sh.tpl
@@ -11,11 +11,6 @@ if [ -z "$ANNOUNCE_IP0" ]; then
 fi
 sed -i "s/REPLACE_ANNOUNCE0/$ANNOUNCE_IP0/" "$HAPROXY_CONF"
 
-if [ "${AUTH:-}" ]; then
-    echo "Setting auth values"
-    ESCAPED_AUTH=$(echo "$AUTH" | sed -e 's/[\/&]/\\&/g');
-    sed -i "s/REPLACE_AUTH_SECRET/${ESCAPED_AUTH}/" "$HAPROXY_CONF"
-fi
 for loop in $(seq 1 10); do
     getent hosts {{.ServiceName}}-announce-1 && break
     echo "Waiting for service {{.ServiceName}}-announce-1 to be ready ($loop) ..." && sleep 1
@@ -27,11 +22,6 @@ if [ -z "$ANNOUNCE_IP1" ]; then
 fi
 sed -i "s/REPLACE_ANNOUNCE1/$ANNOUNCE_IP1/" "$HAPROXY_CONF"
 
-if [ "${AUTH:-}" ]; then
-    echo "Setting auth values"
-    ESCAPED_AUTH=$(echo "$AUTH" | sed -e 's/[\/&]/\\&/g');
-    sed -i "s/REPLACE_AUTH_SECRET/${ESCAPED_AUTH}/" "$HAPROXY_CONF"
-fi
 for loop in $(seq 1 10); do
     getent hosts {{.ServiceName}}-announce-2 && break
     echo "Waiting for service {{.ServiceName}}-announce-2 to be ready ($loop) ..." && sleep 1
@@ -43,8 +33,6 @@ if [ -z "$ANNOUNCE_IP2" ]; then
 fi
 sed -i "s/REPLACE_ANNOUNCE2/$ANNOUNCE_IP2/" "$HAPROXY_CONF"
 
-if [ "${AUTH:-}" ]; then
-    echo "Setting auth values"
-    ESCAPED_AUTH=$(echo "$AUTH" | sed -e 's/[\/&]/\\&/g');
-    sed -i "s/REPLACE_AUTH_SECRET/${ESCAPED_AUTH}/" "$HAPROXY_CONF"
-fi
+auth=$(cat /redis-initial-pass/admin.password)
+sed -i "s/replace-with-redis-auth/$auth/" "$HAPROXY_CONF"
+
diff --git a/build/redis/init.sh.tpl b/build/redis/init.sh.tpl
@@ -23,7 +23,7 @@ set -eu
 sentinel_get_master() {
 set +e
     if [ "$SENTINEL_PORT" -eq 0 ]; then
-        redis-cli -h "${SERVICE}" -p "${SENTINEL_TLS_PORT}"   --tls --cacert /app/config/redis/tls/tls.crt sentinel get-master-addr-by-name "${MASTER_GROUP}" |\
+        redis-cli -h "${SERVICE}" -p "${SENTINEL_TLS_PORT}" --tls --cacert /app/config/redis/tls/tls.crt sentinel get-master-addr-by-name "${MASTER_GROUP}" |\
         grep -E '[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}'
     else
         redis-cli -h "${SERVICE}" -p "${SENTINEL_PORT}"  sentinel get-master-addr-by-name "${MASTER_GROUP}" |\
@@ -133,9 +133,9 @@ setup_defaults() {
 redis_ping() {
 set +e
     if [ "$REDIS_PORT" -eq 0 ]; then
-        redis-cli -h "${MASTER}" -p "${REDIS_TLS_PORT}"  --tls --cacert /app/config/redis/tls/tls.crt ping
+        redis-cli -h "${MASTER}" -a "${AUTH}" --no-auth-warning -p "${REDIS_TLS_PORT}"  --tls --cacert /app/config/redis/tls/tls.crt ping
     else
-        redis-cli -h "${MASTER}" -p "${REDIS_PORT}" ping
+        redis-cli -h "${MASTER}"  -a "${AUTH}" --no-auth-warning -p "${REDIS_PORT}" ping
     fi
 set -e
 }

diff --git a/build/redis/redis.conf.tpl b/build/redis/redis.conf.tpl
@@ -20,3 +20,7 @@ rdbcompression yes
 repl-diskless-sync yes
 save ""
 protected-mode no
+requirepass replace-default-auth
+masterauth replace-default-auth
+
+
diff --git a/build/redis/redis_liveness.sh.tpl b/build/redis/redis_liveness.sh.tpl
@@ -1,5 +1,6 @@
 response=$(
   redis-cli \
+    -a "${AUTH}" --no-auth-warning \
     -h localhost \
     -p 6379 \
 {{- if eq .UseTLS "true"}}

diff --git a/build/redis/redis_readiness.sh.tpl b/build/redis/redis_readiness.sh.tpl
@@ -1,5 +1,6 @@
 response=$(
   redis-cli \
+    -a "${AUTH}" --no-auth-warning \
     -h localhost \
     -p 6379 \
 {{- if eq .UseTLS "true"}}

diff --git a/build/redis/sentinel.conf.tpl b/build/redis/sentinel.conf.tpl
@@ -15,3 +15,4 @@ bind 0.0.0.0
     sentinel failover-timeout argocd 180000
     maxclients 10000
     sentinel parallel-syncs argocd 5
+    sentinel auth-pass argocd replace-default-auth
diff --git a/build/redis/sentinel_liveness.sh.tpl b/build/redis/sentinel_liveness.sh.tpl
@@ -1,5 +1,6 @@
 response=$(
   redis-cli \
+    -a "${AUTH}" --no-auth-warning \
     -h localhost \
     -p 26379 \
 {{- if eq .UseTLS "true"}}

diff --git a/common/defaults.go b/common/defaults.go
@@ -274,6 +274,15 @@ gitlab.com ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCsj2bNKTBSpIYDEGk9KxsGh3mySTRgM
 ssh.dev.azure.com ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hr1oTWqNqOlzGJOfGJ4NakVyIzf1rXYd4d7wo6jBlkLvCA4odBlL0mDUyZ0/QUfTTqeu+tm22gOsv+VrVTMk6vwRU75gY/y9ut5Mb3bR5BV58dKXyq9A9UeB5Cakehn5Zgm6x1mKoVyf+FFn26iYqXJRgzIZZcZ5V6hrE0Qg39kZm4az48o0AUbf6Sp4SLdvnuMa2sVNwHBboS7EJkm57XQPVU3/QpyNLHbWDdzwtrlS+ez30S3AdYhLKEOxAG8weOnyrtLJAUen9mTkol8oII1edf7mWWbWVf0nBmly21+nZcmCTISQBtdcyPaEno7fFQMDD26/s0lfKob4Kw8H
 vs-ssh.visualstudio.com ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hr1oTWqNqOlzGJOfGJ4NakVyIzf1rXYd4d7wo6jBlkLvCA4odBlL0mDUyZ0/QUfTTqeu+tm22gOsv+VrVTMk6vwRU75gY/y9ut5Mb3bR5BV58dKXyq9A9UeB5Cakehn5Zgm6x1mKoVyf+FFn26iYqXJRgzIZZcZ5V6hrE0Qg39kZm4az48o0AUbf6Sp4SLdvnuMa2sVNwHBboS7EJkm57XQPVU3/QpyNLHbWDdzwtrlS+ez30S3AdYhLKEOxAG8weOnyrtLJAUen9mTkol8oII1edf7mWWbWVf0nBmly21+nZcmCTISQBtdcyPaEno7fFQMDD26/s0lfKob4Kw8H
 `
+	// RedisDefaultAdminPasswordLength is the length of the generated default redis admin password.
+	RedisDefaultAdminPasswordLength = 16
+
+	// RedisDefaultAdminPasswordNumDigits is the number of digits to use for the generated default redis admin password.
+	RedisDefaultAdminPasswordNumDigits = 5
+
+	// RedisDefaultAdminPasswordNumSymbols is the number of symbols to use for the generated default redis admin password.
+	RedisDefaultAdminPasswordNumSymbols = 0
+
 	// OperatorMetricsPort is the port that is used to expose default controller-runtime metrics for the operator pod.
 	OperatorMetricsPort = 8080
 

diff --git a/controllers/argocd/deployment.go b/controllers/argocd/deployment.go
@@ -212,6 +212,7 @@ func getArgoRedisArgs(useTLS bool) []string {
 
 	args = append(args, "--save", "")
 	args = append(args, "--appendonly", "no")
+	args = append(args, "--requirepass $(REDIS_PASSWORD)")
 
 	if useTLS {
 		args = append(args, "--tls-port", "6379")
@@ -461,6 +462,18 @@ func (r *ReconcileArgoCD) reconcileGrafanaDeployment(cr *argoproj.ArgoCD) error
 func (r *ReconcileArgoCD) reconcileRedisDeployment(cr *argoproj.ArgoCD, useTLS bool) error {
 	deploy := newDeploymentWithSuffix("redis", "redis", cr)
 
+	env := append(proxyEnvVars(), corev1.EnvVar{
+		Name: "REDIS_PASSWORD",
+		ValueFrom: &corev1.EnvVarSource{
+			SecretKeyRef: &corev1.SecretKeySelector{
+				LocalObjectReference: corev1.LocalObjectReference{
+					Name: fmt.Sprintf("%s-%s", cr.Name, "redis-initial-password"),
+				},
+				Key: "admin.password",
+			},
+		},
+	})
+
 	AddSeccompProfileForOpenShift(r.Client, &deploy.Spec.Template.Spec)
 
 	deploy.Spec.Template.Spec.Containers = []corev1.Container{{
@@ -474,7 +487,7 @@ func (r *ReconcileArgoCD) reconcileRedisDeployment(cr *argoproj.ArgoCD, useTLS b
 			},
 		},
 		Resources: getRedisResources(cr),
-		Env:       proxyEnvVars(),
+		Env:       env,
 		SecurityContext: &corev1.SecurityContext{
 			AllowPrivilegeEscalation: boolPtr(false),
 			Capabilities: &corev1.Capabilities{
@@ -576,6 +589,18 @@ func (r *ReconcileArgoCD) reconcileRedisDeployment(cr *argoproj.ArgoCD, useTLS b
 func (r *ReconcileArgoCD) reconcileRedisHAProxyDeployment(cr *argoproj.ArgoCD) error {
 	deploy := newDeploymentWithSuffix("redis-ha-haproxy", "redis", cr)
 
+	var redisEnv = append(proxyEnvVars(), corev1.EnvVar{
+		Name: "AUTH",
+		ValueFrom: &corev1.EnvVarSource{
+			SecretKeyRef: &corev1.SecretKeySelector{
+				LocalObjectReference: corev1.LocalObjectReference{
+					Name: fmt.Sprintf("%s-%s", cr.Name, "redis-initial-password"),
+				},
+				Key: "admin.password",
+			},
+		},
+	})
+
 	deploy.Spec.Template.Spec.Affinity = &corev1.Affinity{
 		PodAntiAffinity: &corev1.PodAntiAffinity{
 			PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{
@@ -608,7 +633,7 @@ func (r *ReconcileArgoCD) reconcileRedisHAProxyDeployment(cr *argoproj.ArgoCD) e
 		Image:           getRedisHAProxyContainerImage(cr),
 		ImagePullPolicy: corev1.PullIfNotPresent,
 		Name:            "haproxy",
-		Env:             proxyEnvVars(),
+		Env:             redisEnv,
 		LivenessProbe: &corev1.Probe{
 			ProbeHandler: corev1.ProbeHandler{
 				HTTPGet: &corev1.HTTPGetAction{
@@ -681,6 +706,10 @@ func (r *ReconcileArgoCD) reconcileRedisHAProxyDeployment(cr *argoproj.ArgoCD) e
 				Name:      "data",
 				MountPath: "/data",
 			},
+			{
+				Name:      "redis-initial-pass",
+				MountPath: "/redis-initial-pass",
+			},
 		},
 	}}
 
@@ -716,6 +745,15 @@ func (r *ReconcileArgoCD) reconcileRedisHAProxyDeployment(cr *argoproj.ArgoCD) e
 				},
 			},
 		},
+		{
+			Name: "redis-initial-pass",
+			VolumeSource: corev1.VolumeSource{
+				Secret: &corev1.SecretVolumeSource{
+					SecretName: fmt.Sprintf("%s-%s", cr.Name, "redis-initial-password"),
+					Optional:   boolPtr(true),
+				},
+			},
+		},
 	}
 
 	deploy.Spec.Template.Spec.SecurityContext = &corev1.PodSecurityContext{
@@ -794,6 +832,17 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
 
 	// Global proxy env vars go first
 	repoEnv := cr.Spec.Repo.Env
+	repoEnv = append(repoEnv, corev1.EnvVar{
+		Name: "REDIS_PASSWORD",
+		ValueFrom: &corev1.EnvVarSource{
+			SecretKeyRef: &corev1.SecretKeySelector{
+				LocalObjectReference: corev1.LocalObjectReference{
+					Name: fmt.Sprintf("%s-%s", cr.Name, "redis-initial-password"),
+				},
+				Key: "admin.password",
+			},
+		},
+	})
 	// Environment specified in the CR take precedence over everything else
 	repoEnv = argoutil.EnvMerge(repoEnv, proxyEnvVars(), false)
 	if cr.Spec.Repo.ExecTimeout != nil {
@@ -1097,6 +1146,17 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
 func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSForRedis bool) error {
 	deploy := newDeploymentWithSuffix("server", "server", cr)
 	serverEnv := cr.Spec.Server.Env
+	serverEnv = append(serverEnv, corev1.EnvVar{
+		Name: "REDIS_PASSWORD",
+		ValueFrom: &corev1.EnvVarSource{
+			SecretKeyRef: &corev1.SecretKeySelector{
+				LocalObjectReference: corev1.LocalObjectReference{
+					Name: fmt.Sprintf("%s-%s", cr.Name, "redis-initial-password"),
+				},
+				Key: "admin.password",
+			},
+		},
+	})
 	serverEnv = argoutil.EnvMerge(serverEnv, proxyEnvVars(), false)
 	AddSeccompProfileForOpenShift(r.Client, &deploy.Spec.Template.Spec)
 	deploy.Spec.Template.Spec.Containers = []corev1.Container{{