Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11331 )
Change subject: Add missing authorization in KRPC ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11331/1/be/src/rpc/authentication.cc File be/src/rpc/authentication.cc: http://gerrit.cloudera.org:8080/#/c/11331/1/be/src/rpc/authentication.cc@608 PS1, Line 608: sasl::TSaslServer::SaslInit(GENERAL_CALLBACKS, APP_NAME.c_str()); > Alternatively, you could add these callbacks to the Kudu callback list for Todd pointed out during an offline discussion that KRPC internally implements authorization by overriding the authz_method defined in the RPC service. The authz_method is invoked for all RPCs. So, adding a SASL callback in some sense is adding another path which authorization is done. The argument for it is that it's a per-connection authorization instead of a per-RPC authorization. The latter may be needed if we need finer grained authorization (e.g. RPC method level authorization) so we can save some CPU overhead of having to parse the principal repeatedly for each RPC. It's unclear how much we are saving in the grand scheme of things. I will explore the alternative of overriding the SASL callbacks but IMHO, using the existing mechanism in KRPC to do the authorization (which is what's implemented in this patch) seems to be the easiest path for now. May help if Todd or others can chime in on their opinion. I think it may make sense to explore the option of adding a way for caller to specify Sasl callbacks as part of constructing a messenger. -- To view, visit http://gerrit.cloudera.org:8080/11331 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2f82dee5e721f2ed23e75fd91abbc6ab7addd4c5 Gerrit-Change-Number: 11331 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Mon, 27 Aug 2018 18:57:38 +0000 Gerrit-HasComments: Yes
