veyron/services/mgmt/suidhelper: record username
The node manager tests do not run the suid helper with root
permissions. Consequently, the tests cannot run test apps as different
users. Instead, this change modifies the suid helper to work around
this issue. When in test mode, the suid helper records the username
requested by the node manager and reports this to the test harness
for validation.
Change-Id: Ibedd26b461216ac677fe44774e53862435c9e800
diff --git a/services/mgmt/node/impl/impl_test.go b/services/mgmt/node/impl/impl_test.go
index c0f74d7..493c0f4 100644
--- a/services/mgmt/node/impl/impl_test.go
+++ b/services/mgmt/node/impl/impl_test.go
@@ -5,10 +5,12 @@
"crypto/md5"
"encoding/base64"
"encoding/hex"
+ "encoding/json"
"fmt"
"io/ioutil"
"os"
goexec "os/exec"
+ "os/user"
"path/filepath"
"reflect"
"sort"
@@ -170,7 +172,7 @@
}
func ping() {
- if call, err := rt.R().Client().StartCall(rt.R().NewContext(), "pingserver", "Ping", nil); err != nil {
+ if call, err := rt.R().Client().StartCall(rt.R().NewContext(), "pingserver", "Ping", []interface{}{os.Getenv(suidhelper.SavedArgs)}); err != nil {
vlog.Fatalf("StartCall failed: %v", err)
} else if err = call.Finish(); err != nil {
vlog.Fatalf("Finish failed: %v", err)
@@ -493,9 +495,11 @@
runNM.ExpectEOFAndWait()
}
-type pingServerDisp chan<- struct{}
+type pingServerDisp chan<- string
-func (p pingServerDisp) Ping(ipc.ServerCall) { p <- struct{}{} }
+func (p pingServerDisp) Ping(_ ipc.ServerCall, arg string) {
+ p <- arg
+}
func verifyAppWorkspace(t *testing.T, root, appID, instanceID string) {
// HACK ALERT: for now, we peek inside the node manager's directory
@@ -523,6 +527,20 @@
// END HACK
}
+// TODO(rjkroege): Consider validating additional parameters.
+func verifyHelperArgs(t *testing.T, env, username string) {
+ d := json.NewDecoder(strings.NewReader(env))
+ var savedArgs suidhelper.ArgsSavedForTest
+
+ if err := d.Decode(&savedArgs); err != nil {
+ t.Fatalf("failed to decode preserved argument %v: %v", env, err)
+ }
+
+ if savedArgs.Uname != username {
+ t.Fatalf("got username %v, expected username %v", savedArgs.Uname, username)
+ }
+}
+
// TestAppLifeCycle installs an app, starts it, suspends it, resumes it, and
// then stops it.
func TestAppLifeCycle(t *testing.T) {
@@ -551,7 +569,7 @@
// Create the local server that the app uses to let us know it's ready.
server, _ := newServer()
defer server.Stop()
- pingCh := make(chan struct{}, 1)
+ pingCh := make(chan string, 1)
if err := server.Serve("pingserver", ipc.LeafDispatcher(pingServerDisp(pingCh), nil)); err != nil {
t.Fatalf("Serve(%q, <dispatcher>) failed: %v", "pingserver", err)
}
@@ -567,7 +585,13 @@
// Start an instance of the app.
instance1ID := startApp(t, appID)
- <-pingCh // Wait until the app pings us that it's ready.
+
+ u, err := user.Current()
+ if err != nil {
+ t.Fatalf("user.Current() failed: %v", err)
+ }
+ verifyHelperArgs(t, <-pingCh, u.Username) // Wait until the app pings us that it's ready.
+
v1EP1 := resolve(t, "appV1", 1)[0]
// Suspend the app instance.
@@ -575,7 +599,7 @@
resolveExpectNotFound(t, "appV1")
resumeApp(t, appID, instance1ID)
- <-pingCh
+ verifyHelperArgs(t, <-pingCh, u.Username) // Wait until the app pings us that it's ready.
oldV1EP1 := v1EP1
if v1EP1 = resolve(t, "appV1", 1)[0]; v1EP1 == oldV1EP1 {
t.Fatalf("Expected a new endpoint for the app after suspend/resume")
@@ -583,7 +607,7 @@
// Start a second instance.
instance2ID := startApp(t, appID)
- <-pingCh // Wait until the app pings us that it's ready.
+ verifyHelperArgs(t, <-pingCh, u.Username) // Wait until the app pings us that it's ready.
// There should be two endpoints mounted as "appV1", one for each
// instance of the app.
@@ -629,7 +653,7 @@
// Resume first instance.
resumeApp(t, appID, instance1ID)
- <-pingCh
+ verifyHelperArgs(t, <-pingCh, u.Username) // Wait until the app pings us that it's ready.
// Both instances should still be running the first version of the app.
// Check that the mounttable contains two endpoints, one of which is
// v1EP2.
@@ -653,7 +677,7 @@
// Start a third instance.
instance3ID := startApp(t, appID)
- <-pingCh // Wait until the app pings us that it's ready.
+ verifyHelperArgs(t, <-pingCh, u.Username) // Wait until the app pings us that it's ready.
resolve(t, "appV2", 1)
// Stop second instance.
@@ -669,7 +693,7 @@
// Start a fourth instance. It should be started from version 1.
instance4ID := startApp(t, appID)
- <-pingCh // Wait until the app pings us that it's ready.
+ verifyHelperArgs(t, <-pingCh, u.Username) // Wait until the app pings us that it's ready.
resolve(t, "appV1", 1)
stopApp(t, appID, instance4ID)
resolveExpectNotFound(t, "appV1")
diff --git a/services/mgmt/suidhelper/impl/args.go b/services/mgmt/suidhelper/impl/args.go
index 4e72e05..964f910 100644
--- a/services/mgmt/suidhelper/impl/args.go
+++ b/services/mgmt/suidhelper/impl/args.go
@@ -1,8 +1,11 @@
package impl
import (
+ "bytes"
+ "encoding/json"
"flag"
"fmt"
+ "os"
"os/user"
"strconv"
)
@@ -18,6 +21,16 @@
envv []string
}
+type ArgsSavedForTest struct {
+ Uname string
+ Workpace string
+ Run string
+ StdoutLog string
+ StderrLog string
+}
+
+const SavedArgs = "VEYRON_SAVED_ARGS"
+
var flagUsername, flagWorkspace, flagStdoutLog, flagStderrLog, flagRun *string
var flagMinimumUid *int64
@@ -62,6 +75,21 @@
return fmt.Errorf("suidhelper does not permit uids less than %d", uint32(*flagMinimumUid))
}
+ // Preserve the arguments for examination by the test harness if executed
+ // in the course of a test.
+ if os.Getenv("VEYRON_SUIDHELPER_TEST") != "" {
+ b := new(bytes.Buffer)
+ enc := json.NewEncoder(b)
+ enc.Encode(ArgsSavedForTest{
+ Uname: *flagUsername,
+ Workpace: *flagWorkspace,
+ Run: *flagRun,
+ StdoutLog: *flagStdoutLog,
+ StderrLog: *flagStderrLog,
+ })
+ env = append(env, SavedArgs+"="+b.String())
+ }
+
wp.uid = uint32(uid)
wp.gid = uint32(gid)
wp.workspace = *flagWorkspace
@@ -69,6 +97,7 @@
wp.stdoutLog = *flagStdoutLog
wp.stderrLog = *flagStderrLog
wp.argv = fs.Args()
+ // TODO(rjkroege): Reduce the environment to the absolute minimum needed.
wp.envv = env
return nil
