You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by gr...@apache.org on 2020/03/23 14:11:06 UTC

[kudu] branch master updated (58f7a30 -> 647a3d2)

This is an automated email from the ASF dual-hosted git repository.

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


    from 58f7a30  tablet: plumb delta stats into delta compaction outputs
     new 84d2367  [ranger] fix incorrect authz enforcement in Ranger authz provider
     new 647a3d2  server: make FS layout creation message less agressive

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/kudu/fs/dir_util.cc                  |  2 +-
 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 +++++++++-
 src/kudu/server/server_base.cc           |  3 +--
 6 files changed, 50 insertions(+), 13 deletions(-)


[kudu] 02/02: server: make FS layout creation message less agressive

Posted by gr...@apache.org.
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 647a3d2f18576ca4f17c0a80f10590ca0187e6b6
Author: Andrew <an...@gmail.com>
AuthorDate: Sat Mar 21 21:45:04 2020 -0700

    server: make FS layout creation message less agressive
    
    When we start a Kudu server, we first attempt to open an existing FS
    layout, and if this fails, we assume that this was intentional and that
    we're actually deploying a new server, so we'll create some new FS
    metadata. The error message for this is a bit jarring:
    
    I0110 22:35:52.430444  1043 fs_manager.cc:263] Metadata directory not provided
    I0110 22:35:52.430590  1043 fs_manager.cc:269] Using write-ahead log directory (fs_wal_dir) as metadata directory
    I0110 22:35:52.430953  1043 server_base.cc:451] Could not load existing FS layout: Not found: could not find a healthy instance file
    I0110 22:35:52.431128  1043 server_base.cc:452] Attempting to create new FS layout instead
    I0110 22:35:52.434439  1043 fs_manager.cc:617] Generated new instance metadata in path /tmp/dist-test-tasklK1NQQ/test-tmp/client_failover-itest.0.ClientFailoverTServerTimeoutITest.FailoverOnLeaderTimeout.1578695741089585-462/minicluster-data/ts-2/data/instance:
    uuid: "230bae04bca94f7989168f2ce71830fd"
    format_stamp: "Formatted at 2020-01-10 22:35:52 on dist-test-slave-dist-test-slave-nntq"
    I0110 22:35:52.435400  1043 fs_manager.cc:617] Generated new instance metadata in path /tmp/dist-test-tasklK1NQQ/test-tmp/client_failover-itest.0.ClientFailoverTServerTimeoutITest.FailoverOnLeaderTimeout.1578695741089585-462/minicluster-data/ts-2/wal/instance:
    uuid: "230bae04bca94f7989168f2ce71830fd"
    format_stamp: "Formatted at 2020-01-10 22:35:52 on dist-test-slave-dist-test-slave-nntq"
    I0110 22:35:52.440639  1043 dir_util.cc:225] Instance is unhealthy: Not found: Failed to read metadata file from /tmp/dist-test-tasklK1NQQ/test-tmp/client_failover-itest.0.ClientFailoverTServerTimeoutITest.FailoverOnLeaderTimeout.1578695741089585-462/minicluster-data/ts-2/data/data/block_manager_instance: /tmp/dist-test-tasklK1NQQ/test-tmp/client_failover-itest.0.ClientFailoverTServerTimeoutITest.FailoverOnLeaderTimeout.1578695741089585-462/minicluster-data/ts-2/data/data/block_manage [...]
    I0110 22:35:52.456311  1043 fs_manager.cc:518] Time spent creating directory manager: real 0.020s    user 0.017s    sys 0.003s
    
    I updated the dir_util message to be a VLOG instead. If the directory is
    failed, this will surface as an error later down the line with actual
    warning logs. I also made the initial server_base messages less
    concerning.
    
    Change-Id: Iba600eb4fbfecc444731d8ae3e971785fd703785
    Reviewed-on: http://gerrit.cloudera.org:8080/15521
    Tested-by: Kudu Jenkins
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
 src/kudu/fs/dir_util.cc        | 2 +-
 src/kudu/server/server_base.cc | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/kudu/fs/dir_util.cc b/src/kudu/fs/dir_util.cc
index 98f4e82..9b12c3b 100644
--- a/src/kudu/fs/dir_util.cc
+++ b/src/kudu/fs/dir_util.cc
@@ -118,7 +118,7 @@ Status CheckHolePunch(Env* env, const string& path) {
     const Status _s_prepended = _s.CloneAndPrepend(msg); \
     if (_s.IsNotFound() || _s.IsDiskFailure()) { \
       health_status_ = _s_prepended; \
-      LOG(INFO) << "Instance is unhealthy: " << _s_prepended.ToString(); \
+      VLOG(1) << "Directory instance has status: " << _s_prepended.ToString(); \
       return Status::OK(); \
     } \
     return _s_prepended; \
diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc
index 754c5a5..604486a 100644
--- a/src/kudu/server/server_base.cc
+++ b/src/kudu/server/server_base.cc
@@ -485,8 +485,7 @@ Status ServerBase::Init() {
   Status s = fs_manager_->Open(&report);
   // No instance files existed. Try creating a new FS layout.
   if (s.IsNotFound()) {
-    LOG(INFO) << "Could not load existing FS layout: " << s.ToString();
-    LOG(INFO) << "Attempting to create new FS layout instead";
+    LOG(INFO) << "This appears to be a new deployment of Kudu; creating new FS layout";
     is_first_run_ = true;
     s = fs_manager_->CreateInitialFileSystemLayout();
     if (s.IsAlreadyPresent()) {


[kudu] 01/02: [ranger] fix incorrect authz enforcement in Ranger authz provider

Posted by gr...@apache.org.
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 <ha...@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 <aw...@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