[2/2] impala git commit: IMPALA-7585: Always set user credentials after creating RPC proxy

[kw...@apache.org]

IMPALA-7585: Always set user credentials after creating RPC proxy

kudu::rpc::Proxy() ctor may fail in GetLoggedInUser() for various reasons
(e.g. missing certain libraries). This resulted in an empty username being
used in kudu::rpc::ConnectionId. With plaintext SASL (e.g. in an insecure
Impala cluster), this may result in the following error during connection
negotiation:

Not authorized: Client connection negotiation failed: client connection to 127.0.0.1:27000: SASL(-1): generic failure: All-whitespace username.

In Impala, we don't consider failing GetLoggedInUser() a fatal error.
This change fixes the issue above by always explicitly setting the
username after creating the proxy. The username is "impala". Please
note that this username is not really used anywhere for authorization
for RPC services. Authorization is only done when authentication is
enabled with Kerberos. With Kerberos enabled, the username is derived
from the Kerberos principal instead of the user credentials set in
the ConnectionId. It's there mostly to satisfy the SASL plaintext case.

rpc-mgr-test has been updated to test for this string when Kerberos is
disabled.

Testing done: core test; rpc-mgr-test; rpc-mgr-kerberized-test

Change-Id: I75059f55bcdb8f95916610100cad4d8280daf3f6
Reviewed-on: http://gerrit.cloudera.org:8080/11477
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/e1d1b4f1
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/e1d1b4f1
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/e1d1b4f1

Branch: refs/heads/master
Commit: e1d1b4f14f1a2d8cab378b419eed3c4e4590a311
Parents: 20bde28
Author: Michael Ho <kw...@cloudera.com>
Authored: Tue Sep 18 23:59:01 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Tue Oct 2 21:55:44 2018 +0000

----------------------------------------------------------------------
 be/src/exec/kudu-util.h     | 1 +
 be/src/rpc/rpc-mgr-test.h   | 7 ++++++-
 be/src/rpc/rpc-mgr.inline.h | 8 ++++++++
 3 files changed, 15 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/e1d1b4f1/be/src/exec/kudu-util.h
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-util.h b/be/src/exec/kudu-util.h
index 8ed25a9..37755bd 100644
--- a/be/src/exec/kudu-util.h
+++ b/be/src/exec/kudu-util.h
@@ -21,6 +21,7 @@
 // TODO: Remove when toolchain callbacks.h properly defines ::tm.
 struct tm;
 
+#include <gutil/strings/substitute.h>
 #include <kudu/client/callbacks.h>
 #include <kudu/client/client.h>
 #include <kudu/client/value.h>

http://git-wip-us.apache.org/repos/asf/impala/blob/e1d1b4f1/be/src/rpc/rpc-mgr-test.h
----------------------------------------------------------------------
diff --git a/be/src/rpc/rpc-mgr-test.h b/be/src/rpc/rpc-mgr-test.h
index bd2ea96..d95df36 100644
--- a/be/src/rpc/rpc-mgr-test.h
+++ b/be/src/rpc/rpc-mgr-test.h
@@ -31,6 +31,7 @@
 #include "runtime/mem-tracker.h"
 #include "testutil/gtest-util.h"
 #include "testutil/scoped-flag-setter.h"
+#include "util/auth-util.h"
 #include "util/network-util.h"
 #include "util/openssl-util.h"
 #include "util/test-info.h"
@@ -175,7 +176,11 @@ class PingServiceImpl : public PingServiceIf {
 
   virtual bool Authorize(const google::protobuf::Message* req,
       google::protobuf::Message* resp, RpcContext* context) override {
-    return rpc_mgr_->Authorize("PingService", context, mem_tracker());
+    if (!IsKerberosEnabled()) {
+      return context->remote_user().username() == "impala";
+    } else {
+      return rpc_mgr_->Authorize("PingService", context, mem_tracker());
+    }
   }
 
   virtual void Ping(const PingRequestPB* request, PingResponsePB* response, RpcContext*

http://git-wip-us.apache.org/repos/asf/impala/blob/e1d1b4f1/be/src/rpc/rpc-mgr.inline.h
----------------------------------------------------------------------
diff --git a/be/src/rpc/rpc-mgr.inline.h b/be/src/rpc/rpc-mgr.inline.h
index 934b28b..9a1fc7e 100644
--- a/be/src/rpc/rpc-mgr.inline.h
+++ b/be/src/rpc/rpc-mgr.inline.h
@@ -24,6 +24,7 @@
 #include "kudu/rpc/messenger.h"
 #include "kudu/rpc/rpc_header.pb.h"
 #include "kudu/rpc/service_pool.h"
+#include "kudu/rpc/user_credentials.h"
 #include "util/network-util.h"
 
 namespace impala {
@@ -38,6 +39,13 @@ Status RpcMgr::GetProxy(const TNetworkAddress& address, const std::string& hostn
   kudu::Sockaddr sockaddr;
   RETURN_IF_ERROR(TNetworkAddressToSockaddr(address, &sockaddr));
   proxy->reset(new P(messenger_, sockaddr, hostname));
+
+  // Always set the user credentials as Proxy ctor may fail in GetLoggedInUser().
+  // An empty user name will result in SASL failure. See IMPALA-7585.
+  kudu::rpc::UserCredentials creds;
+  creds.set_real_user("impala");
+  (*proxy)->set_user_credentials(creds);
+
   return Status::OK();
 }