chrootuser: default to GID 0 when given a numeric --user

When we're given a numeric --user value, default to GID 0 if the numeric
ID doesn't correspond to a user entry in /etc/passwd that can provide us
with the user's primary group ID.

Make sure that GetAdditionalGroupsForUser() returns wrapped errors.

Also test various user:group forms.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

Closes: #728
Approved by: mheon

4 files changed, 45 insertions, 14 deletions

diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 5168e987b..b1420aa55 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -1119,13 +1119,15 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 		}
 	}
 
-	// Look up and add groups the user belongs to
-	groups, err := chrootuser.GetAdditionalGroupsForUser(c.state.Mountpoint, uint64(g.Spec().Process.User.UID))
-	if err != nil && err != chrootuser.ErrNoSuchUser {
-		return nil, err
-	}
-	for _, gid := range groups {
-		g.AddProcessAdditionalGid(gid)
+	// Look up and add groups the user belongs to, if a group wasn't directly specified
+	if !strings.Contains(c.config.User, ":") {
+		groups, err := chrootuser.GetAdditionalGroupsForUser(c.state.Mountpoint, uint64(g.Spec().Process.User.UID))
+		if err != nil && errors.Cause(err) != chrootuser.ErrNoSuchUser {
+			return nil, err
+		}
+		for _, gid := range groups {
+			g.AddProcessAdditionalGid(gid)
+		}
 	}
 
 	// Add shared namespaces from other containers
diff --git a/pkg/chrootuser/user.go b/pkg/chrootuser/user.go
index 54917b843..1fbb5566e 100644
--- a/pkg/chrootuser/user.go
+++ b/pkg/chrootuser/user.go
@@ -11,7 +11,7 @@ import (
 // GetUser will return the uid, gid of the user specified in the userspec
 // it will use the /etc/passwd and /etc/group files inside of the rootdir
 // to return this information.
-// userspace format [user | user:group | uid | uid:gid | user:gid | uid:group ]
+// userspec format [user | user:group | uid | uid:gid | user:gid | uid:group ]
 func GetUser(rootdir, userspec string) (uint32, uint32, error) {
 	var gid64 uint64
 	var gerr error = user.UnknownGroupError("error looking up group")
@@ -37,8 +37,8 @@ func GetUser(rootdir, userspec string) (uint32, uint32, error) {
 			userspec = name
 		} else {
 			// Leave userspec alone, but swallow the error and just
-			// use GID == UID.
-			gid64 = uid64
+			// use GID 0.
+			gid64 = 0
 			gerr = nil
 		}
 	}
@@ -70,7 +70,7 @@ func GetUser(rootdir, userspec string) (uint32, uint32, error) {
 	return 0, 0, err
 }
 
-// GetGroup returns the gid by looking it up in the /etc/passwd file
+// GetGroup returns the gid by looking it up in the /etc/group file
 // groupspec format [ group | gid ]
 func GetGroup(rootdir, groupspec string) (uint32, error) {
 	gid64, gerr := strconv.ParseUint(groupspec, 10, 32)
@@ -87,5 +87,9 @@ func GetGroup(rootdir, groupspec string) (uint32, error) {
 
 // GetAdditionalGroupsForUser returns a list of gids that userid is associated with
 func GetAdditionalGroupsForUser(rootdir string, userid uint64) ([]uint32, error) {
-	return lookupAdditionalGroupsForUIDInContainer(rootdir, userid)
+	gids, err := lookupAdditionalGroupsForUIDInContainer(rootdir, userid)
+	if err != nil {
+		return nil, errors.Wrapf(err, "error looking up supplemental groups for uid %d", userid)
+	}
+	return gids, nil
 }
diff --git a/pkg/chrootuser/user_basic.go b/pkg/chrootuser/user_basic.go
index 4f89af557..4ed7918e9 100644
--- a/pkg/chrootuser/user_basic.go
+++ b/pkg/chrootuser/user_basic.go
@@ -17,3 +17,7 @@ func lookupGroupInContainer(rootdir, groupname string) (uint64, error) {
 func lookupGroupForUIDInContainer(rootdir string, userid uint64) (string, uint64, error) {
 	return "", 0, errors.New("primary group lookup by uid not supported")
 }
+
+func lookupAdditionalGroupsForUIDInContainer(rootdir string, userid uint64) (gid []uint32, err error) {
+	return nil, errors.New("supplemental groups list lookup by uid not supported")
+}
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index 5eb21c433..cc8492958 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -334,11 +334,18 @@ var _ = Describe("Podman run", func() {
 		Expect(session.OutputToString()).To(Equal("uid=0(root) gid=0(root) groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),20(dialout),26(tape),27(video)"))
 	})
 
-	It("podman run with user (integer)", func() {
+	It("podman run with user (integer, not in /etc/passwd)", func() {
 		session := podmanTest.Podman([]string{"run", "--rm", "--user=1234", ALPINE, "id"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
-		Expect(session.OutputToString()).To(Equal("uid=1234 gid=1234"))
+		Expect(session.OutputToString()).To(Equal("uid=1234 gid=0(root)"))
+	})
+
+	It("podman run with user (integer, in /etc/passwd)", func() {
+		session := podmanTest.Podman([]string{"run", "--rm", "--user=8", ALPINE, "id"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("uid=8(mail) gid=12(mail) groups=12(mail)"))
 	})
 
 	It("podman run with user (username)", func() {
@@ -348,6 +355,20 @@ var _ = Describe("Podman run", func() {
 		Expect(session.OutputToString()).To(Equal("uid=8(mail) gid=12(mail) groups=12(mail)"))
 	})
 
+	It("podman run with user:group (username:integer)", func() {
+		session := podmanTest.Podman([]string{"run", "--rm", "--user=mail:21", ALPINE, "id"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("uid=8(mail) gid=21(ftp)"))
+	})
+
+	It("podman run with user:group (integer:groupname)", func() {
+		session := podmanTest.Podman([]string{"run", "--rm", "--user=8:ftp", ALPINE, "id"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("uid=8(mail) gid=21(ftp)"))
+	})
+
 	It("podman run with attach stdin outputs container ID", func() {
 		session := podmanTest.Podman([]string{"run", "--attach", "stdin", ALPINE, "printenv"})
 		session.WaitWithDefaultTimeout()