kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From granthe...@apache.org
Subject [kudu] 01/02: [ranger] fix incorrect authz enforcement in Ranger authz provider
Date Mon, 23 Mar 2020 14:11:07 GMT
This is an automated email from the ASF dual-hosted git repository.

granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 84d23679905f6953a976f2baedcc1cb18c641cfc
Author: Hao Hao <hao.hao@cloudera.com>
AuthorDate: Fri Mar 13 23:44:27 2020 -0700

    [ranger] fix incorrect authz enforcement in Ranger authz provider
    
    Previous commit 0d29977 introduced Ranger authorization provider,
    however incorrect authz enforcement is used against table creation
    and table rename (CREATE on TABLE). This patch fixes it via checking
    (CREATE on DATABASE) privilege.
    
    Change-Id: I267aabc5f224ee7ceeffd6187785595dd6f16487
    Reviewed-on: http://gerrit.cloudera.org:8080/15436
    Tested-by: Kudu Jenkins
    Reviewed-by: Andrew Wong <awong@cloudera.com>
---
 src/kudu/master/ranger_authz_provider.cc | 17 ++++++++++++-----
 src/kudu/master/ranger_authz_provider.h  |  8 ++++++++
 src/kudu/ranger/ranger_client.cc         | 23 +++++++++++++++++++----
 src/kudu/ranger/ranger_client.h          | 10 +++++++++-
 4 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/src/kudu/master/ranger_authz_provider.cc b/src/kudu/master/ranger_authz_provider.cc
index 5d92f7a..0944284 100644
--- a/src/kudu/master/ranger_authz_provider.cc
+++ b/src/kudu/master/ranger_authz_provider.cc
@@ -39,6 +39,7 @@ using kudu::security::ColumnPrivilegePB;
 using kudu::security::TablePrivilegePB;
 using kudu::ranger::ActionPB;
 using kudu::ranger::ActionHash;
+using kudu::ranger::RangerClient;
 using std::string;
 using std::unordered_set;
 
@@ -57,7 +58,9 @@ Status RangerAuthzProvider::AuthorizeCreateTable(const string& table_name,
   if (IsTrustedUser(user)) {
     return Status::OK();
   }
-  return client_.AuthorizeAction(user, ActionPB::CREATE, table_name);
+  // Table creation requires 'CREATE ON DATABASE' privilege.
+  return client_.AuthorizeAction(user, ActionPB::CREATE, table_name,
+                                 RangerClient::Scope::DATABASE);
 }
 
 Status RangerAuthzProvider::AuthorizeDropTable(const string& table_name,
@@ -65,6 +68,7 @@ Status RangerAuthzProvider::AuthorizeDropTable(const string& table_name,
   if (IsTrustedUser(user)) {
     return Status::OK();
   }
+  // Table deletion requires 'DROP ON TABLE' privilege.
   return client_.AuthorizeAction(user, ActionPB::DROP, table_name);
 }
 
@@ -74,15 +78,16 @@ Status RangerAuthzProvider::AuthorizeAlterTable(const string& old_table,
   if (IsTrustedUser(user)) {
     return Status::OK();
   }
-  // Table alteration requires ALTER ON TABLE if no rename is done. To prevent
-  // privilege escalation we require ALL on the old table and CREATE on the new
-  // table (database).
+  // Table alteration (without table rename) requires ALTER ON TABLE.
   if (old_table == new_table) {
     return client_.AuthorizeAction(user, ActionPB::ALTER, old_table);
   }
 
+  // To prevent privilege escalation we require ALL on the old TABLE
+  // and CREATE on the new DATABASE for table rename.
   RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::ALL, old_table));
-  return client_.AuthorizeAction(user, ActionPB::CREATE, new_table);
+  return client_.AuthorizeAction(user, ActionPB::CREATE, new_table,
+                                 RangerClient::Scope::DATABASE);
 }
 
 Status RangerAuthzProvider::AuthorizeGetTableMetadata(const string& table_name,
@@ -90,6 +95,7 @@ Status RangerAuthzProvider::AuthorizeGetTableMetadata(const string&
table_name,
   if (IsTrustedUser(user)) {
     return Status::OK();
   }
+  // Get table metadata requires 'METADATA ON TABLE' privilege.
   return client_.AuthorizeAction(user, ActionPB::METADATA, table_name);
 }
 
@@ -102,6 +108,7 @@ Status RangerAuthzProvider::AuthorizeListTables(const string& user,
   }
 
   *checked_table_names = true;
+  // List tables requires 'METADATA ON TABLE' privilege on all tables being listed.
   return client_.AuthorizeActionMultipleTables(user, ActionPB::METADATA, table_names);
 }
 
