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

Enable password-protected Redis for HA #11387

Closed
crenshaw-dev opened this issue Nov 21, 2022 · 8 comments
Closed

Enable password-protected Redis for HA #11387

crenshaw-dev opened this issue Nov 21, 2022 · 8 comments
Labels
enhancement New feature or request security Security related

Comments

@crenshaw-dev
Copy link
Member

Summary

Our HA manifests should support running Redis with password protection. The redis-haproxy config doesn't currently support auth.

Motivation

Password-protecting Redis is #3130. Adding password protection provides an additional layer of security to protect cached information (e.g. rendered manifests) from unauthorized access.

Proposal

Modify the redis-haproxy manifests to support passing auth information.

@crenshaw-dev crenshaw-dev added enhancement New feature or request security Security related labels Nov 21, 2022
@edmondshtogu
Copy link

Is there any plan for adding this functionality any time soon?

@crenshaw-dev
Copy link
Member Author

@edmondshtogu in my mind, yes. 😄 I'm not precisely sure when I/someone will get to this though.

@jdoylei
Copy link

jdoylei commented Feb 13, 2023

@crenshaw-dev - Thanks for capturing this issue. We're running the HA Argo CD install, and our security team flagged the unprotected Redis as an issue. If I understand it right, the only way right now to password-protect Argo CD Redis is to replace the HA install with the simple install, right?

@crenshaw-dev
Copy link
Member Author

@jdoylei based on this comment, you might be able to modify your install manifests to get it to work with HA: #3130 (comment)

@jdoylei
Copy link

jdoylei commented Feb 16, 2023

@crenshaw-dev - Thanks for the tip! If I translate that comment into changes applied to Argo CD's HA namespace-install.yaml, it looks like there are 4 "tcp-check send PING" commands in the haproxy.cfg configmap entry. Would I need to replace each of the 4 occurrences, per that comment?

@jessesuen
Copy link
Member

jessesuen commented Feb 28, 2023

Michael asked me to jot down some notes about redis gotchas with respect to password updates in redis-ha. Theres some challenges if you ever need to update ACL on a ha redis:

Redis (or at least the way we use it) does not replicate ACL configuration to the other two nodes (unlike the actual redis data which does replicate). So if you are connecting to the shared hostname (e.g. redis-cli -h argocd-redis-ha-haproxy:6379) and running acl commands, you will only be updating the ACL configuration for one of three redis servers (whichever is the master).

Redis does have an ACL LOAD command to reload an ACL file from disk after it's changed, but because kubernetes takes time to propagate secrets to volume mounts, you don't know for certain when that acl file was updated (it can take minutes). You also still have the problem of needing to run ACL LOAD on all three nodes.

So, the easiest way to update redis ACLs after a change, is to bounce all three redis servers after changing the acl on disk (but restarting redis ha statefulset can be slow, and can sometimes cause issues).

Note that all of the above is only ever a problem if ACL needs to be changed or password needs to be rotated on an already running redis.

@jdoylei
Copy link

jdoylei commented Sep 5, 2023

@jessesuen and @crenshaw-dev - Just revisiting this, because we had some other reason to override haproxy.cfg (see #15319), and I had some second thoughts about the haproxy.cfg tcp-check send AUTH command shown in #3130 (comment). I was trying to decide whether there was some way to extract the password from haproxy.cfg, since embedding the password seemed to create more of a security issue. Would you expect that the haproxy.cfg file specified in Argo CD's argocd-redis-ha-configmap should be able to reference Environment Variables, like they're documented by HAProxy?

@todaywasawesome
Copy link
Contributor

Closed by f1a449e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request security Security related
Projects
None yet
Development

No branches or pull requests

5 participants