Fix exec user behavior

dcantah · dcantah · commit 149f1e35483f · 2021-09-17T02:21:52.000-07:00
Signed-off-by: Daniel Canter &lt;dcanter@microsoft.com&gt;
diff --git a/internal/guest/runtime/hcsv2/container.go b/internal/guest/runtime/hcsv2/container.go
@@ -66,6 +66,19 @@ func (c *Container) ExecProcess(ctx context.Context, process *oci.Process, conSe
 		return -1, err
 	}
 
+	// If the client provided a user for the container to run as, we want to have the exec run as this user as well
+	// unless the exec's spec was explicitly set to a different user. If the Username field is filled in on the containers
+	// spec, at this point that means the work to find a uid:gid pairing for this username has already been done, so simply
+	// assign the uid:gid from the container.
+	if process.User.Username != "" {
+		// The exec provided a user string of it's own. Grab the uid:gid pairing for the string (if one exists).
+		if err := setUserStr(&oci.Spec{Root: c.spec.Root, Process: process}, process.User.Username); err != nil {
+			return -1, err
+		}
+	} else if c.spec.Process.User.Username != "" {
+		process.User = c.spec.Process.User
+	}
+
 	p, err := c.container.ExecProcess(process, stdioSet)
 	if err != nil {
 		stdioSet.Close()
diff --git a/internal/guest/runtime/hcsv2/sandbox_container.go b/internal/guest/runtime/hcsv2/sandbox_container.go
@@ -102,8 +102,12 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (
 		return errors.Wrap(err, "failed to write sandbox resolv.conf")
 	}
 
-	if userstr, ok := spec.Annotations["io.microsoft.lcow.userstr"]; ok {
-		if err := setUserStr(spec, userstr); err != nil {
+	// User.Username is generally only used on Windows, but as there's no (easy/fast at least) way to grab
+	// a uid:gid pairing for a username string on the host, we need to defer whatever user string the
+	// client provided in the guest. The username field is used as a temporary holding place until we
+	// can perform this work here when we actually have the rootfs to inspect.
+	if spec.Process.User.Username != "" {
+		if err := setUserStr(spec, spec.Process.User.Username); err != nil {
 			return err
 		}
 	}
diff --git a/internal/guest/runtime/hcsv2/spec.go b/internal/guest/runtime/hcsv2/spec.go
@@ -5,12 +5,12 @@ package hcsv2
 import (
 	"context"
 	"fmt"
-	"github.com/Microsoft/hcsshim/internal/log"
-	"github.com/opencontainers/runc/libcontainer/devices"
 	"path/filepath"
 	"strconv"
 	"strings"
 
+	"github.com/Microsoft/hcsshim/internal/log"
+	"github.com/opencontainers/runc/libcontainer/devices"
 	"github.com/opencontainers/runc/libcontainer/user"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
@@ -225,11 +225,5 @@ func applyAnnotationsToSpec(ctx context.Context, spec *oci.Spec) error {
 		}
 	}
 
-	// Check if we need to set non-default user
-	if userstr, ok := spec.Annotations["io.microsoft.lcow.userstr"]; ok {
-		if err := setUserStr(spec, userstr); err != nil {
-			return err
-		}
-	}
 	return nil
 }
diff --git a/internal/guest/runtime/hcsv2/workload_container.go b/internal/guest/runtime/hcsv2/workload_container.go
@@ -161,6 +161,16 @@ func setupWorkloadContainerSpec(ctx context.Context, sbid, id string, spec *oci.
 		return err
 	}
 
+	// User.Username is generally only used on Windows, but as there's no (easy/fast at least) way to grab
+	// a uid:gid pairing for a username string on the host, we need to defer whatever user string the
+	// client provided in the guest. The username field is used as a temporary holding place until we
+	// can perform this work here when we actually have the rootfs to inspect.
+	if spec.Process.User.Username != "" {
+		if err := setUserStr(spec, spec.Process.User.Username); err != nil {
+			return err
+		}
+	}
+
 	// Force the parent cgroup into our /containers root
 	spec.Linux.CgroupsPath = "/containers/" + id
 

Original file line number	Diff line number	Diff line change
`@@ -102,8 +102,12 @@ func setupSandboxContainerSpec(ctx context.Context, id string, spec *oci.Spec) (`
`102`	`102`	`return errors.Wrap(err, "failed to write sandbox resolv.conf")`
`103`	`103`	`}`
`104`	`104`
`105`		`- if userstr, ok := spec.Annotations["io.microsoft.lcow.userstr"]; ok {`
`106`		`- if err := setUserStr(spec, userstr); err != nil {`
	`105`	`+ // User.Username is generally only used on Windows, but as there's no (easy/fast at least) way to grab`
	`106`	`+ // a uid:gid pairing for a username string on the host, we need to defer whatever user string the`
	`107`	`+ // client provided in the guest. The username field is used as a temporary holding place until we`
	`108`	`+ // can perform this work here when we actually have the rootfs to inspect.`
	`109`	`+ if spec.Process.User.Username != "" {`
	`110`	`+ if err := setUserStr(spec, spec.Process.User.Username); err != nil {`
`107`	`111`	`return err`
`108`	`112`	`}`
`109`	`113`	`}`