Merge pull request #977 from cyphar/nsenter-userns-ordering

nsenter: guarantee correct user namespace ordering

libcontainer/configs/namespaces_unix.go

@@ -22,8 +22,8 @@ var (
 	supportedNamespaces = make(map[NamespaceType]bool)
 )
 
-// nsToFile converts the namespace type to its filename
-func nsToFile(ns NamespaceType) string {
+// NsName converts the namespace type to its filename
+func NsName(ns NamespaceType) string {
 	switch ns {
 	case NEWNET:
 		return "net"
@@ -50,7 +50,7 @@ func IsNamespaceSupported(ns NamespaceType) bool {
 	if ok {
 		return supported
 	}
-	nsFile := nsToFile(ns)
+	nsFile := NsName(ns)
 	// if the namespace type is unknown, just return false
 	if nsFile == "" {
 		return false
@@ -84,7 +84,7 @@ func (n *Namespace) GetPath(pid int) string {
 	if n.Path != "" {
 		return n.Path
 	}
-	return fmt.Sprintf("/proc/%d/ns/%s", pid, nsToFile(n.Type))
+	return fmt.Sprintf("/proc/%d/ns/%s", pid, NsName(n.Type))
 }
 
 func (n *Namespaces) Remove(t NamespaceType) bool {

libcontainer/container_linux.go

@@ -1223,16 +1223,22 @@ func (c *linuxContainer) currentState() (*State, error) {
 // can setns in order.
 func (c *linuxContainer) orderNamespacePaths(namespaces map[configs.NamespaceType]string) ([]string, error) {
 	paths := []string{}
-	nsTypes := []configs.NamespaceType{
+	order := []configs.NamespaceType{
+		// The user namespace *must* be done first.
+		configs.NEWUSER,
 		configs.NEWIPC,
 		configs.NEWUTS,
 		configs.NEWNET,
 		configs.NEWPID,
 		configs.NEWNS,
 	}
-	// join userns if the init process explicitly requires NEWUSER
-	if c.config.Namespaces.Contains(configs.NEWUSER) {
-		nsTypes = append(nsTypes, configs.NEWUSER)
+
+	// Remove namespaces that we don't need to join.
+	var nsTypes []configs.NamespaceType
+	for _, ns := range order {
+		if c.config.Namespaces.Contains(ns) {
+			nsTypes = append(nsTypes, ns)
+		}
 	}
 	for _, nsType := range nsTypes {
 		if p, ok := namespaces[nsType]; ok && p != "" {
@@ -1249,7 +1255,7 @@ func (c *linuxContainer) orderNamespacePaths(namespaces map[configs.NamespaceTyp
 			if strings.ContainsRune(p, ',') {
 				return nil, newSystemError(fmt.Errorf("invalid path %s", p))
 			}
-			paths = append(paths, p)
+			paths = append(paths, fmt.Sprintf("%s:%s", configs.NsName(nsType), p))
 		}
 	}
 	return paths, nil

libcontainer/nsenter/namespace.h

@@ -0,0 +1,32 @@
+#ifndef NSENTER_NAMESPACE_H
+#define NSENTER_NAMESPACE_H
+
+#ifndef _GNU_SOURCE
+#	define _GNU_SOURCE
+#endif
+#include <sched.h>
+
+/* All of these are taken from include/uapi/linux/sched.h */
+#ifndef CLONE_NEWNS
+#	define CLONE_NEWNS 0x00020000 /* New mount namespace group */
+#endif
+#ifndef CLONE_NEWCGROUP
+#	define CLONE_NEWCGROUP 0x02000000 /* New cgroup namespace */
+#endif
+#ifndef CLONE_NEWUTS
+#	define CLONE_NEWUTS 0x04000000 /* New utsname namespace */
+#endif
+#ifndef CLONE_NEWIPC
+#	define CLONE_NEWIPC 0x08000000 /* New ipc namespace */
+#endif
+#ifndef CLONE_NEWUSER
+#	define CLONE_NEWUSER 0x10000000 /* New user namespace */
+#endif
+#ifndef CLONE_NEWPID
+#	define CLONE_NEWPID 0x20000000 /* New pid namespace */
+#endif
+#ifndef CLONE_NEWNET
+#	define CLONE_NEWNET 0x40000000 /* New network namespace */
+#endif
+
+#endif /* NSENTER_NAMESPACE_H */

libcontainer/nsenter/nsenter_test.go

@@ -29,7 +29,7 @@ func TestNsenterValidPaths(t *testing.T) {
 
 	namespaces := []string{
 		// join pid ns of the current process
-		fmt.Sprintf("/proc/%d/ns/pid", os.Getpid()),
+		fmt.Sprintf("pid:/proc/%d/ns/pid", os.Getpid()),
 	}
 	cmd := &exec.Cmd{
 		Path:       os.Args[0],
@@ -87,7 +87,47 @@ func TestNsenterInvalidPaths(t *testing.T) {
 
 	namespaces := []string{
 		// join pid ns of the current process
-		fmt.Sprintf("/proc/%d/ns/pid", -1),
+		fmt.Sprintf("pid:/proc/%d/ns/pid", -1),
+	}
+	cmd := &exec.Cmd{
+		Path:       os.Args[0],
+		Args:       args,
+		ExtraFiles: []*os.File{child},
+		Env:        []string{"_LIBCONTAINER_INITPIPE=3"},
+	}
+
+	if err := cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	// write cloneFlags
+	r := nl.NewNetlinkRequest(int(libcontainer.InitMsg), 0)
+	r.AddData(&libcontainer.Int32msg{
+		Type:  libcontainer.CloneFlagsAttr,
+		Value: uint32(syscall.CLONE_NEWNET),
+	})
+	r.AddData(&libcontainer.Bytemsg{
+		Type:  libcontainer.NsPathsAttr,
+		Value: []byte(strings.Join(namespaces, ",")),
+	})
+	if _, err := io.Copy(parent, bytes.NewReader(r.Serialize())); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := cmd.Wait(); err == nil {
+		t.Fatalf("nsenter exits with a zero exit status")
+	}
+}
+
+func TestNsenterIncorrectPathType(t *testing.T) {
+	args := []string{"nsenter-exec"}
+	parent, child, err := newPipe()
+	if err != nil {
+		t.Fatalf("failed to create pipe %v", err)
+	}
+
+	namespaces := []string{
+		// join pid ns of the current process
+		fmt.Sprintf("net:/proc/%d/ns/pid", os.Getpid()),
 	}
 	cmd := &exec.Cmd{
 		Path:       os.Args[0],