From 9a592ee3fc195f20732c6b1f90894a0be25ccc19 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Apr 28 2014 19:57:01 +0000 Subject: util: Change file check fns to use a mode mask Instead of using a custom way to chck file type, use the system provided macros and a mode mask to decide when we want to check. Additionally a mask also allows us to selectively check permissions. Related: https://bugzilla.redhat.com/1089098 Resolves: https://fedorahosted.org/sssd/ticket/2321 Signed-off-by: Simo Sorce Reviewed-by: Pavel Reichl --- diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 845968d..85655de 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -2775,7 +2775,8 @@ int main(int argc, const char *argv[]) #endif /* Warn if nscd seems to be running */ - ret = check_file(NSCD_SOCKET_PATH, -1, -1, -1, CHECK_SOCK, NULL, false); + ret = check_file(NSCD_SOCKET_PATH, + -1, -1, S_IFSOCK, S_IFMT, NULL, false); if (ret == EOK) { ret = sss_nscd_parse_conf(NSCD_CONF_PATH); diff --git a/src/sbus/sbus_client.c b/src/sbus/sbus_client.c index 41cada5..6cf5002 100644 --- a/src/sbus/sbus_client.c +++ b/src/sbus/sbus_client.c @@ -45,7 +45,8 @@ int sbus_client_init(TALLOC_CTX *mem_ctx, return EIO; } - ret = check_file(filename, 0, 0, 0600, CHECK_SOCK, NULL, true); + ret = check_file(filename, + 0, 0, S_IFSOCK|S_IRUSR|S_IWUSR, 0, NULL, true); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "check_file failed for [%s].\n", filename); return EIO; diff --git a/src/sbus/sssd_dbus_server.c b/src/sbus/sssd_dbus_server.c index 39c315d..3a7de8f 100644 --- a/src/sbus/sssd_dbus_server.c +++ b/src/sbus/sssd_dbus_server.c @@ -249,15 +249,16 @@ int sbus_new_server(TALLOC_CTX *mem_ctx, /* Both check_file and chmod can handle both the symlink and * the socket */ - ret = check_file(filename, getuid(), getgid(), -1, CHECK_SOCK, &stat_buf, true); + ret = check_file(filename, + getuid(), getgid(), S_IFSOCK, S_IFMT, &stat_buf, true); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "check_file failed for [%s].\n", filename); ret = EIO; goto done; } - if ((stat_buf.st_mode & ~S_IFMT) != 0600) { - ret = chmod(filename, 0600); + if ((stat_buf.st_mode & ~S_IFMT) != (S_IRUSR|S_IWUSR)) { + ret = chmod(filename, (S_IRUSR|S_IWUSR)); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "chmod failed for [%s]: [%d][%s].\n", filename, errno, diff --git a/src/tests/check_and_open-tests.c b/src/tests/check_and_open-tests.c index 6ff40d0..8837acc 100644 --- a/src/tests/check_and_open-tests.c +++ b/src/tests/check_and_open-tests.c @@ -79,7 +79,8 @@ START_TEST(test_wrong_filename) { int ret; - ret = check_and_open_readonly("/bla/bla/bla", &fd, uid, gid, mode, CHECK_REG); + ret = check_and_open_readonly("/bla/bla/bla", &fd, + uid, gid, S_IFREG|mode, 0); fail_unless(ret == ENOENT, "check_and_open_readonly succeeded on non-existing file"); fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1"); @@ -104,7 +105,7 @@ START_TEST(test_symlink) ret = symlink(filename, newpath); fail_unless(ret == 0, "symlink failed [%d][%s]", ret, strerror(errno)); - ret = check_file(newpath, uid, gid, mode, CHECK_REG, NULL, false); + ret = check_file(newpath, uid, gid, S_IFREG|mode, 0, NULL, false); unlink(newpath); fail_unless(ret == EINVAL, @@ -130,7 +131,7 @@ START_TEST(test_follow_symlink) ret = symlink(filename, newpath); fail_unless(ret == 0, "symlink failed [%d][%s]", ret, strerror(errno)); - ret = check_file(newpath, uid, gid, mode, CHECK_REG, NULL, true); + ret = check_file(newpath, uid, gid, S_IFREG|mode, 0, NULL, true); unlink(newpath); fail_unless(ret == EOK, @@ -142,7 +143,7 @@ START_TEST(test_not_regular_file) { int ret; - ret = check_and_open_readonly("/dev/null", &fd, uid, gid, mode, CHECK_REG); + ret = check_and_open_readonly("/dev/null", &fd, uid, gid, S_IFREG|mode, 0); fail_unless(ret == EINVAL, "check_and_open_readonly succeeded on non-regular file"); fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1"); @@ -153,7 +154,7 @@ START_TEST(test_wrong_uid) { int ret; - ret = check_and_open_readonly(filename, &fd, uid+1, gid, mode, CHECK_REG); + ret = check_and_open_readonly(filename, &fd, uid+1, gid, S_IFREG|mode, 0); fail_unless(ret == EINVAL, "check_and_open_readonly succeeded with wrong uid"); fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1"); @@ -164,7 +165,7 @@ START_TEST(test_wrong_gid) { int ret; - ret = check_and_open_readonly(filename, &fd, uid, gid+1, mode, CHECK_REG); + ret = check_and_open_readonly(filename, &fd, uid, gid+1, S_IFREG|mode, 0); fail_unless(ret == EINVAL, "check_and_open_readonly succeeded with wrong gid"); fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1"); @@ -175,8 +176,8 @@ START_TEST(test_wrong_permission) { int ret; - ret = check_and_open_readonly(filename, &fd, uid, gid, (mode|S_IWOTH), - CHECK_REG); + ret = check_and_open_readonly(filename, &fd, + uid, gid, S_IFREG|mode|S_IWOTH, 0); fail_unless(ret == EINVAL, "check_and_open_readonly succeeded with wrong mode"); fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1"); @@ -187,7 +188,7 @@ START_TEST(test_ok) { int ret; - ret = check_and_open_readonly(filename, &fd, uid, gid, mode, CHECK_REG); + ret = check_and_open_readonly(filename, &fd, uid, gid, S_IFREG|mode, 0); fail_unless(ret == EOK, "check_and_open_readonly failed"); fail_unless(fd >= 0, @@ -201,7 +202,7 @@ START_TEST(test_write) ssize_t size; errno_t my_errno; - ret = check_and_open_readonly(filename, &fd, uid, gid, mode, CHECK_REG); + ret = check_and_open_readonly(filename, &fd, uid, gid, S_IFREG|mode, 0); fail_unless(ret == EOK, "check_and_open_readonly failed"); fail_unless(fd >= 0, diff --git a/src/tests/files-tests.c b/src/tests/files-tests.c index 2a0e7ce..8b649e7 100644 --- a/src/tests/files-tests.c +++ b/src/tests/files-tests.c @@ -192,7 +192,7 @@ START_TEST(test_simple_copy) fail_unless(ret == 0, "destination directory not there\n"); tmp = talloc_asprintf(test_ctx, "%s/bar", dst_path); - ret = check_and_open_readonly(tmp, &fd, uid, gid, 0700, CHECK_REG); + ret = check_and_open_readonly(tmp, &fd, uid, gid, S_IFREG|S_IRWXU, 0); fail_unless(ret == EOK, "Cannot open %s\n"); close(fd); talloc_free(tmp); diff --git a/src/util/check_and_open.c b/src/util/check_and_open.c index 7bf7805..80ec00e 100644 --- a/src/util/check_and_open.c +++ b/src/util/check_and_open.c @@ -30,11 +30,11 @@ #include "util/util.h" static errno_t perform_checks(struct stat *stat_buf, - const int uid, const int gid, - const int mode, enum check_file_type type); + uid_t uid, gid_t gid, + mode_t mode, mode_t mask); -errno_t check_file(const char *filename, const int uid, const int gid, - const int mode, enum check_file_type type, +errno_t check_file(const char *filename, + uid_t uid, uid_t gid, mode_t mode, mode_t mask, struct stat *caller_stat_buf, bool follow_symlink) { int ret; @@ -47,19 +47,22 @@ errno_t check_file(const char *filename, const int uid, const int gid, stat_buf = caller_stat_buf; } - ret = follow_symlink ? stat(filename, stat_buf) : \ - lstat(filename, stat_buf); + if (follow_symlink) { + ret = stat(filename, stat_buf); + } else { + ret = lstat(filename, stat_buf); + } if (ret == -1) { - DEBUG(SSSDBG_TRACE_FUNC, "lstat for [%s] failed: [%d][%s].\n", filename, errno, - strerror(errno)); + DEBUG(SSSDBG_TRACE_FUNC, "lstat for [%s] failed: [%d][%s].\n", + filename, errno, strerror(errno)); return errno; } - return perform_checks(stat_buf, uid, gid, mode, type); + return perform_checks(stat_buf, uid, gid, mode, mask); } -errno_t check_fd(int fd, const int uid, const int gid, - const int mode, enum check_file_type type, +errno_t check_fd(int fd, uid_t uid, gid_t gid, + mode_t mode, mode_t mask, struct stat *caller_stat_buf) { int ret; @@ -80,63 +83,39 @@ errno_t check_fd(int fd, const int uid, const int gid, return errno; } - return perform_checks(stat_buf, uid, gid, mode, type); + return perform_checks(stat_buf, uid, gid, mode, mask); } static errno_t perform_checks(struct stat *stat_buf, - const int uid, const int gid, - const int mode, enum check_file_type type) + uid_t uid, gid_t gid, + mode_t mode, mode_t mask) { - bool type_check; - - switch (type) { - case CHECK_DONT_CHECK_FILE_TYPE: - type_check = true; - break; - case CHECK_REG: - type_check = S_ISREG(stat_buf->st_mode); - break; - case CHECK_DIR: - type_check = S_ISDIR(stat_buf->st_mode); - break; - case CHECK_CHR: - type_check = S_ISCHR(stat_buf->st_mode); - break; - case CHECK_BLK: - type_check = S_ISBLK(stat_buf->st_mode); - break; - case CHECK_FIFO: - type_check = S_ISFIFO(stat_buf->st_mode); - break; - case CHECK_LNK: - type_check = S_ISLNK(stat_buf->st_mode); - break; - case CHECK_SOCK: - type_check = S_ISSOCK(stat_buf->st_mode); - break; - default: - DEBUG(SSSDBG_CRIT_FAILURE, "Unsupported file type.\n"); - return EINVAL; + mode_t st_mode; + + if (mask) { + st_mode = stat_buf->st_mode & mask; + } else { + st_mode = stat_buf->st_mode & (S_IFMT|ALLPERMS); } - if (!type_check) { + if ((mode & S_IFMT) != (st_mode & S_IFMT)) { DEBUG(SSSDBG_CRIT_FAILURE, "File is not the right type.\n"); return EINVAL; } - if (mode >= 0 && (stat_buf->st_mode & ~S_IFMT) != mode) { + if ((st_mode & ALLPERMS) != (mode & ALLPERMS)) { DEBUG(SSSDBG_CRIT_FAILURE, "File has the wrong mode [%.7o], expected [%.7o].\n", - (stat_buf->st_mode & ~S_IFMT), mode); + (st_mode & ALLPERMS), (mode & ALLPERMS)); return EINVAL; } - if (uid >= 0 && stat_buf->st_uid != uid) { + if (uid != (uid_t)(-1) && stat_buf->st_uid != uid) { DEBUG(SSSDBG_CRIT_FAILURE, "File must be owned by uid [%d].\n", uid); return EINVAL; } - if (gid >= 0 && stat_buf->st_gid != gid) { + if (gid != (gid_t)(-1) && stat_buf->st_gid != gid) { DEBUG(SSSDBG_CRIT_FAILURE, "File must be owned by gid [%d].\n", gid); return EINVAL; } @@ -144,9 +123,9 @@ static errno_t perform_checks(struct stat *stat_buf, return EOK; } -errno_t check_and_open_readonly(const char *filename, int *fd, const uid_t uid, - const gid_t gid, const mode_t mode, - enum check_file_type type) +errno_t check_and_open_readonly(const char *filename, int *fd, + uid_t uid, gid_t gid, + mode_t mode, mode_t mask) { int ret; struct stat stat_buf; @@ -159,7 +138,7 @@ errno_t check_and_open_readonly(const char *filename, int *fd, const uid_t uid, return errno; } - ret = check_fd(*fd, uid, gid, mode, type, &stat_buf); + ret = check_fd(*fd, uid, gid, mode, mask, &stat_buf); if (ret != EOK) { close(*fd); *fd = -1; diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 5293ae9..89b133d 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -120,7 +120,7 @@ int sss_ini_config_file_open(struct sss_ini_initdata *init_data, &init_data->file); #elif defined(HAVE_LIBINI_CONFIG_V0) return check_and_open_readonly(config_file, &init_data->file, 0, 0, - (S_IRUSR|S_IWUSR), CHECK_REG); + S_IFREG|S_IRUSR|S_IWUSR, 0); #endif } diff --git a/src/util/util.h b/src/util/util.h index c74ff5b..54c6c6c 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -329,18 +329,6 @@ sss_get_domain_name(TALLOC_CTX *mem_ctx, const char *orig_name, /* from backup-file.c */ int backup_file(const char *src, int dbglvl); -/* from check_and_open.c */ -enum check_file_type { - CHECK_DONT_CHECK_FILE_TYPE = -1, - CHECK_REG, - CHECK_DIR, - CHECK_CHR, - CHECK_BLK, - CHECK_FIFO, - CHECK_LNK, - CHECK_SOCK -}; - /* check_file() * Verify that a file has certain permissions and/or is of a certain * file type. This function can be used to determine if a file is a @@ -352,8 +340,8 @@ enum check_file_type { * inode to minimize impact. Permission changes may have occurred, * however. */ -errno_t check_file(const char *filename, const int uid, const int gid, - const int mode, enum check_file_type type, +errno_t check_file(const char *filename, + uid_t uid, gid_t gid, mode_t mode, mode_t mask, struct stat *caller_stat_buf, bool follow_symlink); /* check_fd() @@ -363,8 +351,8 @@ errno_t check_file(const char *filename, const int uid, const int gid, * is the safer way to perform file checks and should be preferred * over check_file for nearly all situations. */ -errno_t check_fd(int fd, const int uid, const int gid, - const int mode, enum check_file_type type, +errno_t check_fd(int fd, uid_t uid, gid_t gid, + mode_t mode, mode_t mask, struct stat *caller_stat_buf); /* check_and_open_readonly() @@ -372,9 +360,9 @@ errno_t check_fd(int fd, const int uid, const int gid, * permissions and is of a certain file type. This function wraps * check_fd(), and is considered race-condition safe. */ -errno_t check_and_open_readonly(const char *filename, int *fd, const uid_t uid, - const gid_t gid, const mode_t mode, - enum check_file_type type); +errno_t check_and_open_readonly(const char *filename, int *fd, + uid_t uid, gid_t gid, + mode_t mode, mode_t mask); /* from util.c */ #define SSS_NO_LINKLOCAL 0x01