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.
> I guess that could work, I assumed they were separated for a reason.
Right -- token signing keys (TSKs) are separate from the root CA private key.  
A few reasons for separation that I can list right away are the following:
  * TSKs and CA private key are used for signing different entities: Kudu 
server certificates and authn tokens.  Server certificates are given to 
kudu-master and kudu-tserver, while authn tokens are handled over to Kudu 
clients.
  * Authn tokens are set to expire pretty fast (one week) compared with the CA 
cert expiration (years).
  * There was a requirement of revoking a TSK if a token signed with the TSK 
has been leaked/compromised (that hasn't been implemented, but current design 
allows to do so, if needed).
  * We might use shorter/lighter keys for TSKs for performance reasons, and 
shorten their validity period.

Nevertheless, if thinking about encrypting the keys, I guess we could use the 
CA private key to encrypt the token signing keys.  I hope the requirements 
don't prohibit doing that (FIPS/FISMA, etc.), right?


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();
            :           }
            :     ));
> I think that makes sense. I'll try that.
Thanks!



--
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: Tue, 28 Mar 2023 23:03:50 +0000
Gerrit-HasComments: Yes

Reply via email to