CVE-2019-11328

@@ -180,9 +180,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)
@@ -457,7 +455,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)
 		}

@@ -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
 	}

@@ -15,48 +15,34 @@ import (
 	"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 (
 	// OciSubDir represents directory where OCI instance files are stored
 	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
@@ -86,7 +72,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
@@ -101,52 +87,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 err != nil {
-		return path, err
-	} else 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 {
@@ -164,16 +125,16 @@ 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
 	}
 	_, err := Get(name, subDir)
 	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
 	}
@@ -185,63 +146,41 @@ 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 {
+	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
 		}
-		pattern := filepath.Join(path, name, name+".json")
-		files, err := filepath.Glob(pattern)
+		b, err := ioutil.ReadAll(r)
+		r.Close()
 		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)
-			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
+		f := &File{Path: file}
+		if err := json.Unmarshal(b, f); err != nil {
+			return nil, err
 		}
+		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
@@ -259,119 +198,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/<pid>/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, err := ProcName(i.Name, i.User)
-	if err != nil {
-		return err
-	}
-	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
 	}
@@ -388,12 +231,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
 	}

@@ -10,7 +10,6 @@ import (
 	"path/filepath"
 	"testing"
 
-	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sylabs/singularity/internal/pkg/test"
 )
 
@@ -159,103 +158,20 @@ func TestCheckName(t *testing.T) {
 	}
 }
 
-func TestGetDirUnprivileged(t *testing.T) {
-	test.EnsurePrivilege(t)
-
-	hostname, err := os.Hostname()
-	if err != nil {
-		t.Fatalf("unable to retrieve hostname: %s", err)
-	}
-
-	instancePath := filepath.Join("/root", unprivPath, testSubDir, hostname, "root")
-
-	tests := []struct {
-		name          string
-		path          string
-		expectFailure bool
-	}{
-		{
-			name:          "test",
-			path:          filepath.Join(instancePath, "test"),
-			expectFailure: false,
-		},
-		{
-			name:          "test/123",
-			expectFailure: true,
-		},
-	}
-	for _, e := range tests {
-		path, err := GetDirUnprivileged(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)
-		} else if !e.expectFailure && path != e.path {
-			t.Errorf("unexpected path returned %s instead of %s", path, e.path)
-		}
-	}
-}
-
-func TestGetDirPrivileged(t *testing.T) {
-	test.EnsurePrivilege(t)
-
-	instancePath := filepath.Join(privPath, testSubDir, "root")
-
-	tests := []struct {
-		name          string
-		path          string
-		expectFailure bool
-	}{
-		{
-			name:          "test",
-			path:          filepath.Join(instancePath, "test"),
-			expectFailure: false,
-		},
-		{
-			name:          "test/123",
-			expectFailure: true,
-		},
-	}
-	for _, e := range tests {
-		path, err := GetDirPrivileged(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)
-		} else if !e.expectFailure && path != e.path {
-			t.Errorf("unexpected path returned %s instead of %s", path, e.path)
-		}
-	}
-}
-
 var instanceTests = []struct {
 	name          string
-	privileged    bool
 	expectFailure bool
 }{
 	{
-		name:          "valid_privileged_instance",
-		privileged:    true,
-		expectFailure: false,
-	},
-	{
-		name:          "valid_privileged_instance",
-		privileged:    true,
-		expectFailure: true,
-	},
-	{
-		name:          "valid_unprivileged_instance",
-		privileged:    false,
+		name:          "valid_instance",
 		expectFailure: false,
 	},
 	{
-		name:          "invalid privileged_instance",
-		privileged:    true,
+		name:          "valid_instance",
 		expectFailure: true,
 	},
 	{
-		name:          "invalid unprivileged_instance",
-		privileged:    false,
+		name:          "invalid instance",
 		expectFailure: true,
 	},
 }
@@ -267,41 +183,29 @@ func TestAdd(t *testing.T) {
 		var err error
 		var file *File
 
-		file, err = Add(e.name, e.privileged, testSubDir)
+		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 {
-			file.User = "root"
-			file.Pid = os.Getpid()
-			if err := file.Update(); err != nil {
-				t.Errorf("error while creating instance %s: %s", e.name, err)
-			}
-			if err := file.MountNamespaces(); err != nil {
-				t.Errorf("error while mounting namespaces: %s", err)
-			}
-			err := file.UpdateNamespacesPath([]specs.LinuxNamespace{})
-			if err == nil {
-				t.Errorf("unexpected success while updating namespace paths")
-			}
-			// should always fail with 'no command line match found'
-			file.PPid = file.Pid
-			err = file.UpdateNamespacesPath([]specs.LinuxNamespace{})
-			if err == nil {
-				t.Errorf("unexpected success while updating namespace paths")
-			}
-			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)
-			}
+		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)
 		}
 	}
 }
@@ -319,19 +223,23 @@ func TestGet(t *testing.T) {
 		} else if err == nil && e.expectFailure {
 			t.Errorf("unexpected success for name %s", e.name)
 		}
-		if file != nil {
-			if file.User != "root" {
-				t.Errorf("unexpected user returned %s", file.User)
-			}
-			if e.privileged && !file.PrivilegedPath() {
-				t.Errorf("unexpected path for privileged instance")
-			} else if !e.privileged && file.PrivilegedPath() {
-				t.Errorf("unexpected path for unprivileged instance")
-			}
-			err = file.Delete()
-			if err != nil && !e.expectFailure {
-				t.Errorf("unexpected error while deleting instance %s: %s", e.name, err)
-			}
+		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)
 		}
 	}
 }

@@ -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
 	}
@@ -803,7 +803,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
 	}

@@ -229,7 +229,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
 		}

@@ -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"
 )
@@ -47,33 +44,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()
 	}
 

@@ -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 {
@@ -359,37 +375,170 @@ 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, just check for cool values
+	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,
 	}
 	if err := json.Unmarshal(file.Config, instanceConfig); err != nil {
 		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/<pid> 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/<pid> (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
+		if _, err := mainthread.Readlink("root"); !os.IsPermission(err) {
+			return fmt.Errorf("trying to join a wrong instance process")
+		}
+		// "/proc/pid/task" directory must be owned by user UID/GID
+		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.
+		// Also we don't use absolute path because we want to return an error
+		// if current working directory is deleted.
+		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/<pid>
+	// since starter process is in /proc/<pid> 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
@@ -401,7 +550,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
 		}
@@ -411,7 +563,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)
@@ -420,7 +572,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)
@@ -429,7 +581,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)
@@ -438,15 +590,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
 }

@@ -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
 }