Spec - Replace redundant BLS in QBFT with RSA (#372)

* Drop BLS check where redundant

* Check that the message signers belong to the instance committee

* Add signer in committee check for proposal

* Fix tests error msgs. Drop old tests

* Refactor into WithVerification NoVerification functions

* Drop unnecessary config arguments

* Add msg.Validate() tests for round-change

* Remove unused argument

* Add NetworkPubKey to Operator

* Fix roots due to changes in field names in the Operator type

* Generate JSON tests

* Make validator.ProcessMsg receive a SignedSSVMessage; Add SignatureVerifier module to Validator

* Add validation checks to Validator.ProcessMessage

* Testing util for SignedSSVMessage conversion

* Add SignedSSVMessage to tests

* Generate tests JSON

* Add tests for Validator.ProcessMessage

* Generate tests JSONs

* Rename BeaconPubKey to SharePubKey

* Fix roots due to the field name change

* Generate tests JSONs

* Store SignedSSVMessage in broadcaster

* Rename NetworkKey to SSVKey

* Fix tests roots due to name change

* Generate JSON tests

* Rename NoVerification to IgnoreSignature

* Test msg with invalid signature for runner-consensus

* Split signer into SSVShare and SSVOperator. Rename SSVKey to SSVOperatorKey

* Add mutex and fix test roots

* Generate test JSONs

* Fix lock usage

* Drop duplicated check in msg validation

* Verify RSA signatures of broadcasted messages

* Apply suggestions

* Add suggestions

* Change decode and verify order

* Rename SSVOperatorKeys to OperatorKeys in testing util

* Define signature to have 256 bytes

* Rename Network to Operator in tests

p2p/validation/msg_validation.go

@@ -14,28 +14,13 @@ import (
 // MsgValidatorFunc represents a message validator
 type MsgValidatorFunc = func(ctx context.Context, p peer.ID, msg *pubsub.Message) pubsub.ValidationResult
 
-// To-Do: better spec the intended Message Validation behavior. Issue #375: https://github.com/bloxapp/ssv-spec/issues/375
-type MessageValidator interface {
-	ValidateSignedSSVMessage(runner ssv.Runner, signedSSVMessage *types.SignedSSVMessage) error
-	// Verifies the message's RSA signature with PKCSv1.5 encoding. Ref: https://datatracker.ietf.org/doc/html/rfc8017#section-8.2.2
-	VerifySignature(runner ssv.Runner, signedSSVMessage *types.SignedSSVMessage) error
-}
-
-func MsgValidation(runner ssv.Runner, msgValidator MessageValidator) MsgValidatorFunc {
+func MsgValidation(runner ssv.Runner) MsgValidatorFunc {
 	return func(ctx context.Context, p peer.ID, msg *pubsub.Message) pubsub.ValidationResult {
 		signedSSVMsg, err := DecodePubsubMsg(msg)
 		if err != nil {
 			return pubsub.ValidationReject
 		}
 
-		// Message Validator
-		if err := msgValidator.ValidateSignedSSVMessage(runner, signedSSVMsg); err != nil {
-			return pubsub.ValidationReject
-		}
-		if err := msgValidator.VerifySignature(runner, signedSSVMsg); err != nil {
-			return pubsub.ValidationReject
-		}
-
 		// Get SSVMessage
 		ssvMsg, err := signedSSVMsg.GetSSVMessageFromData()
 		if err != nil {

qbft/commit.go

@@ -2,6 +2,7 @@ package qbft
 
 import (
 	"bytes"
+
 	"github.com/bloxapp/ssv-spec/types"
 	"github.com/pkg/errors"
 )
@@ -81,7 +82,7 @@ func CreateCommit(state *State, config IConfig, root [32]byte) (*SignedMessage,
 
 		Root: root,
 	}
-	sig, err := config.GetSigner().SignRoot(msg, types.QBFTSignatureType, state.Share.SharePubKey)
+	sig, err := config.GetShareSigner().SignRoot(msg, types.QBFTSignatureType, state.Share.SharePubKey)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed signing commit msg")
 	}
@@ -94,8 +95,7 @@ func CreateCommit(state *State, config IConfig, root [32]byte) (*SignedMessage,
 	return signedMsg, nil
 }
 
-func baseCommitValidation(
-	config IConfig,
+func baseCommitValidationIgnoreSignature(
 	signedCommit *SignedMessage,
 	height Height,
 	operators []*types.Operator,
@@ -111,6 +111,24 @@ func baseCommitValidation(
 		return errors.Wrap(err, "signed commit invalid")
 	}
 
+	if !signedCommit.CheckSignersInCommittee(operators) {
+		return errors.New("signer not in committee")
+	}
+
+	return nil
+}
+
+func baseCommitValidationVerifySignature(
+	config IConfig,
+	signedCommit *SignedMessage,
+	height Height,
+	operators []*types.Operator,
+) error {
+
+	if err := baseCommitValidationIgnoreSignature(signedCommit, height, operators); err != nil {
+		return err
+	}
+
 	// verify signature
 	if err := signedCommit.Signature.VerifyByOperators(signedCommit, config.GetSignatureDomainType(), types.QBFTSignatureType, operators); err != nil {
 		return errors.Wrap(err, "msg signature invalid")
@@ -120,14 +138,13 @@ func baseCommitValidation(
 }
 
 func validateCommit(
-	config IConfig,
 	signedCommit *SignedMessage,
 	height Height,
 	round Round,
 	proposedMsg *SignedMessage,
 	operators []*types.Operator,
 ) error {
-	if err := baseCommitValidation(config, signedCommit, height, operators); err != nil {
+	if err := baseCommitValidationIgnoreSignature(signedCommit, height, operators); err != nil {
 		return err
 	}
 

qbft/decided.go

@@ -2,6 +2,7 @@ package qbft
 
 import (
 	"bytes"
+
 	"github.com/bloxapp/ssv-spec/types"
 	"github.com/pkg/errors"
 )
@@ -64,7 +65,7 @@ func ValidateDecided(
 		return errors.Wrap(err, "invalid decided msg")
 	}
 
-	if err := baseCommitValidation(config, signedDecided, signedDecided.Message.Height, share.Committee); err != nil {
+	if err := baseCommitValidationVerifySignature(config, signedDecided, signedDecided.Message.Height, share.Committee); err != nil {
 		return errors.Wrap(err, "invalid decided msg")
 	}
 

qbft/instance.go

@@ -155,8 +155,7 @@ func (i *Instance) BaseMsgValidation(msg *SignedMessage) error {
 		if proposedMsg == nil {
 			return errors.New("did not receive proposal for this round")
 		}
-		return validSignedPrepareForHeightRoundAndRoot(
-			i.config,
+		return validSignedPrepareForHeightRoundAndRootIgnoreSignature(
 			msg,
 			i.State.Height,
 			i.State.Round,
@@ -169,15 +168,14 @@ func (i *Instance) BaseMsgValidation(msg *SignedMessage) error {
 			return errors.New("did not receive proposal for this round")
 		}
 		return validateCommit(
-			i.config,
 			msg,
 			i.State.Height,
 			i.State.Round,
 			i.State.ProposalAcceptedForCurrentRound,
 			i.State.Share.Committee,
 		)
 	case RoundChangeMsgType:
-		return validRoundChangeForData(i.State, i.config, msg, i.State.Height, msg.Message.Round, msg.FullData)
+		return validRoundChangeForDataIgnoreSignature(i.State, i.config, msg, i.State.Height, msg.Message.Round, msg.FullData)
 	default:
 		return errors.New("signed message type not supported")
 	}

qbft/instance_test.go

@@ -38,8 +38,8 @@ func TestInstance_Marshaling(t *testing.T) {
 		PartialQuorum:   2,
 		Committee: []*types.Operator{
 			{
-				OperatorID: 1,
-				PubKey:     TestingSK.GetPublicKey().Serialize(),
+				OperatorID:  1,
+				SharePubKey: TestingSK.GetPublicKey().Serialize(),
 			},
 		},
 	}

qbft/messages.go

@@ -285,3 +285,20 @@ func (signedMsg *SignedMessage) WithoutFUllData() *SignedMessage {
 		Message:   signedMsg.Message,
 	}
 }
+
+// Check if all signedMsg's signers belong to the given committee in O(n+m)
+func (signedMsg *SignedMessage) CheckSignersInCommittee(committee []*types.Operator) bool {
+	// Committee's operators map
+	committeeMap := make(map[uint64]struct{})
+	for _, operator := range committee {
+		committeeMap[operator.OperatorID] = struct{}{}
+	}
+
+	// Check that all message signers belong to the map
+	for _, signer := range signedMsg.Signers {
+		if _, ok := committeeMap[signer]; !ok {
+			return false
+		}
+	}
+	return true
+}

qbft/prepare.go

@@ -2,6 +2,7 @@ package qbft
 
 import (
 	"bytes"
+
 	"github.com/bloxapp/ssv-spec/types"
 	"github.com/pkg/errors"
 )
@@ -59,8 +60,7 @@ func getRoundChangeJustification(state *State, config IConfig, prepareMsgContain
 	prepareMsgs := prepareMsgContainer.MessagesForRound(state.LastPreparedRound)
 	ret := make([]*SignedMessage, 0)
 	for _, msg := range prepareMsgs {
-		if err := validSignedPrepareForHeightRoundAndRoot(
-			config,
+		if err := validSignedPrepareForHeightRoundAndRootIgnoreSignature(
 			msg,
 			state.Height,
 			state.LastPreparedRound,
@@ -79,8 +79,7 @@ func getRoundChangeJustification(state *State, config IConfig, prepareMsgContain
 
 // validSignedPrepareForHeightRoundAndRoot known in dafny spec as validSignedPrepareForHeightRoundAndDigest
 // https://entethalliance.github.io/client-spec/qbft_spec.html#dfn-qbftspecification
-func validSignedPrepareForHeightRoundAndRoot(
-	config IConfig,
+func validSignedPrepareForHeightRoundAndRootIgnoreSignature(
 	signedPrepare *SignedMessage,
 	height Height,
 	round Round,
@@ -108,6 +107,26 @@ func validSignedPrepareForHeightRoundAndRoot(
 		return errors.New("msg allows 1 signer")
 	}
 
+	if !signedPrepare.CheckSignersInCommittee(operators) {
+		return errors.New("signer not in committee")
+	}
+
+	return nil
+}
+
+func validSignedPrepareForHeightRoundAndRootVerifySignature(
+	config IConfig,
+	signedPrepare *SignedMessage,
+	height Height,
+	round Round,
+	root [32]byte,
+	operators []*types.Operator) error {
+
+	if err := validSignedPrepareForHeightRoundAndRootIgnoreSignature(signedPrepare, height, round, root, operators); err != nil {
+		return err
+	}
+
+	// Verify signature
 	if err := signedPrepare.Signature.VerifyByOperators(signedPrepare, config.GetSignatureDomainType(), types.QBFTSignatureType, operators); err != nil {
 		return errors.Wrap(err, "msg signature invalid")
 	}
@@ -136,7 +155,7 @@ func CreatePrepare(state *State, config IConfig, newRound Round, root [32]byte)
 
 		Root: root,
 	}
-	sig, err := config.GetSigner().SignRoot(msg, types.QBFTSignatureType, state.Share.SharePubKey)
+	sig, err := config.GetShareSigner().SignRoot(msg, types.QBFTSignatureType, state.Share.SharePubKey)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed signing prepare msg")
 	}

qbft/proposal.go

@@ -2,6 +2,7 @@ package qbft
 
 import (
 	"bytes"
+
 	"github.com/bloxapp/ssv-spec/types"
 	"github.com/pkg/errors"
 )
@@ -60,9 +61,11 @@ func isValidProposal(
 	if len(signedProposal.GetSigners()) != 1 {
 		return errors.New("msg allows 1 signer")
 	}
-	if err := signedProposal.Signature.VerifyByOperators(signedProposal, config.GetSignatureDomainType(), types.QBFTSignatureType, operators); err != nil {
-		return errors.Wrap(err, "msg signature invalid")
+
+	if !signedProposal.CheckSignersInCommittee(state.Share.Committee) {
+		return errors.New("signer not in committee")
 	}
+
 	if !signedProposal.MatchedSigners([]types.OperatorID{proposer(state, config, signedProposal.Message.Round)}) {
 		return errors.New("proposal leader invalid")
 	}
@@ -126,7 +129,7 @@ func isProposalJustification(
 		// no quorum, duplicate signers,  invalid still has quorum, invalid no quorum
 		// prepared
 		for _, rc := range roundChangeMsgs {
-			if err := validRoundChangeForData(state, config, rc, height, round, fullData); err != nil {
+			if err := validRoundChangeForDataVerifySignature(state, config, rc, height, round, fullData); err != nil {
 				return errors.Wrap(err, "change round msg not valid")
 			}
 		}
@@ -178,7 +181,7 @@ func isProposalJustification(
 
 			// validate each prepare message against the highest previously prepared fullData and round
 			for _, pm := range prepareMsgs {
-				if err := validSignedPrepareForHeightRoundAndRoot(
+				if err := validSignedPrepareForHeightRoundAndRootVerifySignature(
 					config,
 					pm,
 					height,
@@ -237,7 +240,7 @@ func CreateProposal(state *State, config IConfig, fullData []byte, roundChanges,
 		RoundChangeJustification: roundChangesData,
 		PrepareJustification:     preparesData,
 	}
-	sig, err := config.GetSigner().SignRoot(msg, types.QBFTSignatureType, state.Share.SharePubKey)
+	sig, err := config.GetShareSigner().SignRoot(msg, types.QBFTSignatureType, state.Share.SharePubKey)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed signing prepare msg")
 	}

qbft/round_change.go

@@ -2,6 +2,7 @@ package qbft
 
 import (
 	"bytes"
+
 	"github.com/bloxapp/ssv-spec/types"
 	"github.com/pkg/errors"
 )
@@ -206,7 +207,7 @@ func isReceivedProposalJustification(
 	return nil
 }
 
-func validRoundChangeForData(
+func validRoundChangeForDataIgnoreSignature(
 	state *State,
 	config IConfig,
 	signedMsg *SignedMessage,
@@ -227,12 +228,12 @@ func validRoundChangeForData(
 		return errors.New("msg allows 1 signer")
 	}
 
-	if err := signedMsg.Signature.VerifyByOperators(signedMsg, config.GetSignatureDomainType(), types.QBFTSignatureType, state.Share.Committee); err != nil {
-		return errors.Wrap(err, "msg signature invalid")
+	if err := signedMsg.Validate(); err != nil {
+		return errors.Wrap(err, "roundChange invalid")
 	}
 
-	if err := signedMsg.Message.Validate(); err != nil {
-		return errors.Wrap(err, "roundChange invalid")
+	if !signedMsg.CheckSignersInCommittee(state.Share.Committee) {
+		return errors.New("signer not in committee")
 	}
 
 	// Addition to formal spec
@@ -246,7 +247,7 @@ func validRoundChangeForData(
 		// validate prepare message justifications
 		prepareMsgs, _ := signedMsg.Message.GetRoundChangeJustifications() // no need to check error, checked on signedMsg.Message.Validate()
 		for _, pm := range prepareMsgs {
-			if err := validSignedPrepareForHeightRoundAndRoot(
+			if err := validSignedPrepareForHeightRoundAndRootVerifySignature(
 				config,
 				pm,
 				state.Height,
@@ -271,6 +272,28 @@ func validRoundChangeForData(
 
 		return nil
 	}
+
+	return nil
+}
+
+func validRoundChangeForDataVerifySignature(
+	state *State,
+	config IConfig,
+	signedMsg *SignedMessage,
+	height Height,
+	round Round,
+	fullData []byte,
+) error {
+
+	if err := validRoundChangeForDataIgnoreSignature(state, config, signedMsg, height, round, fullData); err != nil {
+		return err
+	}
+
+	// Verify signature
+	if err := signedMsg.Signature.VerifyByOperators(signedMsg, config.GetSignatureDomainType(), types.QBFTSignatureType, state.Share.Committee); err != nil {
+		return errors.Wrap(err, "msg signature invalid")
+	}
+
 	return nil
 }
 
@@ -355,7 +378,7 @@ func CreateRoundChange(state *State, config IConfig, newRound Round, instanceSta
 		DataRound:                round,
 		RoundChangeJustification: justificationsData,
 	}
-	sig, err := config.GetSigner().SignRoot(msg, types.QBFTSignatureType, state.Share.SharePubKey)
+	sig, err := config.GetShareSigner().SignRoot(msg, types.QBFTSignatureType, state.Share.SharePubKey)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed signing prepare msg")
 	}