Skip to content

Commit

Permalink
Check identity exists when validating malfeasance proofs (#5930)
Browse files Browse the repository at this point in the history
* Check if identity exists when validating incoming malfeasance proofs
  • Loading branch information
fasmat committed May 13, 2024
1 parent 0d2e5c3 commit e81e03d
Show file tree
Hide file tree
Showing 4 changed files with 470 additions and 206 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ See [RELEASE](./RELEASE.md) for workflow instructions.
* [#5929](https://github.com/spacemeshos/go-spacemesh/pull/5929) Fix "no nonce" error when persisting malicious
(initial) ATXs.

* [#5930](https://github.com/spacemeshos/go-spacemesh/pull/5930) Check if identity for a given malfeasance proof
exists when validating it.

## Release v1.5.2-hotfix1

This release includes our first CVE fix. A vulnerability was found in the way a node handles incoming ATXs. We urge all
Expand Down
40 changes: 27 additions & 13 deletions activation/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1398,24 +1398,40 @@ func testHandler_PostMalfeasanceProofs(t *testing.T, synced bool) {
require.NoError(t, err)
nodeID := sig.NodeID()

proof, err := identities.GetMalfeasanceProof(atxHdlr.cdb, nodeID)
require.ErrorIs(t, err, sql.ErrNotFound)
require.Nil(t, proof)

ch := types.NIPostChallenge{
initialCh := types.NIPostChallenge{
Sequence: 0,
PrevATXID: types.EmptyATXID,
PublishEpoch: 1,
PositioningATX: goldenATXID,
CommitmentATX: &goldenATXID,
InitialPost: &types.Post{},
}
initialNipost := newNIPostWithPoet(t, []byte{0x76, 0x45})

initialAtx := newAtx(initialCh, initialNipost.NIPost, 100, types.GenerateAddress([]byte("aaaa")))
initialAtx.NodeID = &nodeID
vrfNonce := types.VRFPostIndex(0)
initialAtx.VRFNonce = &vrfNonce
initialAtx.SetEffectiveNumUnits(100)
initialAtx.SetReceived(time.Now())
require.NoError(t, SignAndFinalizeAtx(sig, initialAtx))
vAtx, err := initialAtx.Verify(0, 100)
require.NoError(t, err)
require.NoError(t, atxs.Add(atxHdlr.cdb, vAtx))

proof, err := identities.GetMalfeasanceProof(atxHdlr.cdb, nodeID)
require.ErrorIs(t, err, sql.ErrNotFound)
require.Nil(t, proof)

ch := types.NIPostChallenge{
Sequence: 1,
PrevATXID: initialAtx.ID(),
PublishEpoch: 3,
PositioningATX: initialAtx.ID(),
}
nipost := newNIPostWithPoet(t, []byte{0x76, 0x45})

atx := newAtx(ch, nipost.NIPost, 100, types.GenerateAddress([]byte("aaaa")))
atx.NodeID = &nodeID
vrfNonce := types.VRFPostIndex(0)
atx.VRFNonce = &vrfNonce
atx.SetEffectiveNumUnits(100)
atx.SetReceived(time.Now())
require.NoError(t, SignAndFinalizeAtx(sig, atx))
Expand All @@ -1424,13 +1440,11 @@ func testHandler_PostMalfeasanceProofs(t *testing.T, synced bool) {

var got mwire.MalfeasanceGossip
atxHdlr.mclock.EXPECT().CurrentLayer().Return(atx.PublishEpoch.FirstLayer())
atxHdlr.mValidator.EXPECT().VRFNonce(gomock.Any(), goldenATXID, gomock.Any(), gomock.Any(), gomock.Any())
atxHdlr.mValidator.EXPECT().
Post(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
atxHdlr.mockFetch.EXPECT().RegisterPeerHashes(gomock.Any(), gomock.Any())
atxHdlr.mockFetch.EXPECT().GetPoetProof(gomock.Any(), atx.GetPoetProofRef())
atxHdlr.mValidator.EXPECT().InitialNIPostChallenge(&atx.NIPostChallenge, gomock.Any(), goldenATXID)
atxHdlr.mValidator.EXPECT().PositioningAtx(goldenATXID, gomock.Any(), goldenATXID, atx.PublishEpoch)
atxHdlr.mockFetch.EXPECT().GetAtxs(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
atxHdlr.mValidator.EXPECT().NIPostChallenge(&atx.NIPostChallenge, gomock.Any(), sig.NodeID())
atxHdlr.mValidator.EXPECT().PositioningAtx(initialAtx.ID(), gomock.Any(), goldenATXID, atx.PublishEpoch)
atxHdlr.mValidator.EXPECT().
NIPost(gomock.Any(), gomock.Any(), goldenATXID, atx.NIPost, gomock.Any(), atx.NumUnits, gomock.Any()).
Return(0, &verifying.ErrInvalidIndex{Index: 2})
Expand Down
21 changes: 17 additions & 4 deletions malfeasance/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func updateMetrics(tp wire.Proof) {
}
}

func checkIdentityExists(db sql.Executor, nodeID types.NodeID) error {
func hasPublishedAtxs(db sql.Executor, nodeID types.NodeID) error {
_, err := atxs.GetLastIDByNodeID(db, nodeID)
if err != nil {
if errors.Is(err, sql.ErrNotFound) {
Expand Down Expand Up @@ -257,7 +257,7 @@ func validateHareEquivocation(
return types.EmptyNodeID, errors.New("invalid signature")
}
if firstNid == types.EmptyNodeID {
if err := checkIdentityExists(db, msg.SmesherID); err != nil {
if err := hasPublishedAtxs(db, msg.SmesherID); err != nil {
return types.EmptyNodeID, fmt.Errorf("check identity in hare malfeasance %v: %w", msg.SmesherID, err)
}
firstNid = msg.SmesherID
Expand Down Expand Up @@ -308,7 +308,7 @@ func validateMultipleATXs(
return types.EmptyNodeID, errors.New("invalid signature")
}
if firstNid == types.EmptyNodeID {
if err := checkIdentityExists(db, msg.SmesherID); err != nil {
if err := hasPublishedAtxs(db, msg.SmesherID); err != nil {
return types.EmptyNodeID, fmt.Errorf("check identity in atx malfeasance %v: %w", msg.SmesherID, err)
}
firstNid = msg.SmesherID
Expand Down Expand Up @@ -359,7 +359,7 @@ func validateMultipleBallots(
return types.EmptyNodeID, errors.New("invalid signature")
}
if firstNid == types.EmptyNodeID {
if err = checkIdentityExists(db, msg.SmesherID); err != nil {
if err = hasPublishedAtxs(db, msg.SmesherID); err != nil {
return types.EmptyNodeID, fmt.Errorf("check identity in ballot malfeasance %v: %w", msg.SmesherID, err)
}
firstNid = msg.SmesherID
Expand Down Expand Up @@ -391,6 +391,10 @@ func validateInvalidPostIndex(
proof *wire.InvalidPostIndexProof,
) (types.NodeID, error) {
atx := &proof.Atx
if err := hasPublishedAtxs(db, atx.SmesherID); err != nil {
return types.EmptyNodeID, fmt.Errorf("check identity %v in invalid post index: %w", atx.SmesherID, err)
}

if !edVerifier.Verify(signing.ATX, atx.SmesherID, atx.SignedBytes(), atx.Signature) {
return types.EmptyNodeID, errors.New("invalid signature")
}
Expand Down Expand Up @@ -429,11 +433,20 @@ func validateInvalidPrevATX(
proof *wire.InvalidPrevATXProof,
) (types.NodeID, error) {
atx1 := proof.Atx1
if err := hasPublishedAtxs(db, atx1.SmesherID); err != nil {
return types.EmptyNodeID, fmt.Errorf("check identity %v in invalid previous ATX: %w", atx1.SmesherID, err)
}

if !edVerifier.Verify(signing.ATX, atx1.SmesherID, atx1.SignedBytes(), atx1.Signature) {
return types.EmptyNodeID, errors.New("atx1: invalid signature")
}

atx2 := proof.Atx2
if atx1.SmesherID != atx2.SmesherID {
numInvalidProofsPrevATX.Inc()
return types.EmptyNodeID, errors.New("invalid old prev ATX malfeasance proof: smesher IDs are different")
}

if !edVerifier.Verify(signing.ATX, atx2.SmesherID, atx2.SignedBytes(), atx2.Signature) {
return types.EmptyNodeID, errors.New("atx2: invalid signature")
}
Expand Down
Loading

0 comments on commit e81e03d

Please sign in to comment.