logs with redacted password in URL

[eguzki]

What

Fixes #360 and also configuration dump on boot stage is also redacted.

Verification steps for starting with a wrong password

The verification steps will be using existing sandbox with limitador configured to connect to redis using pass.

• Let's set a wrong password

git checkout 360-logs-show-plaintext-redis-password-when-theres-connection-errors

patch -p1 -N <<EOF
diff --git a/limitador-server/sandbox/docker-compose-limitador-redis-tls.yaml b/limitador-server/sandbox/docker-compose-limitador-redis-tls.yaml
index 5c603a5..e14d187 100644
--- a/limitador-server/sandbox/docker-compose-limitador-redis-tls.yaml
+++ b/limitador-server/sandbox/docker-compose-limitador-redis-tls.yaml
@@ -22,7 +22,7 @@ services:
       - --grpc-reflection-service
       - /opt/kuadrant/limits/limits.yaml
       - redis
-      - rediss://:foobared@redis:6379/#insecure
+      - rediss://:wrongpassword@redis:6379/#insecure
     expose:
       - "8080"
       - "8081"
EOF

then prepare and boot the environment

# clean any pre-existing images to enforce building a new one
docker rmi limitador-testing:latest
cd limitador-server/sandbox
make deploy-redis-tls 2>&1 | grep limitador

The logs will show some loglines from limitador with the configuration redacted

limitador-1  | 2024-10-09T08:54:55.871860Z  INFO limitador_server: Using config: Configuration { limits_file: "/opt/kuadrant/limits/limits.yaml", storage: Redis(Foo { cache: None, url: rediss://:****@redis:6379/#insecure }), rls_host: "0.0.0.0", rls_port: 8081, http_host: "0.0.0.0", http_port: 8080, limit_name_in_labels: false, tracing_endpoint: "", log_level: Some(LevelFilter::DEBUG), rate_limit_headers: None, grpc_reflection_service: true }

And the error is also redacted

limitador-1  | Failed to connect to Redis at rediss://:****@redis:6379/#insecure: Password authentication failed- AuthenticationFailed

Verification steps for re-connections with a wrong password

The verification steps will be using existing sandbox with limitador configured to connect to redis using pass. The main idea is that, initially, limitador connects correctly with the right password. Then, Redis changes it's own password, so limitador no longer can access redis. The limitador logs should never show the password being used.

• clean the previous environment

make clean
# revert previous patch
git checkout .

• prepare and boot the environment

make deploy-redis-tls 2>&1 | grep limitador

• Send some traffic

curl -i -H "Host: example.com" http://127.0.0.1:18000/get

It should return 200 OK, and after few more request, it should return 429 Too Many Requests. That would mean that limitador can connect to redis successfully.

• Let's change redis password and restart only redis service

cd ${LIMITADOR_PROJECT_PATH}

patch -p1 -N <<EOF
diff --git a/limitador-server/sandbox/redis-tls/redis-config.conf b/limitador-server/sandbox/redis-tls/redis-config.conf
index b478e76..1bae31a 100644
--- a/limitador-server/sandbox/redis-tls/redis-config.conf
+++ b/limitador-server/sandbox/redis-tls/redis-config.conf
@@ -1,4 +1,4 @@
-requirepass foobared
+requirepass otherpassword
 port 0
 tls-port 6379
 tls-cert-file /usr/local/etc/redis/certs/redis.crt
EOF

then restart only the redis service

cd limitador-server/sandbox
docker compose -p sandbox restart redis

• Send some traffic

curl -i -H "Host: example.com" http://127.0.0.1:18000/get

The logs will show some loglines from limitador reporting about connection errors

limitador-1  | 2024-10-10T09:23:01.753896Z ERROR should_rate_limit: limitador_server::envoy_rls::server: Error: Storage("broken pipe")
limitador-1  | 2024-10-10T09:23:35.960749Z ERROR should_rate_limit: limitador_server::envoy_rls::server: Error: Storage("Password authentication failed- AuthenticationFailed")

Logs should not reveal any URL or password.

[alexsnaps]

The down side is that if err ever contains that password, we'd "reintroduce" the bug. Or do we know that is guarantee provided by the crate?
E.g. what if redis is restarted with a new password and the reconnect path fails? That might be worth a quick (even manual) test.

[eguzki]

I added verification steps with a manual test.

I would never expect the crate to return the url of the connection serialized in the error string, but who knows maybe one day. Today, according to my tests, it is not doing so. Would you suggest to apply some regex replace to the string before writing it to stderr?

[alexsnaps]

No, I would have possibly dealt with that error differently... but if this is fine, well... it's fine 🙌

[eguzki]

let me know how you would dealt with the error otherwise. Happy to consider it