Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19617 )
Change subject: KUDU-3448 Add support for encrypting TSKs ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/19617/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19617/6//COMMIT_MSG@9 PS6, Line 9: In a previous patch, support for encrypting IPKI root CA private keys : has been added. This is a follow up patch, to add encryption support for : token signing keys as well. It is controlled by a new flag: : --tsk_private_key_password_cmd. Would it be possible to use the IPKI root CA private key in some form as the secret to encrypt the token signing keys? http://gerrit.cloudera.org:8080/#/c/19617/6/src/kudu/security/token_signing_key.cc File src/kudu/security/token_signing_key.cc: http://gerrit.cloudera.org:8080/#/c/19617/6/src/kudu/security/token_signing_key.cc@98 PS6, Line 98: CHECK_OK(key_->ToEncryptedString(&private_key_der_, DataFormat::DER, : [&](string* password){ : RETURN_NOT_OK_PREPEND(GetPasswordFromShellCommand( : FLAGS_tsk_private_key_password_cmd, password), : "could not get TSK private key password from configured command"); : return Status::OK(); : } : )); Since a token signing key cay be created at any arbitrary point of kudu-master's lifetime, it could be very expensive to call fork() on a multi-gigabyte process with hundreds of threads (and locks!) just to run a shell command that obtains a secret for key encryption. Also, since there might be several of token signing keys out there, loading them upon becoming a leader master will make it slow and decrease the HA of a multi-master Kudu cluster. One solution might be using a sub-process server running aside the big-and-bloated kudu-master for running those shell commands. However, in this context I'd rather vote for using the IPKI private key in some form as the secret for encrypting the TSK keys in the system catalog table. That way the key-encrypting stuff would have less knobs, and that's better from the usability and maintenance standpoints at least. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/19617 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8d770de7ed824cfc725003bbe77f1e42629029b Gerrit-Change-Number: 19617 Gerrit-PatchSet: 6 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Ádám Bakai <[email protected]> Gerrit-Comment-Date: Wed, 22 Mar 2023 21:24:22 +0000 Gerrit-HasComments: Yes
