You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2016/02/04 01:29:45 UTC

[1/6] hbase git commit: HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname

Repository: hbase
Updated Branches:
  refs/heads/0.98 60c6b6df1 -> 8172c6c4b
  refs/heads/branch-1 f1e7c0653 -> 360bb6246
  refs/heads/branch-1.0 5c2022f56 -> b43442c58
  refs/heads/branch-1.1 0ca1cac29 -> 15db2244e
  refs/heads/branch-1.2 54e5ca7da -> 1ec438b64
  refs/heads/master 2f5767376 -> 6256ce4e6


HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/6256ce4e
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/6256ce4e
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/6256ce4e

Branch: refs/heads/master
Commit: 6256ce4e63bbff16f0678a1fbd6c33649c373f29
Parents: 2f57673
Author: Andrew Purtell <ap...@apache.org>
Authored: Mon Feb 1 09:48:16 2016 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed Feb 3 10:43:20 2016 -0800

----------------------------------------------------------------------
 .../hbase/zookeeper/ZooKeeperWatcher.java       | 56 ++++++++++++++++++--
 1 file changed, 52 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/6256ce4e/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
index f7a2175..0bc75eb 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
@@ -26,6 +26,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -126,6 +128,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
 
   private final Exception constructorCaller;
 
+  /* A pattern that matches a Kerberos name, borrowed from Hadoop's KerberosName */
+  private static final Pattern NAME_PATTERN = Pattern.compile("([^/@]*)(/([^/@]*))?@([^/@]*)");
+
   /**
    * Instantiate a ZooKeeper connection and watcher.
    * @param identifier string that is passed to RecoverableZookeeper to be used as
@@ -215,6 +220,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    */
   public void checkAndSetZNodeAcls() {
     if (!ZKUtil.isSecureZooKeeper(getConfiguration())) {
+      LOG.info("not a secure deployment, proceeding");
       return;
     }
 
@@ -259,6 +265,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    * @throws IOException
    */
   private boolean isBaseZnodeAclSetup(List<ACL> acls) throws IOException {
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Checking znode ACLs");
+    }
     String[] superUsers = conf.getStrings(Superusers.SUPERUSER_CONF_KEY);
     // Check whether ACL set for all superusers
     if (superUsers != null && !checkACLForSuperUsers(superUsers, acls)) {
@@ -270,6 +279,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
     String hbaseUser = UserGroupInformation.getCurrentUser().getShortUserName();
 
     if (acls.isEmpty()) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("ACL is empty");
+      }
       return false;
     }
 
@@ -280,17 +292,45 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
       // and one for the hbase user
       if (Ids.ANYONE_ID_UNSAFE.equals(id)) {
         if (perms != Perms.READ) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+              id, perms, Perms.READ));
+          }
           return false;
         }
       } else if (superUsers != null && isSuperUserId(superUsers, id)) {
         if (perms != Perms.ALL) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+              id, perms, Perms.ALL));
+          }
           return false;
         }
-      } else if (new Id("sasl", hbaseUser).equals(id)) {
-        if (perms != Perms.ALL) {
+      } else if ("sasl".equals(id.getScheme())) {
+        String name = id.getId();
+        // If ZooKeeper recorded the Kerberos full name in the ACL, use only the shortname
+        Matcher match = NAME_PATTERN.matcher(name);
+        if (match.matches()) {
+          name = match.group(1);
+        }
+        if (name.equals(hbaseUser)) {
+          if (perms != Perms.ALL) {
+            if (LOG.isDebugEnabled()) {
+              LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+                id, perms, Perms.ALL));
+            }
+            return false;
+          }
+        } else {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("Unexpected shortname in SASL ACL: " + id);
+          }
           return false;
         }
       } else {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("unexpected ACL id '" + id + "'");
+        }
         return false;
       }
     }
