security: Use VDL's mechanism for wire <-> native conversions for
Blessings.
This commit is the counterpart to the API change in:
https://vanadium-review.googlesource.com/6290
Mostly simplifies, but there are some rough edges still left
which will be addressed in separate commits:
(1) At a few places, the caller wants to access the underlying
certificate chains of a security.Blessings object.
Have to work out this API, but for now they either use
security.MarshalBlessings or do a VOM-roundtrip (when
performance is a non-concern)
(2) The BlessingStore implementation can be simplified considerably
now, and should have a clear wire spec (.vdl file).
(3) The persistent store used by the mgmt code
(veyron/services/mgmt/lib/fs) is using GOB to write out
serialized forms. There is a plan to convert this to VOM
but in the mean time I had to do something ugly - translate
from application.Envelope (which is no longer GOBable) to
a different type. This will go away once the store is converted
to VOM.
MultiPart: 2/2
Change-Id: Iafb791afedf0a3ea4c9c1c766300213dace7d692
diff --git a/runtimes/google/ipc/blessings_cache.go b/runtimes/google/ipc/blessings_cache.go
index 204715a..06ec74b 100644
--- a/runtimes/google/ipc/blessings_cache.go
+++ b/runtimes/google/ipc/blessings_cache.go
@@ -109,8 +109,7 @@
return ipc.BlessingsRequest{Key: val.id}
}
// otherwise we still need to send both key and blessings, but we must ensure that we send the same key.
- wireBlessings := security.MarshalBlessings(blessings)
- return ipc.BlessingsRequest{val.id, &wireBlessings}
+ return ipc.BlessingsRequest{val.id, &blessings}
}
// nextIdLocked creates a new id for inserting blessings. It must be called after acquiring a writer lock.
@@ -153,10 +152,7 @@
stats.recordBlessingCache(false)
// Slowpath, might need to update the cache, or check that the received blessings are
// the same as what's in the cache.
- recv, err := security.NewBlessings(*req.Blessings)
- if err != nil {
- return security.Blessings{}, fmt.Errorf("ipc: create new client blessings failed: %v", err)
- }
+ recv := *req.Blessings
c.Lock()
defer c.Unlock()
if cached, exists := c.m[req.Key]; exists {
diff --git a/runtimes/google/ipc/client.go b/runtimes/google/ipc/client.go
index 2a9f92a..f9ced6b 100644
--- a/runtimes/google/ipc/client.go
+++ b/runtimes/google/ipc/client.go
@@ -799,7 +799,7 @@
Method: method,
NumPosArgs: uint64(len(args)),
Deadline: vtime.Deadline{deadline},
- GrantedBlessings: security.MarshalBlessings(fc.grantedBlessings),
+ GrantedBlessings: fc.grantedBlessings,
Blessings: blessingsRequest,
Discharges: discharges,
TraceRequest: ivtrace.Request(fc.ctx),
diff --git a/runtimes/google/ipc/server.go b/runtimes/google/ipc/server.go
index 3c7db7c..8340516 100644
--- a/runtimes/google/ipc/server.go
+++ b/runtimes/google/ipc/server.go
@@ -1149,11 +1149,6 @@
func (fs *flowServer) initSecurity(req *ipc.Request) error {
// If additional credentials are provided, make them available in the context
- blessings, err := security.NewBlessings(req.GrantedBlessings)
- if err != nil {
- return verror.New(verror.ErrBadProtocol, fs.T, newErrBadBlessings(fs.T, err))
- }
- fs.grantedBlessings = blessings
// Detect unusable blessings now, rather then discovering they are unusable on
// first use.
//
@@ -1161,11 +1156,12 @@
// 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?
- if blessings.PublicKey() != nil && !reflect.DeepEqual(blessings.PublicKey(), fs.flow.LocalPrincipal().PublicKey()) {
- return verror.New(verror.ErrNoAccess, fs.T, fmt.Sprintf("blessing granted not bound to this server(%v vs %v)", blessings.PublicKey(), fs.flow.LocalPrincipal().PublicKey()))
+ if b := req.GrantedBlessings; b.PublicKey() != nil && !reflect.DeepEqual(b.PublicKey(), fs.flow.LocalPrincipal().PublicKey()) {
+ return verror.New(verror.ErrNoAccess, fs.T, fmt.Sprintf("blessing granted not bound to this server(%v vs %v)", b.PublicKey(), fs.flow.LocalPrincipal().PublicKey()))
}
- fs.clientBlessings, err = serverDecodeBlessings(fs.flow.VCDataCache(), req.Blessings, fs.server.stats)
- if err != nil {
+ fs.grantedBlessings = req.GrantedBlessings
+ var err error
+ if fs.clientBlessings, err = serverDecodeBlessings(fs.flow.VCDataCache(), req.Blessings, fs.server.stats); err != nil {
// When the server can't access the blessings cache, the client is not following
// protocol, so the server closes the VCs corresponding to the client endpoint.
// TODO(suharshs,toddw): Figure out a way to only shutdown the current VC, instead
diff --git a/runtimes/google/ipc/stream/vc/auth.go b/runtimes/google/ipc/stream/vc/auth.go
index b333b3a..43442b3 100644
--- a/runtimes/google/ipc/stream/vc/auth.go
+++ b/runtimes/google/ipc/stream/vc/auth.go
@@ -94,7 +94,7 @@
if err := enc.Encode(signature); err != nil {
return err
}
- if err := enc.Encode(security.MarshalBlessings(b)); err != nil {
+ if err := enc.Encode(b); err != nil {
return err
}
if v >= version.IPCVersion7 {
@@ -143,13 +143,13 @@
}
var (
- wireb security.WireBlessings
- sig security.Signature
+ blessings security.Blessings
+ sig security.Signature
)
if err = dec.Decode(&sig); err != nil {
return noBlessings, nil, err
}
- if err = dec.Decode(&wireb); err != nil {
+ if err = dec.Decode(&blessings); err != nil {
return noBlessings, nil, err
}
var discharges map[string]security.Discharge
@@ -177,10 +177,6 @@
}
}
}
- blessings, err := security.NewBlessings(wireb)
- if err != nil {
- return noBlessings, nil, err
- }
if !sig.Verify(blessings.PublicKey(), append(tag, crypter.ChannelBinding()...)) {
return noBlessings, nil, errInvalidSignatureInMessage
}
