veyron/runtimes/google/ipc: The blessings the server is started with
are the its blessings FOREVER!
If one wants to use new blessings they need to create a new server
with those blessings.
Change-Id: I49a06b5cfe367d728f37c0b4dc43f4a4d7a4e808
diff --git a/runtimes/google/naming/namespace/all_test.go b/runtimes/google/naming/namespace/all_test.go
index 823b547..29e7bcc 100644
--- a/runtimes/google/naming/namespace/all_test.go
+++ b/runtimes/google/naming/namespace/all_test.go
@@ -715,23 +715,19 @@
}
// Intermediate mounttables should be authenticated.
- // Simulate an "attacker" by changing the blessings presented by this
- // intermediate mounttable (which will not be consistent with the
- // blessing pattern in the mount entry). A subtle annoyance here: This
- // "attack" is valid only until the child mounttable remounts itself.
- // Currently this remounting period is set to 1 minute (publishPeriod
- // in ipc/consts.go), but if that changes - then this test may need
- // to change as well.
- runMT(t, mtCtx, "mt")
- attacker, err := veyron2.GetPrincipal(mtCtx).BlessSelf("attacker")
- if err != nil {
- t.Fatal(err)
- }
+ mt := runMT(t, mtCtx, "mt")
+ // Mount a server on "mt".
if err := mount("mt/server", ep1, time.Minute, naming.ReplaceMountOpt(true)); err != nil {
t.Error(err)
- } else if err := veyron2.GetPrincipal(mtCtx).BlessingStore().SetDefault(attacker); err != nil {
+ }
+ // Imagine that the network address of "mt" has been taken over by an attacker. However, this attacker cannot
+ // mess with the mount entry for "mt". This would result in "mt" and its mount entry (in the global mounttable)
+ // having inconsistent blessings. Simulate this by explicitly changing the mount entry for "mt".
+ if err := veyron2.GetNamespace(mtCtx).Mount(mtCtx, "mt", mt.name, time.Minute, naming.ServesMountTableOpt(true), naming.MountedServerBlessingsOpt{"realmounttable"}, naming.ReplaceMountOpt(true)); err != nil {
t.Error(err)
- } else if e, err := resolve("mt/server", options.SkipResolveAuthorization{}); err != nil {
+ }
+
+ if e, err := resolve("mt/server", options.SkipResolveAuthorization{}); err != nil {
t.Errorf("Resolve should succeed when skipping server authorization. Got (%v, %v)", e, err)
} else if e, err := resolve("mt/server"); !verror.Is(err, verror.ErrNotTrusted.ID) {
t.Errorf("Resolve should have failed with %q because an attacker has taken over the intermediate mounttable. Got (%+v, errorid=%q:%v)", verror.ErrNotTrusted.ID, e, verror.ErrorID(err), err)
diff --git a/runtimes/google/rt/ipc_test.go b/runtimes/google/rt/ipc_test.go
index 3758881..42e3d57 100644
--- a/runtimes/google/rt/ipc_test.go
+++ b/runtimes/google/rt/ipc_test.go
@@ -161,26 +161,21 @@
}
}
}
- // Start the server process.
- server, serverObjectName, err := startServer(serverCtx, testService{})
- if err != nil {
- t.Fatal(err)
- }
- defer server.Stop()
-
// Let it rip!
for _, test := range tests {
- // TODO(ashankar,suharshs): Once blessings are exchanged "per-RPC", one client for all cases will suffice.
- // Also, we need server to lameduck VCs when server.BlessingStore().Default() has changed
- // for one client to be sufficient.
- ctx, client, err := veyron2.SetNewClient(clientCtx)
- if err != nil {
- panic(err)
- }
if err := pserver.BlessingStore().SetDefault(test.server); err != nil {
t.Errorf("pserver.SetDefault(%v) failed: %v", test.server, err)
continue
}
+ server, serverObjectName, err := startServer(serverCtx, testService{})
+ if err != nil {
+ t.Fatal(err)
+ }
+ ctx, client, err := veyron2.SetNewClient(clientCtx)
+ if err != nil {
+ panic(err)
+ }
+
var gotClient []string
if call, err := client.StartCall(ctx, serverObjectName, "EchoBlessings", nil); err != nil {
t.Errorf("client.StartCall failed: %v", err)
@@ -191,6 +186,9 @@
} else if gotServer, _ := call.RemoteBlessings(); !reflect.DeepEqual(gotServer, test.wantServer) {
t.Errorf("%v: Got %v, want %v for server blessings", test.server, gotServer, test.wantClient)
}
+
+ server.Stop()
+ client.Close()
}
}
@@ -206,25 +204,24 @@
root = tsecurity.NewIDProvider("root")
)
- // Start the server and discharge server.
- server, serverName, err := startServer(serverCtx, &testService{})
- if err != nil {
+ // Setup the server's and discharger's blessing store and blessing roots, and
+ // start the server and discharger.
+ if err := root.Bless(pdischarger, "discharger"); err != nil {
t.Fatal(err)
}
- defer server.Stop()
dischargeServer, dischargeServerName, err := startServer(dischargerCtx, &dischargeService{})
if err != nil {
t.Fatal(err)
}
defer dischargeServer.Stop()
-
- // Setup the server's and discharger's blessing store and blessing roots.
if err := root.Bless(pserver, "server", mkThirdPartyCaveat(pdischarger.PublicKey(), dischargeServerName)); err != nil {
t.Fatal(err)
}
- if err := root.Bless(pdischarger, "discharger"); err != nil {
+ server, serverName, err := startServer(serverCtx, &testService{})
+ if err != nil {
t.Fatal(err)
}
+ defer server.Stop()
// Setup up the client's blessing store so that it can talk to the server.
rootClient := mkBlessings(root.NewBlessings(pclient, "client"))
diff --git a/runtimes/google/rt/runtime.go b/runtimes/google/rt/runtime.go
index af853c3..d31348c 100644
--- a/runtimes/google/rt/runtime.go
+++ b/runtimes/google/rt/runtime.go
@@ -14,6 +14,7 @@
"v.io/core/veyron2/i18n"
"v.io/core/veyron2/ipc"
"v.io/core/veyron2/naming"
+ "v.io/core/veyron2/options"
"v.io/core/veyron2/security"
"v.io/core/veyron2/verror"
"v.io/core/veyron2/vlog"
@@ -225,6 +226,10 @@
if protocols, ok := ctx.Value(protocolsKey).([]string); ok {
otherOpts = append(otherOpts, iipc.PreferredServerResolveProtocols(protocols))
}
+
+ if !hasServerBlessingsOpt(opts) && principal != nil {
+ otherOpts = append(otherOpts, options.ServerBlessings{principal.BlessingStore().Default()})
+ }
server, err := iipc.InternalNewServer(ctx, sm, ns, r.GetClient(ctx), otherOpts...)
if err != nil {
return nil, err
@@ -241,6 +246,15 @@
return server, nil
}
+func hasServerBlessingsOpt(opts []ipc.ServerOpt) bool {
+ for _, o := range opts {
+ if _, ok := o.(options.ServerBlessings); ok {
+ return true
+ }
+ }
+ return false
+}
+
func newStreamManager() (stream.Manager, error) {
rid, err := naming.NewRoutingID()
if err != nil {
