Merge pull request #1715 from baude/getusergroup

get user and group information using securejoin and runc's user library

3 files changed, 45 insertions, 63 deletions

diff --git a/libpod/container_api.go b/libpod/container_api.go
index e522038a3..30c67eb2a 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -10,8 +10,8 @@ import (
 	"time"
 
 	"github.com/containers/libpod/libpod/driver"
-	"github.com/containers/libpod/pkg/chrootuser"
 	"github.com/containers/libpod/pkg/inspect"
+	"github.com/containers/libpod/pkg/lookup"
 	"github.com/containers/storage/pkg/stringid"
 	"github.com/docker/docker/daemon/caps"
 	"github.com/pkg/errors"
@@ -292,13 +292,13 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e
 	// the host
 	hostUser := ""
 	if user != "" {
-		uid, gid, err := chrootuser.GetUser(c.state.Mountpoint, user)
+		execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, user, nil)
 		if err != nil {
-			return errors.Wrapf(err, "error getting user to launch exec session as")
+			return err
 		}
 
 		// runc expects user formatted as uid:gid
-		hostUser = fmt.Sprintf("%d:%d", uid, gid)
+		hostUser = fmt.Sprintf("%d:%d", execUser.Uid, execUser.Gid)
 	}
 
 	// Generate exec session ID
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 2af216358..d928c4aed 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"github.com/opencontainers/runc/libcontainer/user"
 	"io"
 	"io/ioutil"
 	"os"
@@ -14,9 +15,9 @@ import (
 	"syscall"
 
 	"github.com/containers/buildah/imagebuildah"
-	"github.com/containers/libpod/pkg/chrootuser"
 	"github.com/containers/libpod/pkg/hooks"
 	"github.com/containers/libpod/pkg/hooks/exec"
+	"github.com/containers/libpod/pkg/lookup"
 	"github.com/containers/libpod/pkg/resolvconf"
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/libpod/pkg/secrets"
@@ -1029,7 +1030,8 @@ func (c *Container) writeStringToRundir(destFile, output string) (string, error)
 func (c *Container) generatePasswd() (string, error) {
 	var (
 		groupspec string
-		gid       uint32
+		group     *user.Group
+		gid       int
 	)
 	if c.config.User == "" {
 		return "", nil
@@ -1044,21 +1046,27 @@ func (c *Container) generatePasswd() (string, error) {
 	if err != nil {
 		return "", nil
 	}
-	// if UID exists inside of container rootfs /etc/passwd then
-	// don't generate passwd
-	if _, _, err := chrootuser.LookupUIDInContainer(c.state.Mountpoint, uid); err == nil {
+	// Lookup the user to see if it exists in the container image
+	_, err = lookup.GetUser(c.state.Mountpoint, userspec)
+	if err != nil && err != user.ErrNoPasswdEntries {
+		return "", err
+	}
+	if err == nil {
 		return "", nil
 	}
-	if err == nil && groupspec != "" {
+	if groupspec != "" {
 		if !c.state.Mounted {
 			return "", errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate group field for passwd record", c.ID())
 		}
-		gid, err = chrootuser.GetGroup(c.state.Mountpoint, groupspec)
+		group, err = lookup.GetGroup(c.state.Mountpoint, groupspec)
 		if err != nil {
-			return "", errors.Wrapf(err, "unable to get gid from %s formporary passwd file")
+			if err == user.ErrNoGroupEntries {
+				return "", errors.Wrapf(err, "unable to get gid %s from group file", groupspec)
+			}
+			return "", err
 		}
+		gid = group.Gid
 	}
-
 	originPasswdFile := filepath.Join(c.state.Mountpoint, "/etc/passwd")
 	orig, err := ioutil.ReadFile(originPasswdFile)
 	if err != nil {
@@ -1153,6 +1161,7 @@ func (c *Container) generateHosts() (string, error) {
 }
 
 func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator) error {
+	var uid, gid int
 	mountPoint := c.state.Mountpoint
 	if !c.state.Mounted {
 		return errors.Wrapf(ErrInternal, "container is not mounted")
@@ -1176,6 +1185,18 @@ func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator)
 		}
 	}
 
+	if c.config.User != "" {
+		if !c.state.Mounted {
+			return errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate User field", c.ID())
+		}
+		execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, c.config.User, nil)
+		if err != nil {
+			return err
+		}
+		uid = execUser.Uid
+		gid = execUser.Gid
+	}
+
 	for k := range imageData.ContainerConfig.Volumes {
 		mount := spec.Mount{
 			Destination: k,
@@ -1186,19 +1207,6 @@ func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator)
 			continue
 		}
 		volumePath := filepath.Join(c.config.StaticDir, "volumes", k)
-		var (
-			uid uint32
-			gid uint32
-		)
-		if c.config.User != "" {
-			if !c.state.Mounted {
-				return errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate User field", c.ID())
-			}
-			uid, gid, err = chrootuser.GetUser(c.state.Mountpoint, c.config.User)
-			if err != nil {
-				return err
-			}
-		}
 
 		// Ensure the symlinks are resolved
 		resolvedSymlink, err := imagebuildah.ResolveSymLink(mountPoint, k)
@@ -1218,7 +1226,7 @@ func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator)
 				return errors.Wrapf(err, "error creating directory %q for volume %q in container %q", volumePath, k, c.ID())
 			}
 
-			if err = os.Chown(srcPath, int(uid), int(gid)); err != nil {
+			if err = os.Chown(srcPath, uid, gid); err != nil {
 				return errors.Wrapf(err, "error chowning directory %q for volume %q in container %q", srcPath, k, c.ID())
 			}
 		}
@@ -1228,7 +1236,7 @@ func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator)
 				return errors.Wrapf(err, "error creating directory %q for volume %q in container %q", volumePath, k, c.ID())
 			}
 
