Dan Burkert has posted comments on this change. Change subject: security: authorize all RPCs against coarse-grained ACLs ......................................................................
Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/5998/6//COMMIT_MSG Commit Message: Line 9: This adds two new flags: 'superuser_acl' and 'client_acl'. Unless there is precedent for these names, I would suggest 'admin_acl' and 'user_acl' instead. Line 20: user, since it's the endpoint that exports signed IPKI certs. I'm not following this logic. Is this so a superuser can't request service credentials and then turn around and give those credentials to someone else? I don't think we can stop this kind of thing in practice; the superuser could just share its own credentials. It seems like allowing superuser access to all RPC methods would simplify this patch somewhat. http://gerrit.cloudera.org:8080/#/c/5998/6/src/kudu/security/init.cc File src/kudu/security/init.cc: PS6, Line 348: onwn typo http://gerrit.cloudera.org:8080/#/c/5998/6/src/kudu/security/simple_acl.cc File src/kudu/security/simple_acl.cc: PS6, Line 44: user use? http://gerrit.cloudera.org:8080/#/c/5998/6/src/kudu/server/server_base.cc File src/kudu/server/server_base.cc: Line 272: if (!messenger_->authentication_required()) { Is there a downside to moving this to OPTIONAL | REQUIRED? I know it doesn't improve security, but it would prevent accidental usage of APIs, when an ACL is set. -- To view, visit http://gerrit.cloudera.org:8080/5998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id24a6429273aff355e70e127086a26b7e4a03cd8 Gerrit-PatchSet: 6 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: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
