Merge pull request #471 from csrwng/user_spec

Merged by openshift-bot

Author: OpenShift Bot

pkg/docker/util.go

@@ -1,14 +1,14 @@
 package docker
 
 import (
+	"bufio"
 	"fmt"
 	"io"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 
-	"bufio"
-
 	client "github.com/fsouza/go-dockerclient"
 	"github.com/golang/glog"
 	"github.com/openshift/source-to-image/pkg/api"
@@ -202,10 +202,11 @@ func CheckAllowedUser(d Docker, imageName string, uids user.RangeList, isOnbuild
 	if uids == nil || uids.Empty() {
 		return nil
 	}
-	imageUser, err := d.GetImageUser(imageName)
+	imageUserSpec, err := d.GetImageUser(imageName)
 	if err != nil {
 		return err
 	}
+	imageUser := extractUser(imageUserSpec)
 	if !user.IsUserAllowed(imageUser, &uids) {
 		return errors.NewBuilderUserNotAllowedError(imageName, false)
 	}
@@ -214,13 +215,41 @@ func CheckAllowedUser(d Docker, imageName string, uids user.RangeList, isOnbuild
 		if err != nil {
 			return err
 		}
-		if !user.IsOnbuildAllowed(cmds, &uids) {
+		if !isOnbuildAllowed(cmds, &uids) {
 			return errors.NewBuilderUserNotAllowedError(imageName, true)
 		}
 	}
 	return nil
 }
 
+var dockerLineDelim = regexp.MustCompile(`[\t\v\f\r ]+`)
+
+// isOnbuildAllowed checks a list of Docker ONBUILD instructions for
+// user directives. It ensures that any users specified by the directives
+// falls within the specified range list of users.
+func isOnbuildAllowed(directives []string, allowed *user.RangeList) bool {
+	for _, line := range directives {
+		parts := dockerLineDelim.Split(line, 2)
+		if strings.ToLower(parts[0]) != "user" {
+			continue
+		}
+		uname := extractUser(parts[1])
+		if !user.IsUserAllowed(uname, allowed) {
+			return false
+		}
+	}
+	return true
+}
+
+func extractUser(userSpec string) string {
+	user := userSpec
+	if strings.Contains(user, ":") {
+		parts := strings.SplitN(userSpec, ":", 2)
+		user = parts[0]
+	}
+	return strings.TrimSpace(user)
+}
+
 // IsReachable returns true if the Docker daemon is reachable from s2i
 func IsReachable(config *api.Config) bool {
 	d, err := New(config.DockerConfig, config.PullAuthentication)

pkg/docker/util_test.go

@@ -78,6 +78,45 @@ func TestCheckAllowedUser(t *testing.T) {
 			onbuild:     []string{"USER 501", "VOLUME /data"},
 			expectErr:   false,
 		},
+		{
+			name:        "AllowedUIDs is set, numeric user with group",
+			allowedUIDs: rangeList("1-"),
+			user:        "5:5000",
+			expectErr:   false,
+		},
+		{
+			name:        "AllowedUIDs is set, numeric user with named group",
+			allowedUIDs: rangeList("1-"),
+			user:        "5:group",
+			expectErr:   false,
+		},
+		{
+			name:        "AllowedUIDs is set, named user with group",
+			allowedUIDs: rangeList("1-"),
+			user:        "root:wheel",
+			expectErr:   true,
+		},
+		{
+			name:        "AllowedUIDs is set, numeric user, onbuild user with group",
+			allowedUIDs: rangeList("1-"),
+			user:        "200",
+			onbuild:     []string{"RUN echo \"hello world\"", "USER 10:100"},
+			expectErr:   false,
+		},
+		{
+			name:        "AllowedUIDs is set, numeric user, onbuild named user with group",
+			allowedUIDs: rangeList("1-"),
+			user:        "200",
+			onbuild:     []string{"RUN echo \"hello world\"", "USER root:wheel"},
+			expectErr:   true,
+		},
+		{
+			name:        "AllowedUIDs is set, numeric user, onbuild user with named group",
+			allowedUIDs: rangeList("1-"),
+			user:        "200",
+			onbuild:     []string{"RUN echo \"hello world\"", "USER 10:wheel"},
+			expectErr:   false,
+		},
 	}
 
 	for _, tc := range tests {

pkg/util/user/rangelist.go

@@ -1,7 +1,6 @@
 package user
 
 import (
-	"regexp"
 	"strconv"
 	"strings"
 )
@@ -82,21 +81,3 @@ func IsUserAllowed(user string, allowed *RangeList) bool {
 	}
 	return allowed.Contains(uid)
 }
-
-var dockerLineDelim = regexp.MustCompile(`[\t\v\f\r ]+`)
-
-// IsOnbuildAllowed checks a list of Docker ONBUILD instructions for
-// user directives. It ensures that any users specified by the directives
-// falls within the specified range list of users.
-func IsOnbuildAllowed(directives []string, allowed *RangeList) bool {
-	for _, line := range directives {
-		parts := dockerLineDelim.Split(line, 2)
-		if strings.ToLower(parts[0]) != "user" {
-			continue
-		}
-		if !IsUserAllowed(parts[1], allowed) {
-			return false
-		}
-	}
-	return true
-}