Repository: hadoop
Updated Branches:
refs/heads/trunk f659485ee -> ea724181d
YARN-9132. Added file permission check for auxiliary services manifest file.
Contributed by Billie Rinaldi
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/ea724181
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/ea724181
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/ea724181
Branch: refs/heads/trunk
Commit: ea724181d66ebe3c2cc7ed071948e9bc463bf223
Parents: f659485
Author: Eric Yang <[email protected]>
Authored: Fri Dec 21 14:56:39 2018 -0500
Committer: Eric Yang <[email protected]>
Committed: Fri Dec 21 14:56:39 2018 -0500
----------------------------------------------------------------------
.../containermanager/AuxServices.java | 43 ++++++++++++++++-
.../containermanager/TestAuxServices.java | 51 ++++++++++++++++++++
2 files changed, 92 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/ea724181/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java
----------------------------------------------------------------------
diff --git
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java
index ba936d6..59a76da 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java
@@ -41,6 +41,7 @@ import java.util.regex.Pattern;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.security.authorize.AccessControlList;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.records.AuxServiceConfiguration;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.records.AuxServiceFile;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.records.AuxServiceRecord;
@@ -481,6 +482,35 @@ public class AuxServices extends AbstractService
loadManifest(getConfig(), true);
}
+ private boolean checkManifestPermissions(FileStatus status) throws
+ IOException {
+ if ((status.getPermission().toShort() & 0022) != 0) {
+ LOG.error("Manifest file and parents must not be writable by group or " +
+ "others. The current Permission of " + status.getPath() + " is " +
+ status.getPermission());
+ return false;
+ }
+ Path parent = status.getPath().getParent();
+ if (parent == null) {
+ return true;
+ }
+ return checkManifestPermissions(manifestFS.getFileStatus(parent));
+ }
+
+ private boolean checkManifestOwnerAndPermissions(FileStatus status) throws
+ IOException {
+ AccessControlList yarnAdminAcl = new AccessControlList(getConfig().get(
+ YarnConfiguration.YARN_ADMIN_ACL,
+ YarnConfiguration.DEFAULT_YARN_ADMIN_ACL));
+ if (!yarnAdminAcl.isUserAllowed(
+ UserGroupInformation.createRemoteUser(status.getOwner()))) {
+ LOG.error("Manifest must be owned by YARN admin: " + manifest);
+ return false;
+ }
+
+ return checkManifestPermissions(status);
+ }
+
/**
* Reads the manifest file if it is configured, exists, and has not been
* modified since the last read.
@@ -494,14 +524,20 @@ public class AuxServices extends AbstractService
return null;
}
if (!manifestFS.exists(manifest)) {
- LOG.info("Manifest file " + manifest + " doesn't exist");
+ LOG.warn("Manifest file " + manifest + " doesn't exist");
return null;
}
FileStatus status;
try {
status = manifestFS.getFileStatus(manifest);
} catch (FileNotFoundException e) {
- LOG.info("Manifest file " + manifest + " doesn't exist");
+ LOG.warn("Manifest file " + manifest + " doesn't exist");
+ return null;
+ }
+ if (!status.isFile()) {
+ LOG.warn("Manifest file " + manifest + " is not a file");
+ }
+ if (!checkManifestOwnerAndPermissions(status)) {
return null;
}
if (status.getModificationTime() == manifestModifyTS) {
@@ -721,6 +757,9 @@ public class AuxServices extends AbstractService
serviceMap.clear();
serviceRecordMap.clear();
serviceMetaData.clear();
+ if (manifestFS != null) {
+ manifestFS.close();
+ }
if (manifestReloadTimer != null) {
manifestReloadTimer.cancel();
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/ea724181/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestAuxServices.java
----------------------------------------------------------------------
diff --git
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestAuxServices.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestAuxServices.java
index ece8d9b..1f944b4 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestAuxServices.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestAuxServices.java
@@ -34,9 +34,11 @@ import static org.mockito.Mockito.verify;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hadoop.security.UserGroupInformation;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.records.AuxServiceRecord;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.records.AuxServiceRecords;
import org.junit.After;
+import org.junit.Assume;
import org.junit.Before;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -834,4 +836,53 @@ public class TestAuxServices {
aux.stop();
}
+
+ @Test
+ public void testAuxServicesManifestPermissions() throws IOException {
+ Assume.assumeTrue(useManifest);
+ Configuration conf = getABConf();
+ FileSystem fs = FileSystem.get(conf);
+ fs.setPermission(new Path(manifest.getAbsolutePath()), FsPermission
+ .createImmutable((short) 0777));
+ AuxServices aux = new AuxServices(MOCK_AUX_PATH_HANDLER,
+ MOCK_CONTEXT, MOCK_DEL_SERVICE);
+ aux.init(conf);
+ assertEquals(0, aux.getServices().size());
+
+ fs.setPermission(new Path(manifest.getAbsolutePath()), FsPermission
+ .createImmutable((short) 0775));
+ aux = new AuxServices(MOCK_AUX_PATH_HANDLER,
+ MOCK_CONTEXT, MOCK_DEL_SERVICE);
+ aux.init(conf);
+ assertEquals(0, aux.getServices().size());
+
+ fs.setPermission(new Path(manifest.getAbsolutePath()), FsPermission
+ .createImmutable((short) 0755));
+ fs.setPermission(new Path(rootDir.getAbsolutePath()), FsPermission
+ .createImmutable((short) 0775));
+ aux = new AuxServices(MOCK_AUX_PATH_HANDLER,
+ MOCK_CONTEXT, MOCK_DEL_SERVICE);
+ aux.init(conf);
+ assertEquals(0, aux.getServices().size());
+
+ fs.setPermission(new Path(rootDir.getAbsolutePath()), FsPermission
+ .createImmutable((short) 0755));
+ aux = new AuxServices(MOCK_AUX_PATH_HANDLER,
+ MOCK_CONTEXT, MOCK_DEL_SERVICE);
+ aux.init(conf);
+ assertEquals(2, aux.getServices().size());
+
+ conf.set(YarnConfiguration.YARN_ADMIN_ACL, "");
+ aux = new AuxServices(MOCK_AUX_PATH_HANDLER,
+ MOCK_CONTEXT, MOCK_DEL_SERVICE);
+ aux.init(conf);
+ assertEquals(0, aux.getServices().size());
+
+ conf.set(YarnConfiguration.YARN_ADMIN_ACL, UserGroupInformation
+ .getCurrentUser().getShortUserName());
+ aux = new AuxServices(MOCK_AUX_PATH_HANDLER,
+ MOCK_CONTEXT, MOCK_DEL_SERVICE);
+ aux.init(conf);
+ assertEquals(2, aux.getServices().size());
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
