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

CVE issues in podman bullseye



Hello all,

podman bullseye version 3.0.1+dfsg1-3+deb11u1 are affected by the below CVE's,
https://security-tracker.debian.org/tracker/source-package/libpod

CVE-2022-27649 - Fixed in bookworm
CVE-2022-2989 - Not fixed in bookworm
CVE-2022-1227 - Fixed in bookworm

We backported the CVE fixes (CVE-2022-27649 and CVE-2022-1227) to bullseye since we are working on debian bullseye derivative and want to send the patches to debian. We understand these issues are not DSA and have to go though a point release.

What would be the best way to handle them?

The patches are attached,
golang-github-containers-storage:
0001-pkg-idtools-export-RawTo-Container-Host.patch

golang-github-containers-psgo:
0001-internal-proc-do-not-join-the-process-user-namespace.patch
0001-Add-golang-github-containers-storage-dev-dependency.patch

Please could you evaluate and check if these patches can be applied in debian bullseye.

Thank you.

Regards,
Vignesh

>From efd1eb2b2f061f5915c7c5d7e1367087496b69f2 Mon Sep 17 00:00:00 2001
From: Vignesh Raman <vignesh.raman@collabora.com>
Date: Wed, 21 Sep 2022 19:46:16 +0530
Subject: [PATCH] Add golang-github-containers-storage-dev dependency

Add golang-github-containers-storage-dev as dependency
to fix CVE-2022-1227

Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com>
---
 debian/control | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/debian/control b/debian/control
index 5a52891..aad7a8c 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,
  golang-github-opencontainers-runc-dev,
  golang-github-pkg-errors-dev,
  golang-github-sirupsen-logrus-dev,
@@ -26,6 +27,7 @@ Package: golang-github-containers-psgo-dev
 Architecture: all
 Depends:
  ${misc:Depends},
+ golang-github-containers-storage-dev,
  golang-github-opencontainers-runc-dev,
  golang-github-pkg-errors-dev,
  golang-github-sirupsen-logrus-dev,
-- 
2.30.2

>From 0f799b8698f0e180b19679bb05213303189c929f Mon Sep 17 00:00:00 2001
From: Andre Moreira Magalhaes <andrunko@gmail.com>
Date: Mon, 19 Sep 2022 13:59:39 -0300
Subject: [PATCH] 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

(backported from upstream commit d9466da9f563a9de1ece79dcae86b37b1db75443)
---
 internal/proc/ns.go     |  15 ++----
 internal/proc/status.go | 102 +++++++++++++++++++++++++++++-----------
 psgo.go                 |  35 +++-----------
 3 files changed, 86 insertions(+), 66 deletions(-)

diff --git a/internal/proc/ns.go b/internal/proc/ns.go
index 53e5ebd..6671141 100644
--- a/internal/proc/ns.go
+++ b/internal/proc/ns.go
@@ -20,15 +20,10 @@ import (
 	"io"
 	"os"
 
+	"github.com/containers/storage/pkg/idtools"
 	"github.com/pkg/errors"
 )
 
-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) (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, 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, 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, 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})
 	}
 }
diff --git a/internal/proc/status.go b/internal/proc/status.go
index df31139..3f79a5b 100644
--- a/internal/proc/status.go
+++ b/internal/proc/status.go
@@ -17,10 +17,13 @@ package proc
 import (
 	"bufio"
 	"fmt"
+	"io/ioutil"
 	"os"
-	"os/exec"
+	"strconv"
 	"strings"
+	"sync"
 
+	"github.com/containers/storage/pkg/idtools"
 	"github.com/pkg/errors"
 )
 
@@ -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) ([]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)
+}
+
+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
+}
 
-	if joinUserNS {
-		lines, err = readStatusUserNS(pid)
-	} else {
-		lines, err = readStatusDefault(pid)
+// 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.
diff --git a/psgo.go b/psgo.go
index 7c74fd7..5823f37 100644
--- a/psgo.go
+++ b/psgo.go
@@ -40,29 +40,19 @@ 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"
 	"github.com/pkg/errors"
 	"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
 	}
@@ -339,29 +329,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
 		}
-- 
2.37.2

>From 3da85a122411a57b5a65dc243ae56f89d7fd2564 Mon Sep 17 00:00:00 2001
From: Aleksa Sarai <cyphar@cyphar.com>
Date: Wed, 12 Jan 2022 12:56:56 +1100
Subject: [PATCH 1/4] pkg: idtools: export RawTo{Container,Host}

