From c735589f415b7a55c4cdc171cb6a4b0de9488721 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Fri, 4 Nov 2016 15:24:28 -0700 Subject: [PATCH] Choose safer default advertise address * -dev mode defaults bind & advertise to localhost * Normal mode defaults bind to 0.0.0.0 & advertise to the resolved hostname. If the hostname resolves to localhost it will refuse to start and advertise must be manually set. --- command/agent/agent.go | 164 ++++++--------------------------------- command/agent/command.go | 74 +++++++++++++++++- command/agent/config.go | 3 +- command/agent/http.go | 2 +- 4 files changed, 97 insertions(+), 146 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index 1b5c19e8c..4d1da5e1a 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -11,7 +11,6 @@ import ( "strconv" "strings" "sync" - "sync/atomic" "time" "github.com/hashicorp/nomad/client" @@ -45,13 +44,9 @@ type Agent struct { // consulSyncer registers the Nomad agent with the Consul Agent consulSyncer *consul.Syncer - client *client.Client - clientHTTPAddr string + client *client.Client - server *nomad.Server - serverHTTPAddr string - serverRPCAddr string - serverSerfAddr string + server *nomad.Server shutdown bool shutdownCh chan struct{} @@ -115,7 +110,7 @@ func (a *Agent) serverConfig() (*nomad.Config, error) { if a.config.Server.BootstrapExpect == 1 { conf.Bootstrap = true } else { - atomic.StoreInt32(&conf.BootstrapExpect, int32(a.config.Server.BootstrapExpect)) + conf.BootstrapExpect = int32(a.config.Server.BootstrapExpect) } } if a.config.DataDir != "" { @@ -135,11 +130,11 @@ func (a *Agent) serverConfig() (*nomad.Config, error) { } // Set up the bind addresses - rpcAddr, err := a.getRPCAddr(true) + rpcAddr, err := net.ResolveTCPAddr("tcp", a.config.Addresses.RPC) if err != nil { return nil, err } - serfAddr, err := a.getSerfAddr(true) + serfAddr, err := net.ResolveTCPAddr("tcp", a.config.Addresses.Serf) if err != nil { return nil, err } @@ -149,21 +144,14 @@ func (a *Agent) serverConfig() (*nomad.Config, error) { conf.SerfConfig.MemberlistConfig.BindAddr = serfAddr.IP.String() // Set up the advertise addresses - httpAddr, err := a.getHTTPAddr(false) + rpcAddr, err = net.ResolveTCPAddr("tcp", a.config.AdvertiseAddrs.RPC) if err != nil { return nil, err } - rpcAddr, err = a.getRPCAddr(false) + serfAddr, err = net.ResolveTCPAddr("tcp", a.config.AdvertiseAddrs.Serf) if err != nil { return nil, err } - serfAddr, err = a.getSerfAddr(false) - if err != nil { - return nil, err - } - a.serverHTTPAddr = net.JoinHostPort(httpAddr.IP.String(), strconv.Itoa(httpAddr.Port)) - a.serverRPCAddr = net.JoinHostPort(rpcAddr.IP.String(), strconv.Itoa(rpcAddr.Port)) - a.serverSerfAddr = net.JoinHostPort(serfAddr.IP.String(), strconv.Itoa(serfAddr.Port)) conf.RPCAdvertise = rpcAddr conf.SerfConfig.MemberlistConfig.AdvertiseAddr = serfAddr.IP.String() conf.SerfConfig.MemberlistConfig.AdvertisePort = serfAddr.Port @@ -267,12 +255,7 @@ func (a *Agent) clientConfig() (*clientconfig.Config, error) { conf.Node.NodeClass = a.config.Client.NodeClass // Set up the HTTP advertise address - httpAddr, err := a.selectAddr(a.getHTTPAddr, false) - if err != nil { - return nil, err - } - conf.Node.HTTPAddr = httpAddr - a.clientHTTPAddr = httpAddr + conf.Node.HTTPAddr = a.config.Addresses.HTTP // Reserve resources on the node. r := conf.Node.Reserved @@ -330,18 +313,14 @@ func (a *Agent) setupServer() error { } a.server = server - // Resolve consul check addresses. Always use advertise address for services - httpCheckAddr, err := a.selectAddr(a.getHTTPAddr, !a.config.Consul.ChecksUseAdvertise) - if err != nil { - return err - } - rpcCheckAddr, err := a.selectAddr(a.getRPCAddr, !a.config.Consul.ChecksUseAdvertise) - if err != nil { - return err - } - serfCheckAddr, err := a.selectAddr(a.getSerfAddr, !a.config.Consul.ChecksUseAdvertise) - if err != nil { - return err + // Consul check addresses default to bind but can be toggled to use advertise + httpCheckAddr := a.config.Addresses.HTTP + rpcCheckAddr := a.config.Addresses.RPC + serfCheckAddr := a.config.Addresses.Serf + if a.config.Consul.ChecksUseAdvertise { + httpCheckAddr = a.config.AdvertiseAddrs.HTTP + rpcCheckAddr = a.config.AdvertiseAddrs.RPC + serfCheckAddr = a.config.AdvertiseAddrs.Serf } // Create the Nomad Server services for Consul @@ -349,7 +328,7 @@ func (a *Agent) setupServer() error { if a.config.Consul.AutoAdvertise { httpServ := &structs.Service{ Name: a.config.Consul.ServerServiceName, - PortLabel: a.serverHTTPAddr, + PortLabel: a.config.AdvertiseAddrs.HTTP, Tags: []string{consul.ServiceTagHTTP}, Checks: []*structs.ServiceCheck{ &structs.ServiceCheck{ @@ -365,7 +344,7 @@ func (a *Agent) setupServer() error { } rpcServ := &structs.Service{ Name: a.config.Consul.ServerServiceName, - PortLabel: a.serverRPCAddr, + PortLabel: a.config.AdvertiseAddrs.RPC, Tags: []string{consul.ServiceTagRPC}, Checks: []*structs.ServiceCheck{ &structs.ServiceCheck{ @@ -378,8 +357,8 @@ func (a *Agent) setupServer() error { }, } serfServ := &structs.Service{ - PortLabel: a.serverSerfAddr, Name: a.config.Consul.ServerServiceName, + PortLabel: a.config.AdvertiseAddrs.Serf, Tags: []string{consul.ServiceTagSerf}, Checks: []*structs.ServiceCheck{ &structs.ServiceCheck{ @@ -458,9 +437,9 @@ func (a *Agent) setupClient() error { a.client = client // Resolve the http check address - httpCheckAddr, err := a.selectAddr(a.getHTTPAddr, !a.config.Consul.ChecksUseAdvertise) - if err != nil { - return err + httpCheckAddr := a.config.Addresses.HTTP + if a.config.Consul.ChecksUseAdvertise { + httpCheckAddr = a.config.AdvertiseAddrs.HTTP } // Create the Nomad Client services for Consul @@ -469,7 +448,7 @@ func (a *Agent) setupClient() error { if a.config.Consul.AutoAdvertise { httpServ := &structs.Service{ Name: a.config.Consul.ClientServiceName, - PortLabel: a.clientHTTPAddr, + PortLabel: a.config.AdvertiseAddrs.HTTP, Tags: []string{consul.ServiceTagHTTP}, Checks: []*structs.ServiceCheck{ &structs.ServiceCheck{ @@ -493,103 +472,6 @@ func (a *Agent) setupClient() error { return nil } -// Defines the selector interface -type addrSelector func(bool) (*net.TCPAddr, error) - -// selectAddr returns the right address given a selector, and return it as a PortLabel -// preferBind is a weak preference, and will skip 0.0.0.0 -func (a *Agent) selectAddr(selector addrSelector, preferBind bool) (string, error) { - addr, err := selector(preferBind) - if err != nil { - return "", err - } - - if preferBind && addr.IP.String() == "0.0.0.0" { - addr, err = selector(false) - if err != nil { - return "", err - } - } - - address := net.JoinHostPort(addr.IP.String(), strconv.Itoa(addr.Port)) - return address, nil -} - -// getHTTPAddr returns the HTTP address to use based on the clients -// configuration. If bind is true, an address appropriate for binding is -// returned, otherwise an address for advertising is returned. Skip 0.0.0.0 -// unless returning a bind address, since that's the only time it's useful. -func (a *Agent) getHTTPAddr(bind bool) (*net.TCPAddr, error) { - advertAddr := a.config.AdvertiseAddrs.HTTP - bindAddr := a.config.Addresses.HTTP - globalBindAddr := a.config.BindAddr - port := a.config.Ports.HTTP - return pickAddress(bind, globalBindAddr, advertAddr, bindAddr, port, "HTTP") -} - -// getRPCAddr returns the HTTP address to use based on the clients -// configuration. If bind is true, an address appropriate for binding is -// returned, otherwise an address for advertising is returned. Skip 0.0.0.0 -// unless returning a bind address, since that's the only time it's useful. -func (a *Agent) getRPCAddr(bind bool) (*net.TCPAddr, error) { - advertAddr := a.config.AdvertiseAddrs.RPC - bindAddr := a.config.Addresses.RPC - globalBindAddr := a.config.BindAddr - port := a.config.Ports.RPC - return pickAddress(bind, globalBindAddr, advertAddr, bindAddr, port, "RPC") -} - -// getSerfAddr returns the Serf address to use based on the clients -// configuration. If bind is true, an address appropriate for binding is -// returned, otherwise an address for advertising is returned. Skip 0.0.0.0 -// unless returning a bind address, since that's the only time it's useful. -func (a *Agent) getSerfAddr(bind bool) (*net.TCPAddr, error) { - advertAddr := a.config.AdvertiseAddrs.Serf - bindAddr := a.config.Addresses.Serf - globalBindAddr := a.config.BindAddr - port := a.config.Ports.Serf - return pickAddress(bind, globalBindAddr, advertAddr, bindAddr, port, "Serf") -} - -// pickAddress is a shared helper to pick the address to either bind to or -// advertise. -func pickAddress(bind bool, globalBindAddr, advertiseAddr, bindAddr string, port int, service string) (*net.TCPAddr, error) { - var serverAddr string - if advertiseAddr != "" && !bind { - serverAddr = advertiseAddr - - // Check if the advertise has a port - if host, pport, err := net.SplitHostPort(advertiseAddr); err == nil { - if parsed, err := strconv.Atoi(pport); err == nil { - serverAddr = host - port = parsed - } - } - } else if bindAddr != "" && !(bindAddr == "0.0.0.0" && !bind) { - serverAddr = bindAddr - } else if globalBindAddr != "" && !(globalBindAddr == "0.0.0.0" && !bind) { - serverAddr = globalBindAddr - } else { - serverAddr = "127.0.0.1" - } - - ip := net.ParseIP(serverAddr) - if ip == nil { - joined := net.JoinHostPort(serverAddr, strconv.Itoa(port)) - addr, err := net.ResolveTCPAddr("tcp", joined) - if err == nil { - return addr, nil - } - - return nil, fmt.Errorf("Failed to parse %s %q as IP and failed to resolve address: %v", service, serverAddr, err) - } - - return &net.TCPAddr{ - IP: ip, - Port: port, - }, nil -} - // reservePortsForClient reserves a range of ports for the client to use when // it creates various plugins for log collection, executors, drivers, etc func (a *Agent) reservePortsForClient(conf *clientconfig.Config) error { diff --git a/command/agent/command.go b/command/agent/command.go index 9b1348c94..dff1adfc2 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "log" + "net" "os" "os/signal" "path/filepath" @@ -201,9 +202,71 @@ func (c *Command) readConfig() *Config { config.Version = c.Version config.VersionPrerelease = c.VersionPrerelease - if dev { - // Skip validation for dev mode - return config + // Normalize addresses + if config.Addresses.HTTP == "" { + config.Addresses.HTTP = fmt.Sprintf("%s:%d", config.BindAddr, config.Ports.HTTP) + } + if config.Addresses.RPC == "" { + config.Addresses.RPC = fmt.Sprintf("%s:%d", config.BindAddr, config.Ports.RPC) + } + if config.Addresses.Serf == "" { + config.Addresses.Serf = fmt.Sprintf("%s:%d", config.BindAddr, config.Ports.Serf) + } + + // Use getAdvertise(&config.AdvertiseAddrs., ) to + // retrieve a default address for advertising if one isn't set. + defaultAdvertise := "" + getAdvertise := func(addr *string, port int) bool { + if *addr != "" { + // Default to using manually configured address + return true + } + + // Autoconfigure using previously set default + if defaultAdvertise != "" { + newaddr := fmt.Sprintf("%s:%d", defaultAdvertise, port) + *addr = newaddr + return true + } + + // Fallback to Bind address if it's not 0.0.0.0 + if config.BindAddr != "0.0.0.0" { + defaultAdvertise = config.BindAddr + newaddr := fmt.Sprintf("%s:%d", defaultAdvertise, port) + *addr = newaddr + return true + } + + // As a last resort resolve the hostname and use it if it's not + // localhost (as localhost is never a sensible default) + host, err := os.Hostname() + if err != nil { + c.Ui.Error(fmt.Sprintf("Unable to get hostname to set advertise address: %v", err)) + return false + } + ip, err := net.ResolveIPAddr("ip", host) + if err != nil { + c.Ui.Error(fmt.Sprintf("Unable to resolve hostname to set advertise address: %v", err)) + return false + } + if ip.IP.IsLoopback() && !dev { + c.Ui.Error("Unable to select default advertise address as hostname resolves to localhost") + return false + } + defaultAdvertise = ip.IP.String() + newaddr := fmt.Sprintf("%s:%d", defaultAdvertise, port) + *addr = newaddr + return true + } + + if ok := getAdvertise(&config.AdvertiseAddrs.HTTP, config.Ports.HTTP); !ok { + return nil + } + if ok := getAdvertise(&config.AdvertiseAddrs.RPC, config.Ports.RPC); !ok { + return nil + } + if ok := getAdvertise(&config.AdvertiseAddrs.Serf, config.Ports.Serf); !ok { + return nil } if config.Server.EncryptKey != "" { @@ -217,6 +280,11 @@ func (c *Command) readConfig() *Config { } } + if dev { + // Skip validation for dev mode + return config + } + // Parse the RetryInterval. dur, err := time.ParseDuration(config.Server.RetryInterval) if err != nil { diff --git a/command/agent/config.go b/command/agent/config.go index b98784690..0a31e93bb 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -431,6 +431,7 @@ func (r *Resources) ParseReserved() error { // DevConfig is a Config that is used for dev mode of Nomad. func DevConfig() *Config { conf := DefaultConfig() + conf.BindAddr = "127.0.0.1" conf.LogLevel = "DEBUG" conf.Client.Enabled = true conf.Server.Enabled = true @@ -459,7 +460,7 @@ func DefaultConfig() *Config { LogLevel: "INFO", Region: "global", Datacenter: "dc1", - BindAddr: "127.0.0.1", + BindAddr: "0.0.0.0", Ports: &Ports{ HTTP: 4646, RPC: 4647, diff --git a/command/agent/http.go b/command/agent/http.go index 2d0c0ed2d..fe3c417cf 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -49,7 +49,7 @@ type HTTPServer struct { // NewHTTPServer starts new HTTP server over the agent func NewHTTPServer(agent *Agent, config *Config, logOutput io.Writer) (*HTTPServer, error) { // Start the listener - lnAddr, err := agent.getHTTPAddr(true) + lnAddr, err := net.ResolveTCPAddr("tcp", config.Addresses.HTTP) if err != nil { return nil, err }