Merge pull request #6665 from multiversx/fixes-header-sig-verification

Fixes header sig verification

cmd/node/factory/interface.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/multiversx/mx-chain-core-go/core"
 	"github.com/multiversx/mx-chain-core-go/data"
+
 	"github.com/multiversx/mx-chain-go/p2p"
 )
 
@@ -15,7 +16,6 @@ type HeaderSigVerifierHandler interface {
 	VerifyRandSeedAndLeaderSignature(header data.HeaderHandler) error
 	VerifySignature(header data.HeaderHandler) error
 	VerifySignatureForHash(header data.HeaderHandler, hash []byte, pubkeysBitmap []byte, signature []byte) error
-	VerifyPreviousBlockProof(header data.HeaderHandler) error
 	VerifyHeaderProof(headerProof data.HeaderProofHandler) error
 	IsInterfaceNil() bool
 }

consensus/broadcast/delayedBroadcast.go

@@ -699,9 +699,8 @@ func (dbb *delayedBlockBroadcaster) interceptedHeader(_ string, headerHash []byt
 
 	// TODO: should be handled from interceptor
 	proof := headerHandler.GetPreviousProof()
-
 	var aggSig, bitmap []byte
-	if proof != nil {
+	if !check.IfNilReflect(proof) {
 		aggSig, bitmap = proof.GetAggregatedSignature(), proof.GetPubKeysBitmap()
 	}
 

consensus/interface.go

@@ -127,13 +127,13 @@ type HeaderSigVerifier interface {
 	VerifyLeaderSignature(header data.HeaderHandler) error
 	VerifySignature(header data.HeaderHandler) error
 	VerifySignatureForHash(header data.HeaderHandler, hash []byte, pubkeysBitmap []byte, signature []byte) error
-	VerifyPreviousBlockProof(header data.HeaderHandler) error
 	VerifyHeaderProof(headerProof data.HeaderProofHandler) error
 	IsInterfaceNil() bool
 }
 
 // FallbackHeaderValidator defines the behaviour of a component able to signal when a fallback header validation could be applied
 type FallbackHeaderValidator interface {
+	ShouldApplyFallbackValidationForHeaderWith(shardID uint32, startOfEpochBlock bool, round uint64, prevHeaderHash []byte) bool
 	ShouldApplyFallbackValidation(headerHandler data.HeaderHandler) bool
 	IsInterfaceNil() bool
 }

consensus/spos/bls/v1/subroundBlock.go

@@ -415,9 +415,14 @@ func (sr *subroundBlock) receivedBlockBodyAndHeader(ctx context.Context, cnsDta
 		return false
 	}
 
+	header := sr.BlockProcessor().DecodeBlockHeader(cnsDta.Header)
+	if headerHasProof(header) {
+		return false
+	}
+
 	sr.SetData(cnsDta.BlockHeaderHash)
 	sr.SetBody(sr.BlockProcessor().DecodeBlockBody(cnsDta.Body))
-	sr.SetHeader(sr.BlockProcessor().DecodeBlockHeader(cnsDta.Header))
+	sr.SetHeader(header)
 
 	isInvalidData := check.IfNil(sr.GetBody()) || sr.isInvalidHeaderOrData()
 	if isInvalidData {
@@ -514,8 +519,13 @@ func (sr *subroundBlock) receivedBlockHeader(ctx context.Context, cnsDta *consen
 		return false
 	}
 
+	header := sr.BlockProcessor().DecodeBlockHeader(cnsDta.Header)
+	if headerHasProof(header) {
+		return false
+	}
+
 	sr.SetData(cnsDta.BlockHeaderHash)
-	sr.SetHeader(sr.BlockProcessor().DecodeBlockHeader(cnsDta.Header))
+	sr.SetHeader(header)
 
 	if sr.isInvalidHeaderOrData() {
 		return false
@@ -535,6 +545,13 @@ func (sr *subroundBlock) receivedBlockHeader(ctx context.Context, cnsDta *consen
 	return blockProcessedWithSuccess
 }
 
+func headerHasProof(headerHandler data.HeaderHandler) bool {
+	if check.IfNil(headerHandler) {
+		return false
+	}
+	return !check.IfNilReflect(headerHandler.GetPreviousProof())
+}
+
 func (sr *subroundBlock) processReceivedBlock(ctx context.Context, cnsDta *consensus.Message) bool {
 	if check.IfNil(sr.GetBody()) {
 		return false

consensus/spos/bls/v2/subroundBlock.go

@@ -9,6 +9,7 @@ import (
 	"github.com/multiversx/mx-chain-core-go/core"
 	"github.com/multiversx/mx-chain-core-go/core/check"
 	"github.com/multiversx/mx-chain-core-go/data"
+
 	"github.com/multiversx/mx-chain-go/common"
 	"github.com/multiversx/mx-chain-go/consensus"
 	"github.com/multiversx/mx-chain-go/consensus/spos"
@@ -110,7 +111,6 @@ func (sr *subroundBlock) doBlockJob(ctx context.Context) bool {
 		return false
 	}
 
-	// todo: check again the block proof verification & leader signature verification
 	// block proof verification should be done over the header that contains the leader signature
 	leaderSignature, err := sr.signBlockHeader(header)
 	if err != nil {
@@ -177,7 +177,7 @@ func printLogMessage(ctx context.Context, baseMessage string, err error) {
 	log.Debug(baseMessage, "error", err.Error())
 }
 
-func (sr *subroundBlock) sendBlock(header data.HeaderHandler, body data.BodyHandler, leader string) bool {
+func (sr *subroundBlock) sendBlock(header data.HeaderHandler, body data.BodyHandler, _ string) bool {
 	marshalledBody, err := sr.Marshalizer().Marshal(body)
 	if err != nil {
 		log.Debug("sendBlock.Marshal: body", "error", err.Error())

consensus/spos/errors.go

@@ -253,9 +253,6 @@ var ErrEquivalentMessageAlreadyReceived = errors.New("equivalent message already
 // ErrNilEnableEpochsHandler signals that a nil enable epochs handler has been provided
 var ErrNilEnableEpochsHandler = errors.New("nil enable epochs handler")
 
-// ErrMissingProposerSignature signals that proposer signature is missing
-var ErrMissingProposerSignature = errors.New("missing proposer signature")
-
 // ErrNilThrottler signals that a nil throttler has been provided
 var ErrNilThrottler = errors.New("nil throttler")
 
@@ -267,3 +264,9 @@ var ErrNilEquivalentProofPool = errors.New("nil equivalent proof pool")
 
 // ErrNilHeaderProof signals that a nil header proof has been provided
 var ErrNilHeaderProof = errors.New("nil header proof")
+
+// ErrHeaderProofNotExpected signals that a header proof was not expected
+var ErrHeaderProofNotExpected = errors.New("header proof not expected")
+
+// ErrConsensusMessageNotExpected signals that a consensus message was not expected
+var ErrConsensusMessageNotExpected = errors.New("consensus message not expected")

consensus/spos/interface.go

@@ -138,7 +138,7 @@ type HeaderSigVerifier interface {
 	VerifyLeaderSignature(header data.HeaderHandler) error
 	VerifySignature(header data.HeaderHandler) error
 	VerifySignatureForHash(header data.HeaderHandler, hash []byte, pubkeysBitmap []byte, signature []byte) error
-	VerifyPreviousBlockProof(header data.HeaderHandler) error
+	VerifyHeaderWithProof(header data.HeaderHandler) error
 	VerifyHeaderProof(headerProof data.HeaderProofHandler) error
 	IsInterfaceNil() bool
 }

consensus/spos/worker.go

@@ -525,10 +525,9 @@ func (wrk *Worker) doJobOnMessageWithHeader(cnsMsg *consensus.Message) error {
 			err)
 	}
 
-	err = wrk.headerSigVerifier.VerifyPreviousBlockProof(header)
+	err = wrk.checkHeaderPreviousProof(header)
 	if err != nil {
-		return fmt.Errorf("%w : verify previous block proof for received header from consensus topic failed",
-			err)
+		return err
 	}
 
 	wrk.processReceivedHeaderMetric(cnsMsg)
@@ -544,6 +543,18 @@ func (wrk *Worker) doJobOnMessageWithHeader(cnsMsg *consensus.Message) error {
 	return nil
 }
 
+func (wrk *Worker) checkHeaderPreviousProof(header data.HeaderHandler) error {
+	if wrk.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, header.GetEpoch()) {
+		return fmt.Errorf("%w : received header on consensus topic after equivalent messages activation", ErrConsensusMessageNotExpected)
+	}
+
+	if !check.IfNilReflect(header.GetPreviousProof()) {
+		return fmt.Errorf("%w : received header from consensus topic has previous proof", ErrHeaderProofNotExpected)
+	}
+
+	return nil
+}
+
 func (wrk *Worker) verifyHeaderHash(hash []byte, marshalledHeader []byte) bool {
 	computedHash := wrk.hasher.Compute(string(marshalledHeader))
 	return bytes.Equal(hash, computedHash)

dataRetriever/dataPool/proofsCache/proofsPool.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"sync"
 
+	"github.com/multiversx/mx-chain-core-go/core/check"
 	"github.com/multiversx/mx-chain-core-go/data"
 	logger "github.com/multiversx/mx-chain-logger-go"
 )
@@ -26,7 +27,7 @@ func NewProofsPool() *proofsPool {
 func (pp *proofsPool) AddProof(
 	headerProof data.HeaderProofHandler,
 ) error {
-	if headerProof == nil {
+	if check.IfNilReflect(headerProof) {
 		return ErrNilProof
 	}
 

epochStart/bootstrap/disabled/disabledHeaderSigVerifier.go

@@ -2,6 +2,7 @@ package disabled
 
 import (
 	"github.com/multiversx/mx-chain-core-go/data"
+
 	"github.com/multiversx/mx-chain-go/process"
 )
 
@@ -40,8 +41,8 @@ func (h *headerSigVerifier) VerifySignatureForHash(_ data.HeaderHandler, _ []byt
 	return nil
 }
 
-// VerifyPreviousBlockProof returns nil as it is disabled
-func (h *headerSigVerifier) VerifyPreviousBlockProof(_ data.HeaderHandler) error {
+// VerifyHeaderWithProof returns nil as it is disabled
+func (h *headerSigVerifier) VerifyHeaderWithProof(_ data.HeaderHandler) error {
 	return nil
 }
 

fallback/headerValidator.go

@@ -5,10 +5,11 @@ import (
 	"github.com/multiversx/mx-chain-core-go/core/check"
 	"github.com/multiversx/mx-chain-core-go/data"
 	"github.com/multiversx/mx-chain-core-go/marshal"
+	"github.com/multiversx/mx-chain-logger-go"
+
 	"github.com/multiversx/mx-chain-go/common"
 	"github.com/multiversx/mx-chain-go/dataRetriever"
 	"github.com/multiversx/mx-chain-go/process"
-	"github.com/multiversx/mx-chain-logger-go"
 )
 
 var log = logger.GetOrCreate("fallback")
@@ -45,28 +46,34 @@ func NewFallbackHeaderValidator(
 	return hv, nil
 }
 
-// ShouldApplyFallbackValidation returns if for the given header could be applied fallback validation or not
-func (fhv *fallbackHeaderValidator) ShouldApplyFallbackValidation(headerHandler data.HeaderHandler) bool {
-	if check.IfNil(headerHandler) {
-		return false
-	}
-	if headerHandler.GetShardID() != core.MetachainShardId {
+// ShouldApplyFallbackValidationForHeaderWith returns if for the given header data fallback validation could be applied or not
+func (fhv *fallbackHeaderValidator) ShouldApplyFallbackValidationForHeaderWith(shardID uint32, startOfEpochBlock bool, round uint64, prevHeaderHash []byte) bool {
+	if shardID != core.MetachainShardId {
 		return false
 	}
-	if !headerHandler.IsStartOfEpochBlock() {
+	if !startOfEpochBlock {
 		return false
 	}
 
-	previousHeader, err := process.GetMetaHeader(headerHandler.GetPrevHash(), fhv.headersPool, fhv.marshalizer, fhv.storageService)
+	previousHeader, err := process.GetMetaHeader(prevHeaderHash, fhv.headersPool, fhv.marshalizer, fhv.storageService)
 	if err != nil {
 		log.Debug("ShouldApplyFallbackValidation", "GetMetaHeader", err.Error())
 		return false
 	}
 
-	isRoundTooOld := int64(headerHandler.GetRound())-int64(previousHeader.GetRound()) >= common.MaxRoundsWithoutCommittedStartInEpochBlock
+	isRoundTooOld := int64(round)-int64(previousHeader.GetRound()) >= common.MaxRoundsWithoutCommittedStartInEpochBlock
 	return isRoundTooOld
 }
 
+// ShouldApplyFallbackValidation returns if for the given header could be applied fallback validation or not
+func (fhv *fallbackHeaderValidator) ShouldApplyFallbackValidation(headerHandler data.HeaderHandler) bool {
+	if check.IfNil(headerHandler) {
+		return false
+	}
+
+	return fhv.ShouldApplyFallbackValidationForHeaderWith(headerHandler.GetShardID(), headerHandler.IsStartOfEpochBlock(), headerHandler.GetRound(), headerHandler.GetPrevHash())
+}
+
 // IsInterfaceNil returns true if there is no value under the interface
 func (fhv *fallbackHeaderValidator) IsInterfaceNil() bool {
 	return fhv == nil

integrationTests/consensus/consensusSigning_test.go

@@ -8,8 +8,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/multiversx/mx-chain-go/integrationTests"
 	"github.com/stretchr/testify/assert"
+
+	"github.com/multiversx/mx-chain-go/integrationTests"
 )
 
 func initNodesWithTestSigner(

process/block/baseProcess.go

@@ -364,7 +364,7 @@ func displayHeader(headerHandler data.HeaderHandler) []*display.LineData {
 	proof := headerHandler.GetPreviousProof()
 
 	var prevAggregatedSig, prevBitmap []byte
-	if proof != nil {
+	if !check.IfNilReflect(proof) {
 		prevAggregatedSig, prevBitmap = proof.GetAggregatedSignature(), proof.GetPubKeysBitmap()
 	}
 

process/block/baseProcess_test.go

@@ -3140,7 +3140,10 @@ func TestBaseProcessor_CheckSentSignaturesAtCommitTime(t *testing.T) {
 		arguments.NodesCoordinator = nodesCoordinatorInstance
 		bp, _ := blproc.NewShardProcessor(arguments)
 
-		err := bp.CheckSentSignaturesAtCommitTime(&block.Header{})
+		err := bp.CheckSentSignaturesAtCommitTime(&block.Header{
+			RandSeed:     []byte("randSeed"),
+			PrevRandSeed: []byte("prevRandSeed"),
+		})
 		assert.Equal(t, expectedErr, err)
 	})
 	t.Run("should work with bitmap", func(t *testing.T) {
@@ -3164,6 +3167,8 @@ func TestBaseProcessor_CheckSentSignaturesAtCommitTime(t *testing.T) {
 		bp, _ := blproc.NewShardProcessor(arguments)
 
 		err := bp.CheckSentSignaturesAtCommitTime(&block.Header{
+			RandSeed:      []byte("randSeed"),
+			PrevRandSeed:  []byte("prevRandSeed"),
 			PubKeysBitmap: []byte{0b00000101},
 		})
 		assert.Nil(t, err)

process/block/interceptedBlocks/common.go

@@ -6,6 +6,7 @@ import (
 	"github.com/multiversx/mx-chain-core-go/core"
 	"github.com/multiversx/mx-chain-core-go/core/check"
 	"github.com/multiversx/mx-chain-core-go/data"
+
 	"github.com/multiversx/mx-chain-go/common"
 	"github.com/multiversx/mx-chain-go/process"
 	"github.com/multiversx/mx-chain-go/sharding"
@@ -70,14 +71,15 @@ func checkMiniblockArgument(arg *ArgInterceptedMiniblock) error {
 }
 
 func checkHeaderHandler(hdr data.HeaderHandler, enableEpochsHandler common.EnableEpochsHandler) error {
-	// TODO[cleanup cns finality]: remove these checks
-	if len(hdr.GetPubKeysBitmap()) == 0 && !enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, hdr.GetEpoch()) {
+	equivalentMessagesEnabled := enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, hdr.GetEpoch())
+
+	if len(hdr.GetPubKeysBitmap()) == 0 && !equivalentMessagesEnabled {
 		return process.ErrNilPubKeysBitmap
 	}
 	if len(hdr.GetPrevHash()) == 0 {
 		return process.ErrNilPreviousBlockHash
 	}
-	if len(hdr.GetSignature()) == 0 && !enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, hdr.GetEpoch()) {
+	if len(hdr.GetSignature()) == 0 && !equivalentMessagesEnabled {
 		return process.ErrNilSignature
 	}
 	if len(hdr.GetRootHash()) == 0 {
@@ -90,16 +92,42 @@ func checkHeaderHandler(hdr data.HeaderHandler, enableEpochsHandler common.Enabl
 		return process.ErrNilPrevRandSeed
 	}
 
+	err := checkProofIntegrity(hdr, enableEpochsHandler)
+	if err != nil {
+		return err
+	}
+
 	return hdr.CheckFieldsForNil()
 }
 
+func checkProofIntegrity(hdr data.HeaderHandler, enableEpochsHandler common.EnableEpochsHandler) error {
+	equivalentMessagesEnabled := enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, hdr.GetEpoch())
+
+	prevHeaderProof := hdr.GetPreviousProof()
+	nilPreviousProof := check.IfNilReflect(prevHeaderProof)
+	missingProof := nilPreviousProof && equivalentMessagesEnabled
+	unexpectedProof := !nilPreviousProof && !equivalentMessagesEnabled
+	hasProof := !nilPreviousProof && equivalentMessagesEnabled
+
+	if missingProof {
+		return process.ErrMissingHeaderProof
+	}
+	if unexpectedProof {
+		return process.ErrUnexpectedHeaderProof
+	}
+	if hasProof && isIncompleteProof(prevHeaderProof) {
+		return process.ErrInvalidHeaderProof
+	}
+
+	return nil
+}
+
 func checkMetaShardInfo(
 	shardInfo []data.ShardDataHandler,
 	coordinator sharding.Coordinator,
 	headerSigVerifier process.InterceptedHeaderSigVerifier,
 ) error {
 	wgProofsVerification := sync.WaitGroup{}
-	wgProofsVerification.Add(len(shardInfo))
 	errChan := make(chan error, len(shardInfo))
 	for _, sd := range shardInfo {
 		if sd.GetShardID() >= coordinator.NumberOfShards() && sd.GetShardID() != core.MetachainShardId {
@@ -111,6 +139,7 @@ func checkMetaShardInfo(
 			return err
 		}
 
+		wgProofsVerification.Add(1)
 		checkProofAsync(sd.GetPreviousProof(), headerSigVerifier, &wgProofsVerification, errChan)
 	}
 

process/block/interceptedBlocks/interceptedBlockHeader.go

@@ -78,11 +78,6 @@ func (inHdr *InterceptedHeader) CheckValidity() error {
 		return err
 	}
 
-	err = inHdr.sigVerifier.VerifyPreviousBlockProof(inHdr.hdr)
-	if err != nil {
-		return err
-	}
-
 	return inHdr.verifySignatures()
 }