This is largely code motion. --- src/appliance.c | 40 +++++++----------------------------- src/guestfs-internal.h | 1 + src/tmpdirs.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 33 deletions(-) diff --git a/src/appliance.c b/src/appliance.c index 2cf6374..d293c2b 100644 --- a/src/appliance.c +++ b/src/appliance.c @@ -48,7 +48,7 @@ static int dir_contains_files (guestfs_h *g, const char *dir, ...); static int contains_old_style_appliance (guestfs_h *g, const char *path, void *data); static int contains_fixed_appliance (guestfs_h *g, const char *path, void *data); static int contains_supermin_appliance (guestfs_h *g, const char *path, void *data); -static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, uid_t uid, char **kernel, char **initrd, char **appliance); +static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, char **kernel, char **initrd, char **appliance); static int run_supermin_build (guestfs_h *g, const char *lockfile, const char *appliancedir, const char *supermin_path); /** @@ -133,7 +133,6 @@ build_appliance (guestfs_h *g, char **appliance) { int r; - uid_t uid = geteuid (); CLEANUP_FREE char *supermin_path = NULL; CLEANUP_FREE char *path = NULL; @@ -144,7 +143,7 @@ build_appliance (guestfs_h *g, if (r == 1) /* Step (2): build supermin appliance. */ - return build_supermin_appliance (g, supermin_path, uid, + return build_supermin_appliance (g, supermin_path, kernel, initrd, appliance); /* Step (3). */ @@ -212,43 +211,18 @@ contains_supermin_appliance (guestfs_h *g, const char *path, void *data) static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, - uid_t uid, char **kernel, char **initrd, char **appliance) { - CLEANUP_FREE char *tmpdir = guestfs_get_cachedir (g); CLEANUP_FREE char *cachedir = NULL, *lockfile = NULL, *appliancedir = NULL; - struct stat statbuf; - cachedir = safe_asprintf (g, "%s/.guestfs-%ju", tmpdir, (uintmax_t) uid); - lockfile = safe_asprintf (g, "%s/lock", cachedir); + cachedir = guestfs_int_lazy_make_supermin_appliance_dir (g); + if (cachedir == NULL) + return -1; + appliancedir = safe_asprintf (g, "%s/appliance.d", cachedir); + lockfile = safe_asprintf (g, "%s/lock", cachedir); - ignore_value (mkdir (cachedir, 0755)); - ignore_value (chmod (cachedir, 0755)); /* RHBZ#921292 */ - - /* See if the cache directory exists and passes some simple checks - * to make sure it has not been tampered with. - */ - if (lstat (cachedir, &statbuf) == -1) - return 0; - if (statbuf.st_uid != uid) { - error (g, _("security: cached appliance %s is not owned by UID %ju"), - cachedir, (uintmax_t) uid); - return -1; - } - if (!S_ISDIR (statbuf.st_mode)) { - error (g, _("security: cached appliance %s is not a directory (mode %o)"), - cachedir, statbuf.st_mode); - return -1; - } - if ((statbuf.st_mode & 0022) != 0) { - error (g, _("security: cached appliance %s is writable by group or other (mode %o)"), - cachedir, statbuf.st_mode); - return -1; - } - - (void) utimes (cachedir, NULL); debug (g, "begin building supermin appliance"); /* Build the appliance if it needs to be built. */ diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index dbd0a98..d325f50 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -758,6 +758,7 @@ extern int guestfs_int_set_env_tmpdir (guestfs_h *g, const char *tmpdir); extern int guestfs_int_set_env_runtimedir (guestfs_h *g, const char *runtimedir); extern int guestfs_int_lazy_make_tmpdir (guestfs_h *g); extern int guestfs_int_lazy_make_sockdir (guestfs_h *g); +extern char *guestfs_int_lazy_make_supermin_appliance_dir (guestfs_h *g); extern void guestfs_int_remove_tmpdir (guestfs_h *g); extern void guestfs_int_remove_sockdir (guestfs_h *g); extern void guestfs_int_recursive_remove_dir (guestfs_h *g, const char *dir); diff --git a/src/tmpdirs.c b/src/tmpdirs.c index 293e4ea..6c8fe0d 100644 --- a/src/tmpdirs.c +++ b/src/tmpdirs.c @@ -213,6 +213,61 @@ guestfs_int_lazy_make_sockdir (guestfs_h *g) } /** + * Create the supermin appliance directory under cachedir, if it does + * not exist. + * + * Sanity-check that the permissions on the cachedir are safe, in case + * it has been pre-created maliciously or tampered with. + * + * Returns the directory name which the caller must free. + */ +char * +guestfs_int_lazy_make_supermin_appliance_dir (guestfs_h *g) +{ + CLEANUP_FREE char *tmpdir = guestfs_get_cachedir (g); + char *ret = NULL; + struct stat statbuf; + uid_t uid = geteuid (); + + ret = safe_asprintf (g, "%s/.guestfs-%ju", tmpdir, (uintmax_t) uid); + + ignore_value (mkdir (ret, 0755)); + ignore_value (chmod (ret, 0755)); /* RHBZ#921292 */ + + /* See if the cache directory exists and passes some simple checks + * to make sure it has not been tampered with. + */ + if (lstat (ret, &statbuf) == -1) { + perrorf (g, _("stat: %s"), ret); + free (ret); + return NULL; + } + if (statbuf.st_uid != uid) { + error (g, _("security: cached appliance %s is not owned by UID %ju"), + ret, (uintmax_t) uid); + free (ret); + return NULL; + } + if (!S_ISDIR (statbuf.st_mode)) { + error (g, _("security: cached appliance %s is not a directory (mode %o)"), + ret, statbuf.st_mode); + free (ret); + return NULL; + } + if ((statbuf.st_mode & 0022) != 0) { + error (g, _("security: cached appliance %s is writable by group or other (mode %o)"), + ret, statbuf.st_mode); + free (ret); + return NULL; + } + + /* "Touch" the directory. */ + ignore_value (utimes (ret, NULL)); + + return ret; +} + +/** * Recursively remove a temporary directory. If removal fails, just * return (it's a temporary directory so it'll eventually be cleaned * up by a temp cleaner). -- 2.7.4