Skip to content

Commit

Permalink
internal: proc: do not join the process user namespace
Browse files Browse the repository at this point in the history
The only reason we joined the process user namespace was to map a
handful of fields into the same usernamepsace as that process. This
procedure can be implemented entirely in Go without having to run code
inside the container.

In addition, since psgo is used inside "podman top", we were actually
executing the nsenter binary *from the container* without all of the
container's security profiles applied. At the very least this would
allow a container process to return bad data to psgo (possibly confusing
management scripts using psgo) and at the very worst it would allow the
container process to escalate privileges by getting podman to execute
code without all of the container security profiles applied.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
  • Loading branch information
cyphar committed Jan 11, 2022
1 parent 87c257b commit d67bb38
Show file tree
Hide file tree
Showing 24 changed files with 1,400 additions and 97 deletions.
1 change: 1 addition & 0 deletions go.mod
Expand Up @@ -5,6 +5,7 @@ go 1.14
require (
github.com/opencontainers/runc v1.0.3
github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417
github.com/opencontainers/umoci v0.4.7
github.com/stretchr/testify v1.7.0
golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2
)
141 changes: 139 additions & 2 deletions go.sum

Large diffs are not rendered by default.

103 changes: 76 additions & 27 deletions internal/proc/status.go
Expand Up @@ -18,8 +18,12 @@ import (
"bufio"
"fmt"
"os"
"os/exec"
"strconv"
"strings"
"sync"

rspec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/opencontainers/umoci/pkg/idtools"
)

// Status is a direct translation of a `/proc/[pid]/status`, which provides much
Expand Down Expand Up @@ -173,23 +177,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 {
Expand All @@ -203,21 +192,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 []rspec.LinuxIDMapping, overflow string) {
hostId, err := strconv.Atoi(*field)
if err != nil {
*field = overflow
return
}
contId, err := idtools.ToContainer(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.
Expand Down
202 changes: 202 additions & 0 deletions vendor/github.com/opencontainers/umoci/COPYING

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d67bb38

Please sign in to comment.