Bug#1027258: bullseye-pu: package golang-github-containers-psgo/1.5.2-2~deb11u1
Package: release.debian.org
Severity: normal
Tags: bullseye
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: golang-github-containers-psgo@packages.debian.org, siretart@tauware.de, siretart@gmail.com, Vignesh Raman vignesh.raman@collabora.com
Control: affects -1 + src:golang-github-containers-psgo
[ Reason ]
Backport for CVE-2022-1227, taken from https://github.com/containers/psgo/pull/92
This prevents an exploit when running 'podman top'
[ Impact ]
[ Tests ]
No new test is added. The code change is rather small and straight-forward to
review. It required small changes to apply to this older version.
[ Risks ]
I determined that the code change and adaptions to version 1.5.2 is easier to
review than updating containers-psgo to upstream version 1.7.2, which would
be closer to how upstream or Fedora/RedHat would recommend to address this issue.
[ Checklist ]
[x] *all* changes are documented in the d/changelog
[x] I reviewed all changes and I approve them
[x] attach debdiff against the package in (old)stable
[x] the issue is verified as fixed in unstable
[ Changes ]
diff --git a/debian/changelog b/debian/changelog
index a1f0d96..0448fdf 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+golang-github-containers-psgo (1.5.2-2~deb11u1) bullseye; urgency=medium
+
+ * CVE-2022-1227: do not join the process user namespace
+
+ -- Reinhard Tartler <siretart@tauware.de> Wed, 28 Dec 2022 21:21:57 -0500
+
golang-github-containers-psgo (1.5.2-1) unstable; urgency=medium
* Team upload
diff --git a/debian/control b/debian/control
index 5a52891..ba4c8e5 100644
--- a/debian/control
+++ b/debian/control
@@ -10,6 +10,7 @@ Build-Depends:
dh-golang,
Build-Depends-Indep:
golang-any,
+ golang-github-containers-storage-dev (>= 1.24.8+dfsg1-2~deb11u1),
golang-github-opencontainers-runc-dev,
golang-github-pkg-errors-dev,
golang-github-sirupsen-logrus-dev,
diff --git a/debian/patches/CVE-2022-1227.patch b/debian/patches/CVE-2022-1227.patch
new file mode 100644
index 0000000..991a7c5
--- /dev/null
+++ b/debian/patches/CVE-2022-1227.patch
@@ -0,0 +1,279 @@
+commit 3ae3044916481f5c001f64a9d26110b878a676e0 (github/v1.7.1-fedora)
+Author: Aleksa Sarai <cyphar@cyphar.com>
+Date: Wed Jan 12 01:01:30 2022 +1100
+
+ internal: proc: do not join the process user namespace
+
+ 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.
+
+ Fixes: CVE-2022-1227
+
+ Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
+ Backported-by: Valentin Rothberg <vrothberg@redhat.com>
+ Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
+
+Index: golang-github-containers-psgo/internal/proc/ns.go
+===================================================================
+--- golang-github-containers-psgo.orig/internal/proc/ns.go
++++ golang-github-containers-psgo/internal/proc/ns.go
+@@ -21,14 +21,9 @@ import (
+ "os"
+
+ "github.com/pkg/errors"
++ "github.com/containers/storage/pkg/idtools"
+ )
+
+-type IDMap struct {
+- ContainerID int
+- HostID int
+- Size int
+-}
+-
+ // ParsePIDNamespace returns the content of /proc/$pid/ns/pid.
+ func ParsePIDNamespace(pid string) (string, error) {
+ pidNS, err := os.Readlink(fmt.Sprintf("/proc/%s/ns/pid", pid))
+@@ -48,14 +43,14 @@ func ParseUserNamespace(pid string) (str
+ }
+
+ // 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, errors.Wrapf(err, "cannot open %s", path)
+ }
+ defer file.Close()
+
+- mappings := []IDMap{}
++ var mappings []idtools.IDMap
+
+ buf := bufio.NewReader(file)
+ for {
+@@ -70,10 +65,10 @@ func ReadMappings(path string) ([]IDMap,
+ 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, errors.Wrapf(err, "cannot parse %s", string(line))
+ }
+- mappings = append(mappings, IDMap{ContainerID: containerID, HostID: hostID, Size: size})
++ mappings = append(mappings, idtools.IDMap{ContainerID: containerID, HostID: hostID, Size: size})
+ }
+ }
+Index: golang-github-containers-psgo/internal/proc/status.go
+===================================================================
+--- golang-github-containers-psgo.orig/internal/proc/status.go
++++ golang-github-containers-psgo/internal/proc/status.go
+@@ -17,11 +17,14 @@ package proc
+ import (
+ "bufio"
+ "fmt"
++ "io/ioutil"
+ "os"
+- "os/exec"
++ "strconv"
+ "strings"
++ "sync"
+
+ "github.com/pkg/errors"
++ "github.com/containers/storage/pkg/idtools"
+ )
+
+ // Status is a direct translation of a `/proc/[pid]/status`, which provides much
+@@ -175,23 +178,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: %v", 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 {
+@@ -205,21 +193,81 @@ func readStatusDefault(pid string) ([]st
+ 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
+-
+- if joinUserNS {
+- lines, err = readStatusUserNS(pid)
+- } else {
+- lines, err = readStatusDefault(pid)
++// 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)
++}
++
++var (
++ overflowOnce sync.Once
++ overflowUid = "65534"
++ overflowGid = "65534"
++)
++
++func overflowIds() (string, string) {
++ overflowOnce.Do(func() {
++ if uid, err := ioutil.ReadFile("/proc/sys/kernel/overflowuid"); err == nil {
++ overflowUid = strings.TrimSpace(string(uid))
++ }
++ if gid, err := ioutil.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
+ }
+- return parseStatus(pid, lines)
++ status, err := parseStatus(pid, lines)
++ if err != nil {
++ return nil, err
++ }
++ 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.
+Index: golang-github-containers-psgo/psgo.go
+===================================================================
+--- golang-github-containers-psgo.orig/psgo.go
++++ golang-github-containers-psgo/psgo.go
+@@ -41,6 +41,7 @@ import (
+ "github.com/containers/psgo/internal/proc"
+ "github.com/containers/psgo/internal/process"
+ "github.com/pkg/errors"
++ "github.com/containers/storage/pkg/idtools"
+ "golang.org/x/sys/unix"
+ )
+
+@@ -59,10 +60,10 @@ type IDMap struct {
+ 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 +103,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
+ }
+@@ -339,29 +340,16 @@ func JoinNamespaceAndProcessInfo(pid str
+ 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
+ }
diff --git a/debian/patches/series b/debian/patches/series
index 0fa5bd3..c79ee17 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,2 @@
test--skip-TestGetPIDSFromCgroup.patch
+CVE-2022-1227.patch
[ Other info ]
It has to be looked at in context of #1027257 which introduces an
important helper function that is being used in this version.
Reply to: