hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From apurt...@apache.org
Subject [1/5] hbase git commit: HBASE-13294 Fix the critical ancient loopholes in security testing infrastructure (Srikanth Srungarapu)
Date Wed, 25 Mar 2015 18:21:45 GMT
Repository: hbase
Updated Branches:
  refs/heads/0.98 9575868f1 -> 389f4051f
  refs/heads/branch-1 01fdafb5e -> 050028c32
  refs/heads/branch-1.0 01fa677d2 -> 993258b1a
  refs/heads/master 5e1fc2587 -> 5a58116bb


HBASE-13294 Fix the critical ancient loopholes in security testing infrastructure (Srikanth
Srungarapu)


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

Branch: refs/heads/0.98
Commit: 389f4051fb270ff209dddf4841f0ab448f47b105
Parents: 9575868
Author: Andrew Purtell <apurtell@apache.org>
Authored: Wed Mar 25 09:28:09 2015 -0700
Committer: Andrew Purtell <apurtell@apache.org>
Committed: Wed Mar 25 09:28:09 2015 -0700

----------------------------------------------------------------------
 .../hbase/security/access/SecureTestUtil.java   | 63 ++++++++++++--------
 .../security/access/TestAccessController.java   | 51 ++++++----------
 .../access/TestCellACLWithMultipleVersions.java |  2 +-
 .../hbase/security/access/TestCellACLs.java     |  4 +-
 .../security/access/TestNamespaceCommands.java  |  6 +-
 .../access/TestScanEarlyTermination.java        |  2 +-
 6 files changed, 64 insertions(+), 64 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/389f4051/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
index 5d979bd..0c8fa81 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
@@ -143,6 +143,7 @@ public class SecureTestUtil {
    */
   static interface AccessTestAction extends PrivilegedExceptionAction<Object> { }
 
+  /** This fails only in case of ADE or empty list for any of the actions */
   public static void verifyAllowed(User user, AccessTestAction... actions) throws Exception
{
     for (AccessTestAction action : actions) {
       try {
@@ -159,6 +160,7 @@ public class SecureTestUtil {
     }
   }
 
+  /** This fails in case of ADE or empty list for any of the users. */
   public static void verifyAllowed(AccessTestAction action, User... users) throws Exception
{
     for (User user : users) {
       verifyAllowed(user, action);
@@ -180,36 +182,53 @@ public class SecureTestUtil {
     }
   }
 
-  public static void verifyDeniedWithException(User user, AccessTestAction... actions)
-      throws Exception {
-    verifyDenied(user, true, actions);
-  }
-
-  public static void verifyDeniedWithException(AccessTestAction action, User... users)
-      throws Exception {
+  /** This passes only in case of ADE for all users. */
+  public static void verifyDenied(AccessTestAction action, User... users) throws Exception
{
     for (User user : users) {
-      verifyDenied(user, true, action);
+      verifyDenied(user, action);
     }
   }
 
-  public static void verifyDenied(User user, AccessTestAction... actions) throws Exception
{
-    verifyDenied(user, false, actions);
-  }
-
-  public static void verifyDenied(User user, boolean requireException,
-      AccessTestAction... actions) throws Exception {
-    for (AccessTestAction action : actions) {
+  /** This passes only in case of empty list for all users. */
+  public static void verifyIfEmptyList(AccessTestAction action, User... users) throws Exception
{
+    for (User user : users) {
       try {
         Object obj = user.runAs(action);
-        if (requireException) {
-          fail("Expected exception was not thrown for user '" + user.getShortName() + "'");
-        }
         if (obj != null && obj instanceof List<?>) {
           List<?> results = (List<?>) obj;
           if (results != null && !results.isEmpty()) {
-            fail("Unexpected results for user '" + user.getShortName() + "'");
+            fail("Unexpected action results: " +  results + " for user '"
+                + user.getShortName() + "'");
           }
+        } else {
+          fail("Unexpected results for user '" + user.getShortName() + "'");
         }
+      } catch (AccessDeniedException ade) {
+        fail("Expected action to pass for user '" + user.getShortName() + "' but was denied");
+      }
+    }
+  }
+
+  /** This passes only in case of null for all users. */
+  public static void verifyIfNull(AccessTestAction  action, User... users) throws Exception
{
+    for (User user : users) {
+      try {
+        Object obj = user.runAs(action);
+        if (obj != null) {
+          fail("Non null results from action for user '" + user.getShortName() + "'");
+        }
+      } catch (AccessDeniedException ade) {
+        fail("Expected action to pass for user '" + user.getShortName() + "' but was denied");
+      }
+    }
+  }
+
+  /** This passes only in case of ADE for all actions */
+  public static void verifyDenied(User user, AccessTestAction... actions) throws Exception
{
+    for (AccessTestAction action : actions) {
+      try {
+        user.runAs(action);
+        fail("Expected exception was not thrown for user '" + user.getShortName() + "'");
       } catch (IOException e) {
         boolean isAccessDeniedException = false;
         if(e instanceof RetriesExhaustedWithDetailsException) {
@@ -255,12 +274,6 @@ public class SecureTestUtil {
     }
   }
 
-  public static void verifyDenied(AccessTestAction action, User... users) throws Exception
{
-    for (User user : users) {
-      verifyDenied(user, action);
-    }
-  }
-
   private static List<AccessController> getAccessControllers(MiniHBaseCluster cluster)
{
     List<AccessController> result = Lists.newArrayList();
     for (RegionServerThread t: cluster.getLiveRegionServerThreads()) {

http://git-wip-us.apache.org/repos/asf/hbase/blob/389f4051/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index 9ee7450..1762e41 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -307,7 +307,7 @@ public class TestAccessController extends SecureTestUtil {
         htd.addFamily(new HColumnDescriptor(TEST_FAMILY));
         htd.addFamily(new HColumnDescriptor("fam_" + User.getCurrent().getShortName()));
         ACCESS_CONTROLLER.preModifyTable(ObserverContext.createAndPrepare(CP_ENV, null),
-          TEST_TABLE.getTableName(), htd);
+            TEST_TABLE.getTableName(), htd);
         return null;
       }
     };
@@ -338,13 +338,13 @@ public class TestAccessController extends SecureTestUtil {
       public Object run() throws Exception {
         ACCESS_CONTROLLER
             .preTruncateTable(ObserverContext.createAndPrepare(CP_ENV, null),
-              TEST_TABLE.getTableName());
+                TEST_TABLE.getTableName());
         return null;
       }
     };
 
-    verifyAllowed(truncateTable, SUPERUSER, USER_ADMIN, USER_CREATE);
-    verifyDenied(truncateTable, USER_RW, USER_RO, USER_NONE, USER_OWNER);
+    verifyAllowed(truncateTable, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER);
+    verifyDenied(truncateTable, USER_RW, USER_RO, USER_NONE);
   }
 
   @Test
@@ -354,7 +354,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preAddColumn(ObserverContext.createAndPrepare(CP_ENV, null), TEST_TABLE.getTableName(),
-          hcd);
+            hcd);
         return null;
       }
     };
@@ -371,7 +371,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preModifyColumn(ObserverContext.createAndPrepare(CP_ENV, null),
-          TEST_TABLE.getTableName(), hcd);
+            TEST_TABLE.getTableName(), hcd);
         return null;
       }
     };
@@ -386,7 +386,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preDeleteColumn(ObserverContext.createAndPrepare(CP_ENV, null),
-          TEST_TABLE.getTableName(), TEST_FAMILY);
+            TEST_TABLE.getTableName(), TEST_FAMILY);
         return null;
       }
     };
