From e101a9eb0fab6725e0a239a92f9b50822c494a3e Mon Sep 17 00:00:00 2001 From: Rian Hunter Date: Fri, 12 Oct 2018 19:45:06 -0700 Subject: lib/canonicalize: do restricted canonicalize in a subprocess Accessing FUSE mounts require suid/sgid (saved uid) to be equal to the owner of the mount. If mount is running as a setuid process, swapping creds by only setting the euid/egid isn't enough to change the suid/sgid as well. We must do a full setuid()/setgid(), but that removes our ability to re-assume the identity of the original euid. The solution is swap creds in a child process, preserving the creds of the parent. [kzak@redhat.com: - use switch() rather than if() for fork - use all-io.h - close unused pipe[] ends - be more strict about used types] Addresses: https://github.com/karelzak/util-linux/pull/705 Co-Author: Karel Zak Signed-off-by: Karel Zak --- lib/canonicalize.c | 99 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 77 insertions(+), 22 deletions(-) (limited to 'lib') diff --git a/lib/canonicalize.c b/lib/canonicalize.c index f3a2a3af2..e85e0357c 100644 --- a/lib/canonicalize.c +++ b/lib/canonicalize.c @@ -14,9 +14,11 @@ #include #include #include +#include #include "canonicalize.h" #include "pathnames.h" +#include "all-io.h" /* * Converts private "dm-N" names to "/dev/mapper/" @@ -140,39 +142,92 @@ char *canonicalize_path(const char *path) char *canonicalize_path_restricted(const char *path) { - char *canonical, *dmname; - int errsv; - uid_t euid; - gid_t egid; + char *canonical = NULL; + int errsv = 0; + int pipes[2]; + ssize_t len; + pid_t pid; if (!path || !*path) return NULL; - euid = geteuid(); - egid = getegid(); - - /* drop permissions */ - if (setegid(getgid()) < 0 || seteuid(getuid()) < 0) + if (pipe(pipes) != 0) return NULL; - errsv = errno = 0; - - canonical = realpath(path, NULL); - if (!canonical) - errsv = errno; - else if (is_dm_devname(canonical, &dmname)) { - char *dm = canonicalize_dm_name(dmname); - if (dm) { - free(canonical); - canonical = dm; + /* + * To accurately assume identity of getuid() we must use setuid() + * but if we do that, we lose ability to reassume euid of 0, so + * we fork to do the check to keep euid intact. + */ + pid = fork(); + switch (pid) { + case -1: + close(pipes[0]); + close(pipes[1]); + return NULL; /* fork error */ + case 0: + close(pipes[0]); /* close unused end */ + pipes[0] = -1; + errno = 0; + + /* drop permissions */ + if (setgid(getgid()) < 0 || setuid(getuid()) < 0) + canonical = NULL; /* failed */ + else { + char *dmname = NULL; + + canonical = realpath(path, NULL); + if (canonical && is_dm_devname(canonical, &dmname)) { + char *dm = canonicalize_dm_name(dmname); + if (dm) { + free(canonical); + canonical = dm; + } + } } + + len = canonical ? strlen(canonical) : errno ? -errno : -EINVAL; + + /* send lenght or errno */ + write_all(pipes[1], (char *) &len, sizeof(len)); + if (canonical) + write_all(pipes[1], canonical, len); + exit(0); + default: + break; } - /* restore */ - if (setegid(egid) < 0 || seteuid(euid) < 0) { + close(pipes[1]); /* close unused end */ + pipes[1] = -1; + + /* read size or -errno */ + if (read_all(pipes[0], (char *) &len, sizeof(len)) != sizeof(len)) + goto done; + if (len < 0) { + errsv = -len; + goto done; + } + + canonical = malloc(len + 1); + if (!canonical) { + errsv = ENOMEM; + goto done; + } + /* read path */ + if (read_all(pipes[0], canonical, len) != len) { + errsv = errno; + goto done; + } + canonical[len] = '\0'; +done: + if (errsv) { free(canonical); - return NULL; + canonical = NULL; } + close(pipes[0]); + + /* We make a best effort to reap child */ + waitpid(pid, NULL, 0); errno = errsv; return canonical; -- cgit v1.2.3-55-g7522