Package: release.debian.org Severity: normal Tags: bullseye User: release.debian....@packages.debian.org Usertags: pu X-Debbugs-Cc: golang-github-containers-p...@packages.debian.org, siret...@tauware.de, siret...@gmail.com, Vignesh Raman vignesh.ra...@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 <siret...@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 <cyp...@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 <cyp...@cyphar.com> + Backported-by: Valentin Rothberg <vrothb...@redhat.com> + Signed-off-by: Valentin Rothberg <vrothb...@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.
