veyron/lib/exec: adding support for passing callback endpoint and
nonce to the child process
Change-Id: Ia161ac70bac47f4f3d06de792a21391825ea2dc3
diff --git a/lib/exec/child.go b/lib/exec/child.go
index 676beb2..f200751 100644
--- a/lib/exec/child.go
+++ b/lib/exec/child.go
@@ -1,54 +1,81 @@
package exec
import (
+ "encoding/binary"
"errors"
+ "io"
"os"
)
var (
ErrNoVersion = errors.New(versionVariable + " environment variable missing")
- ErrUnsupportedVersion = errors.New("Unsupported version of veyron/runtimes/google/lib/exec request by " + versionVariable + " environment variable")
+ ErrUnsupportedVersion = errors.New("Unsupported version of veyron/lib/exec request by " + versionVariable + " environment variable")
)
type ChildHandle struct {
- // A Secret passed to the child by its parent via a trusted channel
- Secret string
+ // Endpoint is a callback endpoint that can be use to notify the
+ // parent that the child has started up successfully via the
+ // Callback() RPC.
+ Endpoint string
+ // ID is a callback ID that can be used by a parent to identify this
+ // child when the child invokes the Callback() RPC using the
+ // callback endpoint.
+ ID string
+ // Secret is a secret passed to the child by its parent via a
+ // trusted channel.
+ Secret string
+ // statusPipe is a pipe that is used to notify the parent that the
+ // child process has started successfully. Unlike the Callback()
+ // RPC, which is to be invoked by the application to notify the
+ // parent that the application is "ready", the statusPipe is to be
+ // invoked by the veyron framework to notify the parent that the
+ // child process has successfully started.
statusPipe *os.File
}
-// fileOffset accounts for the file descriptors that are always passed to the
-// child by the parent: stderr, stdin, stdout, token read, and status write.
-// Any extra files added by the client will follow fileOffset.
+// fileOffset accounts for the file descriptors that are always passed
+// to the child by the parent: stderr, stdin, stdout, data read, and
+// status write. Any extra files added by the client will follow
+// fileOffset.
const fileOffset = 5
// NewChildHandle creates a new ChildHandle that can be used to signal
// that the child is 'ready' (by calling SetReady) to its parent. The
-// value of the ChildHandle's Secret securely passed to it by the parent; this
-// is intended for subsequent use to create a secure communication channels
-// and or authentication.
+// value of the ChildHandle's Secret securely passed to it by the
+// parent; this is intended for subsequent use to create a secure
+// communication channels and or authentication.
//
-// If the child is relying on exec.Cmd.ExtraFiles then its first file descriptor
-// will not be 3, but will be offset by extra files added by the framework. The
-// developer should use the NewExtraFile method to robustly get their extra
-// files with the correct offset applied.
+// If the child is relying on exec.Cmd.ExtraFiles then its first file
+// descriptor will not be 3, but will be offset by extra files added
+// by the framework. The developer should use the NewExtraFile method
+// to robustly get their extra files with the correct offset applied.
func NewChildHandle() (*ChildHandle, error) {
switch os.Getenv(versionVariable) {
case "":
return nil, ErrNoVersion
case version1:
- // TODO(cnicolaou): need to use major.minor.build format for version #s
+ // TODO(cnicolaou): need to use major.minor.build format for
+ // version #s.
default:
return nil, ErrUnsupportedVersion
}
- tokenPipe := os.NewFile(3, "token_rd")
-
- buf := make([]byte, MaxSecretSize)
- n, err := tokenPipe.Read(buf)
+ dataPipe := os.NewFile(3, "data_rd")
+ endpoint, err := readData(dataPipe)
+ if err != nil {
+ return nil, err
+ }
+ id, err := readData(dataPipe)
+ if err != nil {
+ return nil, err
+ }
+ secret, err := readData(dataPipe)
if err != nil {
return nil, err
}
c := &ChildHandle{
- Secret: string(buf[:n]),
+ Endpoint: endpoint,
+ ID: id,
+ Secret: secret,
statusPipe: os.NewFile(4, "status_wr"),
}
return c, nil
@@ -67,3 +94,19 @@
func (c *ChildHandle) NewExtraFile(i uintptr, name string) *os.File {
return os.NewFile(i+fileOffset, name)
}
+
+func readData(r io.Reader) (string, error) {
+ var l int64 = 0
+ if err := binary.Read(r, binary.BigEndian, &l); err != nil {
+ return "", err
+ }
+ var data []byte = make([]byte, l)
+ if n, err := r.Read(data); err != nil || int64(n) != l {
+ if err != nil {
+ return "", err
+ } else {
+ return "", errors.New("partial read")
+ }
+ }
+ return string(data), nil
+}
diff --git a/lib/exec/example_test.go b/lib/exec/example_test.go
index 8e828ca..e5eeb5a 100644
--- a/lib/exec/example_test.go
+++ b/lib/exec/example_test.go
@@ -17,7 +17,7 @@
func ExampleParentHandle() {
cmd := exec.Command("/bin/hostname")
- ph := NewParentHandle(cmd, "secret")
+ ph := NewParentHandle(cmd, SecretOpt("secret"))
// Start the child process.
if err := ph.Start(); err != nil {
diff --git a/lib/exec/exec_test.go b/lib/exec/exec_test.go
index d84ab8b..4da8b5b 100644
--- a/lib/exec/exec_test.go
+++ b/lib/exec/exec_test.go
@@ -15,9 +15,10 @@
"veyron/runtimes/google/testing/timekeeper"
)
-// We always expect there to be exactly three open file descriptors when the
-// test starts out: STDIN, STDOUT, and STDERR. This assumption is tested
-// in init below, and in the rare cases where it's wrong, we bail out.
+// We always expect there to be exactly three open file descriptors
+// when the test starts out: STDIN, STDOUT, and STDERR. This
+// assumption is tested in init below, and in the rare cases where it
+// is wrong, we bail out.
const baselineOpenFiles = 3
func init() {
@@ -25,14 +26,18 @@
return
}
if want, got := baselineOpenFiles, openFiles(); want != got {
- panic(fmt.Errorf("Test expected to start with %d open files, found %d instead.\nThis can happen if parent process has any open file descriptors, e.g. pipes, that are being inherited.", want, got))
+ message := `Test expected to start with %d open files, found %d instead.
+This can happen if parent process has any open file descriptors,
+e.g. pipes, that are being inherited.`
+ panic(fmt.Errorf(message, want, got))
}
}
-// These tests need to run a subprocess and we reuse this same test binary
-// to do so. A fake test 'TestHelperProcess' contains the code we need to
-// run in the child and we simply run this same binary with a test.run= arg
-// that runs just that test. This idea was taken from the tests for os/exec.
+// These tests need to run a subprocess and we reuse this same test
+// binary to do so. A fake test 'TestHelperProcess' contains the code
+// we need to run in the child and we simply run this same binary with
+// a test.run= arg that runs just that test. This idea was taken from
+// the tests for os/exec.
func helperCommand(s ...string) *exec.Cmd {
cs := []string{"-test.run=TestHelperProcess", "--"}
cs = append(cs, s...)
@@ -94,19 +99,55 @@
panic("unreachable")
}
-func TestAuthExchange(t *testing.T) {
- cmd := helperCommand("testAuth")
+func TestEndpointExchange(t *testing.T) {
+ cmd := helperCommand("testEndpoint")
stderr, _ := cmd.StderrPipe()
- ph := vexec.NewParentHandle(cmd, "dummy_secret")
+ ph := vexec.NewParentHandle(cmd, vexec.CallbackEndpointOpt("dummy_endpoint"))
err := ph.Start()
if err != nil {
- t.Fatalf("testAuthTest: start: %v", err)
+ t.Fatalf("testEndpointTest: start: %v", err)
+ }
+ if !expectMessage(stderr, "dummy_endpoint") {
+ t.Errorf("unexpected output from child")
+ } else {
+ if err = cmd.Wait(); err != nil {
+ t.Errorf("testEndpointTest: wait: %v", err)
+ }
+ }
+ clean(t, ph)
+}
+
+func TestIDExchange(t *testing.T) {
+ cmd := helperCommand("testID")
+ stderr, _ := cmd.StderrPipe()
+ ph := vexec.NewParentHandle(cmd, vexec.CallbackIDOpt("dummy_id"))
+ err := ph.Start()
+ if err != nil {
+ t.Fatalf("testIDTest: start: %v", err)
+ }
+ if !expectMessage(stderr, "dummy_id") {
+ t.Errorf("unexpected output from child")
+ } else {
+ if err = cmd.Wait(); err != nil {
+ t.Errorf("testIDTest: wait: %v", err)
+ }
+ }
+ clean(t, ph)
+}
+
+func TestSecretExchange(t *testing.T) {
+ cmd := helperCommand("testSecret")
+ stderr, _ := cmd.StderrPipe()
+ ph := vexec.NewParentHandle(cmd, vexec.SecretOpt("dummy_secret"))
+ err := ph.Start()
+ if err != nil {
+ t.Fatalf("testSecretTest: start: %v", err)
}
if !expectMessage(stderr, "dummy_secret") {
t.Errorf("unexpected output from child")
} else {
if err = cmd.Wait(); err != nil {
- t.Errorf("testAuthTest: wait: %v", err)
+ t.Errorf("testSecretTest: wait: %v", err)
}
}
clean(t, ph)
@@ -134,7 +175,7 @@
if err != nil {
t.Fatalf("%s: failed to get stderr pipe\n", name)
}
- ph := vexec.NewParentHandle(cmd, "")
+ ph := vexec.NewParentHandle(cmd)
if err := waitForReady(t, cmd, name, 4, ph); err != nil {
t.Errorf("%s: WaitForReady: %v (%v)", name, err, ph)
return nil
@@ -169,7 +210,7 @@
cmd[i].ExtraFiles = append(cmd[i].ExtraFiles, controlRead)
stderr[i], _ = cmd[i].StderrPipe()
tk := timekeeper.NewManualTime()
- ph[i] = vexec.NewParentHandle(cmd[i], "", vexec.TimeKeeperOpt{tk})
+ ph[i] = vexec.NewParentHandle(cmd[i], vexec.TimeKeeperOpt{tk})
done.Add(1)
go func() {
// For simulated slow children, wait until the parent
@@ -221,7 +262,7 @@
result := "never ready"
cmd := helperCommand(name)
stderr, _ := cmd.StderrPipe()
- ph := vexec.NewParentHandle(cmd, "")
+ ph := vexec.NewParentHandle(cmd)
err := waitForReady(t, cmd, name, 1, ph)
if err != vexec.ErrTimeout {
t.Errorf("Failed to get timeout: got %v\n", err)
@@ -247,7 +288,7 @@
cmd.ExtraFiles = append(cmd.ExtraFiles, controlRead)
stderr, _ := cmd.StderrPipe()
tk := timekeeper.NewManualTime()
- ph := vexec.NewParentHandle(cmd, "", vexec.TimeKeeperOpt{tk})
+ ph := vexec.NewParentHandle(cmd, vexec.TimeKeeperOpt{tk})
defer clean(t, ph)
defer controlWrite.Close()
defer controlRead.Close()
@@ -288,7 +329,7 @@
t.Fatalf("%s: failed to get stderr pipe", name)
}
tk := timekeeper.NewManualTime()
- ph := vexec.NewParentHandle(cmd, "", vexec.TimeKeeperOpt{tk})
+ ph := vexec.NewParentHandle(cmd, vexec.TimeKeeperOpt{tk})
defer clean(t, ph)
defer controlWrite.Close()
defer controlRead.Close()
@@ -457,7 +498,21 @@
}()
r := <-rc
os.Exit(r)
- case "testAuth":
+ case "testEndpoint":
+ ch, err := vexec.NewChildHandle()
+ if err != nil {
+ log.Fatalf("%v", err)
+ } else {
+ fmt.Fprintf(os.Stderr, "%s", ch.Endpoint)
+ }
+ case "testID":
+ ch, err := vexec.NewChildHandle()
+ if err != nil {
+ log.Fatalf("%s", err)
+ } else {
+ fmt.Fprintf(os.Stderr, "%s", ch.ID)
+ }
+ case "testSecret":
ch, err := vexec.NewChildHandle()
if err != nil {
log.Fatalf("%s", err)
diff --git a/lib/exec/parent.go b/lib/exec/parent.go
index 72a730a..2a77924 100644
--- a/lib/exec/parent.go
+++ b/lib/exec/parent.go
@@ -1,7 +1,9 @@
package exec
import (
+ "encoding/binary"
"errors"
+ "io"
"os"
"os/exec"
"syscall"
@@ -23,6 +25,8 @@
// A ParentHandle is the Parent process' means of managing a single child.
type ParentHandle struct {
c *exec.Cmd
+ endpoint string
+ id string
secret string
statusRead *os.File
statusWrite *os.File
@@ -31,10 +35,33 @@
// ParentHandleOpt is an option for NewParentHandle.
type ParentHandleOpt interface {
- // ExecParentHandleOpt is a signature 'dummy' method for the interface.
+ // ExecParentHandleOpt is a signature 'dummy' method for the
+ // interface.
ExecParentHandleOpt()
}
+// CallbackEndpointOpt can be used to seed the parent handle with a
+// custom callback endpoint.
+type CallbackEndpointOpt string
+
+// ExecParentHandleOpt makes CallbackEndpointOpt an instance of
+// ParentHandleOpt.
+func (cno CallbackEndpointOpt) ExecParentHandleOpt() {}
+
+// CallbackIDOpt can be used to seed the parent handle with a
+// custom callback ID.
+type CallbackIDOpt string
+
+// ExecParentHandleOpt makes CallbackIDOpt an instance of
+// ParentHandleOpt.
+func (cno CallbackIDOpt) ExecParentHandleOpt() {}
+
+// SecretOpt can be used to seed the parent handle with a custom secret.
+type SecretOpt string
+
+// ExecParentHandleOpt makes SecretOpt an instance of ParentHandleOpt.
+func (so SecretOpt) ExecParentHandleOpt() {}
+
// TimeKeeperOpt can be used to seed the parent handle with a custom timekeeper.
type TimeKeeperOpt struct {
timekeeper.TimeKeeper
@@ -45,69 +72,82 @@
// NewParentHandle creates a ParentHandle for the child process represented by
// an instance of exec.Cmd.
-func NewParentHandle(c *exec.Cmd, secret string, opts ...ParentHandleOpt) *ParentHandle {
+func NewParentHandle(c *exec.Cmd, opts ...ParentHandleOpt) *ParentHandle {
c.Env = append(c.Env, versionVariable+"="+version1)
- if len(secret) == 0 {
- secret = emptySecret
- }
- var tk timekeeper.TimeKeeper
+ endpoint, id, secret := emptyEndpoint, emptyID, emptySecret
+ tk := timekeeper.RealTime()
for _, opt := range opts {
switch v := opt.(type) {
+ case CallbackEndpointOpt:
+ endpoint = string(v)
+ case CallbackIDOpt:
+ id = string(v)
+ case SecretOpt:
+ secret = string(v)
case TimeKeeperOpt:
tk = v
default:
vlog.Errorf("Unrecognized parent option: %v", v)
}
}
- if tk == nil {
- tk = timekeeper.RealTime()
- }
return &ParentHandle{
- c: c,
- secret: secret,
- tk: tk,
+ c: c,
+ endpoint: endpoint,
+ id: id,
+ secret: secret,
+ tk: tk,
}
}
// Start starts the child process, sharing a secret with it and
// setting up a communication channel over which to read its status.
func (p *ParentHandle) Start() error {
- if len(p.secret) > MaxSecretSize {
- return ErrSecretTooLarge
- }
if parent.BlackboxTest(p.c.Env) {
if err := parent.InitBlackboxParent(p.c); err != nil {
return err
}
}
- tokenRead, tokenWrite, err := os.Pipe()
+ // Create anonymous pipe for communicating data between the child
+ // and the parent.
+ dataRead, dataWrite, err := os.Pipe()
if err != nil {
return err
}
- defer tokenWrite.Close()
- defer tokenRead.Close()
-
+ defer dataRead.Close()
+ defer dataWrite.Close()
statusRead, statusWrite, err := os.Pipe()
if err != nil {
return err
}
p.statusRead = statusRead
p.statusWrite = statusWrite
-
+ // Add the parent-child pipes to cmd.ExtraFiles, offsetting all
+ // existing file descriptors accordingly.
extraFiles := make([]*os.File, len(p.c.ExtraFiles)+2)
- extraFiles[0] = tokenRead
+ extraFiles[0] = dataRead
extraFiles[1] = statusWrite
for i, _ := range p.c.ExtraFiles {
extraFiles[i+2] = p.c.ExtraFiles[i]
}
p.c.ExtraFiles = extraFiles
+ // Start the child process.
if err := p.c.Start(); err != nil {
p.statusWrite.Close()
p.statusRead.Close()
return err
}
-
- if _, err = tokenWrite.Write([]byte(p.secret)); err != nil {
+ // Pass data to the child using a pipe.
+ if err := writeData(dataWrite, p.endpoint); err != nil {
+ p.statusWrite.Close()
+ p.statusRead.Close()
+ return err
+ }
+ if err := writeData(dataWrite, p.id); err != nil {
+ p.statusWrite.Close()
+ p.statusRead.Close()
+ return err
+ }
+ if err := writeData(dataWrite, p.secret); err != nil {
p.statusWrite.Close()
p.statusRead.Close()
return err
@@ -209,3 +249,18 @@
}
return p.c.Wait()
}
+
+func writeData(w io.Writer, data string) error {
+ l := len(data)
+ if err := binary.Write(w, binary.BigEndian, int64(l)); err != nil {
+ return err
+ }
+ if n, err := w.Write([]byte(data)); err != nil || n != l {
+ if err != nil {
+ return err
+ } else {
+ return errors.New("partial write")
+ }
+ }
+ return nil
+}
diff --git a/lib/exec/shared.go b/lib/exec/shared.go
index 0ab1417..cf31731 100644
--- a/lib/exec/shared.go
+++ b/lib/exec/shared.go
@@ -1,12 +1,12 @@
package exec
const (
- MaxSecretSize = 8192
-
version1 = "1.0.0"
readyStatus = "ready"
initStatus = "init"
versionVariable = "VEYRON_EXEC_VERSION"
- emptySecret = "emptySecret"
+ emptyEndpoint = "emptyEndpoint"
+ emptyID = "emptyID"
+ emptySecret = "emptySecret"
)
diff --git a/services/mgmt/node/impl/invoker.go b/services/mgmt/node/impl/invoker.go
index 83cfc66..a50c6ba 100644
--- a/services/mgmt/node/impl/invoker.go
+++ b/services/mgmt/node/impl/invoker.go
@@ -360,7 +360,7 @@
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
- handle := vexec.NewParentHandle(cmd, "")
+ handle := vexec.NewParentHandle(cmd)
if err := handle.Start(); err != nil {
vlog.Errorf("Start() failed: %v", err)
return errOperationFailed
