Obfuscate passwords in DSN logs

[drewwells]

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]

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]

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]

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

[EvertonCalgarotto]

v0.10.1

[sysadmind]

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]

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]

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]

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]

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.

[gabrielmuras]

@drewwells Have you solved using this new format with DATA_SOURCE_NAME environment variable?

I'm facing similar problem because but slight different. The password is indeed obfuscated in the err message. But then it outputs as plain text in the dsn message.

time=2024-12-04T16:02:30.720Z level=ERROR source=postgres_exporter.go:681 msg="error scraping dsn" err="Error opening connection to database (postgres://postgres_exporter:PASSWORD_REMOVED@host:5432/database?sslmode=require): dial tcp server:5432: connect: connection timed out" dsn="postgres://postgres_exporter:PLAIN_TEXT_PASSWORD@host:5432/database?sslmode=require"

[drewwells]

I think you're running a different version than us. We're still on v0.10.1

DSN format of connection string is obfuscated. We're not getting the same type of error you are so unsure if that fixes it for you.

[gabrielmuras]

Thanks for quick response. I'm using v0.16 and I think that I found the error. It seem to be introduced here #1073

https://github.com/prometheus-community/postgres_exporter/blob/master/cmd/postgres_exporter/postgres_exporter.go#L681 (v0.16.0)

https://github.com/prometheus-community/postgres_exporter/blob/v0.15.0/cmd/postgres_exporter/postgres_exporter.go#L682 (v0.15.0)

It solved my issue using the v0.15. Maybe we should remove this dsn output or use the loggableDSN function to avoid this error

[drewwells]

I see that in v0.16, good find. I opened a ticket to upgrade to v0.16 today and Ill move that back to TBD :)