YARN-7626. Allow regular expression matching in container-executor.cfg for devices and named docker volumes mount. (Zian Chen via wangda)
Change-Id: If461277d4557922ab7e4dce9dd8dc5d0d5f22710 (cherry picked from commit 88f9138e12d2d5a1bd13f0915acef93037c1d086) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/037d7834 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/037d7834 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/037d7834 Branch: refs/heads/trunk Commit: 037d7834833df2d1e60f5015b60d42550b1ddce6 Parents: 4d53ef7 Author: Wangda Tan <wan...@apache.org> Authored: Wed Mar 7 10:41:12 2018 -0800 Committer: Wangda Tan <wan...@apache.org> Committed: Wed Mar 7 11:30:15 2018 -0800 ---------------------------------------------------------------------- .../container-executor/impl/utils/docker-util.c | 71 ++++++++++++++++---- .../test/utils/test_docker_util.cc | 53 +++++++++++++-- 2 files changed, 106 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/037d7834/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c ---------------------------------------------------------------------- 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 04790b5..ccc21fa 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 @@ -115,6 +115,22 @@ int check_trusted_image(const struct configuration *command_config, const struct return ret; } +static int is_regex(const char *str) { + // regex should begin with prefix "regex:" + return (strncmp(str, "regex:", 6) == 0); +} + +static int is_volume_name(const char *volume_name) { + const char *regex_str = "^[a-zA-Z0-9]([a-zA-Z0-9_.-]*)$"; + // execute_regex_match return 0 is matched success + return execute_regex_match(regex_str, volume_name) == 0; +} + +static int is_volume_name_matched_by_regex(const char* requested, const char* pattern) { + // execute_regex_match return 0 is matched success + return is_volume_name(requested) && (execute_regex_match(pattern + sizeof("regex:"), requested) == 0); +} + static int add_param_to_command_if_allowed(const struct configuration *command_config, const struct configuration *executor_cfg, const char *key, const char *allowed_key, const char *param, @@ -150,6 +166,7 @@ static int add_param_to_command_if_allowed(const struct configuration *command_c } if (permitted_values != NULL) { + // Values are user requested. for (i = 0; values[i] != NULL; ++i) { memset(tmp_buffer, 0, tmp_buffer_size); permitted = 0; @@ -162,13 +179,30 @@ static int add_param_to_command_if_allowed(const struct configuration *command_c goto free_and_exit; } } + // Iterate through each permitted value + char* dst = NULL; + char* pattern = NULL; + for (j = 0; permitted_values[j] != NULL; ++j) { if (prefix == 0) { ret = strcmp(values[i], permitted_values[j]); } else { - ret = strncmp(values[i], permitted_values[j], tmp_ptr - values[i]); + // If permitted-Values[j] is a REGEX, use REGEX to compare + if (is_regex(permitted_values[j])) { + size_t offset = tmp_ptr - values[i]; + dst = (char *) alloc_and_clear_memory(offset, sizeof(char)); + strncpy(dst, values[i], offset); + dst[tmp_ptr - values[i]] = '\0'; + pattern = (char *) alloc_and_clear_memory((size_t)(strlen(permitted_values[j]) - 6), sizeof(char)); + strcpy(pattern, permitted_values[j] + 6); + ret = execute_regex_match(pattern, dst); + } else { + ret = strncmp(values[i], permitted_values[j], tmp_ptr - values[i]); + } } if (ret == 0) { + free(dst); + free(pattern); permitted = 1; break; } @@ -941,9 +975,10 @@ static int set_devices(const struct configuration *command_config, const struct * 2. If the path is a directory, add a '/' at the end (if not present) * 3. Return a copy of the canonicalised path(to be freed by the caller) * @param mount path to be canonicalised + * @param isRegexAllowed whether regex matching is allowed for normalize mount * @return pointer to canonicalised path, NULL on error */ -static char* normalize_mount(const char* mount) { +static char* normalize_mount(const char* mount, int isRegexAllowed) { int ret = 0; struct stat buff; char *ret_ptr = NULL, *real_mount = NULL; @@ -953,10 +988,16 @@ static char* normalize_mount(const char* mount) { real_mount = realpath(mount, NULL); if (real_mount == NULL) { // If mount is a valid named volume, just return it and let docker decide - if (validate_volume_name(mount) == 0) { + if (is_volume_name(mount)) { return strdup(mount); } - + // we only allow permitted mount to be REGEX, for permitted mount, we check + // if it's a valid REGEX return; for user mount, we need to strictly check + if (isRegexAllowed) { + if (is_regex(mount)) { + return strdup(mount); + } + } fprintf(ERRORFILE, "Could not determine real path of mount '%s'\n", mount); free(real_mount); return NULL; @@ -987,14 +1028,14 @@ static char* normalize_mount(const char* mount) { return ret_ptr; } -static int normalize_mounts(char **mounts) { +static int normalize_mounts(char **mounts, int isRegexAllowed) { int i = 0; char *tmp = NULL; if (mounts == NULL) { return 0; } for (i = 0; mounts[i] != NULL; ++i) { - tmp = normalize_mount(mounts[i]); + tmp = normalize_mount(mounts[i], isRegexAllowed); if (tmp == NULL) { return -1; } @@ -1007,7 +1048,7 @@ static int normalize_mounts(char **mounts) { static int check_mount_permitted(const char **permitted_mounts, const char *requested) { int i = 0, ret = 0; size_t permitted_mount_len = 0; - char *normalized_path = normalize_mount(requested); + char *normalized_path = normalize_mount(requested, 0); if (permitted_mounts == NULL) { return 0; } @@ -1019,6 +1060,13 @@ static int check_mount_permitted(const char **permitted_mounts, const char *requ ret = 1; break; } + // if (permitted_mounts[i] is a REGEX): use REGEX to compare; return + if (is_regex(permitted_mounts[i]) && + is_volume_name_matched_by_regex(normalized_path, permitted_mounts[i])) { + ret = 1; + break; + } + // directory check permitted_mount_len = strlen(permitted_mounts[i]); struct stat path_stat; @@ -1059,7 +1107,7 @@ static int add_mounts(const struct configuration *command_config, const struct c 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; - const char *container_executor_cfg_path = normalize_mount(get_config_path("")); + const char *container_executor_cfg_path = normalize_mount(get_config_path(""), 0); int i = 0, permitted_rw = 0, permitted_ro = 0, ret = 0; if (ro != 0) { ro_suffix = ":ro"; @@ -1078,9 +1126,8 @@ static int add_mounts(const struct configuration *command_config, const struct c ret = 0; goto free_and_exit; } - - ret = normalize_mounts(permitted_ro_mounts); - ret |= normalize_mounts(permitted_rw_mounts); + ret = normalize_mounts(permitted_ro_mounts, 1); + ret |= normalize_mounts(permitted_rw_mounts, 1); if (ret != 0) { fprintf(ERRORFILE, "Unable to find permitted docker mounts on disk\n"); ret = MOUNT_ACCESS_ERROR; @@ -1108,7 +1155,7 @@ static int add_mounts(const struct configuration *command_config, const struct c goto free_and_exit; } else { // determine if the user can modify the container-executor.cfg file - tmp_path_buffer[0] = normalize_mount(mount_src); + tmp_path_buffer[0] = normalize_mount(mount_src, 0); // just re-use the function, flip the args to check if the container-executor path is in the requested // mount point ret = check_mount_permitted(tmp_path_buffer, container_executor_cfg_path); http://git-wip-us.apache.org/repos/asf/hadoop/blob/037d7834/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/utils/test_docker_util.cc ---------------------------------------------------------------------- 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 81823a8..4bc3404 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 @@ -551,13 +551,16 @@ namespace ContainerExecutor { } TEST_F(TestDockerUtil, test_check_mount_permitted) { - const char *permitted_mounts[] = {"/etc", "/usr/bin/cut", "/tmp/", NULL}; + const char *permitted_mounts[] = {"/etc", "/usr/bin/cut", "/tmp/", "regex:nvidia_driver.*", NULL}; std::vector<std::pair<std::string, int> > test_data; test_data.push_back(std::make_pair<std::string, int>("/etc", 1)); test_data.push_back(std::make_pair<std::string, int>("/etc/", 1)); test_data.push_back(std::make_pair<std::string, int>("/etc/passwd", 1)); test_data.push_back(std::make_pair<std::string, int>("/usr/bin/cut", 1)); test_data.push_back(std::make_pair<std::string, int>("//usr/", 0)); + test_data.push_back(std::make_pair<std::string, int>("nvidia_driver_375.66", 1)); + test_data.push_back(std::make_pair<std::string, int>("nvidia_local_driver", 0)); + test_data.push_back(std::make_pair<std::string, int>("^/usr/.*$", -1)); test_data.push_back(std::make_pair<std::string, int>("/etc/random-file", -1)); std::vector<std::pair<std::string, int> >::const_iterator itr; @@ -568,9 +571,9 @@ namespace ContainerExecutor { } TEST_F(TestDockerUtil, test_normalize_mounts) { - const int entries = 4; - const char *permitted_mounts[] = {"/home", "/etc", "/usr/bin/cut", NULL}; - const char *expected[] = {"/home/", "/etc/", "/usr/bin/cut", NULL}; + const int entries = 5; + const char *permitted_mounts[] = {"/home", "/etc", "/usr/bin/cut", "regex:/dev/nvidia.*", NULL}; + const char *expected[] = {"/home/", "/etc/", "/usr/bin/cut", "regex:/dev/nvidia.*", NULL}; char **ptr = static_cast<char **>(malloc(entries * sizeof(char *))); for (int i = 0; i < entries; ++i) { if (permitted_mounts[i] != NULL) { @@ -579,7 +582,7 @@ namespace ContainerExecutor { ptr[i] = NULL; } } - normalize_mounts(ptr); + normalize_mounts(ptr, 1); for (int i = 0; i < entries; ++i) { ASSERT_STREQ(expected[i], ptr[i]); } @@ -742,7 +745,7 @@ namespace ContainerExecutor { int ret = 0; std::string container_executor_cfg_contents = "[docker]\n" " docker.privileged-containers.registries=hadoop\n" - " docker.allowed.devices=/dev/test-device,/dev/device2"; + " docker.allowed.devices=/dev/test-device,/dev/device2,regex:/dev/nvidia.*,regex:/dev/gpu-uvm.*"; std::vector<std::pair<std::string, std::string> > file_cmd_vec; file_cmd_vec.push_back(std::make_pair<std::string, std::string>( "[docker-command-execution]\n docker-command=run\n image=hadoop/image\n devices=/dev/test-device:/dev/test-device", @@ -755,6 +758,14 @@ namespace ContainerExecutor { " devices=/dev/test-device:/dev/test-device,/dev/device2:/dev/device2", "--device='/dev/test-device:/dev/test-device' --device='/dev/device2:/dev/device2' ")); file_cmd_vec.push_back(std::make_pair<std::string, std::string>( + "[docker-command-execution]\n docker-command=run\n image=hadoop/image\n" + "devices=/dev/nvidiactl:/dev/nvidiactl", + "--device='/dev/nvidiactl:/dev/nvidiactl' ")); + file_cmd_vec.push_back(std::make_pair<std::string, std::string>( + "[docker-command-execution]\n docker-command=run\n image=hadoop/image\n" + "devices=/dev/nvidia1:/dev/nvidia1,/dev/gpu-uvm-tools:/dev/gpu-uvm-tools", + "--device='/dev/nvidia1:/dev/nvidia1' --device='/dev/gpu-uvm-tools:/dev/gpu-uvm-tools' ")); + file_cmd_vec.push_back(std::make_pair<std::string, std::string>( "[docker-command-execution]\n docker-command=run\n image=hadoop/image", "")); write_container_executor_cfg(container_executor_cfg_contents); ret = read_config(container_executor_cfg_file.c_str(), &container_cfg); @@ -804,6 +815,36 @@ namespace ContainerExecutor { ASSERT_EQ(INVALID_DOCKER_DEVICE, ret); ASSERT_EQ(0, strlen(buff)); + write_command_file("[docker-command-execution]\n docker-command=run\n image=hadoop/image\n devices=/dev/testnvidia:/dev/testnvidia"); + ret = read_config(docker_command_file.c_str(), &cmd_cfg); + if (ret != 0) { + FAIL(); + } + strcpy(buff, "test string"); + ret = set_devices(&cmd_cfg, &container_cfg, buff, buff_len); + ASSERT_EQ(INVALID_DOCKER_DEVICE, ret); + ASSERT_EQ(0, strlen(buff)); + + write_command_file("[docker-command-execution]\n docker-command=run\n image=hadoop/image\n devices=/dev/gpu-nvidia-uvm:/dev/gpu-nvidia-uvm"); + ret = read_config(docker_command_file.c_str(), &cmd_cfg); + if (ret != 0) { + FAIL(); + } + strcpy(buff, "test string"); + ret = set_devices(&cmd_cfg, &container_cfg, buff, buff_len); + ASSERT_EQ(INVALID_DOCKER_DEVICE, ret); + ASSERT_EQ(0, strlen(buff)); + + write_command_file("[docker-command-execution]\n docker-command=run\n image=hadoop/image\n devices=/dev/device1"); + ret = read_config(docker_command_file.c_str(), &cmd_cfg); + if (ret != 0) { + FAIL(); + } + strcpy(buff, "test string"); + ret = set_devices(&cmd_cfg, &container_cfg, buff, buff_len); + ASSERT_EQ(INVALID_DOCKER_DEVICE, ret); + ASSERT_EQ(0, strlen(buff)); + container_executor_cfg_contents = "[docker]\n"; write_container_executor_cfg(container_executor_cfg_contents); ret = read_config(container_executor_cfg_file.c_str(), &container_cfg); --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
