Merge pull request #10469 from Roasbeef/lnwire-timestamp-validation

lnwire: enforce non-zero timestamp in gossip messages

discovery/gossiper.go

@@ -2502,6 +2502,22 @@ func (d *AuthenticatedGossiper) handleNodeAnnouncement(ctx context.Context,
 		"node=%x, source=%x", nMsg.peer, timestamp, nodeAnn.NodeID,
 		nMsg.source.SerializeCompressed())
 
+	// Although not explicitly required by BOLT 7 for node announcements
+	// (unlike channel updates), we still enforce non-zero timestamps as a
+	// sanity check. A timestamp of zero is likely indicative of a bug or
+	// uninitialized message.
+	if nodeAnn.Timestamp == 0 {
+		err := fmt.Errorf("rejecting node announcement with zero "+
+			"timestamp for node %x", nodeAnn.NodeID)
+
+		log.Warnf("Rejecting node announcement from peer=%v: %v",
+			nMsg.peer, err)
+
+		nMsg.err <- err
+
+		return nil, false
+	}
+
 	// We'll quickly ask the router if it already has a newer update for
 	// this node so we can skip validating signatures if not required.
 	if d.cfg.Graph.IsStaleNode(ctx, nodeAnn.NodeID, timestamp) {
@@ -3056,6 +3072,27 @@ func (d *AuthenticatedGossiper) handleChanUpdate(ctx context.Context,
 	// quickly reject it.
 	timestamp := time.Unix(int64(upd.Timestamp), 0)
 
+	// Per BOLT 7, the timestamp MUST be greater than 0.
+	if upd.Timestamp == 0 {
+		err := fmt.Errorf("rejecting channel update with zero "+
+			"timestamp for short_chan_id(%v)", shortChanID)
+
+		// Only increase ban score for remote peers.
+		if nMsg.isRemote {
+			log.Warnf("Increasing ban score for peer=%v: %v",
+				nMsg.peer, err)
+
+			dcErr := d.handleBadPeer(nMsg.peer)
+			if dcErr != nil {
+				err = dcErr
+			}
+		}
+
+		nMsg.err <- err
+
+		return nil, false
+	}
+
 	// Fetch the SCID we should be using to lock the channelMtx and make
 	// graph queries with.
 	graphScid, err := d.cfg.FindBaseByAlias(upd.ShortChannelID)

discovery/gossiper_test.go

@@ -2930,6 +2930,78 @@ func TestExtraDataNodeAnnouncementValidation(t *testing.T) {
 	require.NoError(t, err, "unable to process announcement")
 }
 
+// TestZeroTimestampNodeAnnouncementRejection tests that a NodeAnnouncement with
+// a zero timestamp is rejected per BOLT 7.
+func TestZeroTimestampNodeAnnouncementRejection(t *testing.T) {
+	t.Parallel()
+	ctx := t.Context()
+
+	tCtx, err := createTestCtx(t, 0, false)
+	require.NoError(t, err, "can't create context")
+
+	remotePeer := &mockPeer{
+		remoteKeyPriv1.PubKey(), nil, nil, atomic.Bool{},
+	}
+
+	// Create a node announcement with a zero timestamp.
+	nodeAnn, err := createNodeAnnouncement(remoteKeyPriv1, 0)
+	require.NoError(t, err, "can't create node announcement")
+
+	// Processing the announcement should fail with a zero timestamp error.
+	select {
+	case err = <-tCtx.gossiper.ProcessRemoteAnnouncement(
+		ctx, nodeAnn, remotePeer,
+	):
+	case <-time.After(2 * time.Second):
+		t.Fatal("did not process remote announcement")
+	}
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "zero timestamp")
+}
+
+// TestZeroTimestampChannelUpdateRejection tests that a ChannelUpdate with a
+// zero timestamp is rejected per BOLT 7.
+func TestZeroTimestampChannelUpdateRejection(t *testing.T) {
+	t.Parallel()
+	ctx := t.Context()
+
+	tCtx, err := createTestCtx(t, 0, false)
+	require.NoError(t, err, "can't create context")
+
+	remotePeer := &mockPeer{
+		remoteKeyPriv1.PubKey(), nil, nil, atomic.Bool{},
+	}
+
+	// First, we need to process a channel announcement so that the channel
+	// update has a valid channel to refer to.
+	chanAnn, err := tCtx.createRemoteChannelAnnouncement(0)
+	require.NoError(t, err, "unable to create chan ann")
+
+	select {
+	case err = <-tCtx.gossiper.ProcessRemoteAnnouncement(
+		ctx, chanAnn, remotePeer,
+	):
+	case <-time.After(2 * time.Second):
+		t.Fatal("did not process remote announcement")
+	}
+	require.NoError(t, err, "unable to process chan ann")
+
+	// Now create a channel update with a zero timestamp.
+	chanUpdAnn, err := createUpdateAnnouncement(0, 0, remoteKeyPriv1, 0)
+	require.NoError(t, err, "unable to create chan update")
+
+	// Processing the update should fail with a zero timestamp error.
+	select {
+	case err = <-tCtx.gossiper.ProcessRemoteAnnouncement(
+		ctx, chanUpdAnn, remotePeer,
+	):
+	case <-time.After(2 * time.Second):
+		t.Fatal("did not process remote announcement")
+	}
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "zero timestamp")
+}
+
 // assertBroadcast checks that num messages are being broadcasted from the
 // gossiper. The broadcasted messages are returned.
 func assertBroadcast(t *testing.T, ctx *testCtx, num int) []lnwire.Message {

docs/release-notes/release-notes-0.20.1.md

@@ -107,6 +107,12 @@
 # Technical and Architectural Updates
 ## BOLT Spec Updates
 
+* [Enforce non-zero timestamps](https://github.com/lightningnetwork/lnd/pull/10469)
+  for `channel_update` (as required by BOLT 7) and `node_announcement` messages.
+  Gossip messages with zero timestamps are now rejected. For `channel_update`
+  messages, remote peers sending such invalid messages will have their ban score
+  incremented.
+
 ## Testing
 
 ## Database