Merge pull request #227 from steveeJ/ns-verify

pkg/ns: consider PROCFS during NS verification

pkg/ns/ns.go

@@ -20,7 +20,9 @@ import (
 	"os"
 	"path"
 	"runtime"
+	"strings"
 	"sync"
+	"syscall"
 
 	"golang.org/x/sys/unix"
 )
@@ -58,6 +60,7 @@ type NetNS interface {
 type netNS struct {
 	file    *os.File
 	mounted bool
+	closed  bool
 }
 
 func getCurrentThreadNetNSPath() string {
@@ -72,12 +75,55 @@ func GetCurrentNS() (NetNS, error) {
 	return GetNS(getCurrentThreadNetNSPath())
 }
 
+const (
+	// https://github.com/torvalds/linux/blob/master/include/uapi/linux/magic.h
+	NSFS_MAGIC   = 0x6e736673
+	PROCFS_MAGIC = 0x9fa0
+)
+
+func IsNS(nspath string) (isNS bool, msg string, err error) {
+	stat := syscall.Statfs_t{}
+	if err = syscall.Statfs(nspath, &stat); err != nil {
+		err = fmt.Errorf("failed to Statfs %s: %v", nspath, err)
+		return
+	}
+
+	switch stat.Type {
+	case PROCFS_MAGIC:
+		// Kernel < 3.19
+
+		validPathContent := "ns/"
+		validName := strings.Contains(nspath, validPathContent)
+		if !validName {
+			msg = fmt.Sprintf("path doesn't contain %q", validPathContent)
+			return
+		}
+		isNS = true
+	case NSFS_MAGIC:
+		// Kernel >= 3.19
+
+		isNS = true
+	default:
+		msg = fmt.Sprintf("unknown FS magic: %x", stat.Type)
+	}
+	return
+}
+
 // Returns an object representing the namespace referred to by @path
 func GetNS(nspath string) (NetNS, error) {
-	fd, err := os.Open(nspath)
+	isNS, msg, err := IsNS(nspath)
 	if err != nil {
 		return nil, err
 	}
+	if !isNS {
+		return nil, fmt.Errorf("no network namespace detected on %s: %s", nspath, msg)
+	}
+
+	fd, err := os.Open(nspath)
+	if err != nil {
+		return nil, fmt.Errorf("Failed to open %v: %v", nspath, err)
+	}
+
 	return &netNS{file: fd}, nil
 }
 
@@ -165,8 +211,22 @@ func (ns *netNS) Fd() uintptr {
 	return ns.file.Fd()
 }
 
+func (ns *netNS) errorIfClosed() error {
+	if ns.closed {
+		return fmt.Errorf("%q has already been closed", ns.file.Name())
+	}
+	return nil
+}
+
 func (ns *netNS) Close() error {
-	ns.file.Close()
+	if err := ns.errorIfClosed(); err != nil {
+		return err
+	}
+
+	if err := ns.file.Close(); err != nil {
+		return fmt.Errorf("Failed to close %q: %v", ns.file.Name(), err)
+	}
+	ns.closed = true
 
 	if ns.mounted {
 		if err := unix.Unmount(ns.file.Name(), unix.MNT_DETACH); err != nil {
@@ -175,11 +235,17 @@ func (ns *netNS) Close() error {
 		if err := os.RemoveAll(ns.file.Name()); err != nil {
 			return fmt.Errorf("Failed to clean up namespace %s: %v", ns.file.Name(), err)
 		}
+		ns.mounted = false
 	}
+
 	return nil
 }
 
 func (ns *netNS) Do(toRun func(NetNS) error) error {
+	if err := ns.errorIfClosed(); err != nil {
+		return err
+	}
+
 	containedCall := func(hostNS NetNS) error {
 		threadNS, err := GetNS(getCurrentThreadNetNSPath())
 		if err != nil {
@@ -218,6 +284,10 @@ func (ns *netNS) Do(toRun func(NetNS) error) error {
 }
 
 func (ns *netNS) Set() error {
+	if err := ns.errorIfClosed(); err != nil {
+		return err
+	}
+
 	if _, _, err := unix.Syscall(unix.SYS_SETNS, ns.Fd(), uintptr(unix.CLONE_NEWNET), 0); err != 0 {
 		return fmt.Errorf("Error switching to ns %v: %v", ns.file.Name(), err)
 	}
@@ -230,7 +300,7 @@ func (ns *netNS) Set() error {
 func WithNetNSPath(nspath string, toRun func(NetNS) error) error {
 	ns, err := GetNS(nspath)
 	if err != nil {
-		return fmt.Errorf("Failed to open %v: %v", nspath, err)
+		return err
 	}
 	defer ns.Close()
 	return ns.Do(toRun)

pkg/ns/ns_test.go

@@ -17,6 +17,7 @@ package ns_test
 import (
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 
@@ -169,6 +170,70 @@ var _ = Describe("Linux namespace operations", func() {
 					Expect(netnsInode).NotTo(Equal(createdNetNSInode))
 				}
 			})
+
+			It("fails when the path is not a namespace", func() {
+				tempFile, err := ioutil.TempFile("", "nstest")
+				Expect(err).NotTo(HaveOccurred())
+				defer tempFile.Close()
+
+				nspath := tempFile.Name()
+				defer os.Remove(nspath)
+
+				_, err = ns.GetNS(nspath)
+				Expect(err).To(HaveOccurred())
+				errString := fmt.Sprintf("%v", err)
+				Expect(errString).To(HavePrefix("no network namespace detected on %s", nspath))
+			})
+		})
+
+		Describe("closing a network namespace", func() {
+			It("should prevent further operations", func() {
+				createdNetNS, err := ns.NewNS()
+				Expect(err).NotTo(HaveOccurred())
+
+				err = createdNetNS.Close()
+				Expect(err).NotTo(HaveOccurred())
+
+				err = createdNetNS.Do(func(ns.NetNS) error { return nil })
+				Expect(err).To(HaveOccurred())
+
+				err = createdNetNS.Set()
+				Expect(err).To(HaveOccurred())
+			})
+
+			It("should only work once", func() {
+				createdNetNS, err := ns.NewNS()
+				Expect(err).NotTo(HaveOccurred())
+
+				err = createdNetNS.Close()
+				Expect(err).NotTo(HaveOccurred())
+
+				err = createdNetNS.Close()
+				Expect(err).To(HaveOccurred())
+			})
+		})
+	})
+
+	Describe("IsNS", func() {
+		It("should detect a namespace", func() {
+			createdNetNS, err := ns.NewNS()
+			isNSFS, msg, err := ns.IsNS(createdNetNS.Path())
+			Expect(err).NotTo(HaveOccurred())
+			Expect(msg).To(Equal(""))
+			Expect(isNSFS).To(Equal(true))
+		})
+
+		It("should refuse other paths", func() {
+			tempFile, err := ioutil.TempFile("", "nstest")
+			Expect(err).NotTo(HaveOccurred())
+			defer tempFile.Close()
+
+			nspath := tempFile.Name()
+			defer os.Remove(nspath)
+
+			isNSFS, _, err := ns.IsNS(nspath)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(isNSFS).To(Equal(false))
 		})
 	})
 })