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 ce3d08b92b6..98ee71997de 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 @@ -506,8 +506,7 @@ public class AccessController extends BaseMasterAndRegionObserver private void requireGlobalPermission(String request, Action perm, TableName tableName, Map> familyMap) throws IOException { User user = getActiveUser(); - if (authManager.authorize(user, perm) || (tableName != null && - authManager.authorize(user, tableName.getNamespaceAsString(), perm))) { + if (authManager.authorize(user, perm)) { logResult(AuthResult.allow(request, "Global check allowed", user, perm, tableName, familyMap)); } else { logResult(AuthResult.deny(request, "Global check failed", user, perm, tableName, familyMap)); @@ -527,8 +526,7 @@ public class AccessController extends BaseMasterAndRegionObserver private void requireGlobalPermission(String request, Action perm, String namespace) throws IOException { User user = getActiveUser(); - if (authManager.authorize(user, perm) - || (namespace != null && authManager.authorize(user, namespace, perm))) { + if (authManager.authorize(user, perm)) { logResult(AuthResult.allow(request, "Global check allowed", user, perm, namespace)); } else { logResult(AuthResult.deny(request, "Global check failed", user, perm, namespace)); @@ -538,6 +536,34 @@ public class AccessController extends BaseMasterAndRegionObserver } } + /** + * Checks that the user has the given global or namespace permission. + * @param namespace + * @param perm Action being requested + */ + public void requireNamespacePermission(String request, String namespace, + Action... permissions) throws IOException { + User user = getActiveUser(); + AuthResult result = null; + + for (Action permission : permissions) { + if (authManager.authorize(user, namespace, permission)) { + result = AuthResult.allow(request, "Namespace permission granted", + user, permission, namespace); + break; + } else { + // rest of the world + result = AuthResult.deny(request, "Insufficient permissions", user, + permission, namespace); + } + } + logResult(result); + if (!result.isAllowed()) { + throw new AccessDeniedException("Insufficient permissions " + + result.toContextString()); + } + } + /** * Returns true if the current user is allowed the given action * over at least one of the column qualifiers in the given column families. @@ -824,7 +850,7 @@ public class AccessController extends BaseMasterAndRegionObserver } /* ---- MasterObserver implementation ---- */ - + @Override public void start(CoprocessorEnvironment env) throws IOException { CompoundConfiguration conf = new CompoundConfiguration(); conf.add(env.getConfiguration()); @@ -879,6 +905,7 @@ public class AccessController extends BaseMasterAndRegionObserver tableAcls = new MapMaker().weakValues().makeMap(); } + @Override public void stop(CoprocessorEnvironment env) { } @@ -891,7 +918,7 @@ public class AccessController extends BaseMasterAndRegionObserver for (byte[] family: families) { familyMap.put(family, null); } - requireGlobalPermission("createTable", Action.CREATE, desc.getTableName(), familyMap); + requireNamespacePermission("createTable", desc.getTableName().getNamespaceAsString(), Action.CREATE); } @Override @@ -1178,7 +1205,7 @@ public class AccessController extends BaseMasterAndRegionObserver @Override public void preCreateNamespace(ObserverContext ctx, NamespaceDescriptor ns) throws IOException { - requirePermission("createNamespace", Action.ADMIN); + requireGlobalPermission("createNamespace", Action.ADMIN, ns.getName()); } @Override @@ -1204,13 +1231,15 @@ public class AccessController extends BaseMasterAndRegionObserver @Override public void preModifyNamespace(ObserverContext ctx, NamespaceDescriptor ns) throws IOException { + // We require only global permission so that + // a user with NS admin cannot altering namespace configurations. i.e. namespace quota requireGlobalPermission("modifyNamespace", Action.ADMIN, ns.getName()); } @Override public void preGetNamespaceDescriptor(ObserverContext ctx, String namespace) throws IOException { - requireGlobalPermission("getNamespaceDescriptor", Action.ADMIN, namespace); + requireNamespacePermission("getNamespaceDescriptor", namespace, Action.ADMIN); } @Override @@ -1222,7 +1251,7 @@ public class AccessController extends BaseMasterAndRegionObserver while (itr.hasNext()) { NamespaceDescriptor desc = itr.next(); try { - requireGlobalPermission("listNamespaces", Action.ADMIN, desc.getName()); + requireNamespacePermission("listNamespaces", desc.getName(), Action.ADMIN); } catch (AccessDeniedException e) { itr.remove(); } @@ -2163,7 +2192,7 @@ public class AccessController extends BaseMasterAndRegionObserver }); } else if (request.getType() == AccessControlProtos.Permission.Type.Namespace) { final String namespace = request.getNamespaceName().toStringUtf8(); - requireGlobalPermission("userPermissions", Action.ADMIN, namespace); + requireNamespacePermission("userPermissions", namespace, Action.ADMIN); perms = User.runAsLoginUser(new PrivilegedExceptionAction>() { @Override public List run() throws Exception { 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 11a131bdeef..27ee91580e7 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 @@ -285,7 +285,9 @@ public class TestAccessController extends SecureTestUtil { // Test deleted the table, no problem LOG.info("Test deleted table " + TEST_TABLE.getTableName()); } + // Verify all table/namespace permissions are erased assertEquals(0, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size()); + assertEquals(0, AccessControlLists.getNamespacePermissions(conf, TEST_TABLE.getTableName().getNameAsString()).size()); } @Test 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 2ee1100d595..327024705e1 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 @@ -65,14 +65,33 @@ public class TestNamespaceCommands extends SecureTestUtil { // user with all permissions private static User SUPERUSER; + + // user with A permission on global + private static User USER_GLOBAL_ADMIN; + // user with C permission on global + private static User USER_GLOBAL_CREATE; + // user with W permission on global + private static User USER_GLOBAL_WRITE; + // user with R permission on global + private static User USER_GLOBAL_READ; + // user with X permission on global + private static User USER_GLOBAL_EXEC; + + // user with A permission on namespace + private static User USER_NS_ADMIN; + // user with C permission on namespace + private static User USER_NS_CREATE; + // user with W permission on namespace + private static User USER_NS_WRITE; + // user with R permission on namespace. + private static User USER_NS_READ; + // user with X permission on namespace. + private static User USER_NS_EXEC; + // user with rw permissions - private static User USER_RW; - // user with create table permissions alone - private static User USER_CREATE; - // user with permission on namespace for testing all operations. - private static User USER_NSP_WRITE; - // user with admin permission on namespace. - private static User USER_NSP_ADMIN; + private static User USER_TABLE_WRITE; // TODO: WE DO NOT GIVE ANY PERMS TO THIS USER + //user with create table permissions alone + private static User USER_TABLE_CREATE; // TODO: WE DO NOT GIVE ANY PERMS TO THIS USER private static String TEST_TABLE = TEST_NAMESPACE + ":testtable"; private static byte[] TEST_FAMILY = Bytes.toBytes("f1"); @@ -83,10 +102,22 @@ public class TestNamespaceCommands extends SecureTestUtil { enableSecurity(conf); SUPERUSER = User.createUserForTesting(conf, "admin", new String[] { "supergroup" }); - USER_RW = User.createUserForTesting(conf, "rw_user", new String[0]); - USER_CREATE = User.createUserForTesting(conf, "create_user", new String[0]); - USER_NSP_WRITE = User.createUserForTesting(conf, "namespace_write", new String[0]); - USER_NSP_ADMIN = User.createUserForTesting(conf, "namespace_admin", new String[0]); + // Users with global permissions + USER_GLOBAL_ADMIN = User.createUserForTesting(conf, "global_admin", new String[0]); + USER_GLOBAL_CREATE = User.createUserForTesting(conf, "global_create", new String[0]); + USER_GLOBAL_WRITE = User.createUserForTesting(conf, "global_write", new String[0]); + USER_GLOBAL_READ = User.createUserForTesting(conf, "global_read", new String[0]); + USER_GLOBAL_EXEC = User.createUserForTesting(conf, "global_exec", new String[0]); + + USER_NS_ADMIN = User.createUserForTesting(conf, "namespace_admin", new String[0]); + USER_NS_CREATE = User.createUserForTesting(conf, "namespace_create", new String[0]); + USER_NS_WRITE = User.createUserForTesting(conf, "namespace_write", new String[0]); + USER_NS_READ = User.createUserForTesting(conf, "namespace_read", new String[0]); + USER_NS_EXEC = User.createUserForTesting(conf, "namespace_exec", new String[0]); + + USER_TABLE_CREATE = User.createUserForTesting(conf, "table_create", new String[0]); + USER_TABLE_WRITE = User.createUserForTesting(conf, "table_write", new String[0]); + // TODO: other table perms UTIL.startMiniCluster(); // Wait for the ACL table to become available @@ -99,11 +130,21 @@ public class TestNamespaceCommands extends SecureTestUtil { UTIL.getHBaseAdmin().createNamespace(NamespaceDescriptor.create(TEST_NAMESPACE).build()); UTIL.getHBaseAdmin().createNamespace(NamespaceDescriptor.create(TEST_NAMESPACE2).build()); - grantOnNamespace(UTIL, USER_NSP_WRITE.getShortName(), - TEST_NAMESPACE, Permission.Action.WRITE, Permission.Action.CREATE); + // grants on global + grantGlobal(UTIL, USER_GLOBAL_ADMIN.getShortName(), Permission.Action.ADMIN); + grantGlobal(UTIL, USER_GLOBAL_CREATE.getShortName(), Permission.Action.CREATE); + grantGlobal(UTIL, USER_GLOBAL_WRITE.getShortName(), Permission.Action.WRITE); + grantGlobal(UTIL, USER_GLOBAL_READ.getShortName(), Permission.Action.READ); + grantGlobal(UTIL, USER_GLOBAL_EXEC.getShortName(), Permission.Action.EXEC); - grantOnNamespace(UTIL, USER_NSP_ADMIN.getShortName(), TEST_NAMESPACE, Permission.Action.ADMIN); - grantOnNamespace(UTIL, USER_NSP_ADMIN.getShortName(), TEST_NAMESPACE2, Permission.Action.ADMIN); + // grants on namespace + grantOnNamespace(UTIL, USER_NS_ADMIN.getShortName(), TEST_NAMESPACE, Permission.Action.ADMIN); + grantOnNamespace(UTIL, USER_NS_CREATE.getShortName(), TEST_NAMESPACE, Permission.Action.CREATE); + grantOnNamespace(UTIL, USER_NS_WRITE.getShortName(), TEST_NAMESPACE, Permission.Action.WRITE); + grantOnNamespace(UTIL, USER_NS_READ.getShortName(), TEST_NAMESPACE, Permission.Action.READ); + grantOnNamespace(UTIL, USER_NS_EXEC.getShortName(), TEST_NAMESPACE, Permission.Action.EXEC); + + grantOnNamespace(UTIL, USER_NS_ADMIN.getShortName(), TEST_NAMESPACE2, Permission.Action.ADMIN); } @AfterClass @@ -118,15 +159,20 @@ public class TestNamespaceCommands extends SecureTestUtil { String userTestNamespace = "userTestNsp"; Table acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME); try { + ListMultimap perms = + AccessControlLists.getNamespacePermissions(conf, TEST_NAMESPACE); + + perms = AccessControlLists.getNamespacePermissions(conf, TEST_NAMESPACE); + assertEquals(5, perms.size()); + // Grant and check state in ACL table grantOnNamespace(UTIL, userTestNamespace, TEST_NAMESPACE, Permission.Action.WRITE); Result result = acl.get(new Get(Bytes.toBytes(userTestNamespace))); assertTrue(result != null); - ListMultimap perms = - AccessControlLists.getNamespacePermissions(conf, TEST_NAMESPACE); - assertEquals(3, perms.size()); + perms = AccessControlLists.getNamespacePermissions(conf, TEST_NAMESPACE); + assertEquals(6, perms.size()); List namespacePerms = perms.get(userTestNamespace); assertTrue(perms.containsKey(userTestNamespace)); assertEquals(1, namespacePerms.size()); @@ -142,7 +188,7 @@ public class TestNamespaceCommands extends SecureTestUtil { Permission.Action.WRITE); perms = AccessControlLists.getNamespacePermissions(conf, TEST_NAMESPACE); - assertEquals(2, perms.size()); + assertEquals(5, perms.size()); } finally { acl.close(); } @@ -157,15 +203,28 @@ public class TestNamespaceCommands extends SecureTestUtil { return null; } }; - // verify that superuser or hbase admin can modify namespaces. - verifyAllowed(modifyNamespace, SUPERUSER, USER_NSP_ADMIN); - // all others should be denied - verifyDenied(modifyNamespace, USER_NSP_WRITE, USER_CREATE, USER_RW); + + // modifyNamespace: superuser | global(A) | NS(A) + verifyAllowed(modifyNamespace, + SUPERUSER, + USER_GLOBAL_ADMIN); + + verifyDeniedWithException(modifyNamespace, + USER_GLOBAL_CREATE, + USER_GLOBAL_WRITE, + USER_GLOBAL_READ, + USER_GLOBAL_EXEC, + USER_NS_ADMIN, + USER_NS_CREATE, + USER_NS_WRITE, + USER_NS_READ, + USER_NS_EXEC); } @Test public void testCreateAndDeleteNamespace() throws Exception { AccessTestAction createNamespace = new AccessTestAction() { + @Override public Object run() throws Exception { ACCESS_CONTROLLER.preCreateNamespace(ObserverContext.createAndPrepare(CP_ENV, null), NamespaceDescriptor.create(TEST_NAMESPACE2).build()); @@ -174,6 +233,7 @@ public class TestNamespaceCommands extends SecureTestUtil { }; AccessTestAction deleteNamespace = new AccessTestAction() { + @Override public Object run() throws Exception { ACCESS_CONTROLLER.preDeleteNamespace(ObserverContext.createAndPrepare(CP_ENV, null), TEST_NAMESPACE2); @@ -181,29 +241,71 @@ public class TestNamespaceCommands extends SecureTestUtil { } }; - // verify that only superuser can create namespaces. - verifyAllowed(createNamespace, SUPERUSER); - // verify that superuser or hbase admin can delete namespaces. - verifyAllowed(deleteNamespace, SUPERUSER, USER_NSP_ADMIN); + // createNamespace: superuser | global(A) + verifyAllowed(createNamespace, + SUPERUSER, + USER_GLOBAL_ADMIN); // all others should be denied - verifyDenied(createNamespace, USER_NSP_WRITE, USER_CREATE, USER_RW, USER_NSP_ADMIN); - verifyDenied(deleteNamespace, USER_NSP_WRITE, USER_CREATE, USER_RW); + verifyDeniedWithException(createNamespace, + USER_GLOBAL_CREATE, + USER_GLOBAL_WRITE, + USER_GLOBAL_READ, + USER_GLOBAL_EXEC, + USER_NS_ADMIN, + USER_NS_CREATE, + USER_NS_WRITE, + USER_NS_READ, + USER_NS_EXEC, + USER_TABLE_CREATE, + USER_TABLE_WRITE); + + // deleteNamespace: superuser | global(A) | NS(A) + verifyAllowed(deleteNamespace, + SUPERUSER, + USER_GLOBAL_ADMIN); + + verifyDeniedWithException(deleteNamespace, + USER_GLOBAL_CREATE, + USER_GLOBAL_WRITE, + USER_GLOBAL_READ, + USER_GLOBAL_EXEC, + USER_NS_ADMIN, + USER_NS_CREATE, + USER_NS_WRITE, + USER_NS_READ, + USER_NS_EXEC, + USER_TABLE_CREATE, + USER_TABLE_WRITE); } @Test public void testGetNamespaceDescriptor() throws Exception { AccessTestAction getNamespaceAction = new AccessTestAction() { + @Override public Object run() throws Exception { ACCESS_CONTROLLER.preGetNamespaceDescriptor(ObserverContext.createAndPrepare(CP_ENV, null), TEST_NAMESPACE); return null; } }; - // verify that superuser or hbase admin can get the namespace descriptor. - verifyAllowed(getNamespaceAction, SUPERUSER, USER_NSP_ADMIN); - // all others should be denied - verifyDenied(getNamespaceAction, USER_NSP_WRITE, USER_CREATE, USER_RW); + // getNamespaceDescriptor : superuser | global(A) | NS(A) + verifyAllowed(getNamespaceAction, + SUPERUSER, + USER_GLOBAL_ADMIN, + USER_NS_ADMIN); + + verifyDeniedWithException(getNamespaceAction, + USER_GLOBAL_CREATE, + USER_GLOBAL_WRITE, + USER_GLOBAL_READ, + USER_GLOBAL_EXEC, + USER_NS_CREATE, + USER_NS_WRITE, + USER_NS_READ, + USER_NS_EXEC, + USER_TABLE_CREATE, + USER_TABLE_WRITE); } @Test @@ -223,12 +325,30 @@ public class TestNamespaceCommands extends SecureTestUtil { } }; - verifyAllowed(listAction, SUPERUSER, USER_NSP_ADMIN); - verifyDenied(listAction, USER_NSP_WRITE, USER_CREATE, USER_RW); + // listNamespaces : All access* + // * Returned list will only show what you can call getNamespaceDescriptor() + + verifyAllowed(listAction, + SUPERUSER, + USER_GLOBAL_ADMIN, + USER_NS_ADMIN); // we have 3 namespaces: [default, hbase, TEST_NAMESPACE, TEST_NAMESPACE2] assertEquals(4, ((List)SUPERUSER.runAs(listAction)).size()); - assertEquals(2, ((List)USER_NSP_ADMIN.runAs(listAction)).size()); + assertEquals(4, ((List)USER_GLOBAL_ADMIN.runAs(listAction)).size()); + + assertEquals(2, ((List)USER_NS_ADMIN.runAs(listAction)).size()); + + assertEquals(0, ((List)USER_GLOBAL_CREATE.runAs(listAction)).size()); + assertEquals(0, ((List)USER_GLOBAL_WRITE.runAs(listAction)).size()); + assertEquals(0, ((List)USER_GLOBAL_READ.runAs(listAction)).size()); + assertEquals(0, ((List)USER_GLOBAL_EXEC.runAs(listAction)).size()); + assertEquals(0, ((List)USER_NS_CREATE.runAs(listAction)).size()); + assertEquals(0, ((List)USER_NS_WRITE.runAs(listAction)).size()); + assertEquals(0, ((List)USER_NS_READ.runAs(listAction)).size()); + assertEquals(0, ((List)USER_NS_EXEC.runAs(listAction)).size()); + assertEquals(0, ((List)USER_TABLE_CREATE.runAs(listAction)).size()); + assertEquals(0, ((List)USER_TABLE_WRITE.runAs(listAction)).size()); } @Test @@ -238,6 +358,7 @@ public class TestNamespaceCommands extends SecureTestUtil { // Test if client API actions are authorized AccessTestAction grantAction = new AccessTestAction() { + @Override public Object run() throws Exception { Table acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME); try { @@ -285,16 +406,56 @@ public class TestNamespaceCommands extends SecureTestUtil { } }; - // Only HBase super user should be able to grant and revoke permissions to - // namespaces - verifyAllowed(grantAction, SUPERUSER, USER_NSP_ADMIN); - verifyDenied(grantAction, USER_CREATE, USER_RW); - verifyAllowed(revokeAction, SUPERUSER, USER_NSP_ADMIN); - verifyDenied(revokeAction, USER_CREATE, USER_RW); + verifyAllowed(grantAction, + SUPERUSER, + USER_GLOBAL_ADMIN); - // Only an admin should be able to get the user permission - verifyAllowed(revokeAction, SUPERUSER, USER_NSP_ADMIN); - verifyDeniedWithException(revokeAction, USER_CREATE, USER_RW); + verifyDeniedWithException(grantAction, + USER_GLOBAL_CREATE, + USER_GLOBAL_WRITE, + USER_GLOBAL_READ, + USER_GLOBAL_EXEC, + USER_NS_ADMIN, + USER_NS_CREATE, + USER_NS_WRITE, + USER_NS_READ, + USER_NS_EXEC, + USER_TABLE_CREATE, + USER_TABLE_WRITE); + + verifyAllowed(revokeAction, + SUPERUSER, + USER_GLOBAL_ADMIN); + + verifyDeniedWithException(revokeAction, + USER_GLOBAL_CREATE, + USER_GLOBAL_WRITE, + USER_GLOBAL_READ, + USER_GLOBAL_EXEC, + USER_NS_ADMIN, + USER_NS_CREATE, + USER_NS_WRITE, + USER_NS_READ, + USER_NS_EXEC, + USER_TABLE_CREATE, + USER_TABLE_WRITE); + + verifyAllowed(getPermissionsAction, + SUPERUSER, + USER_GLOBAL_ADMIN, + USER_NS_ADMIN); + + verifyDeniedWithException(getPermissionsAction, + USER_GLOBAL_CREATE, + USER_GLOBAL_WRITE, + USER_GLOBAL_READ, + USER_GLOBAL_EXEC, + USER_NS_CREATE, + USER_NS_WRITE, + USER_NS_READ, + USER_NS_EXEC, + USER_TABLE_CREATE, + USER_TABLE_WRITE); } @Test @@ -309,10 +470,22 @@ public class TestNamespaceCommands extends SecureTestUtil { } }; - // Only users with create permissions on namespace should be able to create a new table - verifyAllowed(createTable, SUPERUSER, USER_NSP_WRITE); + //createTable : superuser | global(C) | NS(C) + verifyAllowed(createTable, + SUPERUSER, + USER_GLOBAL_CREATE, + USER_NS_CREATE); - // all others should be denied - verifyDenied(createTable, USER_CREATE, USER_RW); + verifyDeniedWithException(createTable, + USER_GLOBAL_ADMIN, + USER_GLOBAL_WRITE, + USER_GLOBAL_READ, + USER_GLOBAL_EXEC, + USER_NS_ADMIN, + USER_NS_WRITE, + USER_NS_READ, + USER_NS_EXEC, + USER_TABLE_CREATE, + USER_TABLE_WRITE); } }