Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Anita Jebaraj]

> On Sept. 30, 2016, 8:14 p.m., Nate Cole wrote:
> > What is the state of this review?

Hi Nate...based on your comments and the comments from Jonathan Hurley, it is 
clear that writing out the passwords in the commands*.json and just masking it 
in the configurations.json might not be an effective approach. I am working to 
find a solution which would be a fix to the overall problem. I will close the 
review as discarded for now and will reopen it once I come up with the right 
solution.


- Anita


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/#review151074
---


On Sept. 12, 2016, 10: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, 10: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
> 
>

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Nate Cole]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/#review151074
---



What is the state of this review?

- Nate Cole


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
> 
>

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Jonathan Hurley]

---
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)


`_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)


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)


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)


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)


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
> 
>

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Nate Cole]

> On Sept. 13, 2016, 8:37 a.m., Nate Cole wrote:
> > ambari-agent/src/main/python/ambari_agent/ClusterConfiguration.py, lines 
> > 138-140
> > 
> >
> > Again, I'm not sure what you're fixing here.  Every execution command 
> > is going to have the password properties in them anyway, and there will be 
> > hundreds in /var/lib/ambari-agent/data.
> 
> Anita Jebaraj wrote:
> Hi Nate, the idea here is to avoid using plaintext passwords in the files 
> that are saved in the ambari agent cache. To begin with I am trying to come 
> up with an approach to mask the passwords in the configurations.json, since 
> it is used only for debugging purposes and hence no impact is expected. If 
> the approach is accepted, I would like to continue the work of masking the 
> passwords in all the files in the agent side by creating new JIRAs.

That's what I'm saying - since the stack scripts run in their OWN process 
separate from the agent, you can't mask them on serialization.  Flow is:
- Server sends down commands, with passwords in configuration.
- Agent serializes command to disk with passwords
- Agent invokes stack script
- Stack script reads command file with passwords in it

That second step is the important one; we can't mask and persist passwords that 
haven't yet been read.  You're right in that alerts may use only the in-memory 
representation, but that strategy won't work for other stack scripts.


- Nate


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/#review148705
---


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
> 
>

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Anita Jebaraj]

> On Sept. 13, 2016, 12:37 p.m., Nate Cole wrote:
> > ambari-agent/src/main/python/ambari_agent/ClusterConfiguration.py, lines 
> > 138-140
> > 
> >
> > Again, I'm not sure what you're fixing here.  Every execution command 
> > is going to have the password properties in them anyway, and there will be 
> > hundreds in /var/lib/ambari-agent/data.

Hi Nate, the idea here is to avoid using plaintext passwords in the files that 
are saved in the ambari agent cache. To begin with I am trying to come up with 
an approach to mask the passwords in the configurations.json, since it is used 
only for debugging purposes and hence no impact is expected. If the approach is 
accepted, I would like to continue the work of masking the passwords in all the 
files in the agent side by creating new JIRAs.


- Anita


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/#review148705
---


On Sept. 12, 2016, 10: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, 10: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
> 
>

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Nate Cole]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/#review148705
---




ambari-agent/src/main/python/ambari_agent/ClusterConfiguration.py (lines 137 - 
139)


Again, I'm not sure what you're fixing here.  Every execution command is 
going to have the password properties in them anyway, and there will be 
hundreds in /var/lib/ambari-agent/data.


- Nate Cole


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
> 
>

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Anita Jebaraj]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/
---

(Updated Sept. 12, 2016, 10: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 (updated)
-

  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

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Anita Jebaraj]

> On Sept. 8, 2016, 5:42 p.m., Jonathan Hurley wrote:
> > ambari-agent/src/main/python/ambari_agent/ClusterConfiguration.py, line 103
> > 
> >
> > This will cause problems since you're storing the obfuscated 
> > properties. If a command needed the property from the in-memory cache, it 
> > would not have the correct value anymore.
> > 
> > Perhaps this approach is flawed in that there's no credential store 
> > being used for the in-memory passwords.

Thanks for the catch, I updated the code to mask passwords only while dumping 
in the file cache, but missed to remove the code. Was not able to capture this 
in the testing. Thank you.


> On Sept. 8, 2016, 5:42 p.m., Jonathan Hurley wrote:
> > ambari-agent/src/main/python/ambari_agent/ClusterConfiguration.py, line 174
> > 
> >
> > `i` is not a very good name here; maybe something a little clearer.

Updated the code


> On Sept. 8, 2016, 5:42 p.m., Jonathan Hurley wrote:
> > ambari-agent/src/main/python/ambari_agent/ClusterConfiguration.py, lines 
> > 179-181
> > 
> >
> > That's a lot of iterating - over the entire config dictionary per-key 
> > that needs replacing - also you never exit once you have a hit.
> > 
> > This function needs to be cleaned up for better performance.

Updated the code with a different approach


> On Sept. 8, 2016, 5:42 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapper.java,
> >  lines 182-183
> > 
> >
> > What about status commands - those send down configurations too?

The configurations in the status commands are not dumped into the 
configurations.json file. The values are dumped only for alert definition and 
execution commands.


> On Sept. 8, 2016, 5:42 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java,
> >  lines 252-275
> > 
> >
> > This seems like it should be cached once after calculation for the 
> > cluster so that it doesn't need to keep generating it over and over.

Updated the code with a different approach. Instead of getting the value from 
the Ambari Metainfo, I am getting the value from the config based on the 
desired Tags. Similar approach is being used for getting the required config 
properties for the response.


> On Sept. 8, 2016, 5:42 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java,
> >  line 270
> > 
> >
> > You call this property "password config types" and bind a name to a 
> > type, but then never check the type for PASSWORD in python. I'd say either 
> > make this a flat list of password-protected fields or enhance the python 
> > code to extract properties of type PASSWORD to check on.

Yes you are right, key-value pair is not required in this case. I have updated 
the code


- Anita


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/#review148212
---


On Sept. 12, 2016, 8:20 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, 8:20 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.
> 

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Anita Jebaraj]

> On Sept. 8, 2016, 3:25 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java,
> >  lines 252-257
> > 
> >
> > Will this be a performance issue?  On large clusters this can be a lot 
> > of iterating for alert and execution commands.

I have changed the approach for finding the property Types. Instead of getting 
the value from the Ambari Metainfo, I am getting the value from the Config 
based on the desired Tags. Similar approach is being used for getting the 
required config properties for the response.


- Anita


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/#review148191
---


On Sept. 12, 2016, 8:20 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, 8:20 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
> 
>

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Anita Jebaraj]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/
---

(Updated Sept. 12, 2016, 8:20 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 (updated)
-

  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

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Nate Cole]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/#review148191
---



I'm not sure what you're fixing here.  Every execution command is going to have 
the password properties in them anyway, and there will be hundreds in 
/var/lib/ambari-agent/data.  Obfuscating for alerts isn't going to be a huge 
deterrent.


ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java 
(lines 252 - 257)


Will this be a performance issue?  On large clusters this can be a lot of 
iterating for alert and execution commands.


- Nate Cole


On Sept. 7, 2016, 5:14 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51705/
> ---
> 
> (Updated Sept. 7, 2016, 5:14 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 e114daa 
>   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
> 
>

Re: Review Request 51705: Password in the configurations.json file in the ambari-agent cache is not encrypted

[Anita Jebaraj]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51705/
---

(Updated Sept. 7, 2016, 9:14 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 e114daa 
  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)
---

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