While the IDMapping methods are preferable for most users, sometimes it
is necessary to map a single ID using a given mapping. In particular
this is needed for psgo to be able to map the user and group entries in
/proc/$pid/status using the user namespace of the target process.

Required to resolve CVE-2022-1227.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Backported-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/idtools/idtools.go | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/pkg/idtools/idtools.go b/pkg/idtools/idtools.go
index 83bc8c34f..d3d56066e 100644
--- a/pkg/idtools/idtools.go
+++ b/pkg/idtools/idtools.go
@@ -82,7 +82,7 @@ func GetRootUIDGID(uidMap, gidMap []IDMap) (int, int, error) {
 	if len(uidMap) == 1 && uidMap[0].Size == 1 {
 		uid = uidMap[0].HostID
 	} else {
-		uid, err = toHost(0, uidMap)
+		uid, err = RawToHost(0, uidMap)
 		if err != nil {
 			return -1, -1, err
 		}
@@ -90,7 +90,7 @@ func GetRootUIDGID(uidMap, gidMap []IDMap) (int, int, error) {
 	if len(gidMap) == 1 && gidMap[0].Size == 1 {
 		gid = gidMap[0].HostID
 	} else {
-		gid, err = toHost(0, gidMap)
+		gid, err = RawToHost(0, gidMap)
 		if err != nil {
 			return -1, -1, err
 		}
@@ -98,10 +98,14 @@ func GetRootUIDGID(uidMap, gidMap []IDMap) (int, int, error) {
 	return uid, gid, nil
 }
 
-// toContainer takes an id mapping, and uses it to translate a
-// host ID to the remapped ID. If no map is provided, then the translation
-// assumes a 1-to-1 mapping and returns the passed in id
-func toContainer(hostID int, idMap []IDMap) (int, error) {
+// RawToContainer takes an id mapping, and uses it to translate a host ID to
+// the remapped ID. If no map is provided, then the translation assumes a
+// 1-to-1 mapping and returns the passed in id.
+//
+// If you wish to map a (uid,gid) combination you should use the corresponding
+// IDMappings methods, which ensure that you are mapping the correct ID against
+// the correct mapping.
+func RawToContainer(hostID int, idMap []IDMap) (int, error) {
 	if idMap == nil {
 		return hostID, nil
 	}
@@ -114,10 +118,14 @@ func toContainer(hostID int, idMap []IDMap) (int, error) {
 	return -1, fmt.Errorf("Host ID %d cannot be mapped to a container ID", hostID)
 }
 
-// toHost takes an id mapping and a remapped ID, and translates the
-// ID to the mapped host ID. If no map is provided, then the translation
-// assumes a 1-to-1 mapping and returns the passed in id #
-func toHost(contID int, idMap []IDMap) (int, error) {
+// RawToHost takes an id mapping and a remapped ID, and translates the ID to
+// the mapped host ID. If no map is provided, then the translation assumes a
+// 1-to-1 mapping and returns the passed in id.
+//
+// If you wish to map a (uid,gid) combination you should use the corresponding
+// IDMappings methods, which ensure that you are mapping the correct ID against
+// the correct mapping.
+func RawToHost(contID int, idMap []IDMap) (int, error) {
 	if idMap == nil {
 		return contID, nil
 	}
@@ -188,25 +196,25 @@ func (i *IDMappings) ToHost(pair IDPair) (IDPair, error) {
 	target := i.RootPair()
 
 	if pair.UID != target.UID {
-		target.UID, err = toHost(pair.UID, i.uids)
+		target.UID, err = RawToHost(pair.UID, i.uids)
 		if err != nil {
 			return target, err
 		}
 	}
 
 	if pair.GID != target.GID {
-		target.GID, err = toHost(pair.GID, i.gids)
+		target.GID, err = RawToHost(pair.GID, i.gids)
 	}
 	return target, err
 }
 
 // ToContainer returns the container UID and GID for the host uid and gid
 func (i *IDMappings) ToContainer(pair IDPair) (int, int, error) {
-	uid, err := toContainer(pair.UID, i.uids)
+	uid, err := RawToContainer(pair.UID, i.uids)
 	if err != nil {
 		return -1, -1, err
 	}
-	gid, err := toContainer(pair.GID, i.gids)
+	gid, err := RawToContainer(pair.GID, i.gids)
 	return uid, gid, err
 }
 
-- 
2.30.2


Reply to: