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

[NEW]Support for encrypted passwords in redis/sentinel.conf #11706

Open
shadjiiski opened this issue Jan 11, 2023 · 8 comments
Open

[NEW]Support for encrypted passwords in redis/sentinel.conf #11706

shadjiiski opened this issue Jan 11, 2023 · 8 comments

Comments

@shadjiiski
Copy link

The problem/use-case that the feature addresses

We have a Sentinel-managed Redis HA setup running in k8s, based on modified bitnami charts running Redis/Sentinel 6.2.7. We have enabled persistence for both containers and have the persisted storage mounted on the host system. Since Sentinel uses its sentinel.conf for persistence, we end up with plaintext files on the host system that contain passwords, for example:

sentinel auth-pass redis-master masterpass
requirepass "sentinelpass"

The above would be true even if we switched to ACL - Sentinel will still need to know the passwords and not their hashes so it can authenticate to other redis/sentinel nodes.

As per our internal security policies, no plaintext passwords can be stored on the host system.

Note: We don't have this issue with the Redis container. Yes, it also features plaintext passwords in its config file but that configuration can be separated from the persistence volume and thus is not exposed on the host system.

Description of the feature

It would be great if Redis/Sentinel supported an encrypted way to read passwords from its configuration. For example:

pass-encryption-key /path/to/my.key
requirepass ~enc~<some encrypted password>

What is essential here is that the encryption key should not be readable from the config file as well. If that is the case, it can be excluded from the volume mount and will thus be unavailable there (e.g., it can be mounted from a k8s secret).

Alternatives you've considered

  • An alternative could be to allow for separation of the credentials in a separate config file and to make sure they do not get included in the main sentinel.conf. I have tried to achieve something similar by moving the requirepass and sentinel auth-pass directives to a sentinel-creds.conf and including them in the main sentinel.conf, however the confi rewrite adds them to the latter (see [QUESTION] Sentinel: disable config rewrite #10085 (comment))
  • Allowing to specify the secrets with environment variables could also work. Specifying them via command line arguments makes them readable to ps and also results in the creds being added to the sentinel.conf by config rewrite.
  • Separating storage from configuration might also be an option - this works for us for the Redis case

Additional information

N/A

@moticless
Copy link
Collaborator

Hi @shadjiiski ,
You can pass the password as command argument to sentinel:

redis-sentinel sentinel.conf  --requirepass mysecret

@ElCheekytico
Copy link

Hi @shadjiiski , You can pass the password as command argument to sentinel:

redis-sentinel sentinel.conf  --requirepass mysecret

@moticless He clearly stated why that can't be done

Allowing to specify the secrets with environment variables could also work. Specifying them via command line arguments makes them readable to ps and also results in the creds being added to the sentinel.conf by config rewrite.

@ElCheekytico
Copy link

There was obviously a reason for introducing ACLs and only having the hash value stored in the ACL file. This is a needed option for enterprises to have credentials encrypted.

@moticless
Copy link
Collaborator

Only god knows what i was thinking when i wrote it. Sorry about that.

@yossigo please consider.

@yossigo
Copy link
Member

yossigo commented Mar 8, 2023

A mechanism to store sensitive configuration parameters/credentials is needed, and as @shadjiiski indicated, it will also be useful for Redis (e.g., masterauth). The question is how to make it useful in different deployments and support other policies.

Should we take the approach of storing secrets elsewhere and pointing to them vs. storing them encrypted in the config file?

If we encrypt, I don't think we can assume it's always OK to store keys in a different file. Env variables are also not super safe, and we may need to either read them from stdin on startup or provide some hooks that we use to retrieve them.

If we encrypt, we must also define how key rotation takes place.

@moticless
Copy link
Collaborator

IMHO, I think today it is rather become common practice, especially in microservices world, to keep the secrets in a distinct file with various 3rd parties extensions, with posibility to rotations, etc. This is what @shadjiiski mentioned, k8s support the option to mount the secret as a file.

Higher level of security is probably working with an external secret management system and with proper API to identify, query, use the secret and forget it. But this is way behind our scope... As an integration, we can think about some wrapper to Redis that do all this interfacing with secret management and, say, run Redis as a subprocess, providing it the credentials as environment variable, which no one else can see. etc.

I think keeping secrets in a config file is not an option. we cannot encrypt the password in it. Following restart sentinel need to bootstrap all over again and it has zero knowledge about what was its state before, but looking into the config file.

Indeed, passing passwords as arguments can be seen by ps and IMHO we should drop this approach.

@slavak
Copy link
Contributor

slavak commented Mar 12, 2023

Working directly with the API of external secrets management systems is probably beyond the scope of Redis, and would also be rather strongly coupled to a specific vendor.

There are existing wrappers/abstraction layers for such integrations. From what I've seen, they mostly work by either creating a secrets file (e.g.: Vault Agent) or injecting environment variables (Cyberark Summon, Vault Agent Injector).

Punting the actual secrets management to a separate component is probably a better idea than supporting encryption within the config file, which would open up a whole new world of security management. (Key rotation, algorithm management...)

The file-based solution is arguably already supported using the include mechanism, but requires the ability to disable config rewrite to prevent secrets from being pulled in to the main file. (Issue #10085.) Supporting override of specific config values using environment variables might also be beneficial.

@shadjiiski
Copy link
Author

(Speaking from a k8s user point of view)
AFAIK, the general idea is to decouple application from secret management systems. In k8s, the pattern appears to be to store credentials/keys in secrets which can be encrypted (with key rotation left to the user with some tooling from k8s). These are made available to the containerized process as files or env variables.

+1 for the coupling concerns of @slavak . Furthermore, wouldn't relying on the API of a 3rd party creds manager just shift the problem? That API would most probably require some authentication that needs to be known to redis/sentinel :)

I do not consider myself a security expert, but I think the most secure way to handle this is for sentinel to have a way to read credentials/encryption key from a separate file once and then close all handles to it. This way:

  • the sensitive data can be managed separately from the persisted data (similar to redis which does not use the config file for persistence)
  • the file with the sensitive data can optionally be unlinked by the user after starting sentinel. This way, even if a malicious user somehow gets in the container, they will have a hard time finding the secrets

Support for environment variables will also work (a lot of popular db containers have support for this, not sure if it is the only option) - security freaks can probably create a wrapper that converts file-mounted secrets to env vars in a subshell as suggested by @moticless (if one is not available oob)

To me personally, I don't think it makes much difference if I will be mounting an encryption key or credentials directly - sounds like they are the same task (but one has extra steps for redis/sentinel).

I don't think that fully disabling the config rewrite (as asked in #10085) fits all scenarios - isn't this more or less equivelent to disabling the persistence? I would be glad to keep the knowledge of the last known master so we can design our systems in a way that will not do unnecessary failovers after a power cycle (or worse - end up with different sentinels following a different master). Preventing it from putting all included configs in the main sentinel.conf will probably do the trick for my setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants