"x/ref": Add Dump method to BlessingRoots
This CL adds a 'Dump' method to BlessingRoots to obtain
the set of all roots. While I hope that this is useful
in general, it is necessary for addressing:
https://github.com/vanadium/issues/issues/228
MultiPart: 2/2
Change-Id: I705b312aabc0a4747f14728fba360fb9cec683db
diff --git a/services/agent/agentlib/agent_test.go b/services/agent/agentlib/agent_test.go
index 9f3eff7..1a9064a 100644
--- a/services/agent/agentlib/agent_test.go
+++ b/services/agent/agentlib/agent_test.go
@@ -25,6 +25,22 @@
_ "v.io/x/ref/profiles"
)
+// As of April 28, 2015, the benchmarks for serving a principal with and
+// without the agent are as follows:
+//
+// BenchmarkSignNoAgent : 889608 ns/op
+// BenchmarkSignCachedAgent : 6961410 ns/op
+// BenchmarkSignUncachedAgent : 7403763 ns/op
+// BenchmarkDefaultNoAgent : 139 ns/op
+// BenchmarkDefaultCachedAgent : 41 ns/op
+// BenchmarkDefaultUncachedAgent : 9732978 ns/op
+// BenchmarkRecognizedNegativeNoAgent : 34859 ns/op
+// BenchmarkRecognizedNegativeCachedAgent : 31043 ns/op
+// BenchmarkRecognizedNegativeUncachedAgent: 5110308 ns/op
+// BenchmarkRecognizedNoAgent : 13457 ns/op
+// BenchmarkRecognizedCachedAgent : 12609 ns/op
+// BenchmarkRecognizedUncachedAgent : 4232959 ns/op
+
//go:generate v23 test generate
func getPrincipalAndHang(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
diff --git a/services/agent/agentlib/client.go b/services/agent/agentlib/client.go
index bcda630..7dc2c3c 100644
--- a/services/agent/agentlib/client.go
+++ b/services/agent/agentlib/client.go
@@ -272,6 +272,26 @@
return b.caller.call("BlessingRootsRecognized", results(), marshalledKey, blessing)
}
+func (b *blessingRoots) Dump() map[security.BlessingPattern][]security.PublicKey {
+ var marshaledRoots map[security.BlessingPattern][][]byte
+ if err := b.caller.call("BlessingRootsDump", results(&marshaledRoots)); err != nil {
+ vlog.Errorf("error calling BlessingRootsDump: %v", err)
+ return nil
+ }
+ ret := make(map[security.BlessingPattern][]security.PublicKey)
+ for p, marshaledKeys := range marshaledRoots {
+ for _, marshaledKey := range marshaledKeys {
+ key, err := security.UnmarshalPublicKey(marshaledKey)
+ if err != nil {
+ vlog.Errorf("security.UnmarshalPublicKey(%v) returned error: %v", marshaledKey, err)
+ continue
+ }
+ ret[p] = append(ret[p], key)
+ }
+ }
+ return ret
+}
+
func (b *blessingRoots) DebugString() (s string) {
err := b.caller.call("BlessingRootsDebugString", results(&s))
if err != nil {
diff --git a/services/agent/internal/cache/cache.go b/services/agent/internal/cache/cache.go
index e16ca7a..252b6a3 100644
--- a/services/agent/internal/cache/cache.go
+++ b/services/agent/internal/cache/cache.go
@@ -5,7 +5,6 @@
package cache
import (
- "encoding/base64"
"fmt"
"sync"
@@ -29,26 +28,24 @@
// cachedRoots is a security.BlessingRoots implementation that
// wraps over another implementation and adds caching.
-// It caches both known roots, and a blessings known to be untrusted.
-// Only the negative cache is cleared when flushing, since BlessingRoots
-// doesn't allow removal.
type cachedRoots struct {
- mu *sync.RWMutex
- impl security.BlessingRoots
- cache map[string][]security.BlessingPattern
- negative *lru.Cache // key + blessing -> error
+ mu *sync.RWMutex
+ impl security.BlessingRoots
+ cache map[string][]security.BlessingPattern // GUARDED_BY(mu)
+
+ // TODO(ataly): Get rid of the following fields once all agents have been
+ // updated to support the 'Dump' method.
+ dumpExists bool // GUARDED_BY(my)
+ negative *lru.Cache // key + blessing -> error
}
-func newCachedRoots(impl security.BlessingRoots, mu *sync.RWMutex) *cachedRoots {
+func newCachedRoots(impl security.BlessingRoots, mu *sync.RWMutex) (*cachedRoots, error) {
roots := &cachedRoots{mu: mu, impl: impl}
roots.flush()
- return roots
-}
-
-// Must be called while holding mu.
-func (r *cachedRoots) flush() {
- r.negative = lru.New(maxNegativeCacheEntries)
- r.cache = make(map[string][]security.BlessingPattern)
+ if err := roots.fetchAndCacheRoots(); err != nil {
+ return nil, err
+ }
+ return roots, nil
}
func (r *cachedRoots) Add(root security.PublicKey, pattern security.BlessingPattern) error {
@@ -60,19 +57,15 @@
defer r.mu.Unlock()
r.mu.Lock()
- err = r.impl.Add(root, pattern)
- if err == nil {
+ if err := r.impl.Add(root, pattern); err != nil {
+ return err
+ }
+
+ if r.cache != nil {
+
r.cache[cacheKey] = append(r.cache[cacheKey], pattern)
}
- return err
-}
-
-func keyToString(key security.PublicKey) (string, error) {
- bytes, err := key.MarshalBinary()
- if err != nil {
- return "", err
- }
- return base64.StdEncoding.EncodeToString(bytes), nil
+ return nil
}
func (r *cachedRoots) Recognized(root security.PublicKey, blessing string) (result error) {
@@ -82,41 +75,144 @@
}
r.mu.RLock()
+ var cacheExists bool
+ if r.cache != nil {
+ err = r.recognizeFromCache(key, root, blessing)
+ cacheExists = true
+ }
+ r.mu.RUnlock()
+ if !cacheExists {
+ r.mu.Lock()
+ if err := r.fetchAndCacheRoots(); err != nil {
+ r.mu.Unlock()
+ return err
+ }
+ err = r.recognizeFromCache(key, root, blessing)
+ r.mu.Unlock()
+ }
+
+ // TODO(ataly): Get rid of the following block once all agents have been updated
+ // to support the 'Dump' method.
+ r.mu.RLock()
+ if !r.dumpExists && err != nil {
+ negKey := key + blessing
+ negErr, ok := r.negative.Get(negKey)
+ if !ok {
+ r.mu.RUnlock()
+ return r.recognizeFromImpl(key, root, blessing)
+ }
+ r.negative.Put(negKey, err)
+ err = negErr.(error)
+ }
+ r.mu.RUnlock()
+
+ return err
+}
+
+func (r *cachedRoots) Dump() map[security.BlessingPattern][]security.PublicKey {
+ var (
+ cacheExists bool
+ dump map[security.BlessingPattern][]security.PublicKey
+ )
+
+ r.mu.RLock()
+ if r.cache != nil {
+ cacheExists = true
+ dump = r.dumpFromCache()
+ }
+ r.mu.RUnlock()
+
+ if !cacheExists {
+ r.mu.Lock()
+ if err := r.fetchAndCacheRoots(); err != nil {
+ vlog.Errorf("failed to cache roots: %v", err)
+ r.mu.Unlock()
+ return nil
+ }
+ dump = r.dumpFromCache()
+ r.mu.Unlock()
+ }
+ return dump
+}
+
+func (r *cachedRoots) DebugString() string {
+ return r.impl.DebugString()
+}
+
+// Must be called while holding mu.
+func (r *cachedRoots) fetchAndCacheRoots() error {
+ dump := r.impl.Dump()
+ r.cache = make(map[string][]security.BlessingPattern)
+ if dump == nil {
+ r.dumpExists = false
+ return nil
+ }
+
+ for p, keys := range dump {
+ for _, key := range keys {
+ cacheKey, err := keyToString(key)
+ if err != nil {
+ return err
+ }
+ r.cache[cacheKey] = append(r.cache[cacheKey], p)
+ }
+ }
+ r.dumpExists = true
+ return nil
+}
+
+// Must be called while holding mu.
+func (r *cachedRoots) flush() {
+ r.cache = nil
+ r.negative = lru.New(maxNegativeCacheEntries)
+}
+
+// Must be called while holding mu.
+func (r *cachedRoots) dumpFromCache() map[security.BlessingPattern][]security.PublicKey {
+ if !r.dumpExists {
+ return nil
+ }
+ dump := make(map[security.BlessingPattern][]security.PublicKey)
+ for keyStr, patterns := range r.cache {
+ key, err := security.UnmarshalPublicKey([]byte(keyStr))
+ if err != nil {
+ vlog.Errorf("security.UnmarshalPublicKey(%v) returned error: %v", []byte(keyStr), err)
+ return nil
+ }
+ for _, p := range patterns {
+ dump[p] = append(dump[p], key)
+ }
+ }
+ return dump
+}
+
+// Must be called while holding mu.
+func (r *cachedRoots) recognizeFromCache(key string, root security.PublicKey, blessing string) error {
for _, p := range r.cache[key] {
if p.MatchedBy(blessing) {
- r.mu.RUnlock()
return nil
}
}
- r.mu.RUnlock()
- r.mu.Lock()
- negKey := key + blessing
- if err, ok := r.negative.Get(negKey); ok {
- r.negative.Put(negKey, err)
- r.mu.Unlock()
- return err.(error)
- }
- r.mu.Unlock()
- return r.recognizeAndCache(key, root, blessing)
+ return security.NewErrUnrecognizedRoot(nil, root.String(), nil)
}
-func (r *cachedRoots) recognizeAndCache(key string, root security.PublicKey, blessing string) error {
+// TODO(ataly): Get rid of this method once all agents have been updated
+// to support the 'Dump' method.
+func (r *cachedRoots) recognizeFromImpl(key string, root security.PublicKey, blessing string) error {
+ negKey := key + blessing
err := r.impl.Recognized(root, blessing)
+
r.mu.Lock()
if err == nil {
r.cache[key] = append(r.cache[key], security.BlessingPattern(blessing))
} else {
- r.negative.Put(key+blessing, err)
+ r.negative.Put(negKey, err)
}
r.mu.Unlock()
return err
}
-func (r cachedRoots) DebugString() string {
- return r.impl.DebugString()
-}
-
// cachedStore is a security.BlessingStore implementation that
// wraps over another implementation and adds caching.
type cachedStore struct {
@@ -216,7 +312,7 @@
return fmt.Sprintf("cached[%s]", s.impl)
}
-// Must be called while holding mu
+// Must be called while holding mu.
func (s *cachedStore) flush() {
s.hasDef = false
s.peers = nil
@@ -264,7 +360,10 @@
func NewCachedPrincipal(ctx *context.T, impl security.Principal, call rpc.ClientCall) (p security.Principal, err error) {
var mu sync.RWMutex
- cachedRoots := newCachedRoots(impl.Roots(), &mu)
+ cachedRoots, err := newCachedRoots(impl.Roots(), &mu)
+ if err != nil {
+ return
+ }
cachedStore := &cachedStore{mu: &mu, key: impl.PublicKey(), impl: impl.BlessingStore()}
flush := func() {
defer mu.Unlock()
@@ -293,3 +392,11 @@
p = &cachedPrincipal{p, impl}
return
}
+
+func keyToString(key security.PublicKey) (string, error) {
+ bytes, err := key.MarshalBinary()
+ if err != nil {
+ return "", err
+ }
+ return string(bytes), nil
+}
diff --git a/services/agent/internal/cache/cache_test.go b/services/agent/internal/cache/cache_test.go
index 852d0ce..cd89a5d0 100644
--- a/services/agent/internal/cache/cache_test.go
+++ b/services/agent/internal/cache/cache_test.go
@@ -17,7 +17,11 @@
var mu sync.RWMutex
p := testutil.NewPrincipal()
impl := p.Roots()
- return p.PublicKey(), impl, newCachedRoots(impl, &mu)
+ roots, err := newCachedRoots(impl, &mu)
+ if err != nil {
+ panic(err)
+ }
+ return p.PublicKey(), impl, roots
}
func TestCreateRoots(t *testing.T) {
@@ -33,7 +37,7 @@
func expectRecognized(roots security.BlessingRoots, key security.PublicKey, blessing string) string {
err := roots.Recognized(key, blessing)
if err != nil {
- return fmt.Sprintf("Key (%s, %b) not matched by roots:\n%s", key, blessing, roots.DebugString())
+ return fmt.Sprintf("Key (%s, %b) not matched by roots:\n%s, Recognized returns error: %v", key, blessing, roots.DebugString(), err)
}
return ""
}
@@ -129,6 +133,29 @@
}
}
+func TestRootsDump(t *testing.T) {
+ key, impl, cache := createRoots()
+
+ if err := cache.Add(key, "alice/friend"); err != nil {
+ t.Fatalf("Add failed: %v", err)
+ }
+
+ orig := impl.Dump()
+ if got := cache.Dump(); !reflect.DeepEqual(orig, got) {
+ t.Errorf("Dump() got %v, want %v", got, orig)
+ }
+
+ impl.Add(key, "carol")
+ if got := cache.Dump(); !reflect.DeepEqual(orig, got) {
+ t.Errorf("Dump() got %v, want %v", got, orig)
+ }
+
+ cache.flush()
+ if cur, got := impl.Dump(), cache.Dump(); !reflect.DeepEqual(cur, got) {
+ t.Errorf("Dump() got %v, want %v", got, cur)
+ }
+}
+
func createStore(p security.Principal) (security.BlessingStore, *cachedStore) {
var mu sync.RWMutex
impl := p.BlessingStore()
diff --git a/services/agent/internal/server/server.go b/services/agent/internal/server/server.go
index 32ae171..55e239f 100644
--- a/services/agent/internal/server/server.go
+++ b/services/agent/internal/server/server.go
@@ -435,6 +435,22 @@
return a.principal.Roots().Recognized(pkey, blessing)
}
+func (a agentd) BlessingRootsDump(_ *context.T, _ rpc.ServerCall) (map[security.BlessingPattern][][]byte, error) {
+ ret := make(map[security.BlessingPattern][][]byte)
+ a.w.rlock()
+ defer a.w.runlock()
+ for p, keys := range a.principal.Roots().Dump() {
+ for _, key := range keys {
+ marshaledKey, err := key.MarshalBinary()
+ if err != nil {
+ return nil, err
+ }
+ ret[p] = append(ret[p], marshaledKey)
+ }
+ }
+ return ret, nil
+}
+
func (a agentd) BlessingRootsDebugString(_ *context.T, _ rpc.ServerCall) (string, error) {
a.w.rlock()
defer a.w.runlock()
diff --git a/services/agent/wire.vdl b/services/agent/wire.vdl
index 4c93acb..994aaf5 100644
--- a/services/agent/wire.vdl
+++ b/services/agent/wire.vdl
@@ -55,6 +55,7 @@
BlessingRootsAdd(root []byte, pattern security.BlessingPattern) error
BlessingRootsRecognized(root []byte, blessing string) error
+ BlessingRootsDump() (map[security.BlessingPattern][][]byte | error)
BlessingRootsDebugString() (string | error)
// Clients using caching should call NotifyWhenChanged upon connecting to
diff --git a/services/agent/wire.vdl.go b/services/agent/wire.vdl.go
index 6b29ef1..1a0e6a2 100644
--- a/services/agent/wire.vdl.go
+++ b/services/agent/wire.vdl.go
@@ -65,6 +65,7 @@
BlessingStoreDebugString(*context.T, ...rpc.CallOpt) (string, error)
BlessingRootsAdd(ctx *context.T, root []byte, pattern security.BlessingPattern, opts ...rpc.CallOpt) error
BlessingRootsRecognized(ctx *context.T, root []byte, blessing string, opts ...rpc.CallOpt) error
+ BlessingRootsDump(*context.T, ...rpc.CallOpt) (map[security.BlessingPattern][][]byte, error)
BlessingRootsDebugString(*context.T, ...rpc.CallOpt) (string, error)
// Clients using caching should call NotifyWhenChanged upon connecting to
// the server. The server will stream back values whenever the client should
@@ -168,6 +169,11 @@
return
}
+func (c implAgentClientStub) BlessingRootsDump(ctx *context.T, opts ...rpc.CallOpt) (o0 map[security.BlessingPattern][][]byte, err error) {
+ err = v23.GetClient(ctx).Call(ctx, c.name, "BlessingRootsDump", nil, []interface{}{&o0}, opts...)
+ return
+}
+
func (c implAgentClientStub) BlessingRootsDebugString(ctx *context.T, opts ...rpc.CallOpt) (o0 string, err error) {
err = v23.GetClient(ctx).Call(ctx, c.name, "BlessingRootsDebugString", nil, []interface{}{&o0}, opts...)
return
@@ -269,6 +275,7 @@
BlessingStoreDebugString(*context.T, rpc.ServerCall) (string, error)
BlessingRootsAdd(ctx *context.T, call rpc.ServerCall, root []byte, pattern security.BlessingPattern) error
BlessingRootsRecognized(ctx *context.T, call rpc.ServerCall, root []byte, blessing string) error
+ BlessingRootsDump(*context.T, rpc.ServerCall) (map[security.BlessingPattern][][]byte, error)
BlessingRootsDebugString(*context.T, rpc.ServerCall) (string, error)
// Clients using caching should call NotifyWhenChanged upon connecting to
// the server. The server will stream back values whenever the client should
@@ -298,6 +305,7 @@
BlessingStoreDebugString(*context.T, rpc.ServerCall) (string, error)
BlessingRootsAdd(ctx *context.T, call rpc.ServerCall, root []byte, pattern security.BlessingPattern) error
BlessingRootsRecognized(ctx *context.T, call rpc.ServerCall, root []byte, blessing string) error
+ BlessingRootsDump(*context.T, rpc.ServerCall) (map[security.BlessingPattern][][]byte, error)
BlessingRootsDebugString(*context.T, rpc.ServerCall) (string, error)
// Clients using caching should call NotifyWhenChanged upon connecting to
// the server. The server will stream back values whenever the client should
@@ -399,6 +407,10 @@
return s.impl.BlessingRootsRecognized(ctx, call, i0, i1)
}
+func (s implAgentServerStub) BlessingRootsDump(ctx *context.T, call rpc.ServerCall) (map[security.BlessingPattern][][]byte, error) {
+ return s.impl.BlessingRootsDump(ctx, call)
+}
+
func (s implAgentServerStub) BlessingRootsDebugString(ctx *context.T, call rpc.ServerCall) (string, error) {
return s.impl.BlessingRootsDebugString(ctx, call)
}
@@ -554,6 +566,12 @@
},
},
{
+ Name: "BlessingRootsDump",
+ OutArgs: []rpc.ArgDesc{
+ {"", ``}, // map[security.BlessingPattern][][]byte
+ },
+ },
+ {
Name: "BlessingRootsDebugString",
OutArgs: []rpc.ArgDesc{
{"", ``}, // string
