mounttable: fix Resolve access
In all cases Admin implies all rights.
On a directory
- Read means that a client can see every object in the directory,
regardless of the object access rights. However, if the client
doesn't have Read, Admin, or Resolve rights to the object, it can't see
the object's servers or ACLs.
- Resolve means that a client can see every object in the directory
to which it has any access rights. If it has Resolve rights to an
object, it can also see the Servers associated with it.
On an object
- Resolve means that a client can see the Servers associated with it.
Change-Id: I3367bf037ce66cc6101dde1635f5c7d311a98d77
diff --git a/profiles/internal/naming/namespace/acl_test.go b/profiles/internal/naming/namespace/acl_test.go
index 796668e..e2a9d65 100644
--- a/profiles/internal/naming/namespace/acl_test.go
+++ b/profiles/internal/naming/namespace/acl_test.go
@@ -7,6 +7,7 @@
"v.io/v23"
"v.io/v23/context"
+ "v.io/v23/ipc"
"v.io/v23/naming"
"v.io/v23/security"
"v.io/v23/services/security/access"
@@ -59,10 +60,53 @@
return stopFunc, estr
}
+type nopServer struct{ x int }
+
+func (s *nopServer) NOP(call ipc.ServerCall) error {
+ return nil
+}
+
+var nobody = []security.BlessingPattern{""}
+var everybody = []security.BlessingPattern{"..."}
+var closedACL = access.TaggedACLMap{
+ "Resolve": access.ACL{
+ In: nobody,
+ },
+ "Read": access.ACL{
+ In: nobody,
+ },
+ "Admin": access.ACL{
+ In: nobody,
+ },
+ "Create": access.ACL{
+ In: nobody,
+ },
+ "Mount": access.ACL{
+ In: nobody,
+ },
+}
+var openACL = access.TaggedACLMap{
+ "Resolve": access.ACL{
+ In: everybody,
+ },
+ "Read": access.ACL{
+ In: everybody,
+ },
+ "Admin": access.ACL{
+ In: everybody,
+ },
+ "Create": access.ACL{
+ In: everybody,
+ },
+ "Mount": access.ACL{
+ In: everybody,
+ },
+}
+
func TestACLs(t *testing.T) {
// Create three different personalities.
// TODO(p): Use the multiple personalities to test ACL functionality.
- rootCtx, _, _, shutdown := initTest()
+ rootCtx, aliceCtx, _, shutdown := initTest()
defer shutdown()
// Create root mounttable.
@@ -93,46 +137,103 @@
if err != nil {
t.Fatalf("GetACL a/b/c: %s", err)
}
- acl = access.TaggedACLMap{"Read": access.ACL{In: []security.BlessingPattern{security.AllPrincipals}}}
- if err := ns.SetACL(rootCtx, "a/b/c", acl, etag); err != nil {
+ if err := ns.SetACL(rootCtx, "a/b/c", openACL, etag); err != nil {
t.Fatalf("SetACL a/b/c: %s", err)
}
nacl, _, err := ns.GetACL(rootCtx, "a/b/c")
if err != nil {
t.Fatalf("GetACL a/b/c: %s", err)
}
- if !reflect.DeepEqual(acl, nacl) {
- t.Fatalf("want %v, got %v", acl, nacl)
+ if !reflect.DeepEqual(openACL, nacl) {
+ t.Fatalf("want %v, got %v", openACL, nacl)
}
// Now Set/Get the parallel mount point's ACL.
+ name := "a/b/c/d/e"
etag = "" // Parallel setacl with any other value is dangerous
- acl = access.TaggedACLMap{"Read": access.ACL{In: []security.BlessingPattern{security.AllPrincipals}},
- "Admin": access.ACL{In: []security.BlessingPattern{security.AllPrincipals}}}
- if err := ns.SetACL(rootCtx, "a/b/c/d/e", acl, etag); err != nil {
- t.Fatalf("SetACL a/b/c/d/e: %s", err)
+ if err := ns.SetACL(rootCtx, name, openACL, etag); err != nil {
+ t.Fatalf("SetACL %s: %s", name, err)
}
- nacl, _, err = ns.GetACL(rootCtx, "a/b/c/d/e")
+ nacl, _, err = ns.GetACL(rootCtx, name)
if err != nil {
- t.Fatalf("GetACL a/b/c/d/e: %s", err)
+ t.Fatalf("GetACL %s: %s", name, err)
}
- if !reflect.DeepEqual(acl, nacl) {
- t.Fatalf("want %v, got %v", acl, nacl)
+ if !reflect.DeepEqual(openACL, nacl) {
+ t.Fatalf("want %v, got %v", openACL, nacl)
}
// Get from each server individually to make sure both are set.
- nacl, _, err = ns.GetACL(rootCtx, naming.Join(mt1Addr, "d/e"))
+ name = naming.Join(mt1Addr, "d/e")
+ nacl, _, err = ns.GetACL(rootCtx, name)
if err != nil {
- t.Fatalf("GetACL a/b/c/d/e: %s", err)
+ t.Fatalf("GetACL %s: %s", name, err)
}
- if !reflect.DeepEqual(acl, nacl) {
+ if !reflect.DeepEqual(openACL, nacl) {
+ t.Fatalf("want %v, got %v", openACL, nacl)
+ }
+ name = naming.Join(mt2Addr, "d/e")
+ nacl, _, err = ns.GetACL(rootCtx, name)
+ if err != nil {
+ t.Fatalf("GetACL %s: %s", name, err)
+ }
+ if !reflect.DeepEqual(openACL, nacl) {
t.Fatalf("want %v, got %v", acl, nacl)
}
- nacl, _, err = ns.GetACL(rootCtx, naming.Join(mt2Addr, "d/e"))
- if err != nil {
- t.Fatalf("GetACL a/b/c/d/e: %s", err)
+
+ // Create mount points accessible only by root's key.
+ name = "a/b/c/d/f"
+ deadbody := "/the:8888/rain"
+ if err := ns.SetACL(rootCtx, name, closedACL, etag); err != nil {
+ t.Fatalf("SetACL %s: %s", name, err)
}
- if !reflect.DeepEqual(acl, nacl) {
- t.Fatalf("want %v, got %v", acl, nacl)
+ nacl, _, err = ns.GetACL(rootCtx, name)
+ if err != nil {
+ t.Fatalf("GetACL %s: %s", name, err)
+ }
+ if !reflect.DeepEqual(closedACL, nacl) {
+ t.Fatalf("want %v, got %v", closedACL, nacl)
+ }
+ if err := ns.Mount(rootCtx, name, deadbody, 10000); err != nil {
+ t.Fatalf("Mount %s: %s", name, err)
+ }
+
+ // Alice shouldn't be able to resolve it.
+ _, err = v23.GetNamespace(aliceCtx).Resolve(aliceCtx, name)
+ if err == nil {
+ t.Fatalf("as alice we shouldn't be able to Resolve %s", name)
+ }
+
+ // Root should be able to resolve it.
+ _, err = ns.Resolve(rootCtx, name)
+ if err != nil {
+ t.Fatalf("as root Resolve %s: %s", name, err)
+ }
+
+ // Create a mount point via Serve accessible only by root's key.
+ name = "a/b/c/d/g"
+ if err := ns.SetACL(rootCtx, name, closedACL, etag); err != nil {
+ t.Fatalf("SetACL %s: %s", name, err)
+ }
+ server, err := v23.NewServer(rootCtx)
+ if err != nil {
+ t.Fatalf("v23.NewServer failed: %v", err)
+ }
+ if _, err := server.Listen(v23.GetListenSpec(rootCtx)); err != nil {
+ t.Fatalf("Failed to Listen: %s", err)
+ }
+ if err := server.Serve(name, &nopServer{1}, nil); err != nil {
+ t.Fatalf("Failed to Serve: %s", err)
+ }
+
+ // Alice shouldn't be able to resolve it.
+ _, err = v23.GetNamespace(aliceCtx).Resolve(aliceCtx, name)
+ if err == nil {
+ t.Fatalf("as alice we shouldn't be able to Resolve %s", name)
+ }
+
+ // Root should be able to resolve it.
+ _, err = ns.Resolve(rootCtx, name)
+ if err != nil {
+ t.Fatalf("as root Resolve %s: %s", name, err)
}
}
diff --git a/services/mounttable/lib/mounttable.go b/services/mounttable/lib/mounttable.go
index 6abaf60..1e1dc51 100644
--- a/services/mounttable/lib/mounttable.go
+++ b/services/mounttable/lib/mounttable.go
@@ -27,6 +27,7 @@
createTags = []mounttable.Tag{mounttable.Create, mounttable.Admin}
removeTags = []mounttable.Tag{mounttable.Admin}
mountTags = []mounttable.Tag{mounttable.Mount, mounttable.Admin}
+ resolveTags = []mounttable.Tag{mounttable.Read, mounttable.Resolve, mounttable.Admin}
globTags = []mounttable.Tag{mounttable.Read, mounttable.Admin}
setTags = []mounttable.Tag{mounttable.Admin}
getTags = []mounttable.Tag{mounttable.Admin, mounttable.Read}
@@ -343,6 +344,12 @@
if n == nil {
return nil, nil, nil
}
+ // If we can't resolve it, we can't use it.
+ if err := n.satisfies(mt, call, resolveTags); err != nil {
+ n.parent.Unlock()
+ n.Unlock()
+ return nil, nil, err
+ }
if !n.mount.isActive() {
removed := n.removeUseless()
n.parent.Unlock()
@@ -586,9 +593,14 @@
// Don't need the parent lock anymore.
n.parent.Unlock()
me := naming.VDLMountEntry{
- Name: name,
- Servers: m.servers.copyToSlice(),
- MT: n.mount.mt,
+ Name: name,
+ }
+ // Only fill in the mount info if we can resolve this name.
+ if err := n.satisfies(mt, call, resolveTags); err == nil {
+ me.Servers = m.servers.copyToSlice()
+ me.MT = n.mount.mt
+ } else {
+ me.Servers = []naming.VDLMountedServer{}
}
// Unlock while we are sending on the channel to avoid livelock.
n.Unlock()
diff --git a/services/mounttable/lib/mounttable_test.go b/services/mounttable/lib/mounttable_test.go
index 95133aa..272ad8f 100644
--- a/services/mounttable/lib/mounttable_test.go
+++ b/services/mounttable/lib/mounttable_test.go
@@ -643,7 +643,7 @@
// blessing patterns, it will be thought of as bob's server.
doMount(t, bobCtx, mtAddr, suffix, collectionAddr, nil, true)
if e, err := resolve(aliceCtx, naming.JoinAddressName(mtAddr, suffix)); err != nil {
- t.Error(err)
+ t.Errorf("Error resolving %s: %s", naming.JoinAddressName(mtAddr, suffix), err)
} else if len(e.Servers) != 1 {
t.Errorf("Got %v, want exactly 1 server", e.Servers)
} else if got, want := e.Servers[0].BlessingPatterns, strslice("bob"); !reflect.DeepEqual(got, want) {
diff --git a/services/mounttable/lib/testdata/test.acl b/services/mounttable/lib/testdata/test.acl
index 3a4d9eb..cbd4e28 100644
--- a/services/mounttable/lib/testdata/test.acl
+++ b/services/mounttable/lib/testdata/test.acl
@@ -16,6 +16,7 @@
"Create": { "In": ["..."] }
},
"users/%%": {
- "Admin": { "In": ["%%"] }
+ "Admin": { "In": ["%%"] },
+ "Resolve": { "In": ["..."] }
}
}
