STORM-3184: Mask the plaintext passwords from the logs #2801
Conversation
Introduce a `Password` config annotation and use it to mark configs that are sensitive and mask the values while logging.
| @@ -52,6 +79,16 @@ public static ConfigUtils setInstance(ConfigUtils u) { | |||
| return oldInstance; | |||
| } | |||
|
|
|||
| public static Map<String, Object> maskPasswords(final Map<String, Object> conf) { | |||
| Maps.EntryTransformer<String, Object, Object> maskPasswords = | |||
There was a problem hiding this comment.
Could we replace this with Java streams API and remove Guava dependency?
There was a problem hiding this comment.
The problem is that the map is traversed and copied even if we don't intend to log the conf and I was trying to avoid it using the Guava transformer. It may not be a major issue now since the conf is logged only once at the beginning, but wanted to avoid any potential future misuse.
I could not figure out a way to do the same with java 8 Streams API. I have refactored it to return a wrapped object which will be evaluated only when its required to log. Please take a look and let me know if it makes sense or if theres a better way.
There was a problem hiding this comment.
@arunmahadevan
Ah OK that makes sense. I should have made clear that my concern was adding guava to the dependency without shading. You can revert the logic with using shaded guava.
https://github.com/apache/storm/blob/master/shaded-deps/pom.xml#L207-L218
Sorry about the confusion.
There was a problem hiding this comment.
@arunmahadevan
I also think this approach can replace Utils.radactValue via adding @password to STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD.
What do you think? Looks like we don't need to have both approach.
There was a problem hiding this comment.
@HeartSaVioR , thanks for the suggestion, we can replace STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD too.
I was not aware of the "shaded-deps" module and its usage.
Reverted to the earlier approach and using the shaded-deps now.
|
@arunmahadevan Looks like we don't need to introduce Guava for this patch into master branch (maybe we are already using shaded artifact for guava?). I understand this is annoying to apply Java 7 / Java 8 for each branch, but let's keep this approach until we ship Storm 2.0.0. |
This reverts commit 0fe39c8.
Change-Id: Idb0775f1c1b91cbc648087e7752783f119e034b3
Introduce a
Passwordconfig annotation and use it to mark configs that aresensitive and mask the values while logging.
Master port of #2798