commit 3ef6001f437200a47caba89304f37a2623ce8782 Author: Eric Yang Date: Thu Feb 1 15:04:52 2018 -0500 YARN-7221. Added security check for user privileges to run privileged docker. (Contributed by Eric Yang) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c index 597af35..1e1c0df 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c @@ -16,6 +16,9 @@ * limitations under the License. */ +#include +#include +#include #include #include #include @@ -1087,22 +1090,67 @@ static int add_rw_mounts(const struct configuration *command_config, const stru return add_mounts(command_config, conf, "rw-mounts", 0, out, outlen); } +static int check_privileges(const char *user) { + FILE *fp = NULL; + int statval = 0; + char tmpl[] = "id -G -n %s"; + char buffer[4096]; + if (fork()==0) { + char *cmd = (char *) alloc_and_clear_memory(strlen(tmpl) + strlen(user), sizeof(char)); + sprintf(cmd, tmpl, user); + fp = popen(cmd, "r"); + if (fp == NULL) { + exit(127); + } + while (fgets(buffer, sizeof(buffer)-1, fp) != NULL) { + for (char *p = strtok(buffer," "); p != NULL; p = strtok(NULL, " ")) { + if (strcmp(p, "docker") || strcmp(p, "root")) { + exit(0); + } + } + } + pclose(fp); + free(cmd); + execl("/bin/sudo", "sudo", "-U", user, "-n", "-l", "docker", NULL); + exit(127); + } else { + wait(&statval); + if (WIFEXITED(statval)) { + if (WEXITSTATUS(statval)==0) { + return 1; + } + } + } + return 0; +} + static int set_privileged(const struct configuration *command_config, const struct configuration *conf, char *out, const size_t outlen) { size_t tmp_buffer_size = 1024; + char *user = NULL; char *tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char)); char *value = get_configuration_value("privileged", DOCKER_COMMAND_FILE_SECTION, command_config); char *privileged_container_enabled = get_configuration_value("docker.privileged-containers.enabled", CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf); int ret = 0; + int allowed = 0; + + user = get_configuration_value("user", DOCKER_COMMAND_FILE_SECTION, command_config); + if (user == NULL) { + return INVALID_DOCKER_USER_NAME; + } + if (value != NULL && strcasecmp(value, "true") == 0 ) { if (privileged_container_enabled != NULL) { if (strcmp(privileged_container_enabled, "1") == 0 || strcasecmp(privileged_container_enabled, "True") == 0) { - ret = add_to_buffer(out, outlen, "--privileged "); - if (ret != 0) { - ret = BUFFER_TOO_SMALL; + allowed = check_privileges(user); + if (allowed) { + ret = add_to_buffer(out, outlen, "--privileged "); + if (ret != 0) { + ret = BUFFER_TOO_SMALL; + } } } else { fprintf(ERRORFILE, "Privileged containers are disabled\n"); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/utils/test_docker_util.cc b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/utils/test_docker_util.cc index 3651fe0..8edd291 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/utils/test_docker_util.cc +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/utils/test_docker_util.cc @@ -619,8 +619,8 @@ namespace ContainerExecutor { FAIL(); } ret = set_privileged(&cmd_cfg, &container_cfg, buff, buff_len); - ASSERT_EQ(0, ret); - ASSERT_STREQ(itr->second.c_str(), buff); + ASSERT_EQ(6, ret); + ASSERT_EQ(0, strlen(buff)); } } @@ -634,9 +634,7 @@ namespace ContainerExecutor { } file_cmd_vec.clear(); file_cmd_vec.push_back(std::make_pair( - "[docker-command-execution]\n docker-command=run\n privileged=false", "")); - file_cmd_vec.push_back(std::make_pair( - "[docker-command-execution]\n docker-command=run", "")); + "[docker-command-execution]\n docker-command=run\n user=root\n privileged=false", "")); for (itr = file_cmd_vec.begin(); itr != file_cmd_vec.end(); ++itr) { memset(buff, 0, buff_len); write_command_file(itr->first); @@ -648,7 +646,7 @@ namespace ContainerExecutor { ASSERT_EQ(0, ret); ASSERT_STREQ(itr->second.c_str(), buff); } - write_command_file("[docker-command-execution]\n docker-command=run\n privileged=true"); + write_command_file("[docker-command-execution]\n docker-command=run\n user=root\n privileged=true"); ret = read_config(docker_command_file.c_str(), &cmd_cfg); if (ret != 0) { FAIL(); @@ -1034,12 +1032,12 @@ namespace ContainerExecutor { file_cmd_vec.push_back(std::make_pair( "[docker-command-execution]\n" - " docker-command=run\n name=container_e1_12312_11111_02_000001\n image=docker-image\n user=test\n hostname=host-id\n" + " docker-command=run\n name=container_e1_12312_11111_02_000001\n image=docker-image\n user=root\n hostname=host-id\n" " ro-mounts=/var/log:/var/log,/var/lib:/lib,/usr/bin/cut:/usr/bin/cut\n rw-mounts=/tmp:/tmp\n" " network=bridge\n devices=/dev/test:/dev/test\n net=bridge\n privileged=true\n" " cap-add=CHOWN,SETUID\n cgroup-parent=ctr-cgroup\n detach=true\n rm=true\n" " launch-command=bash,test_script.sh,arg1,arg2", - "run --name='container_e1_12312_11111_02_000001' --user='test' -d --rm --net='bridge' -v '/var/log:/var/log:ro' -v '/var/lib:/lib:ro'" + "run --name='container_e1_12312_11111_02_000001' --user='root' -d --rm --net='bridge' -v '/var/log:/var/log:ro' -v '/var/lib:/lib:ro'" " -v '/usr/bin/cut:/usr/bin/cut:ro' -v '/tmp:/tmp' --cgroup-parent='ctr-cgroup' --privileged --cap-drop='ALL' " "--cap-add='CHOWN' --cap-add='SETUID' --hostname='host-id' --device='/dev/test:/dev/test' 'docker-image' " "'bash' 'test_script.sh' 'arg1' 'arg2' ")); @@ -1047,12 +1045,12 @@ namespace ContainerExecutor { file_cmd_vec.push_back(std::make_pair( "[docker-command-execution]\n" - " docker-command=run\n name=container_e1_12312_11111_02_000001\n image=docker-image\n user=test\n hostname=host-id\n" + " docker-command=run\n name=container_e1_12312_11111_02_000001\n image=docker-image\n user=root\n hostname=host-id\n" " ro-mounts=/var/log:/var/log,/var/lib:/lib,/usr/bin/cut:/usr/bin/cut\n rw-mounts=/tmp:/tmp\n" " network=bridge\n devices=/dev/test:/dev/test\n net=bridge\n privileged=true\n" " cap-add=CHOWN,SETUID\n cgroup-parent=ctr-cgroup\n detach=true\n rm=true\n group-add=1000,1001\n" " launch-command=bash,test_script.sh,arg1,arg2", - "run --name='container_e1_12312_11111_02_000001' --user='test' -d --rm --net='bridge' -v '/var/log:/var/log:ro' -v '/var/lib:/lib:ro'" + "run --name='container_e1_12312_11111_02_000001' --user='root' -d --rm --net='bridge' -v '/var/log:/var/log:ro' -v '/var/lib:/lib:ro'" " -v '/usr/bin/cut:/usr/bin/cut:ro' -v '/tmp:/tmp' --cgroup-parent='ctr-cgroup' --privileged --cap-drop='ALL' " "--cap-add='CHOWN' --cap-add='SETUID' --hostname='host-id' --group-add '1000' --group-add '1001' " "--device='/dev/test:/dev/test' 'docker-image' 'bash' 'test_script.sh' 'arg1' 'arg2' "));