From ea75e248fc490aac4322df9ad2b38103959c01b2 Mon Sep 17 00:00:00 2001 From: Dmitry Shulyak Date: Mon, 11 Sep 2023 08:50:02 +0000 Subject: [PATCH] proposal signed data was used in id (#4992) > {"L":"ERROR","T":"2023-09-08T15:46:18.944Z","N":"9aa91.blockGenerator","M":"failed to process hare output","node_id":"9aa91374168177589adb11c04a9984f33684e3e3cee72b4298c4d5d56d03dcdb","module":"blockGenerator","layer_id":12,"errmsg":"preprocess fetch layer 12 proposals: hint: proposalDB, hash: 0x1d286d884f4a8699182bde631972d1ad649549aa000000000000000000000000, err: validation reject: incorrect hash: proposal want 1d286d884f, got c2986cf657\nhint: proposalDB, hash: 0xab3ef72044915bb64ef3c7eacfdd98f923a72195000000000000000000000000, err: validation reject: incorrect hash: proposal want ab3ef72044, got 9eafb3985e\nhint: proposalDB, hash: 0x5a0cbb754637b44d5df63ca6938be1fad001ac4b000000000000000000000000, err: validation reject: incorrect hash: proposal want 5a0cbb7546, got ab4994459c","name":"blockGenerator"} --- proposals/handler.go | 6 ++- proposals/handler_test.go | 101 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/proposals/handler.go b/proposals/handler.go index 0255d268d6..bbd7184099 100644 --- a/proposals/handler.go +++ b/proposals/handler.go @@ -272,25 +272,27 @@ func (h *Handler) handleProposal(ctx context.Context, expHash types.Hash32, peer if p.Layer >= h.cfg.AllowEmptyActiveSet { p.ActiveSet = nil } + if !h.edVerifier.Verify(signing.PROPOSAL, p.SmesherID, p.SignedBytes(), p.Signature) { badSigProposal.Inc() return fmt.Errorf("failed to verify proposal signature") } - p.ActiveSet = set if !h.edVerifier.Verify(signing.BALLOT, p.Ballot.SmesherID, p.Ballot.SignedBytes(), p.Ballot.Signature) { badSigBallot.Inc() return fmt.Errorf("failed to verify ballot signature") } // set the proposal ID when received + // It mustn't contain the active set if layer >= AllowEmptyActiveSet + // (p.Initialize uses SignedBytes again). if err := p.Initialize(); err != nil { failedInit.Inc() return errInitialize } - if expHash != (types.Hash32{}) && p.ID().AsHash32() != expHash { return fmt.Errorf("%w: proposal want %s, got %s", errWrongHash, expHash.ShortString(), p.ID().AsHash32().ShortString()) } + p.ActiveSet = set if p.AtxID == types.EmptyATXID || p.AtxID == h.cfg.GoldenATXID { badData.Inc() diff --git a/proposals/handler_test.go b/proposals/handler_test.go index 4d28aae83a..3919513f2f 100644 --- a/proposals/handler_test.go +++ b/proposals/handler_test.go @@ -1398,3 +1398,104 @@ func TestHandleActiveSet(t *testing.T) { }) } } + +func gproposal(signer *signing.EdSigner, atxid types.ATXID, activeset []types.ATXID, + layer types.LayerID, edata *types.EpochData, +) types.Proposal { + p := types.Proposal{} + p.Layer = layer + p.AtxID = atxid + p.ActiveSet = activeset + p.EpochData = edata + p.Ballot.Signature = signer.Sign(signing.BALLOT, p.Ballot.SignedBytes()) + p.Ballot.SmesherID = signer.NodeID() + p.Signature = signer.Sign(signing.PROPOSAL, p.SignedBytes()) + p.SmesherID = signer.NodeID() + return p +} + +func TestHandleSyncedProposalActiveSet(t *testing.T) { + signer, err := signing.NewEdSigner() + require.NoError(t, err) + + set := []types.ATXID{{1}, {2}} + const acceptEmpty = 20 + good := gproposal(signer, types.ATXID{1}, set, acceptEmpty-1, &types.EpochData{ + ActiveSetHash: types.ATXIDList(set).Hash(), + Beacon: types.Beacon{1}, + }) + require.NoError(t, good.Initialize()) + + woActive := good + woActive.ActiveSet = nil + + notSigned := gproposal(signer, types.ATXID{1}, nil, acceptEmpty, &types.EpochData{ + ActiveSetHash: types.ATXIDList(set).Hash(), + Beacon: types.Beacon{1}, + }) + require.NoError(t, notSigned.Initialize()) + notSigned.ActiveSet = set + + notSignedEmpty := notSigned + notSignedEmpty.ActiveSet = nil + for _, tc := range []struct { + desc string + data []byte + id types.Hash32 + requestSet []types.ATXID + err string + }{ + { + desc: "before empty signed active set", + data: codec.MustEncode(&good), + id: good.ID().AsHash32(), + }, + { + desc: "before empty without active set", + data: codec.MustEncode(&woActive), + id: woActive.ID().AsHash32(), + err: "failed to verify proposal signature", + }, + { + desc: "after empty not signed active set", + data: codec.MustEncode(¬Signed), + id: notSigned.ID().AsHash32(), + }, + { + desc: "after empty not signed active set empty", + data: codec.MustEncode(¬SignedEmpty), + id: notSigned.ID().AsHash32(), + requestSet: set, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + th := createTestHandler(t) + th.cfg.AllowEmptyActiveSet = acceptEmpty + pid := p2p.Peer("any") + + th.mclock.EXPECT().LayerToTime(gomock.Any()) + th.mf.EXPECT().RegisterPeerHashes(pid, gomock.Any()).AnyTimes() + th.md.EXPECT().GetMissingActiveSet(gomock.Any(), gomock.Any()).AnyTimes() + if tc.requestSet != nil { + id := types.ATXIDList(tc.requestSet).Hash() + th.mf.EXPECT().GetActiveSet(gomock.Any(), id) + require.NoError(t, activesets.Add(th.cdb, id, &types.EpochActiveSet{ + Set: tc.requestSet, + })) + } + th.mf.EXPECT().GetAtxs(gomock.Any(), gomock.Any()).AnyTimes() + th.mf.EXPECT().GetBallots(gomock.Any(), gomock.Any()).AnyTimes() + th.mockSet.decodeAnyBallots() + th.mv.EXPECT().CheckEligibility(gomock.Any(), gomock.Any()).AnyTimes().Return(true, nil) + th.mm.EXPECT().AddBallot(gomock.Any(), gomock.Any()).AnyTimes() + th.mm.EXPECT().AddTXsFromProposal(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + + err := th.HandleSyncedProposal(context.Background(), tc.id, pid, tc.data) + if len(tc.err) > 0 { + require.ErrorContains(t, err, tc.err) + } else { + require.NoError(t, err) + } + }) + } +}