>From 785af2aa94691c96e56caa94420b83c5ea346cfc Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 12 Aug 2015 12:41:44 +0200 Subject: [PATCH 1/2] UTIL: Provide a common interface to safely create temporary files --- src/tests/cmocka/test_utils.c | 175 ++++++++++++++++++++++++++++++++++++++++++ src/util/util.c | 127 ++++++++++++++++++++++++++++++ src/util/util.h | 21 +++++ 3 files changed, 323 insertions(+) diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c index d3d00feda0bdd4048519f90ba48ae9d1042a95a1..c7ebe0997ec00197e8852bedbcf26ef1f6394fc3 100644 --- a/src/tests/cmocka/test_utils.c +++ b/src/tests/cmocka/test_utils.c @@ -1212,6 +1212,168 @@ void test_fix_domain_in_name_list(void **state) talloc_free(dom); } +struct unique_file_test_ctx { + char *filename; +}; + +static int unique_file_test_setup(void **state) +{ + struct unique_file_test_ctx *test_ctx; + + assert_true(leak_check_setup()); + check_leaks_push(global_talloc_context); + + test_ctx = talloc_zero(global_talloc_context, struct unique_file_test_ctx); + assert_non_null(test_ctx); + + test_ctx->filename = talloc_strdup(test_ctx, "test_unique_file_XXXXXX"); + assert_non_null(test_ctx); + + *state = test_ctx; + return 0; +} + +static int unique_file_test_teardown(void **state) +{ + struct unique_file_test_ctx *test_ctx; + errno_t ret; + + test_ctx = talloc_get_type(*state, struct unique_file_test_ctx); + + errno = 0; + ret = unlink(test_ctx->filename); + if (ret != 0 && errno != ENOENT) { + fail(); + } + + talloc_free(test_ctx); + assert_true(check_leaks_pop(global_talloc_context) == true); + assert_true(leak_check_teardown()); + return 0; +} + +static void assert_destructor(TALLOC_CTX *owner, + struct unique_file_test_ctx *test_ctx) +{ + int fd; + errno_t ret; + char *check_filename; + + /* Test that the destructor works */ + if (owner == NULL) { + return; + } + + check_filename = talloc_strdup(test_ctx, test_ctx->filename); + assert_non_null(check_filename); + + talloc_free(owner); + + ret = check_and_open_readonly(test_ctx->filename, &fd, + geteuid(), getegid(), + (S_IRUSR | S_IWUSR | S_IFREG), 0); + close(fd); + assert_int_not_equal(ret, EOK); +} + +static void sss_unique_file_test(struct unique_file_test_ctx *test_ctx, + bool test_destructor) +{ + int fd; + errno_t ret; + struct stat sb; + TALLOC_CTX *owner = NULL; + + if (test_destructor) { + owner = talloc_new(test_ctx); + assert_non_null(owner); + } + + fd = sss_unique_file(owner, test_ctx->filename, &ret); + assert_int_not_equal(fd, -1); + assert_int_equal(ret, EOK); + + ret = check_fd(fd, geteuid(), getegid(), + (S_IRUSR | S_IWUSR | S_IFREG), 0, &sb); + close(fd); + assert_int_equal(ret, EOK); + + assert_destructor(owner, test_ctx); +} + +static void test_sss_unique_file(void **state) +{ + struct unique_file_test_ctx *test_ctx; + test_ctx = talloc_get_type(*state, struct unique_file_test_ctx); + sss_unique_file_test(test_ctx, false); +} + +static void test_sss_unique_file_destruct(void **state) +{ + struct unique_file_test_ctx *test_ctx; + test_ctx = talloc_get_type(*state, struct unique_file_test_ctx); + sss_unique_file_test(test_ctx, true); +} + +static void test_sss_unique_file_neg(void **state) +{ + int fd; + errno_t ret; + + fd = sss_unique_file(NULL, discard_const("badpattern"), &ret); + assert_int_equal(fd, -1); + assert_int_equal(ret, EINVAL); +} + +static void sss_unique_filename_test(struct unique_file_test_ctx *test_ctx, + bool test_destructor) +{ + int fd; + errno_t ret; + char *tmp_filename; + TALLOC_CTX *owner = NULL; + + tmp_filename = talloc_strdup(test_ctx, test_ctx->filename); + assert_non_null(tmp_filename); + + if (test_destructor) { + owner = talloc_new(test_ctx); + assert_non_null(owner); + } + + ret = sss_unique_filename(owner, test_ctx->filename); + assert_int_equal(ret, EOK); + + assert_int_equal(strncmp(test_ctx->filename, + tmp_filename, + strlen(tmp_filename) - sizeof("XXXXXX")), + 0); + + ret = check_and_open_readonly(test_ctx->filename, &fd, + geteuid(), getegid(), + (S_IRUSR | S_IWUSR | S_IFREG), 0); + close(fd); + assert_int_equal(ret, EOK); + + assert_destructor(owner, test_ctx); +} + +static void test_sss_unique_filename(void **state) +{ + struct unique_file_test_ctx *test_ctx; + + test_ctx = talloc_get_type(*state, struct unique_file_test_ctx); + sss_unique_filename_test(test_ctx, false); +} + +static void test_sss_unique_filename_destruct(void **state) +{ + struct unique_file_test_ctx *test_ctx; + + test_ctx = talloc_get_type(*state, struct unique_file_test_ctx); + sss_unique_filename_test(test_ctx, true); +} + int main(int argc, const char *argv[]) { poptContext pc; @@ -1275,6 +1437,19 @@ int main(int argc, const char *argv[]) cmocka_unit_test_setup_teardown(test_fix_domain_in_name_list, confdb_test_setup, confdb_test_teardown), + cmocka_unit_test_setup_teardown(test_sss_unique_file, + unique_file_test_setup, + unique_file_test_teardown), + cmocka_unit_test_setup_teardown(test_sss_unique_file_destruct, + unique_file_test_setup, + unique_file_test_teardown), + cmocka_unit_test(test_sss_unique_file_neg), + cmocka_unit_test_setup_teardown(test_sss_unique_filename, + unique_file_test_setup, + unique_file_test_teardown), + cmocka_unit_test_setup_teardown(test_sss_unique_filename_destruct, + unique_file_test_setup, + unique_file_test_teardown), }; /* Set debug level to invalid value so we can deside if -d 0 was used. */ diff --git a/src/util/util.c b/src/util/util.c index 782cd026b7928e607a8980fb5f333c794feb5b1a..f0925051a08970b191f61a533cd49dd53ae128e0 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -979,3 +979,130 @@ errno_t sss_utc_to_time_t(const char *str, const char *format, time_t *_unix_tim *_unix_time = ut; return EOK; } + +struct tmpfile_watch { + const char *filename; +}; + +static int unlink_dbg(const char *filename) +{ + errno_t ret; + + ret = unlink(filename); + if (ret != 0) { + if (errno == 2) { + DEBUG(SSSDBG_TRACE_INTERNAL, + "File already removed: [%s]\n", filename); + return 0; + } else { + DEBUG(SSSDBG_CRIT_FAILURE, + "Cannot remove temporary file [%s]\n", filename); + return -1; + } + } + + return 0; +} + +static int unique_filename_destructor(void *memptr) +{ + struct tmpfile_watch *tw = talloc_get_type(memptr, struct tmpfile_watch); + + if (tw == NULL || tw->filename == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "BUG: Wrong private pointer\n"); + return -1; + } + + DEBUG(SSSDBG_TRACE_INTERNAL, "Unlinking [%s]\n", tw->filename); + + return unlink_dbg(tw->filename); +} + +static struct tmpfile_watch *tmpfile_watch_set(TALLOC_CTX *owner, + const char *filename) +{ + struct tmpfile_watch *tw = NULL; + + tw = talloc_zero(owner, struct tmpfile_watch); + if (tw == NULL) { + return NULL; + } + + tw->filename = talloc_strdup(tw, filename); + if (tw->filename == NULL) { + talloc_free(tw); + return NULL; + } + + talloc_set_destructor((TALLOC_CTX *) tw, + unique_filename_destructor); + return tw; +} + +int sss_unique_file_ex(TALLOC_CTX *owner, + char *path_tmpl, + mode_t file_umask, + errno_t *_err) +{ + size_t tmpl_len; + errno_t ret; + int fd = -1; + mode_t old_umask; + struct tmpfile_watch *tw = NULL; + + tmpl_len = strlen(path_tmpl); + if (tmpl_len < 6 || strcmp(path_tmpl + (tmpl_len - 6), "XXXXXX") != 0) { + DEBUG(SSSDBG_OP_FAILURE, + "Template too short or doesn't end with XXXXXX!\n"); + ret = EINVAL; + goto done; + } + + old_umask = umask(file_umask); + fd = mkstemp(path_tmpl); + umask(old_umask); + if (fd == -1) { + ret = errno; + DEBUG(SSSDBG_OP_FAILURE, + "mkstemp(\"%s\") failed [%d]: %s!\n", + path_tmpl, ret, strerror(ret)); + goto done; + } + + if (owner != NULL) { + tw = tmpfile_watch_set(owner, path_tmpl); + if (tw == NULL) { + unlink_dbg(path_tmpl); + ret = ENOMEM; + goto done; + } + } + + ret = EOK; +done: + if (_err) { + *_err = ret; + } + return fd; +} + +int sss_unique_file(TALLOC_CTX *owner, + char *path_tmpl, + errno_t *_err) +{ + return sss_unique_file_ex(owner, path_tmpl, 077, _err); +} + +errno_t sss_unique_filename(TALLOC_CTX *owner, char *path_tmpl) +{ + int fd; + errno_t ret; + + fd = sss_unique_file(owner, path_tmpl, &ret); + /* We only care about a unique file name */ + if (fd >= 0) { + close(fd); + } + + return ret; +} diff --git a/src/util/util.h b/src/util/util.h index 94a3ddea839f0998cb7796f1d2fe13f743de3aaf..a20d1d82eb8f10dac515ad25e7e424713bb1c099 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -658,4 +658,25 @@ int get_seuser(TALLOC_CTX *mem_ctx, const char *login_name, /* convert time from generalized form to unix time */ errno_t sss_utc_to_time_t(const char *str, const char *format, time_t *unix_time); +/* Creates a unique file using mkstemp with provided umask. The template + * must end with XXXXXX. Returns the fd, sets _err to an errno value on error. + * + * Prefer using sss_unique_file() as it uses a secure umask internally. + */ +int sss_unique_file_ex(TALLOC_CTX *mem_ctx, + char *path_tmpl, + mode_t file_umask, + errno_t *_err); +int sss_unique_file(TALLOC_CTX *owner, + char *path_tmpl, + errno_t *_err); + +/* Creates a unique filename using mkstemp with secure umask. The template + * must end with XXXXXX + * + * path_tmpl must be a talloc context. Destructor would be set on the filename + * so that it's guaranteed the file is removed. + */ +int sss_unique_filename(TALLOC_CTX *owner, char *path_tmpl); + #endif /* __SSSD_UTIL_H__ */ -- 2.4.3