Skip to content
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.

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

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

Merged
merged 1 commit into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading