Skip to content

Commit

Permalink
server: ignore raft messages if member id mismatch
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
overvenus committed Dec 7, 2023
1 parent 4ece556 commit fb769c4
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 2 deletions.
8 changes: 8 additions & 0 deletions server/etcdserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
60 changes: 58 additions & 2 deletions server/etcdserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit fb769c4

Please sign in to comment.