ipc: VCSecurityLevel->Security and no longer a stream opt.
SecurityLevel is not handled only in server.go and client.go.
Everywhere else in the ipc stack, nil principal means SecurityNone.
This also fixes https://github.com/veyron/release-issues/issues/1423
in the process.
MultiPart: 1/2
Change-Id: I55646e574e43b633c69300b1fecb623c95340bbc
diff --git a/cmd/vrun/vrun_v23_test.go b/cmd/vrun/vrun_v23_test.go
index 161b84d..bcd4dd4 100644
--- a/cmd/vrun/vrun_v23_test.go
+++ b/cmd/vrun/vrun_v23_test.go
@@ -92,9 +92,8 @@
if err := pclient.AddToRoots(bserver); err != nil {
i.Fatal(err)
}
- // TODO(ashankar,ribrdb,suharshs): This should not be needed. It seems
- // however, that not providing it messes up the agent: Specifically,
- // the child process is unable to connect to the agent?
+ // We need to add set a default blessings to this pclient for this principal to
+ // work with vrun.
if err := pclient.BlessingStore().SetDefault(bclient); err != nil {
i.Fatal(err)
}
diff --git a/profiles/internal/rpc/blessings_cache.go b/profiles/internal/rpc/blessings_cache.go
index 4d41476..7f5fe9f 100644
--- a/profiles/internal/rpc/blessings_cache.go
+++ b/profiles/internal/rpc/blessings_cache.go
@@ -132,7 +132,7 @@
}
func (c *serverBlessingsCache) getOrInsert(req rpc.BlessingsRequest, stats *rpcStats) (security.Blessings, error) {
- // In the case that the key sent is 0, we are running in VCSecurityNone
+ // In the case that the key sent is 0, we are running in SecurityNone
// and should return the zero value.
if req.Key == 0 {
return security.Blessings{}, nil
diff --git a/profiles/internal/rpc/client.go b/profiles/internal/rpc/client.go
index 961c076..61eaaab 100644
--- a/profiles/internal/rpc/client.go
+++ b/profiles/internal/rpc/client.go
@@ -112,7 +112,7 @@
type vcMapKey struct {
endpoint string
- clientPublicKey string // clientPublicKey = "" means we are running unencrypted (i.e. VCSecurityNone)
+ clientPublicKey string // clientPublicKey = "" means we are running unencrypted (i.e. SecurityNone)
}
func InternalNewClient(streamMgr stream.Manager, ns ns.Namespace, opts ...rpc.ClientOpt) (rpc.Client, error) {
@@ -136,17 +136,13 @@
return c, nil
}
-func (c *client) createFlow(ctx *context.T, ep naming.Endpoint, vcOpts []stream.VCOpt) (stream.Flow, error) {
+func (c *client) createFlow(ctx *context.T, principal security.Principal, ep naming.Endpoint, vcOpts []stream.VCOpt) (stream.Flow, error) {
c.vcMapMu.Lock()
defer c.vcMapMu.Unlock()
if c.vcMap == nil {
return nil, verror.New(errClientCloseAlreadyCalled, ctx)
}
vcKey := vcMapKey{endpoint: ep.String()}
- var principal security.Principal
- if vcEncrypted(vcOpts) {
- principal = v23.GetPrincipal(ctx)
- }
if principal != nil {
vcKey.clientPublicKey = principal.PublicKey().String()
}
@@ -315,7 +311,7 @@
// authorizer, both during creation of the VC underlying the flow and the
// flow itself.
// TODO(cnicolaou): implement real, configurable load balancing.
-func (c *client) tryCreateFlow(ctx *context.T, index int, name, server, method string, auth security.Authorizer, ch chan<- *serverStatus, vcOpts []stream.VCOpt) {
+func (c *client) tryCreateFlow(ctx *context.T, principal security.Principal, index int, name, server, method string, auth security.Authorizer, ch chan<- *serverStatus, vcOpts []stream.VCOpt) {
status := &serverStatus{index: index}
var span vtrace.Span
ctx, span = vtrace.SetNewSpan(ctx, "<client>tryCreateFlow")
@@ -341,7 +337,7 @@
status.err = verror.New(errIncompatibleEndpoint, ctx, ep)
return
}
- if status.flow, status.err = c.createFlow(ctx, ep, append(vcOpts, &vc.ServerAuthorizer{Suffix: status.suffix, Method: method, Policy: auth})); status.err != nil {
+ if status.flow, status.err = c.createFlow(ctx, principal, ep, append(vcOpts, &vc.ServerAuthorizer{Suffix: status.suffix, Method: method, Policy: auth})); status.err != nil {
vlog.VI(2).Infof("rpc: Failed to create Flow with %v: %v", server, status.err)
return
}
@@ -349,7 +345,7 @@
// Authorize the remote end of the flow using the provided authorizer.
if status.flow.LocalPrincipal() == nil {
// LocalPrincipal is nil which means we are operating under
- // VCSecurityNone.
+ // SecurityNone.
return
}
@@ -406,6 +402,23 @@
}
}
+ // We need to ensure calls to v23 factory methods do not occur during runtime
+ // initialization. Currently, the agent, which uses SecurityNone, is the only caller
+ // during runtime initialization. We would like to set the principal in the context
+ // to nil if we are running in SecurityNone, but this always results in a panic since
+ // the agent client would trigger the call v23.SetPrincipal during runtime
+ // initialization. So, we gate the call to v23.GetPrincipal instead since the agent
+ // client will have callEncrypted == false.
+ // Potential solutions to this are:
+ // (1) Create a separate client for the agent so that this code doesn't have to
+ // account for its use during runtime initialization.
+ // (2) Have a ctx.IsRuntimeInitialized() method that we can additionally predicate
+ // on here.
+ var principal security.Principal
+ if callEncrypted(opts) {
+ principal = v23.GetPrincipal(ctx)
+ }
+
// servers is now ordered by the priority heurestic implemented in
// filterAndOrderServers.
//
@@ -427,7 +440,7 @@
// other while manipulating their copy of the options.
vcOptsCopy := make([]stream.VCOpt, len(vcOpts))
copy(vcOptsCopy, vcOpts)
- go c.tryCreateFlow(ctx, i, name, server, method, authorizer, ch, vcOptsCopy)
+ go c.tryCreateFlow(ctx, principal, i, name, server, method, authorizer, ch, vcOptsCopy)
}
var timeoutChan <-chan time.Time
@@ -598,7 +611,7 @@
// blessings.
func (fc *flowClient) prepareBlessingsAndDischarges(method string, args []interface{}, rejectedServerBlessings []security.RejectedBlessing, opts []rpc.CallOpt) error {
// LocalPrincipal is nil which means we are operating under
- // VCSecurityNone.
+ // SecurityNone.
if fc.flow.LocalPrincipal() == nil {
return nil
}
diff --git a/profiles/internal/rpc/full_test.go b/profiles/internal/rpc/full_test.go
index aa1d52d..6789fda 100644
--- a/profiles/internal/rpc/full_test.go
+++ b/profiles/internal/rpc/full_test.go
@@ -1608,7 +1608,7 @@
sm := imanager.InternalNew(naming.FixedRoutingID(0x66666666))
defer sm.Shutdown()
ns := tnaming.NewSimpleNamespace()
- server, err := testInternalNewServer(ctx, sm, ns, nil, options.VCSecurityNone)
+ server, err := testInternalNewServer(ctx, sm, ns, nil, options.SecurityNone)
if err != nil {
t.Fatalf("InternalNewServer failed: %v", err)
}
@@ -1623,9 +1623,9 @@
if err != nil {
t.Fatalf("InternalNewClient failed: %v", err)
}
- // When using VCSecurityNone, all authorization checks should be skipped, so
+ // When using SecurityNone, all authorization checks should be skipped, so
// unauthorized methods should be callable.
- call, err := client.StartCall(ctx, "mp/server", "Unauthorized", nil, options.VCSecurityNone)
+ call, err := client.StartCall(ctx, "mp/server", "Unauthorized", nil, options.SecurityNone)
if err != nil {
t.Fatalf("client.StartCall failed: %v", err)
}
@@ -1646,7 +1646,7 @@
if err != nil {
t.Fatalf("InternalNewClient failed: %v", err)
}
- call, err := client.StartCall(nil, "foo", "bar", []interface{}{}, options.VCSecurityNone)
+ call, err := client.StartCall(nil, "foo", "bar", []interface{}{}, options.SecurityNone)
if call != nil {
t.Errorf("Expected nil interface got: %#v", call)
}
diff --git a/profiles/internal/rpc/options.go b/profiles/internal/rpc/options.go
index 148dbfb..d4f91b0 100644
--- a/profiles/internal/rpc/options.go
+++ b/profiles/internal/rpc/options.go
@@ -85,13 +85,13 @@
return
}
-func vcEncrypted(vcOpts []stream.VCOpt) bool {
+func callEncrypted(opts []rpc.CallOpt) bool {
encrypted := true
- for _, o := range vcOpts {
+ for _, o := range opts {
switch o {
- case options.VCSecurityNone:
+ case options.SecurityNone:
encrypted = false
- case options.VCSecurityConfidential:
+ case options.SecurityConfidential:
encrypted = true
}
}
diff --git a/profiles/internal/rpc/server.go b/profiles/internal/rpc/server.go
index a21037e..79c97e0 100644
--- a/profiles/internal/rpc/server.go
+++ b/profiles/internal/rpc/server.go
@@ -170,7 +170,10 @@
ns: ns,
stats: newRPCStats(statsPrefix),
}
- var dischargeExpiryBuffer = vc.DefaultServerDischargeExpiryBuffer
+ var (
+ dischargeExpiryBuffer = vc.DefaultServerDischargeExpiryBuffer
+ securityLevel options.SecurityLevel
+ )
for _, opt := range opts {
switch opt := opt.(type) {
case stream.ListenerOpt:
@@ -188,11 +191,18 @@
s.dispReserved = opt.Dispatcher
case PreferredServerResolveProtocols:
s.preferredProtocols = []string(opt)
+ case options.SecurityLevel:
+ securityLevel = opt
}
}
if s.blessings.IsZero() && principal != nil {
s.blessings = principal.BlessingStore().Default()
}
+ if securityLevel == options.SecurityNone {
+ s.principal = nil
+ s.blessings = security.Blessings{}
+ s.dispReserved = nil
+ }
// Make dischargeExpiryBuffer shorter than the VC discharge buffer to ensure we have fetched
// the discharges by the time the VC asks for them.`
dc := InternalNewDischargeClient(ctx, client, dischargeExpiryBuffer-(5*time.Second))
@@ -202,7 +212,7 @@
// TODO(caprita): revist printing the blessings with %s, and
// instead expose them as a list.
stats.NewString(blessingsStatsName).Set(fmt.Sprintf("%s", s.blessings))
- if principal != nil { // principal should have been passed in, but just in case.
+ if principal != nil {
stats.NewStringFunc(blessingsStatsName, func() string {
return fmt.Sprintf("%s (default)", principal.BlessingStore().Default())
})
@@ -1142,7 +1152,7 @@
func (fs *flowServer) initSecurity(req *rpc.Request) error {
// LocalPrincipal is nil which means we are operating under
- // VCSecurityNone.
+ // SecurityNone.
if fs.flow.LocalPrincipal() == nil {
return nil
}
diff --git a/profiles/internal/rpc/stream/benchmark/dial_vc.go b/profiles/internal/rpc/stream/benchmark/dial_vc.go
index a759fcf..a638288 100644
--- a/profiles/internal/rpc/stream/benchmark/dial_vc.go
+++ b/profiles/internal/rpc/stream/benchmark/dial_vc.go
@@ -11,24 +11,32 @@
"v.io/v23/naming"
"v.io/v23/options"
+ "v.io/v23/security"
)
// benchmarkDialVC measures VC creation time over the underlying VIF.
-func benchmarkDialVC(b *testing.B, mode options.VCSecurityLevel) {
+func benchmarkDialVC(b *testing.B, mode options.SecurityLevel) {
stats := benchmark.AddStats(b, 16)
server := manager.InternalNew(naming.FixedRoutingID(0x5))
client := manager.InternalNew(naming.FixedRoutingID(0xc))
- principal := testutil.NewPrincipal("test")
+ var (
+ principal security.Principal
+ blessings security.Blessings
+ )
+ if mode == securityTLS {
+ principal = testutil.NewPrincipal("test")
+ blessings = principal.BlessingStore().Default()
+ }
- _, ep, err := server.Listen("tcp", "127.0.0.1:0", principal, principal.BlessingStore().Default(), mode)
+ _, ep, err := server.Listen("tcp", "127.0.0.1:0", principal, blessings)
if err != nil {
b.Fatal(err)
}
// Warmup to create the underlying VIF.
- _, err = client.Dial(ep, principal, mode)
+ _, err = client.Dial(ep, principal)
if err != nil {
b.Fatal(err)
}
@@ -39,7 +47,7 @@
b.StartTimer()
start := time.Now()
- _, err := client.Dial(ep, principal, mode)
+ _, err := client.Dial(ep, principal)
if err != nil {
b.Fatal(err)
}
diff --git a/profiles/internal/rpc/stream/benchmark/dial_vif.go b/profiles/internal/rpc/stream/benchmark/dial_vif.go
index 8c646c3..49041ac 100644
--- a/profiles/internal/rpc/stream/benchmark/dial_vif.go
+++ b/profiles/internal/rpc/stream/benchmark/dial_vif.go
@@ -11,13 +11,20 @@
"v.io/v23/naming"
"v.io/v23/options"
+ "v.io/v23/security"
)
// benchmarkDialVIF measures VIF creation time over the underlying net connection.
-func benchmarkDialVIF(b *testing.B, mode options.VCSecurityLevel) {
+func benchmarkDialVIF(b *testing.B, mode options.SecurityLevel) {
stats := benchmark.AddStats(b, 16)
- principal := testutil.NewPrincipal("test")
- blessings := principal.BlessingStore().Default()
+ var (
+ principal security.Principal
+ blessings security.Blessings
+ )
+ if mode == securityTLS {
+ principal = testutil.NewPrincipal("test")
+ blessings = principal.BlessingStore().Default()
+ }
b.ResetTimer() // Exclude setup time from measurement.
@@ -25,7 +32,7 @@
b.StopTimer()
nc, ns := net.Pipe()
- server, err := vif.InternalNewAcceptedVIF(ns, naming.FixedRoutingID(0x5), principal, blessings, nil, mode)
+ server, err := vif.InternalNewAcceptedVIF(ns, naming.FixedRoutingID(0x5), principal, blessings, nil)
if err != nil {
b.Fatal(err)
}
@@ -33,7 +40,7 @@
b.StartTimer()
start := time.Now()
- client, err := vif.InternalNewDialedVIF(nc, naming.FixedRoutingID(0xc), principal, nil, mode)
+ client, err := vif.InternalNewDialedVIF(nc, naming.FixedRoutingID(0xc), principal, nil)
if err != nil {
b.Fatal(err)
}
diff --git a/profiles/internal/rpc/stream/benchmark/throughput_flow.go b/profiles/internal/rpc/stream/benchmark/throughput_flow.go
index afc6884..8ff0ac3 100644
--- a/profiles/internal/rpc/stream/benchmark/throughput_flow.go
+++ b/profiles/internal/rpc/stream/benchmark/throughput_flow.go
@@ -8,14 +8,15 @@
"v.io/v23/naming"
"v.io/v23/options"
+ "v.io/v23/security"
"v.io/x/ref/profiles/internal/rpc/stream"
"v.io/x/ref/test/testutil"
)
const (
// Shorthands
- securityNone = options.VCSecurityNone
- securityTLS = options.VCSecurityConfidential
+ securityNone = options.SecurityNone
+ securityTLS = options.SecurityConfidential
)
type listener struct {
@@ -26,11 +27,18 @@
// createListeners returns N (stream.Listener, naming.Endpoint) pairs, such
// that calling stream.Manager.Dial to each of the endpoints will end up
// creating a new VIF.
-func createListeners(mode options.VCSecurityLevel, m stream.Manager, N int) (servers []listener, err error) {
+func createListeners(mode options.SecurityLevel, m stream.Manager, N int) (servers []listener, err error) {
for i := 0; i < N; i++ {
- var l listener
- principal := testutil.NewPrincipal("test")
- if l.ln, l.ep, err = m.Listen("tcp", "127.0.0.1:0", principal, principal.BlessingStore().Default(), mode); err != nil {
+ var (
+ l listener
+ principal security.Principal
+ blessings security.Blessings
+ )
+ if mode == securityTLS {
+ principal = testutil.NewPrincipal("test")
+ blessings = principal.BlessingStore().Default()
+ }
+ if l.ln, l.ep, err = m.Listen("tcp", "127.0.0.1:0", principal, blessings); err != nil {
return
}
servers = append(servers, l)
@@ -38,10 +46,14 @@
return
}
-func benchmarkFlow(b *testing.B, mode options.VCSecurityLevel, nVIFs, nVCsPerVIF, nFlowsPerVC int) {
+func benchmarkFlow(b *testing.B, mode options.SecurityLevel, nVIFs, nVCsPerVIF, nFlowsPerVC int) {
client := manager.InternalNew(naming.FixedRoutingID(0xcccccccc))
server := manager.InternalNew(naming.FixedRoutingID(0x55555555))
- principal := testutil.NewPrincipal("test")
+
+ var principal security.Principal
+ if mode == securityTLS {
+ principal = testutil.NewPrincipal("test")
+ }
lns, err := createListeners(mode, server, nVIFs)
if err != nil {
@@ -59,7 +71,7 @@
for i := 0; i < nVIFs; i++ {
ep := lns[i].ep
for j := 0; j < nVCsPerVIF; j++ {
- vc, err := client.Dial(ep, principal, mode)
+ vc, err := client.Dial(ep, principal)
if err != nil {
b.Error(err)
return
diff --git a/profiles/internal/rpc/stream/manager/manager_test.go b/profiles/internal/rpc/stream/manager/manager_test.go
index 3426f76..c36a6e0 100644
--- a/profiles/internal/rpc/stream/manager/manager_test.go
+++ b/profiles/internal/rpc/stream/manager/manager_test.go
@@ -179,8 +179,6 @@
clientKey = clientPrincipal.PublicKey()
serverBlessings = serverPrincipal.BlessingStore().Default()
)
- // VCSecurityLevel is intentionally not provided to Listen - to test
- // default behavior.
ln, ep, err := server.Listen(protocol, "127.0.0.1:0", serverPrincipal, serverPrincipal.BlessingStore().Default())
if err != nil {
t.Fatal(err)
@@ -219,8 +217,6 @@
}()
go func() {
- // VCSecurityLevel is intentionally not provided to Dial - to
- // test default behavior.
vc, err := client.Dial(ep, clientPrincipal)
if err != nil {
errs <- err
diff --git a/profiles/internal/rpc/stream/vc/vc.go b/profiles/internal/rpc/stream/vc/vc.go
index b11ed56..4d0c65f 100644
--- a/profiles/internal/rpc/stream/vc/vc.go
+++ b/profiles/internal/rpc/stream/vc/vc.go
@@ -21,7 +21,6 @@
"v.io/v23/context"
"v.io/v23/naming"
- "v.io/v23/options"
"v.io/v23/rpc/version"
"v.io/v23/security"
"v.io/v23/vom"
@@ -400,31 +399,23 @@
// authentication etc.) under the assumption that the VC was initiated by the
// local process (i.e., the local process "Dial"ed to create the VC).
func (vc *VC) HandshakeDialedVC(principal security.Principal, opts ...stream.VCOpt) error {
+ // principal = nil means that we are running in SecurityNone and we don't need
+ // to authenticate the VC.
+ if principal == nil {
+ return nil
+ }
var (
tlsSessionCache crypto.TLSClientSessionCache
- securityLevel options.VCSecurityLevel
auth *ServerAuthorizer
)
for _, o := range opts {
switch v := o.(type) {
- case options.VCSecurityLevel:
- securityLevel = v
case crypto.TLSClientSessionCache:
tlsSessionCache = v
case *ServerAuthorizer:
auth = v
}
}
- switch securityLevel {
- case options.VCSecurityConfidential:
- if principal == nil {
- return fmt.Errorf("principal required for VCSecurityConfidential")
- }
- case options.VCSecurityNone:
- return nil
- default:
- return fmt.Errorf("unrecognized VC security level: %v", securityLevel)
- }
// Establish TLS
handshakeFID := vc.allocFID()
@@ -504,17 +495,13 @@
return result
}
var (
- securityLevel options.VCSecurityLevel
- dischargeClient DischargeClient
-
+ dischargeClient DischargeClient
dischargeExpiryBuffer = DefaultServerDischargeExpiryBuffer
)
for _, o := range opts {
switch v := o.(type) {
case DischargeClient:
dischargeClient = v
- case options.VCSecurityLevel:
- securityLevel = v
case DischargeExpiryBuffer:
dischargeExpiryBuffer = time.Duration(v)
}
@@ -527,20 +514,13 @@
return finish(nil, err)
}
vc.helper.AddReceiveBuffers(vc.VCI(), SharedFlowID, DefaultBytesBufferedPerFlow)
- switch securityLevel {
- case options.VCSecurityConfidential:
- if principal == nil {
- return finish(nil, fmt.Errorf("principal required for VCSecurityConfidential"))
- }
- if lBlessings.IsZero() {
- return finish(nil, fmt.Errorf("blessings required for VCSecurityConfidential"))
- }
- case options.VCSecurityNone:
+
+ // principal = nil means that we are running in SecurityNone and we don't need
+ // to authenticate the VC.
+ if principal == nil {
return finish(ln, nil)
- default:
- ln.Close()
- return finish(nil, fmt.Errorf("unrecognized VC security level: %v", securityLevel))
}
+
go func() {
sendErr := func(err error) {
ln.Close()
diff --git a/profiles/internal/rpc/stream/vc/vc_test.go b/profiles/internal/rpc/stream/vc/vc_test.go
index bb90e51..bb407e2 100644
--- a/profiles/internal/rpc/stream/vc/vc_test.go
+++ b/profiles/internal/rpc/stream/vc/vc_test.go
@@ -42,12 +42,20 @@
// Convenience alias to avoid conflicts between the package name "vc" and variables called "vc".
DefaultBytesBufferedPerFlow = vc.DefaultBytesBufferedPerFlow
// Shorthands
- SecurityNone = options.VCSecurityNone
- SecurityTLS = options.VCSecurityConfidential
+ SecurityNone = options.SecurityNone
+ SecurityTLS = options.SecurityConfidential
LatestVersion = version.RPCVersion7
)
+func createPrincipals(securityLevel options.SecurityLevel) (client, server security.Principal) {
+ if securityLevel == SecurityTLS {
+ client = testutil.NewPrincipal("client")
+ server = testutil.NewPrincipal("server")
+ }
+ return
+}
+
// testFlowEcho writes a random string of 'size' bytes on the flow and then
// ensures that the same string is read back.
func testFlowEcho(t *testing.T, flow stream.Flow, size int) {
@@ -83,12 +91,8 @@
}
func TestHandshake(t *testing.T) {
- // When SecurityNone is used, the blessings should not be sent over the wire.
- var (
- client = testutil.NewPrincipal("client")
- server = testutil.NewPrincipal("server")
- )
- h, vc, err := New(SecurityNone, LatestVersion, client, server, nil, nil)
+ // When the principals are nil, no blessings should be sent over the wire.
+ h, vc, err := New(LatestVersion, nil, nil, nil, nil)
if err != nil {
t.Fatal(err)
}
@@ -247,7 +251,7 @@
},
}
for i, d := range testdata {
- h, vc, err := New(SecurityTLS, LatestVersion, client, server, d.dischargeClient, d.auth)
+ h, vc, err := New(LatestVersion, client, server, d.dischargeClient, d.auth)
if merr := matchesError(err, d.dialErr); merr != nil {
t.Errorf("Test #%d: HandshakeDialedVC with server authorizer %#v:: %v", i, d.auth.Policy, merr)
}
@@ -269,8 +273,9 @@
}
}
-func testConnect_Small(t *testing.T, security options.VCSecurityLevel) {
- h, vc, err := New(security, LatestVersion, testutil.NewPrincipal("client"), testutil.NewPrincipal("server"), nil, nil)
+func testConnect_Small(t *testing.T, securityLevel options.SecurityLevel) {
+ pclient, pserver := createPrincipals(securityLevel)
+ h, vc, err := New(LatestVersion, pclient, pserver, nil, nil)
if err != nil {
t.Fatal(err)
}
@@ -284,8 +289,9 @@
func TestConnect_Small(t *testing.T) { testConnect_Small(t, SecurityNone) }
func TestConnect_SmallTLS(t *testing.T) { testConnect_Small(t, SecurityTLS) }
-func testConnect(t *testing.T, security options.VCSecurityLevel) {
- h, vc, err := New(security, LatestVersion, testutil.NewPrincipal("client"), testutil.NewPrincipal("server"), nil, nil)
+func testConnect(t *testing.T, securityLevel options.SecurityLevel) {
+ pclient, pserver := createPrincipals(securityLevel)
+ h, vc, err := New(LatestVersion, pclient, pserver, nil, nil)
if err != nil {
t.Fatal(err)
}
@@ -299,8 +305,9 @@
func TestConnect(t *testing.T) { testConnect(t, SecurityNone) }
func TestConnectTLS(t *testing.T) { testConnect(t, SecurityTLS) }
-func testConnect_Version7(t *testing.T, security options.VCSecurityLevel) {
- h, vc, err := New(security, version.RPCVersion7, testutil.NewPrincipal("client"), testutil.NewPrincipal("server"), nil, nil)
+func testConnect_Version7(t *testing.T, securityLevel options.SecurityLevel) {
+ pclient, pserver := createPrincipals(securityLevel)
+ h, vc, err := New(version.RPCVersion7, pclient, pserver, nil, nil)
if err != nil {
t.Fatal(err)
}
@@ -317,10 +324,11 @@
// helper function for testing concurrent operations on multiple flows over the
// same VC. Such tests are most useful when running the race detector.
// (go test -race ...)
-func testConcurrentFlows(t *testing.T, security options.VCSecurityLevel, flows, gomaxprocs int) {
+func testConcurrentFlows(t *testing.T, securityLevel options.SecurityLevel, flows, gomaxprocs int) {
mp := runtime.GOMAXPROCS(gomaxprocs)
defer runtime.GOMAXPROCS(mp)
- h, vc, err := New(security, LatestVersion, testutil.NewPrincipal("client"), testutil.NewPrincipal("server"), nil, nil)
+ pclient, pserver := createPrincipals(securityLevel)
+ h, vc, err := New(LatestVersion, pclient, pserver, nil, nil)
if err != nil {
t.Fatal(err)
}
@@ -348,9 +356,10 @@
func TestConcurrentFlows_10(t *testing.T) { testConcurrentFlows(t, SecurityNone, 10, 10) }
func TestConcurrentFlows_10TLS(t *testing.T) { testConcurrentFlows(t, SecurityTLS, 10, 10) }
-func testListen(t *testing.T, security options.VCSecurityLevel) {
+func testListen(t *testing.T, securityLevel options.SecurityLevel) {
data := "the dark knight"
- h, vc, err := New(security, LatestVersion, testutil.NewPrincipal("client"), testutil.NewPrincipal("server"), nil, nil)
+ pclient, pserver := createPrincipals(securityLevel)
+ h, vc, err := New(LatestVersion, pclient, pserver, nil, nil)
if err != nil {
t.Fatal(err)
}
@@ -399,8 +408,9 @@
func TestListen(t *testing.T) { testListen(t, SecurityNone) }
func TestListenTLS(t *testing.T) { testListen(t, SecurityTLS) }
-func testNewFlowAfterClose(t *testing.T, security options.VCSecurityLevel) {
- h, _, err := New(security, LatestVersion, testutil.NewPrincipal("client"), testutil.NewPrincipal("server"), nil, nil)
+func testNewFlowAfterClose(t *testing.T, securityLevel options.SecurityLevel) {
+ pclient, pserver := createPrincipals(securityLevel)
+ h, _, err := New(LatestVersion, pclient, pserver, nil, nil)
if err != nil {
t.Fatal(err)
}
@@ -413,8 +423,9 @@
func TestNewFlowAfterClose(t *testing.T) { testNewFlowAfterClose(t, SecurityNone) }
func TestNewFlowAfterCloseTLS(t *testing.T) { testNewFlowAfterClose(t, SecurityTLS) }
-func testConnectAfterClose(t *testing.T, security options.VCSecurityLevel) {
- h, vc, err := New(security, LatestVersion, testutil.NewPrincipal("client"), testutil.NewPrincipal("server"), nil, nil)
+func testConnectAfterClose(t *testing.T, securityLevel options.SecurityLevel) {
+ pclient, pserver := createPrincipals(securityLevel)
+ h, vc, err := New(LatestVersion, pclient, pserver, nil, nil)
if err != nil {
t.Fatal(err)
}
@@ -439,7 +450,7 @@
// New creates both ends of a VC but returns only the "client" end (i.e., the
// one that initiated the VC). The "server" end (the one that "accepted" the VC)
// listens for flows and simply echoes data read.
-func New(security options.VCSecurityLevel, v version.RPCVersion, client, server security.Principal, dischargeClient vc.DischargeClient, auth *vc.ServerAuthorizer) (*helper, stream.VC, error) {
+func New(v version.RPCVersion, client, server security.Principal, dischargeClient vc.DischargeClient, auth *vc.ServerAuthorizer) (*helper, stream.VC, error) {
clientH := &helper{bq: drrqueue.New(vc.MaxPayloadSizeBytes)}
serverH := &helper{bq: drrqueue.New(vc.MaxPayloadSizeBytes)}
clientH.otherEnd = serverH
@@ -472,8 +483,10 @@
go clientH.pipeLoop(serverH.VC)
go serverH.pipeLoop(clientH.VC)
- lopts := []stream.ListenerOpt{security}
- vcopts := []stream.VCOpt{security}
+ var (
+ lopts []stream.ListenerOpt
+ vcopts []stream.VCOpt
+ )
if dischargeClient != nil {
lopts = append(lopts, dischargeClient)
@@ -482,7 +495,11 @@
vcopts = append(vcopts, auth)
}
- c := serverH.VC.HandshakeAcceptedVC(server, server.BlessingStore().Default(), lopts...)
+ var bserver security.Blessings
+ if server != nil {
+ bserver = server.BlessingStore().Default()
+ }
+ c := serverH.VC.HandshakeAcceptedVC(server, bserver, lopts...)
if err := clientH.VC.HandshakeDialedVC(client, vcopts...); err != nil {
go func() { <-c }()
return nil, nil, err
diff --git a/profiles/internal/rpc/stream/vif/auth.go b/profiles/internal/rpc/stream/vif/auth.go
index 465351b..8dd43ac 100644
--- a/profiles/internal/rpc/stream/vif/auth.go
+++ b/profiles/internal/rpc/stream/vif/auth.go
@@ -8,7 +8,6 @@
"golang.org/x/crypto/nacl/box"
- "v.io/v23/options"
rpcversion "v.io/v23/rpc/version"
"v.io/v23/security"
"v.io/x/ref/profiles/internal/lib/iobuf"
@@ -187,36 +186,16 @@
return c, nil
}
-// serverAuthOptions returns credentials for VIF authentication, based on the provided principal and options list.
-// TODO(suharshs): If/Once we pass dischargeClient explicitly, perhaps we should have a serverAuthParams struct
-// and not have this method at all.
-func serverAuthOptions(principal security.Principal, lBlessings security.Blessings, lopts []stream.ListenerOpt) (security.Principal, security.Blessings, vc.DischargeClient, error) {
- var (
- securityLevel options.VCSecurityLevel
- dischargeClient vc.DischargeClient
- )
+// getDischargeClient returns the dischargeClient needed to fetch server discharges for this call.
+// TODO(suharshs): Perhaps we should pass dischargeClient explicitly?
+func getDischargeClient(lopts []stream.ListenerOpt) vc.DischargeClient {
for _, o := range lopts {
switch v := o.(type) {
case vc.DischargeClient:
- dischargeClient = v
- case options.VCSecurityLevel:
- securityLevel = v
+ return v
}
}
- switch securityLevel {
- case options.VCSecurityConfidential:
- if principal == nil {
- return nil, security.Blessings{}, nil, fmt.Errorf("principal required for VCSecurityConfidential")
- }
- if lBlessings.IsZero() {
- return nil, security.Blessings{}, nil, fmt.Errorf("blessings required for VCSecurityConfidential")
- }
- return principal, lBlessings, dischargeClient, nil
- case options.VCSecurityNone:
- return nil, security.Blessings{}, nil, nil
- default:
- return nil, security.Blessings{}, nil, fmt.Errorf("unrecognized VC security level: %v", securityLevel)
- }
+ return nil
}
// makeHopSetup constructs the options that this process can support.
diff --git a/profiles/internal/rpc/stream/vif/vif.go b/profiles/internal/rpc/stream/vif/vif.go
index 6449209..9f46d03 100644
--- a/profiles/internal/rpc/stream/vif/vif.go
+++ b/profiles/internal/rpc/stream/vif/vif.go
@@ -16,7 +16,6 @@
"v.io/v23/context"
"v.io/v23/naming"
- "v.io/v23/options"
"v.io/v23/security"
"v.io/v23/verror"
"v.io/v23/vtrace"
@@ -133,10 +132,7 @@
// placed inside v.io/x/ref/profiles/internal. Code outside the
// v.io/x/ref/profiles/internal/* packages should never call this method.
func InternalNewDialedVIF(conn net.Conn, rid naming.RoutingID, principal security.Principal, versions *version.Range, opts ...stream.VCOpt) (*VIF, error) {
- ctx, principal, err := clientAuthOptions(principal, opts)
- if err != nil {
- return nil, err
- }
+ ctx := getDialContext(opts)
if ctx != nil {
var span vtrace.Span
ctx, span = vtrace.SetNewSpan(ctx, "InternalNewDialedVIF")
@@ -499,13 +495,11 @@
return errAlreadySetup
}
vif.muListen.Lock()
- principal, lBlessings, dischargeClient, err := serverAuthOptions(vif.principal, vif.blessings, vif.listenerOpts)
+ dischargeClient := getDischargeClient(vif.listenerOpts)
vif.muListen.Unlock()
- if err != nil {
- return errVersionNegotiationFailed
- }
+
vif.writeMu.Lock()
- c, err := AuthenticateAsServer(vif.conn, vif.reader, vif.versions, principal, lBlessings, dischargeClient, m)
+ c, err := AuthenticateAsServer(vif.conn, vif.reader, vif.versions, vif.principal, vif.blessings, dischargeClient, m)
if err != nil {
vif.writeMu.Unlock()
return err
@@ -967,29 +961,13 @@
return ep
}
-// clientAuthOptions returns credentials for VIF authentication, based on the provided principal and options list.
-func clientAuthOptions(principal security.Principal, lopts []stream.VCOpt) (*context.T, security.Principal, error) {
- var (
- securityLevel options.VCSecurityLevel
- ctx *context.T
- )
- for _, o := range lopts {
+// getDialContext returns the DialContext for this call.
+func getDialContext(vopts []stream.VCOpt) *context.T {
+ for _, o := range vopts {
switch v := o.(type) {
case vc.DialContext:
- ctx = v.T
- case options.VCSecurityLevel:
- securityLevel = v
+ return v.T
}
}
- switch securityLevel {
- case options.VCSecurityConfidential:
- if principal == nil {
- return nil, nil, fmt.Errorf("principal required for VCSecurityConfidential")
- }
- return ctx, principal, nil
- case options.VCSecurityNone:
- return ctx, nil, nil
- default:
- return nil, nil, fmt.Errorf("unrecognized VC security level: %v", securityLevel)
- }
+ return nil
}
diff --git a/profiles/internal/rpc/stream/vif/vif_test.go b/profiles/internal/rpc/stream/vif/vif_test.go
index 2b11221..f878645 100644
--- a/profiles/internal/rpc/stream/vif/vif_test.go
+++ b/profiles/internal/rpc/stream/vif/vif_test.go
@@ -1,6 +1,6 @@
// Tests in a separate package to ensure that only the exported API is used in the tests.
//
-// All tests are run with the default security level on VCs (VCSecurityConfidential).
+// All tests are run with the default security level on VCs (SecurityConfidential).
package vif_test
diff --git a/profiles/internal/rt/runtime.go b/profiles/internal/rt/runtime.go
index b994257..46c56e0 100644
--- a/profiles/internal/rt/runtime.go
+++ b/profiles/internal/rt/runtime.go
@@ -141,7 +141,7 @@
}
// The client we create here is incomplete (has a nil principal) and only works
- // because the agent uses anonymous unix sockets and VCSecurityNone.
+ // because the agent uses anonymous unix sockets and SecurityNone.
// After security is initialized we attach a real client.
// We do not capture the ctx here on purpose, to avoid anyone accidentally
// using this client anywhere.
@@ -314,10 +314,12 @@
}
func (r *Runtime) setPrincipal(ctx *context.T, principal security.Principal, deps ...interface{}) (*context.T, error) {
- // We uniquely identity a principal with "security/principal/<publicKey>"
- principalName := "security/principal/" + principal.PublicKey().String()
- stats.NewStringFunc(principalName+"/blessingstore", principal.BlessingStore().DebugString)
- stats.NewStringFunc(principalName+"/blessingroots", principal.Roots().DebugString)
+ if principal != nil {
+ // We uniquely identify a principal with "security/principal/<publicKey>"
+ principalName := "security/principal/" + principal.PublicKey().String()
+ stats.NewStringFunc(principalName+"/blessingstore", principal.BlessingStore().DebugString)
+ stats.NewStringFunc(principalName+"/blessingroots", principal.Roots().DebugString)
+ }
ctx = context.WithValue(ctx, principalKey, principal)
return ctx, r.addChild(ctx, principal, func() {}, deps...)
}
diff --git a/security/agent/agent_v23_test.go b/security/agent/agent_v23_test.go
index 1d5446a..42539e2 100644
--- a/security/agent/agent_v23_test.go
+++ b/security/agent/agent_v23_test.go
@@ -253,12 +253,6 @@
if err := pclient.AddToRoots(bserver); err != nil {
i.Fatal(err)
}
- // TODO(ashankar,ribrdb,suharshs): This should not be needed. It seems
- // however, that not providing it messes up the agent: Specifically,
- // the child process is unable to connect to the agent?
- if err := pclient.BlessingStore().SetDefault(bclient); err != nil {
- i.Fatal(err)
- }
const envvar = "VEYRON_CREDENTIALS="
return agentd.WithEnv(envvar + clientDir), agentd.WithEnv(envvar + serverDir)
}
diff --git a/security/agent/client.go b/security/agent/client.go
index b93914d..f9ce007 100644
--- a/security/agent/client.go
+++ b/security/agent/client.go
@@ -46,8 +46,8 @@
func (c *caller) startCall(name string, args ...interface{}) (rpc.ClientCall, error) {
ctx, _ := vtrace.SetNewTrace(c.ctx)
- // VCSecurityNone is safe here since we're using anonymous unix sockets.
- return c.client.StartCall(ctx, c.name, name, args, options.VCSecurityNone, options.NoResolve{})
+ // SecurityNone is safe here since we're using anonymous unix sockets.
+ return c.client.StartCall(ctx, c.name, name, args, options.SecurityNone, options.NoResolve{})
}
func results(inputs ...interface{}) []interface{} {
diff --git a/security/agent/server/server.go b/security/agent/server/server.go
index e0e0e95..0799414 100644
--- a/security/agent/server/server.go
+++ b/security/agent/server/server.go
@@ -240,13 +240,13 @@
}
}
if clientAddr != nil {
- // VCSecurityNone is safe since we're using anonymous unix sockets.
+ // SecurityNone is safe since we're using anonymous unix sockets.
// Only our child process can possibly communicate on the socket.
//
- // Also, VCSecurityNone implies that s (rpc.Server) created below does not
+ // Also, SecurityNone implies that s (rpc.Server) created below does not
// authenticate to clients, so runtime.Principal is irrelevant for the agent.
// TODO(ribrdb): Shutdown these servers when the connection is closed.
- s, err := v23.NewServer(ctx, options.VCSecurityNone)
+ s, err := v23.NewServer(ctx, options.SecurityNone)
if err != nil {
vlog.Infof("Error creating server: %v", err)
ack()
diff --git a/security/agent/server/wire.vdl b/security/agent/server/wire.vdl
index 9202cd7..53d94d4 100644
--- a/security/agent/server/wire.vdl
+++ b/security/agent/server/wire.vdl
@@ -7,7 +7,7 @@
// unix domain socket. To connect to the agent, a client should create
// a unix domain socket pair. Then send one end of the socket to the agent
// with 1 byte of data. The agent will then serve the Agent service on
-// the recieved socket, using VCSecurityNone.
+// the received socket, using SecurityNone.
//
// The agent also supports an optional mode where it can manage multiple principals.
// Typically this is only used by Device Manager. In this mode, VEYRON_AGENT_FD