@@ -401,7 +401,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preDisableTable(ObserverContext.createAndPrepare(CP_ENV, null),
-          TEST_TABLE.getTableName());
+            TEST_TABLE.getTableName());
         return null;
       }
     };
@@ -452,7 +452,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preMove(ObserverContext.createAndPrepare(CP_ENV, null),
-          firstRegion.getKey(), server, server);
+            firstRegion.getKey(), server, server);
         return null;
       }
     };
@@ -476,7 +476,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preAssign(ObserverContext.createAndPrepare(CP_ENV, null),
-          firstRegion.getKey());
+            firstRegion.getKey());
         return null;
       }
     };
@@ -500,7 +500,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preUnassign(ObserverContext.createAndPrepare(CP_ENV, null),
-          firstRegion.getKey(), false);
+            firstRegion.getKey(), false);
         return null;
       }
     };
@@ -524,7 +524,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preRegionOffline(ObserverContext.createAndPrepare(CP_ENV, null),
-          firstRegion.getKey());
+            firstRegion.getKey());
         return null;
       }
     };
@@ -634,7 +634,7 @@ public class TestAccessController extends SecureTestUtil {
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preMerge(
             ObserverContext.createAndPrepare(RSCP_ENV, null),
-            regions.get(0),regions.get(1));
+            regions.get(0), regions.get(1));
         return null;
       }
     };
@@ -663,7 +663,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preCompact(ObserverContext.createAndPrepare(RCP_ENV, null), null,
null,
-          ScanType.COMPACT_RETAIN_DELETES);
+            ScanType.COMPACT_RETAIN_DELETES);
         return null;
       }
     };
@@ -672,20 +672,6 @@ public class TestAccessController extends SecureTestUtil {
     verifyDenied(action, USER_RW, USER_RO, USER_NONE);
   }
 
-  @Test
-  public void testPreCompactSelection() throws Exception {
-    AccessTestAction action = new AccessTestAction() {
-      @Override
-      public Object run() throws Exception {
-        ACCESS_CONTROLLER.preCompactSelection(ObserverContext.createAndPrepare(RCP_ENV, null),
null, null);
-        return null;
-      }
-    };
-
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_OWNER);
-    verifyDenied(action, USER_CREATE, USER_RW, USER_RO, USER_NONE);
-  }
-
   private void verifyRead(AccessTestAction action) throws Exception {
     verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, USER_RO);
     verifyDenied(action, USER_NONE);
