CVE-2015-1340: Fix race condition between fchown and chmod in idmapset

Shifting a container filesystem in an environment where a user can
modify the container's filesystem as it's being shifted (for example if
the LXD server's / was shared over the network) would allow them to make
use of a race (TOCTOU) between the chown and chmod operations, allowing
for an arbitrary path to be chmod-ed rather than the planned container
path.

The fix is to use a file descriptor to the entry being processed,
validate that the entry itself is sane and then interact with the fd.

This is CVE-2015-1340

Reported-by: Seth Arnold
Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>

shared/idmapset.go → shared/idmapset_linux.go

@@ -223,6 +223,8 @@ func GetOwner(path string) (int, int, error) {
 }
 
 func (set *IdmapSet) doUidshiftIntoContainer(dir string, testmode bool, how string) error {
+	dir = strings.TrimRight(dir, "/")
+
 	convert := func(path string, fi os.FileInfo, err error) (e error) {
 		uid, gid, err := GetOwner(path)
 		if err != nil {
@@ -238,15 +240,9 @@ func (set *IdmapSet) doUidshiftIntoContainer(dir string, testmode bool, how stri
 		if testmode {
 			fmt.Printf("I would shift %q to %d %d\n", path, newuid, newgid)
 		} else {
-			err = os.Lchown(path, int(newuid), int(newgid))
-			if err == nil {
-				m := fi.Mode()
-				if m&os.ModeSymlink == 0 {
-					err = os.Chmod(path, m)
-					if err != nil {
-						fmt.Printf("Error resetting mode on %q, continuing\n", path)
-					}
-				}
+			err = ShiftOwner(dir, path, int(newuid), int(newgid))
+			if err != nil {
+				return err
 			}
 		}
 		return nil

shared/util_linux.go

@@ -17,15 +17,24 @@ import (
 /*
 #define _GNU_SOURCE
 #include <sys/types.h>
+#include <sys/stat.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <grp.h>
 #include <pty.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #include <string.h>
 #include <stdio.h>
-#include <stdio.h>
+
+#ifndef AT_SYMLINK_FOLLOW
+#define AT_SYMLINK_FOLLOW    0x400
+#endif
+
+#ifndef AT_EMPTY_PATH
+#define AT_EMPTY_PATH       0x1000
+#endif
 
 // This is an adaption from https://codereview.appspot.com/4589049, to be
 // included in the stdlib with the stdlib's license.
@@ -107,9 +116,82 @@ void create_pipe(int *master, int *slave) {
 	*master = pipefd[0];
 	*slave = pipefd[1];
 }
+
+int shiftowner(char *basepath, char *path, int uid, int gid) {
+	struct stat sb;
+	int fd, r;
+	char fdpath[PATH_MAX];
+	char realpath[PATH_MAX];
+
+	fd = open(path, O_PATH|O_NOFOLLOW);
+	if (fd < 0 ) {
+		perror("Failed open");
+		return 1;
+	}
+
+	r = sprintf(fdpath, "/proc/self/fd/%d", fd);
+	if (r < 0) {
+		perror("Failed sprintf");
+		close(fd);
+		return 1;
+	}
+
+	r = readlink(fdpath, realpath, PATH_MAX);
+	if (r < 0) {
+		perror("Failed readlink");
+		close(fd);
+		return 1;
+	}
+
+	if (strlen(realpath) < strlen(basepath)) {
+		printf("Invalid path, source is outside of basepath.\n");
+		close(fd);
+		return 1;
+	}
+
+	if (strncmp(realpath, basepath, strlen(basepath))) {
+		printf("Invalid path, source is outside of basepath.\n");
+		close(fd);
+		return 1;
+	}
+
+	r = fstat(fd, &sb);
+	if (r < 0) {
+		perror("Failed fstat");
+		close(fd);
+		return 1;
+	}
+
+	r = fchownat(fd, "", uid, gid, AT_EMPTY_PATH|AT_SYMLINK_NOFOLLOW);
+	if (r < 0) {
+		perror("Failed chown");
+		close(fd);
+		return 1;
+	}
+
+	if (!S_ISLNK(sb.st_mode)) {
+		r = chmod(fdpath, sb.st_mode);
+		if (r < 0) {
+			perror("Failed chmod");
+			close(fd);
+			return 1;
+		}
+	}
+
+	close(fd);
+	return 0;
+}
 */
 import "C"
 
+func ShiftOwner(basepath string, path string, uid int, gid int) error {
+	r := C.shiftowner(C.CString(basepath), C.CString(path), C.int(uid), C.int(gid))
+	if r != 0 {
+		return fmt.Errorf("Failed to change ownership of: %s", path)
+	}
+	return nil
+}
+
 func OpenPty(uid, gid int) (master *os.File, slave *os.File, err error) {
 	fd_master := C.int(-1)
 	fd_slave := C.int(-1)