From c4aee2e2836d671b6da7d6e7052aad78ed88f74f Mon Sep 17 00:00:00 2001 From: Matteo Bertozzi Date: Thu, 11 Dec 2014 16:55:42 +0000 Subject: [PATCH] HBASE-12674 Add permission check to getNamespaceDescriptor() --- .../BaseMasterAndRegionObserver.java | 10 ++++++ .../hbase/coprocessor/BaseMasterObserver.java | 10 ++++++ .../hbase/coprocessor/MasterObserver.java | 18 ++++++++++ .../apache/hadoop/hbase/master/HMaster.java | 36 ++++++++++++++++--- .../hbase/master/MasterCoprocessorHost.java | 22 ++++++++++++ .../security/access/AccessController.java | 6 ++++ .../hbase/coprocessor/TestMasterObserver.java | 27 ++++++++++++++ .../access/TestNamespaceCommands.java | 15 ++++++++ 8 files changed, 139 insertions(+), 5 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterAndRegionObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterAndRegionObserver.java index 5893b163e30..98c0563f120 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterAndRegionObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterAndRegionObserver.java @@ -157,6 +157,16 @@ public abstract class BaseMasterAndRegionObserver extends BaseRegionObserver NamespaceDescriptor ns) throws IOException { } + @Override + public void preGetNamespaceDescriptor(ObserverContext ctx, + String namespace) throws IOException { + } + + @Override + public void postGetNamespaceDescriptor(ObserverContext ctx, + NamespaceDescriptor ns) throws IOException { + } + @Override public void preListNamespaceDescriptors(ObserverContext ctx, List descriptors) throws IOException { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterObserver.java index 6c69f81349f..4748a1be92f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterObserver.java @@ -150,6 +150,16 @@ public class BaseMasterObserver implements MasterObserver { public void postModifyNamespace(ObserverContext ctx, NamespaceDescriptor ns) throws IOException { } + @Override + public void preGetNamespaceDescriptor(ObserverContext ctx, + String namespace) throws IOException { + } + + @Override + public void postGetNamespaceDescriptor(ObserverContext ctx, + NamespaceDescriptor ns) throws IOException { + } + @Override public void preListNamespaceDescriptors(ObserverContext ctx, List descriptors) throws IOException { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java index 9f0428b310a..2d99754accf 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java @@ -788,6 +788,24 @@ public interface MasterObserver extends Coprocessor { void postModifyNamespace(final ObserverContext ctx, NamespaceDescriptor ns) throws IOException; + /** + * Called before a getNamespaceDescriptor request has been processed. + * @param ctx the environment to interact with the framework and master + * @param namespace the name of the namespace + * @throws IOException + */ + void preGetNamespaceDescriptor(ObserverContext ctx, + String namespace) throws IOException; + + /** + * Called after a getNamespaceDescriptor request has been processed. + * @param ctx the environment to interact with the framework and master + * @param ns the NamespaceDescriptor + * @throws IOException + */ + void postGetNamespaceDescriptor(ObserverContext ctx, + NamespaceDescriptor ns) throws IOException; + /** * Called before a listNamespaceDescriptors request has been processed. * @param ctx the environment to interact with the framework and master diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index 7961e0e37f6..90230929473 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -1248,7 +1248,7 @@ public class HMaster extends HRegionServer implements MasterServices, Server { } String namespace = hTableDescriptor.getTableName().getNamespaceAsString(); - getNamespaceDescriptor(namespace); // ensure namespace exists + ensureNamespaceExists(namespace); HRegionInfo[] newRegions = getHRegionInfos(hTableDescriptor, splitKeys); checkInitialized(); @@ -2044,13 +2044,39 @@ public class HMaster extends HRegionServer implements MasterServices, Server { } } - @Override - public NamespaceDescriptor getNamespaceDescriptor(String name) throws IOException { + /** + * Ensure that the specified namespace exists, otherwise throws a NamespaceNotFoundException + * + * @param name the namespace to check + * @throws IOException if the namespace manager is not ready yet. + * @throws NamespaceNotFoundException if the namespace does not exists + */ + private void ensureNamespaceExists(final String name) + throws IOException, NamespaceNotFoundException { checkNamespaceManagerReady(); NamespaceDescriptor nsd = tableNamespaceManager.get(name); if (nsd == null) { throw new NamespaceNotFoundException(name); } + } + + @Override + public NamespaceDescriptor getNamespaceDescriptor(String name) throws IOException { + checkNamespaceManagerReady(); + + if (cpHost != null) { + cpHost.preGetNamespaceDescriptor(name); + } + + NamespaceDescriptor nsd = tableNamespaceManager.get(name); + if (nsd == null) { + throw new NamespaceNotFoundException(name); + } + + if (cpHost != null) { + cpHost.postGetNamespaceDescriptor(nsd); + } + return nsd; } @@ -2076,13 +2102,13 @@ public class HMaster extends HRegionServer implements MasterServices, Server { @Override public List listTableDescriptorsByNamespace(String name) throws IOException { - getNamespaceDescriptor(name); // check that namespace exists + ensureNamespaceExists(name); return listTableDescriptors(name, null, null, true); } @Override public List listTableNamesByNamespace(String name) throws IOException { - getNamespaceDescriptor(name); // check that namespace exists + ensureNamespaceExists(name); return listTableNames(name, null, true); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index 397434ac584..14c2568d7d0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -139,6 +139,28 @@ public class MasterCoprocessorHost }); } + public void preGetNamespaceDescriptor(final String namespaceName) + throws IOException { + execOperation(coprocessors.isEmpty() ? null : new CoprocessorOperation() { + @Override + public void call(MasterObserver oserver, ObserverContext ctx) + throws IOException { + oserver.preGetNamespaceDescriptor(ctx, namespaceName); + } + }); + } + + public void postGetNamespaceDescriptor(final NamespaceDescriptor ns) + throws IOException { + execOperation(coprocessors.isEmpty() ? null : new CoprocessorOperation() { + @Override + public void call(MasterObserver oserver, ObserverContext ctx) + throws IOException { + oserver.postGetNamespaceDescriptor(ctx, ns); + } + }); + } + public boolean preListNamespaceDescriptors(final List descriptors) throws IOException { return execOperation(coprocessors.isEmpty() ? null : new CoprocessorOperation() { 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 9f382c69df9..5050afb8f9a 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 @@ -1184,6 +1184,12 @@ public class AccessController extends BaseMasterAndRegionObserver requireGlobalPermission("modifyNamespace", Action.ADMIN, ns.getName()); } + @Override + public void preGetNamespaceDescriptor(ObserverContext ctx, String namespace) + throws IOException { + requireGlobalPermission("getNamespaceDescriptor", Action.ADMIN, namespace); + } + @Override public void postListNamespaceDescriptors(ObserverContext ctx, List descriptors) throws IOException { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java index f99e9ae3793..419d7f42ef2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java @@ -91,6 +91,8 @@ public class TestMasterObserver { private boolean postDeleteNamespaceCalled; private boolean preModifyNamespaceCalled; private boolean postModifyNamespaceCalled; + private boolean preGetNamespaceDescriptorCalled; + private boolean postGetNamespaceDescriptorCalled; private boolean preListNamespaceDescriptorsCalled; private boolean postListNamespaceDescriptorsCalled; private boolean preAddColumnCalled; @@ -171,6 +173,8 @@ public class TestMasterObserver { postDeleteNamespaceCalled = false; preModifyNamespaceCalled = false; postModifyNamespaceCalled = false; + preGetNamespaceDescriptorCalled = false; + postGetNamespaceDescriptorCalled = false; preListNamespaceDescriptorsCalled = false; postListNamespaceDescriptorsCalled = false; preAddColumnCalled = false; @@ -392,6 +396,23 @@ public class TestMasterObserver { return preModifyNamespaceCalled && !postModifyNamespaceCalled; } + + @Override + public void preGetNamespaceDescriptor(ObserverContext ctx, + String namespace) throws IOException { + preGetNamespaceDescriptorCalled = true; + } + + @Override + public void postGetNamespaceDescriptor(ObserverContext ctx, + NamespaceDescriptor ns) throws IOException { + postGetNamespaceDescriptorCalled = true; + } + + public boolean wasGetNamespaceDescriptorCalled() { + return preGetNamespaceDescriptorCalled && postGetNamespaceDescriptorCalled; + } + @Override public void preListNamespaceDescriptors(ObserverContext env, List descriptors) throws IOException { @@ -1357,6 +1378,8 @@ public class TestMasterObserver { assertTrue("Test namespace should be created", cp.wasCreateNamespaceCalled()); assertNotNull(admin.getNamespaceDescriptor(testNamespace)); + assertTrue("Test namespace descriptor should have been called", + cp.wasGetNamespaceDescriptorCalled()); // turn off bypass, run the tests again cp.enableBypass(true); @@ -1367,11 +1390,15 @@ public class TestMasterObserver { cp.preModifyNamespaceCalledOnly()); assertNotNull(admin.getNamespaceDescriptor(testNamespace)); + assertTrue("Test namespace descriptor should have been called", + cp.wasGetNamespaceDescriptorCalled()); admin.deleteNamespace(testNamespace); assertTrue("Test namespace should not have been deleted", cp.preDeleteNamespaceCalledOnly()); assertNotNull(admin.getNamespaceDescriptor(testNamespace)); + assertTrue("Test namespace descriptor should have been called", + cp.wasGetNamespaceDescriptorCalled()); cp.enableBypass(false); cp.resetStates(); 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 1c1cb6d8281..80f5a97e298 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 @@ -190,6 +190,21 @@ public class TestNamespaceCommands extends SecureTestUtil { verifyDenied(deleteNamespace, USER_NSP_WRITE, USER_CREATE, USER_RW); } + @Test + public void testGetNamespaceDescriptor() throws Exception { + AccessTestAction getNamespaceAction = new AccessTestAction() { + 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); + } + @Test public void testListNamespaces() throws Exception { AccessTestAction listAction = new AccessTestAction() {