search

mhammond / pywin32

Python for Windows (pywin32) Extensions
5.07k stars 799 forks source link

adodbapi exposing the password #1474

Open FlorianVeaux opened 4 years ago

FlorianVeaux commented 4 years ago

Hi

The adodbapi library prints and raise exceptions that contain the complete connection_string including the non-redacted password.

Some examples: https://github.com/mhammond/pywin32/blob/master/adodbapi/adodbapi.py#L119-L120 https://github.com/mhammond/pywin32/blob/master/adodbapi/adodbapi.py#L257 https://github.com/mhammond/pywin32/blob/master/adodbapi/adodbapi.py#L265 https://github.com/mhammond/pywin32/blob/master/adodbapi/adodbapi.py#L273

Yes it is possible for library users to ignore all exceptions messages but at the cost of visibility and understanding the issue. Yes it is possible to hack around that issue and redact the password somewhere else (what we are doing here: https://github.com/DataDog/integrations-core/blob/7c3d6703f8357973bc33d9bbb13ae0b14fc7de90/sqlserver/datadog_checks/sqlserver/sqlserver.py#L658-L669).

But it'd be better to have that safe-guard in the library itself.

vernondcole commented 4 years ago

An interesting idea. It might be expensive to implement, given the many different ways that connection strings are built. The error handler would have to parse the connection string looking for keywords (perhaps we could live with the limitation that password= is the only obfuscation trigger) and then alter the connection string before re-raising the exception. Do you have a patch in mind?

vernondcole commented 4 years ago

Followup: . . . it appears, from a quick sampling of connectionstrings.com, that PostgreSQL and some IBM databases use pwd= and everything else uses password=, so parsing might not be so bad as I feared.

FlorianVeaux commented 4 years ago

Do you have a patch in mind?

I'm not familiar with the internals of the library enough and haven't had time to look more into it than just opening that issue. That being said if password and pwd are the only two keywords, redacting by pattern matching looks like a viable option to me!