veyron/services/mgmt/suidhelper/impl: chown, setuid
Previously, all the setuid helper code has not actually called chown and
setuid when possible. This CL adds the actual root-requiring system calls.
Change-Id: I2966fff6b322611c5fae27a6c3969631f608262c
diff --git a/services/mgmt/suidhelper/impl/args.go b/services/mgmt/suidhelper/impl/args.go
index 3178e1e..2c9da35 100644
--- a/services/mgmt/suidhelper/impl/args.go
+++ b/services/mgmt/suidhelper/impl/args.go
@@ -13,14 +13,15 @@
)
type WorkParameters struct {
- uid uint32
- gid uint32
+ uid int
+ gid int
workspace string
stderrLog string
stdoutLog string
argv0 string
argv []string
envv []string
+ dryrun bool
}
type ArgsSavedForTest struct {
@@ -67,18 +68,18 @@
return fmt.Errorf("--username %s: unknown user", username)
}
- uid, err := strconv.ParseUint(usr.Uid, 0, 32)
+ uid, err := strconv.ParseInt(usr.Uid, 0, 32)
if err != nil {
return fmt.Errorf("user.Lookup() returned an invalid uid %v", usr.Uid)
}
- gid, err := strconv.ParseUint(usr.Gid, 0, 32)
+ gid, err := strconv.ParseInt(usr.Gid, 0, 32)
if err != nil {
return fmt.Errorf("user.Lookup() returned an invalid gid %v", usr.Gid)
}
// Uids less than 501 can be special so we forbid running as them.
- if uint32(uid) < uint32(*flagMinimumUid) {
- return fmt.Errorf("suidhelper does not permit uids less than %d", uint32(*flagMinimumUid))
+ if uid < *flagMinimumUid {
+ return fmt.Errorf("suidhelper does not permit uids less than %d", *flagMinimumUid)
}
// Preserve the arguments for examination by the test harness if executed
@@ -94,10 +95,11 @@
StderrLog: *flagStderrLog,
})
env = append(env, SavedArgs+"="+b.String())
+ wp.dryrun = true
}
- wp.uid = uint32(uid)
- wp.gid = uint32(gid)
+ wp.uid = int(uid)
+ wp.gid = int(gid)
wp.workspace = *flagWorkspace
wp.argv0 = *flagRun
wp.stdoutLog = *flagStdoutLog
diff --git a/services/mgmt/suidhelper/impl/system.go b/services/mgmt/suidhelper/impl/system.go
index d3e9f3c..52d96b3 100644
--- a/services/mgmt/suidhelper/impl/system.go
+++ b/services/mgmt/suidhelper/impl/system.go
@@ -3,27 +3,43 @@
package impl
import (
+ "fmt"
"log"
+ "os"
"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 %d %s\n", hw.uid, hw.workspace)
- log.Printf("chown %d %s\n", hw.uid, hw.stdoutLog)
- log.Printf("chown %d %s\n", hw.uid, hw.stderrLog)
+ chown := func(path string) error {
+ if hw.dryrun {
+ log.Printf("[dryrun] os.Chown(%s, %d, %d)", path, hw.uid, hw.gid)
+ return nil
+ }
+ return os.Chown(path, hw.uid, hw.gid)
+ }
+ for _, p := range []string{hw.workspace, hw.stdoutLog, hw.stderrLog} {
+ // TODO(rjkroege): Ensure that the nodemanager can read log entries.
+ if err := chown(p); err != nil {
+ return fmt.Errorf("os.Chown(%s, %d, %d) failed: %v", p, hw.uid, hw.gid, err)
+ }
+ }
return nil
}
func (hw *WorkParameters) Exec() error {
- log.Printf("should be Exec-ing work parameters: %#v\n", hw)
- log.Printf("su %d\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.
+ if hw.dryrun {
+ log.Printf("[dryrun] syscall.Setuid(%d)", hw.uid)
+ log.Printf("[dryrun] syscall.Setgid(%d)", hw.gid)
+ } else {
+ if err := syscall.Setuid(hw.uid); err != nil {
+ return fmt.Errorf("syscall.Setuid(%d) failed: %v", hw.uid, err)
+ }
+ if err := syscall.Setgid(hw.gid); err != nil {
+ return fmt.Errorf("syscall.Setgid(%d) failed: %v", hw.gid, err)
+ }
+ }
return syscall.Exec(hw.argv0, hw.argv, hw.envv)
}
