diff --git a/bft/bft.go b/bft/bft.go index ca9c3e60ba..011966cff4 100644 --- a/bft/bft.go +++ b/bft/bft.go @@ -155,12 +155,9 @@ func (b *BFT) Start() { func() { b.Controller.Lock() defer b.Controller.Unlock() - // calculate time since - since := time.Since(resetBFT.StartTime) - // allow if 'since' is less than 1 block old - if int(since.Milliseconds()) < b.Config.BlockTimeMS() { + processTime = validProcessTime(resetBFT.StartTime, time.Duration(b.Config.BlockTimeMS())*time.Millisecond) + if processTime != 0 { b.log.Infof("Using included timestamp to calculate process time: %s", resetBFT.StartTime.Format(time.StampMilli)) - processTime = since } // if is a root-chain update reset back to round 0 but maintain locks to prevent 'fork attacks' // else increment the height and don't maintain locks @@ -185,6 +182,13 @@ func (b *BFT) Start() { } } +func validProcessTime(start time.Time, maxAge time.Duration) time.Duration { + if since := time.Since(start); since > 0 && since < maxAge { + return since + } + return 0 +} + // HandlePhase() is the main BFT Phase stepping loop func (b *BFT) HandlePhase() { stopTimers := func() { b.PhaseTimer.Stop() } @@ -609,8 +613,8 @@ func (b *BFT) Pacemaker() bool { continue } totalVotedPower += validator.VotingPower - // if totalVotePower >= +33%, it's safe to advance to that round - if totalVotedPower >= lib.Uint64ReducePercentage(b.ValidatorSet.MinimumMaj23, 50) { + // if totalVotePower > 1/3, it's safe to advance to that round + if totalVotedPower > b.ValidatorSet.TotalPower/3 { pacemakerRound = vote.Qc.Header.Round // set the highest round where +1/3rds have been break } diff --git a/bft/bft_test.go b/bft/bft_test.go index 3592c2fa5d..6139d517b2 100644 --- a/bft/bft_test.go +++ b/bft/bft_test.go @@ -89,6 +89,12 @@ func TestStartElectionPhase(t *testing.T) { } } +func TestValidProcessTimeRejectsFutureTimestamp(t *testing.T) { + require.Zero(t, validProcessTime(time.Now().Add(time.Hour), time.Second)) + require.Zero(t, validProcessTime(time.Now().Add(-time.Hour), time.Second)) + require.Positive(t, validProcessTime(time.Now().Add(-time.Millisecond), time.Second)) +} + func TestStartElectionVotePhase(t *testing.T) { tests := []struct { name string @@ -745,6 +751,19 @@ func TestPacemaker(t *testing.T) { } } +func TestPacemakerRequiresMoreThanOneThird(t *testing.T) { + c := newTestConsensus(t, Propose, 2) + c.bft.ValidatorSet.ValidatorSet.ValidatorSet[0].VotingPower = 67 + c.bft.ValidatorSet.ValidatorSet.ValidatorSet[1].VotingPower = 33 + c.bft.ValidatorSet.TotalPower = 100 + msg := &Message{Qc: &QC{Header: c.view(RoundInterrupt, 3)}} + require.NoError(t, msg.Sign(c.valKeys[1])) + require.NoError(t, c.bft.HandleMessage(msg)) + + c.bft.Pacemaker() + require.Equal(t, uint64(1), c.bft.Round) +} + func TestScheduleForceRound(t *testing.T) { c := newTestConsensus(t, Election, 1) c.bft.Round = 2 diff --git a/bft/mock_test.go b/bft/mock_test.go index 11524fa33c..165be33a70 100644 --- a/bft/mock_test.go +++ b/bft/mock_test.go @@ -239,6 +239,7 @@ func (tc *testConsensus) simLead(t *testing.T, mk crypto.MultiPublicKeyI, round ResultsHash: resHash, Block: block, BlockHash: blkHash, + ProposerKey: tc.bft.ProposerKey, Signature: &lib.AggregateSignature{ Signature: as, Bitmap: mk.Bitmap(), @@ -276,6 +277,7 @@ func (tc *testConsensus) simVote(t *testing.T, round uint64, phase Phase, callba Header: tc.view(phase, round), BlockHash: blkHash, ResultsHash: resHash, + ProposerKey: tc.bft.ProposerKey, }, } // execute callback on the message to allow custom phase functionality @@ -354,6 +356,7 @@ func (tc *testConsensus) newPartialQCDoubleSign(t *testing.T, phase Phase) { Header: tc.view(phase-1, 1), BlockHash: crypto.Hash([]byte("some proposal")), ResultsHash: crypto.Hash([]byte("some results")), + ProposerKey: tc.valKeys[0].PublicKey().Bytes(), } // create the bytes to be signed by the 'double signers' sb := qc.SignBytes() diff --git a/bft/msg.go b/bft/msg.go index 577528237a..e9ca9c99e5 100644 --- a/bft/msg.go +++ b/bft/msg.go @@ -81,6 +81,10 @@ func (b *BFT) CheckProposerMessage(x *Message, p *validateMessageParams) (isPart if err = x.Qc.CheckBasic(); err != nil { return } + // ensure the sender is justified as the proposer + if !bytes.Equal(x.Qc.ProposerKey, x.Signature.PublicKey) { + return false, lib.ErrInvalidSigner() + } // if an unexpected root height if x.Qc.Header.RootHeight != p.rootHeight { // load the proper committee @@ -136,10 +140,6 @@ func (b *BFT) CheckProposerMessage(x *Message, p *validateMessageParams) (isPart return false, lib.ErrInvalidQCCommitteeHeight() } if x.Header.Phase == Propose { - // ensure the sender is justified as the proposer - if !bytes.Equal(x.Qc.ProposerKey, x.Signature.PublicKey) { - return false, lib.ErrInvalidSigner() - } // ensure the block isn't nil if x.Qc.Block == nil { return false, lib.ErrNilBlock() diff --git a/bft/msg_test.go b/bft/msg_test.go index 735ed6570b..97d376dda5 100644 --- a/bft/msg_test.go +++ b/bft/msg_test.go @@ -289,3 +289,14 @@ func TestProposerMessageRejectsInvalidHighQC(t *testing.T) { require.Error(t, errI) require.Equal(t, lib.CodeInvalidAggregateSignature, errI.Code()) } + +func TestProposerMessageRejectsPostElectionRelay(t *testing.T) { + c := newTestConsensus(t, Precommit, 3) + c.simPrecommitPhase(t, 0) + msg := c.bft.Proposals[0][phaseToString(Precommit)][0] + require.NoError(t, msg.Sign(c.valKeys[1])) + + err := c.bft.HandleMessage(msg) + require.Error(t, err) + require.Equal(t, lib.CodeInvalidSigner, err.Code()) +} diff --git a/bft/vote.go b/bft/vote.go index 411bee00dd..228e732ede 100644 --- a/bft/vote.go +++ b/bft/vote.go @@ -5,6 +5,11 @@ import ( "github.com/canopy-network/canopy/lib/crypto" ) +const ( + maxVDFElementSize = 1024 + maxVDFIterations = uint64(1<<31 - 1) +) + // LEADER TRACKING AND AGGREGATING MESSAGES FROM REPLICAS // NOTE: A 'Vote' is a digital signature of SignBytes from a Replica Validator. By signing a message and sending it to the Leader, @@ -52,14 +57,14 @@ func (b *BFT) AddVote(vote *Message) lib.ErrorI { b.Controller.Lock() defer b.Controller.Unlock() voteSet := b.getVoteSet(vote) - // handle high qc and byzantine evidence (only applicable if ELECTION-VOTE) - if err := b.handleHighQCVDFAndEvidence(vote); err != nil { - return err - } // add the vote to the set if err := b.addSigToVoteSet(vote, voteSet); err != nil { return err } + // optional attachments don't invalidate the signed vote + if err := b.handleHighQCVDFAndEvidence(vote); err != nil { + b.log.Warnf("Ignoring invalid vote attachment: %s", err.Error()) + } return nil } @@ -143,6 +148,9 @@ func (b *BFT) handleHighQCVDFAndEvidence(vote *Message) lib.ErrorI { } // pre handle VDF if enabled if b.Config.RunVDF && vote.Vdf != nil && vote.Vdf.Iterations != 0 { + if len(vote.Vdf.Proof) > maxVDFElementSize || len(vote.Vdf.Output) > maxVDFElementSize || vote.Vdf.Iterations > maxVDFIterations { + return lib.ErrInvalidVDF() + } // save the obtained VDF vote to be processed at the PROPOSE phase b.VDFCache = append(b.VDFCache, vote) } diff --git a/bft/vote_test.go b/bft/vote_test.go index 999f3bd764..67e071f399 100644 --- a/bft/vote_test.go +++ b/bft/vote_test.go @@ -2,10 +2,11 @@ package bft import ( "bytes" + "testing" + "github.com/canopy-network/canopy/lib" "github.com/canopy-network/canopy/lib/crypto" "github.com/stretchr/testify/require" - "testing" ) func TestAddVote(t *testing.T) { @@ -92,3 +93,31 @@ func TestAddVote(t *testing.T) { }) } } + +func TestAddVoteAttachments(t *testing.T) { + consensus := newTestConsensus(t, ElectionVote, 4) + consensus.bft.Config.RunVDF = true + newVote := func(i int, vdf *crypto.VDF) *Message { + return &Message{ + Qc: &QC{Header: &lib.View{Phase: ElectionVote}}, + Signature: &lib.Signature{ + PublicKey: consensus.valKeys[i].PublicKey().Bytes(), + Signature: bytes.Repeat([]byte("F"), 96), + }, + Vdf: vdf, + } + } + require.NoError(t, consensus.bft.AddVote(newVote(0, nil))) + err := consensus.bft.AddVote(newVote(0, &crypto.VDF{Proof: make([]byte, maxVDFElementSize+1), Iterations: 1})) + require.ErrorContains(t, err, "duplicate vote") + for i, vdf := range []*crypto.VDF{ + {Proof: make([]byte, maxVDFElementSize+1), Iterations: 1}, + {Output: make([]byte, maxVDFElementSize+1), Iterations: 1}, + {Iterations: maxVDFIterations + 1}, + } { + vote := newVote(i+1, vdf) + require.NoError(t, consensus.bft.AddVote(vote)) + require.NotZero(t, consensus.bft.getVoteSet(vote).TotalVotedPower) + } + require.Empty(t, consensus.bft.VDFCache) +} diff --git a/controller/block.go b/controller/block.go index 04d702b6ad..7aafb31252 100644 --- a/controller/block.go +++ b/controller/block.go @@ -564,6 +564,10 @@ func (c *Controller) ApplyAndValidateBlock(block *lib.Block, commit bool) (b *li // HandlePeerBlock() validates and handles an inbound certificate (with a block) from a remote peer func (c *Controller) HandlePeerBlock(msg *lib.BlockMessage, syncing bool) (*lib.QuorumCertificate, lib.ErrorI) { + return c.handlePeerBlock(msg, syncing, false) +} + +func (c *Controller) handlePeerBlock(msg *lib.BlockMessage, syncing, verifyQC bool) (*lib.QuorumCertificate, lib.ErrorI) { // log the start of 'peer block handling' c.log.Info("Handling peer block") // define a convenience variable for the certificate @@ -599,7 +603,7 @@ func (c *Controller) HandlePeerBlock(msg *lib.BlockMessage, syncing bool) (*lib. } } } - if !syncing || qc.Header.Height%CheckpointFrequency == 0 { + if verifyQC || !syncing || qc.Header.Height%CheckpointFrequency == 0 { // load the committee from the root chain using the root height embedded in the certificate message v, err := c.Consensus.LoadCommittee(c.LoadRootChainId(qc.Header.Height), qc.Header.RootHeight) if err != nil { diff --git a/controller/consensus.go b/controller/consensus.go index 617acf43ac..9271e36247 100644 --- a/controller/consensus.go +++ b/controller/consensus.go @@ -183,7 +183,7 @@ func (c *Controller) processQueue(startHeight, stopHeight uint64, queue map[uint // lock the controller c.Lock() // process the block message received from the peer - _, err := c.HandlePeerBlock(blockMsg, true) + _, err := c.handlePeerBlock(blockMsg, true, height+1 == stopHeight) // unlock controller c.Unlock() // check error from HandlePeerBlock @@ -316,11 +316,6 @@ func (c *Controller) ListenForConsensus() { } // execute in a sub-function to unify error handling and enable 'defer' functionality if err := func() (err lib.ErrorI) { - // check and add the message to the cache to prevent duplicates - if ok := cache.Add(msg); !ok { - // duplicate, exit - return - } // create a new 'consensus message' to unmarshal the bytes to bftMsg := new(bft.Message) // try to unmarshal into a consensus message @@ -328,6 +323,14 @@ func (c *Controller) ListenForConsensus() { // exit with error return } + // direct consensus messages must come from their signer; gossip messages may come from a relay + if !validConsensusSender(msg.Sender.Address.PublicKey, bftMsg, c.P2P.GossipMode()) { + return lib.ErrInvalidSigner() + } + // check and add the message to the cache to prevent duplicates + if ok := cache.Add(msg); !ok { + return + } // check whether the message should be gossiped gossip, exit := c.ShouldGossip(bftMsg) if gossip { @@ -353,6 +356,10 @@ func (c *Controller) ListenForConsensus() { } } +func validConsensusSender(sender []byte, msg *bft.Message, gossipMode bool) bool { + return msg.GetSignature() != nil && (gossipMode || bytes.Equal(sender, msg.Signature.PublicKey)) +} + // ShouldGossip() controls whether a consensus message should be gossiped func (c *Controller) ShouldGossip(msg *bft.Message) (gossip bool, exit bool) { // only gossip when enabled diff --git a/controller/controller_test.go b/controller/controller_test.go index f9c056b766..f5a273378d 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -5,9 +5,19 @@ import ( "path/filepath" "testing" + "github.com/canopy-network/canopy/bft" + "github.com/canopy-network/canopy/lib" "github.com/stretchr/testify/require" ) +func TestValidConsensusSender(t *testing.T) { + msg := &bft.Message{Signature: &lib.Signature{PublicKey: []byte{1}}} + require.True(t, validConsensusSender([]byte{1}, msg, false)) + require.False(t, validConsensusSender([]byte{2}, msg, false)) + require.True(t, validConsensusSender([]byte{2}, msg, true)) + require.False(t, validConsensusSender([]byte{1}, &bft.Message{}, true)) +} + func TestResolvePluginCtlPath(t *testing.T) { wd, err := os.Getwd() require.NoError(t, err) diff --git a/lib/crypto/classgroup.go b/lib/crypto/classgroup.go index 4e72b6fb6a..94b314c469 100644 --- a/lib/crypto/classgroup.go +++ b/lib/crypto/classgroup.go @@ -332,6 +332,9 @@ func (group *ClassGroup) Decode(bz []byte, discriminant *big.Int) (err error) { if err = cdc.Unmarshal(bz, classGroup); err != nil { return err } + if len(classGroup.A) == 0 || len(classGroup.B) == 0 { + return fmt.Errorf("invalid empty class-group coefficient") + } // convert the byte slices to big.Int values a := decodeBigInt(classGroup.A) b := decodeBigInt(classGroup.B) diff --git a/lib/crypto/vdf.go b/lib/crypto/vdf.go index 9c5d0ba122..a88ce86cb8 100644 --- a/lib/crypto/vdf.go +++ b/lib/crypto/vdf.go @@ -30,7 +30,12 @@ func GenerateVDF(seed []byte, iterations int, stop <-chan struct{}) (out []byte, } // VerifyVDF() verifies VDF bytes given a seed and iterations -func VerifyVDF(seed, out, proof []byte, iterations int) bool { +func VerifyVDF(seed, out, proof []byte, iterations int) (valid bool) { + defer func() { + if recover() != nil { + valid = false + } + }() discriminant, classGroup := initVDF(seed) y, p := new(ClassGroup), new(ClassGroup) if err := y.Decode(out, discriminant); err != nil { diff --git a/lib/crypto/vdf_test.go b/lib/crypto/vdf_test.go index b1eb285e2a..b74480fd18 100644 --- a/lib/crypto/vdf_test.go +++ b/lib/crypto/vdf_test.go @@ -117,6 +117,17 @@ func TestRandomInput(t *testing.T) { } } +func TestVerifyVDFRejectsMalformedProofs(t *testing.T) { + seed := []byte{0xde, 0xad, 0xbe, 0xef} + const iterations = 50 + validOut, validProof := GenerateVDF(seed, iterations, nil) + zeroCoefficient := []byte{0x0a, 0x01, 0x00, 0x12, 0x01, 0x00} + require.False(t, VerifyVDF(seed, nil, validProof, iterations)) + require.False(t, VerifyVDF(seed, validOut, nil, iterations)) + require.False(t, VerifyVDF(seed, zeroCoefficient, validProof, iterations)) + require.False(t, VerifyVDF(seed, validOut, zeroCoefficient, iterations)) +} + func TestInterruptGenerator(t *testing.T) { seed := []byte{0xde, 0xad, 0xbe, 0xef} stop := make(chan struct{})