From e9cde3e4b15fcc7aa98189ec8461596ff51539a4 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sat, 8 Aug 2026 09:14:57 -0700 Subject: [PATCH] master: gate raft membership RPCs behind the admin whitelist (#10649) * master: evict a dead peer via the local raft handle OnPeerUpdate only runs on the leader, and the AddVoter branch right above mutates the local raft directly. The remove branch instead dialed our own RaftRemoveServer back over gRPC. Drop the self-dial and remove the peer through the local handle, matching the add path. This also leaves operator tooling as the only caller of the RaftRemoveServer RPC. * master: require whitelist auth for raft membership RPCs RaftAddServer, RaftRemoveServer and RaftLeadershipTransfer rewrite raft quorum but had no caller check beyond "am I the leader". Any client that could reach the master gRPC port could add an unreachable phantom voter and stall the write path. Gate the three on the admin whitelist, mirroring the volume server's checkGrpcAdminAuth. With no whitelist configured the guard allows every caller, so default and single-master deployments are unaffected; operators who set -whiteList get these RPCs locked down to it. The leader's own dead-peer eviction no longer dials these RPCs, so the only remaining callers are operator tooling. --- weed/server/master_grpc_server_raft.go | 49 +++++++++++ .../master_grpc_server_raft_auth_test.go | 83 +++++++++++++++++++ weed/server/master_server.go | 11 +-- 3 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 weed/server/master_grpc_server_raft_auth_test.go diff --git a/weed/server/master_grpc_server_raft.go b/weed/server/master_grpc_server_raft.go index 1c0abea51..f4bf85988 100644 --- a/weed/server/master_grpc_server_raft.go +++ b/weed/server/master_grpc_server_raft.go @@ -3,13 +3,50 @@ package weed_server import ( "context" "fmt" + "net" "github.com/hashicorp/raft" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/peer" + "google.golang.org/grpc/status" "github.com/seaweedfs/seaweedfs/weed/cluster" + "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb/master_pb" ) +// checkGrpcAdminAuth authorizes the raft membership RPCs that mutate cluster +// quorum. It mirrors the volume server's gate: the caller's peer IP is matched +// against the master's -whiteList. With no whitelist configured IsWhiteListed +// allows everyone, so default and single-master deployments are unaffected; +// operators who set a whitelist get these RPCs locked down to it. The cluster's +// own dead-peer eviction no longer dials these RPCs (it uses the local raft +// handle), so the only remaining callers are operator tooling. +func (ms *MasterServer) checkGrpcAdminAuth(ctx context.Context) error { + if ms.guard == nil { + return nil + } + pr, ok := peer.FromContext(ctx) + if !ok { + glog.V(0).Infof("gRPC raft admin auth failed: no peer info") + return status.Error(codes.PermissionDenied, "no peer info") + } + addr := pr.Addr.String() + var host string + if tcpAddr, ok := pr.Addr.(*net.TCPAddr); ok { + host = tcpAddr.IP.String() + } else if h, _, splitErr := net.SplitHostPort(addr); splitErr == nil { + host = h + } else { + host = addr + } + if !ms.guard.IsWhiteListed(host) { + glog.V(0).Infof("gRPC raft admin auth failed: %s is not whitelisted (remote: %s)", host, addr) + return status.Errorf(codes.PermissionDenied, "not authorized: %s", host) + } + return nil +} + func (ms *MasterServer) RaftListClusterServers(ctx context.Context, req *master_pb.RaftListClusterServersRequest) (*master_pb.RaftListClusterServersResponse, error) { resp := &master_pb.RaftListClusterServersResponse{} @@ -62,6 +99,10 @@ func (ms *MasterServer) RaftListClusterServers(ctx context.Context, req *master_ func (ms *MasterServer) RaftAddServer(ctx context.Context, req *master_pb.RaftAddServerRequest) (*master_pb.RaftAddServerResponse, error) { resp := &master_pb.RaftAddServerResponse{} + if err := ms.checkGrpcAdminAuth(ctx); err != nil { + return resp, err + } + ms.Topo.RaftServerAccessLock.RLock() defer ms.Topo.RaftServerAccessLock.RUnlock() @@ -89,6 +130,10 @@ func (ms *MasterServer) RaftAddServer(ctx context.Context, req *master_pb.RaftAd func (ms *MasterServer) RaftRemoveServer(ctx context.Context, req *master_pb.RaftRemoveServerRequest) (*master_pb.RaftRemoveServerResponse, error) { resp := &master_pb.RaftRemoveServerResponse{} + if err := ms.checkGrpcAdminAuth(ctx); err != nil { + return resp, err + } + ms.Topo.RaftServerAccessLock.RLock() defer ms.Topo.RaftServerAccessLock.RUnlock() @@ -119,6 +164,10 @@ func (ms *MasterServer) RaftRemoveServer(ctx context.Context, req *master_pb.Raf func (ms *MasterServer) RaftLeadershipTransfer(ctx context.Context, req *master_pb.RaftLeadershipTransferRequest) (*master_pb.RaftLeadershipTransferResponse, error) { resp := &master_pb.RaftLeadershipTransferResponse{} + if err := ms.checkGrpcAdminAuth(ctx); err != nil { + return resp, err + } + ms.Topo.RaftServerAccessLock.RLock() defer ms.Topo.RaftServerAccessLock.RUnlock() diff --git a/weed/server/master_grpc_server_raft_auth_test.go b/weed/server/master_grpc_server_raft_auth_test.go new file mode 100644 index 000000000..3cf511c08 --- /dev/null +++ b/weed/server/master_grpc_server_raft_auth_test.go @@ -0,0 +1,83 @@ +package weed_server + +import ( + "context" + "net" + "testing" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/peer" + "google.golang.org/grpc/status" + + "github.com/seaweedfs/seaweedfs/weed/pb/master_pb" + "github.com/seaweedfs/seaweedfs/weed/security" + "github.com/seaweedfs/seaweedfs/weed/topology" +) + +// ctxFromHost builds a gRPC context whose peer IP is host, so the whitelist gate +// sees a concrete caller instead of the in-process "@" address tests default to. +func ctxFromHost(host string) context.Context { + return peer.NewContext(context.Background(), &peer.Peer{ + Addr: &net.TCPAddr{IP: net.ParseIP(host), Port: 40000}, + }) +} + +// TestRaftMembershipRPCs_RejectUnwhitelisted verifies the mutating raft +// membership RPCs deny a caller that isn't in a configured whitelist. Before the +// gate they accepted any caller and could rewrite raft quorum. +func TestRaftMembershipRPCs_RejectUnwhitelisted(t *testing.T) { + ms := &MasterServer{ + Topo: topology.NewTopology("test", nil, 0, 0, false), + guard: security.NewGuard([]string{"127.0.0.1"}, "", 0, "", 0), + } + ctx := ctxFromHost("10.13.37.66") + + assertDenied := func(name string, err error) { + t.Helper() + if status.Code(err) != codes.PermissionDenied { + t.Errorf("%s: expected PermissionDenied, got %v", name, err) + } + } + + _, err := ms.RaftAddServer(ctx, &master_pb.RaftAddServerRequest{Id: "evil", Address: "10.13.37.66:19333", Voter: true}) + assertDenied("RaftAddServer", err) + + _, err = ms.RaftRemoveServer(ctx, &master_pb.RaftRemoveServerRequest{Id: "127.0.0.1:9333"}) + assertDenied("RaftRemoveServer", err) + + _, err = ms.RaftLeadershipTransfer(ctx, &master_pb.RaftLeadershipTransferRequest{}) + assertDenied("RaftLeadershipTransfer", err) +} + +// TestRaftMembershipRPCs_AllowWhitelisted confirms a whitelisted caller passes +// the gate and reaches the handler body (which no-ops here because raft is not +// initialized in single-master test mode). +func TestRaftMembershipRPCs_AllowWhitelisted(t *testing.T) { + ms := &MasterServer{ + Topo: topology.NewTopology("test", nil, 0, 0, false), + guard: security.NewGuard([]string{"10.13.37.66"}, "", 0, "", 0), + } + ctx := ctxFromHost("10.13.37.66") + + if _, err := ms.RaftAddServer(ctx, &master_pb.RaftAddServerRequest{Id: "peer", Address: "10.13.37.66:19333", Voter: true}); err != nil { + t.Errorf("RaftAddServer: whitelisted caller unexpectedly rejected: %v", err) + } + if _, err := ms.RaftRemoveServer(ctx, &master_pb.RaftRemoveServerRequest{Id: "peer"}); err != nil { + t.Errorf("RaftRemoveServer: whitelisted caller unexpectedly rejected: %v", err) + } +} + +// TestRaftMembershipRPCs_NoWhitelistFailsOpen documents that with no whitelist +// configured the gate allows every caller, matching the volume server's admin +// gate and keeping default / single-master deployments working. +func TestRaftMembershipRPCs_NoWhitelistFailsOpen(t *testing.T) { + ms := &MasterServer{ + Topo: topology.NewTopology("test", nil, 0, 0, false), + guard: security.NewGuard(nil, "", 0, "", 0), + } + ctx := ctxFromHost("10.13.37.66") + + if _, err := ms.RaftAddServer(ctx, &master_pb.RaftAddServerRequest{Id: "peer", Address: "10.13.37.66:19333", Voter: true}); err != nil { + t.Errorf("RaftAddServer: empty whitelist should fail open, got %v", err) + } +} diff --git a/weed/server/master_server.go b/weed/server/master_server.go index f52737a86..4677c69da 100644 --- a/weed/server/master_server.go +++ b/weed/server/master_server.go @@ -531,13 +531,10 @@ func (ms *MasterServer) OnPeerUpdate(update *master_pb.ClusterNodeUpdate, startF defer cancel() if _, err := client.Ping(ctx, &master_pb.PingRequest{Target: string(peerAddress), TargetType: cluster.MasterType}); err != nil { glog.V(0).Infof("master %s didn't respond to pings. remove raft server", peerName) - if err := ms.MasterClient.WithClient(false, func(client master_pb.SeaweedClient) error { - _, err := client.RaftRemoveServer(context.Background(), &master_pb.RaftRemoveServerRequest{ - Id: peerName, - Force: false, - }) - return err - }); err != nil { + // We are the leader here, so drop the dead peer through the local + // raft handle, mirroring the AddVoter branch above, instead of + // dialing our own RaftRemoveServer RPC. + if err := ms.Topo.HashicorpRaft.RemoveServer(hashicorpRaft.ServerID(peerName), 0, 0).Error(); err != nil { glog.Warningf("failed removing old raft server: %v", err) return err }