@@ -306,8 +346,16 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
       // TODO: Validate super group members also when ZK supports setting node ACL for groups.
       if (!user.startsWith(AuthUtil.GROUP_PREFIX)) {
         for (ACL acl : acls) {
-          if (user.equals(acl.getId().getId()) && acl.getPerms() == Perms.ALL) {
-            hasAccess = true;
+          if (user.equals(acl.getId().getId())) {
+            if (acl.getPerms() == Perms.ALL) {
+              hasAccess = true;
+            } else {
+              if (LOG.isDebugEnabled()) {
+                LOG.debug(String.format(
+                  "superuser '%s' does not have correct permissions: have %0x, want %0x",
+                  acl.getId().getId(), acl.getPerms(), Perms.ALL));
+              }
+            }
             break;
           }
         }


[5/6] hbase git commit: HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname

Posted by ap...@apache.org.
HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname

Conflicts:
	hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/15db2244
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/15db2244
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/15db2244

Branch: refs/heads/branch-1.1
Commit: 15db2244e15647b94155293127828c4d0bad1c6d
Parents: 0ca1cac
Author: Andrew Purtell <ap...@apache.org>
Authored: Mon Feb 1 09:48:16 2016 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed Feb 3 15:11:19 2016 -0800

----------------------------------------------------------------------
 .../hbase/zookeeper/ZooKeeperWatcher.java       | 80 ++++++++++++++++++--
 1 file changed, 74 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/15db2244/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
index 4dffcd3..08a089c 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
@@ -26,12 +26,16 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.security.Superusers;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.Abortable;
+import org.apache.hadoop.hbase.AuthUtil;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.ZooKeeperConnectionException;
@@ -131,6 +135,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
 
   private final Exception constructorCaller;
 
+  /* A pattern that matches a Kerberos name, borrowed from Hadoop's KerberosName */
+  private static final Pattern NAME_PATTERN = Pattern.compile("([^/@]*)(/([^/@]*))?@([^/@]*)");
+
   /**
    * Instantiate a ZooKeeper connection and watcher.
    * @param identifier string that is passed to RecoverableZookeeper to be used as
@@ -223,6 +230,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    */
   public void checkAndSetZNodeAcls() {
     if (!ZKUtil.isSecureZooKeeper(getConfiguration())) {
+      LOG.info("not a secure deployment, proceeding");
       return;
     }
 
@@ -267,13 +275,23 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    * @throws IOException
    */
   private boolean isBaseZnodeAclSetup(List<ACL> acls) throws IOException {
-    String superUser = conf.get("hbase.superuser");
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Checking znode ACLs");
+    }
+    String[] superUsers = conf.getStrings(Superusers.SUPERUSER_CONF_KEY);
+    // Check whether ACL set for all superusers
+    if (superUsers != null && !checkACLForSuperUsers(superUsers, acls)) {
+      return false;
+    }
 
     // this assumes that current authenticated user is the same as zookeeper client user
     // configured via JAAS
     String hbaseUser = UserGroupInformation.getCurrentUser().getShortUserName();
 
     if (acls.isEmpty()) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("ACL is empty");
+      }
       return false;
     }
 
@@ -284,23 +302,73 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
       // and one for the hbase user
       if (Ids.ANYONE_ID_UNSAFE.equals(id)) {
         if (perms != Perms.READ) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+              id, perms, Perms.READ));
+          }
           return false;
         }
-      } else if (superUser != null && new Id("sasl", superUser).equals(id)) {
-        if (perms != Perms.ALL) {
-          return false;
+      } else if ("sasl".equals(id.getScheme())) {
+        String name = id.getId();
+        // If ZooKeeper recorded the Kerberos full name in the ACL, use only the shortname
+        Matcher match = NAME_PATTERN.matcher(name);
+        if (match.matches()) {
+          name = match.group(1);
         }
-      } else if (new Id("sasl", hbaseUser).equals(id)) {
-        if (perms != Perms.ALL) {
+        if (name.equals(hbaseUser)) {
+          if (perms != Perms.ALL) {
+            if (LOG.isDebugEnabled()) {
+              LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+                id, perms, Perms.ALL));
+            }
+            return false;
+          }
+        } else {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("Unexpected shortname in SASL ACL: " + id);
+          }
           return false;
         }
       } else {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("unexpected ACL id '" + id + "'");
+        }
         return false;
       }
     }
     return true;
   }
 
