diff --git a/beacon-chain/core/blocks/error.go b/beacon-chain/core/blocks/error.go index 7d968e7d5e..87218529c6 100644 --- a/beacon-chain/core/blocks/error.go +++ b/beacon-chain/core/blocks/error.go @@ -6,3 +6,4 @@ var errNilSignedWithdrawalMessage = errors.New("nil SignedBLSToExecutionChange m var errNilWithdrawalMessage = errors.New("nil BLSToExecutionChange message") var errInvalidBLSPrefix = errors.New("withdrawal credential prefix is not a BLS prefix") var errInvalidWithdrawalCredentials = errors.New("withdrawal credentials do not match") +var ErrInvalidSignature = errors.New("invalid signature") diff --git a/beacon-chain/core/blocks/signature.go b/beacon-chain/core/blocks/signature.go index 72a542a8db..1d8ec1dcc1 100644 --- a/beacon-chain/core/blocks/signature.go +++ b/beacon-chain/core/blocks/signature.go @@ -114,9 +114,12 @@ func VerifyBlockSignatureUsingCurrentFork(beaconState state.ReadOnlyBeaconState, } proposerPubKey := proposer.PublicKey sig := blk.Signature() - return signing.VerifyBlockSigningRoot(proposerPubKey, sig[:], domain, func() ([32]byte, error) { + if err := signing.VerifyBlockSigningRoot(proposerPubKey, sig[:], domain, func() ([32]byte, error) { return blkRoot, nil - }) + }); err != nil { + return ErrInvalidSignature + } + return nil } // BlockSignatureBatch retrieves the block signature batch from the provided block and its corresponding state. diff --git a/beacon-chain/core/blocks/signature_test.go b/beacon-chain/core/blocks/signature_test.go index 896e531cea..aa86bb8b3c 100644 --- a/beacon-chain/core/blocks/signature_test.go +++ b/beacon-chain/core/blocks/signature_test.go @@ -89,3 +89,36 @@ func TestVerifyBlockSignatureUsingCurrentFork(t *testing.T) { require.NoError(t, err) assert.NoError(t, blocks.VerifyBlockSignatureUsingCurrentFork(bState, wsb, blkRoot)) } + +func TestVerifyBlockSignatureUsingCurrentFork_InvalidSignature(t *testing.T) { + params.SetupTestConfigCleanup(t) + bCfg := params.BeaconConfig() + bCfg.AltairForkEpoch = 100 + bCfg.ForkVersionSchedule[bytesutil.ToBytes4(bCfg.AltairForkVersion)] = 100 + params.OverrideBeaconConfig(bCfg) + bState, keys := util.DeterministicGenesisState(t, 100) + altairBlk := util.NewBeaconBlockAltair() + altairBlk.Block.ProposerIndex = 0 + altairBlk.Block.Slot = params.BeaconConfig().SlotsPerEpoch * 100 + blkRoot, err := altairBlk.Block.HashTreeRoot() + assert.NoError(t, err) + + // Sign with wrong key (proposer index 0, but using key 1) + fData := ðpb.Fork{ + Epoch: 100, + CurrentVersion: params.BeaconConfig().AltairForkVersion, + PreviousVersion: params.BeaconConfig().GenesisForkVersion, + } + domain, err := signing.Domain(fData, 100, params.BeaconConfig().DomainBeaconProposer, bState.GenesisValidatorsRoot()) + assert.NoError(t, err) + rt, err := signing.ComputeSigningRoot(altairBlk.Block, domain) + assert.NoError(t, err) + wrongSig := keys[1].Sign(rt[:]).Marshal() + altairBlk.Signature = wrongSig + + wsb, err := consensusblocks.NewSignedBeaconBlock(altairBlk) + require.NoError(t, err) + + err = blocks.VerifyBlockSignatureUsingCurrentFork(bState, wsb, blkRoot) + require.ErrorIs(t, err, blocks.ErrInvalidSignature, "Expected ErrInvalidSignature for invalid signature") +} diff --git a/beacon-chain/sync/validate_beacon_blocks.go b/beacon-chain/sync/validate_beacon_blocks.go index 49d101cb17..8418ff9819 100644 --- a/beacon-chain/sync/validate_beacon_blocks.go +++ b/beacon-chain/sync/validate_beacon_blocks.go @@ -294,6 +294,9 @@ func (s *Service) validatePhase0Block(ctx context.Context, blk interfaces.ReadOn } if err := blocks.VerifyBlockSignatureUsingCurrentFork(parentState, blk, blockRoot); err != nil { + if errors.Is(err, blocks.ErrInvalidSignature) { + s.setBadBlock(ctx, blockRoot) + } return nil, err } // In the event the block is more than an epoch ahead from its diff --git a/beacon-chain/sync/validate_beacon_blocks_test.go b/beacon-chain/sync/validate_beacon_blocks_test.go index 6f5acb6915..a64aea417e 100644 --- a/beacon-chain/sync/validate_beacon_blocks_test.go +++ b/beacon-chain/sync/validate_beacon_blocks_test.go @@ -103,11 +103,84 @@ func TestValidateBeaconBlockPubSub_InvalidSignature(t *testing.T) { }, } res, err := r.validateBeaconBlockPubSub(ctx, "", m) - require.ErrorIs(t, err, signing.ErrSigFailedToVerify) + require.ErrorContains(t, "invalid signature", err) result := res == pubsub.ValidationReject assert.Equal(t, true, result) } +func TestValidateBeaconBlockPubSub_InvalidSignature_MarksBlockAsBad(t *testing.T) { + db := dbtest.SetupDB(t) + p := p2ptest.NewTestP2P(t) + ctx := t.Context() + beaconState, privKeys := util.DeterministicGenesisState(t, 100) + parentBlock := util.NewBeaconBlock() + util.SaveBlock(t, ctx, db, parentBlock) + bRoot, err := parentBlock.Block.HashTreeRoot() + require.NoError(t, err) + require.NoError(t, db.SaveState(ctx, beaconState, bRoot)) + require.NoError(t, db.SaveStateSummary(ctx, ðpb.StateSummary{Root: bRoot[:]})) + copied := beaconState.Copy() + require.NoError(t, copied.SetSlot(1)) + proposerIdx, err := helpers.BeaconProposerIndex(ctx, copied) + require.NoError(t, err) + msg := util.NewBeaconBlock() + msg.Block.ParentRoot = bRoot[:] + msg.Block.Slot = 1 + msg.Block.ProposerIndex = proposerIdx + badPrivKeyIdx := proposerIdx + 1 // We generate a valid signature from a wrong private key which fails to verify + msg.Signature, err = signing.ComputeDomainAndSign(beaconState, 0, msg.Block, params.BeaconConfig().DomainBeaconProposer, privKeys[badPrivKeyIdx]) + require.NoError(t, err) + + stateGen := stategen.New(db, doublylinkedtree.New()) + chainService := &mock.ChainService{Genesis: time.Unix(time.Now().Unix()-int64(params.BeaconConfig().SecondsPerSlot), 0), + FinalizedCheckPoint: ðpb.Checkpoint{ + Epoch: 0, + Root: make([]byte, 32), + }, + DB: db, + } + r := &Service{ + cfg: &config{ + beaconDB: db, + p2p: p, + initialSync: &mockSync.Sync{IsSyncing: false}, + chain: chainService, + clock: startup.NewClock(chainService.Genesis, chainService.ValidatorsRoot), + blockNotifier: chainService.BlockNotifier(), + stateGen: stateGen, + }, + seenBlockCache: lruwrpr.New(10), + badBlockCache: lruwrpr.New(10), + } + + blockRoot, err := msg.Block.HashTreeRoot() + require.NoError(t, err) + + // Verify block is not marked as bad initially + assert.Equal(t, false, r.hasBadBlock(blockRoot), "block should not be marked as bad initially") + + buf := new(bytes.Buffer) + _, err = p.Encoding().EncodeGossip(buf, msg) + require.NoError(t, err) + topic := p2p.GossipTypeMapping[reflect.TypeOf(msg)] + digest, err := r.currentForkDigest() + assert.NoError(t, err) + topic = r.addDigestToTopic(topic, digest) + m := &pubsub.Message{ + Message: &pubsubpb.Message{ + Data: buf.Bytes(), + Topic: &topic, + }, + } + res, err := r.validateBeaconBlockPubSub(ctx, "", m) + require.ErrorContains(t, "invalid signature", err) + result := res == pubsub.ValidationReject + assert.Equal(t, true, result) + + // Verify block is now marked as bad after invalid signature + assert.Equal(t, true, r.hasBadBlock(blockRoot), "block should be marked as bad after invalid signature") +} + func TestValidateBeaconBlockPubSub_BlockAlreadyPresentInDB(t *testing.T) { db := dbtest.SetupDB(t) ctx := t.Context() @@ -976,7 +1049,7 @@ func TestValidateBeaconBlockPubSub_InvalidParentBlock(t *testing.T) { }, } res, err := r.validateBeaconBlockPubSub(ctx, "", m) - require.ErrorContains(t, "could not unmarshal bytes into signature", err) + require.ErrorContains(t, "invalid signature", err) assert.Equal(t, res, pubsub.ValidationReject, "block with invalid signature should be rejected") require.NoError(t, copied.SetSlot(2)) diff --git a/changelog/potuz_invalid_sig.md b/changelog/potuz_invalid_sig.md new file mode 100644 index 0000000000..756ed7748f --- /dev/null +++ b/changelog/potuz_invalid_sig.md @@ -0,0 +1,3 @@ +### Fixed + +- Mark the block as invalid if it has an invalid signature.