EVM-433-TOB-EDGE-35-Lack-of-domain-separation (#1336)

* Initial changes

* UT fix

* Comments fix

consensus/polybft/checkpoint_manager_test.go

@@ -61,7 +61,7 @@ func TestCheckpointManager_SubmitCheckpoint(t *testing.T) {
 
 	validators.iterAcct(aliases, func(t *testValidator) {
 		bitmap.Set(idx)
-		signatures = append(signatures, t.mustSign(dummyMsg))
+		signatures = append(signatures, t.mustSign(dummyMsg, bls.DomainCheckpointManager))
 		idx++
 	})
 
@@ -139,7 +139,7 @@ func TestCheckpointManager_abiEncodeCheckpointBlock(t *testing.T) {
 	var signatures bls.Signatures
 
 	currentValidators.iterAcct(nil, func(v *testValidator) {
-		signatures = append(signatures, v.mustSign(proposalHash))
+		signatures = append(signatures, v.mustSign(proposalHash, bls.DomainCheckpointManager))
 		bmp.Set(i)
 		i++
 	})

consensus/polybft/consensus_runtime.go

@@ -10,6 +10,7 @@ import (
 	"sync/atomic"
 
 	"github.com/0xPolygon/polygon-edge/consensus/polybft/contractsapi"
+	bls "github.com/0xPolygon/polygon-edge/consensus/polybft/signer"
 	"github.com/0xPolygon/polygon-edge/consensus/polybft/wallet"
 	"github.com/0xPolygon/polygon-edge/txrelayer"
 	"github.com/0xPolygon/polygon-edge/types"
@@ -863,7 +864,7 @@ func (c *consensusRuntime) BuildPrepareMessage(proposalHash []byte, view *proto.
 
 // BuildCommitMessage builds a COMMIT message based on the passed in proposal
 func (c *consensusRuntime) BuildCommitMessage(proposalHash []byte, view *proto.View) *proto.Message {
-	committedSeal, err := c.config.Key.Sign(proposalHash)
+	committedSeal, err := c.config.Key.SignWithDomain(proposalHash, bls.DomainCheckpointManager)
 	if err != nil {
 		c.logger.Error("Cannot create committed seal message.", "error", err)
 

consensus/polybft/consensus_runtime_test.go

@@ -335,7 +335,7 @@ func TestConsensusRuntime_FSM_NotEndOfEpoch_NotEndOfSprint(t *testing.T) {
 			EpochSize:  10,
 			SprintSize: 5,
 		},
-		Key:        wallet.NewKey(validators.getPrivateIdentities()[0], bls.DomainCheckpointManager),
+		Key:        wallet.NewKey(validators.getPrivateIdentities()[0]),
 		blockchain: blockchainMock,
 	}
 	runtime := &consensusRuntime{
@@ -686,7 +686,7 @@ func TestConsensusRuntime_TamperMessageContent(t *testing.T) {
 	}
 	sender := validatorAccounts.getValidator("A")
 	proposalHash := []byte{2, 4, 6, 8, 10}
-	proposalSignature, err := sender.Key().Sign(proposalHash)
+	proposalSignature, err := sender.Key().SignWithDomain(proposalHash, bls.DomainCheckpointManager)
 	require.NoError(t, err)
 
 	msg := &proto.Message{
@@ -999,7 +999,7 @@ func TestConsensusRuntime_BuildCommitMessage(t *testing.T) {
 		},
 	}
 
-	committedSeal, err := key.Sign(proposalHash)
+	committedSeal, err := key.SignWithDomain(proposalHash, bls.DomainCheckpointManager)
 	require.NoError(t, err)
 
 	expected := proto.Message{
@@ -1065,18 +1065,6 @@ func TestConsensusRuntime_BuildPrepareMessage(t *testing.T) {
 	assert.Equal(t, signedMsg, runtime.BuildPrepareMessage(proposalHash, view))
 }
 
-func createTestMessageVote(t *testing.T, hash []byte, validator *testValidator) *MessageSignature {
-	t.Helper()
-
-	signature, err := validator.mustSign(hash).Marshal()
-	require.NoError(t, err)
-
-	return &MessageSignature{
-		From:      validator.Key().String(),
-		Signature: signature,
-	}
-}
-
 func createTestBlocks(t *testing.T, numberOfBlocks, defaultEpochSize uint64,
 	validatorSet AccountSet) (*types.Header, *testHeadersMap) {
 	t.Helper()

consensus/polybft/extra_test.go

@@ -189,7 +189,7 @@ func TestExtra_UnmarshalRLPWith_NegativeCases(t *testing.T) {
 		key, err := wallet.GenerateAccount()
 		require.NoError(t, err)
 
-		parentSignature := createSignature(t, []*wallet.Account{key}, types.BytesToHash([]byte("This is test hash")))
+		parentSignature := createSignature(t, []*wallet.Account{key}, types.BytesToHash([]byte("This is test hash")), bls.DomainCheckpointManager)
 		extraMarshalled.Set(parentSignature.MarshalRLPWith(ar))
 
 		// Committed
@@ -212,11 +212,11 @@ func TestExtra_UnmarshalRLPWith_NegativeCases(t *testing.T) {
 		key, err := wallet.GenerateAccount()
 		require.NoError(t, err)
 
-		parentSignature := createSignature(t, []*wallet.Account{key}, types.BytesToHash(generateRandomBytes(t)))
+		parentSignature := createSignature(t, []*wallet.Account{key}, types.BytesToHash(generateRandomBytes(t)), bls.DomainCheckpointManager)
 		extraMarshalled.Set(parentSignature.MarshalRLPWith(ar))
 
 		// Committed
-		committedSignature := createSignature(t, []*wallet.Account{key}, types.BytesToHash(generateRandomBytes(t)))
+		committedSignature := createSignature(t, []*wallet.Account{key}, types.BytesToHash(generateRandomBytes(t)), bls.DomainCheckpointManager)
 		extraMarshalled.Set(committedSignature.MarshalRLPWith(ar))
 
 		// Checkpoint data
@@ -279,7 +279,7 @@ func TestExtra_ValidateFinalizedData_UnhappyPath(t *testing.T) {
 	polyBackendMock = new(polybftBackendMock)
 	polyBackendMock.On("GetValidators", mock.Anything, mock.Anything).Return(validators.getPublicIdentities())
 
-	noQuorumSignature := createSignature(t, validators.getPrivateIdentities("0", "1"), types.BytesToHash([]byte("FooBar")))
+	noQuorumSignature := createSignature(t, validators.getPrivateIdentities("0", "1"), types.BytesToHash([]byte("FooBar")), bls.DomainCheckpointManager)
 	extra = &Extra{Committed: noQuorumSignature, Checkpoint: checkpoint}
 	checkpointHash, err := checkpoint.Hash(chainID, headerNum, header.Hash)
 	require.NoError(t, err)
@@ -290,7 +290,7 @@ func TestExtra_ValidateFinalizedData_UnhappyPath(t *testing.T) {
 		fmt.Sprintf("failed to verify signatures for block %d (proposal hash %s): quorum not reached", headerNum, checkpointHash))
 
 	// incorrect parent extra size
-	validSignature := createSignature(t, validators.getPrivateIdentities(), checkpointHash)
+	validSignature := createSignature(t, validators.getPrivateIdentities(), checkpointHash, bls.DomainCheckpointManager)
 	extra = &Extra{Committed: validSignature, Checkpoint: checkpoint}
 	err = extra.ValidateFinalizedData(
 		header, parent, nil, chainID, polyBackendMock, bls.DomainCheckpointManager, hclog.NewNullLogger())
@@ -323,7 +323,7 @@ func TestExtra_ValidateParentSignatures(t *testing.T) {
 	// validators not found
 	validators := newTestValidators(t, 5)
 	incorrectHash := types.BytesToHash([]byte("Hello World"))
-	invalidSig := createSignature(t, validators.getPrivateIdentities(), incorrectHash)
+	invalidSig := createSignature(t, validators.getPrivateIdentities(), incorrectHash, bls.DomainCheckpointManager)
 	extra = &Extra{Parent: invalidSig}
 	err = extra.ValidateParentSignatures(
 		headerNum, polyBackendMock, nil, nil, nil, chainID, bls.DomainCheckpointManager, hclog.NewNullLogger())
@@ -347,7 +347,7 @@ func TestExtra_ValidateParentSignatures(t *testing.T) {
 		fmt.Sprintf("failed to verify signatures for parent of block %d (proposal hash: %s): could not verify aggregated signature", headerNum, parentCheckpointHash))
 
 	// valid signature provided
-	validSig := createSignature(t, validators.getPrivateIdentities(), parentCheckpointHash)
+	validSig := createSignature(t, validators.getPrivateIdentities(), parentCheckpointHash, bls.DomainCheckpointManager)
 	extra = &Extra{Parent: validSig}
 	err = extra.ValidateParentSignatures(
 		headerNum, polyBackendMock, nil, parent, parentExtra, chainID, bls.DomainCheckpointManager, hclog.NewNullLogger())

consensus/polybft/fsm.go

@@ -437,7 +437,7 @@ func (f *fsm) VerifyStateTransactions(transactions []*types.Transaction) error {
 				return err
 			}
 
-			verified := aggs.VerifyAggregated(signers.GetBlsKeys(), hash.Bytes(), bls.DomainCheckpointManager)
+			verified := aggs.VerifyAggregated(signers.GetBlsKeys(), hash.Bytes(), bls.DomainStateReceiver)
 			if !verified {
 				return fmt.Errorf("invalid signature for tx = %v", tx.Hash)
 			}

consensus/polybft/fsm_test.go

@@ -117,7 +117,7 @@ func TestFSM_BuildProposal_WithoutCommitEpochTxGood(t *testing.T) {
 	runtime := &consensusRuntime{
 		logger: hclog.NewNullLogger(),
 		config: &runtimeConfig{
-			Key:        wallet.NewKey(validators.getPrivateIdentities()[0], bls.DomainCheckpointManager),
+			Key:        wallet.NewKey(validators.getPrivateIdentities()[0]),
 			blockchain: blockchainMock,
 		},
 	}
@@ -192,7 +192,7 @@ func TestFSM_BuildProposal_WithCommitEpochTxGood(t *testing.T) {
 	runtime := &consensusRuntime{
 		logger: hclog.NewNullLogger(),
 		config: &runtimeConfig{
-			Key:        wallet.NewKey(validators.getPrivateIdentities()[0], bls.DomainCheckpointManager),
+			Key:        wallet.NewKey(validators.getPrivateIdentities()[0]),
 			blockchain: blockChainMock,
 		},
 	}
@@ -571,7 +571,7 @@ func TestFSM_VerifyStateTransactions_StateTransactionInvalidSignature(t *testing
 	aggregatedSigs := bls.Signatures{}
 
 	nonValidators.iterAcct(nil, func(t *testValidator) {
-		aggregatedSigs = append(aggregatedSigs, t.mustSign([]byte("dummyHash")))
+		aggregatedSigs = append(aggregatedSigs, t.mustSign([]byte("dummyHash"), bls.DomainStateReceiver))
 	})
 
 	sig, err := aggregatedSigs.Aggregate().Marshal()
@@ -653,7 +653,7 @@ func TestFSM_ValidateCommit_InvalidHash(t *testing.T) {
 	require.NoError(t, err)
 
 	nonValidatorAcc := newTestValidator(t, "non_validator", 1)
-	wrongSignature, err := nonValidatorAcc.mustSign([]byte("Foo")).Marshal()
+	wrongSignature, err := nonValidatorAcc.mustSign([]byte("Foo"), bls.DomainCheckpointManager).Marshal()
 	require.NoError(t, err)
 
 	err = fsm.ValidateCommit(validators.getValidator("0").Address().Bytes(), wrongSignature, []byte{})
@@ -687,7 +687,7 @@ func TestFSM_ValidateCommit_Good(t *testing.T) {
 	require.NoError(t, block.UnmarshalRLP(proposal))
 
 	validator := validators.getValidator("A")
-	seal, err := validator.mustSign(block.Hash().Bytes()).Marshal()
+	seal, err := validator.mustSign(block.Hash().Bytes(), bls.DomainCheckpointManager).Marshal()
 	require.NoError(t, err)
 	err = fsm.ValidateCommit(validator.Key().Address().Bytes(), seal, block.Hash().Bytes())
 	require.NoError(t, err)
@@ -875,7 +875,7 @@ func TestFSM_Insert_Good(t *testing.T) {
 		seals := make([]*messages.CommittedSeal, signaturesCount)
 
 		for i := 0; i < signaturesCount; i++ {
-			sign, err := allAccounts[i].Bls.Sign(builtBlock.Block.Hash().Bytes(), bls.DomainValidatorSet)
+			sign, err := allAccounts[i].Bls.Sign(builtBlock.Block.Hash().Bytes(), bls.DomainCheckpointManager)
 			require.NoError(t, err)
 			sigRaw, err := sign.Marshal()
 			require.NoError(t, err)
@@ -959,15 +959,15 @@ func TestFSM_Insert_InvalidNode(t *testing.T) {
 	validatorA := validators.getValidator("A")
 	validatorB := validators.getValidator("B")
 	proposalHash := buildBlock.Block.Hash().Bytes()
-	sigA, err := validatorA.mustSign(proposalHash).Marshal()
+	sigA, err := validatorA.mustSign(proposalHash, bls.DomainCheckpointManager).Marshal()
 	require.NoError(t, err)
 
-	sigB, err := validatorB.mustSign(proposalHash).Marshal()
+	sigB, err := validatorB.mustSign(proposalHash, bls.DomainCheckpointManager).Marshal()
 	require.NoError(t, err)
 
 	// create test account outside of validator set
 	nonValidatorAccount := newTestValidator(t, "non_validator", 1)
-	nonValidatorSignature, err := nonValidatorAccount.mustSign(proposalHash).Marshal()
+	nonValidatorSignature, err := nonValidatorAccount.mustSign(proposalHash, bls.DomainCheckpointManager).Marshal()
 	require.NoError(t, err)
 
 	commitedSeals := []*messages.CommittedSeal{
@@ -1059,7 +1059,7 @@ func TestFSM_VerifyStateTransaction_ValidBothTypesOfStateTransactions(t *testing
 			// add register commitment state transaction
 			hash, err := sc.Hash()
 			require.NoError(t, err)
-			signature := createSignature(t, validators.getPrivateIdentities(aliases...), hash)
+			signature := createSignature(t, validators.getPrivateIdentities(aliases...), hash, bls.DomainStateReceiver)
 			sc.AggSignature = *signature
 		}
 
@@ -1116,7 +1116,7 @@ func TestFSM_VerifyStateTransaction_QuorumNotReached(t *testing.T) {
 
 	var txns []*types.Transaction
 
-	signature := createSignature(t, validators.getPrivateIdentities("A", "B"), hash)
+	signature := createSignature(t, validators.getPrivateIdentities("A", "B"), hash, bls.DomainStateReceiver)
 	commitmentMessageSigned.AggSignature = *signature
 
 	inputData, err := commitmentMessageSigned.EncodeAbi()
@@ -1144,9 +1144,9 @@ func TestFSM_VerifyStateTransaction_InvalidSignature(t *testing.T) {
 
 	var txns []*types.Transaction
 
-	signature := createSignature(t, validators.getPrivateIdentities("A", "B", "C", "D"), hash)
+	signature := createSignature(t, validators.getPrivateIdentities("A", "B", "C", "D"), hash, bls.DomainStateReceiver)
 	invalidValidator := newTestValidator(t, "G", 1)
-	invalidSignature, err := invalidValidator.mustSign([]byte("malicious message")).Marshal()
+	invalidSignature, err := invalidValidator.mustSign([]byte("malicious message"), bls.DomainStateReceiver).Marshal()
 	require.NoError(t, err)
 
 	commitmentMessageSigned.AggSignature = Signature{
@@ -1182,7 +1182,7 @@ func TestFSM_VerifyStateTransaction_TwoCommitmentMessages(t *testing.T) {
 
 	var txns []*types.Transaction
 
-	signature := createSignature(t, validators.getPrivateIdentities("A", "B", "C", "D"), hash)
+	signature := createSignature(t, validators.getPrivateIdentities("A", "B", "C", "D"), hash, bls.DomainStateReceiver)
 	commitmentMessageSigned.AggSignature = *signature
 
 	inputData, err := commitmentMessageSigned.EncodeAbi()
@@ -1311,7 +1311,7 @@ func createTestCommitment(t *testing.T, accounts []*wallet.Account) *CommitmentM
 	var signatures bls.Signatures
 
 	for _, a := range accounts {
-		signature, err := a.Bls.Sign(hash.Bytes(), bls.DomainValidatorSet)
+		signature, err := a.Bls.Sign(hash.Bytes(), bls.DomainStateReceiver)
 		assert.NoError(t, err)
 
 		signatures = append(signatures, signature)

consensus/polybft/hash_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/0xPolygon/polygon-edge/consensus/polybft/bitmap"
+	bls "github.com/0xPolygon/polygon-edge/consensus/polybft/signer"
 	"github.com/0xPolygon/polygon-edge/consensus/polybft/wallet"
 	"github.com/0xPolygon/polygon-edge/types"
 	"github.com/stretchr/testify/assert"
@@ -12,7 +13,7 @@ import (
 func Test_setupHeaderHashFunc(t *testing.T) {
 	extra := &Extra{
 		Validators: &ValidatorSetDelta{Removed: bitmap.Bitmap{1}},
-		Parent:     createSignature(t, []*wallet.Account{generateTestAccount(t)}, types.ZeroHash),
+		Parent:     createSignature(t, []*wallet.Account{generateTestAccount(t)}, types.ZeroHash, bls.DomainCheckpointManager),
 		Checkpoint: &CheckpointData{},
 		Seal:       []byte{},
 		Committed:  &Signature{},
@@ -28,7 +29,7 @@ func Test_setupHeaderHashFunc(t *testing.T) {
 	notFullExtraHash := types.HeaderHash(header)
 
 	extra.Seal = []byte{1, 2, 3, 255}
-	extra.Committed = createSignature(t, []*wallet.Account{generateTestAccount(t)}, types.ZeroHash)
+	extra.Committed = createSignature(t, []*wallet.Account{generateTestAccount(t)}, types.ZeroHash, bls.DomainCheckpointManager)
 	header.ExtraData = append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...)
 	fullExtraHash := types.HeaderHash(header)
 

consensus/polybft/helpers_test.go

@@ -21,7 +21,7 @@ import (
 func createTestKey(t *testing.T) *wallet.Key {
 	t.Helper()
 
-	return wallet.NewKey(generateTestAccount(t), bls.DomainCheckpointManager)
+	return wallet.NewKey(generateTestAccount(t))
 }
 
 func createRandomTestKeys(t *testing.T, numberOfKeys int) []*wallet.Key {
@@ -30,13 +30,13 @@ func createRandomTestKeys(t *testing.T, numberOfKeys int) []*wallet.Key {
 	result := make([]*wallet.Key, numberOfKeys, numberOfKeys)
 
 	for i := 0; i < numberOfKeys; i++ {
-		result[i] = wallet.NewKey(generateTestAccount(t), bls.DomainCheckpointManager)
+		result[i] = wallet.NewKey(generateTestAccount(t))
 	}
 
 	return result
 }
 
-func createSignature(t *testing.T, accounts []*wallet.Account, hash types.Hash) *Signature {
+func createSignature(t *testing.T, accounts []*wallet.Account, hash types.Hash, domain []byte) *Signature {
 	t.Helper()
 
 	var signatures bls.Signatures
@@ -45,7 +45,7 @@ func createSignature(t *testing.T, accounts []*wallet.Account, hash types.Hash)
 	for i, x := range accounts {
 		bmp.Set(uint64(i))
 
-		src, err := x.Bls.Sign(hash[:], bls.DomainCheckpointManager)
+		src, err := x.Bls.Sign(hash[:], domain)
 		require.NoError(t, err)
 
 		signatures = append(signatures, src)

consensus/polybft/mocks_test.go

@@ -416,7 +416,7 @@ func (v *testValidator) Address() types.Address {
 }
 
 func (v *testValidator) Key() *wallet.Key {
-	return wallet.NewKey(v.account, bls.DomainCheckpointManager)
+	return wallet.NewKey(v.account)
 }
 
 func (v *testValidator) paramsValidator() *Validator {
@@ -438,8 +438,8 @@ func (v *testValidator) ValidatorMetadata() *ValidatorMetadata {
 	}
 }
 
-func (v *testValidator) mustSign(hash []byte) *bls.Signature {
-	signature, err := v.account.Bls.Sign(hash, bls.DomainCheckpointManager)
+func (v *testValidator) mustSign(hash, domain []byte) *bls.Signature {
+	signature, err := v.account.Bls.Sign(hash, domain)
 	if err != nil {
 		panic(fmt.Sprintf("BUG: failed to sign: %v", err)) //nolint:gocritic
 	}

consensus/polybft/polybft.go

@@ -197,7 +197,7 @@ func (p *Polybft) Initialize() error {
 	}
 
 	// set key
-	p.key = wallet.NewKey(account, bls.DomainCheckpointManager)
+	p.key = wallet.NewKey(account)
 
 	// create and set syncer
 	p.syncer = syncer.NewSyncer(

consensus/polybft/polybft_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/0xPolygon/polygon-edge/consensus"
 	"github.com/0xPolygon/polygon-edge/consensus/ibft/signer"
+	bls "github.com/0xPolygon/polygon-edge/consensus/polybft/signer"
 	"github.com/0xPolygon/polygon-edge/consensus/polybft/wallet"
 	"github.com/0xPolygon/polygon-edge/helper/progress"
 	"github.com/0xPolygon/polygon-edge/txpool"
@@ -54,7 +55,7 @@ func TestPolybft_VerifyHeader(t *testing.T) {
 			checkpointHash, err := extra.Checkpoint.Hash(0, header.Number, header.Hash)
 			require.NoError(t, err)
 
-			extra.Committed = createSignature(t, committedAccounts, checkpointHash)
+			extra.Committed = createSignature(t, committedAccounts, checkpointHash, bls.DomainCheckpointManager)
 			header.ExtraData = append(make([]byte, signer.IstanbulExtraVanity), extra.MarshalRLPTo(nil)...)
 		}
 

consensus/polybft/sc_integration_test.go

@@ -125,7 +125,7 @@ func TestIntegratoin_PerformExit(t *testing.T) {
 	var signatures bls.Signatures
 
 	currentValidators.iterAcct(nil, func(v *testValidator) {
-		signatures = append(signatures, v.mustSign(checkpointHash[:]))
+		signatures = append(signatures, v.mustSign(checkpointHash[:], bls.DomainCheckpointManager))
 		bmp.Set(i)
 		i++
 	})

consensus/polybft/signer/common.go

@@ -26,6 +26,9 @@ var (
 
 	// domain used to map hash to G1 used by child checkpoint manager
 	DomainCheckpointManager = pcrypto.Keccak256([]byte("DOMAIN_CHECKPOINT_MANAGER"))
+
+	DomainCommonSigning = pcrypto.Keccak256([]byte("DOMAIN_COMMON_SIGNING"))
+	DomainStateReceiver = pcrypto.Keccak256([]byte("DOMAIN_STATE_RECEIVER"))
 )
 
 func mustG2Point(str string) *bn256.G2 {