+  /*
+   * Validate whether ACL set for all superusers.
+   */
+  private boolean checkACLForSuperUsers(String[] superUsers, List<ACL> acls) {
+    for (String user : superUsers) {
+      boolean hasAccess = false;
+      // TODO: Validate super group members also when ZK supports setting node ACL for groups.
+      if (!user.startsWith(AuthUtil.GROUP_PREFIX)) {
+        for (ACL acl : acls) {
+          if (user.equals(acl.getId().getId())) {
+            if (acl.getPerms() == Perms.ALL) {
+              hasAccess = true;
+            } else {
+              if (LOG.isDebugEnabled()) {
+                LOG.debug(String.format(
+                  "superuser '%s' does not have correct permissions: have %0x, want %0x",
+                  acl.getId().getId(), acl.getPerms(), Perms.ALL));
+              }
+            }
+            break;
+          }
+        }
+        if (!hasAccess) {
+          return false;
+        }
+      }
+    }
+    return true;
+  }
+
   @Override
   public String toString() {
     return this.identifier + ", quorum=" + quorum + ", baseZNode=" + baseZNode;


[2/6] hbase git commit: HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname

Posted by ap...@apache.org.
HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/360bb624
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/360bb624
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/360bb624

Branch: refs/heads/branch-1
Commit: 360bb62469a80147eb248996373fdb0a79d3f066
Parents: f1e7c06
Author: Andrew Purtell <ap...@apache.org>
Authored: Mon Feb 1 09:48:16 2016 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed Feb 3 10:43:29 2016 -0800

----------------------------------------------------------------------
 .../hbase/zookeeper/ZooKeeperWatcher.java       | 56 ++++++++++++++++++--
 1 file changed, 52 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/360bb624/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
index 9401a62..c4e8990f 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
@@ -26,6 +26,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -136,6 +138,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
 
   private final Exception constructorCaller;
 
+  /* A pattern that matches a Kerberos name, borrowed from Hadoop's KerberosName */
+  private static final Pattern NAME_PATTERN = Pattern.compile("([^/@]*)(/([^/@]*))?@([^/@]*)");
+
   /**
    * Instantiate a ZooKeeper connection and watcher.
    * @param identifier string that is passed to RecoverableZookeeper to be used as
@@ -228,6 +233,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    */
   public void checkAndSetZNodeAcls() {
     if (!ZKUtil.isSecureZooKeeper(getConfiguration())) {
+      LOG.info("not a secure deployment, proceeding");
       return;
     }
 
@@ -272,6 +278,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    * @throws IOException
    */
   private boolean isBaseZnodeAclSetup(List<ACL> acls) throws IOException {
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Checking znode ACLs");
+    }
     String[] superUsers = conf.getStrings(Superusers.SUPERUSER_CONF_KEY);
     // Check whether ACL set for all superusers
     if (superUsers != null && !checkACLForSuperUsers(superUsers, acls)) {
@@ -283,6 +292,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
     String hbaseUser = UserGroupInformation.getCurrentUser().getShortUserName();
 
     if (acls.isEmpty()) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("ACL is empty");
+      }
       return false;
     }
 
@@ -293,17 +305,45 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
       // and one for the hbase user
       if (Ids.ANYONE_ID_UNSAFE.equals(id)) {
         if (perms != Perms.READ) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+              id, perms, Perms.READ));
+          }
           return false;
         }
       } else if (superUsers != null && isSuperUserId(superUsers, id)) {
         if (perms != Perms.ALL) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+              id, perms, Perms.ALL));
+          }
           return false;
         }
