Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19873 )

Change subject: KUDU-3448 Move password retrieval to master init
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19873/3/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

http://gerrit.cloudera.org:8080/#/c/19873/3/src/kudu/master/catalog_manager.h@1400
PS3, Line 1400:   // Password for the encrypted IPKI and TSK private keys 
stored in the
              :   // sys-catalog table.
              :   std::string ipki_private_key_password_;
              :   std::string tsk_private_key_password_;
Does it make sense to obfuscate/scramble these a bit, so at least these strings 
wouldn't appear in core dumps?  Of course it would not solve the fundamental 
security issue, but at least it would make people feel better about this piece 
:)

As an input to the scrambler, there might be invariants of a running process: 
PID, etc.


http://gerrit.cloudera.org:8080/#/c/19873/3/src/kudu/master/master.cc
File src/kudu/master/master.cc:

http://gerrit.cloudera.org:8080/#/c/19873/3/src/kudu/master/master.cc@282
PS3, Line 282:   string ipki_private_key_password;
             :   if (!FLAGS_ipki_private_key_password_cmd.empty()) {
             :     RETURN_NOT_OK_PREPEND(security::GetPasswordFromShellCommand(
             :           FLAGS_ipki_private_key_password_cmd, 
&ipki_private_key_password),
             :         "could not get IPKI private key password from configured 
command");
             :   }
             :
             :   string tsk_private_key_password;
             :   if (!FLAGS_tsk_private_key_password_cmd.empty()) {
             :     RETURN_NOT_OK_PREPEND(security::GetPasswordFromShellCommand(
             :           FLAGS_tsk_private_key_password_cmd, 
&tsk_private_key_password),
             :         "could not get TSK private key password from configured 
command");
             :   }
Does it make sense to move this into the very beginning of the Init() method, 
so the forked process that executes the password-retrieval command has as less 
threads as possible?

If doing so, consider adding corresponding comment about the reasoning to place 
this code there.

Thanks!



--
To view, visit http://gerrit.cloudera.org:8080/19873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I746e657ae8d295f5f34225d63686beea1dff6b7c
Gerrit-Change-Number: 19873
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Comment-Date: Thu, 25 May 2023 20:36:39 +0000
Gerrit-HasComments: Yes

Reply via email to