diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go index 6273a26b7..63abdb00c 100644 --- a/libcontainer/configs/validate/validator.go +++ b/libcontainer/configs/validate/validator.go @@ -6,10 +6,12 @@ import ( "os" "path/filepath" "strings" + "sync" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/intelrdt" selinux "github.com/opencontainers/selinux/go-selinux" + "golang.org/x/sys/unix" ) type Validator interface { @@ -144,6 +146,12 @@ func (v *ConfigValidator) sysctl(config *configs.Config) error { "kernel.shm_rmid_forced": true, } + var ( + netOnce sync.Once + hostnet bool + hostnetErr error + ) + for s := range config.Sysctl { if validSysctlMap[s] || strings.HasPrefix(s, "fs.mqueue.") { if config.Namespaces.Contains(configs.NEWIPC) { @@ -153,16 +161,27 @@ func (v *ConfigValidator) sysctl(config *configs.Config) error { } } if strings.HasPrefix(s, "net.") { - if config.Namespaces.Contains(configs.NEWNET) { - if path := config.Namespaces.PathOf(configs.NEWNET); path != "" { - if err := checkHostNs(s, path); err != nil { - return err - } + // Is container using host netns? + // Here "host" means "current", not "initial". + netOnce.Do(func() { + if !config.Namespaces.Contains(configs.NEWNET) { + hostnet = true + return } - continue - } else { - return fmt.Errorf("sysctl %q is not allowed in the hosts network namespace", s) + path := config.Namespaces.PathOf(configs.NEWNET) + if path == "" { + // own netns, so hostnet = false + return + } + hostnet, hostnetErr = isHostNetNS(path) + }) + if hostnetErr != nil { + return hostnetErr } + if hostnet { + return fmt.Errorf("sysctl %q not allowed in host network namespace", s) + } + continue } if config.Namespaces.Contains(configs.NEWUTS) { switch s { @@ -204,43 +223,17 @@ func (v *ConfigValidator) intelrdt(config *configs.Config) error { return nil } -func isSymbolicLink(path string) (bool, error) { - fi, err := os.Lstat(path) - if err != nil { - return false, err +func isHostNetNS(path string) (bool, error) { + const currentProcessNetns = "/proc/self/ns/net" + + var st1, st2 unix.Stat_t + + if err := unix.Stat(currentProcessNetns, &st1); err != nil { + return false, fmt.Errorf("unable to stat %q: %s", currentProcessNetns, err) + } + if err := unix.Stat(path, &st2); err != nil { + return false, fmt.Errorf("unable to stat %q: %s", path, err) } - return fi.Mode()&os.ModeSymlink == os.ModeSymlink, nil -} - -// checkHostNs checks whether network sysctl is used in host namespace. -func checkHostNs(sysctlConfig string, path string) error { - var currentProcessNetns = "/proc/self/ns/net" - // readlink on the current processes network namespace - destOfCurrentProcess, err := os.Readlink(currentProcessNetns) - if err != nil { - return fmt.Errorf("read soft link %q error", currentProcessNetns) - } - - // First check if the provided path is a symbolic link - symLink, err := isSymbolicLink(path) - if err != nil { - return fmt.Errorf("could not check that %q is a symlink: %v", path, err) - } - - if !symLink { - // The provided namespace is not a symbolic link, - // it is not the host namespace. - return nil - } - - // readlink on the path provided in the struct - destOfContainer, err := os.Readlink(path) - if err != nil { - return fmt.Errorf("read soft link %q error", path) - } - if destOfContainer == destOfCurrentProcess { - return fmt.Errorf("sysctl %q is not allowed in the hosts network namespace", sysctlConfig) - } - return nil + return (st1.Dev == st2.Dev) && (st1.Ino == st2.Ino), nil } diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go index ffbb32646..7c5c7138a 100644 --- a/libcontainer/configs/validate/validator_test.go +++ b/libcontainer/configs/validate/validator_test.go @@ -1,11 +1,14 @@ package validate_test import ( + "io/ioutil" "os" + "path/filepath" "testing" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/configs/validate" + "golang.org/x/sys/unix" ) func TestValidate(t *testing.T) { @@ -254,6 +257,53 @@ func TestValidateSysctlWithSameNs(t *testing.T) { } } +func TestValidateSysctlWithBindHostNetNS(t *testing.T) { + if os.Getuid() != 0 { + t.Skip("requires root") + } + + const selfnet = "/proc/self/ns/net" + + dir, err := ioutil.TempDir("", t.Name()+"-*") + if err != nil { + t.Fatal(err) + } + defer os.Remove(dir) + + file := filepath.Join(dir, "default") + fd, err := os.Create(file) + if err != nil { + t.Fatal(err) + } + defer os.Remove(file) + fd.Close() + + if err := unix.Mount(selfnet, file, "bind", unix.MS_BIND, ""); err != nil { + t.Fatalf("can't bind-mount %s to %s: %s", selfnet, file, err) + } + defer func() { + _ = unix.Unmount(file, unix.MNT_DETACH) + }() + + config := &configs.Config{ + Rootfs: "/var", + Sysctl: map[string]string{"net.ctl": "ctl", "net.foo": "bar"}, + Namespaces: configs.Namespaces( + []configs.Namespace{ + { + Type: configs.NEWNET, + Path: file, + }, + }, + ), + } + + validator := validate.New() + if err := validator.Validate(config); err == nil { + t.Error("Expected error to occur but it was nil") + } +} + func TestValidateSysctlWithoutNETNamespace(t *testing.T) { config := &configs.Config{ Rootfs: "/var",