Alexey Serbin has posted comments on this change.

Change subject: [security] tailored TokenSigner for system catalog
......................................................................


Patch Set 7:

> (3 comments)

Thank you for the review. 

 > I have some high level questions about the direction of TokenSigner
 > and AuthnTokenManager.
 > 
 > Why is the TokenSigner holding all of the previous TSK public keys?
 >  I wouldn't expect a class called 'TokenSigner' to need anything
 > except a single private key and the current sequence number.

Right, that's the only things it needs.  I had a though to put the public parts 
of the keys into the pairing TokenVerifier or something like that, but that 
would require re-doing current scheme of TokenSigner/TokenVerifier.  I agree it 
would be a cleaner approach.

 > I realize that something in the master will need to track of the TSK
 > public keys, but that's true of the tserver as well (which
 > ostensibly won't have a TokenSigner), so I don't see why it should
 > be in the TokenSigner.

Yes, and that's already taken care of by TokenVerifier -- Todd added those two 
classes some time ago.  Take a look at src/kudu/security/token-test.cc to see 
the intended usage of those.

 > 
 > Right now AuthnTokenManager is just a thin wrapper around
 > TokenSigner.  Is it pulling its weight?  Are we planning to add
 > more responsibilities to AuthnTokenManager in the future which
 > would make it more than just a wrapper?

Good observation.  As I understand, AuthnTokenManager was modeled to run along 
with MasterCertAuthority.  Besides aggregating TokenSigner, it has just one 
additional perk of combining the code which outputs signed Token.  We can 
safely move that code into the TokenSigner itself.  I'm not aware of future 
plans to add some extra functionality into the AuthTokenManager.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: No

Reply via email to