-      } else if (new Id("sasl", hbaseUser).equals(id)) {
-        if (perms != Perms.ALL) {
+      } else if ("sasl".equals(id.getScheme())) {
+        String name = id.getId();
+        // If ZooKeeper recorded the Kerberos full name in the ACL, use only the shortname
+        Matcher match = NAME_PATTERN.matcher(name);
+        if (match.matches()) {
+          name = match.group(1);
+        }
+        if (name.equals(hbaseUser)) {
+          if (perms != Perms.ALL) {
+            if (LOG.isDebugEnabled()) {
+              LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+                id, perms, Perms.ALL));
+            }
+            return false;
+          }
+        } else {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("Unexpected shortname in SASL ACL: " + id);
+          }
           return false;
         }
       } else {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("unexpected ACL id '" + id + "'");
+        }
         return false;
       }
     }
@@ -319,8 +359,16 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
       // TODO: Validate super group members also when ZK supports setting node ACL for groups.
       if (!user.startsWith(AuthUtil.GROUP_PREFIX)) {
         for (ACL acl : acls) {
-          if (user.equals(acl.getId().getId()) && acl.getPerms() == Perms.ALL) {
-            hasAccess = true;
+          if (user.equals(acl.getId().getId())) {
+            if (acl.getPerms() == Perms.ALL) {
+              hasAccess = true;
+            } else {
+              if (LOG.isDebugEnabled()) {
+                LOG.debug(String.format(
+                  "superuser '%s' does not have correct permissions: have %0x, want %0x",
+                  acl.getId().getId(), acl.getPerms(), Perms.ALL));
+              }
+            }
             break;
           }
         }


[6/6] hbase git commit: HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname

Posted by ap...@apache.org.
HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname

Conflicts:
	hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/b43442c5
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/b43442c5
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/b43442c5

Branch: refs/heads/branch-1.0
Commit: b43442c58a1a66f1c17f889c081fea159caaebf5
Parents: 5c2022f
Author: Andrew Purtell <ap...@apache.org>
Authored: Mon Feb 1 09:48:16 2016 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed Feb 3 16:19:23 2016 -0800

----------------------------------------------------------------------
 .../hbase/zookeeper/ZooKeeperWatcher.java       | 79 ++++++++++++++++++--
 .../java/org/apache/hadoop/hbase/AuthUtil.java  |  5 ++
 2 files changed, 78 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/b43442c5/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
index 7b591f8..983153f 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
@@ -24,12 +24,15 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.Abortable;
+import org.apache.hadoop.hbase.AuthUtil;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.ZooKeeperConnectionException;
 import org.apache.hadoop.security.UserGroupInformation;
@@ -126,6 +129,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
 
   private final Exception constructorCaller;
 
+  /* A pattern that matches a Kerberos name, borrowed from Hadoop's KerberosName */
+  private static final Pattern NAME_PATTERN = Pattern.compile("([^/@]*)(/([^/@]*))?@([^/@]*)");
+
   /**
    * Instantiate a ZooKeeper connection and watcher.
    * @param identifier string that is passed to RecoverableZookeeper to be used as
@@ -218,6 +224,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    */
   public void checkAndSetZNodeAcls() {
     if (!ZKUtil.isSecureZooKeeper(getConfiguration())) {
+      LOG.info("not a secure deployment, proceeding");
       return;
     }
 
@@ -262,13 +269,23 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    * @throws IOException
    */
   private boolean isBaseZnodeAclSetup(List<ACL> acls) throws IOException {
-    String superUser = conf.get("hbase.superuser");
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Checking znode ACLs");
+    }
+    String superUser = conf.get(AuthUtil.SUPERUSER_CONF_KEY);
+    // Check whether ACL set for all superusers
+    if (superUser != null && !checkACLForSuperUsers(new String[] { superUser }, acls)) {
+      return false;
+    }
 
     // this assumes that current authenticated user is the same as zookeeper client user
     // configured via JAAS
     String hbaseUser = UserGroupInformation.getCurrentUser().getShortUserName();
 
     if (acls.isEmpty()) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("ACL is empty");
+      }
       return false;
     }
 
@@ -279,23 +296,73 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
       // and one for the hbase user
       if (Ids.ANYONE_ID_UNSAFE.equals(id)) {
         if (perms != Perms.READ) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+              id, perms, Perms.READ));
