From cf38c01978aba02c061552ba3b7d6d5ebf655153 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 13 Sep 2026 21:48:14 -0700 Subject: [PATCH] admin: bind worker gRPC listener to -ip instead of wildcard (#11300) * admin: bind worker gRPC listener to -ip instead of wildcard The worker/plugin gRPC control plane called net.Listen("tcp", ":port") directly, so it wildcard-bound every interface and ignored the -ip setting. A cluster bound to loopback still exposed the unauthenticated WorkerService/PluginControlService streams on 0.0.0.0. Bind through util.JoinHostPort(bindIp, port) so the listener honors -ip like the master, filer, and volume gRPC listeners. * admin: warn when worker gRPC is exposed off loopback without mTLS The worker gRPC stream has no password auth, so grpc.admin mTLS is the only effective control once the listener leaves loopback. An operator who sets -adminPassword and binds -ip=0.0.0.0 authenticates the HTTP API but still exposes the unauthenticated worker control plane. Log a startup warning naming the port and the mTLS knobs so the exposure is not silent. * admin: address review on worker gRPC bind fix - mini: reserve the admin gRPC port with util.JoinHostPort so an IPv6 bindIp (e.g. ::1) does not form an invalid unbracketed address and lose the reservation. - worker gRPC: track whether grpc.admin mTLS credentials actually loaded rather than only whether they were configured, and gate the non-loopback exposure warning on that. A cert/key that fails to load now still warns instead of silently suppressing. --- weed/admin/dash/admin_server.go | 13 ++++++++++--- weed/admin/dash/worker_grpc_server.go | 20 ++++++++++++-------- weed/command/admin.go | 19 ++++++++++++++++++- weed/command/mini.go | 2 +- 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/weed/admin/dash/admin_server.go b/weed/admin/dash/admin_server.go index 97f443ed7..d015d571c 100644 --- a/weed/admin/dash/admin_server.go +++ b/weed/admin/dash/admin_server.go @@ -1616,14 +1616,21 @@ func (as *AdminServer) GetConfigInfo(w http.ResponseWriter, r *http.Request) { }) } -// StartWorkerGrpcServer starts the worker gRPC server -func (s *AdminServer) StartWorkerGrpcServer(grpcPort int, listener net.Listener) error { +// StartWorkerGrpcServer starts the worker gRPC server. bindIp is honored when no +// listener is supplied so the worker gRPC does not wildcard-bind past -ip. +func (s *AdminServer) StartWorkerGrpcServer(bindIp string, grpcPort int, listener net.Listener) error { if s.workerGrpcServer != nil { return fmt.Errorf("worker gRPC server is already running") } s.workerGrpcServer = NewWorkerGrpcServer(s) - return s.workerGrpcServer.StartWithTLS(grpcPort, listener) + return s.workerGrpcServer.StartWithTLS(bindIp, grpcPort, listener) +} + +// WorkerGrpcMTLSEnabled reports whether the worker gRPC server actually loaded +// grpc.admin mTLS credentials, not just whether they were configured. +func (s *AdminServer) WorkerGrpcMTLSEnabled() bool { + return s.workerGrpcServer != nil && s.workerGrpcServer.mtlsEnabled } // StopWorkerGrpcServer stops the worker gRPC server diff --git a/weed/admin/dash/worker_grpc_server.go b/weed/admin/dash/worker_grpc_server.go index c6d9f4dba..96e1f2f40 100644 --- a/weed/admin/dash/worker_grpc_server.go +++ b/weed/admin/dash/worker_grpc_server.go @@ -47,10 +47,11 @@ type WorkerGrpcServer struct { logRequestsMutex sync.RWMutex // gRPC server - grpcServer *grpc.Server - listener net.Listener - running bool - stopChan chan struct{} + grpcServer *grpc.Server + listener net.Listener + running bool + stopChan chan struct{} + mtlsEnabled bool } // LogRequestContext tracks pending log requests @@ -84,22 +85,25 @@ func NewWorkerGrpcServer(adminServer *AdminServer) *WorkerGrpcServer { } // StartWithTLS starts the gRPC server on the specified port with optional TLS. -// A caller that already holds the port passes its listener instead. -func (s *WorkerGrpcServer) StartWithTLS(port int, listener net.Listener) error { +// A caller that already holds the port passes its listener instead. When no +// listener is supplied the server binds to bindIp to honor the operator's -ip. +func (s *WorkerGrpcServer) StartWithTLS(bindIp string, port int, listener net.Listener) error { if s.running { return fmt.Errorf("worker gRPC server is already running") } if listener == nil { var err error - listener, err = net.Listen("tcp", fmt.Sprintf(":%d", port)) + listener, err = net.Listen("tcp", util.JoinHostPort(bindIp, port)) if err != nil { return fmt.Errorf("failed to listen on port %d: %v", port, err) } } // Create gRPC server with optional TLS - grpcServer := pb.NewGrpcServer(security.LoadServerTLS(util.GetViper(), "grpc.admin")) + tlsOption, _ := security.LoadServerTLS(util.GetViper(), "grpc.admin") + s.mtlsEnabled = tlsOption != nil + grpcServer := pb.NewGrpcServer(tlsOption) worker_pb.RegisterWorkerServiceServer(grpcServer, s) if plugin := s.adminServer.GetPlugin(); plugin != nil { diff --git a/weed/command/admin.go b/weed/command/admin.go index 055cc368f..f61435fd4 100644 --- a/weed/command/admin.go +++ b/weed/command/admin.go @@ -461,10 +461,11 @@ func startAdminServer(ctx context.Context, options AdminOptions, enableUI bool, } // Start worker gRPC server for worker connections - err = adminServer.StartWorkerGrpcServer(*options.grpcPort, options.workerGrpcListener) + err = adminServer.StartWorkerGrpcServer(*options.ip, *options.grpcPort, options.workerGrpcListener) if err != nil { return fmt.Errorf("failed to start worker gRPC server: %w", err) } + warnInsecureWorkerGrpcBind(*options.ip, *options.grpcPort, adminServer.WorkerGrpcMTLSEnabled()) // Set up cleanup for gRPC server defer func() { @@ -758,3 +759,19 @@ func isLoopbackIp(ip string) bool { } return parsed.IsLoopback() } + +// warnInsecureWorkerGrpcBind warns when the worker gRPC control plane is +// reachable off loopback without grpc.admin mTLS, its only auth once exposed. +// mtlsEnabled reflects whether the worker gRPC actually loaded mTLS, so a +// misconfigured cert/key that fails to load still triggers the warning. +func warnInsecureWorkerGrpcBind(ip string, grpcPort int, mtlsEnabled bool) { + if isLoopbackIp(ip) { + return + } + if mtlsEnabled { + return + } + glog.Warningf("Worker gRPC control plane is bound to %s (non-loopback) without grpc.admin mTLS.", ip) + glog.Warningf("Anyone who can reach port %d can register a maintenance worker unauthenticated.", grpcPort) + glog.Warningf("Enable [grpc.admin] cert/key and [grpc.ca] in security.toml, or bind to loopback.") +} diff --git a/weed/command/mini.go b/weed/command/mini.go index ec34f6e55..8ce45436a 100644 --- a/weed/command/mini.go +++ b/weed/command/mini.go @@ -953,7 +953,7 @@ func ensureAllPortsAvailableOnIP(bindIp string) error { // first: an in-process rerun would otherwise inherit the closed listener // of the previous run and only find out inside Serve. miniAdminOptions.workerGrpcListener = nil - if listener, err := net.Listen("tcp", fmt.Sprintf(":%d", *miniAdminOptions.grpcPort)); err != nil { + if listener, err := net.Listen("tcp", util.JoinHostPort(bindIp, *miniAdminOptions.grpcPort)); err != nil { glog.Warningf("Could not reserve Admin gRPC port %d: %v", *miniAdminOptions.grpcPort, err) } else { miniAdminOptions.workerGrpcListener = listener