[lxc-devel] [lxd/master] Fix file pull and improve nesting experience

stgraber on Github lxc-bot at linuxcontainers.org
Thu Apr 7 21:47:17 UTC 2016


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160407/fd5b901c/attachment.bin>
-------------- next part --------------
From 6ccc0be2d1804d355f64ba932bdf9ecee735c8f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 7 Apr 2016 17:18:58 -0400
Subject: [PATCH 1/2] Fix uid, gid and mode of retrieved files
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #1869

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/container.go      |  2 +-
 lxd/container_file.go | 22 +++--------------
 lxd/container_lxc.go  | 67 +++++++++++++++++++++++++++++++++++++++++++--------
 lxd/nsexec.go         | 21 +++++++++-------
 4 files changed, 74 insertions(+), 38 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index b3d8ac5..dd10b30 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -343,7 +343,7 @@ type container interface {
 	ConfigKeySet(key string, value string) error
 
 	// File handling
-	FilePull(srcpath string, dstpath string) error
+	FilePull(srcpath string, dstpath string) (int, int, os.FileMode, error)
 	FilePush(srcpath string, dstpath string, uid int, gid int, mode os.FileMode) error
 
 	// Command execution
diff --git a/lxd/container_file.go b/lxd/container_file.go
index da1fd30..06590a5 100644
--- a/lxd/container_file.go
+++ b/lxd/container_file.go
@@ -7,8 +7,6 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
-	"strconv"
-	"syscall"
 
 	"github.com/gorilla/mux"
 
@@ -52,27 +50,15 @@ func containerFileGet(c container, path string, r *http.Request) Response {
 	defer temp.Close()
 
 	// Pul the file from the container
-	err = c.FilePull(path, temp.Name())
+	uid, gid, mode, err := c.FilePull(path, temp.Name())
 	if err != nil {
 		return InternalError(err)
 	}
 
-	// Get file attributes
-	fi, err := temp.Stat()
-	if err != nil {
-		return SmartError(err)
-	}
-
-	/*
-	 * Unfortunately, there's no portable way to do this:
-	 * https://groups.google.com/forum/#!topic/golang-nuts/tGYjYyrwsGM
-	 * https://groups.google.com/forum/#!topic/golang-nuts/ywS7xQYJkHY
-	 */
-	sb := fi.Sys().(*syscall.Stat_t)
 	headers := map[string]string{
-		"X-LXD-uid":  strconv.FormatUint(uint64(sb.Uid), 10),
-		"X-LXD-gid":  strconv.FormatUint(uint64(sb.Gid), 10),
-		"X-LXD-mode": fmt.Sprintf("%04o", fi.Mode()&os.ModePerm),
+		"X-LXD-uid":  fmt.Sprintf("%d", uid),
+		"X-LXD-gid":  fmt.Sprintf("%d", gid),
+		"X-LXD-mode": fmt.Sprintf("%04o", mode),
 	}
 
 	// Make a file response struct
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 7262e00..b9cef3d 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -2817,12 +2817,12 @@ func (c *containerLXC) templateApplyNow(trigger string) error {
 	return nil
 }
 
-func (c *containerLXC) FilePull(srcpath string, dstpath string) error {
+func (c *containerLXC) FilePull(srcpath string, dstpath string) (int, int, os.FileMode, error) {
 	// Setup container storage if needed
 	if !c.IsRunning() {
 		err := c.StorageStart()
 		if err != nil {
-			return err
+			return -1, -1, 0, err
 		}
 	}
 
@@ -2840,23 +2840,60 @@ func (c *containerLXC) FilePull(srcpath string, dstpath string) error {
 	if !c.IsRunning() {
 		err := c.StorageStop()
 		if err != nil {
-			return err
+			return -1, -1, 0, err
 		}
 	}
 
+	uid := -1
+	gid := -1
+	mode := -1
+
 	// Process forkgetfile response
-	if string(out) != "" {
-		if strings.HasPrefix(string(out), "error:") {
-			return fmt.Errorf(strings.TrimPrefix(strings.TrimSuffix(string(out), "\n"), "error: "))
+	for _, line := range strings.Split(strings.TrimRight(string(out), "\n"), "\n") {
+		if line == "" {
+			continue
 		}
 
-		for _, line := range strings.Split(strings.TrimRight(string(out), "\n"), "\n") {
-			shared.Debugf("forkgetfile: %s", line)
+		// Extract errors
+		if strings.HasPrefix(line, "error: ") {
+			return -1, -1, 0, fmt.Errorf(strings.TrimPrefix(line, "error: "))
 		}
+
+		// Extract the uid
+		if strings.HasPrefix(line, "uid: ") {
+			uid, err = strconv.Atoi(strings.TrimPrefix(line, "uid: "))
+			if err != nil {
+				return -1, -1, 0, err
+			}
+
+			continue
+		}
+
+		// Extract the gid
+		if strings.HasPrefix(line, "gid: ") {
+			gid, err = strconv.Atoi(strings.TrimPrefix(line, "gid: "))
+			if err != nil {
+				return -1, -1, 0, err
+			}
+
+			continue
+		}
+
+		// Extract the mode
+		if strings.HasPrefix(line, "mode: ") {
+			mode, err = strconv.Atoi(strings.TrimPrefix(line, "mode: "))
+			if err != nil {
+				return -1, -1, 0, err
+			}
+
+			continue
+		}
+
+		shared.Debugf("forkgetfile: %s", line)
 	}
 
 	if err != nil {
-		return fmt.Errorf(
+		return -1, -1, 0, fmt.Errorf(
 			"Error calling 'lxd forkgetfile %s %d %s': err='%v'",
 			dstpath,
 			c.InitPID(),
@@ -2864,7 +2901,17 @@ func (c *containerLXC) FilePull(srcpath string, dstpath string) error {
 			err)
 	}
 
-	return nil
+	// Unap uid and gid if needed
+	idmapset, err := c.LastIdmapSet()
+	if err != nil {
+		return -1, -1, 0, err
+	}
+
+	if idmapset != nil {
+		uid, gid = idmapset.ShiftFromNs(uid, gid)
+	}
+
+	return uid, gid, os.FileMode(mode), nil
 }
 
 func (c *containerLXC) FilePush(srcpath string, dstpath string, uid int, gid int, mode os.FileMode) error {
diff --git a/lxd/nsexec.go b/lxd/nsexec.go
index 170dd54..74073d4 100644
--- a/lxd/nsexec.go
+++ b/lxd/nsexec.go
@@ -121,6 +121,7 @@ int manip_file_in_ns(char *rootfs, int pid, char *host, char *container, bool is
 	int host_fd, container_fd;
 	int ret = -1;
 	int container_open_flags;
+	struct stat st;
 
 	host_fd = open(host, O_RDWR);
 	if (host_fd < 0) {
@@ -167,9 +168,19 @@ int manip_file_in_ns(char *rootfs, int pid, char *host, char *container, bool is
 		}
 
 		ret = 0;
-	} else
+	} else {
 		ret = copy(host_fd, container_fd);
 
+		if (fstat(container_fd, &st) < 0) {
+			perror("error: stat");
+			goto close_container;
+		}
+
+		fprintf(stderr, "uid: %ld\n", (long)st.st_uid);
+		fprintf(stderr, "gid: %ld\n", (long)st.st_gid);
+		fprintf(stderr, "mode: %ld\n", (unsigned long)st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO));
+	}
+
 close_container:
 	close(container_fd);
 close_host:
@@ -346,14 +357,6 @@ void forkdofile(char *buf, char *cur, bool is_put, ssize_t size) {
 		mode = atoi(cur);
 	}
 
-	printf("command: %s\n", command);
-	printf("source: %s\n", source);
-	printf("pid: %d\n", pid);
-	printf("target: %s\n", target);
-	printf("uid: %d\n", uid);
-	printf("gid: %d\n", gid);
-	printf("mode: %d\n", mode);
-
 	_exit(manip_file_in_ns(rootfs, pid, source, target, is_put, uid, gid, mode));
 }
 

From 4b355b64fff28492f367cf2fa2a085006a61a8ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 7 Apr 2016 17:46:31 -0400
Subject: [PATCH 2/2] lxd init: Allow setting security.privileged when nesting
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/main.go | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/lxd/main.go b/lxd/main.go
index 9f90613..2065163 100644
--- a/lxd/main.go
+++ b/lxd/main.go
@@ -575,6 +575,7 @@ func cmdWaitReady() error {
 }
 
 func cmdInit() error {
+	var defaultPrivileged int // controls whether we set security.privileged=true
 	var storageBackend string // dir or zfs
 	var storageMode string    // existing, loop or device
 	var storageLoopSize int   // Size in GB
@@ -584,6 +585,10 @@ func cmdInit() error {
 	var networkPort int       // Port
 	var trustPassword string  // Trust password
 
+	// Detect userns
+	defaultPrivileged = -1
+	runningInUserns = shared.RunningInUserNS()
+
 	// Only root should run this
 	if os.Geteuid() != 0 {
 		return fmt.Errorf("This must be run as root")
@@ -785,6 +790,25 @@ func cmdInit() error {
 			}
 		}
 
+		if runningInUserns {
+			fmt.Printf(`
+We detected that you are running inside an unprivileged container.
+This means that unless you manually configured your host otherwise,
+you will not have enough uid and gid to allocate to your containers.
+
+LXD can re-use your container's own allocation to avoid the problem.
+Doing so makes your nested containers slightly less safe as they could
+in theory attack their parent container and gain more privileges than
+they otherwise would.
+
+`)
+			if askBool("Would you like to have your containers share their parent's allocation (yes/no)? ") {
+				defaultPrivileged = 1
+			} else {
+				defaultPrivileged = 0
+			}
+		}
+
 		if askBool("Would you like LXD to be available over the network (yes/no)? ") {
 			networkAddress = askString("Address to bind LXD to (not including port): ")
 			networkPort = askInt("Port to bind LXD to (8443 recommended): ", 1, 65535)
@@ -847,6 +871,17 @@ func cmdInit() error {
 		}
 	}
 
+	if defaultPrivileged == 0 {
+		err = c.SetProfileConfigItem("default", "security.privileged", "")
+		if err != nil {
+			return err
+		}
+	} else if defaultPrivileged == 1 {
+		err = c.SetProfileConfigItem("default", "security.privileged", "true")
+		if err != nil {
+		}
+	}
+
 	if networkAddress != "" {
 		_, err = c.SetServerConfig("core.https_address", fmt.Sprintf("%s:%d", networkAddress, networkPort))
 		if err != nil {


More information about the lxc-devel mailing list