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:10 UTC
[kudu] 01/03: KUDU-1884 Make Kerberos principal customizable
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)));