Repository: scionproto/scion
Pull request #3822 (2020-07-07 11:05:58)

disable revocation verification
This will allow us to proceed with the new revocation mechanism in a
non-breaking way. In this commit we disable verification later we can
disable signing. Then we can gradually move revocation
creation to the dataplane.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/scionproto/scion/3822)
<!-- Reviewable:end -->



Patch:
diff --git a/go/cs/revocation/BUILD.bazel b/go/cs/revocation/BUILD.bazel
index 13f6beee2..24145c022 100644
--- a/go/cs/revocation/BUILD.bazel
+++ b/go/cs/revocation/BUILD.bazel
@@ -27,12 +27,10 @@ go_test(
     deps = [
         "//go/cs/metrics:go_default_library",
         "//go/cs/revocation/mock_revocation:go_default_library",
-        "//go/lib/common:go_default_library",
         "//go/lib/ctrl:go_default_library",
         "//go/lib/ctrl/ack:go_default_library",
         "//go/lib/ctrl/path_mgmt:go_default_library",
         "//go/lib/infra:go_default_library",
-        "//go/lib/infra/messenger:go_default_library",
         "//go/lib/infra/mock_infra:go_default_library",
         "//go/lib/log:go_default_library",
         "//go/lib/scrypto/cppki:go_default_library",
diff --git a/go/cs/revocation/handler.go b/go/cs/revocation/handler.go
index 78a408d0c..c6a167d71 100644
--- a/go/cs/revocation/handler.go
+++ b/go/cs/revocation/handler.go
@@ -73,7 +73,7 @@ func (h *handler) Handle(request *infra.Request) *infra.HandlerResult {
 	defer cancelF()
 
 	sendAck := messenger.SendAckHelper(subCtx, rw)
-	revInfo, err := revocation.VerifiedRevInfo(subCtx, h.verifier)
+	revInfo, err := revocation.RevInfo()
 	if err != nil {
 		logger.Info("[RevHandler] Parsing/Verifying failed",
 			"signer", revocation.Sign.Src, "err", err)
diff --git a/go/lib/ctrl/path_mgmt/rev_info.go b/go/lib/ctrl/path_mgmt/rev_info.go
index c41ef0db7..c87220e3a 100644
--- a/go/lib/ctrl/path_mgmt/rev_info.go
+++ b/go/lib/ctrl/path_mgmt/rev_info.go
@@ -192,19 +192,6 @@ func (sr *SignedRevInfo) RevInfo() (*RevInfo, error) {
 	return sr.revInfo, err
 }
 
-// VerifiedRevInfo verifies the signature and returns the revocation
-// information or an error in case the verification fails.
-func (sr *SignedRevInfo) VerifiedRevInfo(ctx context.Context, verifier Verifier) (*RevInfo, error) {
-	var err error
-	if sr.revInfo == nil {
-		sr.revInfo, err = NewRevInfoFromRaw(sr.Blob)
-	}
-	if err != nil {
-		return sr.revInfo, err
-	}
-	return sr.revInfo, verifier.Verify(ctx, sr.Blob, sr.Sign)
-}
-
 func (sr *SignedRevInfo) Pack() (common.RawBytes, error) {
 	return proto.PackRoot(sr)
 }
diff --git a/go/lib/infra/modules/segverifier/segverifier.go b/go/lib/infra/modules/segverifier/segverifier.go
index 849e79869..f4a1466f4 100644
--- a/go/lib/infra/modules/segverifier/segverifier.go
+++ b/go/lib/infra/modules/segverifier/segverifier.go
@@ -194,7 +194,7 @@ func verifyRevInfo(ctx context.Context, verifier infra.Verifier, server net.Addr
 func VerifyRevInfo(ctx context.Context, verifier infra.Verifier, server net.Addr,
 	signedRevInfo *path_mgmt.SignedRevInfo) error {
 
-	if _, err := signedRevInfo.VerifiedRevInfo(ctx, verifier.WithServer(server)); err != nil {
+	if _, err := signedRevInfo.RevInfo(); err != nil {
 		return serrors.Wrap(ErrRevocation, err, "rev", signedRevInfo)
 	}
 	return nil


Test patch:
diff --git a/go/cs/ifstate/revoker_test.go b/go/cs/ifstate/revoker_test.go
index dbd857a2f..544583be0 100644
--- a/go/cs/ifstate/revoker_test.go
+++ b/go/cs/ifstate/revoker_test.go
@@ -306,8 +306,8 @@ func checkRevocation(t *testing.T, srev *path_mgmt.SignedRevInfo,
 	revokedIfId common.IFIDType, verifier revVerifier, topoProvider topology.Provider) {
 
 	Convey("Check revocation", func() {
-		revInfo, err := srev.VerifiedRevInfo(context.Background(), verifier)
-		SoMsg("No verification err expected", err, ShouldBeNil)
+		revInfo, err := srev.RevInfo()
+		SoMsg("No err expected", err, ShouldBeNil)
 		SoMsg("correct ifId", revInfo.IfID, ShouldEqual, revokedIfId)
 		SoMsg("correct IA", revInfo.RawIsdas, ShouldEqual, ia.IAInt())
 		SoMsg("correct linkType", revInfo.LinkType,
diff --git a/go/cs/revocation/handler_test.go b/go/cs/revocation/handler_test.go
index 46964fbde..23bd7bede 100644
--- a/go/cs/revocation/handler_test.go
+++ b/go/cs/revocation/handler_test.go
@@ -31,12 +31,10 @@ import (
 
 	"github.com/scionproto/scion/go/cs/metrics"
 	"github.com/scionproto/scion/go/cs/revocation/mock_revocation"
-	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/lib/ctrl"
 	"github.com/scionproto/scion/go/lib/ctrl/ack"
 	"github.com/scionproto/scion/go/lib/ctrl/path_mgmt"
 	"github.com/scionproto/scion/go/lib/infra"
-	"github.com/scionproto/scion/go/lib/infra/messenger"
 	"github.com/scionproto/scion/go/lib/infra/mock_infra"
 	"github.com/scionproto/scion/go/lib/log"
 	"github.com/scionproto/scion/go/lib/scrypto/cppki"
@@ -74,26 +72,12 @@ func TestHandler(t *testing.T) {
 	sRev, err := path_mgmt.NewSignedRevInfo(rev, signer)
 	xtest.FailOnErr(t, err)
 
-	sRevInvalid, err := path_mgmt.NewSignedRevInfo(rev, signer)
-	xtest.FailOnErr(t, err)
-	// flip a bit
-	sRevInvalid.Blob[0] ^= 0xFF
-
 	tests := []struct {
 		Name   string
 		Rev    *path_mgmt.SignedRevInfo
 		Ack    *ack.Ack
 		Result *infra.HandlerResult
 	}{
-		{
-			Name: "Unverifiable revocation is rejected",
-			Rev:  sRevInvalid,
-			Ack: &ack.Ack{
-				Err:     proto.Ack_ErrCode_reject,
-				ErrDesc: messenger.AckRejectFailedToVerify,
-			},
-			Result: infra.MetricsErrInvalid,
-		},
 		{
 			Name: "Verifiable rev is stored and acked",
 			Rev:  sRev,
@@ -111,11 +95,6 @@ func TestHandler(t *testing.T) {
 			defer mctrl.Finish()
 
 			verifier := mock_infra.NewMockVerifier(mctrl)
-			verifier.EXPECT().Verify(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
-				func(_ context.Context, msg common.RawBytes, sign *proto.SignS) error {
-					return verifyecdsa(sign.SigInput(msg, false), sign.Signature, pub)
-				},
-			)
 
 			rw := mock_infra.NewMockResponseWriter(mctrl)
 			if test.Ack != nil {
diff --git a/go/lib/xtest/matchers/matchers.go b/go/lib/xtest/matchers/matchers.go
index 7c76f51dd..63034edb2 100644
--- a/go/lib/xtest/matchers/matchers.go
+++ b/go/lib/xtest/matchers/matchers.go
@@ -18,7 +18,6 @@ package matchers
 
 import (
 	"bytes"
-	"context"
 	"fmt"
 	"reflect"
 	"sort"
@@ -92,7 +91,7 @@ func (m *SignedRevs) Matches(x interface{}) bool {
 	}
 	revInfos := make(map[path_mgmt.RevInfo]*path_mgmt.RevInfo)
 	for _, rev := range sRevs {
-		revInfo, err := rev.VerifiedRevInfo(context.Background(), m.Verifier)
+		revInfo, err := rev.RevInfo()
 		if err != nil {
 			return false
 		}