search

prometheus-community / postgres_exporter

A PostgreSQL metric exporter for Prometheus
Apache License 2.0
2.82k stars 743 forks source link

Obfuscate passwords in DSN logs #1042

Open drewwells opened 5 months ago

drewwells commented 5 months ago

Proposal

Use case. Why is this important? We don't want to log our database passwords in our logs. Add a feature to remove passwords when logging out DSN. If there's other auth methods for exporter, maybe it's sufficient to document that using DSN with password will include the password in the log.

sysadmind commented 5 months ago

Can you provide an example redacted log entry? That would help narrow down the bad code. I can't find anywhere that doesn't redact the password when logging.

EvertonCalgarotto commented 5 months ago

here you have one: ts=2024-06-05T07:34:28.288Z caller=postgres_exporter.go:731 level=error err="Error opening connection to database (host=XXX%20port=5432%20user=YYY%20password=ZZZ%20dbname=AAA%20sslmode=require): pq: password authentication failed for user \"YYY\""

sysadmind commented 5 months ago

@EvertonCalgarotto That log entry looks like it's from an old version of the exporter. What version are you using?

EvertonCalgarotto commented 5 months ago

v0.10.1

sysadmind commented 5 months ago

v0.10.1 is very old. The most recent is v0.15.0. That log entry should not happen on the most recent version.

drewwells commented 5 months ago

v0.10.1. The code on most recent looks the same, is there anything sanitizing these passwords?

https://github.com/prometheus-community/postgres_exporter/blob/v0.10.1/cmd/postgres_exporter/postgres_exporter.go#L731

https://github.com/prometheus-community/postgres_exporter/blob/master/cmd/postgres_exporter/postgres_exporter.go#L682

sysadmind commented 5 months ago

It looks like the cause there is that the old redaction func is not accounting for the key=value style of DSN. The newer structures for DSN do account for this.

The best thing to do here on the code side would probably be to parse this into the DSN and use the String() func from that.

Short term, you could use a URL style connection string which should redact this (postgres://username:password@host/?params)

drewwells commented 5 months ago

Ah okay, so if we used the more modern dsn, we would not be seeing the passwords in our logs?

is this the new format you're referring to? postgres://<username>:<password>@<host>:<port>/<database>?sslmode=require

sysadmind commented 5 months ago

Yes with that format you should not be seeing the password. I believe that the format you reference is correct.

That said, this should still be resolved in code.