[release-1.51] Backport CVE-2024-9676 fix

.cirrus.yml

@@ -119,7 +119,7 @@ lint_task:
     env:
         CIRRUS_WORKING_DIR: "/go/src/github.com/containers/storage"
     container:
-        image: golang
+        image: golang:1.19
     modules_cache:
         fingerprint_script: cat go.sum
         folder: $GOPATH/pkg/mod

userns.go

@@ -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/opencontainers/runc/libcontainer/user"
 	"github.com/sirupsen/logrus"
+	"golang.org/x/sys/unix"
 )
 
 // getAdditionalSubIDs looks up the additional IDs configured for
@@ -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
+				}
 			}
 		}
 	}
@@ -309,3 +331,19 @@ 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) {
+	filePath, err := securejoin.SecureJoin(containerMount, file)
+	if err != nil {
+		return nil, err
+	}
+
+	flags := unix.O_PATH | unix.O_CLOEXEC | unix.O_RDONLY
+	fileHandle, err := os.OpenFile(filePath, flags, 0)
+	if err != nil {
+		return nil, err
+	}
+
+	return fileHandle, nil
+}

userns_test.go

@@ -1,6 +1,10 @@
+//go:build linux
+
 package storage
 
 import (
+	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -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)
+			}
+		})
+	}
+}

userns_unsupported.go

@@ -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")
+}