----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51705/#review148923 -----------------------------------------------------------
I agree with Nate in that there's a much larger problem looming here; all of the command JSON files stored for every agent has the passwords in plaintext. I think you should take time to design how the fix for that would work and then integrate it with the work you're doing here for alerts. ambari-agent/src/main/python/ambari_agent/ClusterConfiguration.py (lines 138 - 139) <https://reviews.apache.org/r/51705/#comment216441> `_mask_passwords_in_configurations` is still changing the values directly in the original `__configurations` dictionary. It should be making a copy to dump. ambari-agent/src/main/python/ambari_agent/ClusterConfiguration.py (lines 169 - 172) <https://reviews.apache.org/r/51705/#comment216439> This function doesn't do much anymore aside from just take params and pass them down the line. It can be removed in favor of just using the `_replace_passwords` directly ambari-server/src/main/java/org/apache/ambari/server/agent/AlertDefinitionCommand.java (lines 59 - 60) <https://reviews.apache.org/r/51705/#comment216442> Documentation around what this set is supposed to actually be ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java (lines 91 - 92) <https://reviews.apache.org/r/51705/#comment216443> Documentation around what this set is supposed to actually be ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java (lines 227 - 246) <https://reviews.apache.org/r/51705/#comment216445> This method is still inefficient in that it repeats the same work over and over for every command. It can simply cache the results of properties which are PASSWORD type and return that. - Jonathan Hurley On Sept. 12, 2016, 6:48 p.m., Anita Jebaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51705/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2016, 6:48 p.m.) > > > Review request for Ambari, Di Li, Jonathan Hurley, and Nate Cole. > > > Bugs: AMBARI-18334 > https://issues.apache.org/jira/browse/AMBARI-18334 > > > Repository: ambari > > > Description > ------- > > The configurations.json file loaded in the ambari-agent cache located at > /var/lib/ambari-agent/cache/cluster_configuration contains password details > in plaintext (Ex: ssl.client.keystore.password,ssl.client.truststore.password > etc.). The values are loaded both in the memory cache and file cache, the > file seems to be used only for debugging purposes, so it would be a better > approach to mask the passwords in the file. > > Approach: > > The password_config_type is included in the heartbeat response for alert > definition command and execution command, for which the values are dumped > into the json file. The password_config_type contains the information on > which properties in the configurations has the propertyType password. Based > on the response, the json is parsed and the password values are masked before > dumping it into the configurations.json file. > > > Diffs > ----- > > ambari-agent/src/main/python/ambari_agent/ClusterConfiguration.py 72b87be > ambari-agent/src/test/python/ambari_agent/TestAlerts.py 2bddc43 > ambari-agent/src/test/python/ambari_agent/TestClusterConfigurationCache.py > a418f6d > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapper.java > 0562c15 > > ambari-server/src/main/java/org/apache/ambari/server/agent/AlertDefinitionCommand.java > 4d2e048 > > ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java > 29737ee > > ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java > 70c24f9 > > Diff: https://reviews.apache.org/r/51705/diff/ > > > Testing > ------- > > Updated the test cases. > Ran mvn test. > > Manually tested by setting up a cluster, the password fields in the > configurations.json is masked. During testing, everytime the ambari agent is > restarted, it registers with the server and the memory cache and file cache > are updated, the alerts in turn uses the value from the memory cache. > > > Thanks, > > Anita Jebaraj > >
