security: Do not skip ACL checks when using same principal.
Syncbase is frequently run using the same principal (key pair) as
its clients. The default authorizer would skip all ACL checks in
this case, which is undesirable since it can e.g. allow the client
to put rows into a read-only collection, which would be rejected
by a remote peer when synced.
For this reason, this CL removes the short-circuit check for the
same public key and always checks the ACL.
MultiPart: 2/3
Change-Id: I18ca2dcc5227580374af460601d633b3982f3ed9
diff --git a/services/mounttable/mounttablelib/mounttable.go b/services/mounttable/mounttablelib/mounttable.go
index 8fab99a..300d578 100644
--- a/services/mounttable/mounttablelib/mounttable.go
+++ b/services/mounttable/mounttablelib/mounttable.go
@@ -231,10 +231,6 @@
if cc.ignorePerms || tags == nil || n.vPerms == nil {
return nil
}
- // "Self-RPCs" are always authorized.
- if cc.self {
- return nil
- }
// Match client's blessings against the AccessLists.
for _, tag := range tags {
if al, exists := n.vPerms.AccessListForTag(string(tag)); exists && al.Includes(cc.rbn...) {
diff --git a/services/syncbase/longevity_tests/client/util.go b/services/syncbase/longevity_tests/client/util.go
index 6febe6a..0fc49d1 100644
--- a/services/syncbase/longevity_tests/client/util.go
+++ b/services/syncbase/longevity_tests/client/util.go
@@ -117,5 +117,5 @@
// blessings.
func defaultPerms(ctx *context.T, allowedTags []access.Tag) access.Permissions {
blessings := security.DefaultBlessingNames(v23.GetPrincipal(ctx))
- return testutil.DefaultPerms(allowedTags, blessings[0])
+ return testutil.DefaultPerms(allowedTags, blessings...)
}
diff --git a/services/syncbase/longevity_tests/control/control_test.go b/services/syncbase/longevity_tests/control/control_test.go
index acbb714..c32c3f4 100644
--- a/services/syncbase/longevity_tests/control/control_test.go
+++ b/services/syncbase/longevity_tests/control/control_test.go
@@ -492,21 +492,27 @@
userAlice := &model.User{Name: "user-alice"}
userBob := &model.User{Name: "user-bob"}
- // Alice has all permissions, and gives Bob read access.
- permsAliceDb := model.Permissions{
- "Admin": model.UserSet{userAlice},
- "Read": model.UserSet{userAlice, userBob},
- "Resolve": model.UserSet{userAlice},
- "Write": model.UserSet{userAlice},
+ users := model.UserSet{userAlice, userBob}
+ permsDb := model.Permissions{
+ "Admin": users,
+ "Read": users,
+ "Resolve": users,
+ "Write": users,
}
- permsAliceCx := permsAliceDb.FilterTags(wire.AllCollectionTags...)
- permsAliceSg := permsAliceDb.FilterTags(wire.AllSyncgroupTags...)
- // Alice is creator of the database and collection.
+ // Alice has all permissions on collection, and gives Bob read access.
+ permsAliceCx := model.Permissions{
+ "Admin": model.UserSet{userAlice},
+ "Read": model.UserSet{userAlice, userBob},
+ "Write": model.UserSet{userAlice},
+ }
+ permsAliceSg := permsAliceCx.FilterTags(wire.AllSyncgroupTags...)
+
+ // Alice is creator of the collection.
dbModel := &model.Database{
Name: "test_db",
Blessing: "root",
- Permissions: permsAliceDb,
+ Permissions: permsDb,
Collections: []model.Collection{
model.Collection{
Name: "test_col",
@@ -562,7 +568,7 @@
// Check that Bob gets ErrNoAccess when writing to the collection on his
// own device because he does not have write permissions.
- bobCtx, err := c.InternalConfigureContext(c.InternalCtx(), userBob.Name)
+ bobCtx, err := c.InternalConfigureContext(c.InternalCtx(), "u"+security.ChainSeparator+userBob.Name)
if err != nil {
t.Fatal(err)
}
diff --git a/services/syncbase/server/watchlog_test.go b/services/syncbase/server/watchlog_test.go
index 29da41d..8d0bae6 100644
--- a/services/syncbase/server/watchlog_test.go
+++ b/services/syncbase/server/watchlog_test.go
@@ -13,6 +13,7 @@
"v.io/v23/security"
"v.io/v23/security/access"
wire "v.io/v23/services/syncbase"
+ "v.io/v23/vdl"
"v.io/v23/vom"
_ "v.io/x/ref/runtime/factories/generic"
"v.io/x/ref/services/syncbase/common"
@@ -27,14 +28,17 @@
type mockCall struct {
security.Call
+ p security.Principal
b security.Blessings
}
func (c *mockCall) Server() rpc.Server { return nil }
func (c *mockCall) GrantedBlessings() security.Blessings { return c.b }
func (c *mockCall) Security() security.Call { return c }
+func (c *mockCall) LocalPrincipal() security.Principal { return c.p }
func (c *mockCall) LocalBlessings() security.Blessings { return c.b }
func (c *mockCall) RemoteBlessings() security.Blessings { return c.b }
+func (c *mockCall) MethodTags() []*vdl.Value { return []*vdl.Value{vdl.ValueOf(access.Admin)} }
func putOp(st store.Store, key string) *watchable.PutOp {
version, _ := watchable.GetVersion(nil, st, []byte(key))
@@ -63,7 +67,7 @@
storedPerms := interfaces.CollectionPerms(perms)
store.Put(ctx, st, c.permsKey(), &storedPerms)
blessings, _ := v23.GetPrincipal(ctx).BlessingStore().Default()
- call := &mockCall{b: blessings}
+ call := &mockCall{p: v23.GetPrincipal(ctx), b: blessings}
var expected []interface{}
resumeMarker, _ := watchable.GetResumeMarker(st)
// Generate Put/Delete events.
