-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[mysql-persistence] Do not log plain passwords #5665
[mysql-persistence] Do not log plain passwords #5665
Conversation
Signed-off-by: Martin van Wingerden <martin@martinvw.nl>
@@ -553,7 +553,7 @@ private void connectToDatabase() { | |||
} catch (Exception e) { | |||
logger.error( | |||
"mySQL: Failed connecting to the SQL database using: driverClass={}, url={}, user={}, password={}", | |||
driverClass, url, user, password, e); | |||
driverClass, url, user, password.replaceAll(".", "*"), e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are still exposing some information about the password here (its length), which should also be avoided, see the comment at eclipse-archived/smarthome#4583 (comment).
So I'd suggest to either put 8 asterisks or maybe simply remove the password part completely here as it anyhow is not helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kaikreuzer, I like that, shall I also remove it then in all other places in this repo.
Should I also apply it for trace logging? or is that configurable and thus okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, can be removed from other places as well from the logging.
I'd also remove it from trace statements.
Signed-off-by: Martin van Wingerden <martin@martinvw.nl>
Hi @kaikreuzer, its updated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Wow these were quite many places 😟
Thank you |
@CrispinP thank you for reporting it. Note that I also created follow up issue for openHAB2 addons https://github.com/openhab/openhab2-addons/issues/3990 |
Fixes: openhab/openhab-distro#762
Signed-off-by: Martin van Wingerden martin@martinvw.nl