veyron/services/mgmt/node/impl/*: add a suidhelper
This is the first patch in a sequence to support running Veyron applications
as the UNIX user associated with the invoking Veyron principal. In this
change, I add the suidhelper code and the node manager's ability to invoke
the helper. A future change will enable the actual setuid functionality
where possible.
Change-Id: I3a2ec6ca1f69c28dc506f5af8c1687335a2fd2f6
diff --git a/services/mgmt/node/impl/app_invoker.go b/services/mgmt/node/impl/app_invoker.go
index 5fce914..682a5ab 100644
--- a/services/mgmt/node/impl/app_invoker.go
+++ b/services/mgmt/node/impl/app_invoker.go
@@ -7,6 +7,7 @@
// TODO(caprita): Not all is yet implemented.
//
// <config.Root>/
+// helper - the setuidhelper binary to invoke an application as a specified user.
// app-<hash 1>/ - the application dir is named using a hash of the application title
// installation-<id 1>/ - installations are labelled with ids
// <status> - one of the values for installationState enum
@@ -89,6 +90,7 @@
"io/ioutil"
"os"
"os/exec"
+ "os/user"
"path/filepath"
"reflect"
"strings"
@@ -385,7 +387,9 @@
return instanceDir, instanceID, nil
}
-func genCmd(instanceDir string) (*exec.Cmd, error) {
+// TODO(rjkroege): Turning on the setuid feature of the suidhelper
+// requires an installer with root permissions to install it in <config.Root>/helper
+func genCmd(instanceDir string, helperPath string) (*exec.Cmd, error) {
versionLink := filepath.Join(instanceDir, "version")
versionDir, err := filepath.EvalSymlinks(versionLink)
if err != nil {
@@ -401,10 +405,28 @@
vlog.Errorf("Stat(%v) failed: %v", binPath, err)
return nil, errOperationFailed
}
- // TODO(caprita): For the purpose of isolating apps, we should run them
- // as different users. We'll need to either use the root process or a
- // suid script to be able to do it.
- cmd := exec.Command(binPath)
+
+ helperStat, err := os.Stat(helperPath)
+ if err != nil {
+ vlog.Errorf("Stat(%v) failed: %v. helper is required.", helperPath, err)
+ return nil, errOperationFailed
+ }
+ cmd := exec.Command(helperPath)
+
+ cmd.Args = append(cmd.Args, "--username")
+ if helperStat.Mode()&os.ModeSetuid == 0 {
+ vlog.Errorf("helper not setuid. Node manager will invoke app with its own userid")
+ user, err := user.Current()
+ if err != nil {
+ vlog.Errorf("user.Current() failed: %v", err)
+ return nil, errOperationFailed
+ }
+ cmd.Args = append(cmd.Args, user.Username)
+ } else {
+ // TODO(rjkroege): Use the username associated with the veyron identity.
+ return nil, errOperationFailed
+ }
+
// TODO(caprita): Also pass in configuration info like NAMESPACE_ROOT to
// the app (to point to the device mounttable).
cmd.Env = envelope.Env
@@ -413,17 +435,32 @@
return nil, err
}
cmd.Dir = rootDir
+ cmd.Args = append(cmd.Args, "--workspace")
+ cmd.Args = append(cmd.Args, rootDir)
+
logDir := filepath.Join(instanceDir, "logs")
if err := mkdir(logDir); err != nil {
return nil, err
}
timestamp := time.Now().UnixNano()
- if cmd.Stdout, err = openWriteFile(filepath.Join(logDir, fmt.Sprintf("STDOUT-%d", timestamp))); err != nil {
+ stdoutLog := filepath.Join(logDir, fmt.Sprintf("STDOUT-%d", timestamp))
+ if cmd.Stdout, err = openWriteFile(stdoutLog); err != nil {
return nil, err
}
- if cmd.Stderr, err = openWriteFile(filepath.Join(logDir, fmt.Sprintf("STDERR-%d", timestamp))); err != nil {
+ cmd.Args = append(cmd.Args, "--stdoutlog")
+ cmd.Args = append(cmd.Args, stdoutLog)
+
+ stderrLog := filepath.Join(logDir, fmt.Sprintf("STDERR-%d", timestamp))
+ if cmd.Stderr, err = openWriteFile(stderrLog); err != nil {
return nil, err
}
+ cmd.Args = append(cmd.Args, "--stderrlog")
+ cmd.Args = append(cmd.Args, stderrLog)
+
+ cmd.Args = append(cmd.Args, "--run")
+ cmd.Args = append(cmd.Args, binPath)
+ cmd.Args = append(cmd.Args, "--")
+
// Set up args and env.
cmd.Args = append(cmd.Args, "--log_dir=../logs")
cmd.Args = append(cmd.Args, envelope.Args...)
@@ -477,7 +514,7 @@
if err := transitionInstance(instanceDir, suspended, starting); err != nil {
return err
}
- cmd, err := genCmd(instanceDir)
+ cmd, err := genCmd(instanceDir, filepath.Join(i.config.Root, "helper"))
if err == nil {
err = i.startCmd(instanceDir, cmd)
}
diff --git a/services/mgmt/node/impl/impl_test.go b/services/mgmt/node/impl/impl_test.go
index 595b66e..cdf251e 100644
--- a/services/mgmt/node/impl/impl_test.go
+++ b/services/mgmt/node/impl/impl_test.go
@@ -23,6 +23,7 @@
"veyron.io/veyron/veyron/services/mgmt/lib/exec"
"veyron.io/veyron/veyron/services/mgmt/node/config"
"veyron.io/veyron/veyron/services/mgmt/node/impl"
+ suidhelper "veyron.io/veyron/veyron/services/mgmt/suidhelper/impl"
"veyron.io/veyron/veyron2"
"veyron.io/veyron/veyron2/ipc"
@@ -37,12 +38,7 @@
// TestHelperProcess is blackbox boilerplate.
func TestHelperProcess(t *testing.T) {
- blackbox.HelperProcess(t)
-}
-
-func init() {
- // All the tests and the subprocesses they start require a runtime; so just
- // create it here.
+ // All TestHelperProcess invocations need a Runtime. Create it here.
rt.Init()
// Disable the cache because we will be manipulating/using the namespace
@@ -53,6 +49,35 @@
blackbox.CommandTable["execScript"] = execScript
blackbox.CommandTable["nodeManager"] = nodeManager
blackbox.CommandTable["app"] = app
+
+ blackbox.HelperProcess(t)
+}
+
+func init() {
+ if os.Getenv("VEYRON_BLACKBOX_TEST") == "1" {
+ return
+ }
+
+ // All the tests require a runtime; so just create it here.
+ rt.Init()
+
+ // Disable the cache because we will be manipulating/using the namespace
+ // across multiple processes and want predictable behaviour without
+ // relying on timeouts.
+ rt.R().Namespace().CacheCtl(naming.DisableCache(true))
+}
+
+// TestSuidHelper is testing boilerplate for suidhelper that does not
+// invoke rt.Init() because the suidhelper is not a Veyron application.
+func TestSuidHelper(t *testing.T) {
+ if os.Getenv("VEYRON_SUIDHELPER_TEST") != "1" {
+ return
+ }
+ vlog.VI(1).Infof("TestSuidHelper starting")
+
+ if err := suidhelper.Run(os.Environ()); err != nil {
+ vlog.Fatalf("Failed to Run() setuidhelper: %v", err)
+ }
}
// execScript launches the script passed as argument.
@@ -149,6 +174,7 @@
}
}
+// TODO(rjkroege): Preserve suidhelper and app logs when errors occur.
func app(args []string) {
if expected, got := 1, len(args); expected != got {
vlog.Fatalf("Unexpected number of arguments: expected %d, got %d", expected, got)
@@ -168,12 +194,14 @@
}
}
-// generateScript is very similar in behavior to its namesake in invoker.go.
+// TODO(rjkroege): generateNodeManagerScript and generateSuidHelperScript have code
+// similarity that might benefit from refactoring.
+// generateNodeManagerScript is very similar in behavior to generateScript in node_invoker.go.
// However, we chose to re-implement it here for two reasons: (1) avoid making
// generateScript public; and (2) how the test choses to invoke the node manager
// subprocess the first time should be independent of how node manager
// implementation sets up its updated versions.
-func generateScript(t *testing.T, root string, cmd *goexec.Cmd) string {
+func generateNodeManagerScript(t *testing.T, root string, cmd *goexec.Cmd) string {
output := "#!/bin/bash\n"
output += strings.Join(config.QuoteEnv(cmd.Env), " ") + " "
output += cmd.Args[0] + " " + strings.Join(cmd.Args[1:], " ")
@@ -190,6 +218,26 @@
return path
}
+// generateSuidHelperScript builds a script to execute the test target as
+// a suidhelper instance and installs it in <root>/helper.
+func generateSuidHelperScript(t *testing.T, root string) {
+ output := "#!/bin/bash\n"
+ output += "VEYRON_SUIDHELPER_TEST=1"
+ output += " "
+ output += "exec" + " " + os.Args[0] + " -test.run=TestSuidHelper $*"
+ output += "\n"
+
+ vlog.VI(1).Infof("script\n%s", output)
+
+ if err := os.MkdirAll(root, 0755); err != nil {
+ t.Fatalf("MkdirAll failed: %v", err)
+ }
+ path := filepath.Join(root, "helper")
+ if err := ioutil.WriteFile(path, []byte(output), 0755); err != nil {
+ t.Fatalf("WriteFile(%v) failed: %v", path, err)
+ }
+}
+
// nodeEnvelopeFromCmd returns a node manager application envelope that
// describes the given command object.
func nodeEnvelopeFromCmd(cmd *goexec.Cmd) *application.Envelope {
@@ -269,7 +317,7 @@
defer setupChildCommand(nm)()
// This is the script that we'll point the current link to initially.
- scriptPathFactory := generateScript(t, root, nm.Cmd)
+ scriptPathFactory := generateNodeManagerScript(t, root, nm.Cmd)
if err := os.Symlink(scriptPathFactory, currLink); err != nil {
t.Fatalf("Symlink(%q, %q) failed: %v", scriptPathFactory, currLink, err)
@@ -479,6 +527,9 @@
root, cleanup := setupRootDir()
defer cleanup()
+ // Create a script wrapping the test target that implements suidhelper.
+ generateSuidHelperScript(t, root)
+
// Set up the node manager. Since we won't do node manager updates,
// don't worry about its application envelope and current link.
nm := blackbox.HelperCommand(t, "nodeManager", "nm", root, "unused app repo name", "unused curr link")
diff --git a/services/mgmt/suidhelper/impl/args.go b/services/mgmt/suidhelper/impl/args.go
new file mode 100644
index 0000000..76a200e
--- /dev/null
+++ b/services/mgmt/suidhelper/impl/args.go
@@ -0,0 +1,58 @@
+package impl
+
+import (
+ "flag"
+ "fmt"
+ "os/user"
+)
+
+type WorkParameters struct {
+ uid string
+ gid string
+ workspace string
+ stderrLog string
+ stdoutLog string
+ argv0 string
+ argv []string
+ envv []string
+}
+
+var flagUsername, flagWorkspace, flagStdoutLog, flagStderrLog, flagRun *string
+
+func init() {
+ // Add flags to global set.
+ setupFlags(flag.CommandLine)
+}
+
+func setupFlags(fs *flag.FlagSet) {
+ flagUsername = fs.String("username", "", "The UNIX user name used for the other functions of this tool.")
+ flagWorkspace = fs.String("workspace", "", "Path to the application's workspace directory.")
+ flagStdoutLog = fs.String("stdoutlog", "", "Path to the stdout log file.")
+ flagStderrLog = fs.String("stderrlog", "", "Path to the stdin log file.")
+ flagRun = fs.String("run", "", "Path to the application to exec.")
+}
+
+// ParseArguments populates the WorkParameter object from the provided args
+// and env strings.
+func (wp *WorkParameters) ProcessArguments(fs *flag.FlagSet, env []string) error {
+ username := *flagUsername
+ if username == "" {
+ return fmt.Errorf("--username missing")
+ }
+
+ usr, err := user.Lookup(username)
+ if err != nil {
+ return fmt.Errorf("--username %s: unknown user", username)
+ }
+
+ wp.uid = usr.Uid
+ wp.gid = usr.Gid
+ wp.workspace = *flagWorkspace
+ wp.argv0 = *flagRun
+ wp.stdoutLog = *flagStdoutLog
+ wp.stderrLog = *flagStderrLog
+ wp.argv = fs.Args()
+ wp.envv = env
+
+ return nil
+}
diff --git a/services/mgmt/suidhelper/impl/args_darwin_test.go b/services/mgmt/suidhelper/impl/args_darwin_test.go
new file mode 100644
index 0000000..7199f15
--- /dev/null
+++ b/services/mgmt/suidhelper/impl/args_darwin_test.go
@@ -0,0 +1,7 @@
+package impl
+
+const (
+ testUserName = "_uucp"
+ testUidString = "4"
+ testGidString = "4"
+)
diff --git a/services/mgmt/suidhelper/impl/args_linux_test.go b/services/mgmt/suidhelper/impl/args_linux_test.go
new file mode 100644
index 0000000..6403af4
--- /dev/null
+++ b/services/mgmt/suidhelper/impl/args_linux_test.go
@@ -0,0 +1,7 @@
+package impl
+
+const (
+ testUserName = "uucp"
+ testUidString = "10"
+ testGidString = "10"
+)
diff --git a/services/mgmt/suidhelper/impl/args_test.go b/services/mgmt/suidhelper/impl/args_test.go
new file mode 100644
index 0000000..56666c0
--- /dev/null
+++ b/services/mgmt/suidhelper/impl/args_test.go
@@ -0,0 +1,71 @@
+package impl
+
+import (
+ "flag"
+ "fmt"
+ "reflect"
+ "testing"
+)
+
+func TestParseArguments(t *testing.T) {
+ cases := []struct {
+ cmdline []string
+ env []string
+ err error
+ expected WorkParameters
+ }{
+
+ {
+ []string{"setuidhelper"},
+ []string{},
+ fmt.Errorf("--username missing"),
+ WorkParameters{},
+ },
+
+ {
+ []string{"setuidhelper", "--username", testUserName},
+ []string{"A=B"},
+ nil,
+ WorkParameters{
+ uid: testUidString,
+ gid: testGidString,
+ workspace: "",
+ stderrLog: "",
+ stdoutLog: "",
+ argv0: "",
+ argv: []string{},
+ envv: []string{"A=B"},
+ },
+ },
+
+ {
+ []string{"setuidhelper", "--username", testUserName, "--workspace", "/hello",
+ "--stdoutlog", "/stdout", "--stderrlog", "/stderr", "--run", "/bin/veyron", "--", "one", "two"},
+ []string{"A=B"},
+ nil,
+ WorkParameters{
+ uid: testUidString,
+ gid: testGidString,
+ workspace: "/hello",
+ stderrLog: "/stderr",
+ stdoutLog: "/stdout",
+ argv0: "/bin/veyron",
+ argv: []string{"one", "two"},
+ envv: []string{"A=B"},
+ },
+ },
+ }
+
+ for _, c := range cases {
+ var wp WorkParameters
+ fs := flag.NewFlagSet(c.cmdline[0], flag.ExitOnError)
+ setupFlags(fs)
+ fs.Parse(c.cmdline[1:])
+ if err := wp.ProcessArguments(fs, c.env); !reflect.DeepEqual(err, c.err) {
+ t.Fatalf("got %v, expected %v error", err, c.err)
+ }
+ if !reflect.DeepEqual(wp, c.expected) {
+ t.Fatalf("got %#v expected %#v", wp, c.expected)
+ }
+ }
+}
diff --git a/services/mgmt/suidhelper/impl/run.go b/services/mgmt/suidhelper/impl/run.go
new file mode 100644
index 0000000..bc7b6dd
--- /dev/null
+++ b/services/mgmt/suidhelper/impl/run.go
@@ -0,0 +1,20 @@
+package impl
+
+import (
+ "flag"
+)
+
+func Run(environ []string) error {
+ var work WorkParameters
+ if err := work.ProcessArguments(flag.CommandLine, environ); err != nil {
+ return err
+ }
+
+ // 1. For each chown directory, chown.
+ if err := work.Chown(); err != nil {
+ return err
+ }
+
+ // 2. Run the command if it exists.
+ return work.Exec()
+}
diff --git a/services/mgmt/suidhelper/impl/system.go b/services/mgmt/suidhelper/impl/system.go
new file mode 100644
index 0000000..6a85486
--- /dev/null
+++ b/services/mgmt/suidhelper/impl/system.go
@@ -0,0 +1,29 @@
+// +build linux darwin
+
+package impl
+
+import (
+ "log"
+ "syscall"
+)
+
+// Chown is only availabe on UNIX platforms so this file has a build
+// restriction.
+func (hw *WorkParameters) Chown() error {
+ // TODO(rjkroege): Insert the actual code to chown in a subsequent CL.
+ log.Printf("chown %s %s\n", hw.uid, hw.workspace)
+ log.Printf("chown %s %s\n", hw.uid, hw.stdoutLog)
+ log.Printf("chown %s %s\n", hw.uid, hw.stderrLog)
+
+ return nil
+}
+
+func (hw *WorkParameters) Exec() error {
+ log.Printf("should be Exec-ing work parameters: %#v\n", hw)
+ log.Printf("su %s\n", hw.uid)
+ log.Printf("exec %s %v", hw.argv0, hw.argv)
+ log.Printf("env: %v", hw.envv)
+
+ // TODO(rjkroege): Insert the actual code to change uid in a subsquent CL.
+ return syscall.Exec(hw.argv0, hw.argv, hw.envv)
+}
diff --git a/services/mgmt/suidhelper/main.go b/services/mgmt/suidhelper/main.go
new file mode 100644
index 0000000..c0b2970
--- /dev/null
+++ b/services/mgmt/suidhelper/main.go
@@ -0,0 +1,20 @@
+package main
+
+// suidhelper should be installed setuid root. Having done this, it will
+// run the provided command as the specified user identity.
+// suidhelper deliberately attempts to be as simple as possible to
+// simplify reviewing it for security concerns.
+
+import (
+ "flag"
+ "os"
+
+ "veyron.io/veyron/veyron/services/mgmt/suidhelper/impl"
+)
+
+func main() {
+ flag.Parse()
+ if err := impl.Run(os.Environ()); err != nil {
+ flag.Usage()
+ }
+}