Alexey Serbin has posted comments on this change. Change subject: [security] avoid sparse seq numbers in TokenSigner ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/6329/1/src/kudu/security/token-test.cc File src/kudu/security/token-test.cc: Line 117: TEST_F(TokenTest, TestTokenSignerAddKeyAfterImport) { > As mentioned on slack, a test for the failure case in the commit message wo Good catch! Done: a new test added. PS1, Line 146: valie > valid Done http://gerrit.cloudera.org:8080/#/c/6329/1/src/kudu/security/token_signer.h File src/kudu/security/token_signer.h: PS1, Line 205: It's not a problem to call this method multiple times but call the AddKey() : // method only once, effectively discarding all the generated keys except for : // the key passed to the AddKey() call as a parameter. In other words, : // it's possible and not a problem to have 'holes' in the key sequence : // numbers. Other components working with verification of the signed tokens : // should take that into account. > Should this be updated? I think the following sequence would now be proble Right, this needs to be updated. Yep, the former sequence should fail during the second AddKey() call. The latter sequence should work with no issues. -- To view, visit http://gerrit.cloudera.org:8080/6329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib84f3d4f2596b3e75d1e0132f1e3362812d5799e Gerrit-PatchSet: 1 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: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
