Skip to content

fix(redis): do not log the Redis password - #1316

Open
bakiburakogun wants to merge 1 commit into
nextcloud:mainfrom
bakiburakogun:fix/redact-redis-credentials
Open

fix(redis): do not log the Redis password#1316
bakiburakogun wants to merge 1 commit into
nextcloud:mainfrom
bakiburakogun:fix/redact-redis-credentials

Conversation

@bakiburakogun

Copy link
Copy Markdown

The problem

RedisAdapter.createRedisClient() logs the configured Redis URL verbatim every time the websocket server starts. On any deployment that authenticates to Redis, that writes the password into the journal in clear text:

Creating Redis client with URL: redis://:s3cret@10.0.0.5:6379

We hit this on a production install: the password we had just put in a 0600 environment file showed up in journalctl a second later, where it is readable by anyone who can read the journal and by whatever ships logs off the box.

The change

Redact the credentials before logging. The host and port, which are the useful part of the line, stay visible:

Creating Redis client with URL: redis://:***@10.0.0.5:6379

A URL that cannot be parsed logs a placeholder rather than the raw string, so a malformed value with a secret in it does not leak either.

Testing

redis://:s3cret@10.0.0.5:6379          -> redis://:***@10.0.0.5:6379
redis://user:pass@host:6379/2          -> redis://***:***@host:6379/2
redis://10.0.0.5:6379                  -> redis://10.0.0.5:6379   (unchanged)
unix:///var/run/redis.sock             -> unix:///var/run/redis.sock (unchanged)
not-a-url                              -> <unparsable REDIS_URL>

The websocket server logs its Redis URL verbatim on every start, so a
deployment that authenticates to Redis writes the password into the journal
in clear text:

    Creating Redis client with URL: redis://:s3cret@10.0.0.5:6379

Redact the credentials before logging. The host and port, which are what the
line is useful for, stay visible.

Signed-off-by: Baki Burak Öğün <63836730+bakiburakogun@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

1 participant