veyron/services/mgmt/node: make suid helper external to the node manager.
The path to the helper should be provided as a config option to the node manager,
and the node manager's internal organization of its root dir should remain
an implementation detail, hidden from code not in mgmt/node/impl.
Change-Id: I04d1ff66e79a2d15f608c13e746f42a83d5d97bc
diff --git a/services/mgmt/node/config/config.go b/services/mgmt/node/config/config.go
index 52eaae9..e7afa99 100644
--- a/services/mgmt/node/config/config.go
+++ b/services/mgmt/node/config/config.go
@@ -50,6 +50,9 @@
// manager is started. Node manager is expected to mutate this during
// a self-update. Must be non-empty.
CurrentLink string
+ // Helper is the path to the setuid helper for running applications as
+ // specific users.
+ Helper string
}
// Validate checks the config state.
@@ -63,6 +66,9 @@
if c.CurrentLink == "" {
return fmt.Errorf("CurrentLink cannot be empty")
}
+ if c.Helper == "" {
+ return fmt.Errorf("Helper must be specified")
+ }
return nil
}
@@ -83,6 +89,7 @@
Root: os.Getenv(RootEnv),
Origin: os.Getenv(OriginEnv),
CurrentLink: os.Getenv(CurrentLinkEnv),
+ Helper: os.Getenv(HelperEnv),
}, nil
}
@@ -104,6 +111,7 @@
RootEnv: c.Root,
OriginEnv: c.Origin,
CurrentLinkEnv: c.CurrentLink,
+ HelperEnv: c.Helper,
}
// We need to manually pass the namespace roots to the child, since we
// currently don't have a way for the child to obtain this information
diff --git a/services/mgmt/node/config/config_test.go b/services/mgmt/node/config/config_test.go
index ccaa875..5735b87 100644
--- a/services/mgmt/node/config/config_test.go
+++ b/services/mgmt/node/config/config_test.go
@@ -44,6 +44,7 @@
Root: "fidos/doghouse",
Origin: "pet/store",
CurrentLink: currLink,
+ Helper: "santas/little/helper",
}
if err := state.Validate(); err != nil {
t.Errorf("Config state %v failed to validate: %v", state, err)
@@ -83,6 +84,7 @@
Root: "b",
Origin: "c",
CurrentLink: "d",
+ Helper: "e",
}
if err := state.Validate(); err != nil {
t.Errorf("Config state %v failed to validate: %v", state, err)
@@ -99,6 +101,10 @@
if err := state.Validate(); err == nil {
t.Errorf("Config state %v should have failed to validate.", state)
}
+ state.Name, state.Helper = "anything", ""
+ if err := state.Validate(); err == nil {
+ t.Errorf("Config state %v should have failed to validate.", state)
+ }
}
// TestQuoteEnv checks the QuoteEnv method.
diff --git a/services/mgmt/node/config/const.go b/services/mgmt/node/config/const.go
index c57cd98..1c5aee5 100644
--- a/services/mgmt/node/config/const.go
+++ b/services/mgmt/node/config/const.go
@@ -18,4 +18,7 @@
// CurrentLinkEnv is the name of the environment variable that holds
// the path to the soft link that points to the current node manager.
CurrentLinkEnv = "VEYRON_NM_CURRENT"
+ // HelperEnv is the name of the environment variable that holds the path
+ // to the suid helper used to start apps as specific system users.
+ HelperEnv = "VEYRON_NM_HELPER"
)
diff --git a/services/mgmt/node/impl/app_invoker.go b/services/mgmt/node/impl/app_invoker.go
index c359a2a..e14dc1e 100644
--- a/services/mgmt/node/impl/app_invoker.go
+++ b/services/mgmt/node/impl/app_invoker.go
@@ -7,7 +7,6 @@
// 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
@@ -34,6 +33,9 @@
// app-<hash 2>
// ...
//
+// The node manager uses the suid helper binary to invoke an application as a
+// specified user. The path to the helper is specified as config.Helper.
+
// When node manager starts up, it goes through all instances and resumes the
// ones that are not suspended. If the application was still running, it
// suspends it first. If an application fails to resume, it stays suspended.
@@ -538,12 +540,12 @@
return nil
}
-func (i *appInvoker) run(instanceDir, systemName, helper string) error {
+func (i *appInvoker) run(instanceDir, systemName string) error {
if err := transitionInstance(instanceDir, suspended, starting); err != nil {
return err
}
- cmd, err := genCmd(instanceDir, helper, systemName)
+ cmd, err := genCmd(instanceDir, i.config.Helper, systemName)
if err == nil {
err = i.startCmd(instanceDir, cmd)
}
@@ -561,8 +563,7 @@
return nil, err
}
- helper := filepath.Join(i.config.Root, "helper")
- systemName, err := systemAccountForHelper(helper, call.RemoteBlessings().ForContext(call), i.uat)
+ systemName, err := systemAccountForHelper(i.config.Helper, call.RemoteBlessings().ForContext(call), i.uat)
if err != nil {
cleanupDir(instanceDir)
return nil, err
@@ -573,7 +574,7 @@
return nil, err
}
- if err = i.run(instanceDir, systemName, helper); err != nil {
+ if err = i.run(instanceDir, systemName); err != nil {
cleanupDir(instanceDir)
return nil, err
}
@@ -605,8 +606,7 @@
return err
}
- helper := filepath.Join(i.config.Root, "helper")
- systemName, err := systemAccountForHelper(helper, call.RemoteBlessings().ForContext(call), i.uat)
+ systemName, err := systemAccountForHelper(i.config.Helper, call.RemoteBlessings().ForContext(call), i.uat)
if err != nil {
return err
}
@@ -619,7 +619,7 @@
if startSystemName != systemName {
return verror.NoAccessf("Not allowed to resume an application under a different system name.")
}
- return i.run(instanceDir, systemName, helper)
+ return i.run(instanceDir, systemName)
}
func stopAppRemotely(appVON string) error {
diff --git a/services/mgmt/node/impl/impl_test.go b/services/mgmt/node/impl/impl_test.go
index efc1c5c..e520c98 100644
--- a/services/mgmt/node/impl/impl_test.go
+++ b/services/mgmt/node/impl/impl_test.go
@@ -154,10 +154,10 @@
// for example, the node manager is invoked 'by hand' instead of via a
// script prepared by a previous version of the node manager.
if len(args) > 0 {
- if want, got := 3, len(args); want != got {
+ if want, got := 4, len(args); want != got {
vlog.Fatalf("expected %d additional arguments, got %d instead", want, got)
}
- configState.Root, configState.Origin, configState.CurrentLink = args[0], args[1], args[2]
+ configState.Root, configState.Helper, configState.Origin, configState.CurrentLink = args[0], args[1], args[2], args[3]
}
dispatcher, err := impl.NewDispatcher(configState)
if err != nil {
@@ -274,7 +274,7 @@
// 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) {
+func generateSuidHelperScript(t *testing.T, root string) string {
output := "#!/bin/bash\n"
output += "VEYRON_SUIDHELPER_TEST=1"
output += " "
@@ -286,10 +286,13 @@
if err := os.MkdirAll(root, 0755); err != nil {
t.Fatalf("MkdirAll failed: %v", err)
}
- path := filepath.Join(root, "helper")
+ // Helper does not need to live under the node manager's root dir, but
+ // we put it there for convenience.
+ path := filepath.Join(root, "helper.sh")
if err := ioutil.WriteFile(path, []byte(output), 0755); err != nil {
t.Fatalf("WriteFile(%v) failed: %v", path, err)
}
+ return path
}
// readPID waits for the "ready:<PID>" line from the child and parses out the
@@ -330,7 +333,7 @@
crDir, crEnv := credentialsForChild("anyvalue")
defer os.RemoveAll(crDir)
- nmArgs := []string{"factoryNM", root, mockApplicationRepoName, currLink}
+ nmArgs := []string{"factoryNM", root, "unused_helper", mockApplicationRepoName, currLink}
args, env := sh.CommandEnvelope(nodeManagerCmd, crEnv, nmArgs...)
scriptPathFactory := generateNodeManagerScript(t, root, args, env)
@@ -560,14 +563,14 @@
defer cleanup()
// Create a script wrapping the test target that implements suidhelper.
- generateSuidHelperScript(t, root)
+ helperPath := generateSuidHelperScript(t, root)
crDir, crEnv := credentialsForChild("anyvalue")
defer os.RemoveAll(crDir)
// Set up the node manager. Since we won't do node manager updates,
// don't worry about its application envelope and current link.
- nmh, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "nm", root, "unused app repo name", "unused curr link")
+ nmh, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "nm", root, helperPath, "unused_app_rep_ name", "unused_curr_link")
readPID(t, nms)
// Create the local server that the app uses to let us know it's ready.
@@ -766,9 +769,12 @@
crDir, crEnv := credentialsForChild("anyvalue")
defer os.RemoveAll(crDir)
+ // Create a script wrapping the test target that implements suidhelper.
+ helperPath := 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.
- _, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "nm", root, "unused app repo name", "unused curr link")
+ _, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "nm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
pid := readPID(t, nms)
defer syscall.Kill(pid, syscall.SIGINT)
@@ -803,9 +809,6 @@
t.Fatalf("Install should have failed from otherRT")
}
- // Create a script wrapping the test target that implements suidhelper.
- generateSuidHelperScript(t, root)
-
// Create the local server that the app uses to let us know it's ready.
pingCh, cleanup := setupPingServer(t)
defer cleanup()
@@ -859,7 +862,7 @@
// Set up the node manager. Since we won't do node manager updates,
// don't worry about its application envelope and current link.
- _, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "nm", root, "unused app repo name", "unused curr link")
+ _, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "nm", root, "unused_helper", "unused_app_repo_name", "unused_curr_link")
pid := readPID(t, nms)
defer syscall.Kill(pid, syscall.SIGINT)
@@ -947,7 +950,7 @@
argsForNodeManager = append(argsForNodeManager, "nm")
// Add vars to instruct the installer how to configure the node manager.
- installerEnv := []string{config.RootEnv + "=" + root, config.CurrentLinkEnv + "=" + currLink}
+ installerEnv := []string{config.RootEnv + "=" + root, config.CurrentLinkEnv + "=" + currLink, config.HelperEnv + "=" + "unused"}
installerh, installers := runShellCommand(t, sh, installerEnv, installerCmd, argsForNodeManager...)
installers.ExpectEOF()
installerh.Shutdown(os.Stderr, os.Stderr)
@@ -988,15 +991,15 @@
crDir, crEnv := credentialsForChild("anyvalue")
defer os.RemoveAll(crDir)
+ // Create a script wrapping the test target that implements suidhelper.
+ helperPath := 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.
- _, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "nm", root, "unused app repo name", "unused curr link")
+ _, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "nm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
pid := readPID(t, nms)
defer syscall.Kill(pid, syscall.SIGINT)
- // Create a script wrapping the test target that implements suidhelper.
- generateSuidHelperScript(t, root)
-
// Create the local server that the app uses to let us know it's ready.
pingCh, cleanup := setupPingServer(t)
defer cleanup()
@@ -1141,7 +1144,7 @@
crFile, crEnv := credentialsForChild("anyvalue")
defer os.RemoveAll(crFile)
- _, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "nm", root, "unused app repo name", "unused curr link")
+ _, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "nm", root, "unused_helper", "unused_app_repo_name", "unused_curr_link")
pid := readPID(t, nms)
defer syscall.Kill(pid, syscall.SIGINT)
@@ -1253,9 +1256,9 @@
// Create a script wrapping the test target that implements
// suidhelper.
- generateSuidHelperScript(t, root)
+ helperPath := generateSuidHelperScript(t, root)
- _, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "-mocksetuid", "nm", root, "unused app repo name", "unused curr link")
+ _, nms := runShellCommand(t, sh, crEnv, nodeManagerCmd, "-mocksetuid", "nm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
pid := readPID(t, nms)
defer syscall.Kill(pid, syscall.SIGINT)
