[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1027258: marked as done (bullseye-pu: package golang-github-containers-psgo/1.5.2-2~deb11u1)



Your message dated Sat, 29 Apr 2023 10:54:14 +0100
with message-id <502b8fb37ece620c9723446611a9287974ba5a0c.camel@adam-barratt.org.uk>
and subject line Closing p-u requests for fixes included in 11.7
has caused the Debian Bug report #1027258,
regarding bullseye-pu: package golang-github-containers-psgo/1.5.2-2~deb11u1
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1027258: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1027258
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
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.

--- End Message ---
--- Begin Message ---
Package: release.debian.org
Version: 11.7

Hi,

Each of the updates referred to in these requests was included in this
morning's 11.7 point release.

Regards,

Adam

--- End Message ---

Reply to: