This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 35de430 [txns] Allow the TxnSystemClient to run in Authorized
environments
35de430 is described below
commit 35de43051b65c8eab0f348bfd7c8a0d6e49c8067
Author: Grant Henke <granthe...@apache.org>
AuthorDate: Sun May 23 16:34:30 2021 -0500
[txns] Allow the TxnSystemClient to run in Authorized environments
When writing a test that runs the TxnSystemClient in an authorized
environment I saw issues where the TxnSystemClient could not
be initialized due to being unauthorized. It turns out that there
are many remote methods that the service user is not authorized
to call. This patch fixes the issue by updating the various required
calls to also allow the service user. I also enhanced a test to leverage
transactions in security-itest.cc
Here is a sample error I saw before this change:
W0524 08:57:12.618356 216932352 server_base.cc:694] Unauthorized access
attempt to method kudu.master.MasterService.ConnectToMaster from
{username='kudu', principal='kudu/127.0....@krbtest.com'} at 127.0.0.1:51639
W0524 08:57:12.618837 94556160 txn_system_client.cc:478] unable to
initialize TxnSystemClient, will retry in 1.000s: Remote error: Could not
connect to the cluster: Not authorized: unauthorized access to method:
ConnectToMaster
Change-Id: I4622586fc27b5b67005bf023e4fbaebaf5454ad0
Reviewed-on: http://gerrit.cloudera.org:8080/17490
Tested-by: Alexey Serbin <aser...@cloudera.com>
Reviewed-by: Grant Henke <granthe...@apache.org>
---
src/kudu/client/client-test.cc | 17 ++++++++++---
src/kudu/integration-tests/security-itest.cc | 26 ++++++++++++++++---
.../integration-tests/txn_status_table-itest.cc | 29 ++++++++++++++++------
src/kudu/master/master.proto | 12 ++++-----
4 files changed, 64 insertions(+), 20 deletions(-)
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index 72725af..e5c073e 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -6881,9 +6881,20 @@ TEST_F(ClientTest,
TestAuthenticationCredentialsRealUser) {
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());
+ // Try to delete a table without setting the user, which should fail: the
+ // coarse, RPC-level authz uses "AuthorizeClient" method for the
+ // MasterService::DeleteTable() RPC.
+ {
+ shared_ptr<KuduClient> c;
+ ASSERT_OK(cluster_->CreateClient(nullptr, &c));
+ ASSERT_NE(nullptr, c.get());
+ // TODO(KUDU-2344): ideally, this should have failed with NotAuthorized
+ const auto s = c->DeleteTable(client_table_->name());
+ const auto errmsg = s.ToString();
+ ASSERT_TRUE(s.IsRemoteError()) << errmsg;
+ ASSERT_STR_CONTAINS(
+ errmsg, "Not authorized: unauthorized access to method: DeleteTable");
+ }
// Create a new client with the imported user name and smoke test it.
KuduClientBuilder client_builder;
diff --git a/src/kudu/integration-tests/security-itest.cc
b/src/kudu/integration-tests/security-itest.cc
index aeb88e2..b6e93c1 100644
--- a/src/kudu/integration-tests/security-itest.cc
+++ b/src/kudu/integration-tests/security-itest.cc
@@ -81,6 +81,8 @@ using kudu::client::KuduSchema;
using kudu::client::KuduSession;
using kudu::client::KuduTable;
using kudu::client::KuduTableCreator;
+using kudu::client::KuduTransaction;
+using kudu::client::sp::shared_ptr;
using kudu::cluster::ExternalMiniCluster;
using kudu::cluster::ExternalMiniClusterOptions;
using kudu::rpc::Messenger;
@@ -134,7 +136,8 @@ 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(const client::sp::shared_ptr<KuduClient>& client,
+ bool transactional = false);
void SmokeTestCluster();
Status TryRegisterAsTS() {
@@ -199,20 +202,31 @@ class SecurityITest : public KuduTest {
unique_ptr<ExternalMiniCluster> cluster_;
};
-void SecurityITest::SmokeTestCluster(const client::sp::shared_ptr<KuduClient>&
client) {
+void SecurityITest::SmokeTestCluster(const client::sp::shared_ptr<KuduClient>&
client,
+ const bool transactional) {
// Create a table.
ASSERT_OK(CreateTestTable(client));
// Insert a row.
client::sp::shared_ptr<KuduTable> table;
ASSERT_OK(client->OpenTable(kTableName, &table));
- client::sp::shared_ptr<KuduSession> session = client->NewSession();
+ shared_ptr<KuduTransaction> txn;
+ client::sp::shared_ptr<KuduSession> session;
+ if (transactional) {
+ ASSERT_OK(client->NewTransaction(&txn));
+ ASSERT_OK(txn->CreateSession(&session));
+ } else {
+ session = client->NewSession();
+ }
session->SetTimeoutMillis(60000);
unique_ptr<KuduInsert> ins(table->NewInsert());
ASSERT_OK(ins->mutable_row()->SetInt32(0, 12345));
ASSERT_OK(ins->mutable_row()->SetInt32(1, 54321));
ASSERT_OK(session->Apply(ins.release()));
FlushSessionOrDie(session);
+ if (transactional) {
+ ASSERT_OK(txn->Commit());
+ }
// Read it back.
ASSERT_EQ(1, CountTableRows(table.get()));
@@ -269,10 +283,14 @@ TEST_F(SecurityITest, TestAuthorizationOnChecksum) {
// Test creating a table, writing some data, reading data, and dropping
// the table.
TEST_F(SecurityITest, SmokeTestAsAuthorizedUser) {
+ cluster_opts_.extra_master_flags.emplace_back("--txn_manager_enabled=true");
ASSERT_OK(StartCluster());
ASSERT_OK(cluster_->kdc()->Kinit("test-user"));
- NO_FATALS(SmokeTestCluster());
+ client::sp::shared_ptr<KuduClient> client;
+ ASSERT_OK(cluster_->CreateClient(nullptr, &client));
+ NO_FATALS(SmokeTestCluster(client));
+ NO_FATALS(SmokeTestCluster(client, /* transactional */ true));
// Non-superuser clients should not be able to set flags.
Status s = TrySetFlagOnTS();
diff --git a/src/kudu/integration-tests/txn_status_table-itest.cc
b/src/kudu/integration-tests/txn_status_table-itest.cc
index ce99d4c..66b429d 100644
--- a/src/kudu/integration-tests/txn_status_table-itest.cc
+++ b/src/kudu/integration-tests/txn_status_table-itest.cc
@@ -313,10 +313,10 @@ TEST_F(TxnStatusTableITest, TestProtectAccess) {
Status _s = (s); \
results.emplace_back(_s.CloneAndPrepend(msg)); \
} while (0)
- const auto attempt_accesses_and_delete = [&] (KuduClient* client) {
- // Go through various table access methods on the transaction status table.
- // Collect the results so we can verify whether we were authorized
- // properly.
+ const auto attempt_accesses = [&] (KuduClient* client) {
+ // Go through various table access methods on the transaction status table,
+ // except for dropping/deleting it. Collect the results so we can verify
+ // whether we were authorized properly.
vector<Status> results;
bool unused;
STORE_AND_PREPEND(client->TableExists(kTableName, &unused), "failed to
check existence");
@@ -325,6 +325,13 @@ TEST_F(TxnStatusTableITest, TestProtectAccess) {
client::KuduTableStatistics* unused_stats = nullptr;
STORE_AND_PREPEND(client->GetTableStatistics(kTableName, &unused_stats),
"failed to get stats");
unique_ptr<client::KuduTableStatistics> unused_unique_stats(unused_stats);
+ return results;
+ };
+ const auto attempt_accesses_and_delete = [&] (KuduClient* client) {
+ // Go through various table access methods on the transaction status table,
+ // including dropping the table. Collect the results so we can verify
+ // whether we were authorized properly.
+ vector<Status> results = attempt_accesses(client);
STORE_AND_PREPEND(client->DeleteTable(kTableName), "failed to delete
table");
return results;
};
@@ -339,13 +346,21 @@ TEST_F(TxnStatusTableITest, TestProtectAccess) {
// Even though we're the service user, we shouldn't be able to access
// anything because most RPCs require us to be a super-user or user.
NO_FATALS(SetSuperuserAndUser("nobody", "nobody"));
- results = attempt_accesses_and_delete(client_sp.get());
+ results = attempt_accesses(client_sp.get());
for (const auto& r : results) {
// NOTE: we don't check the type because authorization errors may surface
// as RemoteErrors or NotAuthorized errors.
- ASSERT_FALSE(r.ok());
- ASSERT_STR_CONTAINS(r.ToString(), "Not authorized");
+ ASSERT_TRUE(r.ok()) << r.ToString();
}
+
+ {
+ const auto s = client_sp->DeleteTable(kTableName);
+ const auto errmsg = s.ToString();
+ ASSERT_TRUE(s.IsRemoteError()) << errmsg;
+ ASSERT_STR_CONTAINS(
+ errmsg, "Not authorized: unauthorized access to method: DeleteTable");
+ }
+
// As a sanity check, our last delete have failed, and we should be unable to
// create another transaction status table.
Status s = txn_sys_client_->CreateTxnStatusTable(100);
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index 92b1752..c1dee96 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -1077,11 +1077,11 @@ service MasterService {
// Used only by Kudu 1.3 and later.
rpc ConnectToMaster(ConnectToMasterRequestPB) returns
(ConnectToMasterResponsePB) {
- option (kudu.rpc.authz_method) = "AuthorizeClient";
+ option (kudu.rpc.authz_method) = "AuthorizeClientOrServiceUser";
}
rpc GetTabletLocations(GetTabletLocationsRequestPB) returns
(GetTabletLocationsResponsePB) {
- option (kudu.rpc.authz_method) = "AuthorizeClient";
+ option (kudu.rpc.authz_method) = "AuthorizeClientOrServiceUser";
}
rpc CreateTable(CreateTableRequestPB) returns (CreateTableResponsePB) {
@@ -1102,18 +1102,18 @@ service MasterService {
}
rpc ListTables(ListTablesRequestPB) returns (ListTablesResponsePB) {
- option (kudu.rpc.authz_method) = "AuthorizeClient";
+ option (kudu.rpc.authz_method) = "AuthorizeClientOrServiceUser";
}
rpc GetTableStatistics(GetTableStatisticsRequestPB) returns
(GetTableStatisticsResponsePB) {
- option (kudu.rpc.authz_method) = "AuthorizeClient";
+ option (kudu.rpc.authz_method) = "AuthorizeClientOrServiceUser";
}
rpc GetTableLocations(GetTableLocationsRequestPB) returns
(GetTableLocationsResponsePB) {
- option (kudu.rpc.authz_method) = "AuthorizeClient";
+ option (kudu.rpc.authz_method) = "AuthorizeClientOrServiceUser";
}
rpc GetTableSchema(GetTableSchemaRequestPB) returns
(GetTableSchemaResponsePB) {
- option (kudu.rpc.authz_method) = "AuthorizeClient";
+ option (kudu.rpc.authz_method) = "AuthorizeClientOrServiceUser";
}
// Administrative/monitoring RPCs
