kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From granthe...@apache.org
Subject kudu git commit: KUDU-2259: add real user to AuthenticationCredentialsPB
Date Thu, 15 Mar 2018 03:50:25 GMT
Repository: kudu
Updated Branches:
  refs/heads/branch-1.7.x 132da394d -> c243d68be


KUDU-2259: add real user to AuthenticationCredentialsPB

This commit adds the 'real user' to the authn credentials token, which
is used when negotiating connections with SASL PLAIN authentication.
This is useful when scan tokens are being sent to remote tasks, it's not
possible to authenticate with a signed authn token to the remote
server[1], coarse-grained ACLs have been set, and the 'planner' and
'executor' processes are being run with different users.

This problematic scenario might also have been solved by allowing tokens
to be used in all scenarios, even when encryption is disabled, but the
approach taken by this commit allows that invariant to remain.

[1]: this most often occurs because the remote server has encryption disabled.

Change-Id: I5d2d901d42501ecfc0f6372f68cf7335eb188b45
Reviewed-on: http://gerrit.cloudera.org:8080/9374
Reviewed-by: Todd Lipcon <todd@apache.org>
Tested-by: Kudu Jenkins
(cherry picked from commit e684de3371941cc5ae8fc4a546ecda7dbe9f4f2f)
Reviewed-on: http://gerrit.cloudera.org:8080/9647
Reviewed-by: Grant Henke <granthenke@gmail.com>
Tested-by: Grant Henke <granthenke@gmail.com>


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

Branch: refs/heads/branch-1.7.x
Commit: c243d68be01d6172df75069d343fed136df34c2b
Parents: 132da39
Author: Dan Burkert <danburkert@apache.org>
Authored: Tue Feb 20 17:37:17 2018 -0800
Committer: Grant Henke <granthenke@gmail.com>
Committed: Thu Mar 15 03:48:20 2018 +0000

----------------------------------------------------------------------
 .../java/org/apache/kudu/client/Negotiator.java |  8 +-
 .../org/apache/kudu/client/SecurityContext.java | 47 ++++++----
 .../client/TestSecurityContextRealUser.java     | 97 ++++++++++++++++++++
 src/kudu/client/client-internal.cc              |  2 +
 src/kudu/client/client-internal.h               |  5 +
 src/kudu/client/client-test.cc                  | 36 ++++++++
 src/kudu/client/client.cc                       | 23 ++++-
 src/kudu/client/client.proto                    |  7 +-
 src/kudu/client/master_rpc.cc                   | 10 ++
 src/kudu/client/master_rpc.h                    |  5 +
 src/kudu/client/meta_cache.cc                   |  1 +
 src/kudu/mini-cluster/external_mini_cluster.cc  |  6 +-
 src/kudu/rpc/connection_id.cc                   |  1 +
 src/kudu/rpc/server_negotiation.cc              |  6 +-
 src/kudu/rpc/user_credentials.cc                |  7 +-
 src/kudu/rpc/user_credentials.h                 |  5 +
 src/kudu/util/user.cc                           |  2 +-
 17 files changed, 235 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
index 95ad907..47e4854 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
@@ -92,7 +92,7 @@ import org.apache.kudu.util.SecurityUtil;
 public class Negotiator extends SimpleChannelUpstreamHandler {
   private static final Logger LOG = LoggerFactory.getLogger(Negotiator.class);
 
-  private static final SaslClientCallbackHandler SASL_CALLBACK = new SaslClientCallbackHandler();
+  private final SaslClientCallbackHandler SASL_CALLBACK = new SaslClientCallbackHandler();
   private static final Set<RpcHeader.RpcFeatureFlag> SUPPORTED_RPC_FEATURES =
       ImmutableSet.of(
           RpcHeader.RpcFeatureFlag.APPLICATION_FEATURE_FLAGS,
@@ -772,7 +772,7 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
 
     // The UserInformationPB is deprecated, but used by servers prior to Kudu 1.1.
     RpcHeader.UserInformationPB.Builder userBuilder = RpcHeader.UserInformationPB.newBuilder();
-    String user = System.getProperty("user.name");
+    String user = securityContext.getRealUser();
     userBuilder.setEffectiveUser(user);
     userBuilder.setRealUser(user);
     builder.setDEPRECATEDUserInfo(userBuilder.build());
@@ -826,11 +826,11 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
     }
   }
 
