From b04d0e9293889cd06df102755e9320846bb06176 Mon Sep 17 00:00:00 2001 From: gVisor bot Date: Fri, 2 Aug 2024 11:35:58 -0700 Subject: [PATCH] Internal change. PiperOrigin-RevId: 658853348 --- pkg/sentry/control/lifecycle.go | 6 +- pkg/sentry/fsimpl/user/BUILD | 1 + pkg/sentry/fsimpl/user/user.go | 160 ++++++++++++++++++------ pkg/sentry/fsimpl/user/user_test.go | 186 ++++++++++++++++++++-------- 4 files changed, 259 insertions(+), 94 deletions(-) diff --git a/pkg/sentry/control/lifecycle.go b/pkg/sentry/control/lifecycle.go index 98efe71b5..926658391 100644 --- a/pkg/sentry/control/lifecycle.go +++ b/pkg/sentry/control/lifecycle.go @@ -215,11 +215,7 @@ func (l *Lifecycle) StartContainer(args *StartContainerArgs, _ *uint32) error { if uid != 0 || gid != 0 { return fmt.Errorf("container spec specified both an explicit UID/GID and a user name, only one or the other may be provided") } - var err error - uid, gid, err = user.GetExecUIDGIDFromUser(l.Kernel.SupervisorContext(), mntns, args.User) - if err != nil { - return fmt.Errorf("couldn't retrieve UID and GID for user %v, err: %v", args.User, err) - } + uid, gid = user.GetExecUIDGIDFromUser(l.Kernel.SupervisorContext(), mntns, args.User) } creds := auth.NewUserCredentials( diff --git a/pkg/sentry/fsimpl/user/BUILD b/pkg/sentry/fsimpl/user/BUILD index 42dcd889a..3f94a847b 100644 --- a/pkg/sentry/fsimpl/user/BUILD +++ b/pkg/sentry/fsimpl/user/BUILD @@ -33,6 +33,7 @@ go_test( deps = [ "//pkg/abi/linux", "//pkg/context", + "//pkg/errors/linuxerr", "//pkg/fspath", "//pkg/sentry/fsimpl/tmpfs", "//pkg/sentry/kernel/auth", diff --git a/pkg/sentry/fsimpl/user/user.go b/pkg/sentry/fsimpl/user/user.go index fcfb07aef..530d2cde8 100644 --- a/pkg/sentry/fsimpl/user/user.go +++ b/pkg/sentry/fsimpl/user/user.go @@ -26,11 +26,17 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/fspath" + "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/pkg/usermem" ) +const ( + defaultUID = auth.KUID(0) + defaultGID = auth.KGID(0) +) + type fileReader struct { ctx context.Context fd *vfs.FileDescription @@ -162,9 +168,80 @@ func findHomeInPasswd(uid uint32, passwd io.Reader, defaultHome string) (string, return defaultHome, nil } -func findUIDGIDInPasswd(passwd io.Reader, user string) (auth.KUID, auth.KGID, error) { - defaultUID := auth.KUID(auth.OverflowUID) - defaultGID := auth.KGID(auth.OverflowGID) +func openFile(ctx context.Context, mns *vfs.MountNamespace, path string) (*vfs.FileDescription, error) { + log.Infof("Opening %q", path) + root := mns.Root(ctx) + defer root.DecRef(ctx) + creds := auth.CredentialsFromContext(ctx) + + target := &vfs.PathOperation{ + Root: root, + Start: root, + Path: fspath.Parse(path), + } + fd, err := root.Mount().Filesystem().VirtualFilesystem().OpenAt(ctx, creds, target, &vfs.OpenOptions{Flags: linux.O_RDONLY}) + if err != nil { + log.Warningf("Failed to open %q, error: %v", path, err) + return nil, err + } + + return fd, nil +} + +// findGroupInGroupFile parses a group file and returns the given group's +// gid. If the gid is a number, we don't need to read the file. +// +// If we don't find the group, we return 0. +func findGroupInGroupFile(ctx context.Context, mns *vfs.MountNamespace, gidString string) auth.KGID { + // gid is a number, we don't need to read the file. + gidInt, err := strconv.Atoi(gidString) + if err == nil { + return auth.KGID(gidInt) + } + + fd, err := openFile(ctx, mns, "/etc/group") + if err != nil { + return defaultGID + } + defer fd.DecRef(ctx) + + r := &fileReader{ + ctx: ctx, + fd: fd, + } + + // Group file format: + // group_name:password:gid: + const ( + grpIdx = 0 + passwdIdx = 1 + gidIdx = 2 + ) + + s := bufio.NewScanner(r) + for s.Scan() { + if err := s.Err(); err != nil { + return defaultGID + } + line := strings.TrimSpace(s.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + parts := strings.Split(line, ":") + if parts[grpIdx] == gidString { + gidInt, err := strconv.Atoi(parts[gidIdx]) + if err != nil { + return defaultGID + } + return auth.KGID(gidInt) + } + } + + // Not found, return 0 + return defaultGID +} + +func findUIDGIDInPasswd(passwd io.Reader, user string) (auth.KUID, auth.KGID) { uid := defaultUID gid := defaultGID @@ -201,7 +278,7 @@ func findUIDGIDInPasswd(passwd io.Reader, user string) (auth.KUID, auth.KGID, er s := bufio.NewScanner(passwd) for s.Scan() { if err := s.Err(); err != nil { - return defaultUID, defaultGID, err + return getDefaultUIDGID(user) } line := strings.TrimSpace(s.Text()) @@ -211,8 +288,8 @@ func findUIDGIDInPasswd(passwd io.Reader, user string) (auth.KUID, auth.KGID, er parts := strings.Split(line, ":") if len(parts) != numFields { - // Return error if the format is invalid. - return defaultUID, defaultGID, fmt.Errorf("invalid line found in /etc/passwd, there should be 7 fields but found %v", len(parts)) + // If format is invalid, return default values. + return getDefaultUIDGID(user) } for i := 0; i < numFields; i++ { // The password, GECOS and user command interpreter fields are @@ -221,49 +298,56 @@ func findUIDGIDInPasswd(passwd io.Reader, user string) (auth.KUID, auth.KGID, er continue } if parts[i] == "" { - // Return error if the format is invalid. - return defaultUID, defaultGID, fmt.Errorf("invalid line found in /etc/passwd, field[%v] is empty", i) + // If format is invalid, return default values. + return getDefaultUIDGID(user) } } if parts[idxToMatch] == uStringOrID { parseUID, err := strconv.ParseUint(parts[uidIdx], 10, 32) if err != nil { - return defaultUID, defaultGID, err + return getDefaultUIDGID(user) } parseGID, err := strconv.ParseUint(parts[gidIdx], 10, 32) if err != nil { - return defaultUID, defaultGID, err - } - - if uid != defaultUID || gid != defaultGID { - return defaultUID, defaultGID, fmt.Errorf("multiple matches for the user: %v", user) + return getDefaultUIDGID(user) } uid = auth.KUID(parseUID) gid = auth.KGID(parseGID) + return uid, gid } } - if uid == defaultUID || gid == defaultGID { - return defaultUID, defaultGID, fmt.Errorf("couldn't retrieve UID/GID from user: %v", user) - } - return uid, gid, nil + + return getDefaultUIDGID(user) } -func getExecUIDGID(ctx context.Context, mns *vfs.MountNamespace, user string) (auth.KUID, auth.KGID, error) { - root := mns.Root(ctx) - defer root.DecRef(ctx) +func getDefaultUIDGID(user string) (auth.KUID, auth.KGID) { + usergroup := strings.SplitN(user, ":", 2) + uid := defaultUID + gid := defaultGID - creds := auth.CredentialsFromContext(ctx) - - target := &vfs.PathOperation{ - Root: root, - Start: root, - Path: fspath.Parse("/etc/passwd"), + // resolving uid. If it is numeric, set uid to the int value, if not keep it to 0. + u, err := strconv.Atoi(usergroup[0]) + if err == nil { + uid = auth.KUID(u) } - fd, err := root.Mount().Filesystem().VirtualFilesystem().OpenAt(ctx, creds, target, &vfs.OpenOptions{Flags: linux.O_RDONLY}) + // if we do have a group, try to get the numeric value. If numeric, set gid to the int value, if + // not keep it to 0. + if len(usergroup) == 2 { + g, err := strconv.Atoi(usergroup[1]) + if err == nil { + gid = auth.KGID(g) + } + } + + return uid, gid +} + +func getExecUIDGID(ctx context.Context, mns *vfs.MountNamespace, user string) (auth.KUID, auth.KGID) { + fd, err := openFile(ctx, mns, "/etc/passwd") if err != nil { - return auth.KUID(auth.OverflowUID), auth.KGID(auth.OverflowGID), fmt.Errorf("couldn't retrieve UID/GID from user: %v, err: %v", user, err) + return getDefaultUIDGID(user) } defer fd.DecRef(ctx) @@ -271,17 +355,21 @@ func getExecUIDGID(ctx context.Context, mns *vfs.MountNamespace, user string) (a ctx: ctx, fd: fd, } + // This return kGid from the passwd file (if we find one). We might have recieved a group id + // string or numeric from the user. + kUID, kGID := findUIDGIDInPasswd(r, user) + usergroup := strings.SplitN(user, ":", 2) + if len(usergroup) == 2 { + kGID = findGroupInGroupFile(ctx, mns, usergroup[1]) + } + return kUID, kGID - return findUIDGIDInPasswd(r, user) } // GetExecUIDGIDFromUser retrieves the UID and GID from /etc/passwd file for // the given user. -func GetExecUIDGIDFromUser(ctx context.Context, vmns *vfs.MountNamespace, user string) (auth.KUID, auth.KGID, error) { +func GetExecUIDGIDFromUser(ctx context.Context, vmns *vfs.MountNamespace, user string) (auth.KUID, auth.KGID) { // Read /etc/passwd and retrieve the UID/GID based on the user string. - uid, gid, err := getExecUIDGID(ctx, vmns, user) - if err != nil { - return uid, gid, fmt.Errorf("error reading /etc/passwd: %v", err) - } - return uid, gid, nil + uid, gid := getExecUIDGID(ctx, vmns, user) + return uid, gid } diff --git a/pkg/sentry/fsimpl/user/user_test.go b/pkg/sentry/fsimpl/user/user_test.go index 68e3f1843..8dc18ec30 100644 --- a/pkg/sentry/fsimpl/user/user_test.go +++ b/pkg/sentry/fsimpl/user/user_test.go @@ -21,6 +21,7 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" "gvisor.dev/gvisor/pkg/sentry/fsimpl/tmpfs" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" @@ -29,10 +30,10 @@ import ( "gvisor.dev/gvisor/pkg/usermem" ) -// createEtcPasswd creates /etc/passwd with the given contents and mode. If +// createEtcFile creates /etc/ with the given contents and mode. If // mode is empty, then no file will be created. If mode is not a regular file // mode, then contents is ignored. -func createEtcPasswd(ctx context.Context, vfsObj *vfs.VirtualFilesystem, creds *auth.Credentials, root vfs.VirtualDentry, contents string, mode linux.FileMode) error { +func createEtcFile(ctx context.Context, vfsObj *vfs.VirtualFilesystem, creds *auth.Credentials, root vfs.VirtualDentry, contents string, mode linux.FileMode, filePath string) error { pop := vfs.PathOperation{ Root: root, Start: root, @@ -40,14 +41,14 @@ func createEtcPasswd(ctx context.Context, vfsObj *vfs.VirtualFilesystem, creds * } if err := vfsObj.MkdirAt(ctx, creds, &pop, &vfs.MkdirOptions{ Mode: 0755, - }); err != nil { + }); err != nil && err != linuxerr.EEXIST { // Ignore error if it already exists. return fmt.Errorf("failed to create directory etc: %v", err) } pop = vfs.PathOperation{ Root: root, Start: root, - Path: fspath.Parse("etc/passwd"), + Path: fspath.Parse(filePath), } switch mode.FileType() { case 0: @@ -128,7 +129,7 @@ func TestGetExecUserHome(t *testing.T) { root := mns.Root(ctx) defer root.DecRef(ctx) - if err := createEtcPasswd(ctx, &vfsObj, creds, root, tc.passwdContents, tc.passwdMode); err != nil { + if err := createEtcFile(ctx, &vfsObj, creds, root, tc.passwdContents, tc.passwdMode, "/etc/passwd"); err != nil { t.Fatalf("createEtcPasswd failed: %v", err) } @@ -215,102 +216,187 @@ func TestGetExecUIDGIDFromUser(t *testing.T) { tests := map[string]struct { user string passwdContents string - passwdMode linux.FileMode + groupContents string + fileMode linux.FileMode expectedUID auth.KUID expectedGID auth.KGID }{ - "success": { + "user0": { user: "user0", passwdContents: "user0::1000:1111:&:/home/user0:/bin/sh", - passwdMode: linux.S_IFREG | 0666, + groupContents: "", + fileMode: linux.S_IFREG | 0666, expectedUID: 1000, expectedGID: 1111, }, - "success_with_uid_only": { + "uid_only": { user: "1000", passwdContents: "user0::1000:1111:&:/home/user0:/bin/sh", - passwdMode: linux.S_IFREG | 0666, + groupContents: "", + fileMode: linux.S_IFREG | 0666, expectedUID: 1000, expectedGID: 1111, }, - "success_with_uid_and_gid": { + "uid_and_gid": { user: "1000:1111", passwdContents: "user0::1000:1111:&:/home/user0:/bin/sh", - passwdMode: linux.S_IFREG | 0666, + groupContents: "", + fileMode: linux.S_IFREG | 0666, expectedUID: 1000, expectedGID: 1111, }, - "success_with_uid_and_wrong_gid": { + "uid_and_wrong_gid": { user: "1000:1112", passwdContents: "user0::1000:1111:&:/home/user0:/bin/sh", - passwdMode: linux.S_IFREG | 0666, + groupContents: "", + fileMode: linux.S_IFREG | 0666, expectedUID: 1000, - expectedGID: 1111, + expectedGID: 1112, }, - "no_user": { + "user_missing": { user: "user1", passwdContents: "user0::1000:1111::/home/user0:/bin/sh", - passwdMode: linux.S_IFREG | 0666, - expectedUID: 65534, - expectedGID: 65534, + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 0, + expectedGID: 0, }, "multiple_user_no_match": { user: "user1", passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", - passwdMode: linux.S_IFREG | 0666, - expectedUID: 65534, - expectedGID: 65534, + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 0, + expectedGID: 0, }, "multiple_user_many_match": { user: "user1", passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser1::1002:1112::/home/user1:/bin/sh\nuser1::1003:1113::/home/user1:/bin/sh", - passwdMode: linux.S_IFREG | 0666, - expectedUID: 65534, - expectedGID: 65534, + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 1002, + expectedGID: 1112, }, "invalid_file": { user: "user1", passwdContents: "user0:1000:1111::/home/user0:/bin/sh\nuser1::1001:1111::/home/user1:/bin/sh\nuser2::/home/user2:/bin/sh", - passwdMode: linux.S_IFREG | 0666, - expectedUID: 65534, - expectedGID: 65534, + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 0, + expectedGID: 0, }, "empty_file": { user: "user1", passwdContents: "", - passwdMode: linux.S_IFREG | 0666, - expectedUID: 65534, - expectedGID: 65534, + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 0, + expectedGID: 0, }, "empty_user": { user: "", passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", - passwdMode: linux.S_IFREG | 0666, - expectedUID: 65534, - expectedGID: 65534, + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 0, + expectedGID: 0, }, - "success_with_comments": { + "with_comments": { user: "user0", passwdContents: "#This is a comment\nuser0::1000:1111:&:/home/user0:/bin/sh\nuser2::1002:1112:&:/home/user2:/bin/sh\nuser3:&:1003:1113::/home/user3:/bin/sh", - passwdMode: linux.S_IFREG | 0666, + groupContents: "", + fileMode: linux.S_IFREG | 0666, expectedUID: 1000, expectedGID: 1111, }, - "success_with_comments_mid_file": { + "with_comments_mid_file": { user: "user0", passwdContents: "user0::1000:1111:&:/home/user0:/bin/sh\nuser2::1002:1112:&:/home/user2:/bin/sh\n#This is a comment\n\nuser3:&:1003:1113::/home/user3:/bin/sh", - passwdMode: linux.S_IFREG | 0666, + groupContents: "", + fileMode: linux.S_IFREG | 0666, expectedUID: 1000, expectedGID: 1111, }, - "success_empty_gecos": { + "empty_gecos": { user: "user0", passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", - passwdMode: linux.S_IFREG | 0666, + groupContents: "", + fileMode: linux.S_IFREG | 0666, expectedUID: 1000, expectedGID: 1111, }, - } + "numeric_user_empty_file": { + user: "1000", + passwdContents: "", + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 1000, + expectedGID: 0, + }, + "numeric_user_does_not_match": { + user: "1005", + passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 1005, + expectedGID: 0, + }, + "numeric_user_numeric_group_does_not_match": { + user: "1005:1006", + passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 1005, + expectedGID: 1006, + }, + "numeric_user_string_group_does_not_match": { + user: "1005:group10", + passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 1005, + expectedGID: 0, + }, + "string_user_numeric_group_does_not_match": { + user: "user10:1006", + passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 0, + expectedGID: 1006, + }, + "string_user_string_group_does_not_match": { + user: "user10:group10", + passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", + groupContents: "", + fileMode: linux.S_IFREG | 0666, + expectedUID: 0, + expectedGID: 0, + }, + "group_does_not_match_but_exist": { + user: "user0:group2", + passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", + groupContents: "group1:x:1110:user0,user1,user2\ngroup2:x:1112:user0,user1,user2", + fileMode: linux.S_IFREG | 0666, + expectedUID: 1000, + expectedGID: 1112, + }, + "group_does_not_match_and_does_not_exist": { + user: "user0:group10", + passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", + groupContents: "group1:x:1110:user0,user1,user2\ngroup2:x:1112:user0,user1,user2", + fileMode: linux.S_IFREG | 0666, + expectedUID: 1000, + expectedGID: 0, + }, + "numeric_group_does_not_match_and_does_not_exist": { + user: "user0:1113", + passwdContents: "user0::1000:1111::/home/user0:/bin/sh\nuser2::1002:1112::/home/user2:/bin/sh\nuser3::1003:1113::/home/user3:/bin/sh", + groupContents: "group1:x:1110:user0,user1,user2\ngroup2:x:1112:user0,user1,user2", + fileMode: linux.S_IFREG | 0666, + expectedUID: 1000, + expectedGID: 1113, + }} for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -333,20 +419,14 @@ func TestGetExecUIDGIDFromUser(t *testing.T) { root := mns.Root(ctx) defer root.DecRef(ctx) - if err := createEtcPasswd(ctx, &vfsObj, creds, root, tc.passwdContents, tc.passwdMode); err != nil { - t.Fatalf("createEtcPasswd failed: %v", err) + if err := createEtcFile(ctx, &vfsObj, creds, root, tc.passwdContents, tc.fileMode, "etc/passwd"); err != nil { + t.Fatalf("createEtcFile failed: %v", err) + } + if err := createEtcFile(ctx, &vfsObj, creds, root, tc.groupContents, tc.fileMode, "/etc/group"); err != nil { + t.Fatalf("createEtcFile failed: %v", err) } - gotUID, gotGID, err := GetExecUIDGIDFromUser(ctx, mns, tc.user) - if strings.HasPrefix(name, "success") { - if err != nil { - t.Fatalf("failed to get UID and GID from user: %v %v", tc.user, err) - } - } else { - if err == nil { - t.Fatalf("retrieved UID and GID when user %v is not in /etc/passwd: %v", tc.user, err) - } - } + gotUID, gotGID := GetExecUIDGIDFromUser(ctx, mns, tc.user) if gotUID != tc.expectedUID { t.Fatalf("expectedUID %v, gotUID: %v", tc.expectedUID, gotUID) }