[lxc-devel] [lxd/master] Tighten directory ownership and permissions

stgraber on Github lxc-bot at linuxcontainers.org
Wed Sep 11 14:35:17 UTC 2019


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/20190911/16b92282/attachment.bin>
-------------- next part --------------
From fa1eccce8e7c9e1520978e648b2b460e9b253894 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Wed, 11 Sep 2019 15:30:45 +0100
Subject: [PATCH 1/2] lxd: Require "ip" be installed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #6185

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/main_daemon.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/main_daemon.go b/lxd/main_daemon.go
index 68a878cb96..64876128a9 100644
--- a/lxd/main_daemon.go
+++ b/lxd/main_daemon.go
@@ -49,7 +49,7 @@ func (c *cmdDaemon) Run(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("This must be run as root")
 	}
 
-	neededPrograms := []string{"setfacl", "rsync", "tar", "unsquashfs", "xz"}
+	neededPrograms := []string{"ip", "rsync", "tar", "unsquashfs", "xz"}
 	for _, p := range neededPrograms {
 		_, err := exec.LookPath(p)
 		if err != nil {

From 9914ef9f03263a885b3e8e96916c019a16285357 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Wed, 11 Sep 2019 15:31:20 +0100
Subject: [PATCH 2/2] lxd/containers: Tigthen directory ownership
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Rather than the old ACLs we had, or the 0711 traversal trick we had in
place, tighten things further by using 0100 and setting the owner to the
container's root on startup and undo on shutdown.

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index f0fdd9c791..d9835a142e 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -2467,8 +2467,8 @@ func (c *containerLXC) startCommon() (string, []func() error, error) {
 		return "", postStartHooks, err
 	}
 
-	// Undo liblxc modifying container directory ownership
-	err = os.Chown(c.Path(), 0, 0)
+	// Set ownership to match container root
+	currentIdmapset, err := c.CurrentIdmap()
 	if err != nil {
 		if ourStart {
 			c.StorageStop()
@@ -2476,15 +2476,21 @@ func (c *containerLXC) startCommon() (string, []func() error, error) {
 		return "", postStartHooks, err
 	}
 
-	// Set right permission to allow traversal
-	var mode os.FileMode
-	if c.isCurrentlyPrivileged() {
-		mode = 0700
-	} else {
-		mode = 0711
+	uid := int64(0)
+	if currentIdmapset != nil {
+		uid, _ = currentIdmapset.ShiftFromNs(0, 0)
+	}
+
+	err = os.Chown(c.Path(), int(uid), 0)
+	if err != nil {
+		if ourStart {
+			c.StorageStop()
+		}
+		return "", postStartHooks, err
 	}
 
-	err = os.Chmod(c.Path(), mode)
+	// We only need traversal by root in the container
+	err = os.Chmod(c.Path(), 0100)
 	if err != nil {
 		if ourStart {
 			c.StorageStop()
@@ -2988,8 +2994,27 @@ func (c *containerLXC) OnStop(target string) error {
 	// Make sure we can't call go-lxc functions by mistake
 	c.fromHook = true
 
+	// Remove directory ownership (to avoid issue if uidmap is re-used)
+	err := os.Chown(c.Path(), 0, 0)
+	if err != nil {
+		if op != nil {
+			op.Done(err)
+		}
+
+		return err
+	}
+
+	err = os.Chmod(c.Path(), 0100)
+	if err != nil {
+		if op != nil {
+			op.Done(err)
+		}
+
+		return err
+	}
+
 	// Stop the storage for this container
-	_, err := c.StorageStop()
+	_, err = c.StorageStop()
 	if err != nil {
 		if op != nil {
 			op.Done(err)


More information about the lxc-devel mailing list