Merge pull request #92 from cyphar/proc-do-not-join-ns

internal: proc: do not join the process user namespace

go.mod

@@ -3,7 +3,8 @@ module github.com/containers/psgo
 go 1.14
 
 require (
+	github.com/containers/storage v1.37.1-0.20220112081123-6a0b394c0ad6
 	github.com/opencontainers/runc v1.0.3
 	github.com/stretchr/testify v1.7.0
-	golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2
+	golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359
 )

internal/proc/ns.go

@@ -19,13 +19,9 @@ import (
 	"fmt"
 	"io"
 	"os"
-)
 
-type IDMap struct {
-	ContainerID int
-	HostID      int
-	Size        int
-}
+	"github.com/containers/storage/pkg/idtools"
+)
 
 // ParsePIDNamespace returns the content of /proc/$pid/ns/pid.
 func ParsePIDNamespace(pid string) (string, error) {
@@ -46,14 +42,14 @@ func ParseUserNamespace(pid string) (string, error) {
 }
 
 // ReadMappings reads the user namespace mappings at the specified path
-func ReadMappings(path string) ([]IDMap, error) {
+func ReadMappings(path string) ([]idtools.IDMap, error) {
 	file, err := os.Open(path)
 	if err != nil {
 		return nil, err
 	}
 	defer file.Close()
 
-	mappings := []IDMap{}
+	var mappings []idtools.IDMap
 
 	buf := bufio.NewReader(file)
 	for {
@@ -68,10 +64,10 @@ func ReadMappings(path string) ([]IDMap, error) {
 			return mappings, nil
 		}
 
-		containerID, hostID, size := 0, 0, 0
+		var containerID, hostID, size int
 		if _, err := fmt.Sscanf(string(line), "%d %d %d", &containerID, &hostID, &size); err != nil {
 			return nil, fmt.Errorf("cannot parse %s: %w", string(line), err)
 		}
-		mappings = append(mappings, IDMap{ContainerID: containerID, HostID: hostID, Size: size})
+		mappings = append(mappings, idtools.IDMap{ContainerID: containerID, HostID: hostID, Size: size})
 	}
 }

internal/proc/status.go

@@ -18,8 +18,11 @@ import (
 	"bufio"
 	"fmt"
 	"os"
-	"os/exec"
+	"strconv"
 	"strings"
+	"sync"
+
+	"github.com/containers/storage/pkg/idtools"
 )
 
 // Status is a direct translation of a `/proc/[pid]/status`, which provides much
@@ -173,23 +176,8 @@ type Status struct {
 	NonvoluntaryCtxtSwitches string
 }
 
-// readStatusUserNS joins the user namespace of pid and returns the content of
-// /proc/pid/status as a string slice.
-func readStatusUserNS(pid string) ([]string, error) {
-	path := fmt.Sprintf("/proc/%s/status", pid)
-	args := []string{"nsenter", "-U", "-t", pid, "cat", path}
-
-	c := exec.Command(args[0], args[1:]...)
-	output, err := c.CombinedOutput()
-	if err != nil {
-		return nil, fmt.Errorf("error executing %q: %w", strings.Join(args, " "), err)
-	}
-
-	return strings.Split(string(output), "\n"), nil
-}
-
-// readStatusDefault returns the content of /proc/pid/status as a string slice.
-func readStatusDefault(pid string) ([]string, error) {
+// readStatus returns the content of /proc/pid/status as a string slice.
+func readStatus(pid string) ([]string, error) {
 	path := fmt.Sprintf("/proc/%s/status", pid)
 	f, err := os.Open(path)
 	if err != nil {
@@ -203,21 +191,81 @@ func readStatusDefault(pid string) ([]string, error) {
 	return lines, nil
 }
 
-// ParseStatus parses the /proc/$pid/status file and returns a *Status.
-func ParseStatus(pid string, joinUserNS bool) (*Status, error) {
-	var lines []string
-	var err error
+// mapField maps a single string-typed ID field given the set of mappings. If
+// no mapping exists, the overflow uid/gid is used.
+func mapStatusField(field *string, mapping []idtools.IDMap, overflow string) {
+	hostId, err := strconv.Atoi(*field)
+	if err != nil {
+		*field = overflow
+		return
+	}
+	contId, err := idtools.RawToContainer(hostId, mapping)
+	if err != nil {
+		*field = overflow
+		return
+	}
+	*field = strconv.Itoa(contId)
+}
 
-	if joinUserNS {
-		lines, err = readStatusUserNS(pid)
-	} else {
-		lines, err = readStatusDefault(pid)
+var (
+	overflowOnce sync.Once
+	overflowUid  = "65534"
+	overflowGid  = "65534"
+)
+
+func overflowIds() (string, string) {
+	overflowOnce.Do(func() {
+		if uid, err := os.ReadFile("/proc/sys/kernel/overflowuid"); err == nil {
+			overflowUid = strings.TrimSpace(string(uid))
+		}
+		if gid, err := os.ReadFile("/proc/sys/kernel/overflowgid"); err == nil {
+			overflowGid = strings.TrimSpace(string(gid))
+		}
+	})
+	return overflowUid, overflowGid
+}
+
+// mapStatus takes a Status struct and remaps all of the relevant fields to
+// match the user namespace of the target process.
+func mapStatus(pid string, status *Status) (*Status, error) {
+	uidMap, err := ReadMappings(fmt.Sprintf("/proc/%s/uid_map", pid))
+	if err != nil {
+		return nil, err
+	}
+	gidMap, err := ReadMappings(fmt.Sprintf("/proc/%s/gid_map", pid))
+	if err != nil {
+		return nil, err
+	}
+	overflowUid, overflowGid := overflowIds()
+	for i := range status.Uids {
+		mapStatusField(&status.Uids[i], uidMap, overflowUid)
+	}
+	for i := range status.Gids {
+		mapStatusField(&status.Gids[i], gidMap, overflowGid)
 	}
+	for i := range status.Groups {
+		mapStatusField(&status.Groups[i], gidMap, overflowGid)
+	}
+	return status, nil
+}
 
+// ParseStatus parses the /proc/$pid/status file and returns a *Status.
+func ParseStatus(pid string, mapUserNS bool) (*Status, error) {
+	lines, err := readStatus(pid)
+	if err != nil {
+		return nil, err
+	}
+	status, err := parseStatus(pid, lines)
 	if err != nil {
 		return nil, err
 	}
-	return parseStatus(pid, lines)
+	if mapUserNS {
+		status, err = mapStatus(pid, status)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return status, nil
 }
 
 // parseStatus extracts data from lines and returns a *Status.

psgo.go

@@ -41,28 +41,18 @@ import (
 	"github.com/containers/psgo/internal/dev"
 	"github.com/containers/psgo/internal/proc"
 	"github.com/containers/psgo/internal/process"
+	"github.com/containers/storage/pkg/idtools"
 	"golang.org/x/sys/unix"
 )
 
-// IDMap specifies a mapping range from the host to the container IDs.
-type IDMap struct {
-	// ContainerID is the first ID in the container.
-	ContainerID int
-	// HostID is the first ID in the host.
-	HostID int
-	// Size specifies how long is the range.  e.g. 1 means a single user
-	// is mapped.
-	Size int
-}
-
 // JoinNamespaceOpts specifies different options for joining the specified namespaces.
 type JoinNamespaceOpts struct {
 	// UIDMap specifies a mapping for UIDs in the container.  If specified
 	// huser will perform the reverse mapping.
-	UIDMap []IDMap
+	UIDMap []idtools.IDMap
 	// GIDMap specifies a mapping for GIDs in the container.  If specified
 	// hgroup will perform the reverse mapping.
-	GIDMap []IDMap
+	GIDMap []idtools.IDMap
 
 	// FillMappings specified whether UIDMap and GIDMap must be initialized
 	// with the current user namespace.
@@ -102,7 +92,7 @@ type aixFormatDescriptor struct {
 }
 
 // findID converts the specified id to the host mapping
-func findID(idStr string, mapping []IDMap, lookupFunc func(uid string) (string, error), overflowFile string) (string, error) {
+func findID(idStr string, mapping []idtools.IDMap, lookupFunc func(uid string) (string, error), overflowFile string) (string, error) {
 	if len(mapping) == 0 {
 		return idStr, nil
 	}
@@ -350,29 +340,16 @@ func JoinNamespaceAndProcessInfo(pid string, descriptors []string) ([][]string,
 	return JoinNamespaceAndProcessInfoWithOptions(pid, descriptors, &JoinNamespaceOpts{})
 }
 
-func readMappings(path string) ([]IDMap, error) {
-	mappings, err := proc.ReadMappings(path)
-	if err != nil {
-		return nil, err
-	}
-	var res []IDMap
-	for _, i := range mappings {
-		m := IDMap{ContainerID: i.ContainerID, HostID: i.HostID, Size: i.Size}
-		res = append(res, m)
-	}
-	return res, nil
-}
-
 func contextFromOptions(options *JoinNamespaceOpts) (*psContext, error) {
 	ctx := new(psContext)
 	ctx.opts = options
 	if ctx.opts != nil && ctx.opts.FillMappings {
-		uidMappings, err := readMappings("/proc/self/uid_map")
+		uidMappings, err := proc.ReadMappings("/proc/self/uid_map")
 		if err != nil {
 			return nil, err
 		}
 
-		gidMappings, err := readMappings("/proc/self/gid_map")
+		gidMappings, err := proc.ReadMappings("/proc/self/gid_map")
 		if err != nil {
 			return nil, err
 		}