diff --git a/src/kudu/master/ranger_authz_provider.h b/src/kudu/master/ranger_authz_provider.h
index bdfb11a..a97ad12 100644
--- a/src/kudu/master/ranger_authz_provider.h
+++ b/src/kudu/master/ranger_authz_provider.h
@@ -40,6 +40,14 @@ namespace master {
 // An implementation of AuthzProvider that connects to Ranger and translates
 // authorization requests to Ranger and allows or denies the actions based on
 // the received responses.
+//
+// The privilege model for Kudu operations with Ranger follows the existing
+// one enforced with Sentry (see sentry_authz_provider.cc). However note that
+// in terms of policy evaluation, Ranger is different than Sentry that a policy
+// with a higher scope in the hierarchy cannot imply a lower scope its hierarchy
+// tree. e.g. 'METADATA on db=a' cannot imply 'METADATA on db=a->table=tbl'.
+// Therefore, in Ranger world one can grant 'METADATA on db=a->table=*->column=*'
+// to match with Sentry policy 'METADATA on db=a'.
 class RangerAuthzProvider : public AuthzProvider {
  public:
 
diff --git a/src/kudu/ranger/ranger_client.cc b/src/kudu/ranger/ranger_client.cc
index 73e17d2..c67e03c 100644
--- a/src/kudu/ranger/ranger_client.cc
+++ b/src/kudu/ranger/ranger_client.cc
@@ -17,6 +17,8 @@
 
 #include "kudu/ranger/ranger_client.h"
 
+#include <stdint.h>
+
 #include <ostream>
 #include <string>
 #include <utility>
@@ -202,6 +204,15 @@ static bool ValidateRangerConfiguration() {
 }
 GROUP_FLAG_VALIDATOR(ranger_config_flags, ValidateRangerConfiguration);
 
+static const char* ScopeToString(RangerClient::Scope scope) {
+  switch (scope) {
+    case RangerClient::Scope::DATABASE: return "database";
+    case RangerClient::Scope::TABLE: return "table";
+  }
+  LOG(FATAL) << static_cast<uint16_t>(scope) << ": unknown scope";
+  __builtin_unreachable();
+}
+
 #define HISTINIT(member, x) member = METRIC_##x.Instantiate(entity)
 RangerSubprocessMetrics::RangerSubprocessMetrics(const scoped_refptr<MetricEntity>&
entity) {
   HISTINIT(sp_inbound_queue_length, ranger_subprocess_inbound_queue_length);
@@ -232,7 +243,8 @@ Status RangerClient::Start() {
 // TODO(abukor): refactor to avoid code duplication
 Status RangerClient::AuthorizeAction(const string& user_name,
                                      const ActionPB& action,
-                                     const string& table_name) {
+                                     const string& table_name,
+                                     Scope scope) {
   DCHECK(subprocess_);
   string db;
   Slice tbl;
@@ -251,7 +263,10 @@ Status RangerClient::AuthorizeAction(const string& user_name,
 
   req->set_action(action);
   req->set_database(db);
-  req->set_table(tbl.ToString());
+  // Only pass the table name if this is table level request.
+  if (scope == Scope::TABLE) {
+    req->set_table(tbl.ToString());
+  }
 
   RETURN_NOT_OK(subprocess_->Execute(req_list, &resp_list));
 
@@ -260,8 +275,8 @@ Status RangerClient::AuthorizeAction(const string& user_name,
     return Status::OK();
   }
 
-  LOG(WARNING) << Substitute("User $0 is not authorized to perform $1 on $2",
-                             user_name, ActionPB_Name(action), table_name);
+  LOG(WARNING) << Substitute("User $0 is not authorized to perform $1 on $2 at scope
($3)",
+                             user_name, ActionPB_Name(action), table_name, ScopeToString(scope));
   return Status::NotAuthorized(kUnauthorizedAction);
 }
 
diff --git a/src/kudu/ranger/ranger_client.h b/src/kudu/ranger/ranger_client.h
index d6e920b..2b1bbe1 100644
--- a/src/kudu/ranger/ranger_client.h
+++ b/src/kudu/ranger/ranger_client.h
@@ -51,6 +51,13 @@ typedef subprocess::SubprocessProxy<RangerRequestListPB, RangerResponseListPB,
 // A client for the Ranger service that communicates with a Java subprocess.
 class RangerClient {
  public:
+  // Similar to SentryAuthorizableScope scope which indicates the
+  // hierarchy of authorizables (database → table).
+  enum Scope {
+    DATABASE,
+    TABLE
+  };
+
   // Creates a Ranger client.
   explicit RangerClient(const scoped_refptr<MetricEntity>& metric_entity);
 
@@ -60,7 +67,8 @@ class RangerClient {
   // Authorizes an action on the table. Returns OK if 'user_name' is authorized
   // to perform 'action' on 'table_name', NotAuthorized otherwise.
   Status AuthorizeAction(const std::string& user_name, const ActionPB& action,
-                         const std::string& table_name) WARN_UNUSED_RESULT;
+                         const std::string& table_name, Scope scope = Scope::TABLE)
+      WARN_UNUSED_RESULT;
 
   // Authorizes action on multiple tables. If there is at least one table that
   // user is authorized to perform the action on, it sets 'table_names' to the


Mime
View raw message