Feature request: improve password hygeine / security

[ferbs]

Search before asking

• [X] I had searched in the issues and found no similar issues.

Motivation

Improve handling of cleartext passwords, keeping them out of version control in particular, and to follow industry recommended practices.

Solution

Please add an option to configure kvrocks with a sha256 hash of the actual password, rather than using cleartext. Though similar to the Redis #<hashedpassword> ACL option, it could be implemented separately from any ACL functionality, as an alternative to using the requirepass directive, perhaps with a kvrocks.conf requirepass_sha256 directive, and/or by setting an environment variable, eg: KVROCKS_SECRET_SHA256=e3b0c442...etc... When a client connects and provides its pass, kvrocks would compare the sha256 of that value if enabled, rather than use the cleartext.

Related, it would helpful for kvrocks to support the file-based convention recommended for secrets used in containers. It's often hard to keep regular environment variables out of version control, and they can leak during runtime too. Instead, one environment variable (eg, KVROCKS_PASSWORD_FILE) holds a path to a file in .ini format (key=value pairs just like env vars) which the app loads and uses for its config.

Are you willing to submit a PR?

• [ ] I'm willing to submit a PR!

[PragmaTwice]

I'm not sure if I understand your words correctly.

You can currently specify the master password in kvrocks.conf. And the configuration file should only be accessible to trusted individuals.

[ferbs]

Placing the master password in kvrocks.conf means you can't check that file into git, and that it won't work with secret managers / vaults in a containerized environment.

Rather than place the password in that file, someone could use the --requirepass switch on start, but in a containerized environment using a secrets manager, this requires overriding the project's original dockerfile entrypoint and dealing with environment variable expansion headaches.

What would be much better is telling kvrocks where to find the secret. For example, masterpass_env KVROCKS_MASTER_PASS tells the app to use the value of that environment variable as its password. Better yet, adding masterpass_file /var/run/secrets/kvrocks.ini tells kvrocks to find that value within a file, making it even easier to lock it down using a secrets manager/vault, and is more likely to keep the cleartext secret out logs/history/environment, places that might leak.

The other suggestion is an option to use sha256 verification of the master password. Where kvrocks is only provided with the hashed password so even if it leaked, it's not in a usable form.

[PragmaTwice]

IMHO from my side it doesn't seem to be the logic that needs to be implemented inside Kvrocks.

e.g. if you are using helm charts, you can refer to some helm packages like https://artifacthub.io/packages/helm/bitnami/redis which support configmap construction and secret management for Redis via several ways, as Kvrocks configuration mechanism is similiar to Redis.

For stronger security, you can refer to TLS connection support in Kvrocks and provide your own client-side certificates, instead of just passwords.

[ferbs]

Not TLS, I'm talking about password management: how to properly provide the secret to kvrocks and to clients without it leaking into git, logs, etc.

With Redis, I can use an ACL file. With postgres, I can use POSTGRES_PASSWORD_FILE. Systems that don't offer that simple mechanism add a burden... modifying config files during deployment with sed, or reconfiguring helm/other, or maintaining a custom entrypoint, etc.

I'm personally overriding the entrypoint and am fine, but am suggesting modest ways you could make it easier for people to secure kvrocks with their current tooling.

[git-hulk]

Yes, perhaps we can support this in Dockerfile like PostgreSQL: https://github.com/docker-library/postgres/blob/cefde5ff6f102fcd381a03210c7734816c59aa3e/docker-entrypoint.sh#L9.

[ferbs]

Good idea to review theirs! Looks like their file_env() would work without modification, and is MIT license. I personally try to minimize time with bash, so nice to borrow from experts.

Off the top of my head, a new kvrocks-entrypoint.sh might look like:

#!/usr/bin/env bash

# copy of file_env() function here

file_env 'KVROCKS_PASSWORD'

exec kvrocks "$@"

If using the above entrypoint, --requirepass could be set using a positional [COMMAND] argument on start, but would require escaping var expansion. I think it would look something like: docker run --env KVROCKS_PASSWORD_FILE=/var/run/secrets/kvrocks.ini apache/kvrocks --requirepass '$${KVROCKS_PASSWORD}'

It would be possible for the entrypoint itself to conditionally include --requirepass when $KVROCKS_PASSWORD is present... should someone with more bash ambition come along. :) Prob involves $([conditional -Z] printf) stuff.