JohnHammond
/
buildkit
mirror of https://github.com/JohnHammond/buildkit.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #473 from AntaresS/fix-oci-groups 

add missing supplementary group IDs
docker-18.09
Tõnis Tiigi 2018-06-29 19:03:19 -07:00 committed by GitHub
parent f3b743bd13 7f64188f17
commit ccd381fb74
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 101 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
executor/containerdexecutor/executor.go
View File

@ -55,14 +55,15 @@ func (w containerdExecutor) Exec(ctx context.Context, meta executor.Meta, root c
}
defer mountable.Release()
uid, gid, err := oci.ParseUser(meta.User)
var sgids []uint32
uid, gid, err := oci.ParseUIDGID(meta.User)
if err != nil {
lm := snapshot.LocalMounterWithMounts(rootMounts)
rootfsPath, err := lm.Mount()
if err != nil {
return err
}
uid, gid, err = oci.GetUser(ctx, rootfsPath, meta.User)
uid, gid, sgids, err = oci.GetUser(ctx, rootfsPath, meta.User)
if err != nil {
lm.Unmount()
return err
@ -70,7 +71,7 @@ func (w containerdExecutor) Exec(ctx context.Context, meta executor.Meta, root c
lm.Unmount()
}
opts := []containerdoci.SpecOpts{containerdoci.WithUIDGID(uid, gid)}
opts := []containerdoci.SpecOpts{oci.WithUIDGID(uid, gid, sgids)}
if meta.ReadonlyRootFS {
opts = append(opts, containerdoci.WithRootFSReadonly())
}

71
executor/oci/user.go
View File

@ -2,27 +2,31 @@ package oci
import (
"context"
"errors"
"os"
"strconv"
"strings"
"github.com/containerd/containerd/containers"
containerdoci "github.com/containerd/containerd/oci"
"github.com/containerd/continuity/fs"
"github.com/opencontainers/runc/libcontainer/user"
"github.com/opencontainers/runtime-spec/specs-go"
)
func GetUser(ctx context.Context, root, username string) (uint32, uint32, error) {
func GetUser(ctx context.Context, root, username string) (uint32, uint32, []uint32, error) {
// fast path from uid/gid
if uid, gid, err := ParseUser(username); err == nil {
return uid, gid, nil
if uid, gid, err := ParseUIDGID(username); err == nil {
return uid, gid, nil, nil
}
passwdPath, err := user.GetPasswdPath()
if err != nil {
return 0, 0, err
return 0, 0, nil, err
}
groupPath, err := user.GetGroupPath()
if err != nil {
return 0, 0, err
return 0, 0, nil, err
}
passwdFile, err := openUserFile(root, passwdPath)
if err == nil {
@ -35,33 +39,29 @@ func GetUser(ctx context.Context, root, username string) (uint32, uint32, error)
execUser, err := user.GetExecUser(username, nil, passwdFile, groupFile)
if err != nil {
return 0, 0, err
return 0, 0, nil, err
}
return uint32(execUser.Uid), uint32(execUser.Gid), nil
var sgids []uint32
for _, g := range execUser.Sgids {
sgids = append(sgids, uint32(g))
}
return uint32(execUser.Uid), uint32(execUser.Gid), sgids, nil
}
func ParseUser(str string) (uid uint32, gid uint32, err error) {
// ParseUIDGID takes the fast path to parse UID and GID if and only if they are both provided
func ParseUIDGID(str string) (uid uint32, gid uint32, err error) {
if str == "" {
return 0, 0, nil
}
parts := strings.SplitN(str, ":", 2)
for i, v := range parts {
switch i {
case 0:
uid, err = parseUID(v)
if err != nil {
return 0, 0, err
}
if len(parts) == 1 {
gid = uid
}
case 1:
gid, err = parseUID(v)
if err != nil {
return 0, 0, err
}
}
if len(parts) == 1 {
return 0, 0, errors.New("groups ID is not provided")
}
if uid, err = parseUID(parts[0]); err != nil {
return 0, 0, err
}
if gid, err = parseUID(parts[1]); err != nil {
return 0, 0, err
}
return
}
@ -84,3 +84,24 @@ func parseUID(str string) (uint32, error) {
}
return uint32(uid), nil
}
// WithUIDGID allows the UID and GID for the Process to be set
// FIXME: This is a temporeray fix for the missing supplementary GIDs from containerd
// once the PR in containerd is merged we should remove this function.
func WithUIDGID(uid, gid uint32, sgids []uint32) containerdoci.SpecOpts {
return func(_ context.Context, _ containerdoci.Client, _ *containers.Container, s *containerdoci.Spec) error {
setProcess(s)
s.Process.User.UID = uid
s.Process.User.GID = gid
s.Process.User.AdditionalGids = sgids
return nil
}
}
// setProcess sets Process to empty if unset
// FIXME: Same on this one. Need to be removed after containerd fix merged
func setProcess(s *containerdoci.Spec) {
if s.Process == nil {
s.Process = &specs.Process{}
}
}

4
executor/runcexecutor/executor.go
View File

@ -133,7 +133,7 @@ func (w *runcExecutor) Exec(ctx context.Context, meta executor.Meta, root cache.
}
defer mount.Unmount(rootFSPath, 0)
uid, gid, err := oci.GetUser(ctx, rootFSPath, meta.User)
uid, gid, sgids, err := oci.GetUser(ctx, rootFSPath, meta.User)
if err != nil {
return err
}
@ -143,7 +143,7 @@ func (w *runcExecutor) Exec(ctx context.Context, meta executor.Meta, root cache.
return err
}
defer f.Close()
opts := []containerdoci.SpecOpts{containerdoci.WithUIDGID(uid, gid)}
opts := []containerdoci.SpecOpts{oci.WithUIDGID(uid, gid, sgids)}
if system.SeccompSupported() {
opts = append(opts, seccomp.WithDefaultProfile())
}

49
frontend/dockerfile/dockerfile_test.go
View File

@ -1121,6 +1121,55 @@ func testUser(t *testing.T, sb integration.Sandbox) {
FROM busybox AS base
RUN mkdir -m 0777 /out
RUN id -un > /out/rootuser
# Make sure our defaults work
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)" = '0:0/root:root' ]
# TODO decide if "args.user = strconv.Itoa(syscall.Getuid())" is acceptable behavior for changeUser in sysvinit instead of "return nil" when "USER" isn't specified (so that we get the proper group list even if that is the empty list, even in the default case of not supplying an explicit USER to run as, which implies USER 0)
USER root
RUN [ "$(id -G):$(id -Gn)" = '0 10:root wheel' ]
# Setup dockerio user and group
RUN echo 'dockerio:x:1001:1001::/bin:/bin/false' >> /etc/passwd && \
echo 'dockerio:x:1001:' >> /etc/group
# Make sure we can switch to our user and all the information is exactly as we expect it to be
USER dockerio
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001:dockerio' ]
# Switch back to root and double check that worked exactly as we might expect it to
USER root
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '0:0/root:root/0 10:root wheel' ] && \
# Add a "supplementary" group for our dockerio user
echo 'supplementary:x:1002:dockerio' >> /etc/group
# ... and then go verify that we get it like we expect
USER dockerio
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001 1002:dockerio supplementary' ]
USER 1001
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001 1002:dockerio supplementary' ]
# super test the new "user:group" syntax
USER dockerio:dockerio
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001:dockerio' ]
USER 1001:dockerio
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001:dockerio' ]
USER dockerio:1001
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001:dockerio' ]
USER 1001:1001
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001:dockerio' ]
USER dockerio:supplementary
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1002/dockerio:supplementary/1002:supplementary' ]
USER dockerio:1002
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1002/dockerio:supplementary/1002:supplementary' ]
USER 1001:supplementary
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1002/dockerio:supplementary/1002:supplementary' ]
USER 1001:1002
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1002/dockerio:supplementary/1002:supplementary' ]
# make sure unknown uid/gid still works properly
USER 1042:1043
RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1042:1043/1042:1043/1043:1043' ]
USER daemon
RUN id -un > /out/daemonuser
FROM scratch