-			if err = os.Chown(volumePath, int(uid), int(gid)); err != nil {
+			if err = os.Chown(volumePath, uid, gid); err != nil {
 				return errors.Wrapf(err, "error chowning directory %q for volume %q in container %q", volumePath, k, c.ID())
 			}
 
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 0a1784ba7..7bf2c71ca 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -19,12 +19,10 @@ import (
 	cnitypes "github.com/containernetworking/cni/pkg/types/current"
 	"github.com/containernetworking/plugins/pkg/ns"
 	crioAnnotations "github.com/containers/libpod/pkg/annotations"
-	"github.com/containers/libpod/pkg/chrootuser"
 	"github.com/containers/libpod/pkg/criu"
+	"github.com/containers/libpod/pkg/lookup"
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/storage/pkg/idtools"
-	"github.com/cyphar/filepath-securejoin"
-	"github.com/opencontainers/runc/libcontainer/user"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
 	"github.com/opencontainers/selinux/go-selinux/label"
@@ -135,6 +133,10 @@ func (c *Container) cleanupNetwork() error {
 // Generate spec for a container
 // Accepts a map of the container's dependencies
 func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
+	execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, c.config.User, nil)
+	if err != nil {
+		return nil, err
+	}
 	g := generate.NewFromSpec(c.config.Spec)
 
 	// If network namespace was requested, add it now
@@ -188,7 +190,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 		}
 	}
 
-	var err error
 	if !rootless.IsRootless() {
 		if c.state.ExtensionStageHooks, err = c.setupOCIHooks(ctx, g.Config); err != nil {
 			return nil, errors.Wrapf(err, "error setting up OCI Hooks")
@@ -206,13 +207,9 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 		if !c.state.Mounted {
 			return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate User field", c.ID())
 		}
-		uid, gid, err := chrootuser.GetUser(c.state.Mountpoint, c.config.User)
-		if err != nil {
-			return nil, err
-		}
 		// User and Group must go together
-		g.SetProcessUID(uid)
-		g.SetProcessGID(gid)
+		g.SetProcessUID(uint32(execUser.Uid))
+		g.SetProcessGID(uint32(execUser.Gid))
 	}
 
 	// Add addition groups if c.config.GroupAdd is not empty
@@ -220,11 +217,8 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 		if !c.state.Mounted {
 			return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to add additional groups", c.ID())
 		}
-		for _, group := range c.config.Groups {
-			gid, err := chrootuser.GetGroup(c.state.Mountpoint, group)
-			if err != nil {
-				return nil, err
-			}
+		gids, _ := lookup.GetContainerGroups(c.config.Groups, c.state.Mountpoint, nil)
+		for _, gid := range gids {
 			g.AddProcessAdditionalGid(gid)
 		}
 	}
@@ -237,26 +231,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 
 	// Look up and add groups the user belongs to, if a group wasn't directly specified
 	if !rootless.IsRootless() && !strings.Contains(c.config.User, ":") {
-		var groupDest, passwdDest string
-		defaultExecUser := user.ExecUser{
-			Uid:  0,
-			Gid:  0,
-			Home: "/",
-		}
-
-		// Make sure the /etc/group  and /etc/passwd destinations are not a symlink to something naughty
-		if groupDest, err = securejoin.SecureJoin(c.state.Mountpoint, "/etc/group"); err != nil {
-			logrus.Debug(err)
-			return nil, err
-		}
-		if passwdDest, err = securejoin.SecureJoin(c.state.Mountpoint, "/etc/passwd"); err != nil {
-			logrus.Debug(err)
-			return nil, err
-		}
-		execUser, err := user.GetExecUserPath(c.config.User, &defaultExecUser, passwdDest, groupDest)
-		if err != nil {
-			return nil, err
-		}
 		for _, gid := range execUser.Sgids {
 			g.AddProcessAdditionalGid(uint32(gid))
 		}