Move UserLookup functionality into a separate pkg/user submodule that implements proper parsing of /etc/passwd and /etc/group, and use that to add support for "docker run -u user:group" and for getting supplementary groups (if ":group" is not specified)

Docker-DCO-1.1-Signed-off-by: Andrew Page <admwiggin@gmail.com> (github: tianon)

execdriver/lxc/init.go

@@ -4,11 +4,10 @@ import (
 	"fmt"
 	"github.com/dotcloud/docker/execdriver"
 	"github.com/dotcloud/docker/pkg/netlink"
-	"github.com/dotcloud/docker/utils"
+	"github.com/dotcloud/docker/pkg/user"
 	"github.com/syndtr/gocapability/capability"
 	"net"
 	"os"
-	"strconv"
 	"strings"
 	"syscall"
 )
@@ -79,28 +78,22 @@ func setupWorkingDirectory(args *execdriver.InitArgs) error {
 
 // Takes care of dropping privileges to the desired user
 func changeUser(args *execdriver.InitArgs) error {
-	if args.User == "" {
+	uid, gid, suppGids, err := user.GetUserGroupSupplementary(
-		return nil
+		args.User,
-	}
+		syscall.Getuid(), syscall.Getgid(),
-	userent, err := utils.UserLookup(args.User)
+	)
 	if err != nil {
-		return fmt.Errorf("Unable to find user %v: %v", args.User, err)
+		return err
 	}
 
-	uid, err := strconv.Atoi(userent.Uid)
+	if err := syscall.Setgroups(suppGids); err != nil {
-	if err != nil {
+		return fmt.Errorf("Setgroups failed: %v", err)
-		return fmt.Errorf("Invalid uid: %v", userent.Uid)
 	}
-	gid, err := strconv.Atoi(userent.Gid)
-	if err != nil {
-		return fmt.Errorf("Invalid gid: %v", userent.Gid)
-	}
-
 	if err := syscall.Setgid(gid); err != nil {
-		return fmt.Errorf("setgid failed: %v", err)
+		return fmt.Errorf("Setgid failed: %v", err)
 	}
 	if err := syscall.Setuid(uid); err != nil {
-		return fmt.Errorf("setuid failed: %v", err)
+		return fmt.Errorf("Setuid failed: %v", err)
 	}
 
 	return nil

integration/buildfile_test.go

@@ -148,6 +148,86 @@ RUN [ "$(/hello.sh)" = "hello world" ]
 		nil,
 	},
 
+	// Users and groups
+	{
+		`
+FROM {IMAGE}
+
+# Make sure our defaults work
+RUN [ "$(id -u):$(id -g)" = '0:0' ]
+RUN [ "$(id -un):$(id -gn)" = '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 -- root' ]
+
+# Setup dockerio user and group
+RUN echo 'dockerio:x:1000:1000::/bin:/bin/false' >> /etc/passwd
+RUN echo 'dockerio:x:1000:' >> /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)" = '1000:1000' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:dockerio' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1000 -- dockerio' ]
+
+# Switch back to root and double check that worked exactly as we might expect it to
+USER root
+RUN [ "$(id -u):$(id -g)" = '0:0' ]
+RUN [ "$(id -un):$(id -gn)" = 'root:root' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '0 -- root' ]
+
+# Add a "supplementary" group for our dockerio user
+RUN echo 'supplementary:x:1001:dockerio' >> /etc/group
+
+# ... and then go verify that we get it like we expect
+USER dockerio
+RUN [ "$(id -u):$(id -g)" = '1000:1000' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:dockerio' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1000 1001 -- dockerio supplementary' ]
+USER 1000
+RUN [ "$(id -u):$(id -g)" = '1000:1000' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:dockerio' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1000 1001 -- dockerio supplementary' ]
+
+# and finally, super test the new "user:group" syntax
+USER dockerio:dockerio
+RUN [ "$(id -u):$(id -g)" = '1000:1000' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:dockerio' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1000 -- dockerio' ]
+USER 1000:dockerio
+RUN [ "$(id -u):$(id -g)" = '1000:1000' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:dockerio' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1000 -- dockerio' ]
+USER dockerio:1000
+RUN [ "$(id -u):$(id -g)" = '1000:1000' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:dockerio' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1000 -- dockerio' ]
+USER 1000:1000
+RUN [ "$(id -u):$(id -g)" = '1000:1000' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:dockerio' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1000 -- dockerio' ]
+USER dockerio:supplementary
+RUN [ "$(id -u):$(id -g)" = '1000:1001' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:supplementary' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1001 -- supplementary' ]
+USER dockerio:1001
+RUN [ "$(id -u):$(id -g)" = '1000:1001' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:supplementary' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1001 -- supplementary' ]
+USER 1000:supplementary
+RUN [ "$(id -u):$(id -g)" = '1000:1001' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:supplementary' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1001 -- supplementary' ]
+USER 1000:1001
+RUN [ "$(id -u):$(id -g)" = '1000:1001' ]
+RUN [ "$(id -un):$(id -gn)" = 'dockerio:supplementary' ]
+RUN [ "$(id -G) -- $(id -Gn)" = '1001 -- supplementary' ]
+`,
+		nil,
+		nil,
+	},
+
 	// Environment variable
 	{
 		`

pkg/user/MAINTAINERS

@@ -0,0 +1 @@
+Tianon Gravi <admwiggin@gmail.com> (@tianon)

pkg/user/user.go

@@ -0,0 +1,245 @@
+package user
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"os"
+	"reflect"
+	"strconv"
+	"strings"
+)
+
+type User struct {
+	Name  string
+	Pass  string
+	Uid   int
+	Gid   int
+	Gecos string
+	Home  string
+	Shell string
+}
+
+type Group struct {
+	Name string
+	Pass string
+	Gid  int
+	List []string
+}
+
+func parseLine(line string, v ...interface{}) {
+	if line == "" {
+		return
+	}
+
+	parts := strings.Split(line, ":")
+	for i, p := range parts {
+		if len(v) <= i {
+			// if we have more "parts" than we have places to put them, bail for great "tolerance" of naughty configuration files
+			break
+		}
+
+		t := reflect.TypeOf(v[i])
+		if t.Kind() != reflect.Ptr {
+			// panic, because this is a programming/logic error, not a runtime one
+			panic("parseLine expects only pointers!  argument " + strconv.Itoa(i) + " is not a pointer!")
+		}
+
+		switch t.Elem().Kind() {
+		case reflect.String:
+			// "root", "adm", "/bin/bash"
+			*v[i].(*string) = p
+		case reflect.Int:
+			// "0", "4", "1000"
+			*v[i].(*int), _ = strconv.Atoi(p)
+			// ignore string to int conversion errors, for great "tolerance" of naughty configuration files
+		case reflect.Slice, reflect.Array:
+			// "", "root", "root,adm,daemon"
+			list := []string{}
+			if p != "" {
+				list = strings.Split(p, ",")
+			}
+			*v[i].(*[]string) = list
+		}
+	}
+}
+
+func ParsePasswd() ([]*User, error) {
+	return ParsePasswdFilter(nil)
+}
+
+func ParsePasswdFilter(filter func(*User) bool) ([]*User, error) {
+	f, err := os.Open("/etc/passwd")
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	return parsePasswdFile(f, filter)
+}
+
+func parsePasswdFile(r io.Reader, filter func(*User) bool) ([]*User, error) {
+	var (
+		s   = bufio.NewScanner(r)
+		out = []*User{}
+	)
+
+	for s.Scan() {
+		if err := s.Err(); err != nil {
+			return nil, err
+		}
+
+		text := strings.TrimSpace(s.Text())
+		if text == "" {
+			continue
+		}
+
+		// see: man 5 passwd
+		//  name:password:UID:GID:GECOS:directory:shell
+		// Name:Pass:Uid:Gid:Gecos:Home:Shell
+		//  root:x:0:0:root:/root:/bin/bash
+		//  adm:x:3:4:adm:/var/adm:/bin/false
+		p := &User{}
+		parseLine(
+			text,
+			&p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell,
+		)
+
+		if filter == nil || filter(p) {
+			out = append(out, p)
+		}
+	}
+
+	return out, nil
+}
+
+func ParseGroup() ([]*Group, error) {
+	return ParseGroupFilter(nil)
+}
+
+func ParseGroupFilter(filter func(*Group) bool) ([]*Group, error) {
+	f, err := os.Open("/etc/group")
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	return parseGroupFile(f, filter)
+}
+
+func parseGroupFile(r io.Reader, filter func(*Group) bool) ([]*Group, error) {
+	var (
+		s   = bufio.NewScanner(r)
+		out = []*Group{}
+	)
+
+	for s.Scan() {
+		if err := s.Err(); err != nil {
+			return nil, err
+		}
+
+		text := s.Text()
+		if text == "" {
+			continue
+		}
+
+		// see: man 5 group
+		//  group_name:password:GID:user_list
+		// Name:Pass:Gid:List
+		//  root:x:0:root
+		//  adm:x:4:root,adm,daemon
+		p := &Group{}
+		parseLine(
+			text,
+			&p.Name, &p.Pass, &p.Gid, &p.List,
+		)
+
+		if filter == nil || filter(p) {
+			out = append(out, p)
+		}
+	}
+
+	return out, nil
+}
+
+// Given a string like "user", "1000", "user:group", "1000:1000", returns the uid, gid, and list of supplementary group IDs, if possible.
+func GetUserGroupSupplementary(userSpec string, defaultUid int, defaultGid int) (int, int, []int, error) {
+	var (
+		uid      = defaultUid
+		gid      = defaultGid
+		suppGids = []int{}
+
+		userArg, groupArg string
+	)
+
+	// allow for userArg to have either "user" syntax, or optionally "user:group" syntax
+	parseLine(userSpec, &userArg, &groupArg)
+
+	users, err := ParsePasswdFilter(func(u *User) bool {
+		if userArg == "" {
+			return u.Uid == uid
+		}
+		return u.Name == userArg || strconv.Itoa(u.Uid) == userArg
+	})
+	if err != nil && !os.IsNotExist(err) {
+		if userArg == "" {
+			userArg = strconv.Itoa(uid)
+		}
+		return 0, 0, nil, fmt.Errorf("Unable to find user %v: %v", userArg, err)
+	}
+
+	haveUser := users != nil && len(users) > 0
+	if haveUser {
+		// if we found any user entries that matched our filter, let's take the first one as "correct"
+		uid = users[0].Uid
+		gid = users[0].Gid
+	} else if userArg != "" {
+		// we asked for a user but didn't find them...  let's check to see if we wanted a numeric user
+		uid, err = strconv.Atoi(userArg)
+		if err != nil {
+			// not numeric - we have to bail
+			return 0, 0, nil, fmt.Errorf("Unable to find user %v", userArg)
+		}
+
+		// if userArg couldn't be found in /etc/passwd but is numeric, just roll with it - this is legit
+	}
+
+	if groupArg != "" || (haveUser && users[0].Name != "") {
+		groups, err := ParseGroupFilter(func(g *Group) bool {
+			if groupArg != "" {
+				return g.Name == groupArg || strconv.Itoa(g.Gid) == groupArg
+			}
+			for _, u := range g.List {
+				if u == users[0].Name {
+					return true
+				}
+			}
+			return false
+		})
+		if err != nil && !os.IsNotExist(err) {
+			return 0, 0, nil, fmt.Errorf("Unable to find groups for user %v: %v", users[0].Name, err)
+		}
+
+		haveGroup := groups != nil && len(groups) > 0
+		if groupArg != "" {
+			if haveGroup {
+				// if we found any group entries that matched our filter, let's take the first one as "correct"
+				gid = groups[0].Gid
+			} else {
+				// we asked for a group but didn't find id...  let's check to see if we wanted a numeric group
+				gid, err = strconv.Atoi(groupArg)
+				if err != nil {
+					// not numeric - we have to bail
+					return 0, 0, nil, fmt.Errorf("Unable to find group %v", groupArg)
+				}
+
+				// if groupArg couldn't be found in /etc/group but is numeric, just roll with it - this is legit
+			}
+		} else if haveGroup {
+			suppGids = make([]int, len(groups))
+			for i, group := range groups {
+				suppGids[i] = group.Gid
+			}
+		}
+	}
+
+	return uid, gid, suppGids, nil
+}

pkg/user/user_test.go

@@ -0,0 +1,94 @@
+package user
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestUserParseLine(t *testing.T) {
+	var (
+		a, b string
+		c    []string
+		d    int
+	)
+
+	parseLine("", &a, &b)
+	if a != "" || b != "" {
+		t.Fatalf("a and b should be empty ('%v', '%v')", a, b)
+	}
+
+	parseLine("a", &a, &b)
+	if a != "a" || b != "" {
+		t.Fatalf("a should be 'a' and b should be empty ('%v', '%v')", a, b)
+	}
+
+	parseLine("bad boys:corny cows", &a, &b)
+	if a != "bad boys" || b != "corny cows" {
+		t.Fatalf("a should be 'bad boys' and b should be 'corny cows' ('%v', '%v')", a, b)
+	}
+
+	parseLine("", &c)
+	if len(c) != 0 {
+		t.Fatalf("c should be empty (%#v)", c)
+	}
+
+	parseLine("d,e,f:g:h:i,j,k", &c, &a, &b, &c)
+	if a != "g" || b != "h" || len(c) != 3 || c[0] != "i" || c[1] != "j" || c[2] != "k" {
+		t.Fatalf("a should be 'g', b should be 'h', and c should be ['i','j','k'] ('%v', '%v', '%#v')", a, b, c)
+	}
+
+	parseLine("::::::::::", &a, &b, &c)
+	if a != "" || b != "" || len(c) != 0 {
+		t.Fatalf("a, b, and c should all be empty ('%v', '%v', '%#v')", a, b, c)
+	}
+
+	parseLine("not a number", &d)
+	if d != 0 {
+		t.Fatalf("d should be 0 (%v)", d)
+	}
+
+	parseLine("b:12:c", &a, &d, &b)
+	if a != "b" || b != "c" || d != 12 {
+		t.Fatalf("a should be 'b' and b should be 'c', and d should be 12 ('%v', '%v', %v)", a, b, d)
+	}
+}
+
+func TestUserParsePasswd(t *testing.T) {
+	users, err := parsePasswdFile(strings.NewReader(`
+root:x:0:0:root:/root:/bin/bash
+adm:x:3:4:adm:/var/adm:/bin/false
+this is just some garbage data
+`), nil)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	if len(users) != 3 {
+		t.Fatalf("Expected 3 users, got %v", len(users))
+	}
+	if users[0].Uid != 0 || users[0].Name != "root" {
+		t.Fatalf("Expected users[0] to be 0 - root, got %v - %v", users[0].Uid, users[0].Name)
+	}
+	if users[1].Uid != 3 || users[1].Name != "adm" {
+		t.Fatalf("Expected users[1] to be 3 - adm, got %v - %v", users[1].Uid, users[1].Name)
+	}
+}
+
+func TestUserParseGroup(t *testing.T) {
+	groups, err := parseGroupFile(strings.NewReader(`
+root:x:0:root
+adm:x:4:root,adm,daemon
+this is just some garbage data
+`), nil)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	if len(groups) != 3 {
+		t.Fatalf("Expected 3 groups, got %v", len(groups))
+	}
+	if groups[0].Gid != 0 || groups[0].Name != "root" || len(groups[0].List) != 1 {
+		t.Fatalf("Expected groups[0] to be 0 - root - 1 member, got %v - %v - %v", groups[0].Gid, groups[0].Name, len(groups[0].List))
+	}
+	if groups[1].Gid != 4 || groups[1].Name != "adm" || len(groups[1].List) != 3 {
+		t.Fatalf("Expected groups[1] to be 4 - adm - 3 members, got %v - %v - %v", groups[1].Gid, groups[1].Name, len(groups[1].List))
+	}
+}

utils/utils.go

@@ -836,37 +836,6 @@ func ParseRepositoryTag(repos string) (string, string) {
 	return repos, ""
 }
 
-type User struct {
-	Uid      string // user id
-	Gid      string // primary group id
-	Username string
-	Name     string
-	HomeDir  string
-}
-
-// UserLookup check if the given username or uid is present in /etc/passwd
-// and returns the user struct.
-// If the username is not found, an error is returned.
-func UserLookup(uid string) (*User, error) {
-	file, err := ioutil.ReadFile("/etc/passwd")
-	if err != nil {
-		return nil, err
-	}
-	for _, line := range strings.Split(string(file), "\n") {
-		data := strings.Split(line, ":")
-		if len(data) > 5 && (data[0] == uid || data[2] == uid) {
-			return &User{
-				Uid:      data[2],
-				Gid:      data[3],
-				Username: data[0],
-				Name:     data[4],
-				HomeDir:  data[5],
-			}, nil
-		}
-	}
-	return nil, fmt.Errorf("User not found in /etc/passwd")
-}
-
 // An StatusError reports an unsuccessful exit by a command.
 type StatusError struct {
 	Status     string