From b20078c0c2f61fc3925ffb949765ec2a73405254 Mon Sep 17 00:00:00 2001 From: MatheusFranco99 <48058141+MatheusFranco99@users.noreply.github.com> Date: Thu, 1 Feb 2024 08:40:55 +0000 Subject: [PATCH] Add verify signatures flag to isProposalJustification. Add validSignedRoundChangeForData function. Propagate name changes. --- qbft/instance.go | 2 +- qbft/proposal.go | 45 +++++++++++++++++++++------- qbft/round_change.go | 70 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 12 deletions(-) diff --git a/qbft/instance.go b/qbft/instance.go index c357cdfdb..8de6fb0dd 100644 --- a/qbft/instance.go +++ b/qbft/instance.go @@ -177,7 +177,7 @@ func (i *Instance) BaseMsgValidation(msg *SignedMessage) error { i.State.Share.Committee, ) case RoundChangeMsgType: - return validRoundChangeForData(i.State, i.config, msg, i.State.Height, msg.Message.Round, msg.FullData) + return validSignedRoundChangeForData(i.State, i.config, msg, i.State.Height, msg.Message.Round, msg.FullData) default: return errors.New("signed message type not supported") } diff --git a/qbft/proposal.go b/qbft/proposal.go index fa62d49fd..0ab10922c 100644 --- a/qbft/proposal.go +++ b/qbft/proposal.go @@ -2,6 +2,7 @@ package qbft import ( "bytes" + "github.com/bloxapp/ssv-spec/types" "github.com/pkg/errors" ) @@ -93,6 +94,7 @@ func isValidProposal( signedProposal.Message.Round, signedProposal.FullData, valCheck, + true, ); err != nil { return errors.Wrap(err, "proposal not justified") } @@ -114,6 +116,7 @@ func isProposalJustification( round Round, fullData []byte, valCheck ProposedValueCheckF, + verifySignatures bool, ) error { if err := valCheck(fullData); err != nil { return errors.Wrap(err, "proposal fullData invalid") @@ -126,9 +129,16 @@ 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 { - return errors.Wrap(err, "change round msg not valid") + if verifySignatures { + if err := validSignedRoundChangeForData(state, config, rc, height, round, fullData); err != nil { + return errors.Wrap(err, "change round msg not valid") + } + } else { + if err := validRoundChangeForData(state, config, rc, height, round, fullData); err != nil { + return errors.Wrap(err, "change round msg not valid") + } } + } // check there is a quorum @@ -178,15 +188,28 @@ func isProposalJustification( // validate each prepare message against the highest previously prepared fullData and round for _, pm := range prepareMsgs { - if err := validSignedPrepareForHeightRoundAndRoot( - config, - pm, - height, - rcm.Message.DataRound, - rcm.Message.Root, - state.Share.Committee, - ); err != nil { - return errors.New("signed prepare not valid") + if verifySignatures { + if err := validSignedPrepareForHeightRoundAndRoot( + config, + pm, + height, + rcm.Message.DataRound, + rcm.Message.Root, + state.Share.Committee, + ); err != nil { + return errors.New("signed prepare not valid") + } + } else { + if err := validPrepareForHeightRoundAndRoot( + config, + pm, + height, + rcm.Message.DataRound, + rcm.Message.Root, + state.Share.Committee, + ); err != nil { + return errors.New("signed prepare not valid") + } } } return nil diff --git a/qbft/round_change.go b/qbft/round_change.go index 9232e60f8..561d95cd1 100644 --- a/qbft/round_change.go +++ b/qbft/round_change.go @@ -2,6 +2,7 @@ package qbft import ( "bytes" + "github.com/bloxapp/ssv-spec/types" "github.com/pkg/errors" ) @@ -200,12 +201,15 @@ func isReceivedProposalJustification( newRound, value, valCheck, + false, ); err != nil { return errors.Wrap(err, "proposal not justified") } return nil } +// validRoundChangeForData is similar to the validSignedRoundChangeForData function +// However, it does not verify signatures func validRoundChangeForData( state *State, config IConfig, @@ -227,6 +231,72 @@ func validRoundChangeForData( return errors.New("msg allows 1 signer") } + if err := signedMsg.Message.Validate(); err != nil { + return errors.Wrap(err, "roundChange invalid") + } + + // Addition to formal spec + // We add this extra tests on the msg itself to filter round change msgs with invalid justifications, before they are inserted into msg containers + if signedMsg.Message.RoundChangePrepared() { + r, err := HashDataRoot(fullData) + if err != nil { + return errors.Wrap(err, "could not hash input data") + } + + // validate prepare message justifications + prepareMsgs, _ := signedMsg.Message.GetRoundChangeJustifications() // no need to check error, checked on signedMsg.Message.Validate() + for _, pm := range prepareMsgs { + if err := validPrepareForHeightRoundAndRoot( + config, + pm, + state.Height, + signedMsg.Message.DataRound, + signedMsg.Message.Root, + state.Share.Committee); err != nil { + return errors.Wrap(err, "round change justification invalid") + } + } + + if !bytes.Equal(r[:], signedMsg.Message.Root[:]) { + return errors.New("H(data) != root") + } + + if !HasQuorum(state.Share, prepareMsgs) { + return errors.New("no justifications quorum") + } + + if signedMsg.Message.DataRound > round { + return errors.New("prepared round > round") + } + + return nil + } + return nil +} + +// validSignedRoundChangeForData is based on dafny's validRoundChange function +// with the addition that nested Prepare messages are also validated +func validSignedRoundChangeForData( + state *State, + config IConfig, + signedMsg *SignedMessage, + height Height, + round Round, + fullData []byte, +) error { + if signedMsg.Message.MsgType != RoundChangeMsgType { + return errors.New("round change msg type is wrong") + } + if signedMsg.Message.Height != height { + return errors.New("wrong msg height") + } + if signedMsg.Message.Round != round { + return errors.New("wrong msg round") + } + if len(signedMsg.GetSigners()) != 1 { + 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") }