+          }
           return false;
         }
-      } else if (superUser != null && new Id("sasl", superUser).equals(id)) {
-        if (perms != Perms.ALL) {
-          return false;
+      } else if ("sasl".equals(id.getScheme())) {
+        String name = id.getId();
+        // If ZooKeeper recorded the Kerberos full name in the ACL, use only the shortname
+        Matcher match = NAME_PATTERN.matcher(name);
+        if (match.matches()) {
+          name = match.group(1);
         }
-      } else if (new Id("sasl", hbaseUser).equals(id)) {
-        if (perms != Perms.ALL) {
+        if (name.equals(hbaseUser)) {
+          if (perms != Perms.ALL) {
+            if (LOG.isDebugEnabled()) {
+              LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+                id, perms, Perms.ALL));
+            }
+            return false;
+          }
+        } else {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("Unexpected shortname in SASL ACL: " + id);
+          }
           return false;
         }
       } else {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("unexpected ACL id '" + id + "'");
+        }
         return false;
       }
     }
     return true;
   }
 
+  /*
+   * Validate whether ACL set for all superusers.
+   */
+  private boolean checkACLForSuperUsers(String[] superUsers, List<ACL> acls) {
+    for (String user : superUsers) {
+      boolean hasAccess = false;
+      // TODO: Validate super group members also when ZK supports setting node ACL for groups.
+      if (!user.startsWith(AuthUtil.GROUP_PREFIX)) {
+        for (ACL acl : acls) {
+          if (user.equals(acl.getId().getId())) {
+            if (acl.getPerms() == Perms.ALL) {
+              hasAccess = true;
+            } else {
+              if (LOG.isDebugEnabled()) {
+                LOG.debug(String.format(
+                  "superuser '%s' does not have correct permissions: have %0x, want %0x",
+                  acl.getId().getId(), acl.getPerms(), Perms.ALL));
+              }
+            }
+            break;
+          }
+        }
+        if (!hasAccess) {
+          return false;
+        }
+      }
+    }
+    return true;
+  }
+
   @Override
   public String toString() {
     return this.identifier + ", quorum=" + quorum + ", baseZNode=" + baseZNode;

http://git-wip-us.apache.org/repos/asf/hbase/blob/b43442c5/hbase-common/src/main/java/org/apache/hadoop/hbase/AuthUtil.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/AuthUtil.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/AuthUtil.java
index cb7ab83..765a4b3 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/AuthUtil.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/AuthUtil.java
@@ -38,6 +38,11 @@ import org.apache.hadoop.security.UserGroupInformation;
 @InterfaceAudience.Public
 @InterfaceStability.Evolving
 public class AuthUtil {
+  /** Prefix character to denote group names */
+  public static final String GROUP_PREFIX = "@";
+  /** Configuration key for superusers */
+  public static final String SUPERUSER_CONF_KEY = "hbase.superuser";
+
   private static final Log LOG = LogFactory.getLog(AuthUtil.class);
 
   private AuthUtil() {


[3/6] hbase git commit: HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname

Posted by ap...@apache.org.
HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/1ec438b6
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/1ec438b6
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/1ec438b6

Branch: refs/heads/branch-1.2
Commit: 1ec438b64200ffd2a081409477123041c01dc51a
Parents: 54e5ca7
Author: Andrew Purtell <ap...@apache.org>
Authored: Mon Feb 1 09:48:16 2016 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed Feb 3 10:44:12 2016 -0800

----------------------------------------------------------------------
 .../hbase/zookeeper/ZooKeeperWatcher.java       | 56 ++++++++++++++++++--
 1 file changed, 52 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/1ec438b6/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
index 9401a62..c4e8990f 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
@@ -26,6 +26,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -136,6 +138,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
 
   private final Exception constructorCaller;
 
+  /* A pattern that matches a Kerberos name, borrowed from Hadoop's KerberosName */
+  private static final Pattern NAME_PATTERN = Pattern.compile("([^/@]*)(/([^/@]*))?@([^/@]*)");
+
   /**
    * Instantiate a ZooKeeper connection and watcher.
    * @param identifier string that is passed to RecoverableZookeeper to be used as
@@ -228,6 +233,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    */
   public void checkAndSetZNodeAcls() {
     if (!ZKUtil.isSecureZooKeeper(getConfiguration())) {
+      LOG.info("not a secure deployment, proceeding");
       return;
     }
 
@@ -272,6 +278,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    * @throws IOException
    */
   private boolean isBaseZnodeAclSetup(List<ACL> acls) throws IOException {
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Checking znode ACLs");
+    }
     String[] superUsers = conf.getStrings(Superusers.SUPERUSER_CONF_KEY);
     // Check whether ACL set for all superusers
     if (superUsers != null && !checkACLForSuperUsers(superUsers, acls)) {
@@ -283,6 +292,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
     String hbaseUser = UserGroupInformation.getCurrentUser().getShortUserName();
 
     if (acls.isEmpty()) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("ACL is empty");
+      }
       return false;
     }
 
@@ -293,17 +305,45 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
       // and one for the hbase user
       if (Ids.ANYONE_ID_UNSAFE.equals(id)) {
         if (perms != Perms.READ) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+              id, perms, Perms.READ));
+          }
           return false;
         }
       } else if (superUsers != null && isSuperUserId(superUsers, id)) {
         if (perms != Perms.ALL) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+              id, perms, Perms.ALL));
