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

@@ -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()
 

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)
+	}
+}

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
 				}