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 }