diff --git a/command/agent/agent.go b/command/agent/agent.go index 5ec4409f1d..0e54a37472 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -24,13 +24,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_maintenance" 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/command_test.go b/command/agent/command_test.go index bcd6be807c..58129bba92 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,10 +184,12 @@ 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 + // Custom mode for socket file + conf.UnixSockets.Perms = "0777" + shutdownCh := make(chan struct{}) defer close(shutdownCh) @@ -200,12 +201,22 @@ 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") + } + + // Ensure permissions were applied to the socket file + if fi.Mode().String() != "Srwxrwxrwx" { + t.Fatalf("bad permissions: %s", fi.Mode()) } } diff --git a/command/agent/config.go b/command/agent/config.go index 2e3ea2eedc..208daf5640 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -343,6 +343,35 @@ type Config struct { // WatchPlans contains the compiled watches WatchPlans []*watch.WatchPlan `mapstructure:"-" json:"-"` + + // UnixSockets is a map of socket configuration data + UnixSockets UnixSocketConfig `mapstructure:"unix_sockets"` +} + +// UnixSocketPermissions contains information about a unix socket, and +// implements the FilePermissions interface. +type UnixSocketPermissions struct { + Usr string `mapstructure:"user"` + Grp string `mapstructure:"group"` + Perms string `mapstructure:"mode"` +} + +func (u UnixSocketPermissions) User() string { + return u.Usr +} + +func (u UnixSocketPermissions) Group() string { + return u.Grp +} + +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, @@ -889,6 +918,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 { diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 3647da2d83..ee436dc7d0 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -588,6 +588,23 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("bad: %#v", config) } + // Domain socket permissions + 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 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 input = `{"disable_update_check": true, "disable_anonymous_signature": true}` config, err = DecodeConfig(bytes.NewReader([]byte(input))) @@ -1034,6 +1051,13 @@ func TestMergeConfig(t *testing.T) { HTTPAPIResponseHeaders: map[string]string{ "Access-Control-Allow-Origin": "*", }, + UnixSockets: UnixSocketConfig{ + UnixSocketPermissions{ + Usr: "500", + Grp: "500", + Perms: "0700", + }, + }, } c := MergeConfig(a, b) diff --git a/command/agent/http.go b/command/agent/http.go index 78aaaa4f17..364be857c4 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/http_test.go b/command/agent/http_test.go index 4677d5ba53..19e8f95af9 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -67,6 +67,11 @@ 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 = UnixSocketConfig{} + c.UnixSockets.Perms = "0777" }) defer os.RemoveAll(dir) defer srv.Shutdown() @@ -77,6 +82,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{ @@ -132,11 +146,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.go b/command/agent/util.go index d8cb465299..5f0bcff511 100644 --- a/command/agent/util.go +++ b/command/agent/util.go @@ -9,7 +9,9 @@ import ( "math/rand" "os" "os/exec" + "os/user" "runtime" + "strconv" "time" "github.com/hashicorp/go-msgpack/codec" @@ -97,3 +99,65 @@ func encodeMsgPack(msg interface{}) ([]byte, error) { 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 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 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(p.User()); err == nil { + uid, _ = strconv.Atoi(u.Uid) + goto GROUP + } + + return fmt.Errorf("invalid user specified: %v", p.User()) + } + +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 { + return fmt.Errorf("failed setting ownership to %d:%d on %q: %s", + uid, gid, path, err) + } + + if p.Mode() != "" { + mode, err := strconv.ParseUint(p.Mode(), 8, 32) + if err != nil { + 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", + mode, path, err) + } + } + + return nil +} diff --git a/command/agent/util_test.go b/command/agent/util_test.go index 92b89d26f4..ab47c5e0f0 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, UnixSocketPermissions{Usr: "%"}); err == nil { + t.Fatalf("should fail") + } + + // Bad GID fails + if err := setFilePermissions(path, UnixSocketPermissions{Grp: "%"}); err == nil { + t.Fatalf("should fail") + } + + // Bad mode fails + if err := setFilePermissions(path, UnixSocketPermissions{Perms: "%"}); err == nil { + t.Fatalf("should fail") + } + + // Allows omitting user/group/mode + if err := setFilePermissions(path, UnixSocketPermissions{}); 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, UnixSocketPermissions{}); 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, UnixSocketPermissions{Perms: "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()) + } +} diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown index 2ecc5e2db7..98399cca4d 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,23 @@ 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. +
+ 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 by the Certificate Authority from the `ca_file`. By default, this is false, and