+          }
           return false;
         }
-      } else if (new Id("sasl", hbaseUser).equals(id)) {
-        if (perms != Perms.ALL) {
+      } else if ("sasl".equals(id.getScheme())) {
+        String name = id.getId();
+        // If ZooKeeper recorded the Kerberos full name in the ACL, use only the shortname
+        Matcher match = NAME_PATTERN.matcher(name);
+        if (match.matches()) {
+          name = match.group(1);
+        }
+        if (name.equals(hbaseUser)) {
+          if (perms != Perms.ALL) {
+            if (LOG.isDebugEnabled()) {
+              LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+                id, perms, Perms.ALL));
+            }
+            return false;
+          }
+        } else {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("Unexpected shortname in SASL ACL: " + id);
+          }
           return false;
         }
       } else {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("unexpected ACL id '" + id + "'");
+        }
         return false;
       }
     }
@@ -319,8 +359,16 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
       // TODO: Validate super group members also when ZK supports setting node ACL for groups.
       if (!user.startsWith(AuthUtil.GROUP_PREFIX)) {
         for (ACL acl : acls) {
-          if (user.equals(acl.getId().getId()) && acl.getPerms() == Perms.ALL) {
-            hasAccess = true;
+          if (user.equals(acl.getId().getId())) {
+            if (acl.getPerms() == Perms.ALL) {
+              hasAccess = true;
+            } else {
+              if (LOG.isDebugEnabled()) {
+                LOG.debug(String.format(
+                  "superuser '%s' does not have correct permissions: have %0x, want %0x",
+                  acl.getId().getId(), acl.getPerms(), Perms.ALL));
+              }
+            }
             break;
           }
         }


[4/6] hbase git commit: HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname

Posted by ap...@apache.org.
HBASE-15200 ZooKeeper znode ACL checks should only compare the shortname


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/8172c6c4
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/8172c6c4
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/8172c6c4

Branch: refs/heads/0.98
Commit: 8172c6c4b5edcfb4bbfd7a01a77303871a7578af
Parents: 60c6b6d
Author: Andrew Purtell <ap...@apache.org>
Authored: Mon Feb 1 09:48:16 2016 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed Feb 3 10:52:14 2016 -0800

----------------------------------------------------------------------
 .../hbase/zookeeper/ZooKeeperWatcher.java       | 56 ++++++++++++++++++--
 1 file changed, 52 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/8172c6c4/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
index ede8806..db07c98 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
@@ -24,6 +24,8 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -130,6 +132,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
 
   private final Exception constructorCaller;
 
+  /* A pattern that matches a Kerberos name, borrowed from Hadoop's KerberosName */
+  private static final Pattern NAME_PATTERN = Pattern.compile("([^/@]*)(/([^/@]*))?@([^/@]*)");
+
   /**
    * Instantiate a ZooKeeper connection and watcher.
    * @param identifier string that is passed to RecoverableZookeeper to be used as
@@ -222,6 +227,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    */
   public void checkAndSetZNodeAcls() {
     if (!ZKUtil.isSecureZooKeeper(getConfiguration())) {
+      LOG.info("not a secure deployment, proceeding");
       return;
     }
 
@@ -268,6 +274,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
    * @throws IOException
    */
   private boolean isBaseZnodeAclSetup(List<ACL> acls) throws IOException {
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Checking znode ACLs");
+    }
     String[] superUsers = conf.getStrings(Superusers.SUPERUSER_CONF_KEY);
     // Check whether ACL set for all superusers
     if (superUsers != null && !checkACLForSuperUsers(superUsers, acls)) {
@@ -279,6 +288,9 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
     String hbaseUser = UserGroupInformation.getCurrentUser().getShortUserName();
 
     if (acls.isEmpty()) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("ACL is empty");
+      }
       return false;
     }
 
@@ -289,17 +301,45 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
       // and one for the hbase user
       if (Ids.ANYONE_ID_UNSAFE.equals(id)) {
         if (perms != Perms.READ) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+              id, perms, Perms.READ));
+          }
           return false;
         }
       } else if (superUsers != null && isSuperUserId(superUsers, id)) {
         if (perms != Perms.ALL) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+              id, perms, Perms.ALL));
