Skip to content

Commit

Permalink
feat: secure redis authentication [CVE-2024-31989] (#1364) (#1371)
Browse files Browse the repository at this point in the history
Signed-off-by: iam-veeramalla <abhishek.veeramalla@gmail.com>
  • Loading branch information
iam-veeramalla committed May 22, 2024
1 parent 5354d55 commit 2143144
Show file tree
Hide file tree
Showing 20 changed files with 680 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 8 additions & 0 deletions build/redis/haproxy.cfg.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 3 additions & 15 deletions build/redis/haproxy_init.sh.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"

6 changes: 3 additions & 3 deletions build/redis/init.sh.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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}" |\
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions build/redis/redis.conf.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@ rdbcompression yes
repl-diskless-sync yes
save ""
protected-mode no
requirepass replace-default-auth
masterauth replace-default-auth


1 change: 1 addition & 0 deletions build/redis/redis_liveness.sh.tpl
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
response=$(
redis-cli \
-a "${AUTH}" --no-auth-warning \
-h localhost \
-p 6379 \
{{- if eq .UseTLS "true"}}
Expand Down
1 change: 1 addition & 0 deletions build/redis/redis_readiness.sh.tpl
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
response=$(
redis-cli \
-a "${AUTH}" --no-auth-warning \
-h localhost \
-p 6379 \
{{- if eq .UseTLS "true"}}
Expand Down
1 change: 1 addition & 0 deletions build/redis/sentinel.conf.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions build/redis/sentinel_liveness.sh.tpl
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
response=$(
redis-cli \
-a "${AUTH}" --no-auth-warning \
-h localhost \
-p 26379 \
{{- if eq .UseTLS "true"}}
Expand Down
9 changes: 9 additions & 0 deletions common/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,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
)
Expand Down
64 changes: 62 additions & 2 deletions controllers/argocd/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -573,6 +574,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{{
Expand All @@ -586,7 +599,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{
Expand Down Expand Up @@ -673,6 +686,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{
Expand Down Expand Up @@ -705,7 +730,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{
Expand Down Expand Up @@ -778,6 +803,10 @@ func (r *ReconcileArgoCD) reconcileRedisHAProxyDeployment(cr *argoproj.ArgoCD) e
Name: "data",
MountPath: "/data",
},
{
Name: "redis-initial-pass",
MountPath: "/redis-initial-pass",
},
},
}}

Expand Down Expand Up @@ -813,6 +842,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{
Expand Down Expand Up @@ -891,6 +929,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 {
Expand Down Expand Up @@ -1181,6 +1230,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{{
Expand Down
Loading

0 comments on commit 2143144

Please sign in to comment.