From 21dd95d9af2412543b220aa33114ce96104c2907 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 20 Jan 2015 13:44:27 -0800 Subject: [PATCH 01/10] agent: beginning socket user/group/mode support as discussed in #612 --- command/agent/agent.go | 7 ------- command/agent/command.go | 23 +++++++++++++++++----- command/agent/config.go | 12 ++++++++++++ command/agent/http.go | 17 ++++++++++++++--- command/agent/util.go | 41 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 15 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index 9454dba4c0..e73af4222d 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -23,13 +23,6 @@ const ( // Path to save local agent checks checksDir = "checks" - // errSocketFileExists is the human-friendly error message displayed when - // trying to bind a socket to an existing file. - errSocketFileExists = "A file exists at the requested socket path %q. " + - "If Consul was not shut down properly, the socket file may " + - "be left behind. If the path looks correct, remove the file " + - "and try again." - // The ID of the faux health checks for maintenance mode serviceMaintCheckPrefix = "_service_maintenance" nodeMaintCheckID = "_node_maintenenace" diff --git a/command/agent/command.go b/command/agent/command.go index 3368918776..96b2a0ef9d 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -295,11 +295,15 @@ func (c *Command) setupAgent(config *Config, logOutput io.Writer, logWriter *log return err } - // Error if we are trying to bind a domain socket to an existing path - if path, ok := unixSocketAddr(config.Addresses.RPC); ok { - if _, err := os.Stat(path); err == nil || !os.IsNotExist(err) { - c.Ui.Output(fmt.Sprintf(errSocketFileExists, path)) - return fmt.Errorf(errSocketFileExists, path) + // Clear the domain socket file if it exists + socketPath, isSocket := unixSocketAddr(config.Addresses.RPC) + if isSocket { + if _, err := os.Stat(socketPath); !os.IsNotExist(err) { + agent.logger.Printf("[WARN] agent: Replacing socket %q", socketPath) + } + if err := os.Remove(socketPath); err != nil && !os.IsNotExist(err) { + c.Ui.Output(fmt.Sprintf("Error removing socket file: %s", err)) + return err } } @@ -310,6 +314,15 @@ func (c *Command) setupAgent(config *Config, logOutput io.Writer, logWriter *log return err } + // Set up ownership/permission bits on the socket file + if isSocket { + if err := setFilePermissions(socketPath, config.UnixSockets); err != nil { + agent.Shutdown() + c.Ui.Error(fmt.Sprintf("Error setting up socket: %s", err)) + return err + } + } + // Start the IPC layer c.Ui.Output("Starting Consul agent RPC...") c.rpcServer = NewAgentRPC(agent, rpcListener, logOutput, logWriter) diff --git a/command/agent/config.go b/command/agent/config.go index 92b9e64d37..28fb461378 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -343,6 +343,9 @@ type Config struct { // WatchPlans contains the compiled watches WatchPlans []*watch.WatchPlan `mapstructure:"-" json:"-"` + + // UnixSockets is a map of socket configuration data + UnixSockets map[string]string `mapstructure:"unix_sockets"` } // unixSocketAddr tests if a given address describes a domain socket, @@ -893,6 +896,15 @@ func MergeConfig(a, b *Config) *Config { } } + if len(b.UnixSockets) != 0 { + if result.UnixSockets == nil { + result.UnixSockets = make(map[string]string) + } + for field, value := range b.UnixSockets { + result.UnixSockets[field] = value + } + } + // Copy the start join addresses result.StartJoin = make([]string, 0, len(a.StartJoin)+len(b.StartJoin)) result.StartJoin = append(result.StartJoin, a.StartJoin...) diff --git a/command/agent/http.go b/command/agent/http.go index 23e4163048..0d736734c6 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -96,9 +96,13 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS } // Error if we are trying to bind a domain socket to an existing path - if path, ok := unixSocketAddr(config.Addresses.HTTP); ok { - if _, err := os.Stat(path); err == nil || !os.IsNotExist(err) { - return nil, fmt.Errorf(errSocketFileExists, path) + socketPath, isSocket := unixSocketAddr(config.Addresses.HTTP) + if isSocket { + if _, err := os.Stat(socketPath); !os.IsNotExist(err) { + agent.logger.Printf("[WARN] agent: Replacing socket %q", socketPath) + } + if err := os.Remove(socketPath); err != nil && !os.IsNotExist(err) { + return nil, fmt.Errorf("error removing socket file: %s", err) } } @@ -113,6 +117,13 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS list = tcpKeepAliveListener{ln.(*net.TCPListener)} } + // Set up ownership/permission bits on the socket file + if isSocket { + if err := setFilePermissions(socketPath, config.UnixSockets); err != nil { + return nil, fmt.Errorf("Failed setting up HTTP socket: %s", err) + } + } + // Create the mux mux := http.NewServeMux() diff --git a/command/agent/util.go b/command/agent/util.go index d8cb465299..a9c897d482 100644 --- a/command/agent/util.go +++ b/command/agent/util.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "runtime" + "strconv" "time" "github.com/hashicorp/go-msgpack/codec" @@ -97,3 +98,43 @@ func encodeMsgPack(msg interface{}) ([]byte, error) { func stringHash(s string) string { return fmt.Sprintf("%x", md5.Sum([]byte(s))) } + +// setFilePermissions handles configuring ownership and permissions settings +// on a given file. It takes a map, which defines the permissions to be set. +// All permission/ownership settings are optional. If no user or group is +// specified, the current user/group will be used. Mode is optional, and has +// no default (the operation is not performed if absent). +func setFilePermissions(path string, perms map[string]string) error { + var err error + + uid, gid := os.Getuid(), os.Getgid() + if _, ok := perms["uid"]; ok { + if uid, err = strconv.Atoi(perms["uid"]); err != nil { + return fmt.Errorf("invalid user id specified: %v", perms["uid"]) + } + } + if _, ok := perms["gid"]; ok { + if gid, err = strconv.Atoi(perms["gid"]); err != nil { + return fmt.Errorf("invalid group id specified: %v", perms["gid"]) + } + } + if err := os.Chown(path, uid, gid); err != nil { + return fmt.Errorf( + "failed setting ownership to %d:%d on %q: %s", + uid, gid, path, err) + } + + if _, ok := perms["mode"]; ok { + mode, err := strconv.ParseUint(perms["mode"], 8, 32) + if err != nil { + return fmt.Errorf("invalid mode specified for %q: %s", + path, perms["mode"]) + } + if err := os.Chmod(path, os.FileMode(mode)); err != nil { + return fmt.Errorf("failed setting permissions to %d on %q: %s", + mode, path, err) + } + } + + return nil +} From 450d05575d60d149da3fb1f7485aa7437f676ed2 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 20 Jan 2015 14:13:36 -0800 Subject: [PATCH 02/10] agent: adjusting tests for new behavior of sockets --- command/agent/command_test.go | 23 ++++++++------ command/agent/http_test.go | 16 +++++++--- command/agent/util_test.go | 58 +++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 15 deletions(-) diff --git a/command/agent/command_test.go b/command/agent/command_test.go index bcd6be807c..d365b338ed 100644 --- a/command/agent/command_test.go +++ b/command/agent/command_test.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "log" "os" - "strings" "testing" "github.com/hashicorp/consul/testutil" @@ -166,7 +165,7 @@ func TestRetryJoinWanFail(t *testing.T) { } } -func TestSetupAgent_UnixSocket_Fails(t *testing.T) { +func TestSetupAgent_RPCUnixSocket_FileExists(t *testing.T) { conf := nextConfig() tmpDir, err := ioutil.TempDir("", "consul") if err != nil { @@ -185,8 +184,7 @@ func TestSetupAgent_UnixSocket_Fails(t *testing.T) { conf.Server = true conf.Bootstrap = true - // Set socket address to an existing file. Consul should fail to - // start and return an error. + // Set socket address to an existing file. conf.Addresses.RPC = "unix://" + socketPath shutdownCh := make(chan struct{}) @@ -200,12 +198,17 @@ func TestSetupAgent_UnixSocket_Fails(t *testing.T) { logWriter := NewLogWriter(512) logOutput := new(bytes.Buffer) - // Ensure we got an error mentioning the socket file - err = cmd.setupAgent(conf, logOutput, logWriter) - if err == nil { - t.Fatalf("should have failed") + // Ensure the server is created + if err := cmd.setupAgent(conf, logOutput, logWriter); err != nil { + t.Fatalf("err: %s", err) } - if !strings.Contains(err.Error(), socketPath) { - t.Fatalf("expected socket file error, got: %q", err) + + // Ensure the file was replaced by the socket + fi, err := os.Stat(socketPath) + if err != nil { + t.Fatalf("err: %s", err) + } + if fi.Mode()&os.ModeSocket == 0 { + t.Fatalf("expected socket to replace file") } } diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 4677d5ba53..0f723ffd62 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -132,11 +132,17 @@ func TestHTTPServer_UnixSocket_FileExists(t *testing.T) { defer os.RemoveAll(dir) // Try to start the server with the same path anyways. - if servers, err := NewHTTPServers(agent, conf, agent.logOutput); err == nil { - for _, server := range servers { - server.Shutdown() - } - t.Fatalf("expected socket binding error") + if _, err := NewHTTPServers(agent, conf, agent.logOutput); err != nil { + t.Fatalf("err: %s", err) + } + + // Ensure the file was replaced by the socket + fi, err = os.Stat(socket) + if err != nil { + t.Fatalf("err: %s", err) + } + if fi.Mode()&os.ModeSocket == 0 { + t.Fatalf("expected socket to replace file") } } diff --git a/command/agent/util_test.go b/command/agent/util_test.go index 92b89d26f4..b140f477d2 100644 --- a/command/agent/util_test.go +++ b/command/agent/util_test.go @@ -1,6 +1,8 @@ package agent import ( + "io/ioutil" + "os" "testing" "time" ) @@ -39,3 +41,59 @@ func TestStringHash(t *testing.T) { t.Fatalf("bad: %s", out) } } + +func TestSetFilePermissions(t *testing.T) { + tempFile, err := ioutil.TempFile("", "consul") + if err != nil { + t.Fatalf("err: %s", err) + } + path := tempFile.Name() + defer os.Remove(path) + + // Bad UID fails + if err := setFilePermissions(path, map[string]string{"uid": "%"}); err == nil { + t.Fatalf("should fail") + } + + // Bad GID fails + if err := setFilePermissions(path, map[string]string{"gid": "%"}); err == nil { + t.Fatalf("should fail") + } + + // Bad mode fails + if err := setFilePermissions(path, map[string]string{"mode": "%"}); err == nil { + t.Fatalf("should fail") + } + + // Allows omitting user/group/mode + if err := setFilePermissions(path, map[string]string{}); err != nil { + t.Fatalf("err: %s", err) + } + + // Doesn't change mode if not given + if err := os.Chmod(path, 0700); err != nil { + t.Fatalf("err: %s", err) + } + if err := setFilePermissions(path, map[string]string{}); err != nil { + t.Fatalf("err: %s", err) + } + fi, err := os.Stat(path) + if err != nil { + t.Fatalf("err: %s", err) + } + if fi.Mode().String() != "-rwx------" { + t.Fatalf("bad: %s", fi.Mode()) + } + + // Changes mode if given + if err := setFilePermissions(path, map[string]string{"mode": "0777"}); err != nil { + t.Fatalf("err: %s", err) + } + fi, err = os.Stat(path) + if err != nil { + t.Fatalf("err: %s", err) + } + if fi.Mode().String() != "-rwxrwxrwx" { + t.Fatalf("bad: %s", fi.Mode()) + } +} From 145c56b47d1f30e9f645ec9b176de6c020f07f65 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 20 Jan 2015 14:32:15 -0800 Subject: [PATCH 03/10] agent: test Unix domain socket permission settings --- command/agent/config_test.go | 20 ++++++++++++++++++++ command/agent/http_test.go | 13 +++++++++++++ 2 files changed, 33 insertions(+) diff --git a/command/agent/config_test.go b/command/agent/config_test.go index fa7bf6f274..6b82b74113 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -588,6 +588,21 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("bad: %#v", config) } + // Domain socket permissions + input = `{"unix_sockets": {"uid": "500", "gid": "500", "mode": "0700"}}` + config, err = DecodeConfig(bytes.NewReader([]byte(input))) + if err != nil { + t.Fatalf("err: %s", err) + } + + if !reflect.DeepEqual(config.UnixSockets, map[string]string{ + "uid": "500", + "gid": "500", + "mode": "0700", + }) { + t.Fatalf("bad: %v", config.UnixSockets) + } + // Disable updates input = `{"disable_update_check": true, "disable_anonymous_signature": true}` config, err = DecodeConfig(bytes.NewReader([]byte(input))) @@ -998,6 +1013,11 @@ func TestMergeConfig(t *testing.T) { HTTPAPIResponseHeaders: map[string]string{ "Access-Control-Allow-Origin": "*", }, + UnixSockets: map[string]string{ + "uid": "500", + "gid": "500", + "mode": "0700", + }, } c := MergeConfig(a, b) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 0f723ffd62..f3caeafb4b 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -67,6 +67,10 @@ func TestHTTPServer_UnixSocket(t *testing.T) { dir, srv := makeHTTPServerWithConfig(t, func(c *Config) { c.Addresses.HTTP = "unix://" + socket + + // Only testing mode, since uid/gid might not be settable + // from test environment. + c.UnixSockets = map[string]string{"mode": "0777"} }) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -77,6 +81,15 @@ func TestHTTPServer_UnixSocket(t *testing.T) { t.Fatalf("err: %s", err) } + // Ensure the mode was set properly + fi, err := os.Stat(socket) + if err != nil { + t.Fatalf("err: %s", err) + } + if fi.Mode().String() != "Srwxrwxrwx" { + t.Fatalf("bad permissions: %s", fi.Mode()) + } + // Ensure we can get a response from the socket. path, _ := unixSocketAddr(srv.agent.config.Addresses.HTTP) client := &http.Client{ From a6c877c7ee5c970dfc5c76e167189a60b68c2899 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 20 Jan 2015 16:21:23 -0800 Subject: [PATCH 04/10] agent: re-add support for user name in socket perms --- command/agent/util.go | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/command/agent/util.go b/command/agent/util.go index a9c897d482..67fd81c3e2 100644 --- a/command/agent/util.go +++ b/command/agent/util.go @@ -9,6 +9,7 @@ import ( "math/rand" "os" "os/exec" + "os/user" "runtime" "strconv" "time" @@ -103,32 +104,41 @@ func stringHash(s string) string { // on a given file. It takes a map, which defines the permissions to be set. // All permission/ownership settings are optional. If no user or group is // specified, the current user/group will be used. Mode is optional, and has -// no default (the operation is not performed if absent). +// no default (the operation is not performed if absent). User may be +// specified by name or ID, but group may only be specified by ID. func setFilePermissions(path string, perms map[string]string) error { var err error - uid, gid := os.Getuid(), os.Getgid() - if _, ok := perms["uid"]; ok { - if uid, err = strconv.Atoi(perms["uid"]); err != nil { - return fmt.Errorf("invalid user id specified: %v", perms["uid"]) + + if _, ok := perms["user"]; ok { + if uid, err = strconv.Atoi(perms["user"]); err == nil { + goto GROUP } + + // Try looking up the user by name + if u, err := user.Lookup(perms["user"]); err == nil { + uid, _ = strconv.Atoi(u.Uid) + goto GROUP + } + + return fmt.Errorf("invalid user specified: %v", perms["user"]) } - if _, ok := perms["gid"]; ok { - if gid, err = strconv.Atoi(perms["gid"]); err != nil { - return fmt.Errorf("invalid group id specified: %v", perms["gid"]) + +GROUP: + if _, ok := perms["group"]; ok { + if gid, err = strconv.Atoi(perms["group"]); err != nil { + return fmt.Errorf("invalid group specified: %v", perms["group"]) } } if err := os.Chown(path, uid, gid); err != nil { - return fmt.Errorf( - "failed setting ownership to %d:%d on %q: %s", + return fmt.Errorf("failed setting ownership to %d:%d on %q: %s", uid, gid, path, err) } if _, ok := perms["mode"]; ok { mode, err := strconv.ParseUint(perms["mode"], 8, 32) if err != nil { - return fmt.Errorf("invalid mode specified for %q: %s", - path, perms["mode"]) + return fmt.Errorf("invalid mode specified: %v", perms["mode"]) } if err := os.Chmod(path, os.FileMode(mode)); err != nil { return fmt.Errorf("failed setting permissions to %d on %q: %s", From bae6334c60feee84893b45de2fc6542bba7d6bd7 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 20 Jan 2015 16:29:22 -0800 Subject: [PATCH 05/10] website: document unix_sockets config --- website/source/docs/agent/options.html.markdown | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown index 2ecc5e2db7..959d41d511 100644 --- a/website/source/docs/agent/options.html.markdown +++ b/website/source/docs/agent/options.html.markdown @@ -244,8 +244,10 @@ definitions support being updated during a reload. Both `rpc` and `http` support binding to Unix domain sockets. A socket can be specified in the form `unix:///path/to/socket`. A new domain socket will be created at the given path. If the specified file path already exists, Consul - will refuse to start and return an error. For information on how to secure - socket file permissions, refer to the manual page for your operating system. + will attempt to clear the file and create the domain socket in its place. +

