postgres_exporter icon indicating copy to clipboard operation
postgres_exporter copied to clipboard

Obfuscate passwords in DSN logs

Open drewwells opened this issue 1 year ago • 9 comments

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.

drewwells avatar Jun 10 '24 19:06 drewwells

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.

sysadmind avatar Jun 10 '24 19:06 sysadmind

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\""

EvertonCalgarotto avatar Jun 10 '24 19:06 EvertonCalgarotto

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

sysadmind avatar Jun 10 '24 20:06 sysadmind

v0.10.1

EvertonCalgarotto avatar Jun 10 '24 20:06 EvertonCalgarotto

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

sysadmind avatar Jun 10 '24 20:06 sysadmind

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

drewwells avatar Jun 10 '24 20:06 drewwells

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)

sysadmind avatar Jun 10 '24 20:06 sysadmind

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

drewwells avatar Jun 10 '24 20:06 drewwells

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.

sysadmind avatar Jun 11 '24 13:06 sysadmind

@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"

gabrielmuras avatar Dec 04 '24 16:12 gabrielmuras

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.

drewwells avatar Dec 04 '24 16:12 drewwells

Thanks for quick response. I'm using v0.16 and I think that I found the error. It seem to be introduced here https://github.com/prometheus-community/postgres_exporter/pull/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

gabrielmuras avatar Dec 04 '24 16:12 gabrielmuras

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 :)

drewwells avatar Dec 04 '24 19:12 drewwells

I can't see a solution in release 0.16.0. The problem still occurs with some errors. Is it a mistake on my side or do we have a bug here?

time=2024-12-17T16:06:52.654Z level=ERROR source=postgres_exporter.go:681 msg="error scraping dsn" err="queryNamespaceMappings returned 1 errors" dsn="postgresql://exporter:qSqIJ5XoqmxO0U8vRmoNGdGJ4XfXLmMlcC5PSWXHgKwghMi6OaNppOuDndRL1iXY@localhost:5432/postgres?sslmode=disable"

From my point of view its a bug.

Best regards

Schmaetz avatar Dec 17 '24 16:12 Schmaetz