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 <[email protected]> Authored: Wed Mar 25 09:28:09 2015 -0700 Committer: Andrew Purtell <[email protected]> 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
