This is an automated email from the ASF dual-hosted git repository.
hahao 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 beaae49 [sentry] add privilege scope validation to SentryAuthzProvider
beaae49 is described below
commit beaae491f407d5746f509f1d2dce872e8a9ae803
Author: Hao Hao <[email protected]>
AuthorDate: Fri Feb 15 10:16:23 2019 -0800
[sentry] add privilege scope validation to SentryAuthzProvider
Currently, SentryAuthzProvider::Authorize() performs authorization based
on the rules that a privilege implies another when:
1. the authorizable from the former implies the authorizable from the
latter, and
2. the action from the former implies the action from the latter, and
3. grant option from the former implies the grant option from the latter.
We fetch from Sentry the privileges for the given user that might imply
the given authorizable and action (via
list_sentry_privileges_by_user_and_itsgroups
Sentry API). We then proceed to check the implication rules on the
returned actions. However, we rely on the API to enforce the implication
rules of the returned authorizables. This ignored the fact that the API
returns all privileges granted to the user that match the authorizable
of each scope in the input authorizable hierarchy (not just the implied
authorizables).
So when Alice tries to create table 'default.b' (this requires 'CREATE
ON DATABASE'), the Sentry API will return anything that matches:
server == "server1" && (db == "default" || db == NULL)
which may include 'ALL ON default.a'. Previously we would only take into
consideration the fact that 'ALL' is returned, and proceed to incorrectly
permit the create operation.
This patch adds privilege scope validation to SentryAuthzProvider to
ensure only authorizable with a higher scope on the hierarchy can imply
authorizables with a lower scope on the hierarchy.
Theoretically speaking, an alternative is to add authorizable scope
filtering in Sentry server. However, such an API is inherently a Kudu-only
API, which by necessity would filter out authorizable wildcard matching
(that Sentry's default client policy validation supports). Moreover,
client-side filtering can also help enabling more aggressive caching.
Change-Id: I89437a04a4fa18e501d21c3abf5d66a2d22ce58a
Reviewed-on: http://gerrit.cloudera.org:8080/12500
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <[email protected]>
Reviewed-by: Andrew Wong <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
src/kudu/master/sentry_authz_provider-test.cc | 274 +++++++++++++++++-----
src/kudu/master/sentry_authz_provider.cc | 142 +++++++----
src/kudu/master/sentry_authz_provider.h | 32 ++-
src/kudu/sentry/CMakeLists.txt | 2 +
src/kudu/sentry/sentry-test-base.h | 8 +-
src/kudu/sentry/sentry_action.cc | 45 ++--
src/kudu/sentry/sentry_action.h | 19 +-
src/kudu/sentry/sentry_authorizable_scope-test.cc | 83 +++++++
src/kudu/sentry/sentry_authorizable_scope.cc | 101 ++++++++
src/kudu/sentry/sentry_authorizable_scope.h | 83 +++++++
src/kudu/sentry/sentry_client-test.cc | 10 +-
src/kudu/sentry/sentry_client.h | 11 +-
12 files changed, 653 insertions(+), 157 deletions(-)
diff --git a/src/kudu/master/sentry_authz_provider-test.cc
b/src/kudu/master/sentry_authz_provider-test.cc
index eb70366..23e1c2f 100644
--- a/src/kudu/master/sentry_authz_provider-test.cc
+++ b/src/kudu/master/sentry_authz_provider-test.cc
@@ -20,12 +20,17 @@
#include <memory>
#include <set>
#include <string>
+#include <vector>
#include <gflags/gflags_declare.h>
#include <gtest/gtest.h>
+#include "kudu/gutil/macros.h"
+#include "kudu/gutil/strings/substitute.h"
#include "kudu/sentry/mini_sentry.h"
#include "kudu/sentry/sentry-test-base.h"
+#include "kudu/sentry/sentry_action.h"
+#include "kudu/sentry/sentry_authorizable_scope.h"
#include "kudu/sentry/sentry_client.h"
#include "kudu/sentry/sentry_policy_service_types.h"
#include "kudu/util/net/net_util.h"
@@ -44,32 +49,37 @@ using sentry::TAlterSentryRoleAddGroupsResponse;
using sentry::TAlterSentryRoleGrantPrivilegeRequest;
using sentry::TAlterSentryRoleGrantPrivilegeResponse;
using sentry::TCreateSentryRoleRequest;
+using sentry::TDropSentryRoleRequest;
using sentry::TSentryGrantOption;
using sentry::TSentryGroup;
using sentry::TSentryPrivilege;
+using std::tuple;
using std::unique_ptr;
using std::set;
using std::string;
+using std::vector;
+using strings::Substitute;
namespace kudu {
+using sentry::SentryAction;
using sentry::SentryTestBase;
+using sentry::SentryAuthorizableScope;
namespace master {
-// Class for SentryAuthzProvider tests. Parameterized by whether
-// Kerberos should be enabled.
class SentryAuthzProviderTest : public SentryTestBase {
public:
const char* const kAdminUser = "test-admin";
- const char* const kUserGroup = "user";
const char* const kTestUser = "test-user";
+ const char* const kUserGroup = "user";
+ const char* const kRoleName = "developer";
void SetUp() override {
SentryTestBase::SetUp();
// Configure the SentryAuthzProvider flags.
- FLAGS_sentry_service_security_mode = kerberos_enabled_ ? "kerberos" :
"none";
+ FLAGS_sentry_service_security_mode = KerberosEnabled() ? "kerberos" :
"none";
FLAGS_sentry_service_rpc_addresses = sentry_->address().ToString();
sentry_authz_provider_.reset(new SentryAuthzProvider());
ASSERT_OK(sentry_authz_provider_->Start());
@@ -87,6 +97,13 @@ class SentryAuthzProviderTest : public SentryTestBase {
return Status::OK();
}
+ Status DropRole(const string& role_name) {
+ TDropSentryRoleRequest role_req;
+ role_req.__set_requestorUserName(kAdminUser);
+ role_req.__set_roleName(role_name);
+ return sentry_client_->DropRole(role_req);
+ }
+
Status CreateRoleAndAddToGroups(const string& role_name, const string&
group_name) {
TCreateSentryRoleRequest role_req;
role_req.__set_requestorUserName(kAdminUser);
@@ -114,38 +131,76 @@ class SentryAuthzProviderTest : public SentryTestBase {
return sentry_client_->AlterRoleGrantPrivilege(privilege_request,
&privilege_response);
}
- // Returns a database level TSentryPrivilege with the given database name,
action
+ // Returns a server level TSentryPrivilege with the server name, action
// and grant option.
- TSentryPrivilege GetDatabasePrivilege(const string& db_name,
- const string& action,
- const TSentryGrantOption::type&
grant_option) {
+ TSentryPrivilege GetServerPrivilege(
+ const string& action,
+ TSentryGrantOption::type grant_option = TSentryGrantOption::DISABLED) {
TSentryPrivilege privilege;
- privilege.__set_privilegeScope("DATABASE");
+ privilege.__set_privilegeScope("SERVER");
privilege.__set_serverName(FLAGS_server_name);
- privilege.__set_dbName(db_name);
privilege.__set_action(action);
privilege.__set_grantOption(grant_option);
return privilege;
}
+ // Returns a database level TSentryPrivilege with the given database name,
action
+ // and grant option.
+ TSentryPrivilege GetDatabasePrivilege(
+ const string& db_name,
+ const string& action,
+ const TSentryGrantOption::type& grant_option =
TSentryGrantOption::DISABLED) {
+ TSentryPrivilege privilege = GetServerPrivilege(action, grant_option);
+ privilege.__set_privilegeScope("DATABASE");
+ privilege.__set_dbName(db_name);
+ return privilege;
+ }
+
// Returns a table level TSentryPrivilege with the given table name,
database name,
// action and grant option.
- TSentryPrivilege GetTablePrivilege(const string& db_name,
- const string& table_name,
- const string& action,
- const TSentryGrantOption::type&
grant_option) {
+ TSentryPrivilege GetTablePrivilege(
+ const string& db_name,
+ const string& table_name,
+ const string& action,
+ const TSentryGrantOption::type& grant_option =
TSentryGrantOption::DISABLED) {
TSentryPrivilege privilege = GetDatabasePrivilege(db_name, action,
grant_option);
+ privilege.__set_privilegeScope("TABLE");
privilege.__set_tableName(table_name);
return privilege;
}
+ // Returns a column level TSentryPrivilege with the given column name, table
name,
+ // database name, action and grant option.
+ TSentryPrivilege GetColumnPrivilege(
+ const string& db_name,
+ const string& table_name,
+ const string& column_name,
+ const string& action,
+ const TSentryGrantOption::type& grant_option =
TSentryGrantOption::DISABLED) {
+ TSentryPrivilege privilege = GetTablePrivilege(db_name, table_name,
+ action, grant_option);
+ privilege.__set_privilegeScope("COLUMN");
+ privilege.__set_columnName(column_name);
+ return privilege;
+ }
+
protected:
unique_ptr<SentryAuthzProvider> sentry_authz_provider_;
};
-INSTANTIATE_TEST_CASE_P(KerberosEnabled, SentryAuthzProviderTest,
::testing::Bool());
+// Tests to ensure SentryAuthzProvider enforces access control on tables as
expected.
+// Parameterized by whether Kerberos should be enabled.
+class TestTableAuthorization : public SentryAuthzProviderTest,
+ public ::testing::WithParamInterface<bool> {
+ public:
+ bool KerberosEnabled() const {
+ return GetParam();
+ }
+};
+
+INSTANTIATE_TEST_CASE_P(KerberosEnabled, TestTableAuthorization,
::testing::Bool());
-TEST_P(SentryAuthzProviderTest, TestAuthorizeCreateTable) {
+TEST_P(TestTableAuthorization, TestAuthorizeCreateTable) {
// Don't authorize create table on a non-existent user.
Status s = sentry_authz_provider_->AuthorizeCreateTable("db.table",
"non-existent-user",
@@ -157,55 +212,54 @@ TEST_P(SentryAuthzProviderTest, TestAuthorizeCreateTable)
{
ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
// Don't authorize create table on a user without required privileges.
- const string role_name = "developer";
- ASSERT_OK(CreateRoleAndAddToGroups(role_name, kUserGroup));
- TSentryPrivilege privilege = GetDatabasePrivilege("db", "DROP",
TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, privilege));
+ ASSERT_OK(CreateRoleAndAddToGroups(kRoleName, kUserGroup));
+ TSentryPrivilege privilege = GetDatabasePrivilege("db", "DROP");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
s = sentry_authz_provider_->AuthorizeCreateTable("db.table", kTestUser,
kTestUser);
ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
// Authorize create table on a user with proper privileges.
- privilege = GetDatabasePrivilege("db", "CREATE",
TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, privilege));
+ privilege = GetDatabasePrivilege("db", "CREATE");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
ASSERT_OK(sentry_authz_provider_->AuthorizeCreateTable("db.table",
kTestUser, kTestUser));
// Table creation with a different owner than the user
// requires the creating user have 'ALL on DATABASE' with grant.
s = sentry_authz_provider_->AuthorizeCreateTable("db.table", kTestUser,
"diff-user");
ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
+ privilege = GetDatabasePrivilege("db", "ALL");
+ s = sentry_authz_provider_->AuthorizeCreateTable("db.table", kTestUser,
"diff-user");
+ ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
privilege = GetDatabasePrivilege("db", "ALL", TSentryGrantOption::ENABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, privilege));
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
ASSERT_OK(sentry_authz_provider_->AuthorizeCreateTable("db.table",
kTestUser, "diff-user"));
}
-TEST_P(SentryAuthzProviderTest, TestAuthorizeDropTable) {
+TEST_P(TestTableAuthorization, TestAuthorizeDropTable) {
// Don't authorize delete table on a user without required privileges.
- const string role_name = "developer";
- ASSERT_OK(CreateRoleAndAddToGroups(role_name, kUserGroup));
- TSentryPrivilege privilege = GetDatabasePrivilege("db", "SELECT",
TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, privilege));
+ ASSERT_OK(CreateRoleAndAddToGroups(kRoleName, kUserGroup));
+ TSentryPrivilege privilege = GetDatabasePrivilege("db", "SELECT");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
Status s = sentry_authz_provider_->AuthorizeDropTable("db.table", kTestUser);
ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
// Authorize delete table on a user with proper privileges.
- privilege = GetDatabasePrivilege("db", "DROP", TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, privilege));
+ privilege = GetDatabasePrivilege("db", "DROP");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
ASSERT_OK(sentry_authz_provider_->AuthorizeDropTable("db.table", kTestUser));
}
-TEST_P(SentryAuthzProviderTest, TestAuthorizeAlterTable) {
+TEST_P(TestTableAuthorization, TestAuthorizeAlterTable) {
// Don't authorize alter table on a user without required privileges.
- const string role_name = "developer";
- ASSERT_OK(CreateRoleAndAddToGroups(role_name, kUserGroup));
- TSentryPrivilege db_privilege = GetDatabasePrivilege("db", "SELECT",
-
TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, db_privilege));
+ ASSERT_OK(CreateRoleAndAddToGroups(kRoleName, kUserGroup));
+ TSentryPrivilege db_privilege = GetDatabasePrivilege("db", "SELECT");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, db_privilege));
Status s = sentry_authz_provider_->AuthorizeAlterTable("db.table",
"db.table", kTestUser);
ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
// Authorize alter table without rename on a user with proper privileges.
- db_privilege = GetDatabasePrivilege("db", "ALTER",
TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, db_privilege));
+ db_privilege = GetDatabasePrivilege("db", "ALTER");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, db_privilege));
ASSERT_OK(sentry_authz_provider_->AuthorizeAlterTable("db.table",
"db.table", kTestUser));
// Table alteration with rename requires 'ALL ON TABLE <old-table>' and
@@ -214,47 +268,43 @@ TEST_P(SentryAuthzProviderTest, TestAuthorizeAlterTable) {
ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
// Authorize alter table without rename on a user with proper privileges.
- db_privilege = GetDatabasePrivilege("new_db", "CREATE",
TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, db_privilege));
- TSentryPrivilege table_privilege = GetTablePrivilege("db", "table", "ALL",
-
TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, table_privilege));
+ db_privilege = GetDatabasePrivilege("new_db", "CREATE");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, db_privilege));
+ TSentryPrivilege table_privilege = GetTablePrivilege("db", "table", "ALL");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, table_privilege));
ASSERT_OK(sentry_authz_provider_->AuthorizeAlterTable("db.table",
"new_db.new_table",
kTestUser));
}
-TEST_P(SentryAuthzProviderTest, TestAuthorizeGetTableMetadata) {
+TEST_P(TestTableAuthorization, TestAuthorizeGetTableMetadata) {
// Don't authorize delete table on a user without required privileges.
- const string role_name = "developer";
- ASSERT_OK(CreateRoleAndAddToGroups(role_name, kUserGroup));
+ ASSERT_OK(CreateRoleAndAddToGroups(kRoleName, kUserGroup));
Status s = sentry_authz_provider_->AuthorizeGetTableMetadata("db.table",
kTestUser);
ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
// Authorize delete table on a user with proper privileges.
- TSentryPrivilege privilege = GetDatabasePrivilege("db", "SELECT",
TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, privilege));
+ TSentryPrivilege privilege = GetDatabasePrivilege("db", "SELECT");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
ASSERT_OK(sentry_authz_provider_->AuthorizeGetTableMetadata("db.table",
kTestUser));
}
// Checks that the SentryAuthzProvider handles reconnecting to Sentry after a
connection failure,
// or service being too busy.
-TEST_P(SentryAuthzProviderTest, TestReconnect) {
+TEST_P(TestTableAuthorization, TestReconnect) {
// Restart SentryAuthzProvider with configured timeout to reduce the run
time of this test.
NO_FATALS(sentry_authz_provider_->Stop());
- FLAGS_sentry_service_security_mode = kerberos_enabled_ ? "kerberos" : "none";
+ FLAGS_sentry_service_security_mode = KerberosEnabled() ? "kerberos" : "none";
FLAGS_sentry_service_rpc_addresses = sentry_->address().ToString();
FLAGS_sentry_service_send_timeout_seconds = AllowSlowTests() ? 5 : 2;
FLAGS_sentry_service_recv_timeout_seconds = AllowSlowTests() ? 5 : 2;
sentry_authz_provider_.reset(new SentryAuthzProvider());
ASSERT_OK(sentry_authz_provider_->Start());
- const string role_name = "developer";
- ASSERT_OK(CreateRoleAndAddToGroups(role_name, kUserGroup));
- TSentryPrivilege privilege = GetDatabasePrivilege("db", "METADATA",
-
TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, privilege));
+ ASSERT_OK(CreateRoleAndAddToGroups(kRoleName, kUserGroup));
+ TSentryPrivilege privilege = GetDatabasePrivilege("db", "METADATA");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
ASSERT_OK(sentry_authz_provider_->AuthorizeGetTableMetadata("db.table",
kTestUser));
// Shutdown Sentry and try a few operations.
@@ -273,8 +323,8 @@ TEST_P(SentryAuthzProviderTest, TestReconnect) {
"db.table", kTestUser));
});
- privilege = GetDatabasePrivilege("db", "DROP", TSentryGrantOption::DISABLED);
- ASSERT_OK(AlterRoleGrantPrivilege(role_name, privilege));
+ privilege = GetDatabasePrivilege("db", "DROP");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
ASSERT_OK(sentry_authz_provider_->AuthorizeDropTable("db.table", kTestUser));
// Pause Sentry and try a few operations.
@@ -294,5 +344,115 @@ TEST_P(SentryAuthzProviderTest, TestReconnect) {
});
}
+TEST_P(TestTableAuthorization, TestInvalidAction) {
+ ASSERT_OK(CreateRoleAndAddToGroups(kRoleName, kUserGroup));
+ TSentryPrivilege privilege = GetDatabasePrivilege("db", "invalid");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
+ // User has privileges with invalid action cannot operate on the table.
+ Status s = sentry_authz_provider_->AuthorizeCreateTable("DB.table",
kTestUser, kTestUser);
+ ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
+}
+
+TEST_P(TestTableAuthorization, TestInvalidAuthzScope) {
+ ASSERT_OK(CreateRoleAndAddToGroups(kRoleName, kUserGroup));
+ TSentryPrivilege privilege = GetDatabasePrivilege("db", "ALL");
+ privilege.__set_privilegeScope("invalid");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
+ // User has privileges with invalid authorizable scope cannot operate
+ // on the table.
+ Status s = sentry_authz_provider_->AuthorizeCreateTable("DB.table",
kTestUser, kTestUser);
+ ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
+}
+
+// Ensures Sentry privileges are case insensitive.
+TEST_P(TestTableAuthorization, TestPrivilegeCaseSensitivity) {
+ ASSERT_OK(CreateRoleAndAddToGroups(kRoleName, kUserGroup));
+ TSentryPrivilege privilege = GetDatabasePrivilege("db", "create");
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
+ ASSERT_OK(sentry_authz_provider_->AuthorizeCreateTable("DB.table",
kTestUser, kTestUser));
+}
+
+// Test to ensure the authorization hierarchy rule of SentryAuthzProvider
+// works as expected.
+class TestAuthzHierarchy : public SentryAuthzProviderTest,
+ public ::testing::WithParamInterface<
+ tuple<bool, SentryAuthorizableScope::Scope>> {
+ public:
+ bool KerberosEnabled() const {
+ return std::get<0>(GetParam());
+ }
+};
+
+TEST_P(TestAuthzHierarchy, TestAuthorizableScope) {
+ SentryAuthorizableScope::Scope scope = std::get<1>(GetParam());
+ const string action = "ALL";
+ const string db = "database";
+ const string tbl = "table";
+ const string col = "col";
+ vector<TSentryPrivilege> lower_hierarchy_privs;
+ vector<TSentryPrivilege> higher_hierarchy_privs;
+ const TSentryPrivilege column_priv = GetColumnPrivilege(db, tbl, col,
action);
+ const TSentryPrivilege table_priv = GetTablePrivilege(db, tbl, action);
+ const TSentryPrivilege db_priv = GetDatabasePrivilege(db, action);
+ const TSentryPrivilege server_priv = GetServerPrivilege(action);
+
+ switch (scope) {
+ case SentryAuthorizableScope::Scope::TABLE:
+ higher_hierarchy_privs.emplace_back(table_priv);
+ FALLTHROUGH_INTENDED;
+ case SentryAuthorizableScope::Scope::DATABASE:
+ higher_hierarchy_privs.emplace_back(db_priv);
+ FALLTHROUGH_INTENDED;
+ case SentryAuthorizableScope::Scope::SERVER:
+ higher_hierarchy_privs.emplace_back(server_priv);
+ break;
+ default:
+ break;
+ }
+
+ switch (scope) {
+ case SentryAuthorizableScope::Scope::SERVER:
+ lower_hierarchy_privs.emplace_back(db_priv);
+ FALLTHROUGH_INTENDED;
+ case SentryAuthorizableScope::Scope::DATABASE:
+ lower_hierarchy_privs.emplace_back(table_priv);
+ FALLTHROUGH_INTENDED;
+ case SentryAuthorizableScope::Scope::TABLE:
+ lower_hierarchy_privs.emplace_back(column_priv);
+ break;
+ default:
+ break;
+ }
+
+ // Privilege with higher scope on the hierarchy can imply privileges
+ // with lower scope on the hierarchy.
+ for (const auto& privilege : higher_hierarchy_privs) {
+ ASSERT_OK(CreateRoleAndAddToGroups(kRoleName, kUserGroup));
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
+ ASSERT_OK(sentry_authz_provider_->Authorize(scope,
SentryAction::Action::ALL,
+ Substitute("$0.$1", db, tbl),
kTestUser));
+ ASSERT_OK(DropRole(kRoleName));
+ }
+
+ // Privilege with lower scope on the hierarchy cannot imply privileges
+ // with higher scope on the hierarchy.
+ for (const auto& privilege : lower_hierarchy_privs) {
+ ASSERT_OK(CreateRoleAndAddToGroups(kRoleName, kUserGroup));
+ ASSERT_OK(AlterRoleGrantPrivilege(kRoleName, privilege));
+ Status s = sentry_authz_provider_->Authorize(scope,
SentryAction::Action::ALL,
+ Substitute("$0.$1", db, tbl),
kTestUser);
+ ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
+ ASSERT_OK(DropRole(kRoleName));
+ }
+}
+
+INSTANTIATE_TEST_CASE_P(AuthzCombinations, TestAuthzHierarchy,
+ ::testing::Combine(::testing::Bool(),
+ // Scope::COLUMN is excluded since column scope for
table
+ // authorizable doesn't make sense.
+
::testing::Values(SentryAuthorizableScope::Scope::SERVER,
+
SentryAuthorizableScope::Scope::DATABASE,
+
SentryAuthorizableScope::Scope::TABLE)));
+
} // namespace master
} // namespace kudu
diff --git a/src/kudu/master/sentry_authz_provider.cc
b/src/kudu/master/sentry_authz_provider.cc
index d2ff567..2c5674b 100644
--- a/src/kudu/master/sentry_authz_provider.cc
+++ b/src/kudu/master/sentry_authz_provider.cc
@@ -117,6 +117,7 @@ namespace kudu {
using sentry::SentryAction;
using sentry::SentryClient;
+using sentry::SentryAuthorizableScope;
namespace master {
@@ -164,28 +165,35 @@ bool SentryAuthzProvider::IsEnabled() {
namespace {
-// Returns an authorizable based on the table name and the given scope.
-Status GetAuthorizable(const string& table_name,
- SentryAuthzProvider::AuthorizableScope scope,
+// Returns an authorizable based on the table identifier (in the format
+// <database-name>.<table-name>) and the given scope.
+Status GetAuthorizable(const string& table_ident,
+ SentryAuthorizableScope::Scope scope,
TSentryAuthorizable* authorizable) {
Slice database;
Slice table;
+ // Authorizable scope for table authorizable type must be equal or higher
than
+ // 'TABLE'.
+ DCHECK_NE(scope, SentryAuthorizableScope::Scope::COLUMN);
switch (scope) {
- case SentryAuthzProvider::AuthorizableScope::TABLE:
- RETURN_NOT_OK(ParseHiveTableIdentifier(table_name, &database, &table));
+ case SentryAuthorizableScope::Scope::TABLE:
+ RETURN_NOT_OK(ParseHiveTableIdentifier(table_ident, &database, &table));
+ DCHECK(!table.empty());
authorizable->__set_table(table.ToString());
FALLTHROUGH_INTENDED;
- case SentryAuthzProvider::AuthorizableScope::DATABASE:
+ case SentryAuthorizableScope::Scope::DATABASE:
if (database.empty() && table.empty()) {
- RETURN_NOT_OK(ParseHiveTableIdentifier(table_name, &database, &table));
+ RETURN_NOT_OK(ParseHiveTableIdentifier(table_ident, &database,
&table));
}
+ DCHECK(!database.empty());
authorizable->__set_db(database.ToString());
FALLTHROUGH_INTENDED;
- case SentryAuthzProvider::AuthorizableScope::SERVER:
+ case SentryAuthorizableScope::Scope::SERVER:
authorizable->__set_server(FLAGS_server_name);
break;
default:
- LOG(FATAL) << "unsupported authorizable scope";
+ LOG(FATAL) << "unsupported SentryAuthorizableScope: "
+ << sentry::ScopeToString(scope);
break;
}
@@ -202,27 +210,25 @@ Status SentryAuthzProvider::AuthorizeCreateTable(const
string& table_name,
// design doc in
[SENTRY-2151](https://issues.apache.org/jira/browse/SENTRY-2151).
//
// Otherwise, table creation requires 'CREATE ON DATABASE' privilege.
- TSentryAuthorizable authorizable;
- RETURN_NOT_OK(GetAuthorizable(table_name, AuthorizableScope::DATABASE,
&authorizable));
- SentryAction action;
+ SentryAction::Action action;
bool grant_option;
if (user == owner) {
- action = SentryAction(SentryAction::Action::CREATE);
+ action = SentryAction::Action::CREATE;
grant_option = false;
} else {
- action = SentryAction(SentryAction::Action::ALL);
+ action = SentryAction::Action::ALL;
grant_option = true;
}
- return Authorize(authorizable, action, user, grant_option);
+ return Authorize(SentryAuthorizableScope::Scope::DATABASE, action,
+ table_name, user, grant_option);
}
Status SentryAuthzProvider::AuthorizeDropTable(const string& table_name,
const string& user) {
// Table deletion requires 'DROP ON TABLE' privilege.
- TSentryAuthorizable authorizable;
- RETURN_NOT_OK(GetAuthorizable(table_name, AuthorizableScope::TABLE,
&authorizable));
- SentryAction action = SentryAction(SentryAction::Action::DROP);
- return Authorize(authorizable, action, user);
+ return Authorize(SentryAuthorizableScope::Scope::TABLE,
+ SentryAction::Action::DROP,
+ table_name, user);
}
Status SentryAuthzProvider::AuthorizeAlterTable(const string& old_table,
@@ -235,43 +241,56 @@ Status SentryAuthzProvider::AuthorizeAlterTable(const
string& old_table,
// 2. 'CREATE ON DATABASE <new-database>'.
// See [SENTRY-2264](https://issues.apache.org/jira/browse/SENTRY-2264).
// TODO(hao): add inline hierarchy validation to avoid multiple RPCs.
- TSentryAuthorizable table_authorizable;
- RETURN_NOT_OK(GetAuthorizable(old_table, AuthorizableScope::TABLE,
&table_authorizable));
if (old_table == new_table) {
- SentryAction action = SentryAction(SentryAction::Action::ALTER);
- return Authorize(table_authorizable, action, user);
+ return Authorize(SentryAuthorizableScope::Scope::TABLE,
+ SentryAction::Action::ALTER,
+ old_table, user);
}
-
- SentryAction table_action = SentryAction(SentryAction::Action::ALL);
- RETURN_NOT_OK(Authorize(table_authorizable, table_action, user));
- TSentryAuthorizable db_authorizable;
- RETURN_NOT_OK(GetAuthorizable(new_table, AuthorizableScope::DATABASE,
&db_authorizable));
- SentryAction db_action = SentryAction(SentryAction::Action::CREATE);
- return Authorize(db_authorizable, db_action, user);
+ RETURN_NOT_OK(Authorize(SentryAuthorizableScope::Scope::TABLE,
+ SentryAction::Action::ALL,
+ old_table, user));
+ return Authorize(SentryAuthorizableScope::Scope::DATABASE,
+ SentryAction::Action::CREATE,
+ new_table, user);
}
Status SentryAuthzProvider::AuthorizeGetTableMetadata(const std::string&
table_name,
const std::string& user)
{
// Retrieving table metadata requires 'METADATA ON TABLE' privilege.
- TSentryAuthorizable authorizable;
- RETURN_NOT_OK(GetAuthorizable(table_name, AuthorizableScope::TABLE,
&authorizable));
- SentryAction action = SentryAction(SentryAction::Action::METADATA);
- return Authorize(authorizable, action, user);
+ return Authorize(SentryAuthorizableScope::Scope::TABLE,
+ SentryAction::Action::METADATA,
+ table_name, user);
}
-Status SentryAuthzProvider::Authorize(const TSentryAuthorizable& authorizable,
- const SentryAction& action,
+Status SentryAuthzProvider::Authorize(SentryAuthorizableScope::Scope scope,
+ SentryAction::Action action,
+ const string& table_ident,
const string& user,
- bool grant_option) {
+ bool require_grant_option) {
- // In general, a privilege implies another when the authorizable from
- // the former implies the authorizable from the latter, the action
- // from the former implies the action from the latter, and grant option
- // from the former implies the grant option from the latter.
+ TSentryAuthorizable authorizable;
+ RETURN_NOT_OK(GetAuthorizable(table_ident, scope, &authorizable));
+
+ // In general, a privilege implies another when:
+ // 1. the authorizable from the former implies the authorizable from the
latter
+ // (authorizable with a higher scope on the hierarchy can imply
authorizables
+ // with a lower scope on the hierarchy, but not vice versa), and
+ // 2. the action from the former implies the action from the latter, and
+ // 3. grant option from the former implies the grant option from the latter.
+ //
+ // See org.apache.sentry.policy.common.CommonPrivilege. Note that policy
validation
+ // in CommonPrivilege also allows wildcard authorizable matching. For
example,
+ // authorizable 'server=server1->db=*' can imply authorizable
'server=server1'.
+ // However, wildcard authorizable granting is neither practical nor useful
(semantics
+ // of granting such privilege are not supported in Apache Hive, Impala and
Hue. And
+ // 'server=server1->db=*' has exactly the same meaning as 'server=server1').
Therefore,
+ // wildcard authorizable matching is dropped in this implementation.
//
- // ListPrivilegesByUser returns all privileges granted to the user that
- // imply the given authorizable. Therefore, we only need to validate if
- // the granted actions can imply the required one.
+ // Moreover, because ListPrivilegesByUser lists all Sentry privileges
granted to the
+ // user that match the authorizable of each scope in the input authorizable
hierarchy,
+ // privileges with lower scope will also be returned in the response. This
contradicts
+ // rule (1) mentioned above. Therefore, we need to validate privilege scope,
in addition
+ // to action and grant option. Otherwise, privilege escalation can happen.
TListSentryPrivilegesRequest request;
request.__set_requestorUserName(FLAGS_kudu_service_name);
@@ -284,26 +303,45 @@ Status SentryAuthzProvider::Authorize(const
TSentryAuthorizable& authorizable,
return client->ListPrivilegesByUser(request, &response);
}));
+ SentryAction required_action(action);
+ SentryAuthorizableScope required_scope(scope);
for (const auto& privilege : response.privileges) {
- // A grant option cannot imply the other if the former is set
- // but the latter is not.
- if (grant_option && privilege.grantOption != TSentryGrantOption::ENABLED) {
+ // A grant option cannot imply the other if the latter is set
+ // but the former is not.
+ if (require_grant_option && privilege.grantOption !=
TSentryGrantOption::ENABLED) {
continue;
}
SentryAction granted_action;
Status s = SentryAction::FromString(privilege.action, &granted_action);
- WARN_NOT_OK(s, Substitute("failed to construct sentry action from $0",
privilege.action));
- if (s.ok() && granted_action.Implies(action)) {
+ if (!s.ok()) {
+ LOG(WARNING) << s.ToString();
+ continue;
+ }
+
+ SentryAuthorizableScope granted_scope;
+ s = SentryAuthorizableScope::FromString(privilege.privilegeScope,
&granted_scope);
+ if (!s.ok()) {
+ LOG(WARNING) << s.ToString();
+ continue;
+ }
+
+ // Both privilege scope and action need to imply the other.
+ if (granted_action.Implies(required_action) &&
+ granted_scope.Implies(required_scope)) {
return Status::OK();
}
}
- // Logs an error if the action is not authorized for debugging purpose, and
+ // Logs a warning if the action is not authorized for debugging purpose, and
// only returns generic error back to the users to avoid side channel leak,
// e.g. 'whether table A exists'.
- LOG(ERROR) << "Action <" << action.action() << "> on authorizable <"
- << authorizable << "> is not permitted for user <" << user << ">";
+ LOG(WARNING) << Substitute("Action <$0> on table <$1> with authorizable
scope "
+ "<$2> is not permitted for user <$3>",
+ sentry::ActionToString(action),
+ table_ident,
+ sentry::ScopeToString(scope),
+ user);
return Status::NotAuthorized("unauthorized action");
}
diff --git a/src/kudu/master/sentry_authz_provider.h
b/src/kudu/master/sentry_authz_provider.h
index c2bbbff..e0f79c3 100644
--- a/src/kudu/master/sentry_authz_provider.h
+++ b/src/kudu/master/sentry_authz_provider.h
@@ -19,22 +19,18 @@
#include <string>
+#include <gtest/gtest_prod.h>
+
#include "kudu/gutil/port.h"
#include "kudu/master/authz_provider.h"
+#include "kudu/sentry/sentry_action.h"
+#include "kudu/sentry/sentry_authorizable_scope.h"
#include "kudu/sentry/sentry_client.h"
#include "kudu/thrift/client.h"
#include "kudu/util/status.h"
-namespace sentry {
-class TSentryAuthorizable;
-} // namespace sentry
-
namespace kudu {
-namespace sentry {
-class SentryAction;
-} // namespace sentry
-
namespace master {
// An implementation of AuthzProvider that connects to the Sentry service
@@ -45,12 +41,6 @@ namespace master {
class SentryAuthzProvider : public AuthzProvider {
public:
- enum class AuthorizableScope {
- SERVER,
- DATABASE,
- TABLE,
- };
-
~SentryAuthzProvider();
// Start SentryAuthzProvider instance which connects to the Sentry service.
@@ -88,13 +78,19 @@ class SentryAuthzProvider : public AuthzProvider {
static bool ValidateAddresses(const char* flag_name, const std::string&
addresses);
private:
+ FRIEND_TEST(TestAuthzHierarchy, TestAuthorizableScope);
- // Checks if the user can perform an action on the given authorizable.
+ // Checks if the user can perform an action on the table identifier (in the
format
+ // <database-name>.<table-name>), based on the given authorizable scope and
the
+ // grant option. Note that the authorizable scope should be equal or higher
than
+ // 'TABLE' scope.
+ //
// If the operation is not authorized, returns Status::NotAuthorized().
- Status Authorize(const ::sentry::TSentryAuthorizable& authorizable,
- const sentry::SentryAction& action,
+ Status Authorize(sentry::SentryAuthorizableScope::Scope scope,
+ sentry::SentryAction::Action action,
+ const std::string& table_ident,
const std::string& user,
- bool grant_option = false);
+ bool require_grant_option = false);
thrift::HaClient<sentry::SentryClient> ha_client_;
};
diff --git a/src/kudu/sentry/CMakeLists.txt b/src/kudu/sentry/CMakeLists.txt
index fee3855..acd0c63 100644
--- a/src/kudu/sentry/CMakeLists.txt
+++ b/src/kudu/sentry/CMakeLists.txt
@@ -33,6 +33,7 @@ add_dependencies(sentry_thrift ${SENTRY_THRIFT_TGTS})
set(SENTRY_SRCS
sentry_action.cc
+ sentry_authorizable_scope.cc
sentry_client.cc)
set(SENTRY_DEPS
kudu_common
@@ -77,6 +78,7 @@ if (NOT NO_TESTS)
mini_sentry)
ADD_KUDU_TEST(sentry_action-test)
+ ADD_KUDU_TEST(sentry_authorizable_scope-test)
ADD_KUDU_TEST(sentry_client-test)
ADD_KUDU_TEST(thrift_operators-test)
endif()
diff --git a/src/kudu/sentry/sentry-test-base.h
b/src/kudu/sentry/sentry-test-base.h
index 9f6bae4..3a2ea71 100644
--- a/src/kudu/sentry/sentry-test-base.h
+++ b/src/kudu/sentry/sentry-test-base.h
@@ -32,8 +32,7 @@
namespace kudu {
namespace sentry {
-class SentryTestBase : public KuduTest,
- public ::testing::WithParamInterface<bool> {
+class SentryTestBase : public KuduTest {
public:
void SetUp() override {
@@ -44,7 +43,7 @@ class SentryTestBase : public KuduTest,
std::string host = GetBindIpForDaemon(1, kDefaultBindMode);
HostPort address(host, 0);
sentry_->SetAddress(address);
- if (kerberos_enabled_) {
+ if (KerberosEnabled()) {
kdc_.reset(new MiniKdc(MiniKdcOptions()));
ASSERT_OK(kdc_->Start());
@@ -80,8 +79,9 @@ class SentryTestBase : public KuduTest,
KuduTest::TearDown();
}
+ virtual bool KerberosEnabled() const = 0;
+
protected:
- const bool kerberos_enabled_ = GetParam();
std::unique_ptr<MiniKdc> kdc_;
std::unique_ptr<MiniSentry> sentry_;
std::unique_ptr<SentryClient> sentry_client_;
diff --git a/src/kudu/sentry/sentry_action.cc b/src/kudu/sentry/sentry_action.cc
index 5512423..f63ac0a 100644
--- a/src/kudu/sentry/sentry_action.cc
+++ b/src/kudu/sentry/sentry_action.cc
@@ -17,6 +17,8 @@
#include "kudu/sentry/sentry_action.h"
+#include <cstdint>
+
#include <ostream>
#include <string>
@@ -34,18 +36,19 @@ namespace sentry {
const char* ActionToString(SentryAction::Action action) {
switch (action) {
case SentryAction::Action::UNINITIALIZED: return "UNINITIALIZED";
- case SentryAction::Action::ALL: return "ALL";
- case SentryAction::Action::METADATA: return "METADATA";
- case SentryAction::Action::SELECT: return "SELECT";
- case SentryAction::Action::INSERT: return "INSERT";
- case SentryAction::Action::UPDATE: return "UPDATE";
- case SentryAction::Action::DELETE: return "DELETE";
- case SentryAction::Action::ALTER: return "ALTER";
- case SentryAction::Action::CREATE: return "CREATE";
- case SentryAction::Action::DROP: return "DROP";
- case SentryAction::Action::OWNER: return "OWNER";
+ case SentryAction::Action::ALL: return kActionAll;
+ case SentryAction::Action::METADATA: return kActionMetadata;
+ case SentryAction::Action::SELECT: return kActionSelect;
+ case SentryAction::Action::INSERT: return kActionInsert;
+ case SentryAction::Action::UPDATE: return kActionUpdate;
+ case SentryAction::Action::DELETE: return kActionDelete;
+ case SentryAction::Action::ALTER: return kActionAlter;
+ case SentryAction::Action::CREATE: return kActionCreate;
+ case SentryAction::Action::DROP: return kActionDrop;
+ case SentryAction::Action::OWNER: return kActionOwner;
}
- return "<cannot reach here>";
+ LOG(FATAL) << static_cast<uint16_t>(action) << ": unknown action";
+ return nullptr;
}
std::ostream& operator<<(std::ostream& o, SentryAction::Action action) {
@@ -67,25 +70,25 @@ Status SentryAction::FromString(const string& str,
SentryAction* action) {
// Java Sentry client.
//
// See
org.apache.sentry.api.service.thrift.SentryPolicyServiceClientDefaultImpl.
- if (boost::iequals(str, "ALL") || str == kWildCard) {
+ if (boost::iequals(str, kActionAll) || str == kWildCard) {
action->action_ = Action::ALL;
- } else if (boost::iequals(str, "METADATA")) {
+ } else if (boost::iequals(str, kActionMetadata)) {
action->action_ = Action::METADATA;
- } else if (boost::iequals(str, "SELECT")) {
+ } else if (boost::iequals(str, kActionSelect)) {
action->action_ = Action::SELECT;
- } else if (boost::iequals(str, "INSERT")) {
+ } else if (boost::iequals(str, kActionInsert)) {
action->action_ = Action::INSERT;
- } else if (boost::iequals(str, "UPDATE")) {
+ } else if (boost::iequals(str, kActionUpdate)) {
action->action_ = Action::UPDATE;
- } else if (boost::iequals(str, "DELETE")) {
+ } else if (boost::iequals(str, kActionDelete)) {
action->action_ = Action::DELETE;
- } else if (boost::iequals(str, "ALTER")) {
+ } else if (boost::iequals(str, kActionAlter)) {
action->action_ = Action::ALTER;
- } else if (boost::iequals(str, "CREATE")) {
+ } else if (boost::iequals(str, kActionCreate)) {
action->action_ = Action::CREATE;
- } else if (boost::iequals(str, "DROP")) {
+ } else if (boost::iequals(str, kActionDrop)) {
action->action_ = Action::DROP;
- } else if (boost::iequals(str, "OWNER")) {
+ } else if (boost::iequals(str, kActionOwner)) {
action->action_ = Action::OWNER;
} else {
return Status::InvalidArgument(Substitute("unknown SentryAction: $0",
str));
diff --git a/src/kudu/sentry/sentry_action.h b/src/kudu/sentry/sentry_action.h
index b12715c..d12613a 100644
--- a/src/kudu/sentry/sentry_action.h
+++ b/src/kudu/sentry/sentry_action.h
@@ -20,6 +20,7 @@
#include <iosfwd>
#include <string>
+#include "kudu/gutil/port.h"
#include "kudu/util/status.h"
namespace kudu {
@@ -33,7 +34,7 @@ namespace sentry {
// (e.g. create a table, drop a database).
// See org.apache.sentry.core.model.db.HivePrivilegeModel.
//
-// One action can imply another following rules defined in Imply().
+// One action can imply another following rules defined in Implies().
class SentryAction {
public:
static const char* const kWildCard;
@@ -60,6 +61,8 @@ class SentryAction {
OWNER,
};
+ // The default constructor is useful when creating an Action
+ // from string.
SentryAction();
explicit SentryAction(Action action);
@@ -69,7 +72,8 @@ class SentryAction {
}
// Create an Action from string.
- static Status FromString(const std::string& str, SentryAction* action);
+ static Status FromString(const std::string& str,
+ SentryAction* action) WARN_UNUSED_RESULT;
// Check if this action implies 'other'. In general,
// 1. an action only implies itself.
@@ -83,6 +87,17 @@ class SentryAction {
Action action_;
};
+static constexpr const char* const kActionAll = "ALL";
+static constexpr const char* const kActionMetadata = "METADATA";
+static constexpr const char* const kActionSelect = "SELECT";
+static constexpr const char* const kActionInsert = "INSERT";
+static constexpr const char* const kActionUpdate = "UPDATE";
+static constexpr const char* const kActionDelete = "DELETE";
+static constexpr const char* const kActionAlter = "ALTER";
+static constexpr const char* const kActionCreate = "CREATE";
+static constexpr const char* const kActionDrop = "DROP";
+static constexpr const char* const kActionOwner = "OWNER";
+
const char* ActionToString(SentryAction::Action action);
std::ostream& operator<<(std::ostream& o, SentryAction::Action action);
diff --git a/src/kudu/sentry/sentry_authorizable_scope-test.cc
b/src/kudu/sentry/sentry_authorizable_scope-test.cc
new file mode 100644
index 0000000..72bd9c7
--- /dev/null
+++ b/src/kudu/sentry/sentry_authorizable_scope-test.cc
@@ -0,0 +1,83 @@
+// 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.
+
+#include "kudu/sentry/sentry_authorizable_scope.h"
+
+#include <string>
+#include <utility>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include "kudu/util/status.h"
+#include "kudu/util/test_macros.h"
+
+using std::pair;
+using std::string;
+using std::vector;
+
+namespace kudu {
+
+namespace sentry {
+
+TEST(SentryAuthorizableScopeTest, TestImplyScope) {
+ SentryAuthorizableScope server(SentryAuthorizableScope::Scope::SERVER);
+ SentryAuthorizableScope database(SentryAuthorizableScope::Scope::DATABASE);
+ SentryAuthorizableScope table(SentryAuthorizableScope::Scope::TABLE);
+ SentryAuthorizableScope column(SentryAuthorizableScope::Scope::COLUMN);
+
+ vector<SentryAuthorizableScope> scopes({ column, table, database, server });
+ vector<SentryAuthorizableScope> lower_scopes;
+ vector<SentryAuthorizableScope> higher_scopes({ server, database, table,
column });
+
+ // A higher scope in the hierarchy can imply a lower scope in the hierarchy,
+ // not vice versa.
+ for (const auto& scope : scopes) {
+ lower_scopes.emplace_back(scope);
+ higher_scopes.pop_back();
+ for (const auto &lower_scope : lower_scopes) {
+ ASSERT_TRUE(scope.Implies(lower_scope));
+ }
+ for (const auto &higher_scope : higher_scopes) {
+ ASSERT_FALSE(scope.Implies(higher_scope));
+ }
+ }
+}
+
+TEST(SentryAuthorizableScopeTest, TestFromString) {
+ const vector<pair<string, SentryAuthorizableScope::Scope>> valid_scopes = {
+ {"server", SentryAuthorizableScope::Scope::SERVER},
+ {"database", SentryAuthorizableScope::Scope::DATABASE},
+ {"table", SentryAuthorizableScope::Scope::TABLE},
+ {"column", SentryAuthorizableScope::Scope::COLUMN}
+ };
+ SentryAuthorizableScope scope;
+ for (const auto& valid_scope : valid_scopes) {
+ ASSERT_OK(SentryAuthorizableScope::FromString(valid_scope.first, &scope));
+ ASSERT_EQ(valid_scope.second, scope.scope());
+ }
+
+ // Unsupported scope returns invalid argument error.
+ const vector<string> invalid_scopes({ "UNINITIALIZED", "tablecolumn" });
+ for (const auto& invalid_scope : invalid_scopes) {
+ Status s = SentryAuthorizableScope::FromString(invalid_scope, &scope);
+ ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
+ }
+}
+
+} // namespace sentry
+} // namespace kudu
diff --git a/src/kudu/sentry/sentry_authorizable_scope.cc
b/src/kudu/sentry/sentry_authorizable_scope.cc
new file mode 100644
index 0000000..deac923
--- /dev/null
+++ b/src/kudu/sentry/sentry_authorizable_scope.cc
@@ -0,0 +1,101 @@
+// 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.
+
+#include "kudu/sentry/sentry_authorizable_scope.h"
+
+#include <cstdint>
+
+#include <ostream>
+#include <string>
+
+#include <boost/algorithm/string/predicate.hpp>
+#include <glog/logging.h>
+
+#include "kudu/gutil/strings/substitute.h"
+
+using std::string;
+using strings::Substitute;
+
+namespace kudu {
+namespace sentry {
+
+const char* ScopeToString(SentryAuthorizableScope::Scope scope) {
+ switch (scope) {
+ case SentryAuthorizableScope::Scope::UNINITIALIZED: return "UNINITIALIZED";
+ case SentryAuthorizableScope::Scope::SERVER: return kSever;
+ case SentryAuthorizableScope::Scope::DATABASE: return kDatabase;
+ case SentryAuthorizableScope::Scope::TABLE: return kTable;
+ case SentryAuthorizableScope::Scope::COLUMN: return kColumn;
+ }
+ LOG(FATAL) << static_cast<uint16_t>(scope) << ": unknown scope";
+ return nullptr;
+}
+
+std::ostream& operator<<(std::ostream& o, SentryAuthorizableScope::Scope
scope) {
+ return o << ScopeToString(scope);
+}
+
+SentryAuthorizableScope::SentryAuthorizableScope()
+ : scope_(Scope::UNINITIALIZED) {
+}
+
+SentryAuthorizableScope::SentryAuthorizableScope(Scope scope)
+ : scope_(scope) {
+}
+
+Status SentryAuthorizableScope::FromString(const string& str,
+ SentryAuthorizableScope* scope) {
+ if (boost::iequals(str, kSever)) {
+ scope->scope_ = Scope::SERVER;
+ } else if (boost::iequals(str, kDatabase)) {
+ scope->scope_ = Scope::DATABASE;
+ } else if (boost::iequals(str, kTable)) {
+ scope->scope_ = Scope::TABLE;
+ } else if (boost::iequals(str, kColumn)) {
+ scope->scope_ = Scope::COLUMN;
+ } else {
+ return Status::InvalidArgument(Substitute("unknown
SentryAuthorizableScope: $0", str));
+ }
+
+ return Status::OK();
+}
+
+bool SentryAuthorizableScope::Implies(const SentryAuthorizableScope& other)
const {
+ switch (scope_) {
+ case Scope::COLUMN:
+ return other.scope_ == Scope::COLUMN;
+ case Scope::TABLE:
+ return other.scope_ == Scope::TABLE ||
+ other.scope_ == Scope::COLUMN;
+ case Scope::DATABASE:
+ return other.scope_ == Scope::DATABASE ||
+ other.scope_ == Scope::TABLE ||
+ other.scope_ == Scope::COLUMN;
+ case Scope::SERVER:
+ return other.scope_ == Scope::SERVER ||
+ other.scope_ == Scope::DATABASE ||
+ other.scope_ == Scope::TABLE ||
+ other.scope_ == Scope::COLUMN;
+ default:
+ LOG(FATAL) << "unsupported SentryAuthorizableScope";
+ break;
+ }
+ return false;
+}
+
+} // namespace sentry
+} // namespace kudu
diff --git a/src/kudu/sentry/sentry_authorizable_scope.h
b/src/kudu/sentry/sentry_authorizable_scope.h
new file mode 100644
index 0000000..57ee201
--- /dev/null
+++ b/src/kudu/sentry/sentry_authorizable_scope.h
@@ -0,0 +1,83 @@
+// 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.
+
+#pragma once
+
+#include <iosfwd>
+#include <string>
+
+#include "kudu/gutil/port.h"
+#include "kudu/util/status.h"
+
+namespace kudu {
+namespace sentry {
+
+// A carbon copy of Sentry authorizable scope, which indicates the
+// hierarchy of authorizables (server → database → table → column).
+// For example, authorizable 'server=server1->db=a' has database
+// level scope, while authorizable 'server=server1' has server level
+// scope.
+//
+// A privilege scope can imply another following rules defined in
+// Implies().
+class SentryAuthorizableScope {
+ public:
+
+ // Note that 'UNINITIALIZED' is not an actual scope but
+ // only to represent the uninitialized state.
+ enum class Scope {
+ UNINITIALIZED,
+ SERVER,
+ DATABASE,
+ TABLE,
+ COLUMN,
+ };
+
+ // The default constructor is useful when creating an authorizable scope
+ // from string.
+ SentryAuthorizableScope();
+
+ explicit SentryAuthorizableScope(Scope scope);
+
+ Scope scope() const {
+ return scope_;
+ }
+
+ // Create an authorizable scope from string.
+ static Status FromString(const std::string& str,
+ SentryAuthorizableScope* scope) WARN_UNUSED_RESULT;
+
+ // Returns true if one authorizable scope can imply another. In general,
+ // 1. an authorizable scope implies itself.
+ // 2. a higher scope in the hierarchy implies a lower scope in the
hierarchy.
+ bool Implies(const SentryAuthorizableScope& other) const;
+
+ private:
+ Scope scope_;
+};
+
+static constexpr const char* const kSever = "SERVER";
+static constexpr const char* const kDatabase = "DATABASE";
+static constexpr const char* const kTable = "TABLE";
+static constexpr const char* const kColumn = "COLUMN";
+
+const char* ScopeToString(SentryAuthorizableScope::Scope scope);
+
+std::ostream& operator<<(std::ostream& o, SentryAuthorizableScope::Scope
scope);
+
+} // namespace sentry
+} // namespace kudu
diff --git a/src/kudu/sentry/sentry_client-test.cc
b/src/kudu/sentry/sentry_client-test.cc
index 58af6fc..67e821b 100644
--- a/src/kudu/sentry/sentry_client-test.cc
+++ b/src/kudu/sentry/sentry_client-test.cc
@@ -50,15 +50,21 @@ using std::vector;
namespace kudu {
namespace sentry {
-class SentryClientTest : public SentryTestBase {
+class SentryClientTest : public SentryTestBase,
+ public ::testing::WithParamInterface<bool> {
+ public:
+ bool KerberosEnabled() const {
+ return GetParam();
+ }
};
+
INSTANTIATE_TEST_CASE_P(KerberosEnabled, SentryClientTest, ::testing::Bool());
TEST_P(SentryClientTest, TestMiniSentryLifecycle) {
// Create an HA Sentry client and ensure it automatically reconnects after
service interruption.
thrift::HaClient<SentryClient> client;
thrift::ClientOptions sentry_client_opts;
- if (kerberos_enabled_) {
+ if (KerberosEnabled()) {
sentry_client_opts.enable_kerberos = true;
sentry_client_opts.service_principal = "sentry";
}
diff --git a/src/kudu/sentry/sentry_client.h b/src/kudu/sentry/sentry_client.h
index 27cc266..c57f907 100644
--- a/src/kudu/sentry/sentry_client.h
+++ b/src/kudu/sentry/sentry_client.h
@@ -88,7 +88,16 @@ class SentryClient {
// Drops a role in Sentry.
Status DropRole(const ::sentry::TDropSentryRoleRequest& request)
WARN_UNUSED_RESULT;
- // List Sentry privileges by user.
+ // Given an authorizable, list Sentry privileges granted to the user that
match
+ // the authorizable in each privilege scope on the hierarchy (regardless of
the
+ // action).
+ //
+ // For example, for authorizable 'server=server1->db=db1', it returns any
privileges
+ // granted to the user that matches:
+ // server == "server1" && (db == "db1" || db == NULL)
+ //
+ // If the user is granted both 'ALL on SERVER server1' and 'SELECT on TABLE
db1.a'
+ // privileges, then both privileges are returned.
Status ListPrivilegesByUser(const ::sentry::TListSentryPrivilegesRequest&
request,
::sentry::TListSentryPrivilegesResponse* response) WARN_UNUSED_RESULT;
