diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java index 919d7a0..e6b0b01 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java @@ -301,7 +301,9 @@ public class AccessControlClient { } /** - * List all the userPermissions matching the given pattern. + * List all the userPermissions matching the given pattern. If pattern is null, the behavior is + * dependent on whether user has global admin privileges or not. If yes, the global permissions + * along with the list of superusers would be returned. Else, no rows get returned. * @param conf * @param tableRegex The regular expression string to match against * @return - returns an array of UserPermissions diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/Superusers.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/Superusers.java index b4ce36e..0f28e26 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/Superusers.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/Superusers.java @@ -121,4 +121,8 @@ public final class Superusers { return false; } } + + public static List getSuperUsers() { + return superUsers; + } } \ No newline at end of file diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index e2f4105..4c65fcf 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -1106,7 +1106,7 @@ public class AccessController extends BaseMasterAndRegionObserver @Override public Void run() throws Exception { UserPermission userperm = new UserPermission(Bytes.toBytes(owner), - htd.getTableName(), null, Action.values()); + htd.getTableName(), null, Action.values()); AccessControlLists.addUserPermission(conf, userperm); return null; } @@ -1124,7 +1124,7 @@ public class AccessController extends BaseMasterAndRegionObserver public void preModifyColumn(ObserverContext c, TableName tableName, HColumnDescriptor descriptor) throws IOException { requirePermission("modifyColumn", tableName, descriptor.getName(), null, Action.ADMIN, - Action.CREATE); + Action.CREATE); } @Override @@ -1299,7 +1299,7 @@ public class AccessController extends BaseMasterAndRegionObserver } }); this.authManager.getZKPermissionWatcher().deleteNamespaceACLNode(namespace); - LOG.info(namespace + " entry deleted in "+AccessControlLists.ACL_TABLE_NAME+" table."); + LOG.info(namespace + " entry deleted in " + AccessControlLists.ACL_TABLE_NAME + " table."); } @Override @@ -1755,7 +1755,7 @@ public class AccessController extends BaseMasterAndRegionObserver Map> families = makeFamilyMap(family, qualifier); User user = getActiveUser(); AuthResult authResult = permissionGranted(OpType.CHECK_AND_DELETE, user, env, families, - Action.READ, Action.WRITE); + Action.READ, Action.WRITE); logResult(authResult); if (!authResult.isAllowed()) { if (cellFeaturesEnabled && !compatibleEarlyTermination) { @@ -1807,7 +1807,7 @@ public class AccessController extends BaseMasterAndRegionObserver Map> families = makeFamilyMap(family, qualifier); User user = getActiveUser(); AuthResult authResult = permissionGranted(OpType.INCREMENT_COLUMN_VALUE, user, env, families, - Action.WRITE); + Action.WRITE); if (!authResult.isAllowed() && cellFeaturesEnabled && !compatibleEarlyTermination) { authResult.setAllowed(checkCoveringPermission(OpType.INCREMENT_COLUMN_VALUE, env, row, families, HConstants.LATEST_TIMESTAMP, Action.WRITE)); @@ -1989,7 +1989,7 @@ public class AccessController extends BaseMasterAndRegionObserver LOG.trace("Carrying forward ACLs from " + oldCell + ": " + perms); } tags.add(new Tag(AccessControlLists.ACL_TAG_TYPE, - ProtobufUtil.toUsersAndPermissions(perms).toByteArray())); + ProtobufUtil.toUsersAndPermissions(perms).toByteArray())); } } @@ -2267,6 +2267,13 @@ public class AccessController extends BaseMasterAndRegionObserver return AccessControlLists.getUserPermissions(regionEnv.getConfiguration(), null); } }); + // Adding superusers explicitly to the result set as AccessControlLists do not store them. + // Also using acl as table name to be inline with the results of global admin and will + // help in avoiding any leakage of information about being superusers. + for (String user: Superusers.getSuperUsers()) { + perms.add(new UserPermission(user.getBytes(), AccessControlLists.ACL_TABLE_NAME, null, + Action.values())); + } } response = ResponseConverter.buildGetUserPermissionsResponse(perms); } else { 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 5abef21..9b198b8 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 @@ -27,6 +27,7 @@ import static org.junit.Assert.fail; import java.io.IOException; import java.security.PrivilegedAction; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -47,6 +48,7 @@ import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HRegionLocation; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.KeyValue; +import org.apache.hadoop.hbase.security.Superusers; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.MiniHBaseCluster; import org.apache.hadoop.hbase.NamespaceDescriptor; @@ -344,7 +346,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, htd); + TEST_TABLE, htd); return null; } }; @@ -434,7 +436,7 @@ public class TestAccessController extends SecureTestUtil { }; verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER, USER_ADMIN_CF, - USER_GROUP_CREATE, USER_GROUP_ADMIN); + USER_GROUP_CREATE, USER_GROUP_ADMIN); verifyDenied(action, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE); } @@ -664,7 +666,7 @@ public class TestAccessController extends SecureTestUtil { verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_OWNER, USER_GROUP_ADMIN); verifyDenied(action, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, - USER_GROUP_WRITE, USER_GROUP_CREATE); + USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test @@ -732,7 +734,7 @@ public class TestAccessController extends SecureTestUtil { private void verifyReadWrite(AccessTestAction action) throws Exception { verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW); verifyDenied(action, USER_NONE, USER_RO, USER_GROUP_ADMIN, USER_GROUP_CREATE, USER_GROUP_READ, - USER_GROUP_WRITE); + USER_GROUP_WRITE); } @Test @@ -1064,7 +1066,7 @@ public class TestAccessController extends SecureTestUtil { verifyAllowed(grantAction, SUPERUSER, USER_ADMIN, USER_OWNER, USER_GROUP_ADMIN); verifyDenied(grantAction, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, - USER_GROUP_WRITE, USER_GROUP_CREATE); + USER_GROUP_WRITE, USER_GROUP_CREATE); try { verifyAllowed(revokeAction, SUPERUSER, USER_ADMIN, USER_OWNER, USER_GROUP_ADMIN); verifyDenied(revokeAction, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, @@ -1328,6 +1330,11 @@ public class TestAccessController extends SecureTestUtil { } } + private boolean hasFoundUserPermission(List userPermissions, + List perms) { + return perms.containsAll(userPermissions); + } + private boolean hasFoundUserPermission(UserPermission userPermission, List perms) { return perms.contains(userPermission); } @@ -1579,10 +1586,17 @@ public class TestAccessController extends SecureTestUtil { } finally { acl.close(); } - UserPermission adminPerm = new UserPermission(Bytes.toBytes(USER_ADMIN.getShortName()), - AccessControlLists.ACL_TABLE_NAME, null, null, Bytes.toBytes("ACRW")); - assertTrue("Only global users and user admin has permission on table _acl_ per setup", - perms.size() == 5 && hasFoundUserPermission(adminPerm, perms)); + List adminPerms = new ArrayList(); + adminPerms.add(new UserPermission(Bytes.toBytes(USER_ADMIN.getShortName()), + AccessControlLists.ACL_TABLE_NAME, null, null, Bytes.toBytes("ACRW"))); + List superUsers = Superusers.getSuperUsers(); + for(String user: superUsers) { + adminPerms.add(new UserPermission(Bytes.toBytes(user), AccessControlLists.ACL_TABLE_NAME, + null, null, Action.values())); + } + assertTrue("Only super users, global users and user admin has permission on table hbase:acl " + + "per setup", perms.size() == 5 + superUsers.size() && + hasFoundUserPermission(adminPerms, perms)); } /** global operations */