@@ -704,6 +690,7 @@ public class TestAccessController extends SecureTestUtil {
       public Object run() throws Exception {
         Get g = new Get(TEST_ROW);
         g.addFamily(TEST_FAMILY);
+
         HTable t = new HTable(conf, TEST_TABLE.getTableName());
         try {
           t.get(g);
@@ -1054,7 +1041,7 @@ public class TestAccessController extends SecureTestUtil {
     verifyDenied(getTablePermissionsAction, USER_CREATE, USER_RW, USER_RO, USER_NONE);
 
     verifyAllowed(getGlobalPermissionsAction, SUPERUSER, USER_ADMIN);
-    verifyDeniedWithException(getGlobalPermissionsAction, USER_CREATE,
+    verifyDenied(getGlobalPermissionsAction, USER_CREATE,
         USER_OWNER, USER_RW, USER_RO, USER_NONE);
   }
 
@@ -1484,7 +1471,7 @@ public class TestAccessController extends SecureTestUtil {
     UserPermission ownerperm = new UserPermission(
       Bytes.toBytes(USER_OWNER.getName()), tableName, null, Action.values());
     assertTrue("Owner should have all permissions on table",
-      hasFoundUserPermission(ownerperm, perms));
+        hasFoundUserPermission(ownerperm, perms));
 
     User user = User.createUserForTesting(TEST_UTIL.getConfiguration(), "user", new String[0]);
     byte[] userName = Bytes.toBytes(user.getShortName());
@@ -1496,7 +1483,7 @@ public class TestAccessController extends SecureTestUtil {
 
     // grant read permission
     grantOnTable(TEST_UTIL, user.getShortName(),
-      tableName, family1, qualifier, Permission.Action.READ);
+        tableName, family1, qualifier, Permission.Action.READ);
 
     acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
     try {
@@ -1575,7 +1562,7 @@ public class TestAccessController extends SecureTestUtil {
     UserPermission newOwnerperm = new UserPermission(
       Bytes.toBytes(newOwner.getName()), tableName, null, Action.values());
     assertTrue("New owner should have all permissions on table",
-      hasFoundUserPermission(newOwnerperm, perms));
+        hasFoundUserPermission(newOwnerperm, perms));
 
     // delete table
     admin.deleteTable(tableName);

http://git-wip-us.apache.org/repos/asf/hbase/blob/389f4051/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java
index 7d5a07e..98e864d 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java
@@ -439,7 +439,7 @@ public class TestCellACLWithMultipleVersions extends SecureTestUtil {
     
     // The other put should be covered by the tombstone
 
-    verifyDenied(getQ2, USER_OTHER);
+    verifyIfNull(getQ2, USER_OTHER);
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/hbase/blob/389f4051/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLs.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLs.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLs.java
index 62eb3c2..4a1ee09 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLs.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLs.java
@@ -224,8 +224,8 @@ public class TestCellACLs extends SecureTestUtil {
 
     // Confirm this access does not extend to other cells
 
-    verifyDenied(getQ3, USER_OTHER);
-    verifyDenied(getQ4, USER_OTHER);
+    verifyIfNull(getQ3, USER_OTHER);
+    verifyIfNull(getQ4, USER_OTHER);
 
     /* ---- Scans ---- */
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/389f4051/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
index 5e022bb..62a8935 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
@@ -28,10 +28,10 @@ import org.apache.hadoop.hbase.HColumnDescriptor;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.hbase.NamespaceDescriptor;
+import org.apache.hadoop.hbase.client.HTable;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.Get;
-import org.apache.hadoop.hbase.client.HTable;
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment;
 import org.apache.hadoop.hbase.coprocessor.ObserverContext;
@@ -147,7 +147,7 @@ public class TestNamespaceCommands extends SecureTestUtil {
     AccessTestAction modifyNamespace = new AccessTestAction() {
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preModifyNamespace(ObserverContext.createAndPrepare(CP_ENV, null),
-          NamespaceDescriptor.create(TEST_NAMESPACE).addConfiguration("abc", "156").build());
+            NamespaceDescriptor.create(TEST_NAMESPACE).addConfiguration("abc", "156").build());
         return null;
       }
     };
@@ -248,7 +248,7 @@ public class TestNamespaceCommands extends SecureTestUtil {
 
     // Only an admin should be able to get the user permission
     verifyAllowed(revokeAction, SUPERUSER, USER_NSP_ADMIN);
-    verifyDeniedWithException(revokeAction, USER_CREATE, USER_RW);
+    verifyDenied(revokeAction, USER_CREATE, USER_RW);
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/hbase/blob/389f4051/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestScanEarlyTermination.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestScanEarlyTermination.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestScanEarlyTermination.java
index 0211381..6b3332e 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestScanEarlyTermination.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestScanEarlyTermination.java
@@ -219,7 +219,7 @@ public class TestScanEarlyTermination extends SecureTestUtil {
     }, USER_OTHER);
 
     // A scan of FAMILY2 will throw an AccessDeniedException
-    verifyDeniedWithException(new AccessTestAction() {
+    verifyDenied(new AccessTestAction() {
       @Override
       public Object run() throws Exception {
         // force a new RS connection


Mime
View raw message