From c2af7f7d7f6b0f1aaa884204a037e8275092121a Mon Sep 17 00:00:00 2001 From: zhongtao Date: Sat, 2 Sep 2023 10:38:29 +0000 Subject: [PATCH 25/33] !2166 move ensure_isulad_tmpdir_security function to main.c * move ensure_isulad_tmpdir_security function to main.c --- src/cmd/isulad/main.c | 101 ++++++++++++++++++ .../container/leftover_cleanup/cleanup.c | 66 +----------- src/utils/tar/util_archive.c | 2 +- 3 files changed, 103 insertions(+), 66 deletions(-) diff --git a/src/cmd/isulad/main.c b/src/cmd/isulad/main.c index 4740f91a..e32fed6a 100644 --- a/src/cmd/isulad/main.c +++ b/src/cmd/isulad/main.c @@ -1222,6 +1222,101 @@ out: return ret; } +static int isulad_tmpdir_security_check(const char *tmp_dir) +{ + struct stat st = { 0 }; + + if (lstat(tmp_dir, &st) != 0) { + SYSERROR("Failed to lstat %s", tmp_dir); + return -1; + } + + if (!S_ISDIR(st.st_mode)) { + return -1; + } + + if ((st.st_mode & 0777) != ISULAD_TEMP_DIRECTORY_MODE) { + return -1; + } + + if (st.st_uid != 0) { + return -1; + } + + if (S_ISLNK(st.st_mode)) { + return -1; + } + + return 0; +} + +static int recreate_tmpdir(const char *tmp_dir) +{ + if (util_recursive_rmdir(tmp_dir, 0) != 0) { + ERROR("Failed to remove directory %s", tmp_dir); + return -1; + } + + if (util_mkdir_p(tmp_dir, ISULAD_TEMP_DIRECTORY_MODE) != 0) { + ERROR("Failed to create directory %s", tmp_dir); + return -1; + } + + return 0; +} + +static int do_ensure_isulad_tmpdir_security(const char *isulad_tmp_dir) +{ + int nret; + char tmp_dir[PATH_MAX] = { 0 }; + char cleanpath[PATH_MAX] = { 0 }; + + nret = snprintf(tmp_dir, PATH_MAX, "%s/isulad_tmpdir", isulad_tmp_dir); + if (nret < 0 || (size_t)nret >= PATH_MAX) { + ERROR("Failed to snprintf"); + return -1; + } + + if (util_clean_path(tmp_dir, cleanpath, sizeof(cleanpath)) == NULL) { + ERROR("Failed to clean path for %s", tmp_dir); + return -1; + } + + if (isulad_tmpdir_security_check(cleanpath) == 0) { + return 0; + } + + INFO("iSulad tmpdir: %s does not meet security requirements, recreate it", isulad_tmp_dir); + return recreate_tmpdir(cleanpath); +} + +static int ensure_isulad_tmpdir_security() +{ + char *isulad_tmp_dir = NULL; + + isulad_tmp_dir = getenv("ISULAD_TMPDIR"); + if (!util_valid_str(isulad_tmp_dir)) { + isulad_tmp_dir = "/tmp"; + } + + if (do_ensure_isulad_tmpdir_security(isulad_tmp_dir) != 0) { + ERROR("Failed to ensure the %s directory is a safe directory", isulad_tmp_dir); + return -1; + } + + if (strcmp(isulad_tmp_dir, "/tmp") == 0) { + return 0; + } + + // No matter whether ISULAD_TMPDIR is set or not, + // ensure the "/tmp" directory is a safe directory + if (do_ensure_isulad_tmpdir_security("/tmp") != 0) { + WARN("Failed to ensure the /tmp directory is a safe directory"); + } + + return 0; +} + static int isulad_server_init_common() { int ret = -1; @@ -1261,6 +1356,12 @@ static int isulad_server_init_common() goto out; } + // preventing the use of insecure isulad tmpdir directory + if (ensure_isulad_tmpdir_security() != 0) { + ERROR("Failed to ensure isulad tmpdir security"); + goto out; + } + if (volume_init(args->json_confs->graph) != 0) { ERROR("Failed to init volume"); goto out; diff --git a/src/daemon/modules/container/leftover_cleanup/cleanup.c b/src/daemon/modules/container/leftover_cleanup/cleanup.c index f24ec467..9a38ffc2 100644 --- a/src/daemon/modules/container/leftover_cleanup/cleanup.c +++ b/src/daemon/modules/container/leftover_cleanup/cleanup.c @@ -13,8 +13,6 @@ * Description: provide cleanup functions *********************************************************************************/ #include -#include -#include #include "utils.h" #include "utils_fs.h" @@ -171,67 +169,6 @@ static bool walk_isulad_tmpdir_cb(const char *path_name, const struct dirent *su return true; } -static int isulad_tmpdir_security_check(const char *tmpdir) -{ - struct stat st = { 0 }; - - if (lstat(tmpdir, &st) != 0) { - SYSERROR("Failed to lstat %s", tmpdir); - return -1; - } - - if (!S_ISDIR(st.st_mode)) { - return -1; - } - - if ((st.st_mode & 0777) != ISULAD_TEMP_DIRECTORY_MODE) { - return -1; - } - - if (st.st_uid != 0) { - return -1; - } - - if (S_ISLNK(st.st_mode)) { - return -1; - } - - return 0; -} - -static int recreate_tmpdir(const char *tmpdir) -{ - int ret; - struct stat st = { 0 }; - - if (util_recursive_rmdir(tmpdir, 0)) { - ERROR("Failed to remove directory %s", tmpdir); - return -1; - } - - if (util_mkdir_p(tmpdir, ISULAD_TEMP_DIRECTORY_MODE)) { - ERROR("Failed to create directory %s", tmpdir); - return -1; - } - - if (lstat(tmpdir, &st) != 0) { - SYSERROR("Failed to lstat %s", tmpdir); - return -1; - } - - return ret; -} - -static int ensure_isulad_tmpdir_security(const char *tmpdir) -{ - if (isulad_tmpdir_security_check(tmpdir) == 0) { - return 0; - } - - INFO("iSulad tmpdir does not meet security requirements, recreate it"); - return recreate_tmpdir(tmpdir); -} - static void cleanup_path(char *dir) { int nret; @@ -249,8 +186,7 @@ static void cleanup_path(char *dir) return; } - // preventing the use of insecure isulad tmpdir directory - if (ensure_isulad_tmpdir_security(cleanpath) != 0) { + if (!util_dir_exists(cleanpath)) { return; } diff --git a/src/utils/tar/util_archive.c b/src/utils/tar/util_archive.c index 82194654..82e940a5 100644 --- a/src/utils/tar/util_archive.c +++ b/src/utils/tar/util_archive.c @@ -220,7 +220,7 @@ static int make_safedir_is_noexec(const char *flock_path, const char *dstdir, ch } // ensure parent dir is exist - if (util_mkdir_p(cleanpath, buf.st_mode) != 0) { + if (util_mkdir_p(cleanpath, ISULAD_TEMP_DIRECTORY_MODE) != 0) { return -1; } -- 2.40.1