Openhab-addons: Do not log passwords to console / logs

[martinvw]

in openHAB1-addons we recently checked for passwords being logged, however a similar check should be done for openHAB2-addons, the first hit I found was:

https://github.com/openhab/openhab2-addons/blob/5c03ebb95e9177960543aa9db249829071ce7e5e/addons/binding/org.openhab.binding.dscalarm/src/main/java/org/openhab/binding/dscalarm/internal/handler/EnvisalinkBridgeHandler.java#L76

But I expect more hits, so it is about logging passwords for example because of including them in a toString, just because of the plain logging or by putting them in an Exception (message).

Anyone wanting to pick this up is appreciated :-)

[wborn]

we recently checked for passwords being logged

Did you also check for pincodes, oauth tokens, private keys etc? Logging those is also problematic when logfiles are attached to issues or posted to forums. I know the Nest binding logs some:

https://github.com/openhab/openhab2-addons/blob/12558ff48b38677b906e227b5d2201cb13a203cf/addons/binding/org.openhab.binding.nest/src/main/java/org/openhab/binding/nest/handler/NestBridgeHandler.java#L106-L109

Sometimes these details are also part of sent/received messages that get logged. These are less easy to spot in existing code.

We could also add these practices to the Logging section in the Coding Guidelines.

[cweitkamp]

Should we additionally check for context password in the config-description parameters?

<parameter name="password" type="text" required="true">
    <context>password</context>
    <label>Password</label>
    <description>Password to access ...</description>
</parameter>

[cweitkamp]

I additionally expect some "hidden" leaks of passwords, API keys, etc. as well. Imagine one has to call an interface and either uses HTTP Basic access authentication - in worst case deprecated plain text URL encoding syntax: username:[email protected] - or sends a request containing a parameter for the API key (e.g. https://example.com&apikey=12345).

@martinvw Do you think we should provide an util method withBasicAuthentication(String username, String password) in the HttpRequestBuilder API to generate and add the authorization header to the request?

[lolodomo]

@clinique: in the netatmo binding, the configuration setting "Password" is marked as password.
Should we consider marking as password also the settings "Client ID" and "Client Secret" ?
Hopefully, all these information are not logged but "Client ID" and "Client Secret" are readable in Paper UI.

[clinique]

Yes, I think at least Client Secret should be masked.