From fb769c4306eb65932bfddb4af0974efa828a5e59 Mon Sep 17 00:00:00 2001
From: Neil Shen <overvenus@gmail.com>
Date: Thu, 7 Dec 2023 10:49:03 +0800
Subject: [PATCH] server: ignore raft messages if member id mismatch

Ignore Raft messages when the `To` field mismatches the local member ID.
In cases where incorrect Raft messages are dispatched, potentially due
to a malfunctioning switch, this proactive check prevents panics,
such as "tocommit is out of range".

Signed-off-by: Neil Shen <overvenus@gmail.com>
---
 server/etcdserver/server.go      |  8 +++++
 server/etcdserver/server_test.go | 60 ++++++++++++++++++++++++++++++--
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/server/etcdserver/server.go b/server/etcdserver/server.go
index f468400b2..55b197649 100644
--- a/server/etcdserver/server.go
+++ b/server/etcdserver/server.go
@@ -698,6 +698,14 @@ func (s *EtcdServer) Process(ctx context.Context, m raftpb.Message) error {
 		)
 		return httptypes.NewHTTPError(http.StatusForbidden, "cannot process message from removed member")
 	}
+	if s.MemberId() != types.ID(m.To) {
+		lg.Warn(
+			"rejected Raft message to mismatch member",
+			zap.String("local-member-id", s.MemberId().String()),
+			zap.String("mismatch-member-id", types.ID(m.To).String()),
+		)
+		return httptypes.NewHTTPError(http.StatusForbidden, "cannot process message to mismatch member")
+	}
 	if m.Type == raftpb.MsgApp {
 		s.stats.RecvAppendReq(types.ID(m.From).String(), m.Size())
 	}
diff --git a/server/etcdserver/server_test.go b/server/etcdserver/server_test.go
index 944307dfa..c3f56ee2a 100644
--- a/server/etcdserver/server_test.go
+++ b/server/etcdserver/server_test.go
@@ -470,7 +470,7 @@ func TestApplyConfigChangeUpdatesConsistIndex(t *testing.T) {
 		lgMu:         new(sync.RWMutex),
 		lg:           lg,
 		memberId:     1,
-		r:            *realisticRaftNode(lg),
+		r:            *realisticRaftNode(lg, nil),
 		cluster:      cl,
 		w:            wait.New(),
 		consistIndex: ci,
@@ -514,9 +514,15 @@ func TestApplyConfigChangeUpdatesConsistIndex(t *testing.T) {
 	assert.Equal(t, consistIndex, rindex)
 }
 
-func realisticRaftNode(lg *zap.Logger) *raftNode {
+func realisticRaftNode(lg *zap.Logger, snap *raftpb.Snapshot) *raftNode {
 	storage := raft.NewMemoryStorage()
 	storage.SetHardState(raftpb.HardState{Commit: 0, Term: 0})
+	if snap != nil {
+		err := storage.ApplySnapshot(*snap)
+		if err != nil {
+			panic(err)
+		}
+	}
 	c := &raft.Config{
 		ID:              1,
 		ElectionTick:    10,
@@ -889,6 +895,56 @@ func TestAddMember(t *testing.T) {
 	}
 }
 
+// TestProcessIgnoreMismatchMessage tests Process must ignore messages to
+// mismatch member.
+func TestProcessIgnoreMismatchMessage(t *testing.T) {
+	lg := zaptest.NewLogger(t)
+	cl := newTestCluster(t)
+	st := v2store.New()
+	cl.SetStore(st)
+	be, _ := betesting.NewDefaultTmpBackend(t)
+	defer betesting.Close(t, be)
+	cl.SetBackend(schema.NewMembershipBackend(lg, be))
+
+	// Bootstrap a 3-node cluster
+	cl.AddMember(&membership.Member{ID: types.ID(1)}, true)
+	cl.AddMember(&membership.Member{ID: types.ID(2)}, true)
+	cl.AddMember(&membership.Member{ID: types.ID(3)}, true)
+	r := realisticRaftNode(lg, &raftpb.Snapshot{
+		Metadata: raftpb.SnapshotMetadata{
+			Index:     11, // magic number
+			Term:      11, // magic number
+			ConfState: raftpb.ConfState{Voters: []uint64{1, 2, 3}},
+		},
+	})
+	s := &EtcdServer{
+		lgMu:         new(sync.RWMutex),
+		lg:           lg,
+		r:            *r,
+		v2store:      st,
+		cluster:      cl,
+		reqIDGen:     idutil.NewGenerator(0, time.Time{}),
+		SyncTicker:   &time.Ticker{},
+		consistIndex: cindex.NewFakeConsistentIndex(0),
+		beHooks:      serverstorage.NewBackendHooks(lg, nil),
+	}
+	// Mock a mad switch dispatching messages to wrong node.
+	m := raftpb.Message{
+		Type:   raftpb.MsgHeartbeat,
+		To:     2, // Wrong ID.
+		From:   3,
+		Term:   11,
+		Commit: 42, // Commit is larger than the last index.
+	}
+	if types.ID(m.To) == s.MemberId() {
+		t.Fatalf("To must mismatch")
+	}
+	err := s.Process(context.Background(), m)
+	if err == nil {
+		t.Fatalf("Must ignore the message and return an error")
+	}
+}
+
 // TestRemoveMember tests RemoveMember can propose and perform node removal.
 func TestRemoveMember(t *testing.T) {
 	lg := zaptest.NewLogger(t)