Merge "services/mgmt/device/impl: fix leaking processes"
diff --git a/profiles/internal/naming/namespace/glob.go b/profiles/internal/naming/namespace/glob.go
index 451dc5b..e81d052 100644
--- a/profiles/internal/naming/namespace/glob.go
+++ b/profiles/internal/naming/namespace/glob.go
@@ -146,7 +146,7 @@
return strings.Count(name, "/") + 1
}
-// globLoop fires off a go routine for each server and read backs replies.
+// globLoop fires off a go routine for each server and reads backs replies.
func (ns *namespace) globLoop(ctx *context.T, e *naming.MountEntry, prefix string, pattern *glob.Glob, reply chan naming.GlobReply, tr *tracks, opts []rpc.CallOpt) {
defer close(reply)
@@ -159,77 +159,72 @@
// root of the search and the full pattern. It will be the first task fired off in the for
// loop that follows.
replies <- &task{me: e, pattern: pattern}
- inFlight := 0
+ replies <- nil
+ inFlight := 1
// Perform a parallel search of the name graph. Each task will send what it learns
// on the replies channel. If the reply is a mount point and the pattern is not completely
// fulfilled, a new task will be fired off to handle it.
- for {
- select {
- case t := <-replies:
- // A nil reply represents a successfully terminated task.
- // If no tasks are running, return.
- if t == nil {
- if inFlight--; inFlight <= 0 {
- return
- }
- continue
- }
-
- // We want to output this entry if there was a real error other than
- // "not a mount table".
- //
- // An error reply is also a terminated task.
- // If no tasks are running, return.
- if t.error != nil {
- if !notAnMT(t.error) {
- reply <- naming.GlobReplyError{naming.GlobError{Name: naming.Join(prefix, t.me.Name), Error: t.error}}
- }
- if inFlight--; inFlight <= 0 {
- return
- }
- continue
- }
-
- // If this is just an error from the mount table, pass it on.
- if t.er != nil {
- x := *t.er
- x.Name = naming.Join(prefix, x.Name)
- reply <- &naming.GlobReplyError{x}
- continue
- }
-
- // Get the pattern elements below the current path.
- suffix := pattern.Split(depth(t.me.Name))
-
- // If we've satisfied the request and this isn't the root,
- // reply to the caller.
- if suffix.Len() == 0 && t.depth != 0 {
- x := *t.me
- x.Name = naming.Join(prefix, x.Name)
- reply <- &naming.GlobReplyEntry{x}
- }
-
- // If the pattern is finished (so we're only querying about the root on the
- // remote server) and the server is not another MT, then we needn't send the
- // query on since we know the server will not supply a new address for the
- // current name.
- if suffix.Finished() {
- if !t.me.ServesMountTable {
- continue
- }
- }
-
- // If this is restricted recursive and not a mount table, don't descend into it.
- if suffix.Restricted() && suffix.Len() == 0 && !t.me.ServesMountTable {
- continue
- }
-
- // Perform a glob at the next server.
- inFlight++
- t.pattern = suffix
- go ns.globAtServer(ctx, t, replies, tr, opts)
+ for inFlight != 0 {
+ t := <-replies
+ // A nil reply represents a successfully terminated task.
+ // If no tasks are running, return.
+ if t == nil {
+ inFlight--
+ continue
}
+
+ // We want to output this entry if there was a real error other than
+ // "not a mount table".
+ //
+ // An error reply is also a terminated task.
+ // If no tasks are running, return.
+ if t.error != nil {
+ if !notAnMT(t.error) {
+ reply <- naming.GlobReplyError{naming.GlobError{Name: naming.Join(prefix, t.me.Name), Error: t.error}}
+ }
+ inFlight--
+ continue
+ }
+
+ // If this is just an error from the mount table, pass it on.
+ if t.er != nil {
+ x := *t.er
+ x.Name = naming.Join(prefix, x.Name)
+ reply <- &naming.GlobReplyError{x}
+ continue
+ }
+
+ // Get the pattern elements below the current path.
+ suffix := pattern.Split(depth(t.me.Name))
+
+ // If we've satisfied the request and this isn't the root,
+ // reply to the caller.
+ if suffix.Len() == 0 && t.depth != 0 {
+ x := *t.me
+ x.Name = naming.Join(prefix, x.Name)
+ reply <- &naming.GlobReplyEntry{x}
+ }
+
+ // If the pattern is finished (so we're only querying about the root on the
+ // remote server) and the server is not another MT, then we needn't send the
+ // query on since we know the server will not supply a new address for the
+ // current name.
+ if suffix.Finished() {
+ if !t.me.ServesMountTable {
+ continue
+ }
+ }
+
+ // If this is restricted recursive and not a mount table, don't descend into it.
+ if suffix.Restricted() && suffix.Len() == 0 && !t.me.ServesMountTable {
+ continue
+ }
+
+ // Perform a glob at the next server.
+ inFlight++
+ t.pattern = suffix
+ go ns.globAtServer(ctx, t, replies, tr, opts)
}
}
diff --git a/services/device/internal/impl/app_service.go b/services/device/internal/impl/app_service.go
index 322b8d9..546a445 100644
--- a/services/device/internal/impl/app_service.go
+++ b/services/device/internal/impl/app_service.go
@@ -668,6 +668,7 @@
// initializeSubAccessLists updates the provided acl for instance-specific ACLs
func (i *appService) initializeSubAccessLists(instanceDir string, blessings []string, acl access.Permissions) error {
for _, b := range blessings {
+ b = b + string(security.ChainSeparator) + string(security.NoExtension)
for _, tag := range access.AllTypicalTags() {
acl.Add(security.BlessingPattern(b), string(tag))
}
diff --git a/services/device/internal/impl/impl_test.go b/services/device/internal/impl/impl_test.go
index 3e8e858..326ab3e 100644
--- a/services/device/internal/impl/impl_test.go
+++ b/services/device/internal/impl/impl_test.go
@@ -1527,6 +1527,19 @@
// Install and start the app as root/self.
appID := installApp(t, selfCtx)
+ vlog.VI(2).Infof("Validate that the created app has the right permission lists.")
+ acl, _, err := appStub(appID).GetPermissions(selfCtx)
+ if err != nil {
+ t.Fatalf("GetPermissions on appID: %v failed %v", appID, err)
+ }
+ expected := make(access.Permissions)
+ for _, tag := range access.AllTypicalTags() {
+ expected[string(tag)] = access.AccessList{In: []security.BlessingPattern{"root/self/$"}}
+ }
+ if got, want := acl.Normalize(), expected.Normalize(); !reflect.DeepEqual(got, want) {
+ t.Errorf("got %#v, expected %#v", got, want)
+ }
+
// Start an instance of the app but this time it should fail: we do not
// have an associated uname for the invoking identity.
startAppExpectError(t, selfCtx, appID, verror.ErrNoAccess.ID)
@@ -1578,6 +1591,21 @@
vlog.VI(2).Infof("other attempting to run an app with access. Should succeed.")
instance2ID := startApp(t, otherCtx, appID)
verifyPingArgs(t, pingCh, testUserName, "flag-val-envelope", "env-var") // Wait until the app pings us that it's ready.
+
+ vlog.VI(2).Infof("Validate that created instance has the right permissions.")
+ expected = make(access.Permissions)
+ for _, tag := range access.AllTypicalTags() {
+ expected[string(tag)] = access.AccessList{In: []security.BlessingPattern{"root/other/$"}}
+ }
+ acl, _, err = appStub(appID, instance2ID).GetPermissions(selfCtx)
+ if err != nil {
+ t.Fatalf("GetPermissions on instance %v/%v failed: %v", appID, instance2ID, err)
+ }
+ if got, want := acl.Normalize(), expected.Normalize(); !reflect.DeepEqual(got, want) {
+ t.Errorf("got %#v, expected %#v ", got, want)
+ }
+
+ // Shutdown the app.
suspendApp(t, otherCtx, appID, instance2ID)
vlog.VI(2).Infof("Verify that Resume with the same systemName works.")
diff --git a/services/role/roled/internal/config.vdl b/services/role/roled/internal/config.vdl
index 734ca95..e940903 100644
--- a/services/role/roled/internal/config.vdl
+++ b/services/role/roled/internal/config.vdl
@@ -28,4 +28,8 @@
// string representation of a time.Duration, e.g. "24h". An empty string
// indicates that the role blessing will not expire.
Expiry string
+ // The blessings issued for this role will only be valid for
+ // communicating with peers that match at least one of these patterns.
+ // If the list is empty, all peers are allowed.
+ Peers []security.BlessingPattern
}
diff --git a/services/role/roled/internal/config.vdl.go b/services/role/roled/internal/config.vdl.go
index 0928eb7..7e23661 100644
--- a/services/role/roled/internal/config.vdl.go
+++ b/services/role/roled/internal/config.vdl.go
@@ -37,6 +37,10 @@
// string representation of a time.Duration, e.g. "24h". An empty string
// indicates that the role blessing will not expire.
Expiry string
+ // The blessings issued for this role will only be valid for
+ // communicating with peers that match at least one of these patterns.
+ // If the list is empty, all peers are allowed.
+ Peers []security.BlessingPattern
}
func (Config) __VDLReflect(struct {
diff --git a/services/role/roled/internal/role.go b/services/role/roled/internal/role.go
index ee578d4..ad1ea41 100644
--- a/services/role/roled/internal/role.go
+++ b/services/role/roled/internal/role.go
@@ -85,18 +85,26 @@
}
func caveats(ctx *context.T, config *Config) ([]security.Caveat, error) {
- if config.Expiry == "" {
- return nil, nil
+ var caveats []security.Caveat
+ if config.Expiry != "" {
+ d, err := time.ParseDuration(config.Expiry)
+ if err != nil {
+ return nil, verror.Convert(verror.ErrInternal, ctx, err)
+ }
+ expiry, err := security.ExpiryCaveat(time.Now().Add(d))
+ if err != nil {
+ return nil, verror.Convert(verror.ErrInternal, ctx, err)
+ }
+ caveats = append(caveats, expiry)
}
- d, err := time.ParseDuration(config.Expiry)
- if err != nil {
- return nil, verror.Convert(verror.ErrInternal, ctx, err)
+ if len(config.Peers) != 0 {
+ peer, err := security.NewCaveat(security.PeerBlessingsCaveat, config.Peers)
+ if err != nil {
+ return nil, verror.Convert(verror.ErrInternal, ctx, err)
+ }
+ caveats = append(caveats, peer)
}
- expiry, err := security.ExpiryCaveat(time.Now().Add(d))
- if err != nil {
- return nil, verror.Convert(verror.ErrInternal, ctx, err)
- }
- return []security.Caveat{expiry}, nil
+ return caveats, nil
}
func createBlessings(ctx *context.T, config *Config, principal security.Principal, extensions []string, caveats []security.Caveat, dischargerLocation string) (security.Blessings, error) {
diff --git a/services/role/roled/internal/role_test.go b/services/role/roled/internal/role_test.go
index 483279f..30d927d 100644
--- a/services/role/roled/internal/role_test.go
+++ b/services/role/roled/internal/role_test.go
@@ -109,16 +109,91 @@
if verror.ErrorID(err) != tc.errID {
t.Errorf("unexpected error ID for (%q, %q). Got %#v, expected %#v", user, tc.role, verror.ErrorID(err), tc.errID)
}
- if err == nil {
- previousBlessings, _ := v23.GetPrincipal(tc.ctx).BlessingStore().Set(blessings, security.AllPrincipals)
- blessingNames, rejected := callTest(t, tc.ctx, testAddr)
- if !reflect.DeepEqual(blessingNames, tc.blessings) {
- t.Errorf("unexpected blessings for (%q, %q). Got %q, expected %q", user, tc.role, blessingNames, tc.blessings)
- }
- if len(rejected) != 0 {
- t.Errorf("unexpected rejected blessings for (%q, %q): %q", user, tc.role, rejected)
- }
- v23.GetPrincipal(tc.ctx).BlessingStore().Set(previousBlessings, security.AllPrincipals)
+ if err != nil {
+ continue
+ }
+ previousBlessings, _ := v23.GetPrincipal(tc.ctx).BlessingStore().Set(blessings, security.AllPrincipals)
+ blessingNames, rejected := callTest(t, tc.ctx, testAddr)
+ if !reflect.DeepEqual(blessingNames, tc.blessings) {
+ t.Errorf("unexpected blessings for (%q, %q). Got %q, expected %q", user, tc.role, blessingNames, tc.blessings)
+ }
+ if len(rejected) != 0 {
+ t.Errorf("unexpected rejected blessings for (%q, %q): %q", user, tc.role, rejected)
+ }
+ v23.GetPrincipal(tc.ctx).BlessingStore().Set(previousBlessings, security.AllPrincipals)
+ }
+}
+
+func TestPeerBlessingCaveats(t *testing.T) {
+ ctx, shutdown := v23.Init()
+ defer shutdown()
+
+ workdir, err := ioutil.TempDir("", "test-role-server-")
+ if err != nil {
+ t.Fatal("ioutil.TempDir failed: %v", err)
+ }
+ defer os.RemoveAll(workdir)
+
+ roleConf := irole.Config{
+ Members: []security.BlessingPattern{"root/users/user/_role"},
+ Peers: []security.BlessingPattern{
+ security.BlessingPattern("root/peer1"),
+ security.BlessingPattern("root/peer3"),
+ },
+ }
+ irole.WriteConfig(t, roleConf, filepath.Join(workdir, "role.conf"))
+
+ var (
+ root = testutil.NewIDProvider("root")
+ user = newPrincipalContext(t, ctx, root, "users/user/_role")
+ peer1 = newPrincipalContext(t, ctx, root, "peer1")
+ peer2 = newPrincipalContext(t, ctx, root, "peer2")
+ peer3 = newPrincipalContext(t, ctx, root, "peer3")
+ )
+
+ roleAddr := newRoleServer(t, newPrincipalContext(t, ctx, root, "roles"), workdir)
+
+ tDisp := &testDispatcher{}
+ server1, testPeer1 := newServer(t, peer1)
+ if err := server1.ServeDispatcher("", tDisp); err != nil {
+ t.Fatalf("server.ServeDispatcher failed: %v", err)
+ }
+ server2, testPeer2 := newServer(t, peer2)
+ if err := server2.ServeDispatcher("", tDisp); err != nil {
+ t.Fatalf("server.ServeDispatcher failed: %v", err)
+ }
+ server3, testPeer3 := newServer(t, peer3)
+ if err := server3.ServeDispatcher("", tDisp); err != nil {
+ t.Fatalf("server.ServeDispatcher failed: %v", err)
+ }
+
+ c := role.RoleClient(naming.Join(roleAddr, "role"))
+ blessings, err := c.SeekBlessings(user)
+ if err != nil {
+ t.Errorf("unexpected erro:", err)
+ }
+ v23.GetPrincipal(user).BlessingStore().Set(blessings, security.AllPrincipals)
+
+ testcases := []struct {
+ peer string
+ blessingNames []string
+ rejectedNames []string
+ }{
+ {testPeer1, []string{"root/roles/role"}, nil},
+ {testPeer2, nil, []string{"root/roles/role"}},
+ {testPeer3, []string{"root/roles/role"}, nil},
+ }
+ for i, tc := range testcases {
+ blessingNames, rejected := callTest(t, user, tc.peer)
+ var rejectedNames []string
+ for _, r := range rejected {
+ rejectedNames = append(rejectedNames, r.Blessing)
+ }
+ if !reflect.DeepEqual(blessingNames, tc.blessingNames) {
+ t.Errorf("Unexpected blessing names for #%d. Got %q, expected %q", i, blessingNames, tc.blessingNames)
+ }
+ if !reflect.DeepEqual(rejectedNames, tc.rejectedNames) {
+ t.Errorf("Unexpected rejected names for #%d. Got %q, expected %q", i, rejectedNames, tc.rejectedNames)
}
}
}
