Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Obfuscate passwords in DSN logs #1042

Open
drewwells opened this issue Jun 10, 2024 · 14 comments · May be fixed by #1104
Open

Obfuscate passwords in DSN logs #1042

drewwells opened this issue Jun 10, 2024 · 14 comments · May be fixed by #1104

Comments

@drewwells
Copy link

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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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

@EvertonCalgarotto
Copy link

v0.10.1

@sysadmind
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

drewwells commented Jun 10, 2024

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
Copy link
Contributor

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
Copy link

@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
Copy link
Author

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
Copy link

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
Copy link
Author

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

@Schmaetz
Copy link

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

sysadmind added a commit to sysadmind/postgres_exporter that referenced this issue Dec 22, 2024
This log line was not sanitized previously which could result in logging sensitive information. I have scanned the rest of the files and I don't see anywhere else that DSN is used in a log line without this filter.

Resolves prometheus-community#1042

Signed-off-by: Joe Adams <[email protected]>
@sysadmind sysadmind linked a pull request Dec 22, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants