TrustStore: unify interface (#3532)

Introduce TRCID and use ChainID and TRCID consistently in trust package. Use value types for simple structs instead of pointers. Fixes #3525
scionproto · Dec 17, 2019 · b72ecf4 · b72ecf4
1 parent 6936c89
commit b72ecf4
Show file tree

Hide file tree

Showing 22 changed files with 397 additions and 273 deletions.
diff --git a/go/lib/infra/common.go b/go/lib/infra/common.go
@@ -508,6 +508,9 @@ type TrustStoreOpts struct {
 	// requests, if they are not available locally. If it is not set, the
 	// trust store does its own server resolution.
 	Server net.Addr
+	// Client indicates the peer who sent this request to the trust store, if
+	// applicable.
+	Client net.Addr
 	// LocalOnly indicates that the store should only check locally.
 	LocalOnly bool
 }

diff --git a/go/lib/infra/modules/trust/v2/db.go b/go/lib/infra/modules/trust/v2/db.go
@@ -80,13 +80,13 @@ type TRCRead interface {
 	// database with differing contents.
 	TRCExists(ctx context.Context, d decoded.TRC) (bool, error)
 	// GetTRC returns the TRC. If it is not found, ErrNotFound is returned.
-	GetTRC(ctx context.Context, isd addr.ISD, version scrypto.Version) (*trc.TRC, error)
+	GetTRC(ctx context.Context, id TRCID) (*trc.TRC, error)
 	// GetRawTRC returns the raw signed TRC bytes. If it is not found,
 	// ErrNotFound is returned.
-	GetRawTRC(ctx context.Context, isd addr.ISD, version scrypto.Version) ([]byte, error)
+	GetRawTRC(ctx context.Context, id TRCID) ([]byte, error)
 	// GetTRCInfo returns the infos for the requested TRC. If it is not found,
 	// ErrNotFound is returned.
-	GetTRCInfo(ctx context.Context, isd addr.ISD, version scrypto.Version) (TRCInfo, error)
+	GetTRCInfo(ctx context.Context, id TRCID) (TRCInfo, error)
 	// GetIssuingKeyInfo returns the infos of the requested AS. If it is not
 	// found, ErrNotFound is returned.
 	GetIssuingKeyInfo(ctx context.Context, ia addr.IA, version scrypto.Version) (KeyInfo, error)
@@ -105,7 +105,7 @@ type TRCWrite interface {
 type ChainRead interface {
 	// GetRawChain returns the raw signed certificate chain bytes. If it is not
 	// found, ErrNotFound is returned.
-	GetRawChain(ctx context.Context, ia addr.IA, version scrypto.Version) ([]byte, error)
+	GetRawChain(ctx context.Context, id ChainID) ([]byte, error)
 	// ChainExists returns whether the certificate chain is found in the
 	// database and the content matches. ErrContentMismatch is returned if any
 	// of the two certificates exist in the database with differing contents.

diff --git a/go/lib/infra/modules/trust/v2/handlers.go b/go/lib/infra/modules/trust/v2/handlers.go
@@ -21,14 +21,12 @@ import (
 
 	"golang.org/x/xerrors"
 
-	"github.com/scionproto/scion/go/lib/addr"
 	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/lib/ctrl/cert_mgmt"
 	"github.com/scionproto/scion/go/lib/infra"
 	"github.com/scionproto/scion/go/lib/infra/messenger"
 	"github.com/scionproto/scion/go/lib/infra/modules/trust/v2/internal/decoded"
 	"github.com/scionproto/scion/go/lib/log"
-	"github.com/scionproto/scion/go/lib/scrypto"
 	"github.com/scionproto/scion/go/lib/scrypto/trc/v2"
 	"github.com/scionproto/scion/go/proto"
 )
@@ -66,8 +64,13 @@ func (h *chainReqHandler) Handle() *infra.HandlerResult {
 		return infra.MetricsErrInternal
 	}
 	sendAck := messenger.SendAckHelper(ctx, rw)
-	raw, err := h.provider.GetRawChain(ctx, chainReq.IA(), chainReq.Version,
-		infra.ChainOpts{AllowInactive: true}, h.request.Peer)
+	raw, err := h.provider.GetRawChain(ctx,
+		ChainID{IA: chainReq.IA(), Version: chainReq.Version},
+		infra.ChainOpts{
+			TrustStoreOpts: infra.TrustStoreOpts{Client: h.request.Peer},
+			AllowInactive:  true,
+		},
+	)
 	if err != nil {
 		logger.Error("[TrustStore:chainReqHandler] Unable to retrieve chain", "err", err)
 		sendAck(proto.Ack_ErrCode_reject, AckNotFound)
@@ -114,8 +117,13 @@ func (h *trcReqHandler) Handle() *infra.HandlerResult {
 		return infra.MetricsErrInternal
 	}
 	sendAck := messenger.SendAckHelper(ctx, rw)
-	raw, err := h.provider.GetRawTRC(ctx, trcReq.ISD, trcReq.Version,
-		infra.TRCOpts{AllowInactive: true}, h.request.Peer)
+	raw, err := h.provider.GetRawTRC(ctx,
+		TRCID{ISD: trcReq.ISD, Version: trcReq.Version},
+		infra.TRCOpts{
+			TrustStoreOpts: infra.TrustStoreOpts{Client: h.request.Peer},
+			AllowInactive:  true,
+		},
+	)
 	if err != nil {
 		logger.Error("[TrustStore:trcReqHandler] Unable to retrieve TRC", "err", err)
 		sendAck(proto.Ack_ErrCode_reject, AckNotFound)
@@ -254,15 +262,15 @@ func (h *trcPushHandler) Handle() *infra.HandlerResult {
 }
 
 func newTRCGetter(provider CryptoProvider, peer net.Addr) func(context.Context,
-	addr.ISD, scrypto.Version) (*trc.TRC, error) {
+	TRCID) (*trc.TRC, error) {
 
-	return func(ctx context.Context, isd addr.ISD, version scrypto.Version) (*trc.TRC, error) {
+	return func(ctx context.Context, id TRCID) (*trc.TRC, error) {
 		opts := infra.TRCOpts{
 			TrustStoreOpts: infra.TrustStoreOpts{
 				Server: peer,
 			},
 			AllowInactive: true,
 		}
-		return provider.GetTRC(ctx, isd, version, opts)
+		return provider.GetTRC(ctx, id, opts)
 	}
 }
diff --git a/go/lib/infra/modules/trust/v2/handlers_test.go b/go/lib/infra/modules/trust/v2/handlers_test.go
@@ -84,8 +84,9 @@ func TestChainReqHandler(t *testing.T) {
 					AllowInactive: true,
 				}
 				p := mock_v2.NewMockCryptoProvider(ctrl)
-				p.EXPECT().GetRawChain(gomock.Any(), ia110, scrypto.LatestVer,
-					opts, nil).Return(nil, trust.ErrNotFound)
+				p.EXPECT().GetRawChain(gomock.Any(),
+					trust.ChainID{IA: ia110, Version: scrypto.LatestVer},
+					opts).Return(nil, trust.ErrNotFound)
 				return p
 			},
 			ExpectedResult: infra.MetricsErrTrustStore(trust.ErrNotFound),
@@ -109,8 +110,9 @@ func TestChainReqHandler(t *testing.T) {
 					AllowInactive: true,
 				}
 				p := mock_v2.NewMockCryptoProvider(ctrl)
-				p.EXPECT().GetRawChain(gomock.Any(), ia110, scrypto.LatestVer,
-					opts, nil).Return([]byte("test"), nil)
+				p.EXPECT().GetRawChain(gomock.Any(),
+					trust.ChainID{IA: ia110, Version: scrypto.LatestVer},
+					opts).Return([]byte("test"), nil)
 				return p
 			},
 			ExpectedResult: infra.MetricsErrMsger(infra.ErrTransport),
@@ -134,8 +136,9 @@ func TestChainReqHandler(t *testing.T) {
 					AllowInactive: true,
 				}
 				p := mock_v2.NewMockCryptoProvider(ctrl)
-				p.EXPECT().GetRawChain(gomock.Any(), ia110, scrypto.LatestVer,
-					opts, nil).Return([]byte("test"), nil)
+				p.EXPECT().GetRawChain(gomock.Any(),
+					trust.ChainID{IA: ia110, Version: scrypto.LatestVer},
+					opts).Return([]byte("test"), nil)
 				return p
 			},
 			ExpectedResult: infra.MetricsResultOk,
@@ -208,8 +211,9 @@ func TestTRCReqHandler(t *testing.T) {
 					AllowInactive: true,
 				}
 				p := mock_v2.NewMockCryptoProvider(ctrl)
-				p.EXPECT().GetRawTRC(gomock.Any(), addr.ISD(1), scrypto.LatestVer,
-					opts, nil).Return(nil, trust.ErrNotFound)
+				p.EXPECT().GetRawTRC(gomock.Any(),
+					trust.TRCID{ISD: addr.ISD(1), Version: scrypto.LatestVer},
+					opts).Return(nil, trust.ErrNotFound)
 				return p
 			},
 			ExpectedResult: infra.MetricsErrTrustStore(trust.ErrNotFound),
@@ -233,8 +237,9 @@ func TestTRCReqHandler(t *testing.T) {
 					AllowInactive: true,
 				}
 				p := mock_v2.NewMockCryptoProvider(ctrl)
-				p.EXPECT().GetRawTRC(gomock.Any(), addr.ISD(1), scrypto.LatestVer,
-					opts, nil).Return([]byte("test"), nil)
+				p.EXPECT().GetRawTRC(gomock.Any(),
+					trust.TRCID{ISD: addr.ISD(1), Version: scrypto.LatestVer},
+					opts).Return([]byte("test"), nil)
 				return p
 			},
 			ExpectedResult: infra.MetricsErrMsger(infra.ErrTransport),
@@ -258,8 +263,9 @@ func TestTRCReqHandler(t *testing.T) {
 					AllowInactive: true,
 				}
 				p := mock_v2.NewMockCryptoProvider(ctrl)
-				p.EXPECT().GetRawTRC(gomock.Any(), addr.ISD(1), scrypto.LatestVer,
-					opts, nil).Return([]byte("test"), nil)
+				p.EXPECT().GetRawTRC(gomock.Any(),
+					trust.TRCID{ISD: addr.ISD(1), Version: scrypto.LatestVer},
+					opts).Return([]byte("test"), nil)
 				return p
 			},
 			ExpectedResult: infra.MetricsResultOk,

diff --git a/go/lib/infra/modules/trust/v2/inserter.go b/go/lib/infra/modules/trust/v2/inserter.go
@@ -17,9 +17,7 @@ package trust
 import (
 	"context"
 
-	"github.com/scionproto/scion/go/lib/addr"
 	"github.com/scionproto/scion/go/lib/infra/modules/trust/v2/internal/decoded"
-	"github.com/scionproto/scion/go/lib/scrypto"
 	"github.com/scionproto/scion/go/lib/scrypto/cert/v2"
 	"github.com/scionproto/scion/go/lib/scrypto/trc/v2"
 	"github.com/scionproto/scion/go/lib/serrors"
@@ -47,7 +45,7 @@ type Inserter interface {
 
 // TRCProviderFunc provides TRCs. It is used to configure the TRC retrieval
 // method of the inserter.
-type TRCProviderFunc func(context.Context, addr.ISD, scrypto.Version) (*trc.TRC, error)
+type TRCProviderFunc func(context.Context, TRCID) (*trc.TRC, error)
 
 // inserter is used to verify and insert trust material into the database.
 type inserter struct {
@@ -168,7 +166,7 @@ func (ins *baseInserter) shouldInsertTRC(ctx context.Context, decTRC decoded.TRC
 		}
 		return false, serrors.WithCtx(ErrBaseNotSupported, "trc", decTRC)
 	}
-	prev, err := trcProvider(ctx, decTRC.TRC.ISD, decTRC.TRC.Version-1)
+	prev, err := trcProvider(ctx, TRCID{ISD: decTRC.TRC.ISD, Version: decTRC.TRC.Version - 1})
 	if err != nil {
 		return false, serrors.WrapStr("unable to get previous TRC", err,
 			"isd", decTRC.TRC.ISD, "version", decTRC.TRC.Version-1)
@@ -212,7 +210,10 @@ func (ins *baseInserter) shouldInsertChain(ctx context.Context, chain decoded.Ch
 	if err := ins.validateChain(chain); err != nil {
 		return false, serrors.WrapStr("error validating the certificate chain", err)
 	}
-	t, err := trcProvider(ctx, chain.Issuer.Subject.I, chain.Issuer.Issuer.TRCVersion)
+	t, err := trcProvider(ctx, TRCID{
+		ISD:     chain.Issuer.Subject.I,
+		Version: chain.Issuer.Issuer.TRCVersion,
+	})
 	if err != nil {
 		return false, serrors.WrapStr("unable to get issuing TRC", err,
 			"isd", chain.Issuer.Subject.I, "version", chain.Issuer.Issuer.TRCVersion)

diff --git a/go/lib/infra/modules/trust/v2/inserter_test.go b/go/lib/infra/modules/trust/v2/inserter_test.go
@@ -24,7 +24,6 @@ import (
 	"github.com/scionproto/scion/go/lib/infra/modules/trust/v2"
 	"github.com/scionproto/scion/go/lib/infra/modules/trust/v2/internal/decoded"
 	"github.com/scionproto/scion/go/lib/infra/modules/trust/v2/mock_v2"
-	"github.com/scionproto/scion/go/lib/scrypto"
 	"github.com/scionproto/scion/go/lib/scrypto/trc/v2"
 	"github.com/scionproto/scion/go/lib/serrors"
 	"github.com/scionproto/scion/go/lib/snet"
@@ -137,7 +136,7 @@ func TestInserterInsertChain(t *testing.T) {
 					false, nil,
 				)
 			},
-			TRCProvider: func(context.Context, addr.ISD, scrypto.Version) (*trc.TRC, error) {
+			TRCProvider: func(context.Context, trust.TRCID) (*trc.TRC, error) {
 				return nil, notFound
 			},
 			ExpectedErr: notFound,
@@ -201,7 +200,7 @@ func TestInserterInsertChain(t *testing.T) {
 			ins := trust.NewInserter(db, false)
 
 			decTRC := loadTRC(t, trc1v1)
-			p := func(ctx context.Context, isd addr.ISD, ver scrypto.Version) (*trc.TRC, error) {
+			p := func(_ context.Context, _ trust.TRCID) (*trc.TRC, error) {
 				return decTRC.TRC, nil
 			}
 			if test.TRCProvider != nil {
@@ -292,7 +291,7 @@ func TestFwdInserterInsertChain(t *testing.T) {
 			ins := trust.NewFwdInserter(m.DB, m.RPC)
 
 			decTRC := loadTRC(t, trc1v1)
-			p := func(ctx context.Context, isd addr.ISD, ver scrypto.Version) (*trc.TRC, error) {
+			p := func(_ context.Context, _ trust.TRCID) (*trc.TRC, error) {
 				return decTRC.TRC, nil
 			}
 			if test.TRCProvider != nil {

diff --git a/go/lib/infra/modules/trust/v2/inspector.go b/go/lib/infra/modules/trust/v2/inspector.go
@@ -44,7 +44,9 @@ func (i *inspector) ByAttributes(ctx context.Context, isd addr.ISD,
 	opts infra.ASInspectorOpts) ([]addr.IA, error) {
 
 	trcOpts := infra.TRCOpts{TrustStoreOpts: opts.TrustStoreOpts}
-	t, err := i.provider.GetTRC(ctx, isd, scrypto.Version(scrypto.LatestVer), trcOpts)
+	t, err := i.provider.GetTRC(ctx, TRCID{
+		ISD: isd, Version: scrypto.Version(scrypto.LatestVer)},
+		trcOpts)
 	if err != nil {
 		return nil, serrors.WrapStr("unable to get latest TRC", err, "isd", isd)
 	}
@@ -63,7 +65,9 @@ func (i *inspector) HasAttributes(ctx context.Context, ia addr.IA,
 	opts infra.ASInspectorOpts) (bool, error) {
 
 	trcOpts := infra.TRCOpts{TrustStoreOpts: opts.TrustStoreOpts}
-	trc, err := i.provider.GetTRC(ctx, ia.I, scrypto.Version(scrypto.LatestVer), trcOpts)
+	trc, err := i.provider.GetTRC(ctx, TRCID{
+		ISD: ia.I, Version: scrypto.Version(scrypto.LatestVer)},
+		trcOpts)
 	if err != nil {
 		return false, serrors.WrapStr("unable to get latest TRC", err, "isd", ia.I)
 	}

diff --git a/go/lib/infra/modules/trust/v2/inspector_test.go b/go/lib/infra/modules/trust/v2/inspector_test.go
@@ -60,8 +60,8 @@ func TestInspectorByAttributes(t *testing.T) {
 		},
 		"error": {
 			Expect: func(provider *mock_v2.MockCryptoProvider, _ *trc.TRC) {
-				provider.EXPECT().GetTRC(gomock.Any(), trc1v1.ISD,
-					scrypto.LatestVer, gomock.Any()).Return(nil, trust.ErrNotFound)
+				provider.EXPECT().GetTRC(gomock.Any(), trust.TRCID{ISD: trc1v1.ISD,
+					Version: scrypto.LatestVer}, gomock.Any()).Return(nil, trust.ErrNotFound)
 			},
 			ExpectedErr: trust.ErrNotFound,
 		},
@@ -140,8 +140,8 @@ func TestInspectorHasAttributes(t *testing.T) {
 		"error": {
 			IA: ia110,
 			Expect: func(provider *mock_v2.MockCryptoProvider, _ *trc.TRC) {
-				provider.EXPECT().GetTRC(gomock.Any(), ia110.I,
-					scrypto.LatestVer, gomock.Any()).Return(nil, trust.ErrNotFound)
+				provider.EXPECT().GetTRC(gomock.Any(), trust.TRCID{ISD: ia110.I,
+					Version: scrypto.LatestVer}, gomock.Any()).Return(nil, trust.ErrNotFound)
 			},
 			ExpectedErr: trust.ErrNotFound,
 		},
@@ -182,6 +182,6 @@ func defaultExpect(provider *mock_v2.MockCryptoProvider, trcObj *trc.TRC) {
 	entry = trcObj.PrimaryASes[ia120.A]
 	entry.Attributes = []trc.Attribute{trc.Issuing}
 	trcObj.PrimaryASes[ia120.A] = entry
-	provider.EXPECT().GetTRC(gomock.Any(), addr.ISD(1),
-		scrypto.LatestVer, gomock.Any()).Return(trcObj, nil)
+	provider.EXPECT().GetTRC(gomock.Any(), trust.TRCID{ISD: addr.ISD(1),
+		Version: scrypto.LatestVer}, gomock.Any()).Return(trcObj, nil)
 }