This is an automated email from the ASF dual-hosted git repository. adar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 424f1dcf41ca121406b46507356db7ca44ffd2a5 Author: Andrew Wong <[email protected]> AuthorDate: Wed Apr 3 17:19:39 2019 -0700 sentry: populate TablePrivilegePBs This patch allows the authz provider to populate TablePrivilegePBs, which will be used as authorization metadata for authz tokens. A follow-up patch will integrate this functionality into the GetTableSchema endpoint. Change-Id: Icb6c533a04d2749e0ee68546dac61011432cba2f Reviewed-on: http://gerrit.cloudera.org:8080/12941 Reviewed-by: Adar Dembo <[email protected]> Tested-by: Kudu Jenkins Reviewed-by: Hao Hao <[email protected]> --- src/kudu/master/authz_provider.h | 16 +++ src/kudu/master/default_authz_provider.h | 19 +++ src/kudu/master/sentry_authz_provider-test.cc | 167 ++++++++++++++++++++++---- src/kudu/master/sentry_authz_provider.cc | 77 ++++++++++++ src/kudu/master/sentry_authz_provider.h | 17 ++- 5 files changed, 271 insertions(+), 25 deletions(-) diff --git a/src/kudu/master/authz_provider.h b/src/kudu/master/authz_provider.h index d45f03f..4231fde 100644 --- a/src/kudu/master/authz_provider.h +++ b/src/kudu/master/authz_provider.h @@ -24,6 +24,13 @@ #include "kudu/util/status.h" namespace kudu { + +class SchemaPB; + +namespace security { +class TablePrivilegePB; +} // namespace security + namespace master { // An interface for handling authorizations on Kudu operations. @@ -71,6 +78,15 @@ class AuthzProvider { virtual Status AuthorizeGetTableMetadata(const std::string& table_name, const std::string& user) WARN_UNUSED_RESULT = 0; + // Populates the privilege fields of 'pb' with the table-specific privileges + // for the given user, using 'schema_pb' for metadata (e.g. column IDs). This + // does not populate the table ID field of 'pb' -- only the privilege fields; + // as such, it is expected that the table ID field is already set. + virtual Status FillTablePrivilegePB(const std::string& table_name, + const std::string& user, + const SchemaPB& schema_pb, + security::TablePrivilegePB* pb) WARN_UNUSED_RESULT = 0; + virtual ~AuthzProvider() {} // Checks if the given user is trusted and thus can be exempted from diff --git a/src/kudu/master/default_authz_provider.h b/src/kudu/master/default_authz_provider.h index 6c512f6..516dab1 100644 --- a/src/kudu/master/default_authz_provider.h +++ b/src/kudu/master/default_authz_provider.h @@ -19,10 +19,16 @@ #include <string> +#include <glog/logging.h> + #include "kudu/master/authz_provider.h" +#include "kudu/security/token.pb.h" #include "kudu/util/status.h" namespace kudu { + +class SchemaPB; + namespace master { // Default AuthzProvider which always authorizes any operations. @@ -54,6 +60,19 @@ class DefaultAuthzProvider : public AuthzProvider { const std::string& /*user*/) override WARN_UNUSED_RESULT { return Status::OK(); } + + Status FillTablePrivilegePB(const std::string& /*table_name*/, + const std::string& /*user*/, + const SchemaPB& /*schema_pb*/, + security::TablePrivilegePB* pb) override WARN_UNUSED_RESULT { + DCHECK(pb); + DCHECK(pb->has_table_id()); + pb->set_delete_privilege(true); + pb->set_insert_privilege(true); + pb->set_scan_privilege(true); + pb->set_update_privilege(true); + return Status::OK(); + } }; } // namespace master diff --git a/src/kudu/master/sentry_authz_provider-test.cc b/src/kudu/master/sentry_authz_provider-test.cc index bb2df0a..853998d 100644 --- a/src/kudu/master/sentry_authz_provider-test.cc +++ b/src/kudu/master/sentry_authz_provider-test.cc @@ -21,17 +21,25 @@ #include <memory> #include <ostream> #include <string> +#include <unordered_map> +#include <unordered_set> #include <vector> #include <gflags/gflags_declare.h> #include <glog/logging.h> +#include <google/protobuf/stubs/port.h> +#include <google/protobuf/util/message_differencer.h> #include <gtest/gtest.h> +#include "kudu/common/common.pb.h" +#include "kudu/common/schema.h" +#include "kudu/common/wire_protocol.h" #include "kudu/gutil/macros.h" #include "kudu/gutil/map-util.h" #include "kudu/gutil/ref_counted.h" #include "kudu/gutil/strings/substitute.h" #include "kudu/master/sentry_authz_provider-test-base.h" +#include "kudu/security/token.pb.h" #include "kudu/sentry/mini_sentry.h" #include "kudu/sentry/sentry-test-base.h" #include "kudu/sentry/sentry_action.h" @@ -41,6 +49,7 @@ #include "kudu/util/hdr_histogram.h" #include "kudu/util/metrics.h" #include "kudu/util/net/net_util.h" +#include "kudu/util/pb_util.h" #include "kudu/util/random.h" #include "kudu/util/random_util.h" #include "kudu/util/status.h" @@ -60,17 +69,22 @@ METRIC_DECLARE_counter(sentry_client_reconnections_succeeded); METRIC_DECLARE_counter(sentry_client_reconnections_failed); METRIC_DECLARE_histogram(sentry_client_task_execution_time_us); +using kudu::pb_util::SecureDebugString; +using kudu::security::ColumnPrivilegePB; +using kudu::security::TablePrivilegePB; using kudu::sentry::AuthorizableScopesSet; using kudu::sentry::SentryAction; using kudu::sentry::SentryActionsSet; using kudu::sentry::SentryTestBase; using kudu::sentry::SentryAuthorizableScope; +using google::protobuf::util::MessageDifferencer; using sentry::TSentryAuthorizable; using sentry::TSentryGrantOption; using sentry::TSentryPrivilege; using std::string; -using std::tuple; using std::unique_ptr; +using std::unordered_map; +using std::unordered_set; using std::vector; using strings::Substitute; @@ -206,6 +220,23 @@ class SentryAuthzProviderTest : public SentryTestBase { namespace { +const SentryActionsSet kAllActions({ + SentryAction::ALL, + SentryAction::METADATA, + SentryAction::SELECT, + SentryAction::INSERT, + SentryAction::UPDATE, + SentryAction::DELETE, + SentryAction::ALTER, + SentryAction::CREATE, + SentryAction::DROP, + SentryAction::OWNER, +}); + +} // anonymous namespace + +namespace { + // Indicates different invalid privilege response types to be injected. enum class InvalidPrivilege { // No error is injected. @@ -233,30 +264,15 @@ enum class InvalidPrivilege { FLIPPED_FIELD, }; -const SentryActionsSet kAllActions({ - SentryAction::ALL, - SentryAction::METADATA, - SentryAction::SELECT, - SentryAction::INSERT, - SentryAction::UPDATE, - SentryAction::DELETE, - SentryAction::ALTER, - SentryAction::CREATE, - SentryAction::DROP, - SentryAction::OWNER, -}); - constexpr const char* kDb = "db"; constexpr const char* kTable = "table"; constexpr const char* kColumn = "column"; } // anonymous namespace -class SentryAuthzProviderFilterResponsesTest : - public SentryAuthzProviderTest, - public ::testing::WithParamInterface<SentryAuthorizableScope::Scope> { +class SentryAuthzProviderFilterPrivilegesTest : public SentryAuthzProviderTest { public: - SentryAuthzProviderFilterResponsesTest() + SentryAuthzProviderFilterPrivilegesTest() : prng_(SeedRandom()) {} void SetUp() override { @@ -277,7 +293,7 @@ class SentryAuthzProviderFilterResponsesTest : const SentryAuthorizableScope& scope, const SentryAction& action, InvalidPrivilege invalid_privilege = InvalidPrivilege::NONE) { DCHECK(!full_authorizable.server.empty() && !full_authorizable.db.empty() && - !full_authorizable.table.empty() && !full_authorizable.column.empty()); + !full_authorizable.table.empty() && !full_authorizable.column.empty()); TSentryPrivilege privilege; privilege.__set_action(invalid_privilege == InvalidPrivilege::INCORRECT_ACTION ? "foobar" : ActionToString(action.action())); @@ -342,10 +358,117 @@ class SentryAuthzProviderFilterResponsesTest : mutable Random prng_; }; +TEST_F(SentryAuthzProviderFilterPrivilegesTest, TestTablePrivilegePBParsing) { + constexpr int kNumColumns = 10; + SchemaBuilder schema_builder; + schema_builder.AddKeyColumn("col0", DataType::INT32); + vector<string> column_names = { "col0" }; + for (int i = 1; i < kNumColumns; i++) { + const string col = Substitute("col$0", i); + schema_builder.AddColumn(ColumnSchema(col, DataType::INT32), + /*is_key=*/false); + column_names.emplace_back(col); + } + SchemaPB schema_pb; + ASSERT_OK(SchemaToPB(schema_builder.Build(), &schema_pb)); + unordered_map<string, ColumnId> col_name_to_id; + for (const auto& col_pb : schema_pb.columns()) { + EmplaceOrDie(&col_name_to_id, col_pb.name(), ColumnId(col_pb.id())); + } + + // First, grant some privileges at the table authorizable scope or higher. + Random prng(SeedRandom()); + vector<SentryAuthorizableScope::Scope> scope_to_grant_that_implies_table = + SelectRandomSubset<vector<SentryAuthorizableScope::Scope>, + SentryAuthorizableScope::Scope, Random>({ SentryAuthorizableScope::SERVER, + SentryAuthorizableScope::DATABASE, + SentryAuthorizableScope::TABLE }, 0, &prng); + unordered_map<SentryAuthorizableScope::Scope, SentryActionsSet, std::hash<int>> + granted_privileges; + SentryActionsSet table_privileges; + TSentryAuthorizable table_authorizable; + table_authorizable.__set_server(FLAGS_server_name); + table_authorizable.__set_db(kDb); + table_authorizable.__set_table(kTable); + table_authorizable.__set_column(column_names[0]); + for (const auto& granted_scope : scope_to_grant_that_implies_table) { + for (const auto& action : SelectRandomSubset<SentryActionsSet, SentryAction::Action, Random>( + kAllActions, 0, &prng)) { + // Grant the privilege to the user. + TSentryPrivilege table_privilege = CreatePrivilege(table_authorizable, + SentryAuthorizableScope(granted_scope), SentryAction(action)); + ASSERT_OK(AlterRoleGrantPrivilege(sentry_client_.get(), kRoleName, table_privilege)); + + // All of the privileges imply the table-level action. + InsertIfNotPresent(&table_privileges, action); + } + } + + // Grant some privileges at the column scope. + vector<string> columns_to_grant = + SelectRandomSubset<vector<string>, string, Random>(column_names, 0, &prng); + unordered_set<ColumnId> scannable_columns; + for (const auto& column_name : columns_to_grant) { + for (const auto& action : SelectRandomSubset<SentryActionsSet, SentryAction::Action, Random>( + kAllActions, 0, &prng)) { + // Grant the privilege to the user. + TSentryPrivilege column_privilege = + GetColumnPrivilege(kDb, kTable, column_name, ActionToString(action)); + ASSERT_OK(AlterRoleGrantPrivilege(sentry_client_.get(), kRoleName, column_privilege)); + + if (SentryAction(action).Implies(SentryAction(SentryAction::SELECT))) { + InsertIfNotPresent(&scannable_columns, FindOrDie(col_name_to_id, column_name)); + } + } + } + + // Make sure that any implied privileges make their way to the token. + const string kTableId = "table-id"; + TablePrivilegePB expected_pb; + expected_pb.set_table_id(kTableId); + for (const auto& granted_table_action : table_privileges) { + if (SentryAction(granted_table_action).Implies(SentryAction(SentryAction::INSERT))) { + expected_pb.set_insert_privilege(true); + } + if (SentryAction(granted_table_action).Implies(SentryAction(SentryAction::UPDATE))) { + expected_pb.set_update_privilege(true); + } + if (SentryAction(granted_table_action).Implies(SentryAction(SentryAction::DELETE))) { + expected_pb.set_delete_privilege(true); + } + if (SentryAction(granted_table_action).Implies(SentryAction(SentryAction::SELECT))) { + expected_pb.set_scan_privilege(true); + } + } + + // If any of the table-level privileges imply privileges on scan, we + // shouldn't expect per-column scan privileges. Otherwise, we should expect + // the columns privileges that implied SELECT to have scan privileges. + if (!expected_pb.scan_privilege()) { + ColumnPrivilegePB scan_col_privilege; + scan_col_privilege.set_scan_privilege(true); + for (const auto& id : scannable_columns) { + InsertIfNotPresent(expected_pb.mutable_column_privileges(), id, scan_col_privilege); + } + } + // Validate the privileges went through. + TablePrivilegePB privilege_pb; + privilege_pb.set_table_id(kTableId); + ASSERT_OK(sentry_authz_provider_->FillTablePrivilegePB(Substitute("$0.$1", kDb, kTable), + kTestUser, schema_pb, &privilege_pb)); + ASSERT_TRUE(MessageDifferencer::Equals(expected_pb, privilege_pb)) + << Substitute("$0 vs $1", SecureDebugString(expected_pb), SecureDebugString(privilege_pb)); +} + +// Parameterized on the scope at which the privilege will be granted. +class SentryAuthzProviderFilterPrivilegesScopeTest : + public SentryAuthzProviderFilterPrivilegesTest, + public ::testing::WithParamInterface<SentryAuthorizableScope::Scope> {}; + // Attempst to grant privileges for various actions on a single scope of an // authorizable, injecting various invalid privileges, and checking that Kudu // ignores them. -TEST_P(SentryAuthzProviderFilterResponsesTest, TestFilterInvalidResponses) { +TEST_P(SentryAuthzProviderFilterPrivilegesScopeTest, TestFilterInvalidResponses) { const string& table_ident = Substitute("$0.$1", full_authorizable_.db, full_authorizable_.table); static constexpr InvalidPrivilege kInvalidPrivileges[] = { InvalidPrivilege::INCORRECT_ACTION, @@ -373,7 +496,7 @@ TEST_P(SentryAuthzProviderFilterResponsesTest, TestFilterInvalidResponses) { } // Grants privileges for various actions on a single scope of an authorizable. -TEST_P(SentryAuthzProviderFilterResponsesTest, TestFilterValidResponses) { +TEST_P(SentryAuthzProviderFilterPrivilegesScopeTest, TestFilterValidResponses) { const string& table_ident = Substitute("$0.$1", full_authorizable_.db, full_authorizable_.table); SentryAuthorizableScope granted_scope(GetParam()); // Send valid requests and verify that we can get it back through the @@ -397,7 +520,7 @@ TEST_P(SentryAuthzProviderFilterResponsesTest, TestFilterValidResponses) { } } -INSTANTIATE_TEST_CASE_P(GrantedScopes, SentryAuthzProviderFilterResponsesTest, +INSTANTIATE_TEST_CASE_P(GrantedScopes, SentryAuthzProviderFilterPrivilegesScopeTest, ::testing::Values(SentryAuthorizableScope::SERVER, SentryAuthorizableScope::DATABASE, SentryAuthorizableScope::TABLE, diff --git a/src/kudu/master/sentry_authz_provider.cc b/src/kudu/master/sentry_authz_provider.cc index 187661a..dfb6895 100644 --- a/src/kudu/master/sentry_authz_provider.cc +++ b/src/kudu/master/sentry_authz_provider.cc @@ -21,6 +21,7 @@ #include <ostream> #include <type_traits> #include <unordered_map> +#include <unordered_set> #include <utility> #include <vector> @@ -28,11 +29,13 @@ #include <gflags/gflags.h> #include <glog/logging.h> +#include "kudu/common/common.pb.h" #include "kudu/common/table_util.h" #include "kudu/gutil/macros.h" #include "kudu/gutil/map-util.h" #include "kudu/gutil/strings/substitute.h" #include "kudu/master/sentry_client_metrics.h" +#include "kudu/security/token.pb.h" #include "kudu/sentry/sentry_action.h" #include "kudu/sentry/sentry_client.h" #include "kudu/sentry/sentry_policy_service_types.h" @@ -50,6 +53,7 @@ using sentry::TSentryGrantOption; using sentry::TSentryPrivilege; using std::string; using std::unordered_map; +using std::unordered_set; using std::vector; DEFINE_string(sentry_service_rpc_addresses, "", @@ -123,6 +127,8 @@ using strings::Substitute; namespace kudu { +using security::ColumnPrivilegePB; +using security::TablePrivilegePB; using sentry::SentryAction; using sentry::SentryAuthorizableScope; using sentry::AuthorizableScopesSet; @@ -541,6 +547,77 @@ Status SentryAuthzProvider::GetSentryPrivileges(SentryAuthorizableScope::Scope s return Status::OK(); } +Status SentryAuthzProvider::FillTablePrivilegePB(const string& table_name, + const string& user, + const SchemaPB& schema_pb, + TablePrivilegePB* pb) { + DCHECK(pb); + DCHECK(pb->has_table_id()); + if (AuthzProvider::IsTrustedUser(user)) { + pb->set_delete_privilege(true); + pb->set_insert_privilege(true); + pb->set_scan_privilege(true); + pb->set_update_privilege(true); + return Status::OK(); + } + static ColumnPrivilegePB scan_col_privilege; + scan_col_privilege.set_scan_privilege(true); + + // Note: it might seem like we could cache these TablePrivilegePBs rather + // than parsing them from Sentry privileges every time. This is tricky + // because the column-level privileges depend on the input schema, which may + // be different upon subsequent calls to this function. + SentryPrivilegesBranch privileges; + RETURN_NOT_OK(GetSentryPrivileges(SentryAuthorizableScope::TABLE, table_name, + user, &privileges)); + unordered_set<string> scannable_col_names; + static const SentryAuthorizableScope kTableScope(SentryAuthorizableScope::TABLE); + for (const auto& privilege : privileges.privileges) { + if (SentryAuthorizableScope(privilege.scope).Implies(kTableScope)) { + // Pull out any privileges at the table scope or higher. + if (ContainsKey(privilege.granted_privileges, SentryAction::ALL) || + ContainsKey(privilege.granted_privileges, SentryAction::OWNER)) { + // Generate privilege with everything. + pb->set_delete_privilege(true); + pb->set_insert_privilege(true); + pb->set_scan_privilege(true); + pb->set_update_privilege(true); + return Status::OK(); + } + if (ContainsKey(privilege.granted_privileges, SentryAction::DELETE)) { + pb->set_delete_privilege(true); + } + if (ContainsKey(privilege.granted_privileges, SentryAction::INSERT)) { + pb->set_insert_privilege(true); + } + if (ContainsKey(privilege.granted_privileges, SentryAction::SELECT)) { + pb->set_scan_privilege(true); + } + if (ContainsKey(privilege.granted_privileges, SentryAction::UPDATE)) { + pb->set_update_privilege(true); + } + } else if (!pb->scan_privilege() && + (ContainsKey(privilege.granted_privileges, SentryAction::ALL) || + ContainsKey(privilege.granted_privileges, SentryAction::OWNER) || + ContainsKey(privilege.granted_privileges, SentryAction::SELECT))) { + // Pull out any scan privileges at the column scope. + DCHECK_EQ(SentryAuthorizableScope::COLUMN, privilege.scope); + DCHECK(!privilege.column_name.empty()); + EmplaceIfNotPresent(&scannable_col_names, privilege.column_name); + } + } + // If we got any column-level scan privileges and we don't already have + // table-level scan privileges, set them now. + if (!pb->scan_privilege()) { + for (const auto& col : schema_pb.columns()) { + if (ContainsKey(scannable_col_names, col.name())) { + InsertIfNotPresent(pb->mutable_column_privileges(), col.id(), scan_col_privilege); + } + } + } + return Status::OK(); +} + Status SentryAuthzProvider::Authorize(SentryAuthorizableScope::Scope scope, SentryAction::Action action, const string& table_ident, diff --git a/src/kudu/master/sentry_authz_provider.h b/src/kudu/master/sentry_authz_provider.h index 4be72b3..1a1efa2 100644 --- a/src/kudu/master/sentry_authz_provider.h +++ b/src/kudu/master/sentry_authz_provider.h @@ -43,6 +43,12 @@ class TSentryPrivilege; namespace kudu { +class SchemaPB; + +namespace security { +class TablePrivilegePB; +} // namespace security + namespace master { // Utility struct to facilitate evaluating the privileges of a given @@ -150,12 +156,17 @@ class SentryAuthzProvider : public AuthzProvider { Status AuthorizeGetTableMetadata(const std::string& table_name, const std::string& user) override WARN_UNUSED_RESULT; + Status FillTablePrivilegePB(const std::string& table_name, + const std::string& user, + const SchemaPB& schema_pb, + security::TablePrivilegePB* pb) override WARN_UNUSED_RESULT; + private: - friend class SentryAuthzProviderFilterResponsesTest; + friend class SentryAuthzProviderFilterPrivilegesTest; FRIEND_TEST(SentryAuthzProviderStaticTest, TestPrivilegesWellFormed); FRIEND_TEST(TestAuthzHierarchy, TestAuthorizableScope); - FRIEND_TEST(SentryAuthzProviderFilterResponsesTest, TestFilterInvalidResponses); - FRIEND_TEST(SentryAuthzProviderFilterResponsesTest, TestFilterValidResponses); + FRIEND_TEST(SentryAuthzProviderFilterPrivilegesScopeTest, TestFilterInvalidResponses); + FRIEND_TEST(SentryAuthzProviderFilterPrivilegesScopeTest, TestFilterValidResponses); // Utility function to determine whether the given privilege is a well-formed // possibly Kudu-related privilege describing a descendent or ancestor of the
