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

Censor password in logs when database is configured with a connection URI #189

Open
jysandy opened this issue Sep 4, 2020 · 1 comment

Comments

@jysandy
Copy link

jysandy commented Sep 4, 2020

If there's an error connecting to the database and the connection is configured with a connection URI, the entire connection URI is printed to the logs. Like so:

Error creating DB connection for {:connection-uri "jdbc:postgresql://localhost:19401/chronograph_dev?user=chronograph_dev&password=chronograph_devpwd"}

This could result in the password being leaked.

One possible solution could be to log only the first part of the connection URI up until the host and port. Alternatively, we could redact the entire connection URI. I noticed that there's already a function to censor the password, but this only works if the db-spec has a :password key. https://github.com/yogthos/migratus/blob/master/src/migratus/utils.clj#L90

Happy to raise a PR if you agree with either of these solutions.

@yogthos
Copy link
Owner

yogthos commented Sep 4, 2020

Hi,

I agree the feature would be useful. One thing to consider here is that there is some variation in how the connection string can be specified for different databases. So, might be safest to parse out from :// to of the address. For example, Oracle connections can look like jdbc:oracle:thin:user/pass@//123.11.222.33:1521/schema. And a PR for this would be very welcome.

jysandy added a commit to nilenso/chronograph that referenced this issue Sep 4, 2020
This is because migratus prints out the entire connection string
in case of errors, which can leak sensitive information.

See yogthos/migratus#189
yogthos pushed a commit that referenced this issue Oct 21, 2020
An aggressive fix for #189
It simply replaces a non-empty string entirely, to avoid the difficulties
in parsing the uri and removing only the password.
whenceforth added a commit to whenceforth/migratus that referenced this issue Oct 21, 2020
A fix for yogthos#189

When logging the db-spec on a connection failure, it replaces a non-empty value
for key :connection-uri entirely, to avoid the difficulties of parsing the uri
and removing only the password.
yogthos pushed a commit that referenced this issue Oct 21, 2020
A fix for #189

When logging the db-spec on a connection failure, it replaces a non-empty value
for key :connection-uri entirely, to avoid the difficulties of parsing the uri
and removing only the password.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants