Skip to content

Commit e980d11

Browse files
authored
[gnmi] Add Unix Domain Socket listener for local access (sonic-net#579)
* Add co-existing Unix domain socket listener support Add --unix_socket flag that creates an additional UDS listener alongside the TCP listener. The UDS listener runs without TLS, enabling secure local-only access for clients like device-ops-agent. Key changes: - Add udsServer and udsListener fields to Server struct - Split NewServer opts into tlsOpts (TCP only) and commonOpts (both) - Serve() runs both listeners concurrently - Stop()/ForceStop() cleanup both servers and remove socket file - Socket permissions set to 0660 for root-only access Flag behavior: - --port > 0: TCP listener with TLS - --unix_socket set: additional UDS listener without TLS - Both can be used together Signed-off-by: Dawei Huang <[email protected]> * Fix format Signed-off-by: Dawei Huang <[email protected]> * Fix TestServerPort for new port validation Signed-off-by: Dawei Huang <[email protected]> * Add tests for UDS server code paths to improve coverage Signed-off-by: Dawei Huang <[email protected]> * Add default unix_socket path and create socket directory if needed Signed-off-by: Dawei Huang <[email protected]> * Remove contrived TestServerUnixSocketInvalidPath test The test tried to simulate directory creation failure by placing a file in the path, but this scenario is unrealistic and difficult to test reliably across environments. Signed-off-by: Dawei Huang <[email protected]> * Address PR review feedback from Copilot - Check os.Chmod error: warn and disable UDS instead of failing server - Fix misleading comment about socket permissions - Remove duplicate Address function comment - Fix Serve() blocking forever on graceful stop Signed-off-by: Dawei Huang <[email protected]> * Add TestServerUnixSocketServeAndStop for UDS Serve() coverage Signed-off-by: Dawei Huang <[email protected]> * Restrict UDS socket directory permissions to 0750 Addresses PR review feedback: prevent TOCTOU race condition where an attacker monitoring the directory could connect to the socket after net.Listen creates it but before os.Chmod restricts access. Signed-off-by: Dawei Huang <[email protected]> --------- Signed-off-by: Dawei Huang <[email protected]>
1 parent aa7d5e1 commit e980d11

4 files changed

Lines changed: 407 additions & 89 deletions

File tree

gnmi_server/server.go

Lines changed: 167 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"errors"
77
"fmt"
88
"net"
9+
"os"
10+
"path/filepath"
911
"strings"
1012
"sync"
1113

@@ -48,11 +50,17 @@ var (
4850
// via Subscribe or Get will receive a stream of updates based on the requested
4951
// path. Set request is processed by server too.
5052
type Server struct {
51-
s *grpc.Server
52-
lis net.Listener
53-
config *Config
54-
cMu sync.Mutex
55-
clients map[string]*Client
53+
s *grpc.Server
54+
lis net.Listener
55+
// udsServer is the gRPC server for Unix domain socket connections (no TLS).
56+
// This is nil if UnixSocket is not configured.
57+
udsServer *grpc.Server
58+
// udsListener is the listener for Unix domain socket connections.
59+
// This is nil if UnixSocket is not configured.
60+
udsListener net.Listener
61+
config *Config
62+
cMu sync.Mutex
63+
clients map[string]*Client
5664
// SaveStartupConfig points to a function that is called to save changes of
5765
// configuration to a file. By default it points to an empty function -
5866
// the configuration is not saved to a file.
@@ -163,8 +171,11 @@ type AuthTypes map[string]bool
163171
// Config is a collection of values for Server
164172
type Config struct {
165173
// Port for the Server to listen on. If 0 or unset the Server will pick a port
166-
// for this Server.
167-
Port int64
174+
// for this Server. Port > 0 enables the TCP listener.
175+
Port int64
176+
// UnixSocket is the path to a Unix domain socket to listen on.
177+
// When set, an additional listener is created for local connections without TLS.
178+
UnixSocket string
168179
LogLevel int
169180
Threshold int
170181
UserAuth AuthTypes
@@ -265,41 +276,58 @@ func (i AuthTypes) Unset(mode string) error {
265276
return nil
266277
}
267278

268-
// New returns an initialized Server.
269-
func NewServer(config *Config, opts []grpc.ServerOption) (*Server, error) {
279+
// registerAllServices registers all gNMI and gNOI services on the given gRPC server.
280+
func registerAllServices(s *grpc.Server, srv *Server, fileSrv *FileServer,
281+
osSrv *OSServer, containerzSrv *ContainerzServer,
282+
debugSrv *DebugServer, healthzSrv *HealthzServer) {
283+
gnmipb.RegisterGNMIServer(s, srv)
284+
factory_reset.RegisterFactoryResetServer(s, srv)
285+
spb_jwt_gnoi.RegisterSonicJwtServiceServer(s, srv)
286+
if srv.config.EnableTranslibWrite || srv.config.EnableNativeWrite {
287+
gnoi_system_pb.RegisterSystemServer(s, srv)
288+
gnoi_file_pb.RegisterFileServer(s, fileSrv)
289+
gnoi_os_pb.RegisterOSServer(s, osSrv)
290+
gnoi_containerz_pb.RegisterContainerzServer(s, containerzSrv)
291+
gnoi_debug_pb.RegisterDebugServer(s, debugSrv)
292+
gnoi_healthz_pb.RegisterHealthzServer(s, healthzSrv)
293+
}
294+
if srv.config.EnableTranslibWrite {
295+
spb_gnoi.RegisterSonicServiceServer(s, srv)
296+
}
297+
spb_gnoi.RegisterDebugServer(s, srv)
298+
}
299+
300+
// NewServer returns an initialized Server.
301+
//
302+
// tlsOpts contains TLS credentials and is used only for the TCP listener.
303+
// commonOpts contains interceptors, keepalive params, etc. and is used for both listeners.
304+
//
305+
// When config.Port > 0, a TCP listener is created with TLS.
306+
// When config.UnixSocket is set, an additional UDS listener is created without TLS.
307+
func NewServer(config *Config, tlsOpts []grpc.ServerOption, commonOpts []grpc.ServerOption) (*Server, error) {
270308
if config == nil {
271309
return nil, errors.New("config not provided")
272310
}
273311
common_utils.InitCounters()
274312

275-
s := grpc.NewServer(opts...)
276-
reflection.Register(s)
277-
278313
srv := &Server{
279-
s: s,
280314
config: config,
281315
clients: map[string]*Client{},
282316
SaveStartupConfig: saveOnSetDisabled,
283-
// ReqFromMaster point to a function that is called to verify if
284-
// the request comes from a master controller.
285-
ReqFromMaster: ReqFromMasterDisabledMA,
286-
masterEID: uint128{High: 0, Low: 0},
317+
ReqFromMaster: ReqFromMasterDisabledMA,
318+
masterEID: uint128{High: 0, Low: 0},
287319
}
288320

321+
// Create service servers (shared between TCP and UDS)
289322
fileSrv := &FileServer{Server: srv}
290-
291-
// Create an instance of the concrete OSBackend implementation
292323
osBackend := &DBusOSBackend{}
293-
294324
osSrv := &OSServer{
295325
Server: srv,
296-
backend: osBackend, // Pass the installer dependency
326+
backend: osBackend,
297327
ImgDir: srv.config.ImgDir,
298328
}
299-
300329
containerzSrv := &ContainerzServer{server: srv}
301330
healthzSrv := &HealthzServer{Server: srv}
302-
303331
readWhitelist, writeWhitelist := gnoi_debug.ConstructWhitelists()
304332
debugSrv := &DebugServer{
305333
Server: srv,
@@ -308,64 +336,145 @@ func NewServer(config *Config, opts []grpc.ServerOption) (*Server, error) {
308336
}
309337

310338
var err error
311-
if srv.config.Port < 0 {
312-
srv.config.Port = 0
313-
}
314-
srv.lis, err = net.Listen("tcp", fmt.Sprintf(":%d", srv.config.Port))
315-
if err != nil {
316-
return nil, fmt.Errorf("failed to open listener port %d: %v", srv.config.Port, err)
339+
340+
// TCP Server (Port > 0)
341+
if config.Port > 0 {
342+
tcpOpts := append(tlsOpts, commonOpts...)
343+
srv.s = grpc.NewServer(tcpOpts...)
344+
reflection.Register(srv.s)
345+
346+
srv.lis, err = net.Listen("tcp", fmt.Sprintf(":%d", config.Port))
347+
if err != nil {
348+
return nil, fmt.Errorf("failed to open listener port %d: %v", config.Port, err)
349+
}
350+
351+
registerAllServices(srv.s, srv, fileSrv, osSrv, containerzSrv, debugSrv, healthzSrv)
317352
}
318-
gnmipb.RegisterGNMIServer(srv.s, srv)
319-
factory_reset.RegisterFactoryResetServer(srv.s, srv)
320-
spb_jwt_gnoi.RegisterSonicJwtServiceServer(srv.s, srv)
321-
if srv.config.EnableTranslibWrite || srv.config.EnableNativeWrite {
322-
gnoi_system_pb.RegisterSystemServer(srv.s, srv)
323-
gnoi_file_pb.RegisterFileServer(srv.s, fileSrv)
324-
gnoi_os_pb.RegisterOSServer(srv.s, osSrv)
325-
gnoi_containerz_pb.RegisterContainerzServer(srv.s, containerzSrv)
326-
gnoi_debug_pb.RegisterDebugServer(srv.s, debugSrv)
327-
gnoi_healthz_pb.RegisterHealthzServer(srv.s, healthzSrv)
353+
354+
// UDS Server (UnixSocket set)
355+
if config.UnixSocket != "" {
356+
// UDS server uses only commonOpts (no TLS)
357+
srv.udsServer = grpc.NewServer(commonOpts...)
358+
reflection.Register(srv.udsServer)
359+
360+
// Create socket directory if it doesn't exist (0750 to prevent unauthorized access
361+
// during the window between socket creation and permission setting)
362+
socketDir := filepath.Dir(config.UnixSocket)
363+
if err := os.MkdirAll(socketDir, 0750); err != nil {
364+
if srv.lis != nil {
365+
srv.lis.Close()
366+
}
367+
return nil, fmt.Errorf("failed to create socket directory %s: %v", socketDir, err)
368+
}
369+
370+
os.Remove(config.UnixSocket) // Remove stale socket
371+
srv.udsListener, err = net.Listen("unix", config.UnixSocket)
372+
if err != nil {
373+
// Cleanup TCP listener if it was created
374+
if srv.lis != nil {
375+
srv.lis.Close()
376+
}
377+
return nil, fmt.Errorf("failed to listen on unix socket %s: %v", config.UnixSocket, err)
378+
}
379+
// Restrict socket access to container user (root) and group
380+
if err := os.Chmod(config.UnixSocket, 0660); err != nil {
381+
log.Warningf("Failed to set permissions on unix socket %s: %v; disabling UDS listener", config.UnixSocket, err)
382+
srv.udsListener.Close()
383+
os.Remove(config.UnixSocket)
384+
srv.udsListener = nil
385+
srv.udsServer = nil
386+
} else {
387+
registerAllServices(srv.udsServer, srv, fileSrv, osSrv, containerzSrv, debugSrv, healthzSrv)
388+
}
328389
}
329-
if srv.config.EnableTranslibWrite {
330-
spb_gnoi.RegisterSonicServiceServer(srv.s, srv)
331390

391+
// Require at least one listener
392+
if srv.lis == nil && srv.udsListener == nil {
393+
return nil, errors.New("no listener configured: port must be > 0 or unix_socket must be set")
332394
}
333-
spb_gnoi.RegisterDebugServer(srv.s, srv)
395+
334396
log.V(1).Infof("Created Server on %s, read-only: %t", srv.Address(), !srv.config.EnableTranslibWrite)
335397
return srv, nil
336398
}
337399

338400
// Serve will start the Server serving and block until closed.
401+
// If both TCP and UDS listeners are configured, both are served concurrently.
339402
func (srv *Server) Serve() error {
340-
s := srv.s
341-
if s == nil {
403+
if srv.s == nil && srv.udsServer == nil {
342404
return fmt.Errorf("Serve() failed: not initialized")
343405
}
344-
return srv.s.Serve(srv.lis)
406+
407+
errChan := make(chan error, 2)
408+
409+
// Start TCP server if configured
410+
if srv.s != nil && srv.lis != nil {
411+
go func() {
412+
log.V(1).Infof("Starting TCP server on %s", srv.lis.Addr().String())
413+
err := srv.s.Serve(srv.lis)
414+
if err != nil {
415+
errChan <- fmt.Errorf("TCP server: %w", err)
416+
} else {
417+
errChan <- nil
418+
}
419+
}()
420+
}
421+
422+
// Start UDS server if configured
423+
if srv.udsServer != nil && srv.udsListener != nil {
424+
go func() {
425+
log.V(1).Infof("Starting UDS server on %s", srv.udsListener.Addr().String())
426+
err := srv.udsServer.Serve(srv.udsListener)
427+
if err != nil {
428+
errChan <- fmt.Errorf("UDS server: %w", err)
429+
} else {
430+
errChan <- nil
431+
}
432+
}()
433+
}
434+
435+
// Block until first error (or server stop)
436+
return <-errChan
345437
}
346438

439+
// ForceStop stops the server immediately without waiting for connections to close.
347440
func (srv *Server) ForceStop() {
348-
s := srv.s
349-
if s == nil {
350-
log.Errorf("ForceStop() failed: not initialized")
351-
return
441+
if srv.s != nil {
442+
srv.s.Stop()
443+
}
444+
if srv.udsServer != nil {
445+
srv.udsServer.Stop()
446+
}
447+
// Cleanup UDS socket file
448+
if srv.config != nil && srv.config.UnixSocket != "" {
449+
os.Remove(srv.config.UnixSocket)
352450
}
353-
s.Stop()
354451
}
355452

453+
// Stop gracefully stops the server, waiting for active connections to close.
356454
func (srv *Server) Stop() {
357-
s := srv.s
358-
if s == nil {
359-
log.Errorf("Stop() failed: not initialized")
360-
return
455+
if srv.s != nil {
456+
srv.s.GracefulStop()
457+
}
458+
if srv.udsServer != nil {
459+
srv.udsServer.GracefulStop()
460+
}
461+
// Cleanup UDS socket file
462+
if srv.config != nil && srv.config.UnixSocket != "" {
463+
os.Remove(srv.config.UnixSocket)
361464
}
362-
s.GracefulStop()
363465
}
364466

365-
// Address returns the port the Server is listening to.
467+
// Address returns the addresses the Server is listening on.
366468
func (srv *Server) Address() string {
367-
addr := srv.lis.Addr().String()
368-
return strings.Replace(addr, "[::]", "localhost", 1)
469+
var addrs []string
470+
if srv.lis != nil {
471+
addr := srv.lis.Addr().String()
472+
addrs = append(addrs, strings.Replace(addr, "[::]", "localhost", 1))
473+
}
474+
if srv.udsListener != nil {
475+
addrs = append(addrs, srv.udsListener.Addr().String())
476+
}
477+
return strings.Join(addrs, ", ")
369478
}
370479

371480
// Port returns the port the Server is listening to.

0 commit comments

Comments
 (0)