Alexey Serbin has posted comments on this change. Change subject: [security] derive TSK params from authn token ones ......................................................................
Patch Set 4: (2 comments) > (2 comments) > > I like this change, it makes it simpler to understand the key > rotation intervals. I'm glad you find it useful. The inspiration comes from Todd's comments on the very first patch set :) http://gerrit.cloudera.org:8080/#/c/6071/4/src/kudu/security/token_signer.h File src/kudu/security/token_signer.h: PS4, Line 173: explicit > explicit is only necessary with 1-argument ctors, I think? It's possible to use initializer list as well, so it might be TokenSigner s = { 1, 2 }; I'm not sure we want to allow this. If yes, then sure -- explicit is not needed. Line 174: int64_t key_rotation_seconds, > Why not change the ctor to take authn_token_validity and key_rotation, inst I thought about that approach and decided to leave current set of parameters. The reason is that the authn token validity is not so crucial for the operation of the TokenSigner and, in principle, is independent from TSK rotation schedule -- it could be set to any value from 1 to maximum feasible defined by key_rotation_seconds and key_validity_seconds. However, if we want to have a tighter binding for TSK and authn tokens or want to make sure the token validity interval corresponds to the maximum feasible interval allowed by TokenSigner's key rotation schedule, then sure -- that would be the change to make. So, it's up to our judgment call: whether we want or don't want such coupling. Todd, Dan: what do you think, guys? -- To view, visit http://gerrit.cloudera.org:8080/6071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95bc64897ed16becda4ab8de6817695fdb48e9eb Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
