dkg: improved nodesigs sender id check

dkg/nodesigs.go

@@ -126,15 +126,11 @@ func (n *nodeSigBcast) broadcastCallback(ctx context.Context, senderID peer.ID,
 
 	msgPeerIdx := int(nodeSig.GetPeerIndex())
 
-	sig := nodeSig.GetSignature()
-	if bytes.Equal(sig, noneData) {
-		// For certain protocols we allow exchanging nil signatures.
-		n.setSig(noneData, msgPeerIdx)
-
-		return nil
+	if msgPeerIdx < 0 || msgPeerIdx >= len(n.peers) {
+		return errors.New("invalid peer index")
 	}
 
-	if (msgPeerIdx == n.nodeIdx.PeerIdx) || (msgPeerIdx < 0 || msgPeerIdx >= len(n.peers)) {
+	if msgPeerIdx == n.nodeIdx.PeerIdx {
 		return errors.New("invalid peer index")
 	}
 
@@ -143,6 +139,14 @@ func (n *nodeSigBcast) broadcastCallback(ctx context.Context, senderID peer.ID,
 		return errors.New("sender peer ID does not match claimed peer index")
 	}
 
+	sig := nodeSig.GetSignature()
+	if bytes.Equal(sig, noneData) {
+		// For certain protocols we allow exchanging nil signatures.
+		n.setSig(noneData, msgPeerIdx)
+
+		return nil
+	}
+
 	lockHash, err := n.lockHash(ctx)
 	if err != nil {
 		return errors.Wrap(err, "lock hash wait")

dkg/nodesigs_internal_test.go

@@ -245,6 +245,22 @@ func TestSigsCallbacks(t *testing.T) {
 		require.ErrorContains(t, err, "sender peer ID does not match claimed peer index")
 	})
 
+	t.Run("sender ID checked before noneData", func(t *testing.T) {
+		msg := &dkgpb.MsgNodeSig{
+			Signature: noneData,
+			PeerIndex: uint32(2), // Claims to be from peer 2
+		}
+
+		// But actually sent by peer 1 - should fail even with noneData signature
+		err := ns.broadcastCallback(context.Background(),
+			peers[1],
+			"",
+			msg,
+		)
+
+		require.ErrorContains(t, err, "sender peer ID does not match claimed peer index")
+	})
+
 	t.Run("signature verification failed", func(t *testing.T) {
 		ns.lockHashData = bytes.Repeat([]byte{42}, 32)