security: Define a VDL type for discharges (Part 2/2)
More details in:
commit 4617b0475f31283d1ffde494f99e36f67cb2c1c5
= https://vanadium-review.googlesource.com/#/c/3601/
= https://github.com/veyron/release-issues/issues/634
With this commit, the security infrastructure should no longer decode
into the "vdl.Any" type, i.e., it always decodes into concrete types and
thus should be immune to renamings/reorganization of packages.
Change-Id: Ibf1be9537c855e040e2df45f99ede4ec2be6c43e
diff --git a/runtimes/google/ipc/client.go b/runtimes/google/ipc/client.go
index e330ff7..7945478 100644
--- a/runtimes/google/ipc/client.go
+++ b/runtimes/google/ipc/client.go
@@ -764,14 +764,9 @@
fc.blessings = fc.flow.LocalPrincipal().BlessingStore().ForPeer(fc.server...)
blessingsRequest = clientEncodeBlessings(fc.flow.VCDataCache(), fc.blessings)
}
- // TODO(ashankar):DISCHARGEVDL: This should become:
- // discharges := make([]security.WireDischarge, len(fc.discharges))
- // for i, d := range fc.discharges {
- // discharges[i] = security.MarshalDischarge(d)
- // }
- anyDischarges := make([]vdl.AnyRep, len(fc.discharges))
+ discharges := make([]security.WireDischarge, len(fc.discharges))
for i, d := range fc.discharges {
- anyDischarges[i] = d
+ discharges[i] = security.MarshalDischarge(d)
}
req := ipc.Request{
Suffix: suffix,
@@ -780,7 +775,7 @@
Timeout: int64(timeout),
GrantedBlessings: security.MarshalBlessings(blessings),
Blessings: blessingsRequest,
- Discharges: anyDischarges,
+ Discharges: discharges,
TraceRequest: ivtrace.Request(fc.ctx),
}
if err := fc.enc.Encode(req); err != nil {
diff --git a/runtimes/google/ipc/discharges.go b/runtimes/google/ipc/discharges.go
index 2c196cc..5bf42f7 100644
--- a/runtimes/google/ipc/discharges.go
+++ b/runtimes/google/ipc/discharges.go
@@ -113,28 +113,15 @@
vlog.VI(3).Infof("Discharge fetch for %v failed: %v", tp, err)
return
}
- // TODO(ashankar):DISCHARGEVDL:This should become:
- // var wire security.WireDischarge
- var dAny vdl.AnyRep
- if ierr := call.Finish(&dAny, &err); ierr != nil || err != nil {
+ var wire security.WireDischarge
+ if ierr := call.Finish(&wire, &err); ierr != nil || err != nil {
vlog.VI(3).Infof("Discharge fetch for %v failed: (%v, %v)", cav, err, ierr)
return
}
- var d security.Discharge
- switch v := dAny.(type) {
- case security.WireDischarge:
- if d, err = security.NewDischarge(v); err != nil {
- vlog.Errorf("fetchDischarges: server at %s sent an invalid discharge: %v", tp.Location(), err)
- } else {
- vlog.VI(3).Infof("Fetched discharge for %v: %v", tp, d)
- }
- case security.Discharge:
- d = v
- vlog.VI(3).Infof("Fetched discharge for %v: %v", tp, d)
- default:
- vlog.Errorf("fetchDischarges: server at %s sent an unexpected %T", tp.Location(), dAny)
+ d, err := security.NewDischarge(wire)
+ if err != nil {
+ vlog.Errorf("fetchDischarges: server at %s sent a malformed discharge: %v", tp.Location(), err)
}
-
discharges <- fetched{i, d}
}(i, ctx, caveats[i])
}
diff --git a/runtimes/google/ipc/errors.vdl b/runtimes/google/ipc/errors.vdl
index 8f36256..8a1118c 100644
--- a/runtimes/google/ipc/errors.vdl
+++ b/runtimes/google/ipc/errors.vdl
@@ -26,12 +26,6 @@
badDischarge(index uint64, err error) {
"en": "failed to decode discharge #{index}: {err}",
}
- badDischargeType(index uint64, t typeobject) {
- "en": "discharge #{index} type {t} isn't registered",
- }
- badDischargeImpl(index uint64, t string) {
- "en": "discharge #{index} type {t} doesn't implement security.Discharge",
- }
badAuth(suffix, method string, err error) {
"en": "not authorized to call {suffix}.{method}: {err}",
}
diff --git a/runtimes/google/ipc/errors.vdl.go b/runtimes/google/ipc/errors.vdl.go
index cc9a829..820833a 100644
--- a/runtimes/google/ipc/errors.vdl.go
+++ b/runtimes/google/ipc/errors.vdl.go
@@ -7,7 +7,6 @@
// VDL system imports
"v.io/core/veyron2/context"
"v.io/core/veyron2/i18n"
- "v.io/core/veyron2/vdl"
"v.io/core/veyron2/verror"
// VDL user imports
@@ -23,8 +22,6 @@
errBadBlessings = verror.Register("v.io/core/veyron/runtimes/google/ipc.badBlessings", verror.NoRetry, "{1:}{2:} failed to decode blessings: {3}")
errBadBlessingsCache = verror.Register("v.io/core/veyron/runtimes/google/ipc.badBlessingsCache", verror.NoRetry, "{1:}{2:} failed to find blessings in cache: {3}")
errBadDischarge = verror.Register("v.io/core/veyron/runtimes/google/ipc.badDischarge", verror.NoRetry, "{1:}{2:} failed to decode discharge #{3}: {4}")
- errBadDischargeType = verror.Register("v.io/core/veyron/runtimes/google/ipc.badDischargeType", verror.NoRetry, "{1:}{2:} discharge #{3} type {4} isn't registered")
- errBadDischargeImpl = verror.Register("v.io/core/veyron/runtimes/google/ipc.badDischargeImpl", verror.NoRetry, "{1:}{2:} discharge #{3} type {4} doesn't implement security.Discharge")
errBadAuth = verror.Register("v.io/core/veyron/runtimes/google/ipc.badAuth", verror.NoRetry, "{1:}{2:} not authorized to call {3}.{4}: {5}")
)
@@ -36,8 +33,6 @@
i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(errBadBlessings.ID), "{1:}{2:} failed to decode blessings: {3}")
i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(errBadBlessingsCache.ID), "{1:}{2:} failed to find blessings in cache: {3}")
i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(errBadDischarge.ID), "{1:}{2:} failed to decode discharge #{3}: {4}")
- i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(errBadDischargeType.ID), "{1:}{2:} discharge #{3} type {4} isn't registered")
- i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(errBadDischargeImpl.ID), "{1:}{2:} discharge #{3} type {4} doesn't implement security.Discharge")
i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(errBadAuth.ID), "{1:}{2:} not authorized to call {3}.{4}: {5}")
}
@@ -76,16 +71,6 @@
return verror.New(errBadDischarge, ctx, index, err)
}
-// newErrBadDischargeType returns an error with the errBadDischargeType ID.
-func newErrBadDischargeType(ctx *context.T, index uint64, t *vdl.Type) error {
- return verror.New(errBadDischargeType, ctx, index, t)
-}
-
-// newErrBadDischargeImpl returns an error with the errBadDischargeImpl ID.
-func newErrBadDischargeImpl(ctx *context.T, index uint64, t string) error {
- return verror.New(errBadDischargeImpl, ctx, index, t)
-}
-
// newErrBadAuth returns an error with the errBadAuth ID.
func newErrBadAuth(ctx *context.T, suffix string, method string, err error) error {
return verror.New(errBadAuth, ctx, suffix, method, err)
diff --git a/runtimes/google/ipc/full_test.go b/runtimes/google/ipc/full_test.go
index 185b13c..f64dea2 100644
--- a/runtimes/google/ipc/full_test.go
+++ b/runtimes/google/ipc/full_test.go
@@ -185,7 +185,7 @@
type dischargeServer struct{}
-func (*dischargeServer) Discharge(ctx ipc.ServerCall, cav security.Caveat, _ security.DischargeImpetus) (vdl.AnyRep, error) {
+func (*dischargeServer) Discharge(ctx ipc.ServerCall, cav security.Caveat, _ security.DischargeImpetus) (security.WireDischarge, error) {
tp := cav.ThirdPartyDetails()
if tp == nil {
return nil, fmt.Errorf("discharger: %v does not represent a third-party caveat", cav)
@@ -198,7 +198,11 @@
if err != nil {
return nil, fmt.Errorf("failed to create an expiration on the discharge: %v", err)
}
- return ctx.LocalPrincipal().MintDischarge(cav, expiry)
+ d, err := ctx.LocalPrincipal().MintDischarge(cav, expiry)
+ if err != nil {
+ return nil, err
+ }
+ return security.MarshalDischarge(d), nil
}
func startServer(t *testing.T, principal security.Principal, sm stream.Manager, ns naming.Namespace, name string, disp ipc.Dispatcher, opts ...ipc.ServerOpt) (naming.Endpoint, ipc.Server) {
@@ -758,7 +762,7 @@
traceid []uniqueid.Id
}
-func (s *dischargeTestServer) Discharge(ctx ipc.ServerContext, cav vdl.AnyRep, impetus security.DischargeImpetus) (vdl.AnyRep, error) {
+func (s *dischargeTestServer) Discharge(ctx ipc.ServerContext, cav security.Caveat, impetus security.DischargeImpetus) (security.WireDischarge, error) {
s.impetus = append(s.impetus, impetus)
s.traceid = append(s.traceid, vtrace.GetSpan(ctx.Context()).Trace())
return nil, fmt.Errorf("discharges not issued")
@@ -1471,11 +1475,15 @@
called bool
}
-func (m *mockDischarger) Discharge(ctx ipc.ServerContext, caveat security.Caveat, _ security.DischargeImpetus) (vdl.AnyRep, error) {
+func (m *mockDischarger) Discharge(ctx ipc.ServerContext, caveat security.Caveat, _ security.DischargeImpetus) (security.WireDischarge, error) {
m.mu.Lock()
m.called = true
m.mu.Unlock()
- return ctx.LocalPrincipal().MintDischarge(caveat, security.UnconstrainedUse())
+ d, err := ctx.LocalPrincipal().MintDischarge(caveat, security.UnconstrainedUse())
+ if err != nil {
+ return nil, err
+ }
+ return security.MarshalDischarge(d), nil
}
func TestNoDischargesOpt(t *testing.T) {
diff --git a/runtimes/google/ipc/server.go b/runtimes/google/ipc/server.go
index 7030d7c..3665d59 100644
--- a/runtimes/google/ipc/server.go
+++ b/runtimes/google/ipc/server.go
@@ -18,7 +18,6 @@
"v.io/core/veyron2/options"
"v.io/core/veyron2/security"
"v.io/core/veyron2/services/security/access"
- "v.io/core/veyron2/vdl"
"v.io/core/veyron2/verror"
"v.io/core/veyron2/vlog"
"v.io/core/veyron2/vom"
@@ -1175,24 +1174,11 @@
}
for i, d := range req.Discharges {
- if w, ok := d.(security.WireDischarge); ok {
- dis, err := security.NewDischarge(w)
- if err != nil {
- return verror.New(verror.BadProtocol, fs.T, newErrBadDischarge(fs.T, uint64(i), err))
- }
- fs.discharges[dis.ID()] = dis
- continue
+ dis, err := security.NewDischarge(d)
+ if err != nil {
+ return verror.New(verror.BadProtocol, fs.T, newErrBadDischarge(fs.T, uint64(i), err))
}
- // TODO(ashankar):DISCHARGEVDL: The rest of this block (inside
- // the for loop) should go away.
- if dis, ok := d.(security.Discharge); ok {
- fs.discharges[dis.ID()] = dis
- continue
- }
- if v, ok := d.(*vdl.Value); ok {
- return verror.New(verror.BadProtocol, fs.T, newErrBadDischargeType(fs.T, uint64(i), v.Type()))
- }
- return verror.New(verror.BadProtocol, fs.T, newErrBadDischargeImpl(fs.T, uint64(i), fmt.Sprintf("%T", d)))
+ fs.discharges[dis.ID()] = dis
}
return nil
}
diff --git a/runtimes/google/rt/ipc_test.go b/runtimes/google/rt/ipc_test.go
index df5e560..488b4e8 100644
--- a/runtimes/google/rt/ipc_test.go
+++ b/runtimes/google/rt/ipc_test.go
@@ -15,7 +15,6 @@
"v.io/core/veyron/lib/testutil"
tsecurity "v.io/core/veyron/lib/testutil/security"
_ "v.io/core/veyron/profiles"
- "v.io/core/veyron2/vdl"
"v.io/core/veyron2/verror"
)
@@ -30,7 +29,7 @@
type dischargeService struct{}
-func (dischargeService) Discharge(ctx ipc.ServerCall, cav security.Caveat, _ security.DischargeImpetus) (vdl.AnyRep, error) {
+func (dischargeService) Discharge(ctx ipc.ServerCall, cav security.Caveat, _ security.DischargeImpetus) (security.WireDischarge, error) {
tp := cav.ThirdPartyDetails()
if tp == nil {
return nil, fmt.Errorf("discharger: not a third party caveat (%v)", cav)
@@ -38,7 +37,11 @@
if err := tp.Dischargeable(ctx); err != nil {
return nil, fmt.Errorf("third-party caveat %v cannot be discharged for this context: %v", tp, err)
}
- return ctx.LocalPrincipal().MintDischarge(cav, security.UnconstrainedUse())
+ d, err := ctx.LocalPrincipal().MintDischarge(cav, security.UnconstrainedUse())
+ if err != nil {
+ return nil, err
+ }
+ return security.MarshalDischarge(d), nil
}
func newCtx(rootCtx *context.T) *context.T {
diff --git a/services/security/discharger.vdl b/services/security/discharger.vdl
index f6df28a..0cfdd88 100644
--- a/services/security/discharger.vdl
+++ b/services/security/discharger.vdl
@@ -7,7 +7,5 @@
// Discharge is called by a principal that holds a blessing with a third
// party caveat and seeks to get a discharge that proves the fulfillment of
// this caveat.
- //
- // TODO(ashankar):DISCHARGEVDL: The return type should become security.WireDischarge
- Discharge(Caveat security.Caveat, Impetus security.DischargeImpetus) (Discharge any | error)
+ Discharge(Caveat security.Caveat, Impetus security.DischargeImpetus) (Discharge security.WireDischarge | error)
}
diff --git a/services/security/discharger.vdl.go b/services/security/discharger.vdl.go
index 3e3c9f0..041d339 100644
--- a/services/security/discharger.vdl.go
+++ b/services/security/discharger.vdl.go
@@ -8,7 +8,6 @@
"v.io/core/veyron2"
"v.io/core/veyron2/context"
"v.io/core/veyron2/ipc"
- "v.io/core/veyron2/vdl"
// VDL user imports
"v.io/core/veyron2/security"
@@ -22,9 +21,7 @@
// Discharge is called by a principal that holds a blessing with a third
// party caveat and seeks to get a discharge that proves the fulfillment of
// this caveat.
- //
- // TODO(ashankar):DISCHARGEVDL: The return type should become security.WireDischarge
- Discharge(ctx *context.T, Caveat security.Caveat, Impetus security.DischargeImpetus, opts ...ipc.CallOpt) (Discharge vdl.AnyRep, err error)
+ Discharge(ctx *context.T, Caveat security.Caveat, Impetus security.DischargeImpetus, opts ...ipc.CallOpt) (Discharge security.WireDischarge, err error)
}
// DischargerClientStub adds universal methods to DischargerClientMethods.
@@ -56,7 +53,7 @@
return veyron2.GetClient(ctx)
}
-func (c implDischargerClientStub) Discharge(ctx *context.T, i0 security.Caveat, i1 security.DischargeImpetus, opts ...ipc.CallOpt) (o0 vdl.AnyRep, err error) {
+func (c implDischargerClientStub) Discharge(ctx *context.T, i0 security.Caveat, i1 security.DischargeImpetus, opts ...ipc.CallOpt) (o0 security.WireDischarge, err error) {
var call ipc.Call
if call, err = c.c(ctx).StartCall(ctx, c.name, "Discharge", []interface{}{i0, i1}, opts...); err != nil {
return
@@ -75,9 +72,7 @@
// Discharge is called by a principal that holds a blessing with a third
// party caveat and seeks to get a discharge that proves the fulfillment of
// this caveat.
- //
- // TODO(ashankar):DISCHARGEVDL: The return type should become security.WireDischarge
- Discharge(ctx ipc.ServerContext, Caveat security.Caveat, Impetus security.DischargeImpetus) (Discharge vdl.AnyRep, err error)
+ Discharge(ctx ipc.ServerContext, Caveat security.Caveat, Impetus security.DischargeImpetus) (Discharge security.WireDischarge, err error)
}
// DischargerServerStubMethods is the server interface containing
@@ -115,7 +110,7 @@
gs *ipc.GlobState
}
-func (s implDischargerServerStub) Discharge(ctx ipc.ServerContext, i0 security.Caveat, i1 security.DischargeImpetus) (vdl.AnyRep, error) {
+func (s implDischargerServerStub) Discharge(ctx ipc.ServerContext, i0 security.Caveat, i1 security.DischargeImpetus) (security.WireDischarge, error) {
return s.impl.Discharge(ctx, i0, i1)
}
@@ -138,13 +133,13 @@
Methods: []ipc.MethodDesc{
{
Name: "Discharge",
- Doc: "// Discharge is called by a principal that holds a blessing with a third\n// party caveat and seeks to get a discharge that proves the fulfillment of\n// this caveat.\n//\n// TODO(ashankar):DISCHARGEVDL: The return type should become security.WireDischarge",
+ Doc: "// Discharge is called by a principal that holds a blessing with a third\n// party caveat and seeks to get a discharge that proves the fulfillment of\n// this caveat.",
InArgs: []ipc.ArgDesc{
{"Caveat", ``}, // security.Caveat
{"Impetus", ``}, // security.DischargeImpetus
},
OutArgs: []ipc.ArgDesc{
- {"Discharge", ``}, // vdl.AnyRep
+ {"Discharge", ``}, // security.WireDischarge
{"err", ``}, // error
},
},
diff --git a/services/security/discharger/discharger.go b/services/security/discharger/discharger.go
index a61aea1..5f4b436 100644
--- a/services/security/discharger/discharger.go
+++ b/services/security/discharger/discharger.go
@@ -7,14 +7,13 @@
services "v.io/core/veyron/services/security"
"v.io/core/veyron2/ipc"
"v.io/core/veyron2/security"
- "v.io/core/veyron2/vdl"
)
// dischargerd issues discharges for all caveats present in the current
// namespace with no additional caveats iff the caveat is valid.
type dischargerd struct{}
-func (dischargerd) Discharge(ctx ipc.ServerContext, caveat security.Caveat, _ security.DischargeImpetus) (vdl.AnyRep, error) {
+func (dischargerd) Discharge(ctx ipc.ServerContext, caveat security.Caveat, _ security.DischargeImpetus) (security.WireDischarge, error) {
tp := caveat.ThirdPartyDetails()
if tp == nil {
return nil, fmt.Errorf("Caveat %v does not represent a third party caveat", caveat)
@@ -26,7 +25,11 @@
if err != nil {
return nil, fmt.Errorf("unable to create expiration caveat on the discharge: %v", err)
}
- return ctx.LocalPrincipal().MintDischarge(caveat, expiry)
+ d, err := ctx.LocalPrincipal().MintDischarge(caveat, expiry)
+ if err != nil {
+ return nil, err
+ }
+ return security.MarshalDischarge(d), nil
}
// NewDischarger returns a discharger service implementation that grants
