From c36fbc24d9fad8b9050f00daa6bccd1be4fb8cd0 Mon Sep 17 00:00:00 2001 From: "Juan A. Garcia Pardo" Date: Thu, 5 Sep 2019 14:40:57 +0200 Subject: [PATCH 1/3] sciond socket file mode. sciond sets the file mode for both its Reliable and Unix socket files. The file mode comes from the configuration value SocketFileMode, specified in octal as a string. --- go/lib/sciond/sciond.go | 2 ++ go/sciond/internal/config/config.go | 18 ++++++++++++++++++ go/sciond/internal/config/config_test.go | 1 + go/sciond/internal/config/sample.go | 3 +++ go/sciond/internal/servers/server.go | 17 ++++++++++++++--- go/sciond/main.go | 2 +- 6 files changed, 39 insertions(+), 4 deletions(-) diff --git a/go/lib/sciond/sciond.go b/go/lib/sciond/sciond.go index 6f2e1b57fb..c38a827c6f 100644 --- a/go/lib/sciond/sciond.go +++ b/go/lib/sciond/sciond.go @@ -53,6 +53,8 @@ const ( SVCInfoTTL = 10 * time.Second // DefaultSCIONDPath contains the system default for a SCIOND socket. DefaultSCIONDPath = "/run/shm/sciond/default.sock" + // DefaultSocketFileMode allows read/write to the user and group only. + DefaultSocketFileMode = 0770 ) // Service describes a SCIOND endpoint. New connections to SCIOND can be diff --git a/go/sciond/internal/config/config.go b/go/sciond/internal/config/config.go index 60f5df8fe9..8d7df76d2b 100644 --- a/go/sciond/internal/config/config.go +++ b/go/sciond/internal/config/config.go @@ -17,6 +17,8 @@ package config import ( "io" + "os" + "strconv" "time" "github.com/scionproto/scion/go/lib/common" @@ -100,6 +102,8 @@ type SDConfig struct { // Address to listen on for normal unixgram messages. If empty, a // unixgram server on the default socket is started. Unix string + // Socket files (both Reliable and Unix) permissions when created; read from octal (e.g. 0755). + SocketFileMode FileMode // If set to True, the socket is removed before being created DeleteSocket bool // Public is the local address to listen on for SCION messages (if Bind is @@ -124,6 +128,9 @@ func (cfg *SDConfig) InitDefaults() { if cfg.Unix == "" { cfg.Unix = "/run/shm/sciond/default-unix.sock" } + if cfg.SocketFileMode == 0 { + cfg.SocketFileMode = sciond.DefaultSocketFileMode + } if cfg.QueryInterval.Duration == 0 { cfg.QueryInterval.Duration = DefaultQueryInterval } @@ -137,6 +144,9 @@ func (cfg *SDConfig) Validate() error { if cfg.Unix == "" { return common.NewBasicError("Unix must be set", nil) } + if cfg.SocketFileMode == 0 { + return common.NewBasicError("SocketFileMode must be set", nil) + } if cfg.QueryInterval.Duration == 0 { return common.NewBasicError("QueryInterval must not be zero", nil) } @@ -161,3 +171,11 @@ func (cfg *SDConfig) CreateSocketDirs() error { } return nil } + +type FileMode os.FileMode + +func (f *FileMode) UnmarshalText(text []byte) error { + perm, err := strconv.ParseUint(string(text), 8, 32) + *f = FileMode(perm) + return err +} diff --git a/go/sciond/internal/config/config_test.go b/go/sciond/internal/config/config_test.go index 045a3eaa0f..5794e8c645 100644 --- a/go/sciond/internal/config/config_test.go +++ b/go/sciond/internal/config/config_test.go @@ -67,6 +67,7 @@ func CheckTestSDConfig(cfg *SDConfig, id string) { pathstoragetest.CheckTestRevCacheConf(&cfg.RevCache) SoMsg("Reliable correct", cfg.Reliable, ShouldEqual, sciond.DefaultSCIONDPath) SoMsg("Unix correct", cfg.Unix, ShouldEqual, "/run/shm/sciond/default-unix.sock") + SoMsg("File permissions correct", cfg.SocketFileMode, ShouldEqual, 0755) SoMsg("Public correct", cfg.Public.String(), ShouldEqual, "1-ff00:0:110,[127.0.0.1]:0 (UDP)") SoMsg("QueryInterval correct", cfg.QueryInterval.Duration, ShouldEqual, DefaultQueryInterval) diff --git a/go/sciond/internal/config/sample.go b/go/sciond/internal/config/sample.go index 46e6a61dda..881c0b1fc9 100644 --- a/go/sciond/internal/config/sample.go +++ b/go/sciond/internal/config/sample.go @@ -25,6 +25,9 @@ Reliable = "/run/shm/sciond/default.sock" # unixgram server on the default socket is started. Unix = "/run/shm/sciond/default-unix.sock" +# File permissions of both the Reliable and Unix socket files, in octal. +SocketFileMode = "0755" + # If set to True, the socket is removed before being created. (default false) DeleteSocket = false diff --git a/go/sciond/internal/servers/server.go b/go/sciond/internal/servers/server.go index c0c5e57754..3f4e554529 100644 --- a/go/sciond/internal/servers/server.go +++ b/go/sciond/internal/servers/server.go @@ -18,6 +18,7 @@ import ( "context" "io" "net" + "os" "strings" "sync" @@ -35,6 +36,7 @@ type HandlerMap map[proto.SCIONDMsg_Which]Handler type Server struct { network string address string + filemode os.FileMode handlers map[proto.SCIONDMsg_Which]Handler log log.Logger @@ -48,10 +50,11 @@ type Server struct { // HandlerMap. To start listening on the address, call ListenAndServe. // // Network must be "unixpacket" or "rsock". -func NewServer(network string, address string, handlers HandlerMap, logger log.Logger) *Server { +func NewServer(network string, address string, filemode os.FileMode, handlers HandlerMap, logger log.Logger) *Server { return &Server{ network: network, address: address, + filemode: filemode, handlers: handlers, log: logger, } @@ -99,18 +102,26 @@ func (srv *Server) ListenAndServe() error { } func (srv *Server) listen() (net.Listener, error) { + var listener net.Listener + var error error switch srv.network { case "unixpacket": laddr, err := net.ResolveUnixAddr("unixpacket", srv.address) if err != nil { return nil, err } - return net.ListenUnix("unixpacket", laddr) + listener, error = net.ListenUnix("unixpacket", laddr) case "rsock": - return reliable.Listen(srv.address) + listener, error = reliable.Listen(srv.address) default: return nil, common.NewBasicError("unknown network", nil, "net", srv.network) } + if error == nil { + if err := os.Chmod(srv.address, srv.filemode); err != nil { + return nil, common.NewBasicError("chmod failed", err, "address", srv.address) + } + } + return listener, error } // Close makes the Server stop listening for new connections, and immediately diff --git a/go/sciond/main.go b/go/sciond/main.go index 4dc9195f4f..684ca81156 100644 --- a/go/sciond/main.go +++ b/go/sciond/main.go @@ -222,7 +222,7 @@ func startDiscovery() error { func NewServer(network string, rsockPath string, handlers servers.HandlerMap, logger log.Logger) (*servers.Server, func()) { - server := servers.NewServer(network, rsockPath, handlers, logger) + server := servers.NewServer(network, rsockPath, os.FileMode(cfg.SD.SocketFileMode), handlers, logger) shutdownF := func() { ctx, cancelF := context.WithTimeout(context.Background(), ShutdownWaitTimeout) server.Shutdown(ctx) From ff112b3ce29c820909edca1e2cdf5c4e2d161093 Mon Sep 17 00:00:00 2001 From: "Juan A. Garcia Pardo" Date: Thu, 5 Sep 2019 15:51:24 +0200 Subject: [PATCH 2/3] Changes from review. Use default value in config test. Refactor and comments. --- go/sciond/internal/config/config_test.go | 2 +- go/sciond/internal/config/sample.go | 4 ++-- go/sciond/internal/servers/server.go | 20 +++++++++++--------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/go/sciond/internal/config/config_test.go b/go/sciond/internal/config/config_test.go index 5794e8c645..7c3ec1babe 100644 --- a/go/sciond/internal/config/config_test.go +++ b/go/sciond/internal/config/config_test.go @@ -67,7 +67,7 @@ func CheckTestSDConfig(cfg *SDConfig, id string) { pathstoragetest.CheckTestRevCacheConf(&cfg.RevCache) SoMsg("Reliable correct", cfg.Reliable, ShouldEqual, sciond.DefaultSCIONDPath) SoMsg("Unix correct", cfg.Unix, ShouldEqual, "/run/shm/sciond/default-unix.sock") - SoMsg("File permissions correct", cfg.SocketFileMode, ShouldEqual, 0755) + SoMsg("File permissions correct", cfg.SocketFileMode, ShouldEqual, sciond.DefaultSocketFileMode) SoMsg("Public correct", cfg.Public.String(), ShouldEqual, "1-ff00:0:110,[127.0.0.1]:0 (UDP)") SoMsg("QueryInterval correct", cfg.QueryInterval.Duration, ShouldEqual, DefaultQueryInterval) diff --git a/go/sciond/internal/config/sample.go b/go/sciond/internal/config/sample.go index 881c0b1fc9..a328317f12 100644 --- a/go/sciond/internal/config/sample.go +++ b/go/sciond/internal/config/sample.go @@ -25,8 +25,8 @@ Reliable = "/run/shm/sciond/default.sock" # unixgram server on the default socket is started. Unix = "/run/shm/sciond/default-unix.sock" -# File permissions of both the Reliable and Unix socket files, in octal. -SocketFileMode = "0755" +# File permissions of both the Reliable and Unix socket files, in octal. (default "0770") +SocketFileMode = "0770" # If set to True, the socket is removed before being created. (default false) DeleteSocket = false diff --git a/go/sciond/internal/servers/server.go b/go/sciond/internal/servers/server.go index 3f4e554529..14cd790907 100644 --- a/go/sciond/internal/servers/server.go +++ b/go/sciond/internal/servers/server.go @@ -103,25 +103,27 @@ func (srv *Server) ListenAndServe() error { func (srv *Server) listen() (net.Listener, error) { var listener net.Listener - var error error + var err error switch srv.network { case "unixpacket": - laddr, err := net.ResolveUnixAddr("unixpacket", srv.address) + var laddr *net.UnixAddr + laddr, err = net.ResolveUnixAddr("unixpacket", srv.address) if err != nil { return nil, err } - listener, error = net.ListenUnix("unixpacket", laddr) + listener, err = net.ListenUnix("unixpacket", laddr) case "rsock": - listener, error = reliable.Listen(srv.address) + listener, err = reliable.Listen(srv.address) default: return nil, common.NewBasicError("unknown network", nil, "net", srv.network) } - if error == nil { - if err := os.Chmod(srv.address, srv.filemode); err != nil { - return nil, common.NewBasicError("chmod failed", err, "address", srv.address) - } + if err != nil { + return nil, err + } + if err := os.Chmod(srv.address, srv.filemode); err != nil { + return nil, common.NewBasicError("chmod failed", err, "address", srv.address) } - return listener, error + return listener, nil } // Close makes the Server stop listening for new connections, and immediately From 96d26972c4524273113b48a1d51ee78c706d6232 Mon Sep 17 00:00:00 2001 From: "Juan A. Garcia Pardo" Date: Thu, 5 Sep 2019 16:16:43 +0200 Subject: [PATCH 3/3] lint: line length --- go/sciond/internal/servers/server.go | 4 +++- go/sciond/main.go | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/go/sciond/internal/servers/server.go b/go/sciond/internal/servers/server.go index 14cd790907..37b018ca6b 100644 --- a/go/sciond/internal/servers/server.go +++ b/go/sciond/internal/servers/server.go @@ -50,7 +50,9 @@ type Server struct { // HandlerMap. To start listening on the address, call ListenAndServe. // // Network must be "unixpacket" or "rsock". -func NewServer(network string, address string, filemode os.FileMode, handlers HandlerMap, logger log.Logger) *Server { +func NewServer(network string, address string, filemode os.FileMode, handlers HandlerMap, + logger log.Logger) *Server { + return &Server{ network: network, address: address, diff --git a/go/sciond/main.go b/go/sciond/main.go index 684ca81156..3910851ea9 100644 --- a/go/sciond/main.go +++ b/go/sciond/main.go @@ -222,7 +222,8 @@ func startDiscovery() error { func NewServer(network string, rsockPath string, handlers servers.HandlerMap, logger log.Logger) (*servers.Server, func()) { - server := servers.NewServer(network, rsockPath, os.FileMode(cfg.SD.SocketFileMode), handlers, logger) + server := servers.NewServer(network, rsockPath, os.FileMode(cfg.SD.SocketFileMode), handlers, + logger) shutdownF := func() { ctx, cancelF := context.WithTimeout(context.Background(), ShutdownWaitTimeout) server.Shutdown(ctx)