Merge pull request #565 from slingamn/listener_refactor.3

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,17 @@ type TLSListenConfig struct {
 	Key  string
 }
 
-// Config returns the TLS contiguration assicated with this TLSListenConfig.
+// This is the YAML-deserializable type of the value of the `Server.Listeners` map
-func (conf *TLSListenConfig) Config() (*tls.Config, error) {
+type listenerConfigBlock struct {
-	cert, err := tls.LoadX509KeyPair(conf.Cert, conf.Key)
+	TLS TLSListenConfig
-	if err != nil {
+	Tor bool
-		return nil, ErrInvalidCertKeyPair
+}
-	}
 
-	return &tls.Config{
+// listenerConfig is the config governing a particular listener (bound address),
-		Certificates: []tls.Certificate{cert},
+// in particular whether it has TLS or Tor (or both) enabled.
-	}, err
+type listenerConfig struct {
+	TLSConfig *tls.Config
+	IsTor     bool
 }
 
 type AccountConfig struct {
@@ -257,8 +258,8 @@ type FakelagConfig struct {
 }
 
 type TorListenersConfig struct {
-	Listeners                 []string
+	Listeners                 []string // legacy only
-	RequireSasl               bool `yaml:"require-sasl"`
+	RequireSasl               bool     `yaml:"require-sasl"`
 	Vhost                     string
 	MaxConnections            int           `yaml:"max-connections"`
 	ThrottleDuration          time.Duration `yaml:"throttle-duration"`
@@ -272,14 +273,19 @@ type Config struct {
 	}
 
 	Server struct {
-		Password             string
+		Password       string
-		passwordBytes        []byte
+		passwordBytes  []byte
-		Name                 string
+		Name           string
-		nameCasefolded       string
+		nameCasefolded string
-		Listen               []string
+		// Listeners is the new style for configuring listeners:
-		UnixBindMode         os.FileMode                 `yaml:"unix-bind-mode"`
+		Listeners    map[string]listenerConfigBlock
-		TLSListeners         map[string]*TLSListenConfig `yaml:"tls-listeners"`
+		UnixBindMode os.FileMode        `yaml:"unix-bind-mode"`
-		TorListeners         TorListenersConfig          `yaml:"tor-listeners"`
+		TorListeners TorListenersConfig `yaml:"tor-listeners"`
+		// Listen and TLSListeners are the legacy style:
+		Listen       []string
+		TLSListeners map[string]TLSListenConfig `yaml:"tls-listeners"`
+		// either way, the result is this:
+		trueListeners        map[string]listenerConfig
 		STS                  STSConfig
 		CheckIdent           bool `yaml:"check-ident"`
 		MOTD                 string
@@ -485,22 +491,63 @@ 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.
+func loadTlsConfig(config TLSListenConfig) (tlsConfig *tls.Config, err error) {
-func (conf *Config) TLSListeners() (map[string]*tls.Config, error) {
+	cert, err := tls.LoadX509KeyPair(config.Cert, config.Key)
-	tlsListeners := make(map[string]*tls.Config)
+	if err != nil {
-	for s, tlsListenersConf := range conf.Server.TLSListeners {
+		return nil, ErrInvalidCertKeyPair
-		config, err := tlsListenersConf.Config()
-		if err != nil {
-			return nil, err
-		}
-		config.ClientAuth = tls.RequestClientCert
-		tlsListeners[s] = config
 	}
-	return tlsListeners, nil
+	result := tls.Config{
+		Certificates: []tls.Certificate{cert},
+		ClientAuth:   tls.RequestClientCert,
+	}
+	return &result, nil
 }
 
-// LoadConfig loads the given YAML configuration file.
+// prepareListeners populates Config.Server.trueListeners
-func LoadConfig(filename string) (config *Config, err error) {
+func (conf *Config) prepareListeners() (err error) {
+	listeners := make(map[string]listenerConfig)
+	if 0 < len(conf.Server.Listeners) {
+		for addr, block := range conf.Server.Listeners {
+			var lconf listenerConfig
+			lconf.IsTor = block.Tor
+			if block.TLS.Cert != "" {
+				tlsConfig, err := loadTlsConfig(block.TLS)
+				if err != nil {
+					return err
+				}
+				lconf.TLSConfig = tlsConfig
+			}
+			listeners[addr] = lconf
+		}
+	} else if 0 < len(conf.Server.Listen) {
+		log.Printf("WARNING: configuring listeners via the legacy `server.listen` config option")
+		log.Printf("This will be removed in a later release: you should update to use `server.listeners`")
+		torListeners := make(map[string]bool, len(conf.Server.TorListeners.Listeners))
+		for _, addr := range conf.Server.TorListeners.Listeners {
+			torListeners[addr] = true
+		}
+		for _, addr := range conf.Server.Listen {
+			var lconf listenerConfig
+			lconf.IsTor = torListeners[addr]
+			tlsListenConf, ok := conf.Server.TLSListeners[addr]
+			if ok {
+				tlsConfig, err := loadTlsConfig(tlsListenConf)
+				if err != nil {
+					return err
+				}
+				lconf.TLSConfig = tlsConfig
+			}
+			listeners[addr] = lconf
+		}
+	} else {
+		return fmt.Errorf("No listeners were configured")
+	}
+	conf.Server.trueListeners = listeners
+	return nil
+}
+
+// LoadRawConfig loads the config without doing any consistency checks or postprocessing
+func LoadRawConfig(filename string) (config *Config, err error) {
 	data, err := ioutil.ReadFile(filename)
 	if err != nil {
 		return nil, err
@@ -510,6 +557,15 @@ func LoadConfig(filename string) (config *Config, err error) {
 	if err != nil {
 		return nil, err
 	}
+	return
+}
+
+// LoadConfig loads the given YAML configuration file.
+func LoadConfig(filename string) (config *Config, err error) {
+	config, err = LoadRawConfig(filename)
+	if err != nil {
+		return nil, err
+	}
 
 	config.Filename = filename
 
@@ -525,9 +581,6 @@ func LoadConfig(filename string) (config *Config, err error) {
 	if config.Datastore.Path == "" {
 		return nil, ErrDatastorePathMissing
 	}
-	if len(config.Server.Listen) == 0 {
-		return nil, ErrNoListenersDefined
-	}
 	//dan: automagically fix identlen until a few releases in the future (from now, 0.12.0), being a newly-introduced limit
 	if config.Limits.IdentLen < 1 {
 		config.Limits.IdentLen = 20
@@ -757,5 +810,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.
@@ -100,9 +99,8 @@ var (
 )
 
 type clientConn struct {
-	Conn  net.Conn
+	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,
+					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
 			}
 		}
 	}()
@@ -885,52 +883,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.trueListeners[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()
@@ -940,35 +910,34 @@ func (server *Server) setupListeners(config *Config) (err error) {
 	}
 
 	// create new listeners that were not previously configured
-	for _, newaddr := range config.Server.Listen {
+	numTlsListeners := 0
-		_, exists := server.listeners[newaddr]
+	hasStandardTlsListener := false
+	for newAddr, newConfig := range config.Server.trueListeners {
+		if newConfig.TLSConfig != nil {
+			numTlsListeners += 1
+			if strings.HasSuffix(newAddr, ":6697") {
+				hasStandardTlsListener = true
+			}
+		}
+		_, 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 numTlsListeners == 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 !hasStandardTlsListener {
-	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")
 	}
 

oragono.go

@@ -39,6 +39,46 @@ func getPassword() string {
 	return strings.TrimSpace(text)
 }
 
+// implements the `oragono mkcerts` command
+func doMkcerts(configFile string, quiet bool) {
+	config, err := irc.LoadRawConfig(configFile)
+	if err != nil {
+		log.Fatal(err)
+	}
+	if !quiet {
+		log.Println("making self-signed certificates")
+	}
+
+	certToKey := make(map[string]string)
+	for name, conf := range config.Server.Listeners {
+		if conf.TLS.Cert == "" {
+			continue
+		}
+		existingKey, ok := certToKey[conf.TLS.Cert]
+		if ok {
+			if existingKey == conf.TLS.Key {
+				continue
+			} else {
+				log.Fatal("Conflicting TLS key files for", conf.TLS.Cert)
+			}
+		}
+		if !quiet {
+			log.Printf(" making cert for %s listener\n", name)
+		}
+		host := config.Server.Name
+		cert, key := conf.TLS.Cert, conf.TLS.Key
+		err := mkcerts.CreateCert("Oragono", host, cert, key)
+		if err == nil {
+			if !quiet {
+				log.Printf("  Certificate created at %s : %s\n", cert, key)
+			}
+			certToKey[cert] = key
+		} else {
+			log.Fatal("  Could not create certificate:", err.Error())
+		}
+	}
+}
+
 func main() {
 	version := irc.SemVer
 	usage := `oragono.
@@ -88,11 +128,14 @@ Options:
 	} else if arguments["mksecret"].(bool) {
 		fmt.Println(utils.GenerateSecretKey())
 		return
+	} else if arguments["mkcerts"].(bool) {
+		doMkcerts(arguments["--conf"].(string), arguments["--quiet"].(bool))
+		return
 	}
 
 	configfile := arguments["--conf"].(string)
 	config, err := irc.LoadConfig(configfile)
-	if err != nil {
+	if err != nil && !(err == irc.ErrInvalidCertKeyPair && arguments["mkcerts"].(bool)) {
 		log.Fatal("Config file did not load successfully: ", err.Error())
 	}
 
@@ -114,25 +157,6 @@ Options:
 		if !arguments["--quiet"].(bool) {
 			log.Println("database upgraded: ", config.Datastore.Path)
 		}
-	} else if arguments["mkcerts"].(bool) {
-		if !arguments["--quiet"].(bool) {
-			log.Println("making self-signed certificates")
-		}
-
-		for name, conf := range config.Server.TLSListeners {
-			if !arguments["--quiet"].(bool) {
-				log.Printf(" making cert for %s listener\n", name)
-			}
-			host := config.Server.Name
-			err := mkcerts.CreateCert("Oragono", host, conf.Cert, conf.Key)
-			if err == nil {
-				if !arguments["--quiet"].(bool) {
-					log.Printf("  Certificate created at %s : %s\n", conf.Cert, conf.Key)
-				}
-			} else {
-				log.Fatal("  Could not create certificate:", err.Error())
-			}
-		}
 	} else if arguments["run"].(bool) {
 		if !arguments["--quiet"].(bool) {
 			logman.Info("server", fmt.Sprintf("Oragono v%s starting", irc.SemVer))

oragono.yaml

@@ -11,14 +11,30 @@ server:
     name: oragono.test
 
     # addresses to listen on
-    listen:
+    listeners:
-        - ":6697" # SSL/TLS port
+        # The standard plaintext port for IRC is 6667. This will listen on all interfaces:
-        - ":6667" # plaintext port
+        ":6667":
-        # To disable plaintext over the Internet, comment out :6667 and replace with:
+
-        # - "127.0.0.1:6667" # (loopback ipv4, localhost-only)
+        # The standard SSL/TLS port for IRC is 6697. This will listen on all interfaces:
-        # - "[::1]:6667"     # (loopback ipv6, localhost-only)
+        ":6697":
-        # Unix domain socket for proxying:
+            tls:
-        # - "/tmp/oragono_sock"
+                key: tls.key
+                cert: tls.crt
+
+        # Since using plaintext over the public Internet poses security and privacy issues,
+        # you may wish to use plaintext only on local interfaces. To do so, comment out
+        # the `":6667":` line, then uncomment these two lines:
+        # "127.0.0.1:6667": # (loopback ipv4, localhost-only)
+        # "[::1]:6667":     # (loopback ipv6, localhost-only)
+
+        # Example of a Unix domain socket for proxying:
+        # "/tmp/oragono_sock":
+
+        # Example of a Tor listener: any connection that comes in on this listener will
+        # be considered a Tor connection. It is strongly recommended that this listener
+        # *not* be on a public interface --- it should be on 127.0.0.0/8 or unix domain:
+        # "/tmp/oragono_tor_sock":
+        #     tor: true
 
     # sets the permissions for Unix listen sockets. on a typical Linux system,
     # the default is 0775 or 0755, which prevents other users/groups from connecting
@@ -26,23 +42,8 @@ server:
     # where anyone can connect.
     unix-bind-mode: 0777
 
-    # tls listeners
+    # configure the behavior of Tor listeners (ignored if you didn't enable any):
-    tls-listeners:
-        # listener on ":6697"
-        ":6697":
-            key: tls.key
-            cert: tls.crt
-
-    # tor listeners: designate listeners for use by a tor hidden service / .onion address
-    # WARNING: if you are running oragono as a pure hidden service, see the
-    # anonymization / hardening recommendations in docs/MANUAL.md
     tor-listeners:
-        # any connections that come in on these listeners will be considered
-        # Tor connections. it is strongly recommended that these listeners *not*
-        # be on public interfaces: they should be on 127.0.0.0/8 or unix domain
-        listeners:
-        #    - "/tmp/oragono_tor_sock"
-
         # if this is true, connections from Tor must authenticate with SASL
         require-sasl: false