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)
This commit is contained in:
Wangda Tan 2018-03-07 10:41:12 -08:00
parent 4d53ef7eef
commit 037d783483
2 changed files with 106 additions and 18 deletions

View File

@ -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);

View File

@ -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",
@ -754,6 +757,14 @@ namespace ContainerExecutor {
"[docker-command-execution]\n docker-command=run\n image=hadoop/image\n"
" 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);
@ -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);