From 6246942d377bd9ed665a4ac448120352454dd83d Mon Sep 17 00:00:00 2001 From: baude Date: Wed, 24 Oct 2018 10:39:12 -0500 Subject: Increase security and performance when looking up groups We implement the securejoin method to make sure the paths to /etc/passwd and /etc/group are not symlinks to something naughty or outside the container image. And then instead of actually chrooting, we use the runc functions to get information about a user. The net result is increased security and a a performance gain from 41ms to 100us. Signed-off-by: baude --- libpod/container_internal_linux.go | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index b25645e5c..5a6b72580 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -21,6 +21,8 @@ import ( "github.com/containers/libpod/pkg/criu" "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" @@ -197,12 +199,28 @@ 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, ":") { - groups, err := chrootuser.GetAdditionalGroupsForUser(c.state.Mountpoint, uint64(g.Config.Process.User.UID)) - if err != nil && errors.Cause(err) != chrootuser.ErrNoSuchUser { + 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 } - for _, gid := range groups { - g.AddProcessAdditionalGid(gid) + 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)) } } -- cgit v1.2.3-54-g00ecf