[lxc-devel] [lxd/master] Bugfixes

stgraber on Github lxc-bot at linuxcontainers.org
Tue Jan 24 22:35:29 UTC 2017


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/20170124/5add4c7c/attachment.bin>
-------------- next part --------------
From 25a5b2be3790026273ddbc2cf13934d4cf63ef51 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 24 Jan 2017 15:45:59 -0500
Subject: [PATCH 1/4] Don't attempt to read xattrs from symlinks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #2801

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 6a9c7e0..a67dc16 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -4798,10 +4798,12 @@ func (c *containerLXC) tarStoreFile(linkmap map[uint64]string, offset int, tw *t
 		}
 	}
 
-	// Handle xattrs.
-	hdr.Xattrs, err = shared.GetAllXattr(path)
-	if err != nil {
-		return fmt.Errorf("failed to read xattr: %s", err)
+	// Handle xattrs (for real files only)
+	if link == "" {
+		hdr.Xattrs, err = shared.GetAllXattr(path)
+		if err != nil {
+			return fmt.Errorf("failed to read xattr: %s", err)
+		}
 	}
 
 	if err := tw.WriteHeader(hdr); err != nil {

From faa53197550180b80090312bc951c3fe34887684 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 24 Jan 2017 16:32:32 -0500
Subject: [PATCH 2/4] Remove GroupName function and add UserId one
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>
---
 shared/util_linux.go | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/shared/util_linux.go b/shared/util_linux.go
index 9ce2fb0..87b44d6 100644
--- a/shared/util_linux.go
+++ b/shared/util_linux.go
@@ -19,18 +19,19 @@ import (
 // #cgo LDFLAGS: -lutil -lpthread
 /*
 #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 <grp.h>
 #include <limits.h>
 #include <poll.h>
-#include <string.h>
+#include <pty.h>
+#include <pwd.h>
 #include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
 
 #ifndef AT_SYMLINK_FOLLOW
 #define AT_SYMLINK_FOLLOW    0x400
@@ -277,36 +278,36 @@ func Pipe() (master *os.File, slave *os.File, err error) {
 	return master, slave, nil
 }
 
-// GroupName is an adaption from https://codereview.appspot.com/4589049.
-func GroupName(gid int) (string, error) {
-	var grp C.struct_group
-	var result *C.struct_group
+// UserId is an adaption from https://codereview.appspot.com/4589049.
+func UserId(name string) (int, error) {
+	var pw C.struct_passwd
+	var result *C.struct_passwd
 
-	bufSize := C.size_t(C.sysconf(C._SC_GETGR_R_SIZE_MAX))
+	bufSize := C.size_t(C.sysconf(C._SC_GETPW_R_SIZE_MAX))
 	buf := C.malloc(bufSize)
 	if buf == nil {
-		return "", fmt.Errorf("allocation failed")
+		return -1, fmt.Errorf("allocation failed")
 	}
 	defer C.free(buf)
 
-	// mygetgrgid_r is a wrapper around getgrgid_r to
-	// to avoid using gid_t because C.gid_t(gid) for
-	// unknown reasons doesn't work on linux.
-	rv := C.mygetgrgid_r(C.int(gid),
-		&grp,
+	cname := C.CString(name)
+	defer C.free(unsafe.Pointer(cname))
+
+	rv := C.getpwnam_r(cname,
+		&pw,
 		(*C.char)(buf),
 		bufSize,
 		&result)
 
 	if rv != 0 {
-		return "", fmt.Errorf("failed group lookup: %s", syscall.Errno(rv))
+		return -1, fmt.Errorf("failed user lookup: %s", syscall.Errno(rv))
 	}
 
 	if result == nil {
-		return "", fmt.Errorf("unknown group %d", gid)
+		return -1, fmt.Errorf("unknown user %s", name)
 	}
 
-	return C.GoString(result.gr_name), nil
+	return int(C.int(result.pw_uid)), nil
 }
 
 // GroupId is an adaption from https://codereview.appspot.com/4589049.
@@ -321,9 +322,6 @@ func GroupId(name string) (int, error) {
 	}
 	defer C.free(buf)
 
-	// mygetgrgid_r is a wrapper around getgrgid_r to
-	// to avoid using gid_t because C.gid_t(gid) for
-	// unknown reasons doesn't work on linux.
 	cname := C.CString(name)
 	defer C.free(unsafe.Pointer(cname))
 

From 7fd66ac74411afa108cdd70f01bdfd3fc24da9b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 24 Jan 2017 14:57:19 -0500
Subject: [PATCH 3/4] network: Update permissions of network directories
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

So that the unprivileged dnsmasq can read its config on reload.

Closes #2804

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/daemon.go         |  3 +++
 lxd/networks.go       | 19 +++++++++++++++----
 lxd/networks_utils.go |  2 +-
 lxd/patches.go        | 36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/lxd/daemon.go b/lxd/daemon.go
index c895e84..e0fc291 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -769,6 +769,9 @@ func (d *Daemon) Init() error {
 	if err := os.MkdirAll(shared.VarPath("snapshots"), 0700); err != nil {
 		return err
 	}
+	if err := os.MkdirAll(shared.VarPath("networks"), 0711); err != nil {
+		return err
+	}
 
 	/* Detect the filesystem */
 	d.BackingFs, err = filesystemDetect(d.lxcpath)
diff --git a/lxd/networks.go b/lxd/networks.go
index ce12c42..63a298c 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -525,7 +525,7 @@ func (n *network) Rename(name string) error {
 func (n *network) Start() error {
 	// Create directory
 	if !shared.PathExists(shared.VarPath("networks", n.name)) {
-		err := os.MkdirAll(shared.VarPath("networks", n.name), 0700)
+		err := os.MkdirAll(shared.VarPath("networks", n.name), 0711)
 		if err != nil {
 			return err
 		}
@@ -722,7 +722,7 @@ func (n *network) Start() error {
 	}
 
 	// Start building the dnsmasq command line
-	dnsmasqCmd := []string{"dnsmasq", "-u", "nobody", "--strict-order", "--bind-interfaces",
+	dnsmasqCmd := []string{"dnsmasq", "--strict-order", "--bind-interfaces",
 		fmt.Sprintf("--pid-file=%s", shared.VarPath("networks", n.name, "dnsmasq.pid")),
 		"--except-interface=lo",
 		fmt.Sprintf("--interface=%s", n.name)}
@@ -1125,12 +1125,12 @@ func (n *network) Start() error {
 
 	// Configure dnsmasq
 	if n.config["bridge.mode"] == "fan" || !shared.StringInSlice(n.config["ipv4.address"], []string{"", "none"}) || !shared.StringInSlice(n.config["ipv6.address"], []string{"", "none"}) {
+		// Setup the dnsmasq domain
 		dnsDomain := n.config["dns.domain"]
 		if dnsDomain == "" {
 			dnsDomain = "lxd"
 		}
 
-		// Setup the dnsmasq domain
 		if n.config["dns.mode"] != "none" {
 			dnsmasqCmd = append(dnsmasqCmd, []string{"-s", dnsDomain, "-S", fmt.Sprintf("/%s/", dnsDomain)}...)
 		}
@@ -1146,12 +1146,23 @@ func (n *network) Start() error {
 
 		// Create DHCP hosts file
 		if !shared.PathExists(shared.VarPath("networks", n.name, "dnsmasq.hosts")) {
-			err = ioutil.WriteFile(shared.VarPath("networks", n.name, "dnsmasq.hosts"), []byte(""), 0)
+			err = ioutil.WriteFile(shared.VarPath("networks", n.name, "dnsmasq.hosts"), []byte(""), 0644)
 			if err != nil {
 				return err
 			}
 		}
 
+		// Attempt to drop privileges
+		for _, user := range []string{"lxd", "nobody"} {
+			_, err := shared.UserId(user)
+			if err != nil {
+				continue
+			}
+
+			dnsmasqCmd = append(dnsmasqCmd, []string{"-u", user}...)
+			break
+		}
+
 		// Start dnsmasq (occasionally races, try a few times)
 		output, err := tryExec(dnsmasqCmd[0], dnsmasqCmd[1:]...)
 		if err != nil {
diff --git a/lxd/networks_utils.go b/lxd/networks_utils.go
index 6122d9d..63bf295 100644
--- a/lxd/networks_utils.go
+++ b/lxd/networks_utils.go
@@ -749,7 +749,7 @@ func networkUpdateStatic(d *Daemon, name string) error {
 
 		// Update the file
 		if entries == nil {
-			err := ioutil.WriteFile(shared.VarPath("networks", network, "dnsmasq.hosts"), []byte(""), 0)
+			err := ioutil.WriteFile(shared.VarPath("networks", network, "dnsmasq.hosts"), []byte(""), 0644)
 			if err != nil {
 				return err
 			}
diff --git a/lxd/patches.go b/lxd/patches.go
index ae1e258..f3dcc97 100644
--- a/lxd/patches.go
+++ b/lxd/patches.go
@@ -1,6 +1,7 @@
 package main
 
 import (
+	"os"
 	"strings"
 
 	"github.com/lxc/lxd/shared"
@@ -28,6 +29,7 @@ import (
 var patches = []patch{
 	{name: "invalid_profile_names", run: patchInvalidProfileNames},
 	{name: "leftover_profile_config", run: patchLeftoverProfileConfig},
+	{name: "network_permissions", run: patchNetworkPermissions},
 }
 
 type patch struct {
@@ -105,3 +107,37 @@ func patchInvalidProfileNames(name string, d *Daemon) error {
 
 	return nil
 }
+
+func patchNetworkPermissions(name string, d *Daemon) error {
+	// Get the list of networks
+	networks, err := dbNetworks(d.db)
+	if err != nil {
+		return err
+	}
+
+	// Fix the permissions
+	err = os.Chmod(shared.VarPath("networks"), 0711)
+	if err != nil {
+		return err
+	}
+
+	for _, network := range networks {
+		if !shared.PathExists(shared.VarPath("networks", network)) {
+			continue
+		}
+
+		err = os.Chmod(shared.VarPath("networks", network), 0711)
+		if err != nil {
+			return err
+		}
+
+		if shared.PathExists(shared.VarPath("networks", network, "dnsmasq.hosts")) {
+			err = os.Chmod(shared.VarPath("networks", network, "dnsmasq.hosts"), 0644)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}

From f4c1079ac1284652d62ef43d3b7e97ae5d5264b1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 24 Jan 2017 17:30:25 -0500
Subject: [PATCH 4/4] network: Clean up leases for static assignments
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is made a bit more difficult by the fact that dnsmasq won't reload
its leases on SIGHUP...

So we need to do a full dnsmasq restart to change the leases...

Closes #2781

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/container_lxc.go  | 14 ++++++++++++-
 lxd/networks_utils.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index a67dc16..1108e00 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -2649,8 +2649,20 @@ func (c *containerLXC) Delete() error {
 		return err
 	}
 
-	// Update lease files
+	// Update network files
 	networkUpdateStatic(c.daemon, "")
+	for k, m := range c.expandedDevices {
+		if m["type"] != "nic" || m["nictype"] != "bridged" || (m["ipv4.address"] == "" && m["ipv6.address"] == "") {
+			continue
+		}
+
+		m, err := c.fillNetworkDevice(k, m)
+		if err != nil {
+			continue
+		}
+
+		networkClearLease(c.daemon, m["parent"], m["hwaddr"])
+	}
 
 	shared.LogInfo("Deleted container", ctxMap)
 
diff --git a/lxd/networks_utils.go b/lxd/networks_utils.go
index 63bf295..3418054 100644
--- a/lxd/networks_utils.go
+++ b/lxd/networks_utils.go
@@ -749,7 +749,7 @@ func networkUpdateStatic(d *Daemon, name string) error {
 
 		// Update the file
 		if entries == nil {
-			err := ioutil.WriteFile(shared.VarPath("networks", network, "dnsmasq.hosts"), []byte(""), 0644)
+			err := ioutil.WriteFile(shared.VarPath("networks", network, "dnsmasq.hosts"), []byte(""), 0)
 			if err != nil {
 				return err
 			}
@@ -810,3 +810,59 @@ func networkSysctl(path string, value string) error {
 
 	return ioutil.WriteFile(fmt.Sprintf("/proc/sys/net/%s", path), []byte(value), 0)
 }
+
+func networkClearLease(d *Daemon, network string, hwaddr string) error {
+	leaseFile := shared.VarPath("networks", network, "dnsmasq.leases")
+
+	// Check that we are in fact running a dnsmasq for the network
+	if !shared.PathExists(leaseFile) {
+		return nil
+	}
+
+	// Restart the network when we're done here
+	n, err := networkLoadByName(d, network)
+	if err != nil {
+		return err
+	}
+	defer n.Start()
+
+	// Stop dnsmasq
+	err = networkKillDnsmasq(network, false)
+	if err != nil {
+		return err
+	}
+
+	// Mangle the lease file
+	leases, err := ioutil.ReadFile(leaseFile)
+	if err != nil {
+		return err
+	}
+
+	fd, err := os.Create(leaseFile)
+	if err != nil {
+		return err
+	}
+
+	for _, lease := range strings.Split(string(leases), "\n") {
+		if lease == "" {
+			continue
+		}
+
+		fields := strings.Fields(lease)
+		if len(fields) > 2 && strings.ToLower(fields[1]) == strings.ToLower(hwaddr) {
+			continue
+		}
+
+		_, err := fd.WriteString(fmt.Sprintf("%s\n", lease))
+		if err != nil {
+			return err
+		}
+	}
+
+	err = fd.Close()
+	if err != nil {
+		return err
+	}
+
+	return nil
+}


More information about the lxc-devel mailing list