Merge "veyron/runtimes/google/ipc: Move "publisher" to its own package."
diff --git a/examples/rockpaperscissors/impl/impl_test.go b/examples/rockpaperscissors/impl/impl_test.go
index 883b6f8..c8dbf8d 100644
--- a/examples/rockpaperscissors/impl/impl_test.go
+++ b/examples/rockpaperscissors/impl/impl_test.go
@@ -78,27 +78,31 @@
rpsService, rpsStop := startRockPaperScissors(t, runtime, mtAddress)
defer rpsStop()
- if err := rpsService.Player().InitiateGame(); err != nil {
- t.Errorf("Failed to initiate game: %v", err)
+ const numGames = 10
+ for x := 0; x < numGames; x++ {
+ if err := rpsService.Player().InitiateGame(); err != nil {
+ t.Errorf("Failed to initiate game: %v", err)
+ }
}
+ rpsService.Player().WaitUntilIdle()
- // There was only one game, but the player played twice. So, we
- // expected the player to show that it played 2 games, and won 1.
+ // For each game, the player plays twice. So, we expect the player to
+ // show that it played 2×numGames, and won numGames.
played, won := rpsService.Player().Stats()
- if want, got := int64(2), played; want != got {
+ if want, got := int64(2*numGames), played; want != got {
t.Errorf("Unexpected number of played games. Got %d, want %d", got, want)
}
- if want, got := int64(1), won; want != got {
+ if want, got := int64(numGames), won; want != got {
t.Errorf("Unexpected number of won games. Got %d, want %d", got, want)
}
- // The Judge ran exactly one game.
- if want, got := int64(1), rpsService.Judge().Stats(); want != got {
+ // The Judge ran every game.
+ if want, got := int64(numGames), rpsService.Judge().Stats(); want != got {
t.Errorf("Unexpected number of games run. Got %d, want %d", got, want)
}
- // The Score Keeper received one score card.
- if want, got := int64(1), rpsService.ScoreKeeper().Stats(); want != got {
+ // The Score Keeper received one score card per game.
+ if want, got := int64(numGames), rpsService.ScoreKeeper().Stats(); want != got {
t.Errorf("Unexpected number of score cards. Got %d, want %d", got, want)
}
}
diff --git a/examples/rockpaperscissors/impl/player.go b/examples/rockpaperscissors/impl/player.go
index fa7ee3a..1416f41 100644
--- a/examples/rockpaperscissors/impl/player.go
+++ b/examples/rockpaperscissors/impl/player.go
@@ -16,10 +16,11 @@
)
type Player struct {
- mt naming.MountTable
- lock sync.Mutex
- gamesPlayed common.Counter
- gamesWon common.Counter
+ mt naming.MountTable
+ lock sync.Mutex
+ gamesPlayed common.Counter
+ gamesWon common.Counter
+ gamesInProgress common.Counter
}
func NewPlayer(mt naming.MountTable) *Player {
@@ -32,6 +33,13 @@
return
}
+// only used by tests.
+func (p *Player) WaitUntilIdle() {
+ for p.gamesInProgress.Value() != 0 {
+ time.Sleep(10 * time.Millisecond)
+ }
+}
+
func (p *Player) InitiateGame() error {
judge, err := common.FindJudge(p.mt)
if err != nil {
@@ -99,6 +107,8 @@
// playGame plays an entire game, which really only consists of reading
// commands from the server, and picking a random "move" when asked to.
func (p *Player) playGame(judge string, gameID rps.GameID) (rps.PlayResult, error) {
+ p.gamesInProgress.Add(1)
+ defer p.gamesInProgress.Add(-1)
j, err := rps.BindRockPaperScissors(judge)
if err != nil {
return rps.PlayResult{}, err
diff --git a/runtimes/google/ipc/client.go b/runtimes/google/ipc/client.go
index 16a711c..d974020 100644
--- a/runtimes/google/ipc/client.go
+++ b/runtimes/google/ipc/client.go
@@ -1,6 +1,7 @@
package ipc
import (
+ "fmt"
"io"
"sync"
"time"
@@ -141,32 +142,15 @@
}
// Validate caveats on the server's identity for the context associated with this call.
- remoteID := flow.RemoteID()
- if remoteID == nil {
- lastErr = verror.NotAuthorizedf("ipc: server identity cannot be nil")
- continue
- }
- // TODO(ataly): Fetch third-party discharges from the server.
- // TODO(ataly): What should the label be for the context? Typically the label is the security.Label
- // of the method but we don't have that information here at the client.
- authorizedRemoteID, err := remoteID.Authorize(isecurity.NewContext(
- isecurity.ContextArgs{
- LocalID: flow.LocalID(),
- RemoteID: remoteID,
- }))
+ blessing, err := authorizeServer(flow.LocalID(), flow.RemoteID(), opts)
if err != nil {
- lastErr = verror.NotAuthorizedf("ipc: server identity %q has one or more invalid caveats: %v", remoteID, err)
+ lastErr = verror.NotAuthorizedf("ipc: client unwilling to talk to server %q: %v", flow.RemoteID(), err)
+ flow.Close()
continue
}
-
- if lastErr = matchServerID(authorizedRemoteID, opts); lastErr != nil {
- continue
- }
-
- // remoteID is authorized for the context associated with this call.
lastErr = nil
fc := newFlowClient(flow)
- if verr := fc.start(suffix, method, args, timeout); verr != nil {
+ if verr := fc.start(suffix, method, args, timeout, blessing); verr != nil {
return nil, verr
}
return fc, nil
@@ -177,6 +161,45 @@
return nil, errNoServers
}
+// authorizeServer validates that server has an identity that the client is willing to converse
+// with, and if so returns a blessing to be provided to the server. This blessing can be nil,
+// which indicates that the client does wish to talk to the server but not provide any blessings.
+func authorizeServer(client, server security.PublicID, opts []ipc.CallOpt) (security.PublicID, error) {
+ if server == nil {
+ return nil, fmt.Errorf("server identity cannot be nil")
+ }
+ // TODO(ataly): Fetch third-party discharges from the server.
+ // TODO(ataly): What should the label be for the context? Typically the label is the security.Label
+ // of the method but we don't have that information here at the client.
+ authID, err := server.Authorize(isecurity.NewContext(isecurity.ContextArgs{
+ LocalID: client,
+ RemoteID: server,
+ }))
+ if err != nil {
+ return nil, err
+ }
+ var granter ipc.Granter
+ for _, o := range opts {
+ switch v := o.(type) {
+ case veyron2.RemoteID:
+ if !authID.Match(security.PrincipalPattern(v)) {
+ return nil, fmt.Errorf("server %q does not match the provided pattern %q", authID, v)
+ }
+ case ipc.Granter:
+ // Later Granters take precedence over earlier ones.
+ // Or should fail if there are multiple provided?
+ granter = v
+ }
+ }
+ var blessing security.PublicID
+ if granter != nil {
+ if blessing, err = granter.Grant(authID); err != nil {
+ return nil, fmt.Errorf("failed to grant credentials to server %q: %v", authID, err)
+ }
+ }
+ return blessing, nil
+}
+
func (c *client) getCallTimeout(opts []ipc.CallOpt) time.Duration {
timeout := c.callTimeout
for _, opt := range opts {
@@ -229,16 +252,22 @@
return verr
}
-func (fc *flowClient) start(suffix, method string, args []interface{}, timeout time.Duration) verror.E {
+func (fc *flowClient) start(suffix, method string, args []interface{}, timeout time.Duration, blessing security.PublicID) verror.E {
req := ipc.Request{
- Suffix: suffix,
- Method: method,
- NumPosArgs: uint64(len(args)),
- Timeout: int64(timeout),
+ Suffix: suffix,
+ Method: method,
+ NumPosArgs: uint64(len(args)),
+ Timeout: int64(timeout),
+ HasBlessing: blessing != nil,
}
if err := fc.enc.Encode(req); err != nil {
return fc.close(verror.BadProtocolf("ipc: request encoding failed: %v", err))
}
+ if blessing != nil {
+ if err := fc.enc.Encode(blessing); err != nil {
+ return fc.close(verror.BadProtocolf("ipc: blessing encoding failed: %v", err))
+ }
+ }
for ix, arg := range args {
if err := fc.enc.Encode(arg); err != nil {
return fc.close(verror.BadProtocolf("ipc: arg %d encoding failed: %v", ix, err))
@@ -357,12 +386,3 @@
func (fc *flowClient) Cancel() {
fc.flow.Cancel()
}
-
-func matchServerID(id security.PublicID, opts []ipc.CallOpt) verror.E {
- for _, opt := range opts {
- if pattern, ok := opt.(veyron2.RemoteID); ok && !id.Match(security.PrincipalPattern(pattern)) {
- return verror.NotAuthorizedf("ipc: server identity %q does not have a name matching the provided pattern %q", id, pattern)
- }
- }
- return nil
-}
diff --git a/runtimes/google/ipc/flow_test.go b/runtimes/google/ipc/flow_test.go
index 0a60b99..35f7662 100644
--- a/runtimes/google/ipc/flow_test.go
+++ b/runtimes/google/ipc/flow_test.go
@@ -122,7 +122,7 @@
clientFlow, serverFlow := newTestFlows()
client := newFlowClient(clientFlow)
server := newFlowServer(serverFlow, ipcServer)
- err := client.start(test.suffix, test.method, test.args, time.Duration(0))
+ err := client.start(test.suffix, test.method, test.args, time.Duration(0), nil)
if err != nil {
t.Errorf("%s client.start unexpected error: %v", name(test), err)
}
diff --git a/runtimes/google/ipc/full_test.go b/runtimes/google/ipc/full_test.go
index 39964a6..5858e11 100644
--- a/runtimes/google/ipc/full_test.go
+++ b/runtimes/google/ipc/full_test.go
@@ -64,6 +64,10 @@
return fmt.Sprintf("%v", call.LocalID()), fmt.Sprintf("%v", call.RemoteID())
}
+func (*testServer) EchoBlessing(call ipc.ServerCall, arg string) (result, blessing string) {
+ return arg, fmt.Sprintf("%v", call.Blessing())
+}
+
func (*testServer) EchoAndError(call ipc.ServerCall, arg string) (string, error) {
result := fmt.Sprintf("method:%q,suffix:%q,arg:%q", call.Method(), call.Suffix(), arg)
if arg == "error" {
@@ -279,16 +283,20 @@
return
}
+func bless(blessor security.PrivateID, blessee security.PublicID, name string, caveats ...security.ServiceCaveat) security.PublicID {
+ blessed, err := blessor.Bless(blessee, name, 24*time.Hour, caveats)
+ if err != nil {
+ panic(err)
+ }
+ return blessed
+}
+
func derive(blessor security.PrivateID, name string, caveats ...security.ServiceCaveat) security.PrivateID {
id, err := isecurity.NewPrivateID("irrelevant")
if err != nil {
panic(err)
}
- blessedID, err := blessor.Bless(id.PublicID(), name, 5*time.Minute, caveats)
- if err != nil {
- panic(err)
- }
- derivedID, err := id.Derive(blessedID)
+ derivedID, err := id.Derive(bless(blessor, id.PublicID(), name, caveats...))
if err != nil {
panic(err)
}
@@ -303,8 +311,8 @@
}
func TestStartCall(t *testing.T) {
- authorizeErr := "has one or more invalid caveats"
- nameErr := "does not have a name matching the provided pattern"
+ authorizeErr := "not authorized because"
+ nameErr := "does not match the provided pattern"
cavOnlyV1 := security.UniversalCaveat(caveat.PeerIdentity{"client/v1"})
now := time.Now()
@@ -436,6 +444,49 @@
}
}
+// granter implements ipc.Granter, returning a fixed (security.PublicID, error) pair.
+type granter struct {
+ ipc.CallOpt
+ id security.PublicID
+ err error
+}
+
+func (g granter) Grant(id security.PublicID) (security.PublicID, error) { return g.id, g.err }
+
+func TestBlessing(t *testing.T) {
+ b := createBundle(t, clientID, serverID, &testServer{})
+ defer b.cleanup(t)
+
+ tests := []struct {
+ granter ipc.CallOpt
+ blessing, starterr, finisherr string
+ }{
+ {blessing: "<nil>"},
+ {granter: granter{id: bless(clientID, serverID.PublicID(), "blessed")}, blessing: "client/blessed"},
+ {granter: granter{err: errors.New("hell no")}, starterr: "hell no"},
+ {granter: granter{id: clientID.PublicID()}, finisherr: "blessing provided not bound to this server"},
+ }
+ for _, test := range tests {
+ call, err := b.client.StartCall(&fakeContext{}, "mountpoint/server/suffix", "EchoBlessing", []interface{}{"argument"}, test.granter)
+ if !matchesErrorPattern(err, test.starterr) {
+ t.Errorf("%+v: StartCall returned error %v", test, err)
+ }
+ if err != nil {
+ continue
+ }
+ var result, blessing string
+ if err = call.Finish(&result, &blessing); !matchesErrorPattern(err, test.finisherr) {
+ t.Errorf("%+v: Finish returned error %v", test, err)
+ }
+ if err != nil {
+ continue
+ }
+ if result != "argument" || blessing != test.blessing {
+ t.Errorf("%+v: Got (%q, %q)", test, result, blessing)
+ }
+ }
+}
+
func TestRPCAuthorization(t *testing.T) {
cavOnlyEcho := security.ServiceCaveat{
Service: security.AllPrincipals,
diff --git a/runtimes/google/ipc/jni/arg_getter_test.go b/runtimes/google/ipc/jni/arg_getter_test.go
index 4694ab6..f931770 100644
--- a/runtimes/google/ipc/jni/arg_getter_test.go
+++ b/runtimes/google/ipc/jni/arg_getter_test.go
@@ -19,7 +19,7 @@
func compareTypes(t *testing.T, method string, got, want []interface{}, argKind string) {
if len(got) != len(want) {
- t.Errorf("mismatch in input arguments: got %v , want %v ", got, want)
+ t.Errorf("mismatch in input arguments: got %v , want %v ", got, want)
}
for i, _ := range got {
compareType(t, method, got[i], want[i], argKind)
@@ -35,12 +35,12 @@
if got, want := getter.vdlPath, iface; got != want {
t.Errorf("invalid pathname: got %v , want %v ", got, want)
}
- data := []struct{
- Method string
- NumInArgs int
- in, out []interface{}
+ data := []struct {
+ Method string
+ NumInArgs int
+ in, out []interface{}
sSend, sRecv interface{}
- sFinish []interface{}
+ sFinish []interface{}
}{
{"MethodA1", 0, nil, nil, nil, nil, nil},
{"MethodA2", 2, []interface{}{(*int32)(nil), (*string)(nil)}, []interface{}{(*string)(nil)}, nil, nil, nil},
@@ -61,4 +61,4 @@
compareType(t, d.Method, m.StreamRecvPtr(), d.sRecv, "stream recv")
compareTypes(t, d.Method, m.StreamFinishPtrs(), d.sFinish, "stream finish")
}
-}
\ No newline at end of file
+}
diff --git a/runtimes/google/ipc/server.go b/runtimes/google/ipc/server.go
index ffa9a11..00d1290 100644
--- a/runtimes/google/ipc/server.go
+++ b/runtimes/google/ipc/server.go
@@ -4,6 +4,7 @@
"fmt"
"io"
"net"
+ "reflect"
"strings"
"sync"
"time"
@@ -246,6 +247,7 @@
// authorizedRemoteID is the PublicID obtained after authorizing the remoteID
// of the underlying flow for the current request context.
authorizedRemoteID security.PublicID
+ blessing security.PublicID
method, name, suffix string
label security.Label
discharges security.CaveatDischargeMap
@@ -364,6 +366,20 @@
return nil, verr
}
}
+ // If additional credentials are provided, make them available in the context
+ if req.HasBlessing {
+ if err := fs.dec.Decode(&fs.blessing); err != nil {
+ return nil, verror.BadProtocolf("ipc: blessing decoding failed: %v", err)
+ }
+ // Detect unusable blessings now, rather then discovering they are unusable on first use.
+ if !reflect.DeepEqual(fs.blessing.PublicKey(), fs.flow.LocalID().PublicKey()) {
+ return nil, verror.BadProtocolf("ipc: blessing provided not bound to this server")
+ }
+ // TODO(ashankar,ataly): Potential confused deputy attack: The client provides the
+ // server's identity as the blessing. Figure out what we want to do about this -
+ // should servers be able to assume that a blessing is something that does not
+ // have the authorizations that the server's own identity has?
+ }
// Lookup the invoker.
invoker, auth, name, suffix, verr := fs.lookup(req.Suffix)
fs.name = name
@@ -485,6 +501,7 @@
func (fs *flowServer) LocalID() security.PublicID { return fs.flow.LocalID() }
func (fs *flowServer) RemoteID() security.PublicID { return fs.authorizedRemoteID }
func (fs *flowServer) Deadline() time.Time { return fs.deadline }
+func (fs *flowServer) Blessing() security.PublicID { return fs.blessing }
func (fs *flowServer) LocalAddr() net.Addr { return fs.flow.LocalAddr() }
func (fs *flowServer) RemoteAddr() net.Addr { return fs.flow.RemoteAddr() }
diff --git a/services/store/memstore/blackbox/util.go b/services/store/memstore/blackbox/util.go
index cc2c921..9a4af7c 100644
--- a/services/store/memstore/blackbox/util.go
+++ b/services/store/memstore/blackbox/util.go
@@ -62,6 +62,10 @@
return rootPublicID
}
+func (rootContext) Blessing() security.PublicID {
+ return nil
+}
+
func (rootContext) LocalAddr() net.Addr {
return nil
}
diff --git a/services/store/memstore/util_test.go b/services/store/memstore/util_test.go
index 261356e..0917a4a 100644
--- a/services/store/memstore/util_test.go
+++ b/services/store/memstore/util_test.go
@@ -56,6 +56,10 @@
return rootPublicID
}
+func (*cancellableContext) Blessing() security.PublicID {
+ return nil
+}
+
func (*cancellableContext) LocalAddr() net.Addr {
return nil
}
diff --git a/services/store/memstore/watch/test_util.go b/services/store/memstore/watch/test_util.go
index 5f10875..ac34f91 100644
--- a/services/store/memstore/watch/test_util.go
+++ b/services/store/memstore/watch/test_util.go
@@ -78,6 +78,10 @@
return nil
}
+func (rootContext) Blessing() security.PublicID {
+ return nil
+}
+
type cancellableContext struct {
rootContext
diff --git a/services/store/server/server_test.go b/services/store/server/server_test.go
index 4f26b36..8acc8d4 100644
--- a/services/store/server/server_test.go
+++ b/services/store/server/server_test.go
@@ -67,6 +67,10 @@
return ctx.id
}
+func (*testContext) Blessing() security.PublicID {
+ return nil
+}
+
func (*testContext) LocalAddr() net.Addr {
return nil
}
