commit bd5f6e30450bca8e6fb48f037c39e1192ad7617d Author: Eric Yang Date: Mon Oct 23 12:58:35 2017 -0400 YARN-7197. Added black list mounts check for container executor. 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 e8e2b9e..6bbc9d1 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 @@ -701,6 +701,37 @@ static int check_mount_permitted(const char **permitted_mounts, const char *requ return ret; } +/** + * Check if the string starts with the prefix. + */ +static int starts_with(const char *pre, const char *str) { + size_t lenpre = strlen(pre), + lenstr = strlen(str); + return lenstr < lenpre ? 0 : strncmp(pre, str, lenpre) == 0; +} + +/** + * Check mount points against banned path. + */ +static int check_banned_mounts(const char **banned_mounts, const char *requested) { + int i = 0, ret = 0; + if (banned_mounts == NULL) { + return 0; + } + char *normalized_path = normalize_mount(requested); + if (normalized_path == NULL) { + return 1; + } + for (i = 0; banned_mounts[i] != NULL; ++i) { + if (starts_with(banned_mounts[i], normalized_path)) { + ret = 1; + break; + } + } + free(normalized_path); + return ret; +} + static char* get_mount_source(const char *mount) { char *src_mount = NULL; const char *tmp = NULL; @@ -713,9 +744,23 @@ static char* get_mount_source(const char *mount) { return src_mount; } +static char* get_mount_target(const char *mount) { + char *target_mount = (char *) alloc_and_clear_memory(strlen(mount), sizeof(char)); + const char *tmp = NULL; + tmp = strchr(mount, ':'); + if (tmp == NULL) { + fprintf(ERRORFILE, "Invalid docker mount '%s'\n", mount); + return NULL; + } + strncpy(target_mount, tmp + 1, strlen(tmp)); + return target_mount; +} + static int add_mounts(const struct configuration *command_config, const struct configuration *conf, const char *key, const int ro, char *out, const size_t outlen) { size_t tmp_buffer_size = 1024; + const char *bind_mount = "type=bind,source=%s,target=%s,readonly"; + const char *empty_dir = "/var/empty/sshd"; const char *ro_suffix = ""; const char *tmp_path_buffer[2] = {NULL, NULL}; char *tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char)); @@ -723,10 +768,12 @@ static int add_mounts(const struct configuration *command_config, const struct c CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf, ","); char **permitted_rw_mounts = get_configuration_values_delimiter("docker.allowed.rw-mounts", CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf, ","); + char **banned_mounts = get_configuration_values_delimiter("docker.banned.mounts", + CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf, ","); char **values = get_configuration_values_delimiter(key, DOCKER_COMMAND_FILE_SECTION, command_config, ","); - char *tmp_buffer_2 = NULL, *mount_src = NULL; + char *tmp_buffer_2 = NULL, *mount_src = NULL, *mount_target = NULL; const char *container_executor_cfg_path = normalize_mount(get_config_path("")); - int i = 0, permitted_rw = 0, permitted_ro = 0, ret = 0; + int i = 0, permitted_rw = 0, permitted_ro = 0, banned = 0, ret = 0; if (ro != 0) { ro_suffix = ":ro"; } @@ -739,6 +786,13 @@ static int add_mounts(const struct configuration *command_config, const struct c ret = MOUNT_ACCESS_ERROR; goto free_and_exit; } + ret = normalize_mounts(banned_mounts); + if (ret != 0) { + fprintf(ERRORFILE, "Unable to find banned docker mounts on disk\n"); + ret = MOUNT_ACCESS_ERROR; + goto free_and_exit; + } + for (i = 0; values[i] != NULL; ++i) { mount_src = get_mount_source(values[i]); if (mount_src == NULL) { @@ -748,6 +802,7 @@ static int add_mounts(const struct configuration *command_config, const struct c } permitted_rw = check_mount_permitted((const char **) permitted_rw_mounts, mount_src); permitted_ro = check_mount_permitted((const char **) permitted_ro_mounts, mount_src); + banned = check_banned_mounts((const char **) banned_mounts, mount_src); if (permitted_ro == -1 || permitted_rw == -1) { fprintf(ERRORFILE, "Invalid docker mount '%s', realpath=%s\n", values[i], mount_src); ret = INVALID_DOCKER_MOUNT; @@ -780,10 +835,19 @@ static int add_mounts(const struct configuration *command_config, const struct c ret = INVALID_DOCKER_RO_MOUNT; goto free_and_exit; } - tmp_buffer_2 = (char *) alloc_and_clear_memory(strlen(values[i]) + strlen(ro_suffix) + 1, sizeof(char)); - strncpy(tmp_buffer_2, values[i], strlen(values[i])); - strncpy(tmp_buffer_2 + strlen(values[i]), ro_suffix, strlen(ro_suffix)); - quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, "-v ", tmp_buffer_2); + if (banned) { + // Replace black list path with empty directory. + mount_target = get_mount_target(values[i]); + tmp_buffer_2 = (char *) alloc_and_clear_memory(strlen(empty_dir) + strlen(mount_target) + strlen(bind_mount), sizeof(char)); + sprintf(tmp_buffer_2, "type=bind,source=%s,target=%s,readonly", empty_dir, mount_target); + quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, "--mount ", tmp_buffer_2); + free(mount_target); + } else { + tmp_buffer_2 = (char *) alloc_and_clear_memory(strlen(values[i]) + strlen(ro_suffix) + 1, sizeof(char)); + strncpy(tmp_buffer_2, values[i], strlen(values[i])); + strncpy(tmp_buffer_2 + strlen(values[i]), ro_suffix, strlen(ro_suffix)); + quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, "-v ", tmp_buffer_2); + } ret = add_to_buffer(out, outlen, tmp_buffer); free(tmp_buffer_2); free(mount_src); @@ -800,6 +864,7 @@ static int add_mounts(const struct configuration *command_config, const struct c free_and_exit: free_values(permitted_ro_mounts); free_values(permitted_rw_mounts); + free_values(banned_mounts); free_values(values); free(mount_src); free((void *) container_executor_cfg_path); 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 c42cd78..e23c990 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 @@ -445,6 +445,22 @@ namespace ContainerExecutor { } } + TEST_F(TestDockerUtil, test_check_banned_mounts) { + const char *banned_mounts[] = {"/sys/", "/run/utmp", "/etc/shadow", NULL}; + std::vector > test_data; + test_data.push_back(std::make_pair("/sys", 1)); + test_data.push_back(std::make_pair("/sys/", 1)); + test_data.push_back(std::make_pair("/var/run/utmp", 1)); + test_data.push_back(std::make_pair("/usr/", 0)); + test_data.push_back(std::make_pair("/etc/shadow", 1)); + + std::vector >::const_iterator itr; + for (itr = test_data.begin(); itr != test_data.end(); ++itr) { + int ret = check_banned_mounts(banned_mounts, itr->first.c_str()); + ASSERT_EQ(itr->second, ret) << "for input " << itr->first; + } + } + TEST_F(TestDockerUtil, test_normalize_mounts) { const int entries = 4; const char *permitted_mounts[] = {"/home", "/etc", "/usr/bin/touch", NULL}; @@ -1000,12 +1016,12 @@ namespace ContainerExecutor { std::string container_executor_contents[] = {"[docker]\n docker.allowed.ro-mounts=/var,/etc,/usr/bin/touch\n" " docker.allowed.rw-mounts=/tmp\n docker.allowed.networks=bridge\n " " docker.allowed.capabilities=CHOWN,SETUID\n" - " docker.allowed.devices=/dev/test", + " docker.allowed.devices=/dev/test\n", "[docker]\n docker.allowed.ro-mounts=/var,/etc,/usr/bin/touch\n" " docker.allowed.rw-mounts=/tmp\n docker.allowed.networks=bridge\n " " docker.allowed.capabilities=CHOWN,SETUID\n" " privileged=0\n" - " docker.allowed.devices=/dev/test"}; + " docker.allowed.devices=/dev/test\n"}; for (int i = 0; i < 2; ++i) { write_file(container_executor_cfg_file, container_executor_contents[i]); int ret = read_config(container_executor_cfg_file.c_str(), &container_executor_cfg); @@ -1065,6 +1081,52 @@ namespace ContainerExecutor { } } + TEST_F(TestDockerUtil, test_docker_run_banned_mounts) { + + std::string container_executor_contents = "[docker]\n docker.allowed.ro-mounts=/var,/run,/usr/bin/touch\n" + " docker.allowed.rw-mounts=/tmp,/etc/\n docker.allowed.networks=bridge\n " + " docker.privileged-containers.enabled=0\n docker.allowed.capabilities=CHOWN,SETUID\n" + " docker.banned.mounts=/etc/passwd\n" + " docker.allowed.devices=/dev/test"; + write_file(container_executor_cfg_file, container_executor_contents); + int ret = read_config(container_executor_cfg_file.c_str(), &container_executor_cfg); + if (ret != 0) { + FAIL(); + } + ret = create_ce_file(); + if (ret != 0) { + std::cerr << "Could not create ce file, skipping test" << std::endl; + return; + } + + std::vector > file_cmd_vec; + 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" + " ro-mounts=/var/log:/var/log,/var/lib:/lib,/usr/bin/touch:/usr/bin/touch\n" + " rw-mounts=/tmp:/tmp,/etc:/etc,/etc/passwd:/etc/passwd\n" + " network=bridge\n devices=/dev/test:/dev/test\n net=bridge\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'" + " -v '/usr/bin/touch:/usr/bin/touch:ro' -v '/tmp:/tmp' -v '/etc:/etc' --mount 'type=bind,source=/var/empty/sshd,target=/etc/passwd,readonly' " + "--cgroup-parent='ctr-cgroup' --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' ")); + + std::vector > bad_file_cmd_vec; + bad_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" + " ro-mounts=/var/log:/var/log,/var/lib:/lib,/usr/bin/touch:/usr/bin/touch\n rw-mounts=/tmp:/tmp,/sys/:/sys\n" + " network=bridge\n devices=/dev/test:/dev/test\n net=bridge\n\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", + static_cast(INVALID_DOCKER_RW_MOUNT))); + + run_docker_command_test(file_cmd_vec, bad_file_cmd_vec, get_docker_run_command); + } + TEST_F(TestDockerUtil, test_docker_config_param) { std::vector > input_output_map; input_output_map.push_back(std::make_pair( diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/DockerContainers.md b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/DockerContainers.md index 36c391a..0ed3da7 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/DockerContainers.md +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/DockerContainers.md @@ -180,6 +180,7 @@ are allowed. It contains the following properties: | `docker.allowed.networks` | Comma separated networks that containers are allowed to use. If no network is specified when launching the container, the default Docker network will be used. | | `docker.allowed.ro-mounts` | Comma separated directories that containers are allowed to mount in read-only mode. By default, no directories are allowed to mounted. | | `docker.allowed.rw-mounts` | Comma separated directories that containers are allowed to mount in read-write mode. By default, no directories are allowed to mounted. | +| `docker.banned.mounts` | Comma separated directories and files that containers are forbidden to mount. | | `docker.privileged-containers.enabled` | Set to 1 or 0 to enable or disable launching privileged containers. Default value is 0. | Please note that if you wish to run Docker containers that require access to the YARN local directories, you must add them to the docker.allowed.rw-mounts list. @@ -205,6 +206,7 @@ yarn.nodemanager.linux-container-executor.group=yarn docker.allowed.networks=bridge,host,none docker.allowed.ro-mounts=/sys/fs/cgroup docker.allowed.rw-mounts=/var/hadoop/yarn/local-dir,/var/hadoop/yarn/log-dir + docker.banned.mounts=/run ```