From 369f731264d77617452e4074d15404bd62ec6093 Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Fri, 28 Jul 2017 00:50:08 -0700 Subject: [PATCH] HDFS-12062. removeErasureCodingPolicy needs super user permission. Contributed by Wei-Chiu Chuang. --- .../org/apache/hadoop/hdfs/DFSClient.java | 26 ++++++++-- .../server/namenode/NameNodeRpcServer.java | 1 + .../hdfs/TestDistributedFileSystem.java | 50 +++++++++++++++++++ .../hdfs/TestErasureCodingPolicies.java | 20 ++++++++ 4 files changed, 93 insertions(+), 4 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java index 8acda61199..677ea3545a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java @@ -2774,25 +2774,43 @@ public HashMap getErasureCodingCodecs() throws IOException { public AddECPolicyResponse[] addErasureCodingPolicies( ErasureCodingPolicy[] policies) throws IOException { checkOpen(); - return namenode.addErasureCodingPolicies(policies); + try (TraceScope ignored = tracer.newScope("addErasureCodingPolicies")) { + return namenode.addErasureCodingPolicies(policies); + } catch (RemoteException re) { + throw re.unwrapRemoteException(AccessControlException.class); + } } public void removeErasureCodingPolicy(String ecPolicyName) throws IOException { checkOpen(); - namenode.removeErasureCodingPolicy(ecPolicyName); + try (TraceScope ignored = tracer.newScope("removeErasureCodingPolicy")) { + namenode.removeErasureCodingPolicy(ecPolicyName); + } catch (RemoteException re) { + throw re.unwrapRemoteException(AccessControlException.class); + } } public void enableErasureCodingPolicy(String ecPolicyName) throws IOException { checkOpen(); - namenode.enableErasureCodingPolicy(ecPolicyName); + try (TraceScope ignored = tracer.newScope("enableErasureCodingPolicy")) { + namenode.enableErasureCodingPolicy(ecPolicyName); + } catch (RemoteException re) { + throw re.unwrapRemoteException(AccessControlException.class, + SafeModeException.class); + } } public void disableErasureCodingPolicy(String ecPolicyName) throws IOException { checkOpen(); - namenode.disableErasureCodingPolicy(ecPolicyName); + try (TraceScope ignored = tracer.newScope("disableErasureCodingPolicy")) { + namenode.disableErasureCodingPolicy(ecPolicyName); + } catch (RemoteException re) { + throw re.unwrapRemoteException(AccessControlException.class, + SafeModeException.class); + } } public DFSInotifyEventInputStream getInotifyEventStream() throws IOException { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java index 39d93dff96..9cd58cb46a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java @@ -2304,6 +2304,7 @@ public AddECPolicyResponse[] addErasureCodingPolicies( public void removeErasureCodingPolicy(String ecPolicyName) throws IOException { checkNNStartup(); + namesystem.checkSuperuserPrivilege(); namesystem.removeErasureCodingPolicy(ecPolicyName); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java index b35d374786..95256096e7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java @@ -93,6 +93,7 @@ import org.apache.hadoop.net.NetUtils; import org.apache.hadoop.net.ScriptBasedMapping; import org.apache.hadoop.net.StaticMapping; +import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.util.DataChecksum; @@ -1561,6 +1562,27 @@ public void testRemoveErasureCodingPolicy() throws Exception { fs.removeErasureCodingPolicy(policyName); assertEquals(policyName, ErasureCodingPolicyManager.getInstance(). getRemovedPolicies().get(0).getName()); + + // remove erasure coding policy as a user without privilege + UserGroupInformation fakeUGI = UserGroupInformation.createUserForTesting( + "ProbablyNotARealUserName", new String[] {"ShangriLa"}); + final MiniDFSCluster finalCluster = cluster; + fakeUGI.doAs(new PrivilegedExceptionAction() { + @Override + public Object run() throws Exception { + DistributedFileSystem fs = finalCluster.getFileSystem(); + try { + fs.removeErasureCodingPolicy(policyName); + fail(); + } catch (AccessControlException ace) { + GenericTestUtils.assertExceptionContains("Access denied for user " + + "ProbablyNotARealUserName. Superuser privilege is required", + ace); + } + return null; + } + }); + } finally { if (cluster != null) { cluster.shutdown(); @@ -1609,6 +1631,34 @@ public void testEnableAndDisableErasureCodingPolicy() throws Exception { GenericTestUtils.assertExceptionContains("does not exists", e); // pass } + + // disable and enable erasure coding policy as a user without privilege + UserGroupInformation fakeUGI = UserGroupInformation.createUserForTesting( + "ProbablyNotARealUserName", new String[] {"ShangriLa"}); + final MiniDFSCluster finalCluster = cluster; + fakeUGI.doAs(new PrivilegedExceptionAction() { + @Override + public Object run() throws Exception { + DistributedFileSystem fs = finalCluster.getFileSystem(); + try { + fs.disableErasureCodingPolicy(policyName); + fail(); + } catch (AccessControlException ace) { + GenericTestUtils.assertExceptionContains("Access denied for user " + + "ProbablyNotARealUserName. Superuser privilege is required", + ace); + } + try { + fs.enableErasureCodingPolicy(policyName); + fail(); + } catch (AccessControlException ace) { + GenericTestUtils.assertExceptionContains("Access denied for user " + + "ProbablyNotARealUserName. Superuser privilege is required", + ace); + } + return null; + } + }); } finally { if (cluster != null) { cluster.shutdown(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestErasureCodingPolicies.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestErasureCodingPolicies.java index f90a2f3128..127dad1e03 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestErasureCodingPolicies.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestErasureCodingPolicies.java @@ -693,5 +693,25 @@ public void testAddErasureCodingPolicies() throws Exception { assertTrue(responses[0].isSucceed()); assertEquals(SystemErasureCodingPolicies.getPolicies().size() + 1, ErasureCodingPolicyManager.getInstance().getPolicies().length); + + // add erasure coding policy as a user without privilege + UserGroupInformation fakeUGI = UserGroupInformation.createUserForTesting( + "ProbablyNotARealUserName", new String[] {"ShangriLa"}); + final ErasureCodingPolicy ecPolicy = newPolicy; + fakeUGI.doAs(new PrivilegedExceptionAction() { + @Override + public Object run() throws Exception { + DistributedFileSystem fs = cluster.getFileSystem(); + try { + fs.addErasureCodingPolicies(new ErasureCodingPolicy[]{ecPolicy}); + fail(); + } catch (AccessControlException ace) { + GenericTestUtils.assertExceptionContains("Access denied for user " + + "ProbablyNotARealUserName. Superuser privilege is required", + ace); + } + return null; + } + }); } }