From 650f95cb06cb5a4e979fbe9f9fcd125a229e4e09 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 4 Dec 2018 11:46:17 -0800 Subject: libpod/container_internal_linux: Allow gids that aren't in the group file When an image config sets config.User [1] to a numeric group (like 1000:1000), but those values do not exist in the container's /etc/group, libpod is currently breaking: $ podman run --rm registry.svc.ci.openshift.org/ci-op-zvml7cd6/pipeline:installer --help error creating temporary passwd file for container 228f6e9943d6f18b93c19644e9b619ec4d459a3e0eb31680e064eeedf6473678: unable to get gid 1000 from group file: no matching entries in group file However, the OCI spec requires converters to copy numeric uid and gid to the runtime config verbatim [2]. With this commit, I'm frontloading the "is groupspec an integer?" check and only bothering with lookup.GetGroup when it was not. I've also removed a few .Mounted checks, which are originally from 00d38cb3 (podman create/run need to load information from the image, 2017-12-18, #110). We don't need a mounted container filesystem to translate integers. And when the lookup code needs to fall back to the mounted root to translate names, it can handle erroring out internally (and looking it over, it seems to do that already). [1]: https://github.com/opencontainers/image-spec/blame/v1.0.1/config.md#L118-L123 [2]: https://github.com/opencontainers/image-spec/blame/v1.0.1/conversion.md#L70 Signed-off-by: W. Trevor King --- libpod/container_internal_linux.go | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 8861d7728..b72eefdf1 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -238,9 +238,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { } if c.config.User != "" { - if !c.state.Mounted { - return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate User field", c.ID()) - } // User and Group must go together g.SetProcessUID(uint32(execUser.Uid)) g.SetProcessGID(uint32(execUser.Gid)) @@ -248,9 +245,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { // Add addition groups if c.config.GroupAdd is not empty if len(c.config.Groups) > 0 { - if !c.state.Mounted { - return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to add additional groups", c.ID()) - } gids, _ := lookup.GetContainerGroups(c.config.Groups, c.state.Mountpoint, nil) for _, gid := range gids { g.AddProcessAdditionalGid(gid) @@ -802,7 +796,6 @@ func (c *Container) generateHosts() (string, error) { func (c *Container) generatePasswd() (string, error) { var ( groupspec string - group *user.Group gid int ) if c.config.User == "" { @@ -827,17 +820,16 @@ func (c *Container) generatePasswd() (string, error) { return "", nil } 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()) - } - group, err = lookup.GetGroup(c.state.Mountpoint, groupspec) - if err != nil { - if err == user.ErrNoGroupEntries { + ugid, err := strconv.ParseUint(groupspec, 10, 32) + if err == nil { + gid = int(ugid) + } else { + group, err := lookup.GetGroup(c.state.Mountpoint, groupspec) + if err != nil { return "", errors.Wrapf(err, "unable to get gid %s from group file", groupspec) } - return "", err + gid = group.Gid } - gid = group.Gid } originPasswdFile := filepath.Join(c.state.Mountpoint, "/etc/passwd") orig, err := ioutil.ReadFile(originPasswdFile) -- cgit v1.2.3-54-g00ecf From 39df2093e89a2816f317b72a73106166031045e6 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 4 Dec 2018 12:50:59 -0800 Subject: pkg/lookup: Return ID-only pointers on ErrNo*Entries Callers that only care about the IDs should try to convert the identifier to an integer before calling the Get* functions, so they can save the cost of hitting the filesystem and maybe or maybe not finding the other fields (User.Name, etc.). But callers that *want* the other fields but only actually need the ID can, with this commit, just call the Get* function and ignore ErrNo*Entries responses: user, err := lookup.GetUser(mount, userIDorName) if err != nil && err != ErrNoPasswdEntries { return err } Previously, they'd have to perform their own integer-conversion attempt in Get* error handling, with logic like: user, err := lookup.GetUser(mount, userIDorName) if err == ErrNoPasswdEntries { uuid, err := strconv.ParseUint(userIDorName, 10, 32) if err == nil { user.Uid = int(uuid) } } else if err != nil { return err } Signed-off-by: W. Trevor King --- pkg/lookup/lookup.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/pkg/lookup/lookup.go b/pkg/lookup/lookup.go index a9d975b4b..70b97144f 100644 --- a/pkg/lookup/lookup.go +++ b/pkg/lookup/lookup.go @@ -99,9 +99,11 @@ func GetContainerGroups(groups []string, containerMount string, override *Overri return uintgids, nil } -// GetUser takes a containermount path and user name or id and returns +// GetUser takes a containermount path and user name or ID and returns // a matching User structure from /etc/passwd. If it cannot locate a user // with the provided information, an ErrNoPasswdEntries is returned. +// When the provided user name was an ID, a User structure with Uid +// set is returned along with ErrNoPasswdEntries. func GetUser(containerMount, userIDorName string) (*user.User, error) { var inputIsName bool uid, err := strconv.Atoi(userIDorName) @@ -124,12 +126,17 @@ func GetUser(containerMount, userIDorName string) (*user.User, error) { if len(users) > 0 { return &users[0], nil } + if !inputIsName { + return &user.User{Uid: uid}, user.ErrNoPasswdEntries + } return nil, user.ErrNoPasswdEntries } -// GetGroup takes ac ontainermount path and a group name or id and returns -// a match Group struct from /etc/group. if it cannot locate a group, -// an ErrNoGroupEntries error is returned. +// GetGroup takes a containermount path and a group name or ID and returns +// a match Group struct from /etc/group. If it cannot locate a group, +// an ErrNoGroupEntries error is returned. When the provided group name +// was an ID, a Group structure with Gid set is returned along with +// ErrNoGroupEntries. func GetGroup(containerMount, groupIDorName string) (*user.Group, error) { var inputIsName bool gid, err := strconv.Atoi(groupIDorName) @@ -154,5 +161,8 @@ func GetGroup(containerMount, groupIDorName string) (*user.Group, error) { if len(groups) > 0 { return &groups[0], nil } + if !inputIsName { + return &user.Group{Gid: gid}, user.ErrNoGroupEntries + } return nil, user.ErrNoGroupEntries } -- cgit v1.2.3-54-g00ecf