Remove redundant fields from Lvl1Req/Resp (scionproto#83)

* removed redundant fields from Lvl1Req/Resp

* add comments in mgmt.proto and fix lint warnings

go/lib/ctrl/drkey/lvl1_req.go

@@ -28,15 +28,13 @@ import (
 
 // Lvl1Req represents a level 1 request between CS.
 type Lvl1Req struct {
-	DstIA     addr.IA
 	ValTime   time.Time
 	Timestamp time.Time
 }
 
 // NewLvl1Req returns a fresh Lvl1Req
-func NewLvl1Req(dstIA addr.IA, valTime time.Time) Lvl1Req {
+func NewLvl1Req(valTime time.Time) Lvl1Req {
 	return Lvl1Req{
-		DstIA:     dstIA,
 		ValTime:   valTime,
 		Timestamp: time.Now(),
 	}
@@ -53,14 +51,13 @@ func Lvl1reqToProtoRequest(req Lvl1Req) (*dkpb.DRKeyLvl1Request, error) {
 		return nil, serrors.WrapStr("invalid timeStamp from request", err)
 	}
 	return &dkpb.DRKeyLvl1Request{
-		DstIa:     uint64(req.DstIA.IAInt()),
 		ValTime:   valTime,
 		Timestamp: timestamp,
 	}, nil
 }
 
 // GetLvl1KeyFromReply extracts the level 1 drkey from the reply.
-func GetLvl1KeyFromReply(rep *dkpb.DRKeyLvl1Response) (drkey.Lvl1Key, error) {
+func GetLvl1KeyFromReply(srcIA, dstIA addr.IA, rep *dkpb.DRKeyLvl1Response) (drkey.Lvl1Key, error) {
 
 	epochBegin, err := ptypes.Timestamp(rep.EpochBegin)
 	if err != nil {
@@ -78,8 +75,8 @@ func GetLvl1KeyFromReply(rep *dkpb.DRKeyLvl1Response) (drkey.Lvl1Key, error) {
 	}
 	return drkey.Lvl1Key{
 		Lvl1Meta: drkey.Lvl1Meta{
-			SrcIA: addr.IAInt(rep.SrcIa).IA(),
-			DstIA: addr.IAInt(rep.DstIa).IA(),
+			SrcIA: srcIA,
+			DstIA: dstIA,
 			Epoch: epoch,
 		},
 		Key: drkey.DRKey(rep.Drkey),
@@ -102,8 +99,6 @@ func KeyToLvl1Resp(drkey drkey.Lvl1Key) (*dkpb.DRKeyLvl1Response, error) {
 	}
 
 	return &dkpb.DRKeyLvl1Response{
-		DstIa:      uint64(drkey.DstIA.IAInt()),
-		SrcIa:      uint64(drkey.SrcIA.IAInt()),
 		EpochBegin: epochBegin,
 		EpochEnd:   epochEnd,
 		Drkey:      []byte(drkey.Key),
@@ -123,7 +118,6 @@ func RequestToLvl1Req(req *dkpb.DRKeyLvl1Request) (Lvl1Req, error) {
 	}
 
 	return Lvl1Req{
-		DstIA:     addr.IAInt(req.DstIa).IA(),
 		ValTime:   valTime,
 		Timestamp: timestamp,
 	}, nil

go/lib/ctrl/drkey/protobuf_test.go

@@ -38,16 +38,12 @@ func TestLvl1reqToProtoRequest(t *testing.T) {
 	timestamp, err := ptypes.TimestampProto(now)
 	require.NoError(t, err)
 
-	dstIA := xtest.MustParseIA("1-ff00:0:110")
-
 	pbReq := &dkpb.DRKeyLvl1Request{
-		DstIa:     uint64(dstIA.IAInt()),
 		ValTime:   valTime,
 		Timestamp: timestamp,
 	}
 
 	lvl1Req := ctrl.Lvl1Req{
-		DstIA:     dstIA,
 		ValTime:   now,
 		Timestamp: now,
 	}
@@ -65,17 +61,13 @@ func TestRequestToLvl1Req(t *testing.T) {
 	timestamp, err := ptypes.TimestampProto(now)
 	require.NoError(t, err)
 
-	dstIA := xtest.MustParseIA("1-ff00:0:110").IAInt()
-
 	req := &dkpb.DRKeyLvl1Request{
-		DstIa:     uint64(dstIA),
 		ValTime:   valTime,
 		Timestamp: timestamp,
 	}
 
 	lvl1Req, err := ctrl.RequestToLvl1Req(req)
 	require.NoError(t, err)
-	assert.Equal(t, xtest.MustParseIA("1-ff00:0:110"), lvl1Req.DstIA)
 	assert.Equal(t, now, lvl1Req.ValTime)
 	assert.Equal(t, now, lvl1Req.Timestamp)
 }
@@ -99,8 +91,6 @@ func TestKeyToLvl1Resp(t *testing.T) {
 	}
 
 	targetResp := &dkpb.DRKeyLvl1Response{
-		DstIa:      uint64(dstIA.IAInt()),
-		SrcIa:      uint64(srcIA.IAInt()),
 		EpochBegin: epochBegin,
 		EpochEnd:   epochEnd,
 		Drkey:      []byte(k),
@@ -123,8 +113,6 @@ func TestGetLvl1KeyFromReply(t *testing.T) {
 	k := xtest.MustParseHexString("c584cad32613547c64823c756651b6f5") // just a level 1 key
 
 	resp := &dkpb.DRKeyLvl1Response{
-		DstIa:      uint64(dstIA.IAInt()),
-		SrcIa:      uint64(srcIA.IAInt()),
 		EpochBegin: epochBegin,
 		EpochEnd:   epochEnd,
 		Drkey:      []byte(k),
@@ -139,7 +127,7 @@ func TestGetLvl1KeyFromReply(t *testing.T) {
 		Key: k,
 	}
 
-	lvl1Key, err := ctrl.GetLvl1KeyFromReply(resp)
+	lvl1Key, err := ctrl.GetLvl1KeyFromReply(srcIA, dstIA, resp)
 	require.NoError(t, err)
 	assert.Equal(t, targetLvl1Key, lvl1Key)
 

go/lib/drkey/exchange/grpc.go

@@ -23,20 +23,16 @@ import (
 	"github.com/scionproto/scion/go/lib/serrors"
 )
 
-func ValitadePeerWithCert(peer *peer.Peer, ia addr.IA) error {
+func ExtractIAFromPeer(peer *peer.Peer) (*addr.IA, error) {
 	tlsInfo, ok := peer.AuthInfo.(credentials.TLSInfo)
 	if !ok {
-		return serrors.New("auth info is not of type TLS info",
+		return nil, serrors.New("auth info is not of type TLS info",
 			"peer", peer, "authType", peer.AuthInfo.AuthType())
 	}
 	chain := tlsInfo.State.PeerCertificates
 	certIA, err := cppki.ExtractIA(chain[0].Subject)
 	if err != nil {
-		return serrors.WrapStr("extracting IA from peer cert", err)
+		return nil, serrors.WrapStr("extracting IA from peer cert", err)
 	}
-	if !ia.Equal(*certIA) {
-		return serrors.New("peer IA from cert and requested IA do not match",
-			"peer IA", certIA, "req IA", ia)
-	}
-	return nil
+	return certIA, nil
 }

go/pkg/cs/drkey/grpc/drkey_fetcher.go

@@ -31,7 +31,8 @@ import (
 )
 
 type Lvl1KeyGetter interface {
-	GetLvl1Key(ctx context.Context, srcIA addr.IA, req *dkpb.DRKeyLvl1Request) (*dkpb.DRKeyLvl1Response, error)
+	GetLvl1Key(ctx context.Context, srcIA addr.IA,
+		req *dkpb.DRKeyLvl1Request) (*dkpb.DRKeyLvl1Response, error)
 }
 
 type Lvl1KeyFetcher struct {
@@ -41,7 +42,8 @@ type Lvl1KeyFetcher struct {
 
 var _ Lvl1KeyGetter = (*Lvl1KeyFetcher)(nil)
 
-func (f Lvl1KeyFetcher) GetLvl1Key(ctx context.Context, srcIA addr.IA, req *dkpb.DRKeyLvl1Request) (*dkpb.DRKeyLvl1Response, error) {
+func (f Lvl1KeyFetcher) GetLvl1Key(ctx context.Context, srcIA addr.IA,
+	req *dkpb.DRKeyLvl1Request) (*dkpb.DRKeyLvl1Response, error) {
 	logger := log.FromCtx(ctx)
 
 	logger.Info("Resolving server", "srcIA", srcIA.String())
@@ -79,7 +81,7 @@ var _ csdrkey.Fetcher = (*DRKeyFetcher)(nil)
 func (f DRKeyFetcher) GetLvl1FromOtherCS(ctx context.Context,
 	srcIA, dstIA addr.IA, valTime time.Time) (drkey.Lvl1Key, error) {
 
-	lvl1req := ctrl.NewLvl1Req(dstIA, valTime)
+	lvl1req := ctrl.NewLvl1Req(valTime)
 	req, err := ctrl.Lvl1reqToProtoRequest(lvl1req)
 	if err != nil {
 		return drkey.Lvl1Key{},
@@ -91,15 +93,10 @@ func (f DRKeyFetcher) GetLvl1FromOtherCS(ctx context.Context,
 		return drkey.Lvl1Key{}, err
 	}
 
-	lvl1Key, err := ctrl.GetLvl1KeyFromReply(rep)
+	lvl1Key, err := ctrl.GetLvl1KeyFromReply(srcIA, dstIA, rep)
 	if err != nil {
 		return drkey.Lvl1Key{}, serrors.WrapStr("obtaining level 1 key from reply", err)
 	}
 
-	if !(lvl1Key.SrcIA.Equal(srcIA)) {
-		return drkey.Lvl1Key{}, serrors.New("Response srcIA does not match intended server IA",
-			"srcIA", lvl1Key.SrcIA.String(), "server IA", srcIA)
-	}
-
 	return lvl1Key, nil
 }

go/pkg/cs/drkey/grpc/drkey_fetcher_test.go

@@ -49,40 +49,22 @@ func TestGetLvl1FromOtherCS(t *testing.T) {
 		"valid": {
 			getter: func(ctrl *gomock.Controller) dk_grpc.Lvl1KeyGetter {
 				rep := &dkpb.DRKeyLvl1Response{
-					DstIa:      uint64(dstIA.IAInt()),
-					SrcIa:      uint64(srcIA.IAInt()),
 					EpochBegin: epochBegin,
 					EpochEnd:   epochEnd,
 					Drkey:      key,
 				}
 				getter := mock_grpc.NewMockLvl1KeyGetter(ctrl)
-				getter.EXPECT().GetLvl1Key(gomock.Any(), gomock.Eq(srcIA), gomock.Any()).Return(rep, nil)
+				getter.EXPECT().GetLvl1Key(gomock.Any(), gomock.Eq(srcIA),
+					gomock.Any()).Return(rep, nil)
 				return getter
 			},
 			assertErr: assert.NoError,
 		},
-		"wrong_srcIA_rep": {
-			getter: func(ctrl *gomock.Controller) dk_grpc.Lvl1KeyGetter {
-				wrongSrcIA := xtest.MustParseIA("1-ff00:0:110")
-				rep := &dkpb.DRKeyLvl1Response{
-					DstIa:      uint64(dstIA.IAInt()),
-					SrcIa:      uint64(wrongSrcIA.IAInt()),
-					EpochBegin: epochBegin,
-					EpochEnd:   epochEnd,
-					Drkey:      key,
-				}
-				getter := mock_grpc.NewMockLvl1KeyGetter(ctrl)
-				getter.EXPECT().GetLvl1Key(gomock.Any(), gomock.Eq(srcIA), gomock.Any()).Return(rep, nil)
-				return getter
-			},
-			assertErr: assert.Error,
-		},
 	}
 
 	for name, tc := range testCases {
 		name, tc := name, tc
 		t.Run(name, func(t *testing.T) {
-			t.Parallel()
 			ctrl := gomock.NewController(t)
 			defer ctrl.Finish()
 

go/pkg/cs/drkey/grpc/drkey_service.go

@@ -61,16 +61,16 @@ func (d *DRKeyServer) DRKeyLvl1(ctx context.Context,
 		return nil, err
 	}
 
-	// validating peer Subject.IA == req.dstIA
-	if err = exchange.ValitadePeerWithCert(peer, parsedReq.DstIA); err != nil {
-		logger.Error("[DRKey gRPC server] Error validating requested dstIA with certicate",
+	dstIA, err := exchange.ExtractIAFromPeer(peer)
+	if err != nil {
+		logger.Error("[DRKey gRPC server] Error retrieving auth info from certicate",
 			"err", err)
-		return nil, serrors.WrapStr("validating requested dstIA", err)
+		return nil, serrors.WrapStr("retrieving info from certficate", err)
 	}
 
 	logger.Debug("[DRKey gRPC server] Received Lvl1 request",
-		"lvl1_req", parsedReq, "peer", peer.Addr.String())
-	lvl1Key, err := d.Store.DeriveLvl1(parsedReq.DstIA, parsedReq.ValTime)
+		"lvl1_req", parsedReq, "peer", peer.Addr.String(), "IA from cert", (*dstIA).String())
+	lvl1Key, err := d.Store.DeriveLvl1(*dstIA, parsedReq.ValTime)
 	if err != nil {
 		logger.Error("Error deriving level 1 key", "err", err)
 		return nil, err

go/pkg/cs/drkey/grpc/lvl1_exchange_test.go

@@ -68,7 +68,6 @@ func TestLvl1KeyFetching(t *testing.T) {
 	chain, err := cppki.ReadPEMCerts(crt111File)
 	_ = chain
 	require.NoError(t, err)
-	ia111 := xtest.MustParseIA("1-ff00:0:111")
 
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
@@ -111,7 +110,7 @@ func TestLvl1KeyFetching(t *testing.T) {
 
 	client := cppb.NewDRKeyLvl1ServiceClient(conn)
 
-	lvl1req := pb_ctrl.NewLvl1Req(ia111, time.Now())
+	lvl1req := pb_ctrl.NewLvl1Req(time.Now())
 	req, err := pb_ctrl.Lvl1reqToProtoRequest(lvl1req)
 	require.NoError(t, err)
 	_, err = client.DRKeyLvl1(context.Background(), req)

go/pkg/cs/drkey/service_store.go

@@ -53,6 +53,13 @@ func (s *ServiceStore) GetLvl1Key(ctx context.Context, meta drkey.Lvl1Meta,
 	if meta.SrcIA == s.LocalIA {
 		return s.DeriveLvl1(meta.DstIA, valTime)
 	}
+
+	if meta.DstIA != s.LocalIA {
+		return drkey.Lvl1Key{},
+			serrors.New("Neither srcIA nor dstIA matches localIA", "srcIA", meta.SrcIA,
+				"dstIA", meta.DstIA, "localIA", s.LocalIA)
+	}
+
 	// look in the DB
 	k, err := s.DB.GetLvl1Key(ctx, meta, util.TimeToSecs(valTime))
 	if err == nil {