From ad27acbb2104d4172a33dd82ef9d94c8f5dba617 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Thu, 28 May 2015 11:47:39 +0200
Subject: [PATCH] dd: add owner to meta-data
Use 'nobody' user to mark dump directories which have no owner and are
publicly readable and chownable. The 'nobody' should not be used
directly but through the function set_no_owner() which is a substitution
for adding S_IROTH (man 2 open) to dump dir mode.
Use meta-data to get the owner only if the dump directory is owned by
super-user, because it is the only state when we can trust that value.
Related: #354
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
src/include/dump_dir.h | 21 ++++++
src/lib/dump_dir.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++-
tests/dump_dir.at | 110 ++++++++++++++++++++++++++++
3 files changed, 322 insertions(+), 4 deletions(-)
diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h
index f33eb99..b37b262 100644
--- a/src/include/dump_dir.h
+++ b/src/include/dump_dir.h
@@ -141,6 +141,26 @@ int dd_rename(struct dump_dir *dd, const char *new_path);
*/
int dd_chown(struct dump_dir *dd, uid_t new_uid);
+/* Sets a new owner (does NOT chown the directory)
+ *
+ * Does not validate the passed uid.
+ * The given dump_dir must be opened for writing.
+ */
+int dd_set_owner(struct dump_dir *dd, uid_t owner);
+
+/* Makes the dump directory owned by nobody.
+ *
+ * The directory will be accessible for all users.
+ * The given dump_dir must be opened for writing.
+ */
+int dd_set_no_owner(struct dump_dir *dd);
+
+/* Gets the owner
+ *
+ * If meta-data misses owner, returns fs owner.
+ * Can be used with DD_OPEN_FD_ONLY.
+ */
+uid_t dd_get_owner(struct dump_dir *dd);
/* reported_to handling */
#define add_reported_to_data libreport_add_reported_to_data
@@ -187,6 +207,7 @@ int dd_accessible_by_uid(struct dump_dir *dd, uid_t uid);
enum {
DD_STAT_ACCESSIBLE_BY_UID = 1,
DD_STAT_OWNED_BY_UID = DD_STAT_ACCESSIBLE_BY_UID << 1,
+ DD_STAT_NO_OWNER = DD_STAT_OWNED_BY_UID << 1,
};
/* Gets information about a dump directory for particular uid.
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index 9ba2352..2cd14bb 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -88,6 +88,7 @@
// parent dump directory. It is not a fatal error, if the meta-data directory
// does not exist (backward compatibility).
#define META_DATA_DIR_NAME ".libreport"
+#define META_DATA_FILE_OWNER "owner"
enum {
/* Try to create meta-data dir if it does not exist */
@@ -106,6 +107,8 @@ static char *load_text_file(const char *path, unsigned flags);
static char *load_text_file_at(int dir_fd, const char *name, unsigned flags);
static void copy_file_from_chroot(struct dump_dir* dd, const char *name,
const char *chroot_dir, const char *file_path);
+static bool save_binary_file_at(int dir_fd, const char *name, const char* data,
+ unsigned size, uid_t uid, gid_t gid, mode_t mode);
static bool isdigit_str(const char *str)
{
@@ -136,6 +139,22 @@ static bool exist_file_dir_at(int dir_fd, const char *name)
#define dd_validate_element_name(name) \
(str_is_correct_filename(name) && (strcmp(META_DATA_DIR_NAME, name) != 0))
+/* nobody user should not own any file */
+static int get_no_owner_uid(uid_t *uid)
+{
+ struct passwd *pw = getpwnam("nobody");
+ if (pw == NULL)
+ {
+ perror_msg("can't get nobody's uid");
+ if (errno == 0)
+ return -ENOENT;
+ return -errno;
+ }
+
+ *uid = pw->pw_uid;
+ return 0;
+}
+
/* Opens the file in the three following steps:
* 1. open the file with O_PATH (get a file descriptor for operations with
* inode) and O_NOFOLLOW (do not dereference symbolick links)
@@ -611,6 +630,113 @@ static int dd_get_meta_data_dir_fd(struct dump_dir *dd, int flags)
return dd->dd_md_fd;
}
+/* Tries to safely overwrite the existing file.
+ *
+ * The functions writes the new value to a temporary file and if the temporary
+ * file is successfully created, then moves the tmp file to the old file name.
+ *
+ * If the meta-data directory does not exist, the function will try to create
+ * it.
+ */
+static int dd_meta_data_save_text(struct dump_dir *dd, const char *name, const char *data)
+{
+ if (!dd->locked)
+ error_msg_and_die("dump_dir is not opened"); /* bug */
+
+ if (!str_is_correct_filename(name))
+ error_msg_and_die("Cannot save meta-data. '%s' is not a valid file name", name);
+
+ int dd_md_fd = dd_get_meta_data_dir_fd(dd, DD_MD_GET_CREATE);
+ if (dd_md_fd < 0)
+ {
+ error_msg("Can't save meta-data: '%s'", name);
+ return dd_md_fd;
+ }
+
+ char *tmp_name = xasprintf("~%s.tmp", name);
+
+ int ret = -1;
+ if (!save_binary_file_at(dd_md_fd, tmp_name, data, strlen(data), dd->dd_uid, dd->dd_gid, dd->mode))
+ goto finito;
+
+ /* man 2 rename
+ *
+ * If newpath already exists it will be atomically replaced (subject to a
+ * few conditions; see ERRORS below), so that there is no point at which
+ * another process attempting to access newpath will find it missing.
+ */
+ if (renameat(dd_md_fd, tmp_name, dd_md_fd, name))
+ {
+ ret = -errno;
+ perror_msg("Failed to move temporary file '%s' to '%s'", tmp_name, name);
+ goto finito;
+ }
+
+ ret = 0;
+
+finito:
+ free(tmp_name);
+ return ret;
+}
+
+int dd_set_owner(struct dump_dir *dd, uid_t owner)
+{
+ /* I was tempted to use the keyword static, but we should have reentracy
+ * always on mind. Who knows! */
+ char long_str[sizeof(long) * 3 + 2];
+
+ if (owner == (uid_t)-1)
+ {
+ owner = getuid();
+ if (owner < 0)
+ perror_msg_and_die("%s: getuid", __func__);
+ }
+
+ snprintf(long_str, sizeof(long_str), "%li", (long)owner);
+ const int ret = dd_meta_data_save_text(dd, META_DATA_FILE_OWNER, long_str);
+ if (ret < 0)
+ error_msg("The dump dir owner wasn't set to '%s'", long_str);
+ return ret;
+}
+
+int dd_set_no_owner(struct dump_dir *dd)
+{
+ uid_t no_owner_uid = (uid_t)-1;
+ int ret = get_no_owner_uid(&no_owner_uid);
+ if (ret < 0)
+ return ret;
+
+ return dd_set_owner(dd, no_owner_uid);
+}
+
+uid_t dd_get_owner(struct dump_dir *dd)
+{
+ static const long long MAX_UID_T = (1ULL << (sizeof(uid_t)*8 - 1)) - 1;
+
+ int dd_md_fd = dd_get_meta_data_dir_fd(dd, /*no create*/0);
+ if (dd_md_fd < 0)
+ {
+ log_info("No meta-data, using fs owner.");
+ return dd->dd_uid;
+ }
+
+ long long owner = -1;
+
+ int ret = read_number_from_file_at(dd_md_fd, META_DATA_FILE_OWNER, "UID",
+ sizeof(uid_t), MAX_UID_T, -1, &owner);
+
+ if (ret < 0)
+ {
+ if (ret != -ENOENT)
+ return ret;
+
+ log_info("No meta-data 'owner', using fs owner.");
+ return dd->dd_uid;
+ }
+
+ return (uid_t)owner;
+}
+
/* A helper function useful for traversing directories.
*
* DIR* d opendir(dir_fd); ... closedir(d); closes also dir_fd but we want to
@@ -1026,6 +1152,12 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int
goto fail;
}
+ if (dd_set_owner(dd, dd->dd_uid) < 0)
+ {
+ log_debug("Failed to initialized 'owner'");
+ goto fail;
+ }
+
if (uid != (uid_t)-1L)
{
dd->dd_uid = 0;
@@ -1087,6 +1219,11 @@ int dd_reset_ownership(struct dump_dir *dd)
if (dd_chown_meta_data(dd, dd->dd_uid, dd->dd_gid) != 0)
error_msg("Failed to reset ownership of meta-data");
+ /* We ignore failures above, so we will ignore failures here too.
+ * The meta-data owner already exist (created by dd_create_skeleton).
+ */
+ dd_set_owner(dd, dd->dd_uid);
+
return r;
}
@@ -1122,8 +1259,14 @@ void dd_create_basic_files(struct dump_dir *dd, uid_t uid, const char *chroot_di
free(time_str);
/* it doesn't make sense to create the uid file if uid == -1 */
+ /* and 'owner' is set since dd_create_skeleton */
if (uid != (uid_t)-1L)
{
+ /* Failure is not a problem here, because we still have the fs
+ * attributes and there is only a little chance that the old value
+ * gets lost. */
+ dd_set_owner(dd, uid);
+
snprintf(long_str, sizeof(long_str), "%li", (long)uid);
dd_save_text(dd, FILENAME_UID, long_str);
}
@@ -1436,6 +1579,9 @@ next:
dd->dd_gid = groups_gid;
}
+ if (chown_res == 0)
+ chown_res = dd_set_owner(dd, (long)dd->dd_uid);
+
return chown_res;
}
@@ -1812,7 +1958,45 @@ static bool uid_in_group(uid_t uid, gid_t gid)
int dd_stat_for_uid(struct dump_dir *dd, uid_t uid)
{
int ddstat = 0;
- if (uid == 0 || (dd->mode & S_IROTH))
+
+ if (uid == 0)
+ {
+ log_debug("directory accessible by super-user");
+ ddstat |= DD_STAT_ACCESSIBLE_BY_UID;
+ }
+
+#define DD_OWNER_FLAGS (DD_STAT_ACCESSIBLE_BY_UID | DD_STAT_OWNED_BY_UID)
+ if (dd->dd_uid == 0)
+ {
+ log_debug("directory owned by super-user: checking meta-data");
+
+ const uid_t owner = dd_get_owner(dd);
+
+ if (owner < 0)
+ goto fsattributes;
+
+ if (owner == uid)
+ {
+ log_debug("meta-data: %ld uid owns directory", (long)uid);
+ ddstat |= DD_OWNER_FLAGS;
+ goto finito;
+ }
+
+ uid_t no_owner_uid = (uid_t)-1;
+ int ret = get_no_owner_uid(&no_owner_uid);
+ if ( ret >= 0
+ && owner == no_owner_uid)
+ {
+ log_debug("meta-data: directory is accessible by %ld uid", (long)uid);
+ ddstat |= DD_STAT_ACCESSIBLE_BY_UID;
+ ddstat |= DD_STAT_NO_OWNER;
+ }
+
+ goto finito;
+ }
+
+fsattributes:
+ if (dd->mode & S_IROTH)
{
log_debug("directory is accessible by %ld uid", (long)uid);
ddstat |= DD_STAT_ACCESSIBLE_BY_UID;
@@ -1824,11 +2008,14 @@ int dd_stat_for_uid(struct dump_dir *dd, uid_t uid)
if (uid_in_group(uid, dd->dd_gid))
#endif
{
- log_debug("%ld uid owns directory", (long)uid);
- ddstat |= DD_STAT_ACCESSIBLE_BY_UID;
- ddstat |= DD_STAT_OWNED_BY_UID;
+ log_debug("fs attributes: %ld uid owns directory", (long)uid);
+ ddstat |= DD_OWNER_FLAGS;
}
+#undef DD_OWNER_FLAGS
+
+finito:
+ log_debug("UID=%d, %s: %o", uid, dd->dd_dirname, ddstat);
return ddstat;
}
diff --git a/tests/dump_dir.at b/tests/dump_dir.at
index 4bf479e..cc0148c 100644
--- a/tests/dump_dir.at
+++ b/tests/dump_dir.at
@@ -203,6 +203,11 @@ int main(int argc, char **argv)
assert(fstatat(dd->dd_fd, ".libreport", &path_md_st, 0) == 0);
assert(md_st.st_ino = path_md_st.st_ino);
+ struct stat owner_md_st;
+ assert(fstatat(dd->dd_md_fd, "owner", &owner_md_st, 0) == 0);
+ assert((dd_st.st_mode & 0666) == (owner_md_st.st_mode & 0666));
+ assert(geteuid() == dd_get_owner(dd));
+
dd_create_basic_files(dd, (uid_t)-1, NULL);
dd_save_text(dd, FILENAME_TYPE, "attest");
@@ -329,6 +334,111 @@ int main(int argc, char **argv)
}
]])
+## -------- ##
+## dd_owner ##
+## -------- ##
+
+AT_TESTFUN([dd_owner],
+[[
+#include "internal_libreport.h"
+#include <errno.h>
+#include <assert.h>
+
+int main(int argc, char **argv)
+{
+ g_verbose = 3;
+
+ char template[] = "/tmp/XXXXXX/dump_dir";
+
+ char *last_slash = strrchr(template, '/');
+ *last_slash = '\0';
+
+ if (mkdtemp(template) == NULL) {
+ perror("mkdtemp()");
+ return EXIT_FAILURE;
+ }
+
+ *last_slash = '/';
+
+ printf("Dump dir path: %s\n", template);
+
+ {
+ fprintf(stderr, "TEST === default owner\n");
+ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640);
+ assert(dd != NULL);
+
+ assert(geteuid() == dd_get_owner(dd));
+
+ assert(dd_delete(dd) == 0);
+ }
+
+ {
+ fprintf(stderr, "TEST === create basic files w/o UID\n");
+ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640);
+ assert(dd != NULL);
+
+ dd_create_basic_files(dd, (uid_t)-1, NULL);
+ assert(geteuid() == dd_get_owner(dd));
+
+ assert(dd_delete(dd) == 0);
+ }
+
+ {
+ fprintf(stderr, "TEST === create basic files with UID\n");
+ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640);
+ assert(dd != NULL);
+
+ dd_create_basic_files(dd, (geteuid() + 1), NULL);
+ assert((geteuid() + 1) == dd_get_owner(dd));
+
+ assert(dd_delete(dd) == 0);
+ }
+
+ {
+ fprintf(stderr, "TEST === set no owner\n");
+ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640);
+ assert(dd != NULL);
+
+ dd_set_no_owner(dd);
+
+ struct passwd *nobody_pw= getpwnam("nobody");
+ assert(nobody_pw != NULL);
+ assert(nobody_pw->pw_uid == dd_get_owner(dd));
+
+ assert(dd_delete(dd) == 0);
+ }
+
+ {
+ fprintf(stderr, "TEST === set artibrary owner\n");
+ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640);
+ assert(dd != NULL);
+
+ dd_set_owner(dd, (geteuid() + 2));
+ assert((geteuid() + 2) == dd_get_owner(dd));
+
+ assert(dd_delete(dd) == 0);
+ }
+
+ {
+ fprintf(stderr, "TEST === chown no owner\n");
+ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640);
+ assert(dd != NULL);
+
+ dd_set_no_owner(dd);
+ assert(geteuid() != dd_get_owner(dd));
+
+ dd_chown(dd, geteuid());
+ assert(geteuid() == dd_get_owner(dd));
+
+ assert(dd_delete(dd) == 0);
+ }
+ *last_slash = '\0';
+ assert(rmdir(template) == 0);
+
+ return EXIT_SUCCESS;
+}
+]])
+
## -------------- ##
## recursive_lock ##
## -------------- ##
--
2.1.0