syncbase: Infer id blessings and enforce on creation.
Id blessings in database, collection, and syncgroup names are
properly inferred from the context - preferring app and app:user,
falling back to ... and user. Inference fails if ambiguous
(blessings for different apps/users or no conventional blessings).
Perms are sanity checked to be non-empty, contain at least one admin,
and contain only tags relevant to the hierarchy level (DB: XRWA,
Collection: RWA, SG: RA).
Passing nil perms when creating a database or collection now defaults
to giving the creator all permissions instead of inheriting from the
parent in the hierarchy.
Implicit permissions are enforced for database, collection, and
syncgroup creation - the creator must have a blessing that matches
the blessing pattern in the id. This requirement is waived for service
admins when creating databases, but not in other cases (collection and
syncgroup metadata is synced, so the chain of trust must not be broken).
Also fixed glob (double encode step).
MultiPart: 2/4
Change-Id: I4cf99ef2c9644bfe6b4bfe0888b7662cd631e4d5
diff --git a/services/syncbase/bridge/cgo/impl.go b/services/syncbase/bridge/cgo/impl.go
index 50fe140..2f951bc 100644
--- a/services/syncbase/bridge/cgo/impl.go
+++ b/services/syncbase/bridge/cgo/impl.go
@@ -36,6 +36,7 @@
"v.io/v23/glob"
"v.io/v23/naming"
"v.io/v23/rpc"
+ "v.io/v23/security"
"v.io/v23/services/permissions"
wire "v.io/v23/services/syncbase"
"v.io/v23/services/watch"
@@ -830,20 +831,22 @@
//export v23_syncbase_AppBlessingFromContext
func v23_syncbase_AppBlessingFromContext(cAppBlessing *C.v23_syncbase_String, cErr *C.v23_syncbase_VError) {
- b, err := util.AppBlessingFromContext(b.Ctx)
+ // TODO(ivanpi): Rename to match Go.
+ ab, _, err := util.AppAndUserPatternFromBlessings(security.DefaultBlessingNames(v23.GetPrincipal(b.Ctx))...)
if err != nil {
cErr.init(err)
return
}
- cAppBlessing.init(b)
+ cAppBlessing.init(string(ab))
}
//export v23_syncbase_UserBlessingFromContext
func v23_syncbase_UserBlessingFromContext(cUserBlessing *C.v23_syncbase_String, cErr *C.v23_syncbase_VError) {
- b, err := util.UserBlessingFromContext(b.Ctx)
+ // TODO(ivanpi): Rename to match Go.
+ _, ub, err := util.AppAndUserPatternFromBlessings(security.DefaultBlessingNames(v23.GetPrincipal(b.Ctx))...)
if err != nil {
cErr.init(err)
return
}
- cUserBlessing.init(b)
+ cUserBlessing.init(string(ub))
}
diff --git a/services/syncbase/common/access_util.go b/services/syncbase/common/access_util.go
new file mode 100644
index 0000000..a76806d
--- /dev/null
+++ b/services/syncbase/common/access_util.go
@@ -0,0 +1,64 @@
+// Copyright 2016 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package common
+
+import (
+ "sort"
+
+ "v.io/v23/context"
+ "v.io/v23/rpc"
+ "v.io/v23/security"
+ "v.io/v23/security/access"
+ wire "v.io/v23/services/syncbase"
+ "v.io/v23/verror"
+)
+
+// ValidatePerms does basic sanity checking on the provided perms:
+// - Perms can contain only tags in the provided whitelist.
+// - At least one admin must be included to avoid permanently losing access.
+func ValidatePerms(ctx *context.T, perms access.Permissions, allowTags []access.Tag) error {
+ // Perms cannot be empty or nil.
+ if len(perms) == 0 {
+ return NewErrPermsEmpty(ctx)
+ }
+ // Perms cannot contain any tags not in the allowTags whitelist.
+ allowTagsSet := make(map[string]struct{}, len(allowTags))
+ for _, tag := range allowTags {
+ allowTagsSet[string(tag)] = struct{}{}
+ }
+ var disallowedTags []string
+ for tag, _ := range perms {
+ if _, ok := allowTagsSet[tag]; !ok {
+ disallowedTags = append(disallowedTags, tag)
+ }
+ }
+ if len(disallowedTags) > 0 {
+ sort.Strings(disallowedTags)
+ return NewErrPermsDisallowedTags(ctx, disallowedTags, access.TagStrings(allowTags...))
+ }
+ // Perms must include at least one Admin.
+ // TODO(ivanpi): More sophisticated admin verification, e.g. check that NotIn
+ // doesn't blacklist all possible In matches.
+ if adminAcl, ok := perms[string(access.Admin)]; !ok || len(adminAcl.In) == 0 {
+ return NewErrPermsNoAdmin(ctx)
+ }
+ // TODO(ivanpi): Check that perms are enforceable? It would make validation
+ // context-dependent.
+ return nil
+}
+
+// CheckImplicitPerms performs an authorization check against the implicit
+// permissions derived from the blessing pattern in the Id. It returns the
+// generated implicit perms or an authorization error.
+// TODO(ivanpi): Change to check against the specific blessing used for signing
+// instead of any blessing in call.Security().
+func CheckImplicitPerms(ctx *context.T, call rpc.ServerCall, id wire.Id, allowedTags []access.Tag) (access.Permissions, error) {
+ implicitPerms := access.Permissions{}.Add(security.BlessingPattern(id.Blessing), access.TagStrings(allowedTags...)...)
+ // Note, allowedTags is expected to contain access.Admin.
+ if err := implicitPerms[string(access.Admin)].Authorize(ctx, call.Security()); err != nil {
+ return nil, verror.New(wire.ErrUnauthorizedCreateId, ctx, id.Blessing, id.Name, err)
+ }
+ return implicitPerms, nil
+}
diff --git a/services/syncbase/common/access_util_test.go b/services/syncbase/common/access_util_test.go
new file mode 100644
index 0000000..d0294fa
--- /dev/null
+++ b/services/syncbase/common/access_util_test.go
@@ -0,0 +1,73 @@
+// Copyright 2016 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package common_test
+
+import (
+ "fmt"
+ "testing"
+
+ "v.io/v23/security/access"
+ "v.io/v23/verror"
+ "v.io/x/ref/services/syncbase/common"
+)
+
+func TestValidatePerms(t *testing.T) {
+ for _, test := range []struct {
+ perms access.Permissions
+ allowTags []access.Tag
+ wantErr error
+ }{
+ {
+ nil,
+ access.AllTypicalTags(),
+ common.NewErrPermsEmpty(nil),
+ },
+ {
+ access.Permissions{},
+ access.AllTypicalTags(),
+ common.NewErrPermsEmpty(nil),
+ },
+ {
+ access.Permissions{}.Add("alice", access.TagStrings(access.Read, access.Write, access.Admin, access.Resolve)...),
+ access.AllTypicalTags(),
+ nil,
+ },
+ {
+ access.Permissions{}.Add("alice", access.TagStrings(access.Read, access.Write, access.Admin, access.Resolve)...),
+ []access.Tag{access.Read, access.Admin},
+ common.NewErrPermsDisallowedTags(nil, access.TagStrings(access.Resolve, access.Write), access.TagStrings(access.Read, access.Admin)),
+ },
+ {
+ access.Permissions{}.Add("alice", access.TagStrings(access.Read, access.Write, access.Resolve)...),
+ access.AllTypicalTags(),
+ common.NewErrPermsNoAdmin(nil),
+ },
+ {
+ access.Permissions{}.Add("alice", access.TagStrings(access.Read)...).Blacklist("alice:phone", access.TagStrings(access.Admin)...),
+ access.AllTypicalTags(),
+ common.NewErrPermsNoAdmin(nil),
+ },
+ {
+ access.Permissions{}.Add("alice", access.TagStrings(access.Read, access.Write)...).Add("bob", access.TagStrings(access.Read, access.Admin)...),
+ access.AllTypicalTags(),
+ nil,
+ },
+ {
+ access.Permissions{}.Add("alice", access.TagStrings(access.Read, access.Admin)...).Blacklist("alice:phone", access.TagStrings(access.Write, access.Admin)...),
+ access.AllTypicalTags(),
+ nil,
+ },
+ {
+ access.Permissions{}.Add("alice", access.TagStrings(access.Read, access.Admin)...).Blacklist("alice:phone", access.TagStrings(access.Write, access.Admin)...),
+ []access.Tag{access.Read, access.Admin},
+ common.NewErrPermsDisallowedTags(nil, access.TagStrings(access.Write), access.TagStrings(access.Read, access.Admin)),
+ },
+ } {
+ err := common.ValidatePerms(nil, test.perms, test.allowTags)
+ if verror.ErrorID(err) != verror.ErrorID(test.wantErr) || fmt.Sprint(err) != fmt.Sprint(test.wantErr) {
+ t.Errorf("ValidatePerms(%v, %v): got error %v, want %v", test.perms, test.allowTags, err, test.wantErr)
+ }
+ }
+}
diff --git a/services/syncbase/common/common.vdl.go b/services/syncbase/common/common.vdl.go
new file mode 100644
index 0000000..404f186
--- /dev/null
+++ b/services/syncbase/common/common.vdl.go
@@ -0,0 +1,69 @@
+// Copyright 2016 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This file was auto-generated by the vanadium vdl tool.
+// Package: common
+
+package common
+
+import (
+ "v.io/v23/context"
+ "v.io/v23/i18n"
+ "v.io/v23/verror"
+)
+
+var _ = __VDLInit() // Must be first; see __VDLInit comments for details.
+
+//////////////////////////////////////////////////
+// Error definitions
+
+var (
+ ErrPermsEmpty = verror.Register("v.io/x/ref/services/syncbase/common.PermsEmpty", verror.NoRetry, "{1:}{2:} permissions cannot be empty")
+ ErrPermsNoAdmin = verror.Register("v.io/x/ref/services/syncbase/common.PermsNoAdmin", verror.NoRetry, "{1:}{2:} permissions must include at least one admin")
+ ErrPermsDisallowedTags = verror.Register("v.io/x/ref/services/syncbase/common.PermsDisallowedTags", verror.NoRetry, "{1:}{2:} permissions tags {3} are not allowed; only {4} are allowed")
+)
+
+// NewErrPermsEmpty returns an error with the ErrPermsEmpty ID.
+func NewErrPermsEmpty(ctx *context.T) error {
+ return verror.New(ErrPermsEmpty, ctx)
+}
+
+// NewErrPermsNoAdmin returns an error with the ErrPermsNoAdmin ID.
+func NewErrPermsNoAdmin(ctx *context.T) error {
+ return verror.New(ErrPermsNoAdmin, ctx)
+}
+
+// NewErrPermsDisallowedTags returns an error with the ErrPermsDisallowedTags ID.
+func NewErrPermsDisallowedTags(ctx *context.T, disallowed []string, allowed []string) error {
+ return verror.New(ErrPermsDisallowedTags, ctx, disallowed, allowed)
+}
+
+var __VDLInitCalled bool
+
+// __VDLInit performs vdl initialization. It is safe to call multiple times.
+// If you have an init ordering issue, just insert the following line verbatim
+// into your source files in this package, right after the "package foo" clause:
+//
+// var _ = __VDLInit()
+//
+// The purpose of this function is to ensure that vdl initialization occurs in
+// the right order, and very early in the init sequence. In particular, vdl
+// registration and package variable initialization needs to occur before
+// functions like vdl.TypeOf will work properly.
+//
+// This function returns a dummy value, so that it can be used to initialize the
+// first var in the file, to take advantage of Go's defined init order.
+func __VDLInit() struct{} {
+ if __VDLInitCalled {
+ return struct{}{}
+ }
+ __VDLInitCalled = true
+
+ // Set error format strings.
+ i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(ErrPermsEmpty.ID), "{1:}{2:} permissions cannot be empty")
+ i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(ErrPermsNoAdmin.ID), "{1:}{2:} permissions must include at least one admin")
+ i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(ErrPermsDisallowedTags.ID), "{1:}{2:} permissions tags {3} are not allowed; only {4} are allowed")
+
+ return struct{}{}
+}
diff --git a/services/syncbase/common/errors.vdl b/services/syncbase/common/errors.vdl
new file mode 100644
index 0000000..d0757d8
--- /dev/null
+++ b/services/syncbase/common/errors.vdl
@@ -0,0 +1,11 @@
+// Copyright 2016 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package common
+
+error (
+ PermsEmpty() {"en": "permissions cannot be empty"}
+ PermsNoAdmin() {"en": "permissions must include at least one admin"}
+ PermsDisallowedTags(disallowed, allowed []string) {"en": "permissions tags {disallowed} are not allowed; only {allowed} are allowed"}
+)
diff --git a/services/syncbase/longevity_tests/checker/equality_test.go b/services/syncbase/longevity_tests/checker/equality_test.go
index 8c83658..c915cbd 100644
--- a/services/syncbase/longevity_tests/checker/equality_test.go
+++ b/services/syncbase/longevity_tests/checker/equality_test.go
@@ -13,7 +13,10 @@
"v.io/v23/context"
"v.io/v23/rpc"
"v.io/v23/security"
+ "v.io/v23/security/access"
+ wire "v.io/v23/services/syncbase"
"v.io/v23/syncbase"
+ sbutil "v.io/v23/syncbase/util"
_ "v.io/x/ref/runtime/factories/roaming"
"v.io/x/ref/services/syncbase/longevity_tests/checker"
"v.io/x/ref/services/syncbase/longevity_tests/model"
@@ -21,6 +24,7 @@
"v.io/x/ref/services/syncbase/server"
"v.io/x/ref/services/syncbase/store"
"v.io/x/ref/services/syncbase/testutil"
+ tsecurity "v.io/x/ref/test/testutil"
)
func newServer(t *testing.T, ctx *context.T) (string, func()) {
@@ -28,7 +32,7 @@
if err != nil {
t.Fatalf("ioutil.TempDir() failed: %v", err)
}
- perms := testutil.DefaultPerms("...")
+ perms := testutil.DefaultPerms(access.AllTypicalTags(), "...")
serverCtx, cancel := context.WithCancel(ctx)
service, err := server.NewService(serverCtx, server.ServiceOptions{
Perms: perms,
@@ -57,6 +61,10 @@
// model universe containing the servers, and a cancel func.
func setupServers(t *testing.T, n int) (*context.T, []syncbase.Service, model.Universe, func()) {
ctx, cancel := v23.Init()
+
+ principal := tsecurity.NewPrincipal("root:u:equality")
+ ctx, _ = v23.WithPrincipal(ctx, principal)
+
ctx = v23.WithListenSpec(ctx, rpc.ListenSpec{
Addrs: rpc.ListenAddrs{{"tcp", "127.0.0.1:0"}},
})
@@ -97,18 +105,19 @@
ctx, services, universe, cancel := setupServers(t, 3)
defer cancel()
- blessing, _ := v23.GetPrincipal(ctx).BlessingStore().Default()
- myBlessing := blessing.String()
- myPerms := testutil.DefaultPerms(myBlessing)
- otherPerms := testutil.DefaultPerms(myBlessing, myBlessing+security.ChainSeparator+"alice")
+ myBlessing := security.DefaultBlessingNames(v23.GetPrincipal(ctx))[0]
+ myPermsDb := testutil.DefaultPerms(wire.AllDatabaseTags, myBlessing)
+ myPermsCx := sbutil.FilterTags(myPermsDb, wire.AllCollectionTags...)
+ otherPermsDb := testutil.DefaultPerms(wire.AllDatabaseTags, myBlessing, myBlessing+security.ChainSeparator+"alice")
+ otherPermsCx := sbutil.FilterTags(otherPermsDb, wire.AllCollectionTags...)
// Seed services with same data.
dbs := util.Databases{
"db1": util.Database{
- Permissions: myPerms,
+ Permissions: myPermsDb,
Collections: util.Collections{
"col1": util.Collection{
- Permissions: otherPerms,
+ Permissions: otherPermsCx,
Rows: util.Rows{
"key1": "val1",
"key2": "val2",
@@ -116,24 +125,24 @@
},
},
"col2": util.Collection{
- Permissions: myPerms,
+ Permissions: myPermsCx,
Rows: util.Rows{},
},
},
},
"db2": util.Database{
- Permissions: otherPerms,
+ Permissions: otherPermsDb,
Collections: util.Collections{},
},
"db3": util.Database{
- Permissions: otherPerms,
+ Permissions: otherPermsDb,
Collections: util.Collections{
"col3": util.Collection{
- Permissions: myPerms,
+ Permissions: myPermsCx,
Rows: util.Rows{},
},
"col4": util.Collection{
- Permissions: otherPerms,
+ Permissions: otherPermsCx,
Rows: util.Rows{
"key4": "val4",
"key5": "val5",
@@ -350,15 +359,14 @@
ctx, services, universe, cancel := setupServers(t, 2)
defer cancel()
- blessing, _ := v23.GetPrincipal(ctx).BlessingStore().Default()
- myBlessing := blessing.String()
- myPerms := testutil.DefaultPerms(myBlessing)
- otherPerms := testutil.DefaultPerms(myBlessing, myBlessing+security.ChainSeparator+"alice")
+ myBlessing := security.DefaultBlessingNames(v23.GetPrincipal(ctx))[0]
+ myPermsDb := testutil.DefaultPerms(wire.AllDatabaseTags, myBlessing)
+ otherPermsDb := testutil.DefaultPerms(wire.AllDatabaseTags, myBlessing, myBlessing+security.ChainSeparator+"alice")
// Seed services with databases with different permissions.
dbs1 := util.Databases{
"db1": util.Database{
- Permissions: myPerms, // Different.
+ Permissions: myPermsDb, // Different.
Collections: util.Collections{
"col1": util.Collection{
Rows: util.Rows{
@@ -372,7 +380,7 @@
}
dbs2 := util.Databases{
"db1": util.Database{
- Permissions: otherPerms, // Different.
+ Permissions: otherPermsDb, // Different.
Collections: util.Collections{
"col1": util.Collection{
Rows: util.Rows{
@@ -404,17 +412,16 @@
ctx, services, universe, cancel := setupServers(t, 2)
defer cancel()
- blessing, _ := v23.GetPrincipal(ctx).BlessingStore().Default()
- myBlessing := blessing.String()
- myPerms := testutil.DefaultPerms(myBlessing)
- otherPerms := testutil.DefaultPerms(myBlessing, myBlessing+security.ChainSeparator+"alice")
+ myBlessing := security.DefaultBlessingNames(v23.GetPrincipal(ctx))[0]
+ myPermsCx := testutil.DefaultPerms(wire.AllCollectionTags, myBlessing)
+ otherPermsCx := testutil.DefaultPerms(wire.AllCollectionTags, myBlessing, myBlessing+security.ChainSeparator+"alice")
// Seed services with databases with different collection name.
dbs1 := util.Databases{
"db1": util.Database{
Collections: util.Collections{
"col1": util.Collection{
- Permissions: myPerms, // Different.
+ Permissions: myPermsCx, // Different.
Rows: util.Rows{
"key1": "val1",
"key2": "val2",
@@ -428,7 +435,7 @@
"db1": util.Database{
Collections: util.Collections{
"col1": util.Collection{
- Permissions: otherPerms, // Different.
+ Permissions: otherPermsCx, // Different.
Rows: util.Rows{
"key1": "val1",
"key2": "val2",
diff --git a/services/syncbase/longevity_tests/client/util.go b/services/syncbase/longevity_tests/client/util.go
index b5189e5..6febe6a 100644
--- a/services/syncbase/longevity_tests/client/util.go
+++ b/services/syncbase/longevity_tests/client/util.go
@@ -30,11 +30,11 @@
syncgroups := []syncbase.Syncgroup{}
dbColsMap := map[syncbase.Database][]syncbase.Collection{}
for _, dbModel := range dbModels {
- dbPerms := defaultPerms(ctx)
+ dbPerms := defaultPerms(ctx, wire.AllDatabaseTags)
if dbModel.Permissions != nil {
dbPerms = dbModel.Permissions.ToWire("root")
}
- allowChecker(dbPerms)
+ allowChecker(dbPerms, wire.AllDatabaseTags)
// Create Database.
// TODO(nlacasse): Don't create the database unless its blessings match
@@ -47,11 +47,11 @@
// Create collections for database.
for _, colModel := range dbModel.Collections {
- colPerms := defaultPerms(ctx)
+ colPerms := defaultPerms(ctx, wire.AllCollectionTags)
if colModel.Permissions != nil {
colPerms = colModel.Permissions.ToWire("root")
}
- allowChecker(colPerms)
+ allowChecker(colPerms, wire.AllCollectionTags)
// TODO(nlacasse): Don't create the collection unless its blessings
// match ours.
@@ -64,7 +64,7 @@
// Create or join syncgroups for database.
for _, sgModel := range dbModel.Syncgroups {
- sg := db.SyncgroupForId(wire.Id{Name: sgModel.NameSuffix, Blessing: "blessing"})
+ sg := db.SyncgroupForId(sgModel.Id())
if sgModel.CreatorDevices.Lookup(sbName) != nil {
// Create the syncgroup.
spec := sgModel.Spec("root")
@@ -72,9 +72,9 @@
if len(spec.Perms) == 0 {
// Syncgroup permissions default to allow anybody.
- spec.Perms = testutil.DefaultPerms("root")
+ spec.Perms = testutil.DefaultPerms(wire.AllSyncgroupTags, "root")
}
- allowChecker(spec.Perms)
+ allowChecker(spec.Perms, wire.AllSyncgroupTags)
if err := sg.Create(ctx, spec, wire.SyncgroupMemberInfo{}); err != nil && verror.ErrorID(err) != verror.ErrExist.ID {
return nil, nil, err
@@ -98,7 +98,7 @@
}
}
if joinErr != nil {
- return nil, nil, fmt.Errorf("could not join syncgroup %q: %v", sgModel.Name(), joinErr)
+ return nil, nil, fmt.Errorf("could not join syncgroup %q (blessing %q): %v", sgModel.Name, sgModel.Blessing, joinErr)
}
}
}
@@ -108,16 +108,14 @@
}
// allowChecker gives the checker access to all tags on the Permissions object.
-func allowChecker(perms access.Permissions) {
+func allowChecker(perms access.Permissions, allowedTags []access.Tag) {
checkerPattern := security.BlessingPattern("root:checker")
- for _, tag := range access.AllTypicalTags() {
- perms.Add(checkerPattern, string(tag))
- }
+ perms.Add(checkerPattern, access.TagStrings(allowedTags...)...)
}
// defaultPerms returns a Permissions object that allows the context's default
// blessings.
-func defaultPerms(ctx *context.T) access.Permissions {
- blessing, _ := v23.GetPrincipal(ctx).BlessingStore().Default()
- return testutil.DefaultPerms(blessing.String())
+func defaultPerms(ctx *context.T, allowedTags []access.Tag) access.Permissions {
+ blessings := security.DefaultBlessingNames(v23.GetPrincipal(ctx))
+ return testutil.DefaultPerms(allowedTags, blessings[0])
}
diff --git a/services/syncbase/longevity_tests/control/control.go b/services/syncbase/longevity_tests/control/control.go
index 7eed919..083732a 100644
--- a/services/syncbase/longevity_tests/control/control.go
+++ b/services/syncbase/longevity_tests/control/control.go
@@ -371,8 +371,8 @@
return nil, err
}
- // Bless instance's principal with name root:<user>:<device>.
- blessingName := strings.Join([]string{user.Name, d.Name}, security.ChainSeparator)
+ // Bless instance's principal with name root:u:<user>:<device>.
+ blessingName := strings.Join([]string{"u", user.Name, d.Name}, security.ChainSeparator)
if err := c.identityProvider.Bless(instancePrincipal, blessingName); err != nil {
return nil, err
}
diff --git a/services/syncbase/longevity_tests/control/control_internal_test.go b/services/syncbase/longevity_tests/control/control_internal_test.go
index 23c6ce8..0c2e4d6 100644
--- a/services/syncbase/longevity_tests/control/control_internal_test.go
+++ b/services/syncbase/longevity_tests/control/control_internal_test.go
@@ -32,15 +32,14 @@
return i.principal
}
-// InternalDefaultBlessings returns the default blessings for the instance.
-// Returns empty blessings in the case of an error.
-func (i *instance) InternalDefaultBlessings() security.Blessings {
+// InternalDefaultBlessingNames returns the default blessing names for the
+// instance. Returns nil in the case of an error.
+func (i *instance) InternalDefaultBlessingNames() []string {
p, err := vsecurity.LoadPersistentPrincipal(i.credsDir, nil)
if err != nil {
- return security.Blessings{}
+ return nil
}
- b, _ := p.BlessingStore().Default()
- return b
+ return security.DefaultBlessingNames(p)
}
// InternalResetClientRegistry resets the client registry to an empty map.
diff --git a/services/syncbase/longevity_tests/control/control_test.go b/services/syncbase/longevity_tests/control/control_test.go
index a53564c..acbb714 100644
--- a/services/syncbase/longevity_tests/control/control_test.go
+++ b/services/syncbase/longevity_tests/control/control_test.go
@@ -128,7 +128,7 @@
}
// Check that instance has correct blessing name.
- gotBlessings := c.InternalGetInstance(deviceName).InternalDefaultBlessings().String()
+ gotBlessings := c.InternalGetInstance(deviceName).InternalDefaultBlessingNames()[0]
wantSuffix := strings.Join([]string{userName, deviceName}, security.ChainSeparator)
if !strings.HasSuffix(gotBlessings, wantSuffix) {
t.Errorf("wanted blessing name to have suffix %v but got %v", wantSuffix, gotBlessings)
@@ -401,21 +401,23 @@
Name: "test-user",
}
users := model.UserSet{user}
- perms := model.Permissions{
+ permsDb := model.Permissions{
"Admin": users,
"Read": users,
"Resolve": users,
"Write": users,
}
+ permsCx := permsDb.FilterTags(wire.AllCollectionTags...)
+ permsSg := permsDb.FilterTags(wire.AllSyncgroupTags...)
dbModel := &model.Database{
Name: "test_db",
Blessing: "root",
- Permissions: perms,
+ Permissions: permsDb,
Collections: []model.Collection{
model.Collection{
Name: "test_col",
Blessing: "root",
- Permissions: perms,
+ Permissions: permsCx,
},
},
}
@@ -434,9 +436,10 @@
// Construct a syncgroup and add it to the database.
sg := model.Syncgroup{
HostDevice: writerDev,
- NameSuffix: "test_sg",
+ Name: "test_sg",
+ Blessing: "root",
Collections: dbModel.Collections,
- Permissions: perms,
+ Permissions: permsSg,
CreatorDevices: model.DeviceSet{writerDev},
JoinerDevices: model.DeviceSet{watcherDev},
}
@@ -490,23 +493,25 @@
userBob := &model.User{Name: "user-bob"}
// Alice has all permissions, and gives Bob read access.
- permsAlice := model.Permissions{
+ permsAliceDb := model.Permissions{
"Admin": model.UserSet{userAlice},
"Read": model.UserSet{userAlice, userBob},
"Resolve": model.UserSet{userAlice},
"Write": model.UserSet{userAlice},
}
+ permsAliceCx := permsAliceDb.FilterTags(wire.AllCollectionTags...)
+ permsAliceSg := permsAliceDb.FilterTags(wire.AllSyncgroupTags...)
// Alice is creator of the database and collection.
dbModel := &model.Database{
Name: "test_db",
Blessing: "root",
- Permissions: permsAlice,
+ Permissions: permsAliceDb,
Collections: []model.Collection{
model.Collection{
Name: "test_col",
Blessing: "root",
- Permissions: permsAlice,
+ Permissions: permsAliceCx,
},
},
}
@@ -530,9 +535,10 @@
// Construct a syncgroup and add it to the database.
sg := model.Syncgroup{
HostDevice: devAliceWriter,
- NameSuffix: "test_sg",
+ Name: "test_sg",
+ Blessing: "root",
Collections: dbModel.Collections,
- Permissions: permsAlice,
+ Permissions: permsAliceSg,
CreatorDevices: model.DeviceSet{devAliceWriter},
JoinerDevices: model.DeviceSet{devBobWatcher},
}
@@ -575,31 +581,34 @@
// simplified.
func syncbasesCanSync(t *testing.T, c *control.Controller, sb1Name, sb2Name string) bool {
ctx := c.InternalCtx()
+ blessing := security.DefaultBlessingNames(v23.GetPrincipal(ctx))[0]
sb1Service, sb2Service := syncbase.NewService(sb1Name), syncbase.NewService(sb2Name)
- openPerms := testutil.DefaultPerms("...")
+ openPermsDb := testutil.DefaultPerms(wire.AllDatabaseTags, "...")
+ openPermsCx := testutil.DefaultPerms(wire.AllCollectionTags, "...")
+ openPermsSg := testutil.DefaultPerms(wire.AllSyncgroupTags, "...")
// Create databases on both syncbase servers.
counter++
dbName := fmt.Sprintf("test_database_%d", counter)
- sb1Db := sb1Service.Database(ctx, dbName, nil)
- if err := sb1Db.Create(ctx, openPerms); err != nil {
+ sb1Db := sb1Service.DatabaseForId(wire.Id{Blessing: blessing, Name: dbName}, nil)
+ if err := sb1Db.Create(ctx, openPermsDb); err != nil {
t.Fatal(err)
}
- sb2Db := sb2Service.Database(ctx, dbName, nil)
- if err := sb2Db.Create(ctx, openPerms); err != nil {
+ sb2Db := sb2Service.DatabaseForId(wire.Id{Blessing: blessing, Name: dbName}, nil)
+ if err := sb2Db.Create(ctx, openPermsDb); err != nil {
t.Fatal(err)
}
// Create collections on both syncbase servers.
counter++
colName := fmt.Sprintf("test_collection_%d", counter)
- sb1Col := sb1Db.Collection(ctx, colName)
- if err := sb1Col.Create(ctx, openPerms); err != nil {
+ sb1Col := sb1Db.CollectionForId(wire.Id{Blessing: blessing, Name: colName})
+ if err := sb1Col.Create(ctx, openPermsCx); err != nil {
t.Fatal(err)
}
- sb2Col := sb2Db.Collection(ctx, colName)
- if err := sb2Col.Create(ctx, openPerms); err != nil {
+ sb2Col := sb2Db.CollectionForId(wire.Id{Blessing: blessing, Name: colName})
+ if err := sb2Col.Create(ctx, openPermsCx); err != nil {
t.Fatal(err)
}
@@ -609,11 +618,11 @@
mounttable := v23.GetNamespace(ctx).Roots()[0]
sbSpec := wire.SyncgroupSpec{
Description: "test syncgroup",
- Perms: openPerms,
+ Perms: openPermsSg,
Collections: []wire.Id{sb1Col.Id()},
MountTables: []string{mounttable},
}
- sb1Sg := sb1Db.SyncgroupForId(wire.Id{Name: sgName, Blessing: "blessing"})
+ sb1Sg := sb1Db.SyncgroupForId(wire.Id{Blessing: blessing, Name: sgName})
if err := sb1Sg.Create(ctx, sbSpec, wire.SyncgroupMemberInfo{}); err != nil {
t.Fatal(err)
}
@@ -621,7 +630,7 @@
// If second syncbase can join the syncgroup, they are connected.
ctxWithTimeout, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()
- sb2Sg := sb2Db.SyncgroupForId(wire.Id{Name: sgName, Blessing: "blessing"})
+ sb2Sg := sb2Db.SyncgroupForId(wire.Id{Blessing: blessing, Name: sgName})
_, err := sb2Sg.Join(ctxWithTimeout, sb1Name, nil, wire.SyncgroupMemberInfo{})
return err == nil
}
diff --git a/services/syncbase/longevity_tests/model/model.go b/services/syncbase/longevity_tests/model/model.go
index 22edff3..37976a4 100644
--- a/services/syncbase/longevity_tests/model/model.go
+++ b/services/syncbase/longevity_tests/model/model.go
@@ -13,11 +13,10 @@
"strings"
"time"
- "v.io/v23/naming"
+ "v.io/v23/conventions"
"v.io/v23/security"
"v.io/v23/security/access"
wire "v.io/v23/services/syncbase"
- "v.io/x/ref/services/syncbase/common"
)
func init() {
@@ -40,16 +39,30 @@
p := access.Permissions{}
for tag, users := range perms {
for _, user := range users {
- userBlessing := user.Name
+ userBlessing := security.BlessingPattern(user.Name)
if rootBlessing != "" {
- userBlessing = rootBlessing + security.ChainSeparator + user.Name
+ parsedBlessing := conventions.Blessing{
+ IdentityProvider: rootBlessing,
+ User: user.Name,
+ }
+ userBlessing = parsedBlessing.UserPattern()
}
- p.Add(security.BlessingPattern(userBlessing), tag)
+ p.Add(userBlessing, tag)
}
}
return p
}
+func (perms Permissions) FilterTags(allowTags ...access.Tag) Permissions {
+ filtered := Permissions{}
+ for _, tag := range allowTags {
+ if users, ok := perms[string(tag)]; ok {
+ filtered[string(tag)] = users
+ }
+ }
+ return filtered
+}
+
// ===========
// Collections
// ===========
@@ -95,7 +108,8 @@
// Syncgroup represents a syncgroup.
type Syncgroup struct {
HostDevice *Device
- NameSuffix string
+ Name string
+ Blessing string
Description string
Collections []Collection
Permissions Permissions
@@ -106,8 +120,16 @@
JoinerDevices DeviceSet
}
-func (sg *Syncgroup) Name() string {
- return naming.Join(sg.HostDevice.Name, common.SyncbaseSuffix, sg.NameSuffix)
+func (sg *Syncgroup) String() string {
+ // TODO(ivanpi): Include more info about syncgroup.
+ return fmt.Sprintf("{syncgroup %v blessing=%v}", sg.Name, sg.Blessing)
+}
+
+func (sg *Syncgroup) Id() wire.Id {
+ return wire.Id{
+ Name: sg.Name,
+ Blessing: sg.Blessing,
+ }
}
func (sg *Syncgroup) Spec(rootBlessing string) wire.SyncgroupSpec {
@@ -130,7 +152,7 @@
func (sgs SyncgroupSet) String() string {
strs := []string{}
for _, sg := range sgs {
- strs = append(strs, sg.Name())
+ strs = append(strs, sg.String())
}
return fmt.Sprintf("[%s]", strings.Join(strs, ", "))
}
@@ -190,7 +212,8 @@
dbs := DatabaseSet{}
for i := 0; i < n; i++ {
db := &Database{
- Name: randomName("db"),
+ Name: randomName("db"),
+ Blessing: string(security.AllPrincipals),
}
dbs = append(dbs, db)
}
diff --git a/services/syncbase/longevity_tests/util/dump_stream_test.go b/services/syncbase/longevity_tests/util/dump_stream_test.go
index bc2e6a0..6c2cfe8 100644
--- a/services/syncbase/longevity_tests/util/dump_stream_test.go
+++ b/services/syncbase/longevity_tests/util/dump_stream_test.go
@@ -26,22 +26,23 @@
// All database and collection blessings must be rooted in our context
// blessing, otherwise we can't write to them.
- blessing, _ := v23.GetPrincipal(ctx).BlessingStore().Default()
- myBlessing := blessing.String()
+ myBlessing := security.DefaultBlessingNames(v23.GetPrincipal(ctx))[0]
// Create some permissions for the databases and collections. The
// permissions must contain our own blessing, otherwise we will be
// prevented from writing to the database/collection.
- myPerms := testutil.DefaultPerms(myBlessing)
- meAndAlicePerms := testutil.DefaultPerms(myBlessing, myBlessing+security.ChainSeparator+"alice")
- meAndBobPerms := testutil.DefaultPerms(myBlessing, myBlessing+security.ChainSeparator+"bob")
+ myPermsDb := testutil.DefaultPerms(wire.AllDatabaseTags, myBlessing)
+ meAndAlicePermsDb := testutil.DefaultPerms(wire.AllDatabaseTags, myBlessing, myBlessing+security.ChainSeparator+"alice")
+ meAndAlicePermsCx := sbUtil.FilterTags(meAndAlicePermsDb, wire.AllCollectionTags...)
+ meAndBobPermsDb := testutil.DefaultPerms(wire.AllDatabaseTags, myBlessing, myBlessing+security.ChainSeparator+"bob")
+ meAndBobPermsCx := sbUtil.FilterTags(meAndBobPermsDb, wire.AllCollectionTags...)
dbs := util.Databases{
"db1": util.Database{
- Permissions: myPerms,
+ Permissions: myPermsDb,
Collections: util.Collections{
"col1": util.Collection{
- Permissions: meAndAlicePerms,
+ Permissions: meAndAlicePermsCx,
Rows: util.Rows{
"key1": "val1",
"key2": "val2",
@@ -49,24 +50,24 @@
},
},
"col2": util.Collection{
- Permissions: meAndBobPerms,
+ Permissions: meAndBobPermsCx,
Rows: util.Rows{},
},
},
},
"db2": util.Database{
- Permissions: meAndBobPerms,
+ Permissions: meAndBobPermsDb,
Collections: util.Collections{},
},
"db3": util.Database{
- Permissions: meAndAlicePerms,
+ Permissions: meAndAlicePermsDb,
Collections: util.Collections{
"col3": util.Collection{
- Permissions: meAndBobPerms,
+ Permissions: meAndBobPermsCx,
Rows: util.Rows{},
},
"col4": util.Collection{
- Permissions: meAndAlicePerms,
+ Permissions: meAndAlicePermsCx,
Rows: util.Rows{
"key4": "val4",
"key5": "val5",
@@ -84,23 +85,19 @@
}
// Get expected blessings for database and collection.
- dbBlessing, err := sbUtil.AppBlessingFromContext(ctx)
- if err != nil {
- t.Fatal(err)
- }
- colBlessing, err := sbUtil.UserBlessingFromContext(ctx)
+ dbBlessing, colBlessing, err := sbUtil.AppAndUserPatternFromBlessings(myBlessing)
if err != nil {
t.Fatal(err)
}
// Get expected Ids for database and collection.
- db1Id := wire.Id{Name: "db1", Blessing: dbBlessing}
- db2Id := wire.Id{Name: "db2", Blessing: dbBlessing}
- db3Id := wire.Id{Name: "db3", Blessing: dbBlessing}
- col1Id := wire.Id{Name: "col1", Blessing: colBlessing}
- col2Id := wire.Id{Name: "col2", Blessing: colBlessing}
- col3Id := wire.Id{Name: "col3", Blessing: colBlessing}
- col4Id := wire.Id{Name: "col4", Blessing: colBlessing}
+ db1Id := wire.Id{Name: "db1", Blessing: string(dbBlessing)}
+ db2Id := wire.Id{Name: "db2", Blessing: string(dbBlessing)}
+ db3Id := wire.Id{Name: "db3", Blessing: string(dbBlessing)}
+ col1Id := wire.Id{Name: "col1", Blessing: string(colBlessing)}
+ col2Id := wire.Id{Name: "col2", Blessing: string(colBlessing)}
+ col3Id := wire.Id{Name: "col3", Blessing: string(colBlessing)}
+ col4Id := wire.Id{Name: "col4", Blessing: string(colBlessing)}
// Get expected permissions.
db1Perms := dbs["db1"].Permissions
diff --git a/services/syncbase/longevity_tests/util/seed_service.go b/services/syncbase/longevity_tests/util/seed_service.go
index 7f27a25..97fd621 100644
--- a/services/syncbase/longevity_tests/util/seed_service.go
+++ b/services/syncbase/longevity_tests/util/seed_service.go
@@ -8,7 +8,6 @@
"v.io/v23/context"
"v.io/v23/security/access"
"v.io/v23/syncbase"
- "v.io/x/ref/services/syncbase/testutil"
)
// Rows is a map from keys to values.
@@ -39,18 +38,12 @@
for dbName, db := range dbs {
sdb := s.Database(ctx, dbName, nil)
dbPerms := db.Permissions
- if len(dbPerms) == 0 {
- dbPerms = testutil.DefaultPerms("...")
- }
if err := sdb.Create(ctx, dbPerms); err != nil {
return err
}
for colName, col := range db.Collections {
scol := sdb.Collection(ctx, colName)
colPerms := col.Permissions
- if len(colPerms) == 0 {
- colPerms = testutil.DefaultPerms("...")
- }
if err := scol.Create(ctx, colPerms); err != nil {
return err
}
diff --git a/services/syncbase/server/collection.go b/services/syncbase/server/collection.go
index a26bc6c..06a2be5 100644
--- a/services/syncbase/server/collection.go
+++ b/services/syncbase/server/collection.go
@@ -33,6 +33,9 @@
// RPC methods
func (c *collectionReq) Create(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle, perms access.Permissions) error {
+ if err := common.ValidatePerms(ctx, perms, wire.AllCollectionTags); err != nil {
+ return err
+ }
impl := func(ts *transactionState) error {
tx := ts.tx
// Check DatabaseData perms.
@@ -40,7 +43,11 @@
if err := util.GetWithAuth(ctx, call, tx, c.d.stKey(), dData); err != nil {
return err
}
-
+ // Check implicit perms derived from blessing pattern in id.
+ implicitPerms, err := common.CheckImplicitPerms(ctx, call, c.id, wire.AllCollectionTags)
+ if err != nil {
+ return err
+ }
// Check for "collection already exists".
if err := store.Get(ctx, tx, c.permsKey(), &interfaces.CollectionPerms{}); verror.ErrorID(err) != verror.ErrNoExist.ID {
if err != nil {
@@ -48,9 +55,10 @@
}
return verror.New(verror.ErrExist, ctx, c.id)
}
- if perms == nil {
- perms = dData.Perms
- }
+ // Collection Create is equivalent to changing permissions from implicit to
+ // explicit. Note, the creator implicitly has all permissions (RWA), so it
+ // is legal to write data and drop the write permission in the same batch.
+ ts.MarkPermsChanged(c.id, implicitPerms, perms)
// Write new CollectionPerms.
storedPerms := interfaces.CollectionPerms(perms)
return store.Put(ctx, tx, c.permsKey(), &storedPerms)
@@ -60,11 +68,11 @@
// TODO(ivanpi): Decouple collection key prefix from collection id to allow
// collection data deletion to be deferred, making deletion faster (reference
-// removal). Same for database deletion.
+// removal).
func (c *collectionReq) Destroy(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) error {
impl := func(ts *transactionState) error {
tx := ts.tx
- // Read-check-delete CollectionPerms.
+ // Read CollectionPerms.
if err := util.GetWithAuth(ctx, call, tx, c.permsKey(), &interfaces.CollectionPerms{}); err != nil {
if verror.ErrorID(err) == verror.ErrNoExist.ID {
return nil // delete is idempotent
@@ -76,6 +84,9 @@
// destroyed. Also check that all collections exist when creating a
// syncgroup. Refactor with common part of DeleteRange.
+ // Reset all tracked changes to the collection. See comment on the method
+ // for more details.
+ ts.ResetCollectionChanges(c.id)
// Delete all data rows.
it := tx.Scan(common.ScanPrefixArgs(common.JoinKeyParts(common.RowPrefix, c.stKeyPart()), ""))
var key []byte
@@ -113,6 +124,9 @@
}
func (c *collectionReq) SetPermissions(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle, newPerms access.Permissions) error {
+ if err := common.ValidatePerms(ctx, newPerms, wire.AllCollectionTags); err != nil {
+ return err
+ }
impl := func(ts *transactionState) error {
tx := ts.tx
currentPerms, err := c.checkAccess(ctx, call, tx)
diff --git a/services/syncbase/server/database.go b/services/syncbase/server/database.go
index 6048b7f..ad0d89e 100644
--- a/services/syncbase/server/database.go
+++ b/services/syncbase/server/database.go
@@ -113,8 +113,19 @@
state.finalPerms = permsAfter
}
+// Resets all tracked changes to the collection. Used on collection destroy. Since destroy
+// cannot happen on a synced collection, the destroy and any updates before it will not be
+// seen remotely, so validation must start from the implicit permissions in case the
+// collection is created again. This also allows destroy to not require both write and
+// admin permissions.
+func (ts *transactionState) ResetCollectionChanges(collectionId wire.Id) {
+ delete(ts.permsChanges, collectionId)
+}
+
// validatePermissionChanges performs an auth check on each collection that has a data change or
// permission change and returns false if any of the auth checks fail.
+// TODO(ivanpi): This check should be done against signing blessings at signing time, in
+// both batch and non-batch cases.
func (ts *transactionState) validatePermissionChanges(ctx *context.T, securityCall security.Call) bool {
for _, collectionState := range ts.permsChanges {
// This collection was modified, make sure that the write acl is either present at
@@ -136,7 +147,6 @@
return false
}
}
-
}
return true
}
@@ -218,8 +228,6 @@
if d.exists {
return verror.New(verror.ErrExist, ctx, d.id)
}
- // TODO(sadovsky): Require id.Blessing to match the client's blessing name.
- // I.e. reserve names at the app level of the hierarchy.
// This database does not yet exist; d is just an ephemeral handle that holds
// {id wire.Id, s *service}. d.s.createDatabase will create a new database
// handle and store it in d.s.dbs[d.id].
@@ -240,10 +248,6 @@
var rng *rand.Rand = rand.New(rand.NewSource(time.Now().UTC().UnixNano()))
func (d *database) BeginBatch(ctx *context.T, call rpc.ServerCall, opts wire.BatchOptions) (wire.BatchHandle, error) {
- return d.beginBatch(ctx, opts)
-}
-
-func (d *database) beginBatch(ctx *context.T, opts wire.BatchOptions) (wire.BatchHandle, error) {
if !d.exists {
return "", verror.New(verror.ErrNoExist, ctx, d.id)
}
@@ -555,14 +559,15 @@
// not, the writeAccessReq arg dictates whether a snapshot or a transaction is
// should be created.
// TODO(ivanpi): Allow passing in non-default user blessings.
- blessing, err := pubutil.UserBlessingFromContext(qdb.ctx)
+ userBlessings, _ := security.RemoteBlessingNames(qdb.ctx, qdb.call.Security())
+ _, user, err := pubutil.AppAndUserPatternFromBlessings(userBlessings...)
if err != nil {
return nil, err
}
qt := &queryTable{
qdb: qdb,
cReq: &collectionReq{
- id: wire.Id{Blessing: blessing, Name: name},
+ id: wire.Id{Blessing: string(user), Name: name},
d: qdb.d,
},
}
@@ -782,8 +787,8 @@
}
// batchLookupInternal parses the batch handle and retrieves the corresponding
-// snapshot or batch. It returns an error if the handle is malformed or
-// the batch does not exist. Otherwise, exactly one of sn and b will be != nil.
+// snapshot or transaction. It returns an error if the handle is malformed or
+// the batch does not exist. Otherwise, exactly one of sn and ts will be != nil.
func (d *database) batchLookupInternal(ctx *context.T, bh wire.BatchHandle) (sn store.Snapshot, ts *transactionState, batchId uint64, _ error) {
if bh == "" {
return nil, nil, 0, verror.New(verror.ErrInternal, ctx, "batch lookup for empty handle")
@@ -811,6 +816,9 @@
if !d.exists {
vlog.Fatalf("database %v does not exist", d.id)
}
+ if err := common.ValidatePerms(ctx, perms, wire.AllDatabaseTags); err != nil {
+ return err
+ }
return store.RunInTransaction(d.st, func(tx store.Transaction) error {
data := &DatabaseData{}
return util.UpdateWithAuth(ctx, call, tx, d.stKey(), data, func() error {
diff --git a/services/syncbase/server/service.go b/services/syncbase/server/service.go
index fde8964..4f7328a 100644
--- a/services/syncbase/server/service.go
+++ b/services/syncbase/server/service.go
@@ -29,6 +29,7 @@
"v.io/v23/security"
"v.io/v23/security/access"
wire "v.io/v23/services/syncbase"
+ pubutil "v.io/v23/syncbase/util"
"v.io/v23/verror"
"v.io/v23/vom"
"v.io/x/ref/services/syncbase/common"
@@ -83,10 +84,8 @@
// provided blessing patterns.
func defaultPerms(blessingPatterns []security.BlessingPattern) access.Permissions {
perms := access.Permissions{}
- for _, tag := range access.AllTypicalTags() {
- for _, bp := range blessingPatterns {
- perms.Add(bp, string(tag))
- }
+ for _, bp := range blessingPatterns {
+ perms.Add(bp, access.TagStrings(access.AllTypicalTags()...)...)
}
return perms
}
@@ -145,9 +144,9 @@
return nil, err
}
- var sd ServiceData
newService := false
- if err := store.Get(ctx, st, s.stKey(), &sd); verror.ErrorID(err) != verror.ErrNoExist.ID {
+ sd := &ServiceData{}
+ if err := store.Get(ctx, st, s.stKey(), sd); verror.ErrorID(err) != verror.ErrNoExist.ID {
if err != nil {
return nil, err
}
@@ -158,6 +157,10 @@
}
}
ctx.Infof("Using persisted permissions: %v", PermsString(ctx, readPerms))
+ // TODO(ivanpi): Check other persisted perms, at runtime or in fsck.
+ if err := common.ValidatePerms(ctx, readPerms, access.AllTypicalTags()); err != nil {
+ ctx.Errorf("Error: persisted service permissions are invalid, check top-level store and Syncbase principal: %v: %v", PermsString(ctx, readPerms), err)
+ }
// Service exists.
// Run garbage collection of inactive databases.
// TODO(ivanpi): This is currently unsafe to call concurrently with
@@ -170,18 +173,21 @@
return nil, verror.New(verror.ErrInternal, ctx, err)
}
} else {
+ // Service does not exist.
newService = true
perms := opts.Perms
- // Service does not exist.
if perms == nil {
ctx.Info("Permissions flag not set. Giving local principal all permissions.")
perms = defaultPerms(security.DefaultBlessingPatterns(v23.GetPrincipal(ctx)))
}
+ if err := common.ValidatePerms(ctx, perms, access.AllTypicalTags()); err != nil {
+ return nil, err
+ }
ctx.Infof("Using permissions: %v", PermsString(ctx, perms))
- data := &ServiceData{
+ sd = &ServiceData{
Perms: perms,
}
- if err := store.Put(ctx, st, s.stKey(), data); err != nil {
+ if err := store.Put(ctx, st, s.stKey(), sd); err != nil {
return nil, err
}
}
@@ -193,8 +199,13 @@
}
if newService && opts.InitialDB != (wire.Id{}) {
- ctx.Info("Creating initial database:", opts.InitialDB)
- if err := s.createDatabase(ctx, nil, opts.InitialDB, nil, nil); err != nil {
+ // TODO(ivanpi): If service initialization fails after putting the service
+ // perms but before finishing initial database creation, initial database
+ // will never be created because service perms existence is used as a marker
+ // for a fully initialized service. Fix this with a separate marker.
+ ctx.Infof("Creating initial database: %v", opts.InitialDB)
+ dbPerms := pubutil.FilterTags(sd.GetPerms(), wire.AllDatabaseTags...)
+ if err := s.createDatabase(ctx, nil, opts.InitialDB, dbPerms, nil); err != nil {
return nil, err
}
}
@@ -333,6 +344,10 @@
}
func (s *service) SetPermissions(ctx *context.T, call rpc.ServerCall, perms access.Permissions, version string) error {
+ if err := common.ValidatePerms(ctx, perms, access.AllTypicalTags()); err != nil {
+ return err
+ }
+
return store.RunInTransaction(s.st, func(tx store.Transaction) error {
data := &ServiceData{}
return util.UpdateWithAuth(ctx, call, tx, s.stKey(), data, func() error {
@@ -401,8 +416,12 @@
// Database management methods
func (s *service) createDatabase(ctx *context.T, call rpc.ServerCall, dbId wire.Id, perms access.Permissions, metadata *wire.SchemaMetadata) (reterr error) {
+ if err := common.ValidatePerms(ctx, perms, wire.AllDatabaseTags); err != nil {
+ return err
+ }
+
// Steps:
- // 1. Check serviceData perms.
+ // 1. Check serviceData perms. Also check implicit perms if not service admin.
// 2. Put dbInfo record into garbage collection log, to clean up database if
// remaining steps fail or syncbased crashes.
// 3. Initialize database.
@@ -414,19 +433,25 @@
return verror.New(verror.ErrExist, ctx, dbId)
}
- // 1. Check serviceData perms.
- getServiceData := func() (sd *ServiceData, err error) {
- sd = new(ServiceData)
- if call != nil {
- err = util.GetWithAuth(ctx, call, s.st, s.stKey(), sd)
- } else {
- err = store.Get(ctx, s.st, s.stKey(), sd)
+ // 1. Check serviceData perms. Also check implicit perms if not service admin.
+ sData := &ServiceData{}
+ if call == nil {
+ // call is nil (called from service initialization). Skip perms checks.
+ if err := store.Get(ctx, s.st, s.stKey(), sData); err != nil {
+ return err
}
- return
- }
- sData, err := getServiceData()
- if err != nil {
- return err
+ } else {
+ // Check serviceData perms.
+ if err := util.GetWithAuth(ctx, call, s.st, s.stKey(), sData); err != nil {
+ return err
+ }
+ if adminAcl, ok := sData.Perms[string(access.Admin)]; !ok || adminAcl.Authorize(ctx, call.Security()) != nil {
+ // Caller is not service admin. Check implicit perms derived from blessing
+ // pattern in id.
+ if _, err := common.CheckImplicitPerms(ctx, call, dbId, wire.AllDatabaseTags); err != nil {
+ return err
+ }
+ }
}
// 2. Put dbInfo record into garbage collection log, to clean up database if
@@ -457,10 +482,6 @@
}()
// 3. Initialize database.
- // TODO(sadovsky): Revisit default perms.
- if perms == nil {
- perms = sData.Perms
- }
// TODO(ivanpi): newDatabase doesn't close the store on failure.
d, err := newDatabase(ctx, s, dbId, metadata, DatabaseOptions{
Perms: perms,
@@ -476,12 +497,16 @@
// 4. Move dbInfo from GC log into active dbs.
if err := store.RunInTransaction(s.st, func(tx store.Transaction) error {
- // Note: To avoid a race, we must re-check service perms, and make sure the
+ // Note: To avoid a race, we must refetch service perms, making sure the
// perms version hasn't changed, inside the transaction that makes the new
- // database visible.
- if sDataRepeat, err := getServiceData(); err != nil {
+ // database visible. If the version matches, there is no need to authorize
+ // the caller again since they were already authorized against the same
+ // perms in step 1.
+ sDataRepeat := &ServiceData{}
+ if err := store.Get(ctx, s.st, s.stKey(), sDataRepeat); err != nil {
return err
- } else if sData.Version != sDataRepeat.Version {
+ }
+ if sData.Version != sDataRepeat.Version {
return verror.NewErrBadVersion(ctx)
}
// Check for "database already exists".
diff --git a/services/syncbase/server/util/glob_children.go b/services/syncbase/server/util/glob_children.go
index 20963b7..e2469e2 100644
--- a/services/syncbase/server/util/glob_children.go
+++ b/services/syncbase/server/util/glob_children.go
@@ -30,12 +30,20 @@
for it.Advance() {
key = it.Key(key)
parts := common.SplitKeyParts(string(key))
- // For explanation of Encode(), see comment in server/dispatcher.go.
- name := pubutil.Encode(parts[len(parts)-1])
- if matcher.Match(name) {
+ id, err := pubutil.DecodeId(parts[len(parts)-1])
+ if err != nil {
+ return verror.New(verror.ErrBadState, ctx, err)
+ }
+ // Note, even though DecodeId followed by EncodeId are currently redundant
+ // (other than checking that the key component is a properly encoded Id),
+ // the Id encoding in the key will soon change to a different format than
+ // the pubutil (wire) encoding.
+ // TODO(ivanpi): Implement the new encoding.
+ encId := pubutil.EncodeId(id)
+ if matcher.Match(encId) {
// TODO(rogulenko): Check for resolve access. (For collection glob, this
// means checking prefix perms.)
- if err := call.SendStream().Send(naming.GlobChildrenReplyName{Value: name}); err != nil {
+ if err := call.SendStream().Send(naming.GlobChildrenReplyName{Value: encId}); err != nil {
return err
}
}
diff --git a/services/syncbase/server/watchlog_test.go b/services/syncbase/server/watchlog_test.go
index 545bd33..29da41d 100644
--- a/services/syncbase/server/watchlog_test.go
+++ b/services/syncbase/server/watchlog_test.go
@@ -56,13 +56,10 @@
st, _ := watchable.Wrap(memstore.New(), clk, &watchable.Options{
ManagedPrefixes: []string{common.RowPrefix, common.CollectionPermsPrefix},
})
- db := &database{id: wire.Id{Blessing: "v.io:a:xyz", Name: "d"}, st: st}
- c := &collectionReq{id: wire.Id{Blessing: "v.io:u:sam", Name: "c"}, d: db}
+ db := &database{id: wire.Id{Blessing: "a", Name: "d"}, st: st}
+ c := &collectionReq{id: wire.Id{Blessing: "u", Name: "c"}, d: db}
// Mock create the collection.
- perms := access.Permissions{}
- for _, tag := range access.AllTypicalTags() {
- perms.Add(security.BlessingPattern("root"), string(tag))
- }
+ perms := access.Permissions{}.Add("root", access.TagStrings(wire.AllCollectionTags...)...)
storedPerms := interfaces.CollectionPerms(perms)
store.Put(ctx, st, c.permsKey(), &storedPerms)
blessings, _ := v23.GetPrincipal(ctx).BlessingStore().Default()
diff --git a/services/syncbase/store/leveldb/stats_blackbox_test.go b/services/syncbase/store/leveldb/stats_blackbox_test.go
index 71172b0..89add07 100644
--- a/services/syncbase/store/leveldb/stats_blackbox_test.go
+++ b/services/syncbase/store/leveldb/stats_blackbox_test.go
@@ -13,7 +13,7 @@
"v.io/v23/context"
"v.io/v23/security"
- "v.io/v23/security/access"
+ wire "v.io/v23/services/syncbase"
"v.io/v23/syncbase"
"v.io/x/ref/lib/stats"
_ "v.io/x/ref/runtime/factories/generic"
@@ -102,7 +102,7 @@
}
count = 0
- for got := range statsValues("db/v_io_a_xyz/*/*/*", since) {
+ for got := range statsValues("db/root_o_app/*/*/*", since) {
count++
// All stats values should be zero (see stats_test.go)
@@ -127,12 +127,12 @@
write100MB(ctx, db)
// Expect at least 50 MB (see stats_test.go)
- fileBytes := <-statsValues("db/v_io_a_xyz/big_db/*/filesystem_bytes", since)
+ fileBytes := <-statsValues("db/root_o_app/big_db/*/filesystem_bytes", since)
if fileBytes.(int64) < 50*1024*1024 {
t.Errorf("Got %v, want more than 50 MB", fileBytes.(int64))
}
// Expect 25 or more (see stats_test.go)
- fileCount := <-statsValues("db/v_io_a_xyz/big_db/*/file_count", since)
+ fileCount := <-statsValues("db/root_o_app/big_db/*/file_count", since)
if fileCount.(int64) < 25 {
t.Errorf("Got %v, want 25 or more", fileCount.(int64))
}
@@ -162,7 +162,7 @@
// Write 100 kB each to 1024 keys, returning keys written.
func write100MB(ctx *context.T, db syncbase.Database) {
coll := db.Collection(ctx, "the_collection")
- err := coll.Create(ctx, permissions)
+ err := coll.Create(ctx, cxPermissions)
if err != nil {
panic(err)
}
@@ -182,12 +182,5 @@
}
var (
- allAccess = access.AccessList{In: []security.BlessingPattern{"..."}}
- permissions = access.Permissions{
- "Admin": allAccess,
- "Write": allAccess,
- "Read": allAccess,
- "Resolve": allAccess,
- "Debug": allAccess,
- }
+ cxPermissions = tu.DefaultPerms(wire.AllCollectionTags, string(security.AllPrincipals))
)
diff --git a/services/syncbase/testutil/constants.go b/services/syncbase/testutil/constants.go
index 87db9ec..0ea5076 100644
--- a/services/syncbase/testutil/constants.go
+++ b/services/syncbase/testutil/constants.go
@@ -15,13 +15,18 @@
"a::b",
"a/b",
"a/",
+ "$",
}
var validBlessingPatterns = []string{
"a",
"a:b",
+ "a:%",
+ "a:%A",
"v.io",
"v.io:foo",
+ "v.io:foobar",
+ "v.io:foo:bar",
"v.io:a:admin@myapp.com",
"v.io:o:app:user",
"\x00",
@@ -59,7 +64,9 @@
"a0_",
"a_b",
"a_0",
+ "foo",
"foobar",
+ "foo_bar",
"BARBAZ",
// 64 bytes
"abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcd",
diff --git a/services/syncbase/testutil/layer.go b/services/syncbase/testutil/layer.go
index b50b130..1092547 100644
--- a/services/syncbase/testutil/layer.go
+++ b/services/syncbase/testutil/layer.go
@@ -8,6 +8,7 @@
"fmt"
"reflect"
"sort"
+ "strings"
"testing"
"v.io/v23/context"
@@ -46,7 +47,7 @@
if err := self.Create(ctx, nil); err != nil {
t.Fatalf("self.Create() failed: %v", err)
}
- if gotPerms, wantPerms := getPermsOrDie(t, ctx, self), DefaultPerms("root:client"); !reflect.DeepEqual(gotPerms, wantPerms) {
+ if gotPerms, wantPerms := getPermsOrDie(t, ctx, self), DefaultPerms(self.AllowedTags(), "root:o:app:client"); !reflect.DeepEqual(gotPerms, wantPerms) {
t.Errorf("Perms do not match: got %v, want %v", gotPerms, wantPerms)
}
@@ -70,7 +71,7 @@
// Test create with non-default perms.
self2 := parent.Child("self2")
perms := access.Permissions{}
- perms.Add(security.BlessingPattern("root:client"), string(access.Admin))
+ perms.Add(security.BlessingPattern("root:o:app:client"), string(access.Admin))
if err := self2.Create(ctx, perms); err != nil {
t.Fatalf("self2.Create() failed: %v", err)
}
@@ -83,8 +84,8 @@
assertExists(t, ctx, self2, "self2", false)
// Test that create fails if the parent perms disallow access.
- perms = DefaultPerms("root:client")
- perms.Blacklist("root:client", string(access.Write))
+ perms = DefaultPerms(parent.AllowedTags(), "root:o:app:client")
+ perms.Blacklist("root:o:app:client", string(access.Write))
if err := parent.SetPermissions(ctx, perms, ""); err != nil {
t.Fatalf("parent.SetPermissions() failed: %v", err)
}
@@ -139,8 +140,7 @@
assertExists(t, ctx, self, "self", true)
- // By default, self perms are copied from parent, so self.Destroy should
- // succeed.
+ // By default, perms allow the creator, so self.Destroy should succeed.
if err := self.Destroy(ctx); err != nil {
t.Fatalf("self.Destroy() failed: %v", err)
}
@@ -170,8 +170,9 @@
if err := self2.Create(ctx, nil); err != nil {
t.Fatalf("self2.Create() failed: %v", err)
}
- perms := DefaultPerms("root:client")
- perms.Clear("root:client", string(access.Write), string(access.Admin))
+ perms := DefaultPerms(self2.AllowedTags(), "root:o:app:client")
+ perms.Clear("root:o:app:client", string(access.Write), string(access.Admin))
+ perms.Add("nobody", string(access.Admin))
if err := self2.SetPermissions(ctx, perms, ""); err != nil {
t.Fatalf("self2.SetPermissions() failed: %v", err)
}
@@ -191,11 +192,13 @@
// Test that destroy succeeds even if the parent and child perms disallow
// access.
- perms = DefaultPerms("root:client")
- perms.Clear("root:client", string(access.Write), string(access.Admin))
+ perms = DefaultPerms(parent.AllowedTags(), "root:o:app:client")
+ perms.Clear("root:o:app:client", string(access.Write), string(access.Admin))
+ perms.Add("nobody", string(access.Admin))
if err := parent.SetPermissions(ctx, perms, ""); err != nil {
t.Fatalf("parent.SetPermissions() failed: %v", err)
}
+ perms = DefaultPerms(child.AllowedTags(), "root:o:app:client")
if err := child.SetPermissions(ctx, perms, ""); err != nil {
t.Fatalf("child.SetPermissions() failed: %v", err)
}
@@ -223,7 +226,7 @@
return sortedStrs
}
-func TestListChildIds(t *testing.T, ctx *context.T, i interface{}, blessings, names []string) {
+func TestListChildIds(t *testing.T, ctx *context.T, rootp security.Principal, i interface{}, blessings, names []string) {
self := makeLayer(i)
var got, want []wire.Id
@@ -231,6 +234,13 @@
ids := []wire.Id{}
for _, blessing := range copyAndSortStrings(blessings) {
+ blessing = "root" + security.ChainSeparator + blessing
+ if len(blessing) > 1024 {
+ // Since we prepend "root:" to the blessing to pass implicit perms in
+ // Create, some otherwise valid blessings can exceed the 1024 byte limit
+ // for an Id blessing when the prefix is added, so we skip them.
+ continue
+ }
for _, name := range copyAndSortStrings(names) {
ids = append(ids, wire.Id{blessing, name})
}
@@ -252,7 +262,8 @@
break
}
id := ids[i]
- if err := self.ChildForId(id).Create(ctx, nil); err != nil {
+ creatorCtx := NewCtx(ctx, rootp, strings.TrimPrefix(id.Blessing, "root"+security.ChainSeparator))
+ if err := self.ChildForId(id).Create(creatorCtx, access.Permissions{}.Add("nobody", string(access.Admin))); err != nil {
t.Fatalf("Create(%v) failed: %v", id, err)
}
}
@@ -264,7 +275,7 @@
// Mirrors v.io/groups/x/ref/services/groups/internal/server/server_test.go.
func TestPerms(t *testing.T, ctx *context.T, ac util.AccessController) {
myperms := access.Permissions{}
- myperms.Add(security.BlessingPattern("root:client"), string(access.Admin))
+ myperms.Add(security.BlessingPattern("root:o:app:client"), string(access.Admin))
// Demonstrate that myperms differs from the current perms.
if reflect.DeepEqual(myperms, getPermsOrDie(t, ctx, ac)) {
t.Fatalf("Permissions should not match: %v", myperms)
@@ -312,7 +323,7 @@
// SetPermissions with empty version should succeed.
permsBefore, versionBefore = permsAfter, versionAfter
- myperms.Add(security.BlessingPattern("root:client"), string(access.Read))
+ myperms.Add(security.BlessingPattern("root:o:app:client"), string(access.Read))
if err := ac.SetPermissions(ctx, myperms, ""); err != nil {
t.Fatalf("SetPermissions failed: %v", err)
}
@@ -341,7 +352,7 @@
}
// Take away our access. SetPermissions and GetPermissions should fail.
- if err := ac.SetPermissions(ctx, access.Permissions{}, ""); err != nil {
+ if err := ac.SetPermissions(ctx, access.Permissions{}.Add("nobody", string(access.Admin)), ""); err != nil {
t.Fatalf("SetPermissions failed: %v", err)
}
if _, _, err := ac.GetPermissions(ctx); verror.ErrorID(err) != verror.ErrNoAccess.ID {
@@ -359,6 +370,7 @@
type layer interface {
util.AccessController
+ AllowedTags() []access.Tag
FullName() string
Create(ctx *context.T, perms access.Permissions) error
Destroy(ctx *context.T) error
@@ -372,6 +384,9 @@
syncbase.Service
}
+func (_ *service) AllowedTags() []access.Tag {
+ return access.AllTypicalTags()
+}
func (s *service) Create(ctx *context.T, perms access.Permissions) error {
panic(notAvailable)
}
@@ -385,7 +400,7 @@
return s.ListDatabases(ctx)
}
func (s *service) Child(childName string) layer {
- return makeLayer(s.DatabaseForId(DbId(childName), nil))
+ return makeLayer(s.DatabaseForId(wire.Id{"root:o:app", childName}, nil))
}
func (s *service) ChildForId(childId wire.Id) layer {
return makeLayer(s.DatabaseForId(childId, nil))
@@ -395,11 +410,14 @@
syncbase.Database
}
+func (_ *database) AllowedTags() []access.Tag {
+ return wire.AllDatabaseTags
+}
func (d *database) ListChildIds(ctx *context.T) ([]wire.Id, error) {
return d.ListCollections(ctx)
}
func (d *database) Child(childName string) layer {
- return makeLayer(d.CollectionForId(CxId(childName)))
+ return makeLayer(d.CollectionForId(wire.Id{"root:o:app:client", childName}))
}
func (d *database) ChildForId(childId wire.Id) layer {
return makeLayer(d.CollectionForId(childId))
@@ -409,6 +427,9 @@
syncbase.Collection
}
+func (_ *collection) AllowedTags() []access.Tag {
+ return wire.AllCollectionTags
+}
func (c *collection) SetPermissions(ctx *context.T, perms access.Permissions, version string) error {
return c.Collection.SetPermissions(ctx, perms)
}
@@ -431,6 +452,9 @@
c syncbase.Collection
}
+func (_ *row) AllowedTags() []access.Tag {
+ return nil
+}
func (r *row) Create(ctx *context.T, perms access.Permissions) error {
if perms != nil {
panic(fmt.Sprintf("bad perms: %v", perms))
diff --git a/services/syncbase/testutil/util.go b/services/syncbase/testutil/util.go
index 7aaad54..9c848ee 100644
--- a/services/syncbase/testutil/util.go
+++ b/services/syncbase/testutil/util.go
@@ -11,6 +11,7 @@
"os"
"reflect"
"runtime/debug"
+ "strings"
"testing"
"v.io/v23"
@@ -46,41 +47,28 @@
// TODO(sadovsky): Standardize on a small set of constants and helper functions
// to share across all Syncbase tests. Currently, our 'featuretests' tests use a
// different set of helpers from our other unit tests.
-func DbId(name string) wire.Id {
- return wire.Id{Blessing: "v.io:a:xyz", Name: name}
-}
-
func CreateDatabase(t testing.TB, ctx *context.T, s syncbase.Service, name string) syncbase.Database {
- d := s.DatabaseForId(DbId(name), nil)
+ d := s.Database(ctx, name, nil)
if err := d.Create(ctx, nil); err != nil {
Fatalf(t, "d.Create() failed: %v", err)
}
return d
}
-func CxId(name string) wire.Id {
- return wire.Id{Blessing: "v.io:u:sam", Name: name}
-}
-
func CreateCollection(t testing.TB, ctx *context.T, d syncbase.Database, name string) syncbase.Collection {
- c := d.CollectionForId(CxId(name))
+ c := d.Collection(ctx, name)
if err := c.Create(ctx, nil); err != nil {
Fatalf(t, "c.Create() failed: %v", err)
}
return c
}
-func CreateSyncgroup(
- t testing.TB,
- ctx *context.T,
- d syncbase.Database,
- c syncbase.Collection,
- name, description string,
-) syncbase.Syncgroup {
- sg := d.SyncgroupForId(CxId(name))
+func CreateSyncgroup(t testing.TB, ctx *context.T, d syncbase.Database, c syncbase.Collection, name, description string) syncbase.Syncgroup {
+ sg := d.Syncgroup(ctx, name)
sgSpec := wire.SyncgroupSpec{
Description: description,
Collections: []wire.Id{c.Id()},
+ Perms: DefaultPerms(wire.AllSyncgroupTags, string(security.AllPrincipals)),
}
sgMembership := wire.SyncgroupMemberInfo{}
if err := sg.Create(ctx, sgSpec, sgMembership); err != nil {
@@ -92,7 +80,7 @@
// TODO(sadovsky): Drop the 'perms' argument. The only client that passes
// non-nil, syncgroup_test.go, should use SetupOrDieCustom instead.
func SetupOrDie(perms access.Permissions) (clientCtx *context.T, serverName string, cleanup func()) {
- _, clientCtx, serverName, _, cleanup = SetupOrDieCustom("client", "server", perms)
+ _, clientCtx, serverName, _, cleanup = SetupOrDieCustom(strings.Join([]string{"o", "app", "client"}, security.ChainSeparator), "server", perms)
return
}
@@ -105,7 +93,7 @@
clientCtx, serverCtx := NewCtx(ctx, rootp, clientSuffix), NewCtx(ctx, rootp, serverSuffix)
if perms == nil {
- perms = DefaultPerms(fmt.Sprintf("%s%s%s", "root", security.ChainSeparator, clientSuffix))
+ perms = DefaultPerms(access.AllTypicalTags(), "root"+security.ChainSeparator+clientSuffix)
}
serverName, stopServer := newServer(serverCtx, perms)
cleanup = func() {
@@ -115,12 +103,10 @@
return
}
-func DefaultPerms(patterns ...string) access.Permissions {
+func DefaultPerms(tags []access.Tag, patterns ...string) access.Permissions {
perms := access.Permissions{}
- for _, tag := range access.AllTypicalTags() {
- for _, pattern := range patterns {
- perms.Add(security.BlessingPattern(pattern), string(tag))
- }
+ for _, pattern := range patterns {
+ perms.Add(security.BlessingPattern(pattern), access.TagStrings(tags...)...)
}
return perms
}
diff --git a/services/syncbase/vsync/initiator_test.go b/services/syncbase/vsync/initiator_test.go
index 4f33b98..6939aee 100644
--- a/services/syncbase/vsync/initiator_test.go
+++ b/services/syncbase/vsync/initiator_test.go
@@ -356,6 +356,7 @@
SpecVersion: "etag-0",
Spec: wire.SyncgroupSpec{
Collections: []wire.Id{makeCxId("foo"), makeCxId("bar")},
+ Perms: mockSgPerms,
MountTables: []string{"1/2/3/4", "5/6/7/8"},
},
Joiners: map[string]wire.SyncgroupMemberInfo{
@@ -422,8 +423,8 @@
}
wantVecs = interfaces.Knowledge{
- "v.io:u:sam,foo\xfe": interfaces.GenVector{10: 0},
- "v.io:u:sam,bar\xfe": interfaces.GenVector{10: 0},
+ "mockuser,foo\xfe": interfaces.GenVector{10: 0},
+ "mockuser,bar\xfe": interfaces.GenVector{10: 0},
}
}
diff --git a/services/syncbase/vsync/peer_manager_test.go b/services/syncbase/vsync/peer_manager_test.go
index 1b07e96..3fd6641 100644
--- a/services/syncbase/vsync/peer_manager_test.go
+++ b/services/syncbase/vsync/peer_manager_test.go
@@ -42,6 +42,7 @@
SpecVersion: "etag-0",
Spec: wire.SyncgroupSpec{
Collections: []wire.Id{makeCxId("foo"), makeCxId("bar")},
+ Perms: mockSgPerms,
MountTables: []string{"1/2/3/4", "5/6/7/8"},
},
Joiners: map[string]wire.SyncgroupMemberInfo{
diff --git a/services/syncbase/vsync/syncgroup.go b/services/syncbase/vsync/syncgroup.go
index 6db580d..05a1000 100644
--- a/services/syncbase/vsync/syncgroup.go
+++ b/services/syncbase/vsync/syncgroup.go
@@ -116,6 +116,11 @@
if len(colls) != len(spec.Collections) {
return verror.New(verror.ErrBadArg, ctx, "group has duplicate collections specified")
}
+
+ if err := common.ValidatePerms(ctx, spec.Perms, wire.AllSyncgroupTags); err != nil {
+ return err
+ }
+
return nil
}
@@ -658,10 +663,16 @@
vlog.VI(2).Infof("sync: CreateSyncgroup: begin: %s, spec %+v", sgId, spec)
defer vlog.VI(2).Infof("sync: CreateSyncgroup: end: %s", sgId)
+ if err := pubutil.ValidateId(sgId); err != nil {
+ return verror.New(wire.ErrInvalidName, ctx, pubutil.EncodeId(sgId), err)
+ }
+
ss := sd.sync.(*syncService)
dbId := sd.db.Id()
// Instantiate sg. Add self as joiner.
+ // TODO(ivanpi): Spec is sanity checked later in addSyncgroup. Do it here
+ // instead to fail early?
gid, version := SgIdToGid(dbId, sgId), ss.newSyncgroupVersion()
sg := &interfaces.Syncgroup{
Id: sgId,
@@ -679,6 +690,13 @@
return err
}
+ // Check implicit perms derived from blessing pattern in id.
+ // TODO(ivanpi): The signing blessing should be (re?)checked against the
+ // implicit perms when signing the syncgroup metadata.
+ if _, err := common.CheckImplicitPerms(ctx, call, sgId, wire.AllSyncgroupTags); err != nil {
+ return err
+ }
+
// Check that all the collections on which the syncgroup is
// being created exist.
for _, c := range spec.Collections {
@@ -690,10 +708,8 @@
}
}
- // TODO(hpucha): Check prefix ACLs on all SG prefixes.
+ // TODO(hpucha): Check ACLs on all SG collections.
// This may need another method on util.Database interface.
- // TODO(hpucha): Do some SG ACL checking. Check creator
- // has Admin privilege.
// Reserve a log generation and position counts for the new syncgroup.
gen, pos := ss.reserveGenAndPosInDbLog(ctx, dbId, sgOID(gid), 1)
@@ -1006,7 +1022,6 @@
// return verror.New(verror.ErrNoAccess, ctx)
// }
- // TODO(hpucha): Check syncgroup ACL for sanity checking.
// TODO(hpucha): Check if the acl change causes neighborhood
// advertising to change.
diff --git a/services/syncbase/vsync/syncgroup_test.go b/services/syncbase/vsync/syncgroup_test.go
index 296ec1f..12c6f8d 100644
--- a/services/syncbase/vsync/syncgroup_test.go
+++ b/services/syncbase/vsync/syncgroup_test.go
@@ -66,6 +66,7 @@
SpecVersion: "etag-0",
Spec: wire.SyncgroupSpec{
Collections: []wire.Id{makeCxId("foo"), makeCxId("bar")},
+ Perms: mockSgPerms,
},
Joiners: map[string]wire.SyncgroupMemberInfo{
"phone": wire.SyncgroupMemberInfo{SyncPriority: 10},
@@ -188,6 +189,7 @@
SpecVersion: "etag-0",
Spec: wire.SyncgroupSpec{
Collections: []wire.Id{makeCxId("foo"), makeCxId("bar")},
+ Perms: mockSgPerms,
},
Joiners: map[string]wire.SyncgroupMemberInfo{
"phone": wire.SyncgroupMemberInfo{SyncPriority: 10},
@@ -280,6 +282,7 @@
SpecVersion: "etag-0",
Spec: wire.SyncgroupSpec{
Collections: []wire.Id{makeCxId("foo"), makeCxId("bar")},
+ Perms: mockSgPerms,
},
Joiners: map[string]wire.SyncgroupMemberInfo{
"phone": wire.SyncgroupMemberInfo{SyncPriority: 10},
@@ -363,6 +366,7 @@
Spec: wire.SyncgroupSpec{
MountTables: []string{"mt1"},
Collections: []wire.Id{makeCxId("foo")},
+ Perms: mockSgPerms,
},
Joiners: map[string]wire.SyncgroupMemberInfo{
"phone": wire.SyncgroupMemberInfo{SyncPriority: 10},
@@ -378,6 +382,7 @@
Spec: wire.SyncgroupSpec{
MountTables: []string{"mt2", "mt3"},
Collections: []wire.Id{makeCxId("bar")},
+ Perms: mockSgPerms,
},
Joiners: map[string]wire.SyncgroupMemberInfo{
"tablet": wire.SyncgroupMemberInfo{SyncPriority: 111},
diff --git a/services/syncbase/vsync/testutil_test.go b/services/syncbase/vsync/testutil_test.go
index 0ebeea7..8fb5a81 100644
--- a/services/syncbase/vsync/testutil_test.go
+++ b/services/syncbase/vsync/testutil_test.go
@@ -15,6 +15,7 @@
"v.io/v23/context"
"v.io/v23/rpc"
+ "v.io/v23/security/access"
wire "v.io/v23/services/syncbase"
pubutil "v.io/v23/syncbase/util"
"v.io/v23/verror"
@@ -30,6 +31,7 @@
var (
mockDbId = wire.Id{Blessing: "mockapp", Name: "mockdb"}
mockCRStream *conflictResolverStream
+ mockSgPerms = access.Permissions{}.Add("mockuser", access.TagStrings(wire.AllSyncgroupTags...)...)
)
// mockService emulates a Syncbase service that includes store and sync.
@@ -159,7 +161,7 @@
// makeCxId returns a collection id with a default user blessing.
func makeCxId(name string) wire.Id {
- return wire.Id{Blessing: "v.io:u:sam", Name: name}
+ return wire.Id{Blessing: "mockuser", Name: name}
}
// makeRowKey returns the database row key for a given application key.
