You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ab...@apache.org on 2021/04/09 13:53:09 UTC
[kudu] branch master updated (6aeb466 -> 308673d)
This is an automated email from the ASF dual-hosted git repository.
abukor pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git.
from 6aeb466 KUDU-3268: fix race between MM scheduling and unregistration
new d546588 KUDU-1884 Make Kerberos principal customizable
new e69b3c6 [java] KUDU-1884 Make SASL proto name configurable
new 308673d [python] KUDU-1884 Make SASL protocol configurable
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../org/apache/kudu/client/AsyncKuduClient.java | 17 ++++++-
.../java/org/apache/kudu/client/Connection.java | 11 +++--
.../org/apache/kudu/client/ConnectionCache.java | 9 +++-
.../java/org/apache/kudu/client/KuduClient.java | 14 ++++++
.../java/org/apache/kudu/client/Negotiator.java | 8 ++-
.../org/apache/kudu/client/TestNegotiator.java | 2 +-
.../java/org/apache/kudu/client/TestSecurity.java | 27 ++++++++++
.../apache/kudu/test/cluster/MiniKuduCluster.java | 16 ++++--
python/kudu/client.pyx | 5 +-
python/kudu/libkudu_client.pxd | 2 +
src/kudu/client/client.cc | 8 +++
src/kudu/client/client.h | 12 +++++
src/kudu/client/client_builder-internal.h | 1 +
src/kudu/integration-tests/security-itest.cc | 57 ++++++++++++++++++++--
src/kudu/mini-cluster/external_mini_cluster.cc | 34 +++++++------
src/kudu/mini-cluster/external_mini_cluster.h | 12 ++++-
src/kudu/security/init.cc | 8 +--
src/kudu/server/server_base.cc | 6 +++
src/kudu/tools/kudu-tool-test.cc | 11 +++++
src/kudu/tools/tool.proto | 4 ++
src/kudu/tools/tool_action_common.cc | 20 +++++---
src/kudu/tools/tool_action_test.cc | 3 ++
22 files changed, 241 insertions(+), 46 deletions(-)
[kudu] 01/03: KUDU-1884 Make Kerberos principal customizable
Posted by ab...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
abukor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit d546588da6d90c72a8dfcb8256e2a898ab55e96d
Author: Attila Bukor <ab...@apache.org>
AuthorDate: Wed Apr 7 14:30:01 2021 +0200
KUDU-1884 Make Kerberos principal customizable
In a Kerberized cluster, all Kudu servers service principal names (SPN)
are, by default, set to "kudu/_HOST". While this is configurable on the
server side, the SASL protocol name was hard-coded to "kudu" on the
client-side, meaning that if the SPN was anything other than "kudu", the
clients weren't able to connect to the Kudu servers. The 'principal'
flag was tagged as unsafe and experimental for this reason.
This patch allows changing the SASL protocol name in the C++ client
using the KuduClientBuilder API and the CLI tools using a new
'--sasl_protocol_name' flag, and makes the servers use the configured
principal for the RPC.
Java and Python client support will be added in separate patches.
Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Reviewed-on: http://gerrit.cloudera.org:8080/17279
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
---
src/kudu/client/client.cc | 8 ++++
src/kudu/client/client.h | 12 ++++++
src/kudu/client/client_builder-internal.h | 1 +
src/kudu/integration-tests/security-itest.cc | 57 ++++++++++++++++++++++++--
src/kudu/mini-cluster/external_mini_cluster.cc | 34 ++++++++-------
src/kudu/mini-cluster/external_mini_cluster.h | 12 +++++-
src/kudu/security/init.cc | 8 +---
src/kudu/server/server_base.cc | 6 +++
src/kudu/tools/kudu-tool-test.cc | 11 +++++
src/kudu/tools/tool.proto | 4 ++
src/kudu/tools/tool_action_common.cc | 20 +++++----
src/kudu/tools/tool_action_test.cc | 3 ++
12 files changed, 143 insertions(+), 33 deletions(-)
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 0ba3f26..40ee5f8 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -318,6 +318,11 @@ KuduClientBuilder& KuduClientBuilder::num_reactors(int num_reactors) {
return *this;
}
+KuduClientBuilder& KuduClientBuilder::sasl_protocol_name(const string& sasl_protocol_name) {
+ data_->sasl_protocol_name_ = sasl_protocol_name;
+ return *this;
+}
+
namespace {
Status ImportAuthnCreds(const string& authn_creds,
Messenger* messenger,
@@ -361,6 +366,9 @@ Status KuduClientBuilder::Build(shared_ptr<KuduClient>* client) {
if (data_->num_reactors_) {
builder.set_num_reactors(data_->num_reactors_.get());
}
+ if (!data_->sasl_protocol_name_.empty()) {
+ builder.set_sasl_proto_name(data_->sasl_protocol_name_);
+ }
std::shared_ptr<Messenger> messenger;
RETURN_NOT_OK(builder.Build(&messenger));
UserCredentials user_credentials;
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index 04f1925..a85d218 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -306,6 +306,18 @@ class KUDU_EXPORT KuduClientBuilder {
/// @return Reference to the updated object.
KuduClientBuilder& num_reactors(int num_reactors);
+ /// Set the SASL protocol name for the connection to a remote server.
+ ///
+ /// If the servers use a non-default Kerberos service principal name (other
+ /// than "kudu" or "kudu/<hostname>", this needs to be set for the client to
+ /// be able to connect to the servers. If unset, the client will assume the
+ /// server is using the default service principal.
+ ///
+ /// @param [in] sasl_protocol_name
+ /// SASL protocol name.
+ /// @return Reference to the updated object.
+ KuduClientBuilder& sasl_protocol_name(const std::string& sasl_protocol_name);
+
/// Create a client object.
///
/// @note KuduClients objects are shared amongst multiple threads and,
diff --git a/src/kudu/client/client_builder-internal.h b/src/kudu/client/client_builder-internal.h
index f30cc7b..f097b3e 100644
--- a/src/kudu/client/client_builder-internal.h
+++ b/src/kudu/client/client_builder-internal.h
@@ -42,6 +42,7 @@ class KuduClientBuilder::Data {
std::string authn_creds_;
internal::ReplicaController::Visibility replica_visibility_;
boost::optional<int> num_reactors_;
+ std::string sasl_protocol_name_;
DISALLOW_COPY_AND_ASSIGN(Data);
};
diff --git a/src/kudu/integration-tests/security-itest.cc b/src/kudu/integration-tests/security-itest.cc
index 370f55f..d8e1aed 100644
--- a/src/kudu/integration-tests/security-itest.cc
+++ b/src/kudu/integration-tests/security-itest.cc
@@ -134,6 +134,7 @@ class SecurityITest : public KuduTest {
}
// Create a table, insert a row, scan it back, and delete the table.
+ void SmokeTestCluster(const client::sp::shared_ptr<KuduClient>& client);
void SmokeTestCluster();
Status TryRegisterAsTS() {
@@ -198,10 +199,7 @@ class SecurityITest : public KuduTest {
unique_ptr<ExternalMiniCluster> cluster_;
};
-void SecurityITest::SmokeTestCluster() {
- client::sp::shared_ptr<KuduClient> client;
- ASSERT_OK(cluster_->CreateClient(nullptr, &client));
-
+void SecurityITest::SmokeTestCluster(const client::sp::shared_ptr<KuduClient>& client) {
// Create a table.
ASSERT_OK(CreateTestTable(client));
@@ -223,6 +221,13 @@ void SecurityITest::SmokeTestCluster() {
ASSERT_OK(client->DeleteTable(kTableName));
}
+void SecurityITest::SmokeTestCluster() {
+ client::sp::shared_ptr<KuduClient> client;
+ ASSERT_OK(cluster_->CreateClient(nullptr, &client));
+
+ SmokeTestCluster(client);
+}
+
// Test authorizing list tablets.
TEST_F(SecurityITest, TestAuthorizationOnListTablets) {
// When enforcing access control, an operator of ListTablets must be
@@ -445,6 +450,50 @@ TEST_F(SecurityITest, TestCorruptKerberosCC) {
}
}
+TEST_F(SecurityITest, TestNonDefaultPrincipal) {
+ const string kPrincipal = "oryx";
+ cluster_opts_.principal = kPrincipal;
+ ASSERT_OK(StartCluster());
+
+ // A client with the default SASL proto shouldn't be able to connect
+ client::sp::shared_ptr<KuduClient> default_client;
+ Status s = cluster_->CreateClient(nullptr, &default_client);
+ ASSERT_TRUE(s.IsNotAuthorized());
+ ASSERT_STR_CONTAINS(s.ToString(), "not found in Kerberos database");
+
+ // Create a client with the matching SASL proto name and verify it's able to
+ // connect to the cluster and perform basic actions.
+ KuduClientBuilder b;
+ b.sasl_protocol_name(kPrincipal);
+ client::sp::shared_ptr<KuduClient> client;
+ ASSERT_OK(cluster_->CreateClient(&b, &client));
+ SmokeTestCluster(client);
+}
+
+TEST_F(SecurityITest, TestNonExistentPrincipal) {
+ cluster_opts_.extra_master_flags.emplace_back("--principal=oryx");
+ cluster_opts_.extra_tserver_flags.emplace_back("--principal=oryx");
+ Status s = StartCluster();
+ ASSERT_TRUE(s.IsRuntimeError());
+ ASSERT_STR_CONTAINS(s.ToString(), "failed to start masters");
+}
+
+TEST_F(SecurityITest, TestMismatchingPrincipals) {
+ ASSERT_OK(StartCluster());
+ string keytab_path;
+ const string kPrincipalBase = "oryx";
+ for (auto i = 0; i < cluster_->num_tablet_servers(); i++) {
+ cluster::ExternalTabletServer* tserver = cluster_->tablet_server(i);
+ const string kPrincipal = kPrincipalBase + "/" + tserver->bound_rpc_addr().host();
+ ASSERT_OK(cluster_->kdc()->CreateServiceKeytab(kPrincipal, &keytab_path));
+ tserver->mutable_flags()->emplace_back("--principal=" + kPrincipal);
+ tserver->mutable_flags()->emplace_back("--keytab_file=" + keytab_path);
+ }
+ cluster_->Shutdown();
+ Status s = cluster_->Restart();
+ ASSERT_TRUE(s.IsTimedOut());
+}
+
Status AssignIPToClient(bool external) {
// If the test does not require an external IP address
// assign loopback IP to FLAGS_local_ip_for_outbound_sockets.
diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc
index dbfcdc3..26109ce 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -118,14 +118,16 @@ ExternalMiniClusterOptions::ExternalMiniClusterOptions()
bind_mode(kDefaultBindMode),
num_data_dirs(1),
enable_kerberos(false),
+ principal("kudu"),
hms_mode(HmsMode::NONE),
enable_ranger(false),
logtostderr(true),
start_process_timeout(MonoDelta::FromSeconds(70)),
rpc_negotiation_timeout(MonoDelta::FromSeconds(3))
#if !defined(NO_CHRONY)
- , num_ntp_servers(1)
- , ntp_config_mode(BuiltinNtpConfigMode::ALL_SERVERS)
+ ,
+ num_ntp_servers(1),
+ ntp_config_mode(BuiltinNtpConfigMode::ALL_SERVERS)
#endif // #if !defined(NO_CHRONY) ...
{
}
@@ -219,13 +221,14 @@ Status ExternalMiniCluster::Start() {
<< tablet_servers_.size() << "). Maybe you meant Restart()?";
RETURN_NOT_OK(HandleOptions());
- RETURN_NOT_OK_PREPEND(rpc::MessengerBuilder("minicluster-messenger")
- .set_num_reactors(1)
- .set_max_negotiation_threads(1)
- .set_rpc_negotiation_timeout_ms(
- opts_.rpc_negotiation_timeout.ToMilliseconds())
- .Build(&messenger_),
- "Failed to start Messenger for minicluster");
+ RETURN_NOT_OK_PREPEND(
+ rpc::MessengerBuilder("minicluster-messenger")
+ .set_num_reactors(1)
+ .set_max_negotiation_threads(1)
+ .set_rpc_negotiation_timeout_ms(opts_.rpc_negotiation_timeout.ToMilliseconds())
+ .set_sasl_proto_name(opts_.principal)
+ .Build(&messenger_),
+ "Failed to start Messenger for minicluster");
Status s = env()->CreateDir(opts_.cluster_root);
if (!s.ok() && !s.IsAlreadyPresent()) {
@@ -573,8 +576,9 @@ Status ExternalMiniCluster::StartMasters() {
scoped_refptr<ExternalMaster> peer = new ExternalMaster(opts);
if (opts_.enable_kerberos) {
- RETURN_NOT_OK_PREPEND(peer->EnableKerberos(kdc_.get(), master_rpc_addrs[i].host()),
- "could not enable Kerberos");
+ RETURN_NOT_OK_PREPEND(
+ peer->EnableKerberos(kdc_.get(), opts_.principal, master_rpc_addrs[i].host()),
+ "could not enable Kerberos");
}
RETURN_NOT_OK_PREPEND(peer->Start(),
Substitute("Unable to start Master at index $0", i));
@@ -630,7 +634,7 @@ Status ExternalMiniCluster::AddTabletServer() {
vector<HostPort> master_hostports = master_rpc_addrs();
scoped_refptr<ExternalTabletServer> ts = new ExternalTabletServer(opts, master_hostports);
if (opts_.enable_kerberos) {
- RETURN_NOT_OK_PREPEND(ts->EnableKerberos(kdc_.get(), bind_host),
+ RETURN_NOT_OK_PREPEND(ts->EnableKerberos(kdc_.get(), opts_.principal, bind_host),
"could not enable Kerberos");
}
@@ -1154,8 +1158,10 @@ void ExternalDaemon::SetMetastoreIntegration(const string& hms_uris,
opts_.extra_flags.emplace_back(Substitute("--hive_metastore_sasl_enabled=$0", enable_kerberos));
}
-Status ExternalDaemon::EnableKerberos(MiniKdc* kdc, const string& bind_host) {
- string spn = "kudu/" + bind_host;
+Status ExternalDaemon::EnableKerberos(MiniKdc* kdc,
+ const string& principal,
+ const string& bind_host) {
+ string spn = principal + "/" + bind_host;
string ktpath;
RETURN_NOT_OK_PREPEND(kdc->CreateServiceKeytab(spn, &ktpath),
"could not create keytab");
diff --git a/src/kudu/mini-cluster/external_mini_cluster.h b/src/kudu/mini-cluster/external_mini_cluster.h
index 496885f..369d9d0 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.h
+++ b/src/kudu/mini-cluster/external_mini_cluster.h
@@ -194,6 +194,11 @@ struct ExternalMiniClusterOptions {
// Default: false.
bool enable_kerberos;
+ // Service principal name for the servers.
+ //
+ // Default: "kudu".
+ std::string principal;
+
// Tri state mode flag that indicates whether to set up a Hive Metastore as
// part of this ExternalMiniCluster and enable Kudu Hive Metastore integration.
//
@@ -566,10 +571,13 @@ class ExternalDaemon : public RefCountedThreadSafe<ExternalDaemon> {
// subprocess such that the server will use Kerberos authentication.
//
// 'bind_host' is the hostname that will be used to generate the Kerberos
- // service principal.
+ // service principal, 'principal_base' will be the first part (i.e.
+ // <principal_base>/<bind_host>).
//
// Must be called before 'StartProcess()'.
- Status EnableKerberos(MiniKdc* kdc, const std::string& bind_host);
+ Status EnableKerberos(MiniKdc* kdc,
+ const std::string& principal_base,
+ const std::string& bind_host);
// Sends a SIGSTOP signal to the daemon.
Status Pause() WARN_UNUSED_RESULT;
diff --git a/src/kudu/security/init.cc b/src/kudu/security/init.cc
index 5a81a95..7769ffd 100644
--- a/src/kudu/security/init.cc
+++ b/src/kudu/security/init.cc
@@ -75,12 +75,8 @@ TAG_FLAG(use_system_auth_to_local, advanced);
DEFINE_string(principal, "kudu/_HOST",
"Kerberos principal that this daemon will log in as. The special token "
"_HOST will be replaced with the FQDN of the local host.");
-TAG_FLAG(principal, experimental);
-// This is currently tagged as unsafe because there is no way for users to configure
-// clients to expect a non-default principal. As such, configuring a server to login
-// as a different one would end up with a cluster that can't be connected to.
-// See KUDU-1884.
-TAG_FLAG(principal, unsafe);
+TAG_FLAG(principal, advanced);
+TAG_FLAG(principal, stable);
DEFINE_string(keytab_file, "",
"Path to the Kerberos Keytab file for this server. Specifying a "
diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc
index a10ac75..ec82d62 100644
--- a/src/kudu/server/server_base.cc
+++ b/src/kudu/server/server_base.cc
@@ -591,6 +591,12 @@ Status ServerBase::Init() {
builder.set_reuseport();
}
+ if (!FLAGS_keytab_file.empty()) {
+ string service_name;
+ RETURN_NOT_OK(security::MapPrincipalToLocalName(FLAGS_principal, &service_name));
+ builder.set_sasl_proto_name(service_name);
+ }
+
RETURN_NOT_OK(builder.Build(&messenger_));
rpc_server_->set_too_busy_hook([this](rpc::ServicePool* pool) {
this->ServiceQueueOverflowed(pool);
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 748b553..f8793fc 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -6117,5 +6117,16 @@ TEST_F(ToolTest, TestDuplicateMastersInKsck) {
ASSERT_STR_CONTAINS(out, "Duplicate master addresses specified");
}
+TEST_F(ToolTest, TestNonDefaultPrincipal) {
+ ExternalMiniClusterOptions opts;
+ opts.enable_kerberos = true;
+ opts.principal = "oryx";
+ NO_FATALS(StartExternalMiniCluster(opts));
+ ASSERT_OK(RunKuduTool({"cluster",
+ "ksck",
+ "--sasl_protocol_name=oryx",
+ HostPort::ToCommaSeparatedString(cluster_->master_rpc_addrs())}));
+}
+
} // namespace tools
} // namespace kudu
diff --git a/src/kudu/tools/tool.proto b/src/kudu/tools/tool.proto
index 107d86b..199f11d 100644
--- a/src/kudu/tools/tool.proto
+++ b/src/kudu/tools/tool.proto
@@ -42,6 +42,10 @@ message CreateClusterRequestPB {
// Whether or not the cluster should be Kerberized.
optional bool enable_kerberos = 3;
+ // Service principal name used by the cluster if Kerberos is enabled (default:
+ // "kudu").
+ optional string principal = 10;
+
// Whether or not Ranger should be enabled
optional bool enable_ranger = 9;
diff --git a/src/kudu/tools/tool_action_common.cc b/src/kudu/tools/tool_action_common.cc
index ebbe05a..626b737 100644
--- a/src/kudu/tools/tool_action_common.cc
+++ b/src/kudu/tools/tool_action_common.cc
@@ -17,10 +17,9 @@
#include "kudu/tools/tool_action_common.h"
-#include <stdlib.h>
-
#include <algorithm>
#include <cstddef>
+#include <cstdlib>
#include <iomanip>
#include <iostream>
#include <iterator>
@@ -146,6 +145,12 @@ DEFINE_int64(negotiation_timeout_ms, 3000,
"Timeout for negotiating an RPC connection to a Kudu server, "
"in milliseconds");
+DEFINE_string(sasl_protocol_name,
+ "kudu",
+ "SASL protocol name used to connnect to a Kerberos-enabled cluster. Must match the "
+ "servers' service principal name base (e.g. if it's \"kudu/_HOST\", then "
+ "sasl_protocol_name must be \"kudu\" to be able to connect.");
+
bool ValidateTimeoutSettings() {
if (FLAGS_timeout_ms < FLAGS_negotiation_timeout_ms) {
LOG(ERROR) << strings::Substitute(
@@ -413,9 +418,10 @@ TServerActionBuilder::TServerActionBuilder(std::string name, ActionRunner runner
Status BuildMessenger(std::string name, shared_ptr<Messenger>* messenger) {
shared_ptr<Messenger> m;
- MessengerBuilder b(std::move(name));
- b.set_rpc_negotiation_timeout_ms(FLAGS_negotiation_timeout_ms);
- auto s = b.Build(&m);
+ Status s = MessengerBuilder(std::move(name))
+ .set_rpc_negotiation_timeout_ms(FLAGS_negotiation_timeout_ms)
+ .set_sasl_proto_name(FLAGS_sasl_protocol_name)
+ .Build(&m);
if (s.ok()) {
*messenger = std::move(m);
}
@@ -582,11 +588,11 @@ Status CreateKuduClient(const vector<string>& master_addresses,
if (can_see_all_replicas) {
ReplicaController::SetVisibility(&b, ReplicaController::Visibility::ALL);
}
- return b
- .master_server_addrs(master_addresses)
+ return b.master_server_addrs(master_addresses)
.default_rpc_timeout(rpc_timeout)
.default_admin_operation_timeout(rpc_timeout)
.connection_negotiation_timeout(negotiation_timeout)
+ .sasl_protocol_name(FLAGS_sasl_protocol_name)
.Build(client);
}
diff --git a/src/kudu/tools/tool_action_test.cc b/src/kudu/tools/tool_action_test.cc
index 08d8680..8e0fab4 100644
--- a/src/kudu/tools/tool_action_test.cc
+++ b/src/kudu/tools/tool_action_test.cc
@@ -175,6 +175,9 @@ Status ProcessRequest(const ControlShellRequestPB& req,
opts.mini_kdc_options.data_root = JoinPathSegments(opts.cluster_root, "krb5kdc");
opts.mini_kdc_options.ticket_lifetime = cc.mini_kdc_options().ticket_lifetime();
opts.mini_kdc_options.renew_lifetime = cc.mini_kdc_options().renew_lifetime();
+ if (cc.has_principal()) {
+ opts.principal = cc.principal();
+ }
}
cluster->reset(new ExternalMiniCluster(std::move(opts)));
[kudu] 02/03: [java] KUDU-1884 Make SASL proto name configurable
Posted by ab...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
abukor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit e69b3c652d0c21393015aad8e5acd5ece2098573
Author: Attila Bukor <ab...@apache.org>
AuthorDate: Wed Apr 7 14:45:22 2021 +0200
[java] KUDU-1884 Make SASL proto name configurable
The server-side Kerberos service principal name must match the
client-side SASL protocol name. This patch makes this configurable
through the client builder API.
Change-Id: I4a881f2cf125651277927b43a5b31afc173a9bab
Reviewed-on: http://gerrit.cloudera.org:8080/17280
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Attila Bukor <ab...@apache.org>
Reviewed-by: Grant Henke <gr...@apache.org>
---
.../org/apache/kudu/client/AsyncKuduClient.java | 17 +++++++++++++-
.../java/org/apache/kudu/client/Connection.java | 11 ++++++---
.../org/apache/kudu/client/ConnectionCache.java | 9 ++++++--
.../java/org/apache/kudu/client/KuduClient.java | 14 +++++++++++
.../java/org/apache/kudu/client/Negotiator.java | 8 +++++--
.../org/apache/kudu/client/TestNegotiator.java | 2 +-
.../java/org/apache/kudu/client/TestSecurity.java | 27 ++++++++++++++++++++++
.../apache/kudu/test/cluster/MiniKuduCluster.java | 16 ++++++++++---
8 files changed, 92 insertions(+), 12 deletions(-)
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
index d8ba883..0710db8 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
@@ -425,7 +425,7 @@ public class AsyncKuduClient implements AutoCloseable {
this.requestTracker = new RequestTracker(clientId);
this.securityContext = new SecurityContext();
- this.connectionCache = new ConnectionCache(securityContext, bootstrap);
+ this.connectionCache = new ConnectionCache(securityContext, bootstrap, b.saslProtocolName);
this.tokenReacquirer = new AuthnTokenReacquirer(this);
this.authzTokenCache = new AuthzTokenCache(this);
}
@@ -2731,6 +2731,7 @@ public class AsyncKuduClient implements AutoCloseable {
private Executor workerExecutor;
private int workerCount = DEFAULT_WORKER_COUNT;
private boolean statisticsDisabled = false;
+ private String saslProtocolName = "kudu";
/**
* Creates a new builder for a client that will connect to the specified masters.
@@ -2892,6 +2893,20 @@ public class AsyncKuduClient implements AutoCloseable {
}
/**
+ * Set the SASL protocol name.
+ * SASL protocol name is used when connecting to a secure (Kerberos-enabled)
+ * cluster. It must match the servers' service principal name (SPN).
+ *
+ * Optional.
+ * If not provided, it will use the default SASL protocol name ("kudu").
+ * @return this builder
+ */
+ public AsyncKuduClientBuilder saslProtocolName(String saslProtocolName) {
+ this.saslProtocolName = saslProtocolName;
+ return this;
+ }
+
+ /**
* Creates a new client that connects to the masters.
* Doesn't block and won't throw an exception if the masters don't exist.
* @return a new asynchronous Kudu client
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
index e712475..74445ef 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
@@ -108,6 +108,8 @@ class Connection extends SimpleChannelInboundHandler<Object> {
/** The Netty client bootstrap used to configure and initialize a connected channel. */
private final Bootstrap bootstrap;
+ private final String saslProtocolName;
+
/** The underlying Netty's socket channel. */
private SocketChannel channel;
@@ -178,14 +180,17 @@ class Connection extends SimpleChannelInboundHandler<Object> {
* @param credentialsPolicy policy controlling which credentials to use while negotiating on the
* connection to the target server:
* if {@link CredentialsPolicy#PRIMARY_CREDENTIALS}, the authentication
- * token from the security context is ignored
+ * @param saslProtocolName SASL protocol name used when connecting to secure
+ * clusters. Must match the servers' service principal name.
*/
Connection(ServerInfo serverInfo,
SecurityContext securityContext,
Bootstrap bootstrap,
- CredentialsPolicy credentialsPolicy) {
+ CredentialsPolicy credentialsPolicy,
+ String saslProtocolName) {
this.serverInfo = serverInfo;
this.securityContext = securityContext;
+ this.saslProtocolName = saslProtocolName;
this.state = State.NEW;
this.credentialsPolicy = credentialsPolicy;
this.bootstrap = bootstrap.clone();
@@ -208,7 +213,7 @@ class Connection extends SimpleChannelInboundHandler<Object> {
}
ctx.writeAndFlush(Unpooled.wrappedBuffer(CONNECTION_HEADER), ctx.voidPromise());
Negotiator negotiator = new Negotiator(serverInfo.getAndCanonicalizeHostname(), securityContext,
- (credentialsPolicy == CredentialsPolicy.PRIMARY_CREDENTIALS));
+ (credentialsPolicy == CredentialsPolicy.PRIMARY_CREDENTIALS), saslProtocolName);
ctx.pipeline().addBefore(ctx.name(), "negotiation", negotiator);
negotiator.sendHello(ctx);
}
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
index 83bff27..705f962 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
@@ -56,6 +56,8 @@ class ConnectionCache {
/** Netty's bootstrap to use by connections. */
private final Bootstrap bootstrap;
+ private final String saslProtocolName;
+
/**
* Container mapping server IP/port into the established connection from the client to the
* server. It may be up to two connections per server: one established with secondary
@@ -67,9 +69,11 @@ class ConnectionCache {
/** Create a new empty ConnectionCache given the specified parameters. */
ConnectionCache(SecurityContext securityContext,
- Bootstrap bootstrap) {
+ Bootstrap bootstrap,
+ String saslProtocolName) {
this.securityContext = securityContext;
this.bootstrap = bootstrap;
+ this.saslProtocolName = saslProtocolName;
}
/**
@@ -122,7 +126,8 @@ class ConnectionCache {
result = new Connection(serverInfo,
securityContext,
bootstrap,
- credentialsPolicy);
+ credentialsPolicy,
+ saslProtocolName);
connections.add(result);
// There can be at most 2 connections to the same destination: one with primary and another
// with secondary credentials.
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
index a77689b..ad518a6 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
@@ -609,6 +609,20 @@ public class KuduClient implements AutoCloseable {
}
/**
+ * Set the SASL protocol name.
+ * SASL protocol name is used when connecting to a secure (Kerberos-enabled)
+ * cluster. It must match the servers' service principal name (SPN).
+ *
+ * Optional.
+ * If not provided, it will use the default SASL protocol name ("kudu").
+ * @return this builder
+ */
+ public KuduClientBuilder saslProtocolName(String saslProtocolName) {
+ clientBuilder.saslProtocolName(saslProtocolName);
+ return this;
+ }
+
+ /**
* Creates a new client that connects to the masters.
* Doesn't block and won't throw an exception if the masters don't exist.
* @return a new asynchronous Kudu client
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 e570665..234984b 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
@@ -222,14 +222,18 @@ public class Negotiator extends SimpleChannelInboundHandler<CallResponse> {
private Certificate peerCert;
+ private String saslProtocolName;
+
@InterfaceAudience.LimitedPrivate("Test")
boolean overrideLoopbackForTests;
public Negotiator(String remoteHostname,
SecurityContext securityContext,
- boolean ignoreAuthnToken) {
+ boolean ignoreAuthnToken,
+ String saslProtocolName) {
this.remoteHostname = remoteHostname;
this.securityContext = securityContext;
+ this.saslProtocolName = saslProtocolName;
SignedTokenPB token = securityContext.getAuthenticationToken();
if (token != null) {
if (ignoreAuthnToken) {
@@ -456,7 +460,7 @@ public class Negotiator extends SimpleChannelInboundHandler<CallResponse> {
try {
saslClient = Sasl.createSaslClient(new String[]{ clientMech.name() },
null,
- "kudu",
+ saslProtocolName,
remoteHostname,
props,
saslCallback);
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
index 4d4dce6..0da8368 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
@@ -117,7 +117,7 @@ public class TestNegotiator {
}
private void startNegotiation(boolean fakeLoopback) {
- Negotiator negotiator = new Negotiator("127.0.0.1", secContext, false);
+ Negotiator negotiator = new Negotiator("127.0.0.1", secContext, false, "kudu");
negotiator.overrideLoopbackForTests = fakeLoopback;
embedder = new EmbeddedChannel(negotiator);
negotiator.sendHello(embedder.pipeline().firstContext());
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
index d4f993d..88037a08 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
@@ -55,6 +55,7 @@ public class TestSecurity {
private static final String TABLE_NAME = "TestSecurity-table";
private static final int TICKET_LIFETIME_SECS = 10;
private static final int RENEWABLE_LIFETIME_SECS = 20;
+ public static final String CUSTOM_PRINCIPAL = "oryx";
private CapturingLogAppender cla;
private MiniKuduCluster miniCluster;
@@ -64,6 +65,7 @@ public class TestSecurity {
LONG_LEADER_ELECTION,
SHORT_TOKENS_AND_TICKETS,
START_TSERVERS,
+ CUSTOM_PRINCIPAL,
}
private static class KeyValueMessage {
@@ -89,6 +91,9 @@ public class TestSecurity {
.kdcRenewLifetime(RENEWABLE_LIFETIME_SECS + "s")
.kdcTicketLifetime(TICKET_LIFETIME_SECS + "s");
}
+ if (opts.contains(Option.CUSTOM_PRINCIPAL)) {
+ mcb.principal(CUSTOM_PRINCIPAL);
+ }
miniCluster = mcb.numMasterServers(3)
.numTabletServers(opts.contains(Option.START_TSERVERS) ? 3 : 0)
.build();
@@ -491,4 +496,26 @@ public class TestSecurity {
}
}
}
+
+ @Test(timeout = 60000)
+ public void testNonDefaultPrincipal() throws Exception {
+ startCluster(ImmutableSet.of(Option.CUSTOM_PRINCIPAL, Option.START_TSERVERS));
+ try {
+ this.client.createTable("TestSecurity-nondefault-principal-1",
+ getBasicSchema(),
+ getBasicCreateTableOptions());
+ Assert.fail("default client shouldn't be able to connect to the cluster.");
+ } catch (NonRecoverableException e) {
+ Assert.assertThat(e.getMessage(), CoreMatchers.containsString(
+ "this client is not authenticated"
+ ));
+ }
+ KuduClient client = new KuduClient.KuduClientBuilder(miniCluster.getMasterAddressesAsString())
+ .saslProtocolName(CUSTOM_PRINCIPAL)
+ .build();
+ Assert.assertNotNull(client.createTable( "TestSecurity-nondefault-principal-2",
+ getBasicSchema(),
+ getBasicCreateTableOptions()));
+ }
+
}
diff --git a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
index 09ac649..2dcd781 100644
--- a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
+++ b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
@@ -106,6 +106,7 @@ public final class MiniKuduCluster implements AutoCloseable {
private final ImmutableList<String> extraMasterFlags;
private final ImmutableList<String> locationInfo;
private final String clusterRoot;
+ private final String principal;
private MiniKdcOptionsPB kdcOptionsPb;
private final Common.HmsMode hmsMode;
@@ -118,7 +119,8 @@ public final class MiniKuduCluster implements AutoCloseable {
List<String> locationInfo,
MiniKdcOptionsPB kdcOptionsPb,
String clusterRoot,
- Common.HmsMode hmsMode) {
+ Common.HmsMode hmsMode,
+ String principal) {
this.enableKerberos = enableKerberos;
this.numMasters = numMasters;
this.numTservers = numTservers;
@@ -126,6 +128,7 @@ public final class MiniKuduCluster implements AutoCloseable {
this.extraMasterFlags = ImmutableList.copyOf(extraMasterFlags);
this.locationInfo = ImmutableList.copyOf(locationInfo);
this.kdcOptionsPb = kdcOptionsPb;
+ this.principal = principal;
this.hmsMode = hmsMode;
if (clusterRoot == null) {
@@ -220,7 +223,8 @@ public final class MiniKuduCluster implements AutoCloseable {
.addAllExtraMasterFlags(extraMasterFlags)
.addAllExtraTserverFlags(extraTserverFlags)
.setMiniKdcOptions(kdcOptionsPb)
- .setClusterRoot(clusterRoot);
+ .setClusterRoot(clusterRoot)
+ .setPrincipal(principal);
// Set up the location mapping command flag if there is location info.
if (!locationInfo.isEmpty()) {
@@ -610,6 +614,7 @@ public final class MiniKuduCluster implements AutoCloseable {
private final List<String> extraMasterServerFlags = new ArrayList<>();
private final List<String> locationInfo = new ArrayList<>();
private String clusterRoot = null;
+ private String principal = "kudu";
private MiniKdcOptionsPB.Builder kdcOptionsPb = MiniKdcOptionsPB.newBuilder();
private Common.HmsMode hmsMode = Common.HmsMode.NONE;
@@ -690,6 +695,11 @@ public final class MiniKuduCluster implements AutoCloseable {
return this;
}
+ public MiniKuduClusterBuilder principal(String principal) {
+ this.principal = principal;
+ return this;
+ }
+
/**
* Builds and starts a new {@link MiniKuduCluster} using builder state.
* @return the newly started {@link MiniKuduCluster}
@@ -700,7 +710,7 @@ public final class MiniKuduCluster implements AutoCloseable {
new MiniKuduCluster(enableKerberos,
numMasterServers, numTabletServers,
extraTabletServerFlags, extraMasterServerFlags, locationInfo,
- kdcOptionsPb.build(), clusterRoot, hmsMode);
+ kdcOptionsPb.build(), clusterRoot, hmsMode, principal);
try {
cluster.start();
} catch (IOException e) {
[kudu] 03/03: [python] KUDU-1884 Make SASL protocol configurable
Posted by ab...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
abukor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 308673dea705a3c4216e3e967b75eea70f39a850
Author: Attila Bukor <ab...@apache.org>
AuthorDate: Wed Apr 7 14:22:18 2021 +0200
[python] KUDU-1884 Make SASL protocol configurable
Similar to the C++ and Java clients in previous commits, this patch adds
support for configuring sasl_protocol_name in the Python client to match
the server-side Kerberos service principal name.
Change-Id: Iddff06c1a4cc2d07a9bbd08bf03d7c863aa6a645
Reviewed-on: http://gerrit.cloudera.org:8080/17281
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
---
python/kudu/client.pyx | 5 ++++-
python/kudu/libkudu_client.pxd | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/python/kudu/client.pyx b/python/kudu/client.pyx
index 8edf511..777ee99 100644
--- a/python/kudu/client.pyx
+++ b/python/kudu/client.pyx
@@ -281,7 +281,7 @@ cdef class Client:
"""
def __cinit__(self, addr_or_addrs, admin_timeout_ms=None,
- rpc_timeout_ms=None):
+ rpc_timeout_ms=None, sasl_protocol_name=None):
cdef:
string c_addr
vector[string] c_addrs
@@ -323,6 +323,9 @@ cdef class Client:
timeout = TimeDelta.from_millis(rpc_timeout_ms)
builder.default_rpc_timeout(timeout.delta)
+ if sasl_protocol_name is not None:
+ builder.sasl_protocol_name(sasl_protocol_name)
+
check_status(builder.Build(&self.client))
# A convenience
diff --git a/python/kudu/libkudu_client.pxd b/python/kudu/libkudu_client.pxd
index 54fc553..dc3b782 100644
--- a/python/kudu/libkudu_client.pxd
+++ b/python/kudu/libkudu_client.pxd
@@ -577,6 +577,8 @@ cdef extern from "kudu/client/client.h" namespace "kudu::client" nogil:
KuduClientBuilder& default_rpc_timeout(const MonoDelta& timeout)
+ KuduClientBuilder& sasl_protocol_name(const string& sasl_protocol_name)
+
Status Build(shared_ptr[KuduClient]* client)
cdef cppclass KuduTabletServer: