Todd Lipcon has posted comments on this change. Change subject: master: issue authentication tokens and CA certs to clients ......................................................................
Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/5871/1/src/kudu/master/authn_token_manager.cc File src/kudu/master/authn_token_manager.cc: Line 22: > nit: consider adding <gflags/gflags.h> Done http://gerrit.cloudera.org:8080/#/c/5871/1/src/kudu/master/authn_token_manager.h File src/kudu/master/authn_token_manager.h: PS1, Line 23: #include "kudu/util/status.h" > nit: consider forward-declaring Status instead of including this header fil Done http://gerrit.cloudera.org:8080/#/c/5871/1/src/kudu/master/master.proto File src/kudu/master/master.proto: PS1, Line 587: > client? Done http://gerrit.cloudera.org:8080/#/c/5871/2/src/kudu/master/master.proto File src/kudu/master/master.proto: Line 548: // TODO(PKI): should the client specify whether it wants an authn token or not? > You hint at it later, but I think the correct thing to do here is not have yea, I think so too. Leaving the TODO here so we come back to it and evalaute before we call this stuff finished. PS2, Line 549: server > Is this supposed to be client? yea, Alexey caught this too. Fixed. Line 563: repeated bytes ca_cert_der = 3; > We've previously discussed cert rolling and decided to punt, although maybe I figured that if we make it repeated now, and have the client add all returned CAs to the store, then we'll be able to get back-compatibility with old clients if/when we add this. Even if we never use it, the extra complexity of a for loop here in the client doesn't cost us much. Does that make sense? http://gerrit.cloudera.org:8080/#/c/5871/1/src/kudu/master/master_service.cc File src/kudu/master/master_service.cc: PS1, Line 368: and not by another token. > Is it already possible to get here authenticating by an authn token? currently it's not, because the tokens aren't used for authentication yet (Dan's working on that). But once Dan's done, it will be, and we'll probably want to be sure this isn't a "back door" to endless token renewal. -- To view, visit http://gerrit.cloudera.org:8080/5871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5969b8e125633b3b14364b98c0d0a992b162f302 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