+          }
           return false;
         }
-      } else if (new Id("sasl", hbaseUser).equals(id)) {
-        if (perms != Perms.ALL) {
+      } else if ("sasl".equals(id.getScheme())) {
+        String name = id.getId();
+        // If ZooKeeper recorded the Kerberos full name in the ACL, use only the shortname
+        Matcher match = NAME_PATTERN.matcher(name);
+        if (match.matches()) {
+          name = match.group(1);
+        }
+        if (name.equals(hbaseUser)) {
+          if (perms != Perms.ALL) {
+            if (LOG.isDebugEnabled()) {
+              LOG.debug(String.format("permissions for '%s' are not correct: have %0x, want %0x",
+                id, perms, Perms.ALL));
+            }
+            return false;
+          }
+        } else {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("Unexpected shortname in SASL ACL: " + id);
+          }
           return false;
         }
       } else {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("unexpected ACL id '" + id + "'");
+        }
         return false;
       }
     }
@@ -315,8 +355,16 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable {
       // TODO: Validate super group members also when ZK supports setting node ACL for groups.
       if (!user.startsWith(AuthUtil.GROUP_PREFIX)) {
         for (ACL acl : acls) {
-          if (user.equals(acl.getId().getId()) && acl.getPerms() == Perms.ALL) {
-            hasAccess = true;
+          if (user.equals(acl.getId().getId())) {
+            if (acl.getPerms() == Perms.ALL) {
+              hasAccess = true;
+            } else {
+              if (LOG.isDebugEnabled()) {
+                LOG.debug(String.format(
+                  "superuser '%s' does not have correct permissions: have %0x, want %0x",
+                  acl.getId().getId(), acl.getPerms(), Perms.ALL));
+              }
+            }
             break;
           }
         }