Skip to content

Commit

Permalink
Merge pull request #2135 from mheon/backport_2024_9676_release155
Browse files Browse the repository at this point in the history
[release-1.55] backport fix for CVE-2026-9676
  • Loading branch information
openshift-merge-bot[bot] authored Oct 15, 2024
2 parents d46cc6a + eccfbd0 commit 465d8c0
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 28 deletions.
2 changes: 1 addition & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ lint_task:
env:
CIRRUS_WORKING_DIR: "/go/src/github.com/containers/storage"
container:
image: golang
image: golang:1.21
modules_cache:
fingerprint_script: cat go.sum
folder: $GOPATH/pkg/mod
Expand Down
87 changes: 60 additions & 27 deletions userns.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
//go:build linux

package storage

import (
"fmt"
"os"
"os/user"
"path/filepath"
"strconv"

drivers "github.com/containers/storage/drivers"
"github.com/containers/storage/pkg/idtools"
"github.com/containers/storage/pkg/unshare"
"github.com/containers/storage/types"
securejoin "github.com/cyphar/filepath-securejoin"
libcontainerUser "github.com/moby/sys/user"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
)

// getAdditionalSubIDs looks up the additional IDs configured for
Expand Down Expand Up @@ -85,40 +88,59 @@ const nobodyUser = 65534
// parseMountedFiles returns the maximum UID and GID found in the /etc/passwd and
// /etc/group files.
func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 {
var (
passwd *os.File
group *os.File
size int
err error
)
if passwdFile == "" {
passwdFile = filepath.Join(containerMount, "etc/passwd")
}
if groupFile == "" {
groupFile = filepath.Join(groupFile, "etc/group")
passwd, err = secureOpen(containerMount, "/etc/passwd")
} else {
// User-specified override from a volume. Will not be in
// container root.
passwd, err = os.Open(passwdFile)
}

size := 0

users, err := libcontainerUser.ParsePasswdFile(passwdFile)
if err == nil {
for _, u := range users {
// Skip the "nobody" user otherwise we end up with 65536
// ids with most images
if u.Name == "nobody" {
continue
}
if u.Uid > size && u.Uid != nobodyUser {
size = u.Uid
}
if u.Gid > size && u.Gid != nobodyUser {
size = u.Gid
defer passwd.Close()

users, err := libcontainerUser.ParsePasswd(passwd)
if err == nil {
for _, u := range users {
// Skip the "nobody" user otherwise we end up with 65536
// ids with most images
if u.Name == "nobody" || u.Name == "nogroup" {
continue
}
if u.Uid > size && u.Uid != nobodyUser {
size = u.Uid + 1
}
if u.Gid > size && u.Gid != nobodyUser {
size = u.Gid + 1
}
}
}
}

groups, err := libcontainerUser.ParseGroupFile(groupFile)
if groupFile == "" {
group, err = secureOpen(containerMount, "/etc/group")
} else {
// User-specified override from a volume. Will not be in
// container root.
group, err = os.Open(groupFile)
}
if err == nil {
for _, g := range groups {
if g.Name == "nobody" {
continue
}
if g.Gid > size && g.Gid != nobodyUser {
size = g.Gid
defer group.Close()

groups, err := libcontainerUser.ParseGroup(group)
if err == nil {
for _, g := range groups {
if g.Name == "nobody" || g.Name == "nogroup" {
continue
}
if g.Gid > size && g.Gid != nobodyUser {
size = g.Gid + 1
}
}
}
}
Expand Down Expand Up @@ -309,3 +331,14 @@ func getAutoUserNSIDMappings(
gidMap := append(availableGIDs.zip(requestedContainerGIDs), additionalGIDMappings...)
return uidMap, gidMap, nil
}

// Securely open (read-only) a file in a container mount.
func secureOpen(containerMount, file string) (*os.File, error) {
tmpFile, err := securejoin.OpenInRoot(containerMount, file)
if err != nil {
return nil, err
}
defer tmpFile.Close()

return securejoin.Reopen(tmpFile, unix.O_RDONLY)
}
97 changes: 97 additions & 0 deletions userns_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
//go:build linux

package storage

import (
"os"
"path/filepath"
"reflect"
"testing"

Expand Down Expand Up @@ -170,3 +174,96 @@ func TestGetAutoUserNSMapping(t *testing.T) {
})
}
}

func TestParseMountedFiles(t *testing.T) {
tests := []struct {
name string
passwdContent string
groupContent string
expectedMax uint32
}{
{
name: "basic case",
passwdContent: `
root:x:0:0:root:/root:/bin/bash
user1:x:1000:1000::/home/user1:/bin/bash
nobody:x:65534:65534:nobody:/nonexistent:/usr/sbin/nologin`,
groupContent: `
root:x:0:
user1:x:1000:
nogroup:x:65534:`,
expectedMax: 1001,
},
{
name: "only passwd",
passwdContent: `
root:x:0:0:root:/root:/bin/bash
user1:x:4001:4001::/home/user1:/bin/bash
nobody:x:65534:65534:nobody:/nonexistent:/usr/sbin/nologin`,
groupContent: "",
expectedMax: 4002,
},
{
name: "only groups",
passwdContent: "",
groupContent: `
root:x:0:
admin:x:3000:
nobody:x:65534:`,
expectedMax: 3001,
},
{
name: "empty files",
passwdContent: "",
groupContent: "",
expectedMax: 0,
},
{
name: "invalid passwd file",
passwdContent: "FOOBAR",
groupContent: "",
expectedMax: 0,
},
{
name: "invalid groups file",
passwdContent: "",
groupContent: "FOOBAR",
expectedMax: 0,
},
{
name: "nogroup ignored",
passwdContent: "",
groupContent: `
root:x:0:
admin:x:4000:
nogroup:x:65533:`,
expectedMax: 4001,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "containermount")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

passwdFile := filepath.Join(tmpDir, "passwd")
if err := os.WriteFile(passwdFile, []byte(tt.passwdContent), 0o644); err != nil {
t.Fatalf("Failed to write passwd file: %v", err)
}

groupFile := filepath.Join(tmpDir, "group")
if err := os.WriteFile(groupFile, []byte(tt.groupContent), 0o644); err != nil {
t.Fatalf("Failed to write group file: %v", err)
}

result := parseMountedFiles(tmpDir, passwdFile, groupFile)

if result != tt.expectedMax {
t.Errorf("Expected max %d, but got %d", tt.expectedMax, result)
}
})
}
}
14 changes: 14 additions & 0 deletions userns_unsupported.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//go:build !linux

package storage

import (
"errors"

"github.com/containers/storage/pkg/idtools"
"github.com/containers/storage/types"
)

func (s *store) getAutoUserNS(_ *types.AutoUserNsOptions, _ *Image, _ rwLayerStore, _ []roLayerStore) ([]idtools.IDMap, []idtools.IDMap, error) {
return nil, nil, errors.New("user namespaces are not supported on this platform")
}

0 comments on commit 465d8c0

Please sign in to comment.