-  private static class SaslClientCallbackHandler implements CallbackHandler {
+  private class SaslClientCallbackHandler implements CallbackHandler {
     public void handle(Callback[] callbacks) throws UnsupportedCallbackException {
       for (Callback callback : callbacks) {
         if (callback instanceof NameCallback) {
-          ((NameCallback) callback).setName(System.getProperty("user.name"));
+          ((NameCallback) callback).setName(securityContext.getRealUser());
         } else if (callback instanceof PasswordCallback) {
           ((PasswordCallback) callback).setPassword(new char[0]);
         } else {

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java b/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
index 613491b..3183b07 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
@@ -71,6 +71,9 @@ class SecurityContext {
   @Nullable
   private SignedTokenPB authnToken;
 
+  @GuardedBy("this")
+  private String realUser;
+
   private final DelegatedTrustManager trustManager = new DelegatedTrustManager();
 
   /**
@@ -107,7 +110,7 @@ class SecurityContext {
      * caller did not provide a Subject with appropriate credentials).
      */
     NONE
-  };
+  }
 
   @Nonnull
   private final SubjectType subjectType;
@@ -116,6 +119,7 @@ class SecurityContext {
    * The currently trusted CA certs, in DER format.
    */
   @VisibleForTesting
+  @GuardedBy("this")
   List<ByteString> trustedCertDers = Collections.emptyList();
 
   @GuardedBy("subjectLock")
@@ -130,6 +134,8 @@ class SecurityContext {
       this.subjectType = p.getFirst();
       this.subject = p.getSecond();
 
+      this.realUser = System.getProperty("user.name");
+
       this.sslContextWithCert = SSLContext.getInstance("TLS");
       sslContextWithCert.init(null, new TrustManager[] { trustManager }, null);
 
@@ -257,16 +263,19 @@ class SecurityContext {
     }
   }
 
+  public synchronized String getRealUser() {
+    return realUser;
+  }
+
   @Nullable
   public synchronized byte[] exportAuthenticationCredentials() {
-    if (authnToken == null || !hasTrustedCerts()) {
-      return null;
+    AuthenticationCredentialsPB.Builder pb = AuthenticationCredentialsPB.newBuilder();
+    pb.setRealUser(realUser);
+    if (authnToken != null) {
+      pb.setAuthnToken(authnToken);
     }
-
-    return AuthenticationCredentialsPB.newBuilder()
-        .setAuthnToken(authnToken)
-        .addAllCaCertDers(trustedCertDers)
-        .build().toByteArray();
+    pb.addAllCaCertDers(trustedCertDers);
+    return pb.build().toByteArray();
   }
 
   private static String getUserFromToken(SignedTokenPB token)
@@ -290,19 +299,23 @@ class SecurityContext {
   public synchronized void importAuthenticationCredentials(byte[] authnData) {
     try {
       AuthenticationCredentialsPB pb = AuthenticationCredentialsPB.parseFrom(authnData);
-      if (authnToken != null) {
+      if (pb.hasAuthnToken() && authnToken != null) {
+        // TODO(todd): also check that, if there is a JAAS subject, that
+        // the subject in the imported authn token matches the Kerberos
+        // principal in the JAAS subject. Alternatively, this could
+        // completely disable the JAAS authentication path (assumedly if
+        // we import a token, we want to _only_ act as the user in that
+        // token, and would rather have a connection failure than flip
+        // back to GSSAPI transparently).
         checkUserMatches(authnToken, pb.getAuthnToken());
       }
-      // TODO(todd): also check that, if there is a JAAS subject, that
-      // the subject in the imported authn token matces the Kerberos
-      // principal in the JAAS subject. Alternatively, this could
-      // completely disable the JAAS authentication path (assumedly if
-      // we import a token, we want to _only_ act as the user in that
-      // token, and would rather have a connection failure than flip
-      // back to GSSAPI transparently.
-      trustCertificates(pb.getCaCertDersList());
+
       authnToken = pb.getAuthnToken();
+      trustCertificates(pb.getCaCertDersList());
 
+      if (pb.hasRealUser()) {
+        realUser = pb.getRealUser();
+      }
     } catch (InvalidProtocolBufferException | CertificateException e) {
       throw new IllegalArgumentException(e);
     }

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java
b/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java
new file mode 100644
index 0000000..ba30763
--- /dev/null
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java
@@ -0,0 +1,97 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.kudu.client;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * Tests that the 'real user' field of the security context is used for
+ * SASL PLAIN negotiations, and is imported from the SecurityCredentialsPB.
+ */
+public class TestSecurityContextRealUser extends BaseKuduTest {
+  private static final Logger LOG = LoggerFactory.getLogger(TestSecurityContextRealUser.class);
+
+  private String tableName;
+
+  @Before
+  public void setTableName() {
+    tableName = TestSecurityContextRealUser.class.getName() + "-" + System.currentTimeMillis();
+  }
+
+  @BeforeClass
+  public static void setUpBeforeClass() throws Exception {
+    // This test requires a delicate setup. We enable Kerberos, make
+    // authentication optional, and set the superuser ACL to test-admin so that
+    // the external mini-cluster is able to connect to the master while creating
+    // the cluster. The user ACL is scoped to a different user so that we can
+    // test real user name propagation.
+    miniClusterBuilder.enableKerberos()
+                      .addMasterFlag("--user-acl=token-user")
+                      .addMasterFlag("--superuser-acl=test-admin")
+                      .addMasterFlag("--rpc-authentication=optional")
+                      .addMasterFlag("--rpc-trace-negotiation")
+                      .addTserverFlag("--user-acl=token-user")
+                      .addTserverFlag("--superuser-acl=test-admin")
+                      .addTserverFlag("--rpc-authentication=optional")
+                      .addTserverFlag("--rpc-trace-negotiation");
+
+    BaseKuduTest.setUpBeforeClass();
+  }
+
+  @Test
+  public void test() throws Exception {
+    // Clear out the Kerberos credentials in the environment.
+    miniCluster.kdestroy();
+
+    // Create a new client instance with the logged in user, and ensure that it
+    // fails to connect (the logged in user is not in the user-acl).
+    try (KuduClient client =
+             new KuduClient.KuduClientBuilder(miniCluster.getMasterAddresses()).build())
{
+      client.listTabletServers();
+      fail();
+    } catch (KuduException e) {
+      // TODO(KUDU-2344): This should fail with NotAuthorized.
+      assertTrue(e.getStatus().toString(), e.getStatus().isServiceUnavailable());
+    }
+
+    // Try again with a correct real user.
+    try (KuduClient client =
+             new KuduClient.KuduClientBuilder(miniCluster.getMasterAddresses()).build())
{
+      Client.AuthenticationCredentialsPB credentials =
+          Client.AuthenticationCredentialsPB.newBuilder().setRealUser("token-user").build();
+      client.importAuthenticationCredentials(credentials.toByteArray());
+      client.listTabletServers();
+
+      // Smoke-test tserver connection by scanning a table.
+      KuduTable table = client.createTable(tableName, getBasicSchema(),
+                                           new CreateTableOptions().setRangePartitionColumns(
+                                               new ArrayList<String>()));
+      assertEquals(0, scanTableToStrings(table).size());
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/client/client-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-internal.cc b/src/kudu/client/client-internal.cc
index 281e9a7..b3c703d 100644
--- a/src/kudu/client/client-internal.cc
+++ b/src/kudu/client/client-internal.cc
@@ -689,6 +689,7 @@ void KuduClient::Data::ConnectedToClusterCb(
     if (status.ok()) {
       leader_master_hostport_ = HostPort(leader_hostname, leader_addr.port());
       master_proxy_.reset(new MasterServiceProxy(messenger_, leader_addr, leader_hostname));
+      master_proxy_->set_user_credentials(user_credentials_);
     }
   }
 
@@ -785,6 +786,7 @@ void KuduClient::Data::ConnectToClusterAsync(KuduClient* client,
         deadline,
         client->default_rpc_timeout(),
         messenger_,
+        user_credentials_,
         creds_policy));
 
     if (creds_policy == CredentialsPolicy::PRIMARY_CREDENTIALS) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/client/client-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-internal.h b/src/kudu/client/client-internal.h
index a4ca44c..e77e3bf 100644
--- a/src/kudu/client/client-internal.h
+++ b/src/kudu/client/client-internal.h
@@ -33,6 +33,7 @@
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/rpc/rpc_controller.h"
+#include "kudu/rpc/user_credentials.h"
 #include "kudu/util/atomic.h"
 #include "kudu/util/locks.h"
 #include "kudu/util/monotime.h"
@@ -228,6 +229,10 @@ class KuduClient::Data {
   // The unique id of this client.
   std::string client_id_;
 
+  // The user credentials of the client. This field is constant after the client
+  // is built.
+  rpc::UserCredentials user_credentials_;
+
   // The request tracker for this client.
   scoped_refptr<rpc::RequestTracker> request_tracker_;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/client/client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index 531b5b1..5b37ca1 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -44,6 +44,7 @@
 #include "kudu/client/client-internal.h"
 #include "kudu/client/client-test-util.h"
 #include "kudu/client/client.h"
+#include "kudu/client/client.pb.h"
 #include "kudu/client/error_collector.h"
 #include "kudu/client/meta_cache.h"
 #include "kudu/client/resource_metrics.h"
@@ -112,6 +113,7 @@ DECLARE_bool(allow_unsafe_replication_factor);
 DECLARE_bool(fail_dns_resolution);
 DECLARE_bool(log_inject_latency);
 DECLARE_bool(master_support_connect_to_master_rpc);
+DECLARE_bool(rpc_trace_negotiation);
 DECLARE_int32(heartbeat_interval_ms);
 DECLARE_int32(leader_failure_exp_backoff_max_delta_ms);
 DECLARE_int32(log_inject_latency_ms_mean);
@@ -124,6 +126,8 @@ DECLARE_int32(scanner_inject_latency_on_each_batch_ms);
 DECLARE_int32(scanner_max_batch_size_bytes);
 DECLARE_int32(scanner_ttl_ms);
 DECLARE_int32(table_locations_ttl_ms);
+DECLARE_string(superuser_acl);
+DECLARE_string(user_acl);
 DEFINE_int32(test_scan_num_rows, 1000, "Number of rows to insert and scan");
 
 METRIC_DECLARE_counter(rpcs_queue_overflow);
@@ -5497,5 +5501,37 @@ TEST_F(ClientTest, TestSubsequentScanRequestReturnsNoData) {
   ASSERT_EQ(0, count);
 }
 
+// Test that the 'real user' included in AuthenticationCredentialsPB is used
+// when the client connects to remote servers with SASL PLAIN.
+TEST_F(ClientTest, TestAuthenticationCredentialsRealUser) {
+  // Scope down the user ACLs and restart the cluster to have it take effect.
+  FLAGS_user_acl = "token-user";
+  FLAGS_superuser_acl = "token-user";
+  FLAGS_rpc_trace_negotiation = true;
+  cluster_->ShutdownNodes(cluster::ClusterNodes::ALL);
+  ASSERT_OK(cluster_->StartSync());
+
+  // Try to connect without setting the user, which should fail
+  // TODO(KUDU-2344): This should fail with NotAuthorized.
+  ASSERT_TRUE(cluster_->CreateClient(nullptr, &client_).IsRemoteError());
+
+  // Create a new client with the imported user name and smoke test it.
+  KuduClientBuilder client_builder;
+  string authn_creds;
+  AuthenticationCredentialsPB pb;
+  pb.set_real_user("token-user");
+  ASSERT_TRUE(pb.SerializeToString(&authn_creds));
+  client_builder.import_authentication_credentials(authn_creds);
+
+  // Recreate the client and open the table.
+  ASSERT_OK(cluster_->CreateClient(&client_builder, &client_));
+  ASSERT_OK(client_->OpenTable(client_table_->name(), &client_table_));
+
+  // Insert some rows and do a scan to force a new connection to the tablet servers.
+  NO_FATALS(InsertTestRows(client_table_.get(), FLAGS_test_scan_num_rows));
+  vector<string> rows;
+  KuduScanner scanner(client_table_.get());
+  ASSERT_OK(ScanToStrings(&scanner, &rows));
+}
 } // namespace client
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/client/client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index e067c04..08ee8e4 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -78,6 +78,7 @@
 #include "kudu/rpc/request_tracker.h"
 #include "kudu/rpc/rpc_controller.h"
 #include "kudu/rpc/sasl_common.h"
+#include "kudu/rpc/user_credentials.h"
 #include "kudu/security/cert.h"
 #include "kudu/security/openssl_util.h"
 #include "kudu/security/tls_context.h"
@@ -112,14 +113,14 @@ using kudu::master::ListTabletServersRequestPB;
 using kudu::master::ListTabletServersResponsePB;
 using kudu::master::ListTabletServersResponsePB_Entry;
 using kudu::master::MasterServiceProxy;
+using kudu::master::TSInfoPB;
 using kudu::master::TableIdentifierPB;
 using kudu::master::TabletLocationsPB;
-using kudu::master::TSInfoPB;
 using kudu::rpc::Messenger;
 using kudu::rpc::MessengerBuilder;
 using kudu::rpc::RpcController;
+using kudu::rpc::UserCredentials;
 using kudu::tserver::ScanResponsePB;
-using std::pair;
 using std::set;
 using std::string;
 using std::unique_ptr;
@@ -285,8 +286,9 @@ KuduClientBuilder& KuduClientBuilder::import_authentication_credentials(string
a
 }
 
 namespace {
-Status ImportAuthnCredsToMessenger(const string& authn_creds,
-                                   Messenger* messenger) {
+Status ImportAuthnCreds(const string& authn_creds,
+                        Messenger* messenger,
+                        UserCredentials* user_credentials) {
   AuthenticationCredentialsPB pb;
   if (!pb.ParseFromString(authn_creds)) {
     return Status::InvalidArgument("invalid authentication data");
@@ -300,6 +302,9 @@ Status ImportAuthnCredsToMessenger(const string& authn_creds,
     }
     messenger->set_authn_token(tok);
   }
+  if (pb.has_real_user()) {
+    user_credentials->set_real_user(pb.real_user());
+  }
   for (const string& cert_der : pb.ca_cert_ders()) {
     security::Cert cert;
     RETURN_NOT_OK_PREPEND(cert.FromString(cert_der, security::DataFormat::DER),
@@ -318,14 +323,21 @@ Status KuduClientBuilder::Build(shared_ptr<KuduClient>* client)
{
   MessengerBuilder builder("client");
   std::shared_ptr<Messenger> messenger;
   RETURN_NOT_OK(builder.Build(&messenger));
+  UserCredentials user_credentials;
 
   // Parse and import the provided authn data, if any.
   if (!data_->authn_creds_.empty()) {
-    RETURN_NOT_OK(ImportAuthnCredsToMessenger(data_->authn_creds_, messenger.get()));
+    RETURN_NOT_OK(ImportAuthnCreds(data_->authn_creds_, messenger.get(), &user_credentials));
+  }
+  if (!user_credentials.has_real_user()) {
+    // If there are no authentication credentials, then set the real user to the
+    // currently logged-in user.
+    RETURN_NOT_OK(user_credentials.SetLoggedInRealUser());
   }
 
   shared_ptr<KuduClient> c(new KuduClient);
   c->data_->messenger_ = std::move(messenger);
+  c->data_->user_credentials_ = std::move(user_credentials);
   c->data_->master_server_addrs_ = data_->master_server_addrs_;
   c->data_->default_admin_operation_timeout_ = data_->default_admin_operation_timeout_;
   c->data_->default_rpc_timeout_ = data_->default_rpc_timeout_;
@@ -593,6 +605,7 @@ Status KuduClient::ExportAuthenticationCredentials(string* authn_creds)
const {
   if (tok) {
     pb.mutable_authn_token()->CopyFrom(*tok);
   }
+  pb.set_real_user(data_->user_credentials_.real_user());
 
   vector<string> cert_ders;
   RETURN_NOT_OK_PREPEND(data_->messenger_->tls_context().DumpTrustedCerts(&cert_ders),

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/client/client.proto
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.proto b/src/kudu/client/client.proto
index 0cab818..eff26e9 100644
--- a/src/kudu/client/client.proto
+++ b/src/kudu/client/client.proto
@@ -103,11 +103,16 @@ message ScanTokenPB {
   optional ReplicaSelection replica_selection = 16 [default = LEADER_ONLY];
 }
 
-
 // All of the data necessary to authenticate to a cluster from a client with
 // no Kerberos credentials.
 message AuthenticationCredentialsPB {
+
+  // A signed token with user credentials issued by the master.
   optional security.SignedTokenPB authn_token = 1;
 
+  // The real user name, used for SASL PLAIN authentication.
+  optional string real_user = 3;
+
+  // Trusted root CA certificates.
   repeated bytes ca_cert_ders = 2;
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/client/master_rpc.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/master_rpc.cc b/src/kudu/client/master_rpc.cc
index a03f5f7..e5e554b 100644
--- a/src/kudu/client/master_rpc.cc
+++ b/src/kudu/client/master_rpc.cc
@@ -82,6 +82,7 @@ class ConnectToMasterRpc : public rpc::Rpc {
                      pair<Sockaddr, string> addr_with_name,
                      const MonoTime& deadline,
                      std::shared_ptr<rpc::Messenger> messenger,
+                     rpc::UserCredentials user_credentials,
                      CredentialsPolicy creds_policy,
                      ConnectToMasterResponsePB* out);
 
@@ -99,6 +100,9 @@ class ConnectToMasterRpc : public rpc::Rpc {
   // The resolved address to try to connect to, along with its original specified hostname.
   const pair<Sockaddr, string> addr_with_name_;
 
+  // The client user credentials.
+  const rpc::UserCredentials user_credentials_;
+
   // Owned by the caller of this RPC, not this instance.
   ConnectToMasterResponsePB* out_;
 
@@ -115,11 +119,13 @@ ConnectToMasterRpc::ConnectToMasterRpc(StatusCallback user_cb,
     pair<Sockaddr, string> addr_with_name,
     const MonoTime& deadline,
     shared_ptr<Messenger> messenger,
+    rpc::UserCredentials user_credentials,
     rpc::CredentialsPolicy creds_policy,
     ConnectToMasterResponsePB* out)
       : Rpc(deadline, std::move(messenger)),
         user_cb_(std::move(user_cb)),
         addr_with_name_(std::move(addr_with_name)),
+        user_credentials_(std::move(user_credentials)),
         out_(DCHECK_NOTNULL(out)) {
   mutable_retrier()->mutable_controller()->set_credentials_policy(creds_policy);
 }
@@ -129,6 +135,7 @@ ConnectToMasterRpc::~ConnectToMasterRpc() {
 
 void ConnectToMasterRpc::SendRpc() {
   MasterServiceProxy proxy(retrier().messenger(), addr_with_name_.first, addr_with_name_.second);
+  proxy.set_user_credentials(user_credentials_);
   rpc::RpcController* controller = mutable_retrier()->mutable_controller();
   // TODO(todd): should this be setting an RPC call deadline based on 'deadline'?
   // it doesn't seem to be.
@@ -215,10 +222,12 @@ ConnectToClusterRpc::ConnectToClusterRpc(LeaderCallback user_cb,
                                          MonoTime deadline,
                                          MonoDelta rpc_timeout,
                                          shared_ptr<Messenger> messenger,
+                                         rpc::UserCredentials user_credentials,
                                          rpc::CredentialsPolicy creds_policy)
     : Rpc(deadline, std::move(messenger)),
       user_cb_(std::move(user_cb)),
       addrs_with_names_(std::move(addrs_with_names)),
+      user_credentials_(std::move(user_credentials)),
       rpc_timeout_(rpc_timeout),
       pending_responses_(0),
       completed_(false) {
@@ -255,6 +264,7 @@ void ConnectToClusterRpc::SendRpc() {
         addrs_with_names_[i],
         actual_deadline,
         retrier().messenger(),
+        user_credentials_,
         retrier().controller().credentials_policy(),
         &responses_[i]);
     rpc->SendRpc();

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/client/master_rpc.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/master_rpc.h b/src/kudu/client/master_rpc.h
index efa1075..83767fc 100644
--- a/src/kudu/client/master_rpc.h
+++ b/src/kudu/client/master_rpc.h
@@ -29,6 +29,7 @@
 #include "kudu/master/master.pb.h"
 #include "kudu/rpc/rpc.h"
 #include "kudu/rpc/rpc_controller.h"
+#include "kudu/rpc/user_credentials.h"
 #include "kudu/util/locks.h"
 #include "kudu/util/monotime.h"
 
@@ -85,6 +86,7 @@ class ConnectToClusterRpc : public rpc::Rpc,
                       MonoTime deadline,
                       MonoDelta rpc_timeout,
                       std::shared_ptr<rpc::Messenger> messenger,
+                      rpc::UserCredentials user_credentials,
                       rpc::CredentialsPolicy creds_policy =
       rpc::CredentialsPolicy::ANY_CREDENTIALS);
 
@@ -110,6 +112,9 @@ class ConnectToClusterRpc : public rpc::Rpc,
   // The addresses of the masters, along with their original specified names.
   const std::vector<std::pair<Sockaddr, std::string>> addrs_with_names_;
 
+  // The user credentials of the client.
+  const rpc::UserCredentials user_credentials_;
+
   // The amount of time alloted to each GetMasterRegistration RPC.
   const MonoDelta rpc_timeout_;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/client/meta_cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/meta_cache.cc b/src/kudu/client/meta_cache.cc
index 9c9520b..a204773 100644
--- a/src/kudu/client/meta_cache.cc
+++ b/src/kudu/client/meta_cache.cc
@@ -118,6 +118,7 @@ void RemoteTabletServer::DnsResolutionFinished(const HostPort& hp,
   {
     std::lock_guard<simple_spinlock> l(lock_);
     proxy_.reset(new TabletServerServiceProxy(client->data_->messenger_, (*addrs)[0],
hp.host()));
+    proxy_->set_user_credentials(client->data_->user_credentials_);
   }
   user_callback.Run(s);
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/mini-cluster/external_mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc
index b90c4ef..869eb4e 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -45,6 +45,7 @@
 #include "kudu/rpc/messenger.h"
 #include "kudu/rpc/rpc_controller.h"
 #include "kudu/rpc/sasl_common.h"
+#include "kudu/rpc/user_credentials.h"
 #include "kudu/security/test/mini_kdc.h"
 #include "kudu/server/server_base.pb.h"
 #include "kudu/server/server_base.proxy.h"
@@ -569,11 +570,14 @@ Status ExternalMiniCluster::GetLeaderMasterIndex(int* idx) {
     }
     sync.StatusCB(status);
   };
+  rpc::UserCredentials user_credentials;
+  RETURN_NOT_OK(user_credentials.SetLoggedInRealUser());
   rpc.reset(new ConnectToClusterRpc(cb,
                                     std::move(addrs_with_names),
                                     deadline,
                                     MonoDelta::FromSeconds(5),
-                                    messenger_));
+                                    messenger_,
+                                    user_credentials));
   rpc->SendRpc();
   RETURN_NOT_OK(sync.Wait());
   bool found = false;

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/rpc/connection_id.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/connection_id.cc b/src/kudu/rpc/connection_id.cc
index 7a3223e..6720807 100644
--- a/src/kudu/rpc/connection_id.cc
+++ b/src/kudu/rpc/connection_id.cc
@@ -42,6 +42,7 @@ ConnectionId::ConnectionId(const Sockaddr& remote,
 }
 
 void ConnectionId::set_user_credentials(UserCredentials user_credentials) {
+  DCHECK(user_credentials.has_real_user());
   user_credentials_ = std::move(user_credentials);
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/rpc/server_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/server_negotiation.cc b/src/kudu/rpc/server_negotiation.cc
index a623853..612701f 100644
--- a/src/kudu/rpc/server_negotiation.cc
+++ b/src/kudu/rpc/server_negotiation.cc
@@ -949,11 +949,11 @@ int ServerNegotiation::GetOptionCb(const char* plugin_name,
 }
 
 int ServerNegotiation::PlainAuthCb(sasl_conn_t* /*conn*/,
-                                   const char*  /*user*/,
-                                   const char*  /*pass*/,
+                                   const char* user,
+                                   const char* /*pass*/,
                                    unsigned /*passlen*/,
                                    struct propctx*  /*propctx*/) {
-  TRACE("Received PLAIN auth.");
+  TRACE("Received PLAIN auth, user=$0", user);
   if (PREDICT_FALSE(!helper_.IsPlainEnabled())) {
     LOG(DFATAL) << "Password authentication callback called while PLAIN auth disabled";
     return SASL_BADPARAM;

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/rpc/user_credentials.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/user_credentials.cc b/src/kudu/rpc/user_credentials.cc
index 1ce8777..7f318fe 100644
--- a/src/kudu/rpc/user_credentials.cc
+++ b/src/kudu/rpc/user_credentials.cc
@@ -24,6 +24,8 @@
 #include <boost/functional/hash/hash.hpp>
 
 #include "kudu/gutil/strings/substitute.h"
+#include "kudu/util/status.h"
+#include "kudu/util/user.h"
 
 using std::string;
 
@@ -38,8 +40,11 @@ void UserCredentials::set_real_user(string real_user) {
   real_user_ = std::move(real_user);
 }
 
+Status UserCredentials::SetLoggedInRealUser() {
+  return GetLoggedInUser(&real_user_);
+}
+
 string UserCredentials::ToString() const {
-  // Does not print the password.
   return strings::Substitute("{real_user=$0}", real_user_);
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/rpc/user_credentials.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/user_credentials.h b/src/kudu/rpc/user_credentials.h
index ae507a2..5a0434c 100644
--- a/src/kudu/rpc/user_credentials.h
+++ b/src/kudu/rpc/user_credentials.h
@@ -19,6 +19,8 @@
 #include <cstddef>
 #include <string>
 
+#include "kudu/util/status.h"
+
 namespace kudu {
 namespace rpc {
 
@@ -33,6 +35,9 @@ class UserCredentials {
   void set_real_user(std::string real_user);
   const std::string& real_user() const { return real_user_; }
 
+  // Sets the real user to the currently logged in user.
+  Status SetLoggedInRealUser();
+
   // Returns a string representation of the object.
   std::string ToString() const;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/c243d68b/src/kudu/util/user.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/user.cc b/src/kudu/util/user.cc
index cdfaea1..1b73e53 100644
--- a/src/kudu/util/user.cc
+++ b/src/kudu/util/user.cc
@@ -47,7 +47,7 @@ Status GetLoggedInUser(string* user_name) {
 
   gscoped_ptr<char[], FreeDeleter> buf(static_cast<char *>(malloc(bufsize)));
   if (buf.get() == nullptr) {
-    return Status::RuntimeError("Malloc failed", ErrnoToString(errno), errno);
+    return Status::RuntimeError("malloc failed", ErrnoToString(errno), errno);
   }
 
   int ret = getpwuid_r(getuid(), &pwd, buf.get(), bufsize, &result);


Mime
View raw message