refactor listener config loading

irc/client.go

@@ -193,7 +193,7 @@ func (server *Server) RunClient(conn clientConn) {
 	var isBanned bool
 	var banMsg string
 	var realIP net.IP
-	if conn.IsTor {
+	if conn.Config.IsTor {
 		realIP = utils.IPv4LoopbackAddress
 		isBanned, banMsg = server.checkTorLimits()
 	} else {
@@ -220,7 +220,7 @@ func (server *Server) RunClient(conn clientConn) {
 		atime:     now,
 		channels:  make(ChannelSet),
 		ctime:     now,
-		isTor:     conn.IsTor,
+		isTor:     conn.Config.IsTor,
 		languages: server.Languages().Default(),
 		loginThrottle: connection_limits.GenericThrottle{
 			Duration: config.Accounts.LoginThrottling.Duration,
@@ -246,13 +246,13 @@ func (server *Server) RunClient(conn clientConn) {
 	session.SetMaxlenRest()
 	client.sessions = []*Session{session}
 
-	if conn.IsTLS {
+	if conn.Config.TLSConfig != nil {
 		client.SetMode(modes.TLS, true)
 		// error is not useful to us here anyways so we can ignore it
 		client.certfp, _ = socket.CertFP()
 	}
 
-	if conn.IsTor {
+	if conn.Config.IsTor {
 		client.SetMode(modes.TLS, true)
 		// cover up details of the tor proxying infrastructure (not a user privacy concern,
 		// but a hardening measure):

irc/config.go

@@ -41,16 +41,11 @@ type TLSListenConfig struct {
 	Key  string
 }
 
-// Config returns the TLS contiguration assicated with this TLSListenConfig.
+// listenerConfig is the config governing a particular listener (bound address),
-func (conf *TLSListenConfig) Config() (*tls.Config, error) {
+// in particular whether it has TLS or Tor (or both) enabled.
-	cert, err := tls.LoadX509KeyPair(conf.Cert, conf.Key)
+type listenerConfig struct {
-	if err != nil {
+	TLSConfig *tls.Config
-		return nil, ErrInvalidCertKeyPair
+	IsTor     bool
-	}
-
-	return &tls.Config{
-		Certificates: []tls.Certificate{cert},
-	}, err
 }
 
 type AccountConfig struct {
@@ -278,8 +273,9 @@ type Config struct {
 		nameCasefolded       string
 		Listen               []string
 		UnixBindMode         os.FileMode                `yaml:"unix-bind-mode"`
-		TLSListeners         map[string]*TLSListenConfig `yaml:"tls-listeners"`
+		TLSListeners         map[string]TLSListenConfig `yaml:"tls-listeners"`
 		TorListeners         TorListenersConfig         `yaml:"tor-listeners"`
+		listeners            map[string]listenerConfig
 		STS                  STSConfig
 		CheckIdent           bool `yaml:"check-ident"`
 		MOTD                 string
@@ -485,18 +481,33 @@ func (conf *Config) Operators(oc map[string]*OperClass) (map[string]*Oper, error
 	return operators, nil
 }
 
-// TLSListeners returns a list of TLS listeners and their configs.
+// prepareListeners populates Config.Server.listeners
-func (conf *Config) TLSListeners() (map[string]*tls.Config, error) {
+func (conf *Config) prepareListeners() (err error) {
-	tlsListeners := make(map[string]*tls.Config)
+	torListeners := make(map[string]bool, len(conf.Server.TorListeners.Listeners))
-	for s, tlsListenersConf := range conf.Server.TLSListeners {
+	for _, addr := range conf.Server.TorListeners.Listeners {
-		config, err := tlsListenersConf.Config()
+		torListeners[addr] = true
+	}
+
+	conf.Server.listeners = make(map[string]listenerConfig, len(conf.Server.Listen))
+
+	for _, addr := range conf.Server.Listen {
+		var lconf listenerConfig
+		lconf.IsTor = torListeners[addr]
+		tlsListenConf, ok := conf.Server.TLSListeners[addr]
+		if ok {
+			cert, err := tls.LoadX509KeyPair(tlsListenConf.Cert, tlsListenConf.Key)
 			if err != nil {
-			return nil, err
+				return ErrInvalidCertKeyPair
 			}
-		config.ClientAuth = tls.RequestClientCert
+			tlsConfig := tls.Config{
-		tlsListeners[s] = config
+				Certificates: []tls.Certificate{cert},
+				ClientAuth:   tls.RequestClientCert,
 			}
-	return tlsListeners, nil
+			lconf.TLSConfig = &tlsConfig
+		}
+		conf.Server.listeners[addr] = lconf
+	}
+	return nil
 }
 
 // LoadConfig loads the given YAML configuration file.
@@ -757,5 +768,10 @@ func LoadConfig(filename string) (config *Config, err error) {
 		}
 	}
 
+	err = config.prepareListeners()
+	if err != nil {
+		return nil, fmt.Errorf("failed to prepare listeners: %v", err)
+	}
+
 	return config, nil
 }

irc/server.go

@@ -50,12 +50,11 @@ var (
 
 // ListenerWrapper wraps a listener so it can be safely reconfigured or stopped
 type ListenerWrapper struct {
+	// protects atomic update of config and shouldStop:
+	sync.Mutex // tier 1
 	listener   net.Listener
-	tlsConfig  *tls.Config
+	config     listenerConfig
-	isTor      bool
 	shouldStop bool
-	// protects atomic update of tlsConfig and shouldStop:
-	configMutex sync.Mutex // tier 1
 }
 
 // Server is the main Oragono server.
@@ -101,8 +100,7 @@ var (
 
 type clientConn struct {
 	Conn   net.Conn
-	IsTLS bool
+	Config listenerConfig
-	IsTor bool
 }
 
 // NewServer returns a new Oragono server.
@@ -262,7 +260,7 @@ func (server *Server) checkTorLimits() (banned bool, message string) {
 //
 
 // createListener starts a given listener.
-func (server *Server) createListener(addr string, tlsConfig *tls.Config, isTor bool, bindMode os.FileMode) (*ListenerWrapper, error) {
+func (server *Server) createListener(addr string, conf listenerConfig, bindMode os.FileMode) (*ListenerWrapper, error) {
 	// make listener
 	var listener net.Listener
 	var err error
@@ -284,8 +282,7 @@ func (server *Server) createListener(addr string, tlsConfig *tls.Config, isTor b
 	// throw our details to the server so we can be modified/killed later
 	wrapper := ListenerWrapper{
 		listener:   listener,
-		tlsConfig:  tlsConfig,
+		config:     conf,
-		isTor:      isTor,
 		shouldStop: false,
 	}
 
@@ -297,28 +294,29 @@ func (server *Server) createListener(addr string, tlsConfig *tls.Config, isTor b
 			conn, err := listener.Accept()
 
 			// synchronously access config data:
-			wrapper.configMutex.Lock()
+			wrapper.Lock()
 			shouldStop = wrapper.shouldStop
-			tlsConfig = wrapper.tlsConfig
+			conf := wrapper.config
-			isTor = wrapper.isTor
+			wrapper.Unlock()
-			wrapper.configMutex.Unlock()
 
-			if err == nil {
+			if shouldStop {
-				if tlsConfig != nil {
+				if conn != nil {
-					conn = tls.Server(conn, tlsConfig)
+					conn.Close()
+				}
+				listener.Close()
+				return
+			} else if err == nil {
+				if conf.TLSConfig != nil {
+					conn = tls.Server(conn, conf.TLSConfig)
 				}
 				newConn := clientConn{
 					Conn:   conn,
-					IsTLS: tlsConfig != nil,
+					Config: conf,
-					IsTor: isTor,
 				}
 				// hand off the connection
 				go server.RunClient(newConn)
-			}
+			} else {
-
+				server.logger.Error("internal", "accept error", addr, err.Error())
-			if shouldStop {
-				listener.Close()
-				return
 			}
 		}
 	}()
@@ -858,52 +856,24 @@ func (server *Server) loadDatastore(config *Config) error {
 }
 
 func (server *Server) setupListeners(config *Config) (err error) {
-	logListener := func(addr string, tlsconfig *tls.Config, isTor bool) {
+	logListener := func(addr string, config listenerConfig) {
 		server.logger.Info("listeners",
-			fmt.Sprintf("now listening on %s, tls=%t, tor=%t.", addr, (tlsconfig != nil), isTor),
+			fmt.Sprintf("now listening on %s, tls=%t, tor=%t.", addr, (config.TLSConfig != nil), config.IsTor),
 		)
 	}
 
-	tlsListeners, err := config.TLSListeners()
-	if err != nil {
-		server.logger.Error("server", "failed to reload TLS certificates, aborting rehash", err.Error())
-		return
-	}
-
-	isTorListener := func(listener string) bool {
-		for _, torListener := range config.Server.TorListeners.Listeners {
-			if listener == torListener {
-				return true
-			}
-		}
-		return false
-	}
-
 	// update or destroy all existing listeners
 	for addr := range server.listeners {
 		currentListener := server.listeners[addr]
-		var stillConfigured bool
+		newConfig, stillConfigured := config.Server.listeners[addr]
-		for _, newaddr := range config.Server.Listen {
-			if newaddr == addr {
-				stillConfigured = true
-				break
-			}
-		}
 
-		// pass new config information to the listener, to be picked up after
+		currentListener.Lock()
-		// its next Accept(). this is like sending over a buffered channel of
-		// size 1, but where sending a second item overwrites the buffered item
-		// instead of blocking.
-		tlsConfig := tlsListeners[addr]
-		isTor := isTorListener(addr)
-		currentListener.configMutex.Lock()
 		currentListener.shouldStop = !stillConfigured
-		currentListener.tlsConfig = tlsConfig
+		currentListener.config = newConfig
-		currentListener.isTor = isTor
+		currentListener.Unlock()
-		currentListener.configMutex.Unlock()
 
 		if stillConfigured {
-			logListener(addr, tlsConfig, isTor)
+			logListener(addr, newConfig)
 		} else {
 			// tell the listener it should stop by interrupting its Accept() call:
 			currentListener.listener.Close()
@@ -913,35 +883,26 @@ func (server *Server) setupListeners(config *Config) (err error) {
 	}
 
 	// create new listeners that were not previously configured
-	for _, newaddr := range config.Server.Listen {
+	for newAddr, newConfig := range config.Server.listeners {
-		_, exists := server.listeners[newaddr]
+		_, exists := server.listeners[newAddr]
 		if !exists {
 			// make new listener
-			isTor := isTorListener(newaddr)
+			listener, listenerErr := server.createListener(newAddr, newConfig, config.Server.UnixBindMode)
-			tlsConfig := tlsListeners[newaddr]
-			listener, listenerErr := server.createListener(newaddr, tlsConfig, isTor, config.Server.UnixBindMode)
 			if listenerErr != nil {
-				server.logger.Error("server", "couldn't listen on", newaddr, listenerErr.Error())
+				server.logger.Error("server", "couldn't listen on", newAddr, listenerErr.Error())
 				err = listenerErr
 				continue
 			}
-			server.listeners[newaddr] = listener
+			server.listeners[newAddr] = listener
-			logListener(newaddr, tlsConfig, isTor)
+			logListener(newAddr, newConfig)
 		}
 	}
 
-	if len(tlsListeners) == 0 {
+	if len(config.Server.TLSListeners) == 0 {
 		server.logger.Warning("server", "You are not exposing an SSL/TLS listening port. You should expose at least one port (typically 6697) to accept TLS connections")
 	}
 
-	var usesStandardTLSPort bool
+	if config.Server.listeners[":6697"].TLSConfig == nil {
-	for addr := range tlsListeners {
-		if strings.HasSuffix(addr, ":6697") {
-			usesStandardTLSPort = true
-			break
-		}
-	}
-	if 0 < len(tlsListeners) && !usesStandardTLSPort {
 		server.logger.Warning("server", "Port 6697 is the standard TLS port for IRC. You should (also) expose port 6697 as a TLS port to ensure clients can connect securely")
 	}