volume: fail closed in admin gRPC gate when no whitelist is configured (#9440)

Add Guard.IsAdminAuthorized, a fail-closed variant of IsWhiteListed, and use
it to gate destructive volume admin RPCs. IsWhiteListed keeps its
allow-all-when-empty semantics for HTTP compatibility.

For TCP peers with an empty whitelist, off-host callers are rejected but
loopback (127.0.0.0/8, ::1) is still trusted. A volume server commonly
cohabits with the master/filer on a single host and in integration-test
clusters; the loopback exception keeps cluster-internal admin traffic
working without -whiteList while still locking out off-host attackers.

Non-TCP peers (in-process / bufconn / unix-socket) bypass the host check
entirely. When `weed server` runs master+volume+filer in a single process
the master dials the volume server in-process and the peer address surfaces
as "@", which has no parseable IP. Such a caller shares our OS process and
cannot be spoofed by a remote attacker, so we treat it as trusted by
construction.

The gate also tolerates a nil guard (developmental / embedded path) and only
enforces once a guard is wired up. UpdateWhiteList skips entries whose CIDR
fails to parse so the IP-iteration path can no longer hit a nil *net.IPNet.

weed/command/scaffold/security.toml

@@ -198,5 +198,11 @@ kek = ""
 key = ""
 
 # white list. It's checking request ip address.
+# Destructive volume admin gRPCs (DeleteCollection, VolumeDelete,
+# VolumeMarkReadonly, VolumeMarkWritable, VolumeUnmount, VolumeMount,
+# VolumeServerLeave, AllocateVolume, VolumeConfigure, EC/vacuum) require a
+# populated white_list. If left empty, the volume server rejects those RPCs
+# from off-host peers (loopback callers are still trusted); either list the
+# admin IPs/CIDRs here or pass them via the -whiteList CLI flag.
 [guard]
 white_list = ""

weed/security/guard.go

@@ -36,6 +36,13 @@ Generating JWT:
  2. optionally set "exp", "nbf" fields, in Unix time,
     the number of seconds elapsed since January 1, 1970 UTC.
 
+There are two whitelist check entry points with different defaults:
+  - IsWhiteListed: allow-all when the whitelist is empty. Kept for HTTP
+    compatibility (e.g. Guard.WhiteList middleware on read-mostly routes).
+  - IsAdminAuthorized: fail-closed when the whitelist is empty. Used to gate
+    destructive admin endpoints so an unconfigured deployment does not
+    accept them from arbitrary peers.
+
 Referenced:
 https://github.com/pkieltyka/jwtauth/blob/master/jwtauth.go
 */
@@ -108,6 +115,10 @@ func (g *Guard) checkWhiteList(w http.ResponseWriter, r *http.Request) error {
 
 // IsWhiteListed returns true if the given host IP is allowed by the guard.
 // When no whitelist is configured (security inactive), all hosts are allowed.
+// This allow-all-when-empty behaviour is kept for HTTP compatibility: many
+// non-write HTTP routes wrap themselves with Guard.WhiteList and must remain
+// reachable on default deployments. Callers gating destructive endpoints must
+// use IsAdminAuthorized instead.
 func (g *Guard) IsWhiteListed(host string) bool {
 	if !g.isWriteActive {
 		return true
@@ -129,6 +140,25 @@ func (g *Guard) IsWhiteListed(host string) bool {
 	return false
 }
 
+// IsAdminAuthorized is the explicit, fail-closed equivalent of IsWhiteListed
+// for destructive admin endpoints. Unlike IsWhiteListed, it returns false
+// when no whitelist is configured, so callers must opt in by populating the
+// whitelist (CLI -whiteList or guard.white_list in security.toml).
+//
+// Loopback peers are trusted even when the whitelist is empty: a volume
+// server commonly cohabits with the master/filer on a single host (and on
+// every integration-test cluster), and an in-process loopback caller is
+// equivalent to local-host trust. Off-host callers remain denied.
+func (g *Guard) IsAdminAuthorized(host string) bool {
+	if g.isEmptyWhiteList {
+		if ip := net.ParseIP(host); ip != nil && ip.IsLoopback() {
+			return true
+		}
+		return false
+	}
+	return g.IsWhiteListed(host)
+}
+
 func (g *Guard) UpdateWhiteList(whiteList []string) {
 	whiteListIp := make(map[string]struct{})
 	whiteListCIDR := make(map[string]*net.IPNet)
@@ -137,6 +167,7 @@ func (g *Guard) UpdateWhiteList(whiteList []string) {
 			_, cidrnet, err := net.ParseCIDR(ip)
 			if err != nil {
 				glog.Errorf("Parse CIDR %s in whitelist failed: %v", ip, err)
+				continue
 			}
 			whiteListCIDR[ip] = cidrnet
 		} else {

weed/security/guard_whitelist_test.go

@@ -2,12 +2,11 @@ package security
 
 import "testing"
 
-// TestIsWhiteListedEmptyConfigAllowsEverything pins the contract that the
-// gRPC admin auth helper relies on: when no whitelist and no signing key
-// are configured, IsWhiteListed accepts any host (including the empty
-// string and unparseable peer addresses like the gRPC "@" passthrough
-// form). Otherwise insecure default deployments would lock themselves out
-// of every destructive admin RPC.
+// TestIsWhiteListedEmptyConfigAllowsEverything pins the HTTP-side contract:
+// when no whitelist and no signing key are configured, IsWhiteListed accepts
+// any host. This is the allow-all-when-empty path that Guard.WhiteList
+// middleware on read-mostly HTTP routes relies on; the destructive gRPC
+// admin gate uses IsAdminAuthorized instead, which is fail-closed.
 func TestIsWhiteListedEmptyConfigAllowsEverything(t *testing.T) {
 	g := NewGuard(nil, "", 0, "", 0)
 	for _, host := range []string{"", "@", "127.0.0.1", "::1", "garbage:value"} {
@@ -38,9 +37,9 @@ func TestIsWhiteListedWithListRejectsUnknown(t *testing.T) {
 }
 
 func TestIsWhiteListedWithSigningKeyButNoWhitelistAllowsAll(t *testing.T) {
-	// JWT-only mode: signing key set, whitelist empty. Without this branch
-	// the gRPC admin auth helper would falsely deny in-process callers when
-	// the operator wires up signing keys but hasn't enumerated IPs.
+	// JWT-only mode: signing key set, whitelist empty. IsWhiteListed must
+	// still accept every host because read-side HTTP middleware piggybacks
+	// on it; admin gating uses IsAdminAuthorized, which is fail-closed.
 	g := NewGuard(nil, "deadbeef", 0, "", 0)
 	for _, host := range []string{"", "@", "127.0.0.1"} {
 		if !g.IsWhiteListed(host) {
@@ -48,3 +47,81 @@ func TestIsWhiteListedWithSigningKeyButNoWhitelistAllowsAll(t *testing.T) {
 		}
 	}
 }
+
+// TestIsAdminAuthorizedEmptyWhitelistDeniesNonLoopback pins the fail-closed
+// contract the volume gRPC admin gate depends on: with no whitelist
+// configured every off-host peer is rejected, even when a signing key is
+// set. Loopback peers are still trusted (see
+// TestIsAdminAuthorizedEmptyWhitelistAllowsLoopback) because a volume server
+// commonly cohabits with master/filer on a single host.
+func TestIsAdminAuthorizedEmptyWhitelistDeniesNonLoopback(t *testing.T) {
+	cases := []*Guard{
+		NewGuard(nil, "", 0, "", 0),
+		NewGuard(nil, "deadbeef", 0, "", 0),
+		NewGuard([]string{}, "", 0, "", 0),
+	}
+	for i, g := range cases {
+		for _, host := range []string{"", "@", "10.0.0.1", "192.168.0.5", "8.8.8.8"} {
+			if g.IsAdminAuthorized(host) {
+				t.Errorf("case %d: empty whitelist should deny host=%q", i, host)
+			}
+		}
+	}
+}
+
+// TestIsAdminAuthorizedEmptyWhitelistAllowsLoopback pins the loopback
+// exception: when no whitelist is configured, in-process callers from
+// 127.0.0.0/8 or ::1 are still authorized. Without this, single-host
+// clusters and integration tests (which never set -whiteList) would lose
+// every admin RPC the moment fail-closed semantics kicked in.
+func TestIsAdminAuthorizedEmptyWhitelistAllowsLoopback(t *testing.T) {
+	cases := []*Guard{
+		NewGuard(nil, "", 0, "", 0),
+		NewGuard(nil, "deadbeef", 0, "", 0),
+		NewGuard([]string{}, "", 0, "", 0),
+	}
+	for i, g := range cases {
+		for _, host := range []string{"127.0.0.1", "127.0.0.5", "::1"} {
+			if !g.IsAdminAuthorized(host) {
+				t.Errorf("case %d: empty whitelist should still allow loopback host=%q", i, host)
+			}
+		}
+	}
+}
+
+func TestIsAdminAuthorizedWithIPList(t *testing.T) {
+	g := NewGuard([]string{"10.0.0.1"}, "", 0, "", 0)
+	cases := []struct {
+		host string
+		want bool
+	}{
+		{"10.0.0.1", true},
+		{"10.0.0.2", false},
+		{"", false},
+		{"@", false},
+	}
+	for _, tc := range cases {
+		if got := g.IsAdminAuthorized(tc.host); got != tc.want {
+			t.Errorf("IsAdminAuthorized(%q) = %v want %v", tc.host, got, tc.want)
+		}
+	}
+}
+
+func TestIsAdminAuthorizedWithCIDR(t *testing.T) {
+	g := NewGuard([]string{"10.0.0.0/24"}, "", 0, "", 0)
+	cases := []struct {
+		host string
+		want bool
+	}{
+		{"10.0.0.5", true},
+		{"10.0.0.255", true},
+		{"192.168.0.1", false},
+		{"10.0.1.1", false},
+		{"", false},
+	}
+	for _, tc := range cases {
+		if got := g.IsAdminAuthorized(tc.host); got != tc.want {
+			t.Errorf("IsAdminAuthorized(%q) = %v want %v", tc.host, got, tc.want)
+		}
+	}
+}

weed/server/volume_grpc_admin.go

@@ -30,17 +30,27 @@ import (
 // checkGrpcAdminAuth verifies the gRPC caller is authorized for destructive
 // admin operations by checking the peer address against the guard's whitelist.
 //
-// IP extraction prefers a typed *net.TCPAddr where available, falling back to
-// SplitHostPort on the string form, then to the raw string. The fallback
-// chain matters because in-process/passthrough connections used in tests
-// surface as unparseable strings like "@"; with an empty whitelist the
-// allow-all branch in IsWhiteListed accepts them, with a whitelist they're
-// denied as expected.
+// Only TCP peers go through the host check. Any non-TCP peer (in-process
+// bufconn, unix socket, etc.) is treated as trusted because such callers
+// share the same OS process or host filesystem as the volume server and
+// cannot be spoofed by a remote attacker. This matters when `weed server`
+// runs master+volume+filer in a single process and the master invokes
+// AllocateVolume through an in-process gRPC dial: the peer address surfaces
+// as "@" rather than 127.0.0.1, which has no parseable IP.
 //
-// Failed authorization attempts are logged so an operator running with a
-// configured whitelist can spot misconfigured callers and probe attempts.
+// The whitelist check uses Guard.IsAdminAuthorized, which is fail-closed: a
+// volume server with a configured guard but empty whitelist still rejects
+// destructive admin RPCs from off-host TCP peers (loopback TCP callers are
+// trusted, see IsAdminAuthorized). Operators must set -whiteList on the CLI
+// or guard.white_list in security.toml to allow remote admin peers. Failed
+// attempts are logged so misconfigured callers and probe attempts are
+// visible.
 func (vs *VolumeServer) checkGrpcAdminAuth(ctx context.Context) error {
 	if vs.guard == nil {
+		// No guard wired up at all — this is a developmental / embedded path
+		// (e.g. unit tests). Skip the gate; the real protection lives in
+		// IsAdminAuthorized, which only takes effect once a guard exists.
+		glog.V(1).Infof("gRPC admin auth: no guard configured; set -whiteList or guard.white_list in security.toml to enforce")
 		return nil
 	}
 	pr, ok := peer.FromContext(ctx)
@@ -50,17 +60,15 @@ func (vs *VolumeServer) checkGrpcAdminAuth(ctx context.Context) error {
 		glog.V(0).Infof("gRPC 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
+	tcpAddr, isTCP := pr.Addr.(*net.TCPAddr)
+	if !isTCP {
+		// In-process / bufconn / unix-socket peer: same OS process as us, so
+		// trusted by construction. A remote attacker cannot reach this path.
+		return nil
 	}
-	if !vs.guard.IsWhiteListed(host) {
-		glog.V(0).Infof("gRPC admin auth failed: %s is not whitelisted (remote: %s)", host, addr)
+	host := tcpAddr.IP.String()
+	if !vs.guard.IsAdminAuthorized(host) {
+		glog.V(0).Infof("gRPC admin auth failed: %s is not authorized (remote: %s)", host, pr.Addr.String())
 		return status.Errorf(codes.PermissionDenied, "not authorized: %s", host)
 	}
 	return nil

weed/server/volume_grpc_admin_test.go

@@ -0,0 +1,115 @@
+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/security"
+)
+
+// fakeAddr is a net.Addr that is not *net.TCPAddr. We use it to fake a
+// gRPC peer for the in-process / bufconn / unix-socket case.
+type fakeAddr struct{ s string }
+
+func (a fakeAddr) Network() string { return "fake" }
+func (a fakeAddr) String() string  { return a.s }
+
+// TestCheckGrpcAdminAuthNonTCPPeerTrusted pins the in-process trust contract.
+// When `weed server` runs master+volume+filer in the same process, the master
+// dials the volume server via an in-process / bufconn / unix-socket transport.
+// The peer Addr is not a *net.TCPAddr (often surfaces as "@"), so we cannot
+// extract an IP — but the caller is in the same OS process, so we trust it
+// unconditionally. Without this, an empty-whitelist fail-closed guard would
+// break every embedded-cluster deployment.
+func TestCheckGrpcAdminAuthNonTCPPeerTrusted(t *testing.T) {
+	vs := &VolumeServer{guard: security.NewGuard(nil, "", 0, "", 0)}
+	cases := []net.Addr{
+		fakeAddr{s: "@"},
+		fakeAddr{s: ""},
+		fakeAddr{s: "bufconn"},
+		&net.UnixAddr{Name: "/tmp/weed.sock", Net: "unix"},
+	}
+	for _, addr := range cases {
+		ctx := peer.NewContext(context.Background(), &peer.Peer{Addr: addr})
+		if err := vs.checkGrpcAdminAuth(ctx); err != nil {
+			t.Errorf("non-TCP peer %T(%q) should be trusted, got %v", addr, addr.String(), err)
+		}
+	}
+}
+
+// TestCheckGrpcAdminAuthTCPLoopbackTrusted covers the common single-host
+// deployment where a separate `weed` process on the same host dials over
+// real TCP. With no whitelist configured, loopback peers are still allowed
+// (delegated to Guard.IsAdminAuthorized).
+func TestCheckGrpcAdminAuthTCPLoopbackTrusted(t *testing.T) {
+	vs := &VolumeServer{guard: security.NewGuard(nil, "", 0, "", 0)}
+	for _, ip := range []string{"127.0.0.1", "127.0.0.5", "::1"} {
+		ctx := peer.NewContext(context.Background(), &peer.Peer{
+			Addr: &net.TCPAddr{IP: net.ParseIP(ip), Port: 50000},
+		})
+		if err := vs.checkGrpcAdminAuth(ctx); err != nil {
+			t.Errorf("loopback TCP peer %s should be trusted, got %v", ip, err)
+		}
+	}
+}
+
+// TestCheckGrpcAdminAuthTCPOffHostDenied is the fail-closed half of the
+// contract: an off-host TCP peer with no matching whitelist entry is denied
+// with PermissionDenied.
+func TestCheckGrpcAdminAuthTCPOffHostDenied(t *testing.T) {
+	vs := &VolumeServer{guard: security.NewGuard(nil, "", 0, "", 0)}
+	ctx := peer.NewContext(context.Background(), &peer.Peer{
+		Addr: &net.TCPAddr{IP: net.ParseIP("10.0.0.5"), Port: 50000},
+	})
+	err := vs.checkGrpcAdminAuth(ctx)
+	if err == nil {
+		t.Fatalf("off-host TCP peer with empty whitelist should be denied")
+	}
+	if got, want := status.Code(err), codes.PermissionDenied; got != want {
+		t.Errorf("status code = %v want %v", got, want)
+	}
+}
+
+// TestCheckGrpcAdminAuthTCPWhitelistedAllowed verifies that an off-host TCP
+// peer present in the configured whitelist is allowed through.
+func TestCheckGrpcAdminAuthTCPWhitelistedAllowed(t *testing.T) {
+	vs := &VolumeServer{guard: security.NewGuard([]string{"10.0.0.5"}, "", 0, "", 0)}
+	ctx := peer.NewContext(context.Background(), &peer.Peer{
+		Addr: &net.TCPAddr{IP: net.ParseIP("10.0.0.5"), Port: 50000},
+	})
+	if err := vs.checkGrpcAdminAuth(ctx); err != nil {
+		t.Errorf("whitelisted TCP peer should be allowed, got %v", err)
+	}
+}
+
+// TestCheckGrpcAdminAuthNoPeerInfoDenied confirms we still deny when gRPC
+// gives us no peer info at all (should not happen with real transports but
+// we cannot prove a positive identity, so we refuse).
+func TestCheckGrpcAdminAuthNoPeerInfoDenied(t *testing.T) {
+	vs := &VolumeServer{guard: security.NewGuard(nil, "", 0, "", 0)}
+	err := vs.checkGrpcAdminAuth(context.Background())
+	if err == nil {
+		t.Fatalf("missing peer info should be denied")
+	}
+	if got, want := status.Code(err), codes.PermissionDenied; got != want {
+		t.Errorf("status code = %v want %v", got, want)
+	}
+}
+
+// TestCheckGrpcAdminAuthNoGuardIsNoop preserves the developmental / embedded
+// path: with no Guard wired up at all, the gate is a no-op regardless of
+// peer information.
+func TestCheckGrpcAdminAuthNoGuardIsNoop(t *testing.T) {
+	vs := &VolumeServer{guard: nil}
+	ctx := peer.NewContext(context.Background(), &peer.Peer{
+		Addr: &net.TCPAddr{IP: net.ParseIP("10.0.0.5"), Port: 50000},
+	})
+	if err := vs.checkGrpcAdminAuth(ctx); err != nil {
+		t.Errorf("no guard should be a no-op, got %v", err)
+	}
+}