diff --git a/CVE-2019-11328.diff b/CVE-2019-11328.diff deleted file mode 100644 index aa31608..0000000 --- a/CVE-2019-11328.diff +++ /dev/null @@ -1,1134 +0,0 @@ -diff --git a/cmd/internal/cli/actions_linux.go b/cmd/internal/cli/actions_linux.go -index 9ee79af6b..6f64d9dd5 100644 ---- a/cmd/internal/cli/actions_linux.go -+++ b/cmd/internal/cli/actions_linux.go -@@ -157,9 +157,7 @@ func execStarter(cobraCmd *cobra.Command, image string, args []string, name stri - if err != nil { - sylog.Fatalf("%s", err) - } -- if !file.Privileged { -- UserNamespace = true -- } -+ UserNamespace = file.UserNs - generator.AddProcessEnv("SINGULARITY_CONTAINER", file.Image) - generator.AddProcessEnv("SINGULARITY_NAME", filepath.Base(file.Image)) - engineConfig.SetImage(image) -@@ -428,7 +426,7 @@ func execStarter(cobraCmd *cobra.Command, image string, args []string, name stri - } - - if engineConfig.GetInstance() { -- stdout, stderr, err := instance.SetLogFile(name, int(uid), instance.SingSubDir) -+ stdout, stderr, err := instance.SetLogFile(name, int(uid), instance.LogSubDir) - if err != nil { - sylog.Fatalf("failed to create instance log files: %s", err) - } -diff --git a/internal/app/singularity/oci_run_linux.go b/internal/app/singularity/oci_run_linux.go -index a5019a713..b077f9132 100644 ---- a/internal/app/singularity/oci_run_linux.go -+++ b/internal/app/singularity/oci_run_linux.go -@@ -20,7 +20,7 @@ import ( - - // OciRun runs a container (equivalent to create/start/delete) - func OciRun(containerID string, args *OciArgs) error { -- dir, err := instance.GetDirPrivileged(containerID, instance.OciSubDir) -+ dir, err := instance.GetDir(containerID, instance.OciSubDir) - if err != nil { - return err - } -diff --git a/internal/pkg/instance/instance.go b/internal/pkg/instance/instance.go -index 1d9290740..6aa8e5364 100644 ---- a/internal/pkg/instance/instance.go -+++ b/internal/pkg/instance/instance.go -@@ -8,19 +8,13 @@ package instance - import ( - "encoding/json" - "fmt" -- "io/ioutil" - "os" - "path/filepath" - "regexp" - "strings" - "syscall" - -- "github.com/sylabs/singularity/internal/pkg/sylog" -- -- specs "github.com/opencontainers/runtime-spec/specs-go" -- - "github.com/sylabs/singularity/internal/pkg/util/user" -- "github.com/sylabs/singularity/pkg/util/fs/proc" - ) - - const ( -@@ -28,35 +22,26 @@ const ( - OciSubDir = "oci" - // SingSubDir represents directory where Singularity instance files are stored - SingSubDir = "sing" -+ // LogSubDir represents directory where Singularity instance log files are stored -+ LogSubDir = "logs" - ) - - const ( -- privPath = "/var/run/singularity/instances" -- unprivPath = ".singularity/instances" -+ instancePath = ".singularity/instances" - authorizedChars = `^[a-zA-Z0-9._-]+$` - prognameFormat = "Singularity instance: %s [%s]" - ) - --var nsMap = map[specs.LinuxNamespaceType]string{ -- specs.PIDNamespace: "pid", -- specs.UTSNamespace: "uts", -- specs.IPCNamespace: "ipc", -- specs.MountNamespace: "mnt", -- specs.CgroupNamespace: "cgroup", -- specs.NetworkNamespace: "net", -- specs.UserNamespace: "user", --} -- - // File represents an instance file storing instance information - type File struct { -- Path string `json:"-"` -- Pid int `json:"pid"` -- PPid int `json:"ppid"` -- Name string `json:"name"` -- User string `json:"user"` -- Image string `json:"image"` -- Privileged bool `json:"privileged"` -- Config []byte `json:"config"` -+ Path string `json:"-"` -+ Pid int `json:"pid"` -+ PPid int `json:"ppid"` -+ Name string `json:"name"` -+ User string `json:"user"` -+ Image string `json:"image"` -+ Config []byte `json:"config"` -+ UserNs bool `json:"userns"` - } - - // ProcName returns processus name based on instance name -@@ -80,7 +65,7 @@ func CheckName(name string) error { - } - - // getPath returns the path where searching for instance files --func getPath(privileged bool, username string, subDir string) (string, error) { -+func getPath(username string, subDir string) (string, error) { - path := "" - var pw *user.User - var err error -@@ -95,50 +80,27 @@ func getPath(privileged bool, username string, subDir string) (string, error) { - } - } - -- if privileged { -- path = filepath.Join(privPath, subDir, pw.Name) -- return path, nil -- } -- -- containerID, hostID, err := proc.ReadIDMap("/proc/self/uid_map") -- if containerID == 0 && containerID != hostID { -- if pw, err = user.GetPwUID(hostID); err != nil { -- return path, err -- } -- } -- - hostname, err := os.Hostname() - if err != nil { - return path, err - } - -- path = filepath.Join(pw.Dir, unprivPath, subDir, hostname, pw.Name) -+ path = filepath.Join(pw.Dir, instancePath, subDir, hostname, pw.Name) - return path, nil - } - --func getDir(privileged bool, name string, subDir string) (string, error) { -+// GetDir returns directory where instances file will be stored -+func GetDir(name string, subDir string) (string, error) { - if err := CheckName(name); err != nil { - return "", err - } -- path, err := getPath(privileged, "", subDir) -+ path, err := getPath("", subDir) - if err != nil { - return "", err - } - return filepath.Join(path, name), nil - } - --// GetDirPrivileged returns directory where instances file will be stored --// if instance is run with privileges --func GetDirPrivileged(name string, subDir string) (string, error) { -- return getDir(true, name, subDir) --} -- --// GetDirUnprivileged returns directory where instances file will be stored --// if instance is run without privileges --func GetDirUnprivileged(name string, subDir string) (string, error) { -- return getDir(false, name, subDir) --} -- - // Get returns the instance file corresponding to instance name - func Get(name string, subDir string) (*File, error) { - if err := CheckName(name); err != nil { -@@ -156,7 +118,7 @@ func Get(name string, subDir string) (*File, error) { - - // Add creates an instance file for a named instance in a privileged - // or unprivileged path --func Add(name string, privileged bool, subDir string) (*File, error) { -+func Add(name string, subDir string) (*File, error) { - if err := CheckName(name); err != nil { - return nil, err - } -@@ -164,8 +126,8 @@ func Add(name string, privileged bool, subDir string) (*File, error) { - if err == nil { - return nil, fmt.Errorf("instance %s already exists", name) - } -- i := &File{Name: name, Privileged: privileged} -- i.Path, err = getPath(privileged, "", subDir) -+ i := &File{Name: name} -+ i.Path, err = getPath("", subDir) - if err != nil { - return nil, err - } -@@ -177,63 +139,39 @@ func Add(name string, privileged bool, subDir string) (*File, error) { - // List returns instance files matching username and/or name pattern - func List(username string, name string, subDir string) ([]*File, error) { - list := make([]*File, 0) -- privileged := true - -- for { -- path, err := getPath(privileged, username, subDir) -- if err != nil { -- return nil, err -- } -- pattern := filepath.Join(path, name, name+".json") -- files, err := filepath.Glob(pattern) -- if err != nil { -+ path, err := getPath(username, subDir) -+ if err != nil { -+ return nil, err -+ } -+ pattern := filepath.Join(path, name, name+".json") -+ files, err := filepath.Glob(pattern) -+ if err != nil { -+ return nil, err -+ } -+ for _, file := range files { -+ r, err := os.Open(file) -+ if os.IsNotExist(err) { -+ continue -+ } else if err != nil { - return nil, err - } -- for _, file := range files { -- r, err := os.Open(file) -- if os.IsNotExist(err) { -- continue -- } -- if err != nil { -- return nil, err -- } -- b, err := ioutil.ReadAll(r) -+ f := &File{} -+ if err := json.NewDecoder(r).Decode(f); err != nil { - r.Close() -- if err != nil { -- return nil, err -- } -- f := &File{Path: file} -- if err := json.Unmarshal(b, f); err != nil { -- return nil, err -- } -- list = append(list, f) -- } -- privileged = !privileged -- if privileged { -- break -+ return nil, err - } -+ r.Close() -+ f.Path = file -+ list = append(list, f) - } - - return list, nil - } - --// PrivilegedPath returns if instance file is stored in privileged path or not --func (i *File) PrivilegedPath() bool { -- return strings.HasPrefix(i.Path, privPath) --} -- - // Delete deletes instance file - func (i *File) Delete() error { -- path := filepath.Dir(i.Path) -- -- nspath := filepath.Join(path, "ns") -- if _, err := os.Stat(nspath); err == nil { -- if err := syscall.Unmount(nspath, syscall.MNT_DETACH); err != nil { -- sylog.Errorf("can't umount %s: %s", nspath, err) -- } -- } -- -- return os.RemoveAll(path) -+ return os.RemoveAll(filepath.Dir(i.Path)) - } - - // Update stores instance information in associated instance file -@@ -251,116 +189,23 @@ func (i *File) Update() error { - if err := os.MkdirAll(path, 0755); err != nil { - return err - } -- if i.PrivilegedPath() { -- pw, err := user.GetPwNam(i.User) -- if err != nil { -- return err -- } -- if err := os.Chmod(path, 0550); err != nil { -- return err -- } -- if err := os.Chown(path, int(pw.UID), 0); err != nil { -- return err -- } -- } -- file, err := os.OpenFile(i.Path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) -+ file, err := os.OpenFile(i.Path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY|syscall.O_NOFOLLOW, 0644) - if err != nil { - return err - } - defer file.Close() - -- b = append(b, '\n') -- if n, err := file.Write(b); err != nil || n != len(b) { -+ if _, err := file.Write(b); err != nil { - return fmt.Errorf("failed to write instance file %s: %s", i.Path, err) - } - - return file.Sync() - } - --// MountNamespaces binds /proc//ns directory into instance folder --func (i *File) MountNamespaces() error { -- path := filepath.Join(filepath.Dir(i.Path), "ns") -- -- oldumask := syscall.Umask(0) -- defer syscall.Umask(oldumask) -- -- if err := os.Mkdir(path, 0755); err != nil { -- return err -- } -- -- nspath, err := filepath.EvalSymlinks(path) -- if err != nil { -- return err -- } -- -- src := fmt.Sprintf("/proc/%d/ns", i.Pid) -- if err := syscall.Mount(src, nspath, "", syscall.MS_BIND, ""); err != nil { -- return fmt.Errorf("mounting %s in instance folder failed: %s", src, err) -- } -- -- return nil --} -- --// UpdateNamespacesPath updates namespaces path for the provided configuration --func (i *File) UpdateNamespacesPath(configNs []specs.LinuxNamespace) error { -- path := filepath.Join(filepath.Dir(i.Path), "ns") -- nspath, err := filepath.EvalSymlinks(path) -- if err != nil { -- return err -- } -- nsBase := filepath.Join(fmt.Sprintf("/proc/%d/root", i.PPid), nspath) -- -- procPath := fmt.Sprintf("/proc/%d/cmdline", i.PPid) -- -- if i.PrivilegedPath() { -- var st syscall.Stat_t -- -- if err := syscall.Stat(procPath, &st); err != nil { -- return err -- } -- if st.Uid != 0 || st.Gid != 0 { -- return fmt.Errorf("not an instance process") -- } -- -- uid := os.Geteuid() -- taskPath := fmt.Sprintf("/proc/%d/task", i.PPid) -- if err := syscall.Stat(taskPath, &st); err != nil { -- return err -- } -- if int(st.Uid) != uid { -- return fmt.Errorf("you do not own the instance") -- } -- } -- -- data, err := ioutil.ReadFile(procPath) -- if err != nil { -- return err -- } -- -- cmdline := string(data[:len(data)-1]) -- procName := ProcName(i.Name, i.User) -- if cmdline != procName { -- return fmt.Errorf("no command line match found") -- } -- -- for i, n := range configNs { -- ns, ok := nsMap[n.Type] -- if !ok { -- configNs[i].Path = "" -- continue -- } -- if n.Path != "" { -- configNs[i].Path = filepath.Join(nsBase, ns) -- } -- } -- -- return nil --} -- - // SetLogFile replaces stdout/stderr streams and redirect content - // to log file - func SetLogFile(name string, uid int, subDir string) (*os.File, *os.File, error) { -- path, err := getPath(false, "", subDir) -+ path, err := getPath("", subDir) - if err != nil { - return nil, nil, err - } -@@ -377,12 +222,12 @@ func SetLogFile(name string, uid int, subDir string) (*os.File, *os.File, error) - return nil, nil, err - } - -- stderr, err := os.OpenFile(stderrPath, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) -+ stderr, err := os.OpenFile(stderrPath, os.O_RDWR|os.O_CREATE|os.O_APPEND|syscall.O_NOFOLLOW, 0644) - if err != nil { - return nil, nil, err - } - -- stdout, err := os.OpenFile(stdoutPath, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) -+ stdout, err := os.OpenFile(stdoutPath, os.O_RDWR|os.O_CREATE|os.O_APPEND|syscall.O_NOFOLLOW, 0644) - if err != nil { - return nil, nil, err - } -diff --git a/internal/pkg/instance/instance_linux_test.go b/internal/pkg/instance/instance_linux_test.go -new file mode 100644 -index 000000000..d3e44563c ---- /dev/null -+++ b/internal/pkg/instance/instance_linux_test.go -@@ -0,0 +1,245 @@ -+// Copyright (c) 2019, Sylabs Inc. All rights reserved. -+// This software is licensed under a 3-clause BSD license. Please consult the -+// LICENSE.md file distributed with the sources of this project regarding your -+// rights to use or distribute this software. -+ -+package instance -+ -+import ( -+ "os" -+ "path/filepath" -+ "testing" -+ -+ "github.com/sylabs/singularity/internal/pkg/test" -+) -+ -+const testSubDir = "testing" -+ -+func TestProcName(t *testing.T) { -+ test.DropPrivilege(t) -+ defer test.ResetPrivilege(t) -+ -+ tests := []struct { -+ desc string -+ name string -+ user string -+ match string -+ expectedFailure bool -+ }{ -+ { -+ desc: "with valid same name/username", -+ name: "test", -+ user: "test", -+ match: "Singularity instance: test [test]", -+ }, -+ { -+ desc: "with valid different name/username", -+ name: "instance", -+ user: "user", -+ match: "Singularity instance: user [instance]", -+ }, -+ { -+ desc: "with empty name", -+ name: "", -+ user: "test", -+ expectedFailure: true, -+ }, -+ { -+ desc: "with empty username", -+ name: "test", -+ user: "", -+ expectedFailure: true, -+ }, -+ { -+ desc: "both empty name/username", -+ name: "", -+ user: "", -+ expectedFailure: true, -+ }, -+ } -+ for _, e := range tests { -+ m, err := ProcName(e.name, e.user) -+ if err != nil && !e.expectedFailure { -+ t.Errorf("unexpected failure for test '%s': %s", e.desc, err) -+ } else if err == nil && e.expectedFailure { -+ t.Errorf("unexpected success for test '%s'", e.desc) -+ } else if m != e.match { -+ t.Errorf("unexpected match %s != %s", m, e.match) -+ } -+ } -+} -+ -+func TestExtractName(t *testing.T) { -+ test.DropPrivilege(t) -+ defer test.ResetPrivilege(t) -+ -+ tests := []struct { -+ input string -+ output string -+ }{ -+ { -+ input: "instance://test", -+ output: "test", -+ }, -+ { -+ input: "instance:/test", -+ output: "instance:/test", -+ }, -+ { -+ input: "instance:///test", -+ output: "/test", -+ }, -+ } -+ for _, e := range tests { -+ o := ExtractName(e.input) -+ if o != e.output { -+ t.Errorf("unexpected result, got %s instead of %s", o, e.output) -+ } -+ } -+} -+ -+func TestCheckName(t *testing.T) { -+ test.DropPrivilege(t) -+ defer test.ResetPrivilege(t) -+ -+ tests := []struct { -+ desc string -+ name string -+ expectFailure bool -+ }{ -+ { -+ desc: "with valid name", -+ name: "test", -+ expectFailure: false, -+ }, -+ { -+ desc: "with valid name containg number", -+ name: "test123", -+ expectFailure: false, -+ }, -+ { -+ desc: "with invalid name containing space", -+ name: "test 123", -+ expectFailure: true, -+ }, -+ { -+ desc: "with valid name containing underscore", -+ name: "test_123", -+ expectFailure: false, -+ }, -+ { -+ desc: "with valid name containing dot", -+ name: "test.123", -+ expectFailure: false, -+ }, -+ { -+ desc: "with valid name containing minus", -+ name: "test-123", -+ expectFailure: false, -+ }, -+ { -+ desc: "with invalid name containing slash", -+ name: "test/123", -+ expectFailure: true, -+ }, -+ { -+ desc: "with empty name", -+ name: "", -+ expectFailure: true, -+ }, -+ } -+ for _, e := range tests { -+ err := CheckName(e.name) -+ if err != nil && !e.expectFailure { -+ t.Errorf("unexpected failure %s: %s", e.desc, err) -+ } else if err == nil && e.expectFailure { -+ t.Errorf("unexpected success %s", e.desc) -+ } -+ } -+} -+ -+var instanceTests = []struct { -+ name string -+ expectFailure bool -+}{ -+ { -+ name: "valid_instance", -+ expectFailure: false, -+ }, -+ { -+ name: "valid_instance", -+ expectFailure: true, -+ }, -+ { -+ name: "invalid instance", -+ expectFailure: true, -+ }, -+} -+ -+func TestAdd(t *testing.T) { -+ test.EnsurePrivilege(t) -+ -+ for _, e := range instanceTests { -+ var err error -+ var file *File -+ -+ file, err = Add(e.name, testSubDir) -+ if err != nil && !e.expectFailure { -+ t.Errorf("unexpected failure for name %s: %s", e.name, err) -+ } else if err == nil && e.expectFailure { -+ t.Errorf("unexpected success for name %s", e.name) -+ } -+ if file == nil { -+ continue -+ } -+ file.User = "root" -+ file.Pid = os.Getpid() -+ if err := file.Update(); err != nil { -+ t.Errorf("error while creating instance %s: %s", e.name, err) -+ } -+ stdout, stderr, err := SetLogFile(e.name, 0, testSubDir) -+ if err != nil { -+ t.Errorf("error while creating instance log file: %s", err) -+ } -+ if err := os.Remove(stdout.Name()); err != nil { -+ t.Errorf("error while delete instance log out file: %s", err) -+ } -+ if err := os.Remove(stderr.Name()); err != nil { -+ t.Errorf("error while deleting instance log err file: %s", err) -+ } -+ } -+} -+ -+func TestGet(t *testing.T) { -+ test.EnsurePrivilege(t) -+ -+ for _, e := range instanceTests { -+ var err error -+ var file *File -+ -+ file, err = Get(e.name, testSubDir) -+ if err != nil && !e.expectFailure { -+ t.Errorf("unexpected failure for name %s: %s", e.name, err) -+ } else if err == nil && e.expectFailure { -+ t.Errorf("unexpected success for name %s", e.name) -+ } -+ if file == nil { -+ continue -+ } -+ if file.User != "root" { -+ t.Errorf("unexpected user returned %s", file.User) -+ } -+ path, err := GetDir(e.name, testSubDir) -+ if err != nil { -+ t.Errorf("unexpected error while retrieving instance directory path: %s", err) -+ } -+ instanceDir := filepath.Dir(file.Path) -+ if path != instanceDir { -+ t.Errorf("unexpected instance directory path, got %s instead of %s", path, instanceDir) -+ } -+ err = file.Delete() -+ if err != nil && !e.expectFailure { -+ t.Errorf("unexpected error while deleting instance %s: %s", e.name, err) -+ } -+ } -+} -diff --git a/internal/pkg/runtime/engines/oci/create.go b/internal/pkg/runtime/engines/oci/create.go -index 33d914a5f..d7cf8968f 100644 ---- a/internal/pkg/runtime/engines/oci/create.go -+++ b/internal/pkg/runtime/engines/oci/create.go -@@ -152,7 +152,7 @@ func (engine *EngineOperations) createState(pid int) error { - - name := engine.CommonConfig.ContainerID - -- file, err := instance.Add(name, true, instance.OciSubDir) -+ file, err := instance.Add(name, instance.OciSubDir) - if err != nil { - return err - } -@@ -795,7 +795,7 @@ func (c *container) addDevices(system *mount.System) error { - func (c *container) addMaskedPathsMount(system *mount.System) error { - paths := c.engine.EngineConfig.OciConfig.Linux.MaskedPaths - -- dir, err := instance.GetDirPrivileged(c.engine.CommonConfig.ContainerID, instance.OciSubDir) -+ dir, err := instance.GetDir(c.engine.CommonConfig.ContainerID, instance.OciSubDir) - if err != nil { - return err - } -diff --git a/internal/pkg/runtime/engines/oci/process.go b/internal/pkg/runtime/engines/oci/process.go -index f13ffec28..227419fb0 100644 ---- a/internal/pkg/runtime/engines/oci/process.go -+++ b/internal/pkg/runtime/engines/oci/process.go -@@ -231,7 +231,7 @@ func (engine *EngineOperations) PreStartProcess(pid int, masterConn net.Conn, fa - logPath := engine.EngineConfig.GetLogPath() - if logPath == "" { - containerID := engine.CommonConfig.ContainerID -- dir, err := instance.GetDirPrivileged(containerID, instance.OciSubDir) -+ dir, err := instance.GetDir(containerID, instance.OciSubDir) - if err != nil { - return err - } -diff --git a/internal/pkg/runtime/engines/singularity/cleanup.go b/internal/pkg/runtime/engines/singularity/cleanup.go -index 9fd0b514c..515988e6e 100644 ---- a/internal/pkg/runtime/engines/singularity/cleanup.go -+++ b/internal/pkg/runtime/engines/singularity/cleanup.go -@@ -6,12 +6,9 @@ - package singularity - - import ( -- "fmt" - "os" - "syscall" - -- "github.com/sylabs/singularity/internal/pkg/util/mainthread" -- - "github.com/sylabs/singularity/internal/pkg/instance" - "github.com/sylabs/singularity/internal/pkg/sylog" - ) -@@ -45,33 +42,10 @@ func (engine *EngineOperations) CleanupContainer(fatal error, status syscall.Wai - } - - if engine.EngineConfig.GetInstance() { -- uid := os.Getuid() -- - file, err := instance.Get(engine.CommonConfig.ContainerID, instance.SingSubDir) - if err != nil { - return err - } -- -- if file.PPid != os.Getpid() { -- return nil -- } -- -- if file.Privileged { -- var err error -- -- mainthread.Execute(func() { -- if err := syscall.Setresuid(0, 0, uid); err != nil { -- err = fmt.Errorf("failed to escalate privileges") -- return -- } -- defer syscall.Setresuid(uid, uid, 0) -- -- if err = file.Delete(); err != nil { -- return -- } -- }) -- return err -- } - return file.Delete() - } - -diff --git a/internal/pkg/runtime/engines/singularity/prepare.go b/internal/pkg/runtime/engines/singularity/prepare.go -index 1b6cc988e..428a51d5a 100644 ---- a/internal/pkg/runtime/engines/singularity/prepare.go -+++ b/internal/pkg/runtime/engines/singularity/prepare.go -@@ -6,11 +6,17 @@ - package singularity - - import ( -+ "bufio" - "encoding/json" - "fmt" -+ "io/ioutil" - "os" - "path/filepath" -+ "strconv" - "strings" -+ "syscall" -+ -+ "github.com/sylabs/singularity/pkg/util/fs/proc" - - specs "github.com/opencontainers/runtime-spec/specs-go" - "github.com/sylabs/singularity/internal/pkg/buildcfg" -@@ -29,6 +35,16 @@ import ( - "github.com/sylabs/singularity/pkg/util/capabilities" - ) - -+var nsProcName = map[specs.LinuxNamespaceType]string{ -+ specs.PIDNamespace: "pid", -+ specs.UTSNamespace: "uts", -+ specs.IPCNamespace: "ipc", -+ specs.MountNamespace: "mnt", -+ specs.CgroupNamespace: "cgroup", -+ specs.NetworkNamespace: "net", -+ specs.UserNamespace: "user", -+} -+ - // prepareUserCaps is responsible for checking that user's requested - // capabilities are authorized - func (e *EngineOperations) prepareUserCaps() error { -@@ -336,14 +352,29 @@ func (e *EngineOperations) prepareInstanceJoinConfig(starterConfig *starter.Conf - return err - } - -- // check if SUID workflow is really used with a privileged instance -- if !file.PrivilegedPath() && starterConfig.GetIsSUID() { -- return fmt.Errorf("try to join unprivileged instance with SUID workflow") -+ // basic checks: -+ // 1. a user must not use SUID workflow to join an instance -+ // started with user namespace -+ // 2. a user must use SUID workflow to join an instance -+ // started without user namespace -+ if starterConfig.GetIsSUID() && file.UserNs { -+ return fmt.Errorf("joining user namespace with SUID workflow is not allowed") -+ } else if !starterConfig.GetIsSUID() && !file.UserNs { -+ return fmt.Errorf("a setuid installation is required to join this instance") - } - -+ // Pid and PPid are stored in instance file and can be controlled -+ // by users, check to make sure these values are sane -+ if file.Pid <= 1 || file.PPid <= 1 { -+ return fmt.Errorf("bad instance process ID found") -+ } -+ -+ // instance configuration holding configuration read -+ // from instance file - instanceEngineConfig := singularityConfig.NewConfig() - -- // extract configuration from instance file -+ // extract engine configuration from instance file, the whole content -+ // of this file can't be trusted - instanceConfig := &config.Common{ - EngineConfig: instanceEngineConfig, - } -@@ -351,22 +382,150 @@ func (e *EngineOperations) prepareInstanceJoinConfig(starterConfig *starter.Conf - return err - } - -- starterConfig.SetJoinMount(true) -+ // configuration may be altered, be sure to not panic -+ if instanceEngineConfig.OciConfig.Linux == nil { -+ instanceEngineConfig.OciConfig.Linux = &specs.Linux{} -+ } - -- // set namespaces to join -- if err := file.UpdateNamespacesPath(instanceEngineConfig.OciConfig.Linux.Namespaces); err != nil { -+ // go into /proc/ directory to open namespaces inodes -+ // relative to current working directory while joining -+ // namespaces within C starter code as changing directory -+ // here also affects starter process thanks to CLONE_FS. -+ // Additionally it would prevent TOCTOU races and symlink -+ // usage. -+ // And if instance process exits during checks or while -+ // entering in namespace, we would get a "no such process" -+ // error because current working directory would point to a -+ // deleted inode: "/proc/self/cwd -> /proc/ (deleted)" -+ path := filepath.Join("/proc", strconv.Itoa(file.Pid)) -+ if err := mainthread.Chdir(path); err != nil { - return err - } - -- if err := starterConfig.SetNsPathFromSpec(instanceEngineConfig.OciConfig.Linux.Namespaces); err != nil { -- return err -+ uid := os.Getuid() -+ gid := os.Getgid() -+ -+ // enforce checks while joining an instance process with SUID workflow -+ // since instance file is stored in user home directory, we can't trust -+ // its content when using SUID workflow -+ if !file.UserNs && uid != 0 { -+ // check if instance is running with user namespace enabled -+ // by reading /proc/pid/uid_map -+ _, hid, err := proc.ReadIDMap("uid_map") -+ -+ // if the error returned is "no such file or directory" it means -+ // that user namespaces are not supported, just skip this check -+ if err != nil && !os.IsNotExist(err) { -+ return fmt.Errorf("failed to read user namespace mapping: %s", err) -+ } else if err == nil && hid > 0 { -+ // a host uid greater than 0 means user namespace is in use for this process -+ return fmt.Errorf("trying to join an instance running with user namespace enabled") -+ } -+ -+ // read "/proc/pid/root" link of instance process must return -+ // a permission denied error. -+ // This is the "sinit" process (PID 1 in container) and it inherited -+ // setuid bit, so most of "/proc/pid" entries are owned by root:root -+ // like "/proc/pid/root" link even if the process has dropped all -+ // privileges and run with user UID/GID. So we expect a "permission denied" -+ // error when reading link. -+ if _, err := mainthread.Readlink("root"); !os.IsPermission(err) { -+ return fmt.Errorf("trying to join a wrong instance process") -+ } -+ // Since we could be tricked to join namespaces of a root owned process, -+ // we will get UID/GID information of task directory to be sure it belongs -+ // to the user currently joining the instance. Also ensure that a user won't -+ // be able to join other user's instances. -+ fi, err := os.Stat("task") -+ if err != nil { -+ return fmt.Errorf("error while getting information for instance task directory: %s", err) -+ } -+ st := fi.Sys().(*syscall.Stat_t) -+ if st.Uid != uint32(uid) || st.Gid != uint32(gid) { -+ return fmt.Errorf("instance process owned by %d:%d instead of %d:%d", st.Uid, st.Gid, uid, gid) -+ } -+ -+ ppid := -1 -+ -+ // read "/proc/pid/status" to check if instance process -+ // is neither orphaned or faked -+ f, err := os.Open("status") -+ if err != nil { -+ return fmt.Errorf("could not open status: %s", err) -+ } -+ -+ for s := bufio.NewScanner(f); s.Scan(); { -+ if n, _ := fmt.Sscanf(s.Text(), "PPid:\t%d", &ppid); n == 1 { -+ break -+ } -+ } -+ f.Close() -+ -+ // check that Ppid/Pid read from instance file are "somewhat" valid -+ // processes -+ if ppid <= 1 || ppid != file.PPid { -+ return fmt.Errorf("orphaned (or faked) instance process") -+ } -+ -+ // read "/proc/ppid/root" link of parent instance process must return -+ // a permission denied error (same logic than "sinit" process). -+ // Also we don't use absolute path because we want to return an error -+ // if current working directory is deleted meaning that instance process -+ // exited. -+ path := filepath.Join("..", strconv.Itoa(file.PPid), "root") -+ if _, err := mainthread.Readlink(path); !os.IsPermission(err) { -+ return fmt.Errorf("trying to join a wrong instance process") -+ } -+ // "/proc/ppid/task" directory must be owned by user UID/GID -+ path = filepath.Join("..", strconv.Itoa(file.PPid), "task") -+ fi, err = os.Stat(path) -+ if err != nil { -+ return fmt.Errorf("error while getting information for parent task directory: %s", err) -+ } -+ st = fi.Sys().(*syscall.Stat_t) -+ if st.Uid != uint32(uid) || st.Gid != uint32(gid) { -+ return fmt.Errorf("parent instance process owned by %d:%d instead of %d:%d", st.Uid, st.Gid, uid, gid) -+ } - } - -- if e.EngineConfig.OciConfig.Process == nil { -- e.EngineConfig.OciConfig.Process = &specs.Process{} -+ // get starter binary in use -+ dest, err := mainthread.Readlink("/proc/self/exe") -+ if err != nil { -+ return fmt.Errorf("failed to read /proc/self/exe link: %s", err) - } -- if e.EngineConfig.OciConfig.Process.Capabilities == nil { -- e.EngineConfig.OciConfig.Process.Capabilities = &specs.LinuxCapabilities{} -+ // should be either starter-suid or starter -+ exe := filepath.Base(dest) -+ path = filepath.Join("..", strconv.Itoa(file.PPid), "comm") -+ b, err := ioutil.ReadFile(path) -+ if err != nil { -+ return fmt.Errorf("failed to read %s: %s", path, err) -+ } -+ // check that the right starter binary is used according -+ // to namespace configuration and joined instance -+ if exe != strings.Trim(string(b), "\n") { -+ return fmt.Errorf("%s not found in %s, wrong instance process", exe, path) -+ } -+ -+ // tell starter that we are joining an instance -+ starterConfig.SetJoinMount(true) -+ -+ // update namespaces path relative to /proc/ -+ // since starter process is in /proc/ directory -+ for i := range instanceEngineConfig.OciConfig.Linux.Namespaces { -+ // ignore unknown namespaces -+ t := instanceEngineConfig.OciConfig.Linux.Namespaces[i].Type -+ if _, ok := nsProcName[t]; !ok { -+ continue -+ } -+ // set namespace relative path -+ instanceEngineConfig.OciConfig.Linux.Namespaces[i].Path = filepath.Join("ns", nsProcName[t]) -+ } -+ -+ // store namespace paths in starter configuration that will -+ // be passed via a shared memory area and used by starter C code -+ // once this process exit -+ if err := starterConfig.SetNsPathFromSpec(instanceEngineConfig.OciConfig.Linux.Namespaces); err != nil { -+ return err - } - - // duplicate instance capabilities -@@ -378,7 +537,10 @@ func (e *EngineOperations) prepareInstanceJoinConfig(starterConfig *starter.Conf - e.EngineConfig.OciConfig.Process.Capabilities.Ambient = instanceEngineConfig.OciConfig.Process.Capabilities.Ambient - } - -- if os.Getuid() == 0 { -+ // check if user is authorized to set those capabilities and remove -+ // unauthorized capabilities from current set according to capability -+ // configuration file -+ if uid == 0 { - if err := e.prepareRootCaps(); err != nil { - return err - } -@@ -388,7 +550,7 @@ func (e *EngineOperations) prepareInstanceJoinConfig(starterConfig *starter.Conf - } - } - -- // restore apparmor profile -+ // restore apparmor profile or apply a new one if provided - param := security.GetParam(e.EngineConfig.GetSecurity(), "apparmor") - if param != "" { - sylog.Debugf("Applying Apparmor profile %s", param) -@@ -397,7 +559,7 @@ func (e *EngineOperations) prepareInstanceJoinConfig(starterConfig *starter.Conf - e.EngineConfig.OciConfig.SetProcessApparmorProfile(instanceEngineConfig.OciConfig.Process.ApparmorProfile) - } - -- // restore selinux context -+ // restore selinux context or apply a new one if provided - param = security.GetParam(e.EngineConfig.GetSecurity(), "selinux") - if param != "" { - sylog.Debugf("Applying SELinux context %s", param) -@@ -406,7 +568,7 @@ func (e *EngineOperations) prepareInstanceJoinConfig(starterConfig *starter.Conf - e.EngineConfig.OciConfig.SetProcessSelinuxLabel(instanceEngineConfig.OciConfig.Process.SelinuxLabel) - } - -- // restore security features -+ // restore seccomp filter or apply a new one if provided - param = security.GetParam(e.EngineConfig.GetSecurity(), "seccomp") - if param != "" { - sylog.Debugf("Applying seccomp rule from %s", param) -@@ -415,15 +577,20 @@ func (e *EngineOperations) prepareInstanceJoinConfig(starterConfig *starter.Conf - return err - } - } else { -- if instanceEngineConfig.OciConfig.Linux != nil { -- if e.EngineConfig.OciConfig.Linux == nil { -- e.EngineConfig.OciConfig.Linux = &specs.Linux{} -- } -- e.EngineConfig.OciConfig.Linux.Seccomp = instanceEngineConfig.OciConfig.Linux.Seccomp -+ if e.EngineConfig.OciConfig.Linux == nil { -+ e.EngineConfig.OciConfig.Linux = &specs.Linux{} - } -+ e.EngineConfig.OciConfig.Linux.Seccomp = instanceEngineConfig.OciConfig.Linux.Seccomp - } - -- e.EngineConfig.OciConfig.Process.NoNewPrivileges = instanceEngineConfig.OciConfig.Process.NoNewPrivileges -+ // only root user can set this value based on instance file -+ // and always set to true for normal users or if instance file -+ // returned a wrong configuration -+ if uid == 0 && instanceEngineConfig.OciConfig.Process != nil { -+ e.EngineConfig.OciConfig.Process.NoNewPrivileges = instanceEngineConfig.OciConfig.Process.NoNewPrivileges -+ } else { -+ e.EngineConfig.OciConfig.Process.NoNewPrivileges = true -+ } - - return nil - } -diff --git a/internal/pkg/runtime/engines/singularity/process.go b/internal/pkg/runtime/engines/singularity/process.go -index 20adccae8..bcd5524ef 100644 ---- a/internal/pkg/runtime/engines/singularity/process.go -+++ b/internal/pkg/runtime/engines/singularity/process.go -@@ -19,7 +19,6 @@ import ( - - "github.com/sylabs/singularity/internal/pkg/security" - -- "github.com/sylabs/singularity/internal/pkg/util/mainthread" - "github.com/sylabs/singularity/internal/pkg/util/user" - - specs "github.com/opencontainers/runtime-spec/specs-go" -@@ -293,24 +292,13 @@ func (engine *EngineOperations) PostStartProcess(pid int) error { - - if engine.EngineConfig.GetInstance() { - uid := os.Getuid() -- gid := os.Getgid() - name := engine.CommonConfig.ContainerID -- privileged := true - - if err := os.Chdir("/"); err != nil { - return fmt.Errorf("failed to change directory to /: %s", err) - } - -- if engine.EngineConfig.OciConfig.Linux != nil { -- for _, ns := range engine.EngineConfig.OciConfig.Linux.Namespaces { -- if ns.Type == specs.UserNamespace { -- privileged = false -- break -- } -- } -- } -- -- file, err := instance.Add(name, privileged, instance.SingSubDir) -+ file, err := instance.Add(name, instance.SingSubDir) - if err != nil { - return err - } -@@ -329,41 +317,14 @@ func (engine *EngineOperations) PostStartProcess(pid int) error { - file.PPid = os.Getpid() - file.Image = engine.EngineConfig.GetImage() - -- if privileged { -- var err error -- -- mainthread.Execute(func() { -- if err = syscall.Setresuid(0, 0, uid); err != nil { -- err = fmt.Errorf("failed to escalate uid privileges") -- return -- } -- if err = syscall.Setresgid(0, 0, gid); err != nil { -- err = fmt.Errorf("failed to escalate gid privileges") -- return -- } -- if err = file.Update(); err != nil { -- return -- } -- if err = file.MountNamespaces(); err != nil { -- return -- } -- if err = syscall.Setresgid(gid, gid, 0); err != nil { -- err = fmt.Errorf("failed to escalate gid privileges") -- return -- } -- if err := syscall.Setresuid(uid, uid, 0); err != nil { -- err = fmt.Errorf("failed to escalate uid privileges") -- return -- } -- }) -- -- return err -+ for _, ns := range engine.EngineConfig.OciConfig.Linux.Namespaces { -+ if ns.Type == specs.UserNamespace { -+ file.UserNs = true -+ break -+ } - } - -- if err := file.Update(); err != nil { -- return err -- } -- return file.MountNamespaces() -+ return file.Update() - } - return nil - }