geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kevin Duling <kdul...@pivotal.io>
Subject Re: Review Request 57431: GEODE-2633: When turning on fine logging, GEODE logs the keystore password in clear text
Date Wed, 08 Mar 2017 22:40:27 GMT


> On March 8, 2017, 1:16 p.m., Jinmei Liao wrote:
> > Any test changes? We probably can create a integrated/dunit test that would start
a server with those ssl properties (including passwords) turned on, and have debug level truned
on, and security truned on as well, and have gfsh connect to it using username and password,
and see if any of the password show up in the logs.
> 
> Kevin Duling wrote:
>     `ArgumentRedactorJUnitTest` already has 85% method coverage and 95% line coverage
of `ArgumentRedactor`.  `SocketCreator.printConfig()` is a void method which only writes out
to the log file.  I could add a specific test for the expected ssl property, but it's already
covered by the comparison `compareKey.toLowerCase().contains("password");`
> 
> Jinmei Liao wrote:
>     ArugmentRedactor is defintely doing the right thing since we have test coverage on
that. It's the SocketCreator that is the problem here.

I disagree.  ArgumentRedactor just has to populate the StringBuilder with the correct string.
 The only additional piece is calling the logger to write the file, which is Log4J, not SocketCreator.
 I'm not clear on what additional testing would prove.

`RestSecurityWithSSLTest` is probably a good test to work from as it is an entry point for
this code.


- Kevin


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


On March 8, 2017, 12:58 p.m., Kevin Duling wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57431/
> -----------------------------------------------------------
> 
> (Updated March 8, 2017, 12:58 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-2633: When turning on fine logging, GEODE logs the keystore password in clear text
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java 742e7f3e93e595844675d0789b995e4ceb4431ac

>   geode-core/src/main/java/org/apache/geode/internal/util/ArgumentRedactor.java 419f3f976159e601c95a2042bafd96cc9fe9465f

> 
> 
> Diff: https://reviews.apache.org/r/57431/diff/1/
> 
> 
> Testing
> -------
> 
> precheckin running
> 
> 
> Thanks,
> 
> Kevin Duling
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message