diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/FSDownload.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/FSDownload.java index 4d69056a70f..627d565d4d0 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/FSDownload.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/FSDownload.java @@ -37,6 +37,7 @@ import org.apache.hadoop.fs.FileUtil; import org.apache.hadoop.fs.Options.Rename; import org.apache.hadoop.fs.Path; import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.util.RunJar; import org.apache.hadoop.yarn.api.records.LocalResource; @@ -90,6 +91,85 @@ public class FSDownload implements Callable { } } + /** + * Returns a boolean to denote whether a cache file is visible to all(public) + * or not + * @param conf + * @param uri + * @return true if the path in the uri is visible to all, false otherwise + * @throws IOException + */ + private static boolean isPublic(FileSystem fs, Path current) throws IOException { + current = fs.makeQualified(current); + //the leaf level file should be readable by others + if (!checkPublicPermsForAll(fs, current, FsAction.READ_EXECUTE, FsAction.READ)) { + return false; + } + return ancestorsHaveExecutePermissions(fs, current.getParent()); + } + + private static boolean checkPublicPermsForAll(FileSystem fs, Path current, + FsAction dir, FsAction file) + throws IOException { + return checkPublicPermsForAll(fs, fs.getFileStatus(current), dir, file); + } + + private static boolean checkPublicPermsForAll(FileSystem fs, + FileStatus status, FsAction dir, FsAction file) + throws IOException { + FsPermission perms = status.getPermission(); + FsAction otherAction = perms.getOtherAction(); + if (status.isDirectory()) { + if (!otherAction.implies(dir)) { + return false; + } + + for (FileStatus child : fs.listStatus(status.getPath())) { + if(!checkPublicPermsForAll(fs, child, dir, file)) { + return false; + } + } + return true; + } + return (otherAction.implies(file)); + } + + /** + * Returns true if all ancestors of the specified path have the 'execute' + * permission set for all users (i.e. that other users can traverse + * the directory heirarchy to the given path) + */ + private static boolean ancestorsHaveExecutePermissions(FileSystem fs, Path path) + throws IOException { + Path current = path; + while (current != null) { + //the subdirs in the path should have execute permissions for others + if (!checkPermissionOfOther(fs, current, FsAction.EXECUTE)) { + return false; + } + current = current.getParent(); + } + return true; + } + + /** + * Checks for a given path whether the Other permissions on it + * imply the permission in the passed FsAction + * @param fs + * @param path + * @param action + * @return true if the path in the uri is visible to all, false otherwise + * @throws IOException + */ + private static boolean checkPermissionOfOther(FileSystem fs, Path path, + FsAction action) throws IOException { + FileStatus status = fs.getFileStatus(path); + FsPermission perms = status.getPermission(); + FsAction otherAction = perms.getOtherAction(); + return otherAction.implies(action); + } + + private Path copy(Path sCopy, Path dstdir) throws IOException { FileSystem sourceFs = sCopy.getFileSystem(conf); Path dCopy = new Path(dstdir, sCopy.getName() + ".tmp"); @@ -99,7 +179,14 @@ public class FSDownload implements Callable { " changed on src filesystem (expected " + resource.getTimestamp() + ", was " + sStat.getModificationTime()); } - + if (resource.getVisibility() == LocalResourceVisibility.PUBLIC) { + if (!isPublic(sourceFs, sCopy)) { + throw new IOException("Resource " + sCopy + + " is not publicly accessable and as such cannot be part of the" + + " public cache."); + } + } + sourceFs.copyToLocalFile(sCopy, dCopy); return dCopy; } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestFSDownload.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestFSDownload.java index 25adf317016..c29b69f1148 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestFSDownload.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestFSDownload.java @@ -113,6 +113,54 @@ public class TestFSDownload { return ret; } + @Test + public void testDownloadBadPublic() throws IOException, URISyntaxException, + InterruptedException { + Configuration conf = new Configuration(); + conf.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY, "077"); + FileContext files = FileContext.getLocalFSFileContext(conf); + final Path basedir = files.makeQualified(new Path("target", + TestFSDownload.class.getSimpleName())); + files.mkdir(basedir, null, true); + conf.setStrings(TestFSDownload.class.getName(), basedir.toString()); + + Map rsrcVis = + new HashMap(); + + Random rand = new Random(); + long sharedSeed = rand.nextLong(); + rand.setSeed(sharedSeed); + System.out.println("SEED: " + sharedSeed); + + Map> pending = + new HashMap>(); + ExecutorService exec = Executors.newSingleThreadExecutor(); + LocalDirAllocator dirs = + new LocalDirAllocator(TestFSDownload.class.getName()); + int size = 512; + LocalResourceVisibility vis = LocalResourceVisibility.PUBLIC; + Path path = new Path(basedir, "test-file"); + LocalResource rsrc = createFile(files, path, size, rand, vis); + rsrcVis.put(rsrc, vis); + Path destPath = dirs.getLocalPathForWrite( + basedir.toString(), size, conf); + FSDownload fsd = + new FSDownload(files, UserGroupInformation.getCurrentUser(), conf, + destPath, rsrc, new Random(sharedSeed)); + pending.put(rsrc, exec.submit(fsd)); + + try { + for (Map.Entry> p : pending.entrySet()) { + p.getValue().get(); + Assert.fail("We localized a file that is not public."); + } + } catch (ExecutionException e) { + Assert.assertTrue(e.getCause() instanceof IOException); + } finally { + exec.shutdown(); + } + } + @Test public void testDownload() throws IOException, URISyntaxException, InterruptedException { @@ -140,14 +188,9 @@ public class TestFSDownload { int[] sizes = new int[10]; for (int i = 0; i < 10; ++i) { sizes[i] = rand.nextInt(512) + 512; - LocalResourceVisibility vis = LocalResourceVisibility.PUBLIC; - switch (i%3) { - case 1: - vis = LocalResourceVisibility.PRIVATE; - break; - case 2: + LocalResourceVisibility vis = LocalResourceVisibility.PRIVATE; + if (i%2 == 1) { vis = LocalResourceVisibility.APPLICATION; - break; } Path p = new Path(basedir, "" + i); LocalResource rsrc = createFile(files, p, sizes[i], rand, vis); @@ -176,17 +219,8 @@ public class TestFSDownload { System.out.println("File permission " + perm + " for rsrc vis " + p.getKey().getVisibility().name()); assert(rsrcVis.containsKey(p.getKey())); - switch (rsrcVis.get(p.getKey())) { - case PUBLIC: - Assert.assertTrue("Public file should be 555", - perm.toShort() == FSDownload.PUBLIC_FILE_PERMS.toShort()); - break; - case PRIVATE: - case APPLICATION: - Assert.assertTrue("Private file should be 500", - perm.toShort() == FSDownload.PRIVATE_FILE_PERMS.toShort()); - break; - } + Assert.assertTrue("Private file should be 500", + perm.toShort() == FSDownload.PRIVATE_FILE_PERMS.toShort()); } } catch (ExecutionException e) { throw new IOException("Failed exec", e); @@ -250,14 +284,9 @@ public class TestFSDownload { LocalDirAllocator dirs = new LocalDirAllocator(TestFSDownload.class.getName()); for (int i = 0; i < 5; ++i) { - LocalResourceVisibility vis = LocalResourceVisibility.PUBLIC; - switch (rand.nextInt()%3) { - case 1: - vis = LocalResourceVisibility.PRIVATE; - break; - case 2: + LocalResourceVisibility vis = LocalResourceVisibility.PRIVATE; + if (i%2 == 1) { vis = LocalResourceVisibility.APPLICATION; - break; } Path p = new Path(basedir, "dir" + i + ".jar");