+ The permissions of the socket file are tunable via the `unix_sockets` config + construct.

When running Consul agent commands against Unix socket interfaces, use the `-rpc-addr` or `-http-addr` arguments to specify the path to the socket. You @@ -429,6 +431,17 @@ definitions support being updated during a reload. * `ui_dir` - Equivalent to the `-ui-dir` command-line flag. +* `unix_sockets` - This allows tuning the ownership and permissions of the + Unix domain socket files created by Consul. Domain sockets are only used if + the HTTP or RPC addresses are configured with the `unix://` prefix. The + following options are valid within this construct, and apply globally to all + sockets created by Consul: +
+ * `user` - The name or ID of the user who will own the socket file. + * `group` - The group ID ownership of the socket file. Note that this option + currently only supports numeric ID's. + * `mode` - The permission bits to set on the file. + * `verify_incoming` - If set to True, Consul requires that all incoming connections make use of TLS, and that the client provides a certificate signed by the Certificate Authority from the `ca_file`. By default, this is false, and From 99d4e7831ed7e65341d32ebb94949a1549999f01 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 20 Jan 2015 16:50:21 -0800 Subject: [PATCH 06/10] agent: fix tests --- command/agent/config_test.go | 14 +++++++------- command/agent/util_test.go | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 6b82b74113..bab0a645d2 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -589,16 +589,16 @@ func TestDecodeConfig(t *testing.T) { } // Domain socket permissions - input = `{"unix_sockets": {"uid": "500", "gid": "500", "mode": "0700"}}` + input = `{"unix_sockets": {"user": "500", "group": "500", "mode": "0700"}}` config, err = DecodeConfig(bytes.NewReader([]byte(input))) if err != nil { t.Fatalf("err: %s", err) } if !reflect.DeepEqual(config.UnixSockets, map[string]string{ - "uid": "500", - "gid": "500", - "mode": "0700", + "user": "500", + "group": "500", + "mode": "0700", }) { t.Fatalf("bad: %v", config.UnixSockets) } @@ -1014,9 +1014,9 @@ func TestMergeConfig(t *testing.T) { "Access-Control-Allow-Origin": "*", }, UnixSockets: map[string]string{ - "uid": "500", - "gid": "500", - "mode": "0700", + "user": "500", + "group": "500", + "mode": "0700", }, } diff --git a/command/agent/util_test.go b/command/agent/util_test.go index b140f477d2..f6d1b35aaa 100644 --- a/command/agent/util_test.go +++ b/command/agent/util_test.go @@ -51,12 +51,12 @@ func TestSetFilePermissions(t *testing.T) { defer os.Remove(path) // Bad UID fails - if err := setFilePermissions(path, map[string]string{"uid": "%"}); err == nil { + if err := setFilePermissions(path, map[string]string{"user": "%"}); err == nil { t.Fatalf("should fail") } // Bad GID fails - if err := setFilePermissions(path, map[string]string{"gid": "%"}); err == nil { + if err := setFilePermissions(path, map[string]string{"group": "%"}); err == nil { t.Fatalf("should fail") } From 782b0ddd880f7c44ceead841008baf3a26b12f6b Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 20 Jan 2015 16:57:00 -0800 Subject: [PATCH 07/10] agent: test permissions are set on rpc socket --- command/agent/command_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/command/agent/command_test.go b/command/agent/command_test.go index d365b338ed..94f7d845b5 100644 --- a/command/agent/command_test.go +++ b/command/agent/command_test.go @@ -187,6 +187,9 @@ func TestSetupAgent_RPCUnixSocket_FileExists(t *testing.T) { // Set socket address to an existing file. conf.Addresses.RPC = "unix://" + socketPath + // Custom mode for socket file + conf.UnixSockets = map[string]string{"mode": "0777"} + shutdownCh := make(chan struct{}) defer close(shutdownCh) @@ -211,4 +214,9 @@ func TestSetupAgent_RPCUnixSocket_FileExists(t *testing.T) { if fi.Mode()&os.ModeSocket == 0 { t.Fatalf("expected socket to replace file") } + + // Ensure permissions were applied to the socket file + if fi.Mode().String() != "Srwxrwxrwx" { + t.Fatalf("bad permissions: %s", fi.Mode()) + } } From b1dae530d4b639b54035836603331959423dce1c Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 20 Jan 2015 18:53:18 -0800 Subject: [PATCH 08/10] agent: use interface for file permissions --- command/agent/command_test.go | 2 +- command/agent/config.go | 40 +++++++++++++++++++++++-------- command/agent/config_test.go | 22 +++++++++-------- command/agent/http_test.go | 2 +- command/agent/util.go | 45 ++++++++++++++++++++++------------- command/agent/util_test.go | 12 +++++----- 6 files changed, 79 insertions(+), 44 deletions(-) diff --git a/command/agent/command_test.go b/command/agent/command_test.go index 94f7d845b5..58129bba92 100644 --- a/command/agent/command_test.go +++ b/command/agent/command_test.go @@ -188,7 +188,7 @@ func TestSetupAgent_RPCUnixSocket_FileExists(t *testing.T) { conf.Addresses.RPC = "unix://" + socketPath // Custom mode for socket file - conf.UnixSockets = map[string]string{"mode": "0777"} + conf.UnixSockets.Perms = "0777" shutdownCh := make(chan struct{}) defer close(shutdownCh) diff --git a/command/agent/config.go b/command/agent/config.go index 28fb461378..163fa18712 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -345,7 +345,27 @@ type Config struct { WatchPlans []*watch.WatchPlan `mapstructure:"-" json:"-"` // UnixSockets is a map of socket configuration data - UnixSockets map[string]string `mapstructure:"unix_sockets"` + UnixSockets UnixSocketConfig `mapstructure:"unix_sockets"` +} + +// UnixSocketConfig contains information about a unix socket, and +// implements the FilePermissions interface. +type UnixSocketConfig struct { + Usr string `mapstructure:"user"` + Grp string `mapstructure:"group"` + Perms string `mapstructure:"mode"` +} + +func (u UnixSocketConfig) User() string { + return u.Usr +} + +func (u UnixSocketConfig) Group() string { + return u.Grp +} + +func (u UnixSocketConfig) Mode() string { + return u.Perms } // unixSocketAddr tests if a given address describes a domain socket, @@ -886,6 +906,15 @@ func MergeConfig(a, b *Config) *Config { if b.DisableAnonymousSignature { result.DisableAnonymousSignature = true } + if b.UnixSockets.Usr != "" { + result.UnixSockets.Usr = b.UnixSockets.Usr + } + if b.UnixSockets.Grp != "" { + result.UnixSockets.Grp = b.UnixSockets.Grp + } + if b.UnixSockets.Perms != "" { + result.UnixSockets.Perms = b.UnixSockets.Perms + } if len(b.HTTPAPIResponseHeaders) != 0 { if result.HTTPAPIResponseHeaders == nil { @@ -896,15 +925,6 @@ func MergeConfig(a, b *Config) *Config { } } - if len(b.UnixSockets) != 0 { - if result.UnixSockets == nil { - result.UnixSockets = make(map[string]string) - } - for field, value := range b.UnixSockets { - result.UnixSockets[field] = value - } - } - // Copy the start join addresses result.StartJoin = make([]string, 0, len(a.StartJoin)+len(b.StartJoin)) result.StartJoin = append(result.StartJoin, a.StartJoin...) diff --git a/command/agent/config_test.go b/command/agent/config_test.go index bab0a645d2..afdb245233 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -595,12 +595,14 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("err: %s", err) } - if !reflect.DeepEqual(config.UnixSockets, map[string]string{ - "user": "500", - "group": "500", - "mode": "0700", - }) { - t.Fatalf("bad: %v", config.UnixSockets) + if config.UnixSockets.Usr != "500" { + t.Fatalf("bad: %#v", config) + } + if config.UnixSockets.Grp != "500" { + t.Fatalf("bad: %#v", config) + } + if config.UnixSockets.Perms != "0700" { + t.Fatalf("bad: %#v", config) } // Disable updates @@ -1013,10 +1015,10 @@ func TestMergeConfig(t *testing.T) { HTTPAPIResponseHeaders: map[string]string{ "Access-Control-Allow-Origin": "*", }, - UnixSockets: map[string]string{ - "user": "500", - "group": "500", - "mode": "0700", + UnixSockets: UnixSocketConfig{ + Usr: "500", + Grp: "500", + Perms: "0700", }, } diff --git a/command/agent/http_test.go b/command/agent/http_test.go index f3caeafb4b..67d597ed21 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -70,7 +70,7 @@ func TestHTTPServer_UnixSocket(t *testing.T) { // Only testing mode, since uid/gid might not be settable // from test environment. - c.UnixSockets = map[string]string{"mode": "0777"} + c.UnixSockets = UnixSocketConfig{Perms: "0777"} }) defer os.RemoveAll(dir) defer srv.Shutdown() diff --git a/command/agent/util.go b/command/agent/util.go index 67fd81c3e2..5f0bcff511 100644 --- a/command/agent/util.go +++ b/command/agent/util.go @@ -100,34 +100,47 @@ func stringHash(s string) string { return fmt.Sprintf("%x", md5.Sum([]byte(s))) } +// FilePermissions is an interface which allows a struct to set +// ownership and permissions easily on a file it describes. +type FilePermissions interface { + // User returns a user ID or user name + User() string + + // Group returns a group ID. Group names are not supported. + Group() string + + // Mode returns a string of file mode bits e.g. "0644" + Mode() string +} + // setFilePermissions handles configuring ownership and permissions settings -// on a given file. It takes a map, which defines the permissions to be set. -// All permission/ownership settings are optional. If no user or group is -// specified, the current user/group will be used. Mode is optional, and has -// no default (the operation is not performed if absent). User may be -// specified by name or ID, but group may only be specified by ID. -func setFilePermissions(path string, perms map[string]string) error { +// on a given file. It takes a path and any struct implementing the +// FilePermissions interface. All permission/ownership settings are optional. +// If no user or group is specified, the current user/group will be used. Mode +// is optional, and has no default (the operation is not performed if absent). +// User may be specified by name or ID, but group may only be specified by ID. +func setFilePermissions(path string, p FilePermissions) error { var err error uid, gid := os.Getuid(), os.Getgid() - if _, ok := perms["user"]; ok { - if uid, err = strconv.Atoi(perms["user"]); err == nil { + if p.User() != "" { + if uid, err = strconv.Atoi(p.User()); err == nil { goto GROUP } // Try looking up the user by name - if u, err := user.Lookup(perms["user"]); err == nil { + if u, err := user.Lookup(p.User()); err == nil { uid, _ = strconv.Atoi(u.Uid) goto GROUP } - return fmt.Errorf("invalid user specified: %v", perms["user"]) + return fmt.Errorf("invalid user specified: %v", p.User()) } GROUP: - if _, ok := perms["group"]; ok { - if gid, err = strconv.Atoi(perms["group"]); err != nil { - return fmt.Errorf("invalid group specified: %v", perms["group"]) + if p.Group() != "" { + if gid, err = strconv.Atoi(p.Group()); err != nil { + return fmt.Errorf("invalid group specified: %v", p.Group()) } } if err := os.Chown(path, uid, gid); err != nil { @@ -135,10 +148,10 @@ GROUP: uid, gid, path, err) } - if _, ok := perms["mode"]; ok { - mode, err := strconv.ParseUint(perms["mode"], 8, 32) + if p.Mode() != "" { + mode, err := strconv.ParseUint(p.Mode(), 8, 32) if err != nil { - return fmt.Errorf("invalid mode specified: %v", perms["mode"]) + return fmt.Errorf("invalid mode specified: %v", p.Mode()) } if err := os.Chmod(path, os.FileMode(mode)); err != nil { return fmt.Errorf("failed setting permissions to %d on %q: %s", diff --git a/command/agent/util_test.go b/command/agent/util_test.go index f6d1b35aaa..df5dba3fad 100644 --- a/command/agent/util_test.go +++ b/command/agent/util_test.go @@ -51,22 +51,22 @@ func TestSetFilePermissions(t *testing.T) { defer os.Remove(path) // Bad UID fails - if err := setFilePermissions(path, map[string]string{"user": "%"}); err == nil { + if err := setFilePermissions(path, UnixSocketConfig{Usr: "%"}); err == nil { t.Fatalf("should fail") } // Bad GID fails - if err := setFilePermissions(path, map[string]string{"group": "%"}); err == nil { + if err := setFilePermissions(path, UnixSocketConfig{Grp: "%"}); err == nil { t.Fatalf("should fail") } // Bad mode fails - if err := setFilePermissions(path, map[string]string{"mode": "%"}); err == nil { + if err := setFilePermissions(path, UnixSocketConfig{Perms: "%"}); err == nil { t.Fatalf("should fail") } // Allows omitting user/group/mode - if err := setFilePermissions(path, map[string]string{}); err != nil { + if err := setFilePermissions(path, UnixSocketConfig{}); err != nil { t.Fatalf("err: %s", err) } @@ -74,7 +74,7 @@ func TestSetFilePermissions(t *testing.T) { if err := os.Chmod(path, 0700); err != nil { t.Fatalf("err: %s", err) } - if err := setFilePermissions(path, map[string]string{}); err != nil { + if err := setFilePermissions(path, UnixSocketConfig{}); err != nil { t.Fatalf("err: %s", err) } fi, err := os.Stat(path) @@ -86,7 +86,7 @@ func TestSetFilePermissions(t *testing.T) { } // Changes mode if given - if err := setFilePermissions(path, map[string]string{"mode": "0777"}); err != nil { + if err := setFilePermissions(path, UnixSocketConfig{Perms: "0777"}); err != nil { t.Fatalf("err: %s", err) } fi, err = os.Stat(path) From 5a8703bb00a717b2fbfff0d1bd9dec4c3fcef1cf Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 20 Jan 2015 19:16:12 -0800 Subject: [PATCH 09/10] website: add notice about socket permission portability --- website/source/docs/agent/options.html.markdown | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown index 959d41d511..98399cca4d 100644 --- a/website/source/docs/agent/options.html.markdown +++ b/website/source/docs/agent/options.html.markdown @@ -441,6 +441,12 @@ definitions support being updated during a reload. * `group` - The group ID ownership of the socket file. Note that this option currently only supports numeric ID's. * `mode` - The permission bits to set on the file. +
+ It is important to note that this option may have different effects on + different operating systems. Linux generally observes socket file permissions, + while many BSD variants ignore permissions on the socket file itself. It is + important to test this feature on your specific distribution. This feature is + currently not functional on Windows hosts. * `verify_incoming` - If set to True, Consul requires that all incoming connections make use of TLS, and that the client provides a certificate signed From c958824bb65d2d627271fb4015f5926970957287 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 20 Jan 2015 20:30:59 -0800 Subject: [PATCH 10/10] agent: use squash mapstructure tag to properly decode embedded structs --- command/agent/config.go | 16 +++++++++++----- command/agent/config_test.go | 8 +++++--- command/agent/http_test.go | 3 ++- command/agent/util_test.go | 12 ++++++------ 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/command/agent/config.go b/command/agent/config.go index 163fa18712..da444ab5cd 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -348,26 +348,32 @@ type Config struct { UnixSockets UnixSocketConfig `mapstructure:"unix_sockets"` } -// UnixSocketConfig contains information about a unix socket, and +// UnixSocketPermissions contains information about a unix socket, and // implements the FilePermissions interface. -type UnixSocketConfig struct { +type UnixSocketPermissions struct { Usr string `mapstructure:"user"` Grp string `mapstructure:"group"` Perms string `mapstructure:"mode"` } -func (u UnixSocketConfig) User() string { +func (u UnixSocketPermissions) User() string { return u.Usr } -func (u UnixSocketConfig) Group() string { +func (u UnixSocketPermissions) Group() string { return u.Grp } -func (u UnixSocketConfig) Mode() string { +func (u UnixSocketPermissions) Mode() string { return u.Perms } +// UnixSocketConfig stores information about various unix sockets which +// Consul creates and uses for communication. +type UnixSocketConfig struct { + UnixSocketPermissions `mapstructure:",squash"` +} + // unixSocketAddr tests if a given address describes a domain socket, // and returns the relevant path part of the string if it is. func unixSocketAddr(addr string) (string, bool) { diff --git a/command/agent/config_test.go b/command/agent/config_test.go index afdb245233..3d4d4b7404 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -1016,9 +1016,11 @@ func TestMergeConfig(t *testing.T) { "Access-Control-Allow-Origin": "*", }, UnixSockets: UnixSocketConfig{ - Usr: "500", - Grp: "500", - Perms: "0700", + UnixSocketPermissions{ + Usr: "500", + Grp: "500", + Perms: "0700", + }, }, } diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 67d597ed21..19e8f95af9 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -70,7 +70,8 @@ func TestHTTPServer_UnixSocket(t *testing.T) { // Only testing mode, since uid/gid might not be settable // from test environment. - c.UnixSockets = UnixSocketConfig{Perms: "0777"} + c.UnixSockets = UnixSocketConfig{} + c.UnixSockets.Perms = "0777" }) defer os.RemoveAll(dir) defer srv.Shutdown() diff --git a/command/agent/util_test.go b/command/agent/util_test.go index df5dba3fad..ab47c5e0f0 100644 --- a/command/agent/util_test.go +++ b/command/agent/util_test.go @@ -51,22 +51,22 @@ func TestSetFilePermissions(t *testing.T) { defer os.Remove(path) // Bad UID fails - if err := setFilePermissions(path, UnixSocketConfig{Usr: "%"}); err == nil { + if err := setFilePermissions(path, UnixSocketPermissions{Usr: "%"}); err == nil { t.Fatalf("should fail") } // Bad GID fails - if err := setFilePermissions(path, UnixSocketConfig{Grp: "%"}); err == nil { + if err := setFilePermissions(path, UnixSocketPermissions{Grp: "%"}); err == nil { t.Fatalf("should fail") } // Bad mode fails - if err := setFilePermissions(path, UnixSocketConfig{Perms: "%"}); err == nil { + if err := setFilePermissions(path, UnixSocketPermissions{Perms: "%"}); err == nil { t.Fatalf("should fail") } // Allows omitting user/group/mode - if err := setFilePermissions(path, UnixSocketConfig{}); err != nil { + if err := setFilePermissions(path, UnixSocketPermissions{}); err != nil { t.Fatalf("err: %s", err) } @@ -74,7 +74,7 @@ func TestSetFilePermissions(t *testing.T) { if err := os.Chmod(path, 0700); err != nil { t.Fatalf("err: %s", err) } - if err := setFilePermissions(path, UnixSocketConfig{}); err != nil { + if err := setFilePermissions(path, UnixSocketPermissions{}); err != nil { t.Fatalf("err: %s", err) } fi, err := os.Stat(path) @@ -86,7 +86,7 @@ func TestSetFilePermissions(t *testing.T) { } // Changes mode if given - if err := setFilePermissions(path, UnixSocketConfig{Perms: "0777"}); err != nil { + if err := setFilePermissions(path, UnixSocketPermissions{Perms: "0777"}); err != nil { t.Fatalf("err: %s", err) } fi, err = os.Stat(path)