Repository: hadoop
Updated Branches:
refs/heads/branch-2.8.2 c17a66967 -> 441295465
YARN-4731. container-executor should not follow symlinks in
recursive_unlink_children. Contributed by Colin Patrick McCabe
(cherry picked from commit c58a6d53c58209a8f78ff64e04e9112933489fb5)
Conflicts:
hadoop-yarn-project/CHANGES.txt
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/44129546
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/44129546
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/44129546
Branch: refs/heads/branch-2.8.2
Commit: 441295465fb59a851e30cd6c984f58b06172d3dc
Parents: c17a669
Author: Jason Lowe <[email protected]>
Authored: Mon Feb 29 15:24:35 2016 +0000
Committer: Jason Lowe <[email protected]>
Committed: Fri Jul 21 14:50:23 2017 -0500
----------------------------------------------------------------------
.../impl/container-executor.c | 54 ++++++++++-
.../test/test-container-executor.c | 99 +++++++++++++++++++-
2 files changed, 150 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/44129546/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
----------------------------------------------------------------------
diff --git
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
index cd6b99b..48eb33e 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
@@ -1811,9 +1811,9 @@ static int rmdir_as_nm(const char* path) {
static int open_helper(int dirfd, const char *name) {
int fd;
if (dirfd >= 0) {
- fd = openat(dirfd, name, O_RDONLY);
+ fd = openat(dirfd, name, O_RDONLY | O_NOFOLLOW);
} else {
- fd = open(name, O_RDONLY);
+ fd = open(name, O_RDONLY | O_NOFOLLOW);
}
if (fd >= 0) {
return fd;
@@ -1847,6 +1847,34 @@ static int unlink_helper(int dirfd, const char *name,
int flags) {
return errno;
}
+/**
+ * Determine if an entry in a directory is a symlink.
+ *
+ * @param dirfd The directory file descriptor, or -1 if there is none.
+ * @param name If dirfd is -1, this is the path to examine.
+ * Otherwise, this is the file name in the directory to
+ * examine.
+ *
+ * @return 0 if the entry is not a symlink
+ * 1 if the entry is a symlink
+ * A negative errno code if we couldn't access the entry.
+ */
+static int is_symlink_helper(int dirfd, const char *name)
+{
+ struct stat stat;
+
+ if (dirfd < 0) {
+ if (lstat(name, &stat) < 0) {
+ return -errno;
+ }
+ } else {
+ if (fstatat(dirfd, name, &stat, AT_SYMLINK_NOFOLLOW) < 0) {
+ return -errno;
+ }
+ }
+ return !!S_ISLNK(stat.st_mode);
+}
+
static int recursive_unlink_helper(int dirfd, const char *name,
const char* fullpath)
{
@@ -1854,6 +1882,28 @@ static int recursive_unlink_helper(int dirfd, const char
*name,
DIR *dfd = NULL;
struct stat stat;
+ // Check to see if the file is a symlink. If so, delete the symlink rather
+ // than what it points to.
+ ret = is_symlink_helper(dirfd, name);
+ if (ret < 0) {
+ // is_symlink_helper failed.
+ ret = -ret;
+ fprintf(LOGFILE, "is_symlink_helper(%s) failed: %s\n",
+ fullpath, strerror(ret));
+ goto done;
+ } else if (ret == 1) {
+ // is_symlink_helper determined that the path is a symlink.
+ ret = unlink_helper(dirfd, name, 0);
+ if (ret) {
+ fprintf(LOGFILE, "failed to unlink symlink %s: %s\n",
+ fullpath, strerror(ret));
+ }
+ goto done;
+ }
+
+ // Open the file. We use O_NOFOLLOW here to ensure that we if a symlink was
+ // swapped in by an attacker, we will fail to follow it rather than deleting
+ // something we potentially should not.
fd = open_helper(dirfd, name);
if (fd == -EACCES) {
ret = chmod_helper(dirfd, name, 0700);
http://git-wip-us.apache.org/repos/asf/hadoop/blob/44129546/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
----------------------------------------------------------------------
diff --git
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
index ff00a8b..286c135 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
@@ -714,6 +714,100 @@ void test_run_container() {
check_pid_file(cgroups_pids[1], child);
}
+static void mkdir_or_die(const char *path) {
+ if (mkdir(path, 0777) < 0) {
+ int err = errno;
+ printf("mkdir(%s) failed: %s\n", path, strerror(err));
+ exit(1);
+ }
+}
+
+static void touch_or_die(const char *path) {
+ FILE* f = fopen(path, "w");
+ if (!f) {
+ int err = errno;
+ printf("fopen(%s, w) failed: %s\n", path, strerror(err));
+ exit(1);
+ }
+ if (fclose(f) < 0) {
+ int err = errno;
+ printf("fclose(%s) failed: %s\n", path, strerror(err));
+ exit(1);
+ }
+}
+
+static void symlink_or_die(const char *old, const char *new) {
+ if (symlink(old, new) < 0) {
+ int err = errno;
+ printf("symlink(%s, %s) failed: %s\n", old, new, strerror(err));
+ exit(1);
+ }
+}
+
+static void expect_type(const char *path, int mode) {
+ struct stat st_buf;
+
+ if (stat(path, &st_buf) < 0) {
+ int err = errno;
+ if (err == ENOENT) {
+ if (mode == 0) {
+ return;
+ }
+ printf("expect_type(%s): stat failed unexpectedly: %s\n",
+ path, strerror(err));
+ exit(1);
+ }
+ }
+ if (mode == 0) {
+ printf("expect_type(%s): we expected the file to be gone, but it "
+ "existed.\n", path);
+ exit(1);
+ }
+ if (!(st_buf.st_mode & mode)) {
+ printf("expect_type(%s): the file existed, but it had mode 0%4o, "
+ "which didn't have bit 0%4o\n", path, st_buf.st_mode, mode);
+ exit(1);
+ }
+}
+
+int recursive_unlink_children(const char *name);
+
+void test_recursive_unlink_children() {
+ int ret;
+
+ mkdir_or_die(TEST_ROOT "/unlinkRoot");
+ mkdir_or_die(TEST_ROOT "/unlinkRoot/a");
+ touch_or_die(TEST_ROOT "/unlinkRoot/b");
+ mkdir_or_die(TEST_ROOT "/unlinkRoot/c");
+ touch_or_die(TEST_ROOT "/unlinkRoot/c/d");
+ touch_or_die(TEST_ROOT "/external");
+ symlink_or_die(TEST_ROOT "/external",
+ TEST_ROOT "/unlinkRoot/c/external");
+ ret = recursive_unlink_children(TEST_ROOT "/unlinkRoot");
+ if (ret != 0) {
+ printf("recursive_unlink_children(%s) failed: error %d\n",
+ TEST_ROOT "/unlinkRoot", ret);
+ exit(1);
+ }
+ // unlinkRoot should still exist.
+ expect_type(TEST_ROOT "/unlinkRoot", S_IFDIR);
+ // Other files under unlinkRoot should have been deleted.
+ expect_type(TEST_ROOT "/unlinkRoot/a", 0);
+ expect_type(TEST_ROOT "/unlinkRoot/b", 0);
+ expect_type(TEST_ROOT "/unlinkRoot/c", 0);
+ // We shouldn't have followed the symlink.
+ expect_type(TEST_ROOT "/external", S_IFREG);
+ // Clean up.
+ if (rmdir(TEST_ROOT "/unlinkRoot") < 0) {
+ int err = errno;
+ printf("failed to rmdir " TEST_ROOT "/unlinkRoot: %s\n", strerror(err));
+ }
+ if (unlink(TEST_ROOT "/external") < 0) {
+ int err = errno;
+ printf("failed to unlink " TEST_ROOT "/external: %s\n", strerror(err));
+ }
+}
+
void test_sanitize_docker_command() {
char *input[] = {
@@ -772,7 +866,7 @@ int main(int argc, char **argv) {
if (mkdirs(TEST_ROOT "/logs/userlogs", 0755) != 0) {
exit(1);
}
-
+
if (write_config_file(TEST_ROOT "/test.cfg", 1) != 0) {
exit(1);
}
@@ -804,6 +898,9 @@ int main(int argc, char **argv) {
printf("\nStarting tests\n");
+ printf("\nTesting recursive_unlink_children()\n");
+ test_recursive_unlink_children();
+
printf("\nTesting resolve_config_path()\n");
test_resolve_config_path();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
