From 3c12fb6254df3fcf5dced7e7add312083bc486d9 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 6 Aug 2018 05:02:47 -0400 Subject: [PATCH 1/3] fix #283 (remove unnecessary log.Fatal) The server should never crash during rehash, even if the config is invalid. --- irc/client.go | 19 +++++++++---------- irc/config.go | 6 +++--- irc/logger/logger.go | 11 +++-------- irc/nickserv.go | 7 ++++++- irc/server.go | 36 ++++++++++++++++++++++++------------ 5 files changed, 45 insertions(+), 34 deletions(-) diff --git a/irc/client.go b/irc/client.go index ad3717f2..d5b6a3f3 100644 --- a/irc/client.go +++ b/irc/client.go @@ -7,7 +7,6 @@ package irc import ( "fmt" - "log" "net" "runtime/debug" "strconv" @@ -82,8 +81,8 @@ type Client struct { vhost string } -// NewClient returns a client with all the appropriate info setup. -func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { +// NewClient sets up a new client and starts its goroutine. +func NewClient(server *Server, conn net.Conn, isTLS bool) { now := time.Now() config := server.Config() fullLineLenLimit := config.Limits.LineLen.Tags + config.Limits.LineLen.Rest @@ -114,15 +113,17 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { } if config.Server.CheckIdent && !utils.AddrIsUnix(conn.RemoteAddr()) { _, serverPortString, err := net.SplitHostPort(conn.LocalAddr().String()) + if err != nil { + server.logger.Error("internal", "bad server address", err.Error()) + return + } serverPort, _ := strconv.Atoi(serverPortString) - if err != nil { - log.Fatal(err) - } clientHost, clientPortString, err := net.SplitHostPort(conn.RemoteAddr().String()) - clientPort, _ := strconv.Atoi(clientPortString) if err != nil { - log.Fatal(err) + server.logger.Error("internal", "bad client address", err.Error()) + return } + clientPort, _ := strconv.Atoi(clientPortString) client.Notice(client.t("*** Looking up your username")) resp, err := ident.Query(clientHost, serverPort, clientPort, IdentTimeoutSeconds) @@ -141,8 +142,6 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { } } go client.run() - - return client } func (client *Client) resetFakelag() { diff --git a/irc/config.go b/irc/config.go index e3680ad1..b27891b6 100644 --- a/irc/config.go +++ b/irc/config.go @@ -410,17 +410,17 @@ func (conf *Config) Operators(oc map[string]*OperClass) (map[string]*Oper, error } // TLSListeners returns a list of TLS listeners and their configs. -func (conf *Config) TLSListeners() map[string]*tls.Config { +func (conf *Config) TLSListeners() (map[string]*tls.Config, error) { tlsListeners := make(map[string]*tls.Config) for s, tlsListenersConf := range conf.Server.TLSListeners { config, err := tlsListenersConf.Config() if err != nil { - log.Fatal(err) + return nil, err } config.ClientAuth = tls.RequestClientCert tlsListeners[s] = config } - return tlsListeners + return tlsListeners, nil } // LoadConfig loads the given YAML configuration file. diff --git a/irc/logger/logger.go b/irc/logger/logger.go index 48cbb097..c6014482 100644 --- a/irc/logger/logger.go +++ b/irc/logger/logger.go @@ -124,7 +124,9 @@ func (logger *Manager) ApplyConfig(config []LoggingConfig) error { stdoutWriteLock: &logger.stdoutWriteLock, fileWriteLock: &logger.fileWriteLock, } - if typeMap["userinput"] || typeMap["useroutput"] || (typeMap["*"] && !(excludedTypeMap["userinput"] && excludedTypeMap["useroutput"])) { + ioEnabled := typeMap["userinput"] || typeMap["useroutput"] || (typeMap["*"] && !(excludedTypeMap["userinput"] && excludedTypeMap["useroutput"])) + // raw I/O is only logged at level debug; + if ioEnabled && logConfig.Level == LogDebug { atomic.StoreUint32(&logger.loggingRawIO, 1) } if sLogger.MethodFile.Enabled { @@ -177,13 +179,6 @@ func (logger *Manager) Error(logType string, messageParts ...string) { logger.Log(LogError, logType, messageParts...) } -// Fatal logs the given message as an error message, then exits. -func (logger *Manager) Fatal(logType string, messageParts ...string) { - logger.Error(logType, messageParts...) - logger.Error("FATAL", "Fatal error encountered, application exiting") - os.Exit(1) -} - type fileMethod struct { Enabled bool Filename string diff --git a/irc/nickserv.go b/irc/nickserv.go index fda9669d..addccd96 100644 --- a/irc/nickserv.go +++ b/irc/nickserv.go @@ -20,6 +20,11 @@ func servCmdRequiresAuthEnabled(server *Server) bool { return server.AccountConfig().AuthenticationEnabled } +func nsGroupEnabled(server *Server) bool { + conf := server.Config() + return conf.Accounts.AuthenticationEnabled && conf.Accounts.NickReservation.Enabled +} + const nickservHelp = `NickServ lets you register and login to an account. To see in-depth help for a specific NickServ command, try: @@ -55,7 +60,7 @@ same user account, letting you reclaim your nickname.`, GROUP links your current nickname with your logged-in account, preventing other users from changing to it (or forcing them to rename).`, helpShort: `$bGROUP$b links your current nickname to your user account.`, - enabled: servCmdRequiresAccreg, + enabled: nsGroupEnabled, authRequired: true, }, diff --git a/irc/server.go b/irc/server.go index c05767ab..cb53f3f8 100644 --- a/irc/server.go +++ b/irc/server.go @@ -10,7 +10,6 @@ import ( "crypto/tls" "encoding/base64" "fmt" - "log" "math/rand" "net" "net/http" @@ -328,8 +327,8 @@ func (server *Server) checkBans(ipaddr net.IP) (banned bool, message string) { // IRC protocol listeners // -// createListener starts the given listeners. -func (server *Server) createListener(addr string, tlsConfig *tls.Config) *ListenerWrapper { +// createListener starts a given listener. +func (server *Server) createListener(addr string, tlsConfig *tls.Config) (*ListenerWrapper, error) { // make listener var listener net.Listener var err error @@ -342,7 +341,7 @@ func (server *Server) createListener(addr string, tlsConfig *tls.Config) *Listen listener, err = net.Listen("tcp", addr) } if err != nil { - log.Fatal(server, "listen error: ", err) + return nil, err } // throw our details to the server so we can be modified/killed later @@ -385,7 +384,7 @@ func (server *Server) createListener(addr string, tlsConfig *tls.Config) *Listen } }() - return &wrapper + return &wrapper, nil } // generateMessageID returns a network-unique message ID. @@ -899,7 +898,7 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { } // we are now open for business - server.setupListeners(config) + err = server.setupListeners(config) if !initial { // push new info to all of our clients @@ -914,7 +913,7 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { } } - return nil + return err } func (server *Server) setupPprofListener(config *Config) { @@ -1024,15 +1023,20 @@ func (server *Server) loadDatastore(config *Config) error { return nil } -func (server *Server) setupListeners(config *Config) { +func (server *Server) setupListeners(config *Config) (err error) { logListener := func(addr string, tlsconfig *tls.Config) { server.logger.Info("listeners", fmt.Sprintf("now listening on %s, tls=%t.", addr, (tlsconfig != nil)), ) } + tlsListeners, err := config.TLSListeners() + if err != nil { + server.logger.Error("rehash", "failed to reload TLS certificates, aborting rehash", err.Error()) + return + } + // update or destroy all existing listeners - tlsListeners := config.TLSListeners() for addr := range server.listeners { currentListener := server.listeners[addr] var stillConfigured bool @@ -1068,7 +1072,13 @@ func (server *Server) setupListeners(config *Config) { if !exists { // make new listener tlsConfig := tlsListeners[newaddr] - server.listeners[newaddr] = server.createListener(newaddr, tlsConfig) + listener, listenerErr := server.createListener(newaddr, tlsConfig) + if listenerErr != nil { + server.logger.Error("rehash", "couldn't listen on", newaddr, listenerErr.Error()) + err = listenerErr + continue + } + server.listeners[newaddr] = listener logListener(newaddr, tlsConfig) } } @@ -1078,8 +1088,8 @@ func (server *Server) setupListeners(config *Config) { } var usesStandardTLSPort bool - for addr := range config.TLSListeners() { - if strings.Contains(addr, "6697") { + for addr := range tlsListeners { + if strings.HasSuffix(addr, ":6697") { usesStandardTLSPort = true break } @@ -1087,6 +1097,8 @@ func (server *Server) setupListeners(config *Config) { if 0 < len(tlsListeners) && !usesStandardTLSPort { server.logger.Warning("startup", "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") } + + return } // elistMatcher takes and matches ELIST conditions From 578ca2fdf6351a38bb2e3751444d03231f430362 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 6 Aug 2018 09:21:29 -0400 Subject: [PATCH 2/3] add support for service command aliases --- irc/chanserv.go | 3 +++ irc/server.go | 16 ---------------- irc/services.go | 35 +++++++++++++++++++++++++++++++++-- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/irc/chanserv.go b/irc/chanserv.go index aac72967..bdf779c0 100644 --- a/irc/chanserv.go +++ b/irc/chanserv.go @@ -63,6 +63,9 @@ invoking the command without a code will display the necessary code.`, helpShort: `$bUNREGISTER$b deletes a channel registration.`, enabled: chanregEnabled, }, + "drop": { + aliasOf: "unregister", + }, "amode": { handler: csAmodeHandler, help: `Syntax: $bAMODE #channel [mode change] [account]$b diff --git a/irc/server.go b/irc/server.go index cb53f3f8..d0c6d37c 100644 --- a/irc/server.go +++ b/irc/server.go @@ -145,22 +145,6 @@ func NewServer(config *Config, logger *logger.Manager) (*Server, error) { return nil, err } - // confirm help entries for ChanServ exist. - // this forces people to write help entries for every single CS command. - for commandName, commandInfo := range chanservCommands { - if commandInfo.help == "" || commandInfo.helpShort == "" { - return nil, fmt.Errorf("Help entry does not exist for ChanServ command %s", commandName) - } - } - - // confirm help entries for NickServ exist. - // this forces people to write help entries for every single NS command. - for commandName, commandInfo := range nickservCommands { - if commandInfo.help == "" || commandInfo.helpShort == "" { - return nil, fmt.Errorf("Help entry does not exist for NickServ command %s", commandName) - } - } - // Attempt to clean up when receiving these signals. signal.Notify(server.signals, ServerExitSignals...) signal.Notify(server.rehashSignal, syscall.SIGHUP) diff --git a/irc/services.go b/irc/services.go index 175b65ec..674b84b1 100644 --- a/irc/services.go +++ b/irc/services.go @@ -5,6 +5,7 @@ package irc import ( "fmt" + "log" "sort" "strings" @@ -25,6 +26,7 @@ type ircService struct { // defines a command associated with a service, e.g., NICKSERV IDENTIFY type serviceCommand struct { + aliasOf string // marks this command as an alias of another capabs []string // oper capabs the given user has to have to access this command handler func(server *Server, client *Client, command, params string, rb *ResponseBuffer) help string @@ -33,6 +35,24 @@ type serviceCommand struct { enabled func(*Server) bool // is this command enabled in the server config? } +// looks up a command in the table of command definitions for a service, resolving aliases +func lookupServiceCommand(commands map[string]*serviceCommand, command string) *serviceCommand { + maxDepth := 1 + depth := 0 + for depth <= maxDepth { + result, ok := commands[command] + if !ok { + return nil + } else if result.aliasOf == "" { + return result + } else { + command = result.aliasOf + depth += 1 + } + } + return nil +} + // all services, by lowercase name var OragonoServices = map[string]*ircService{ "nickserv": { @@ -93,7 +113,7 @@ func servicePrivmsgHandler(service *ircService, server *Server, client *Client, rb.Add(nil, service.Name, "NOTICE", nick, notice) } - cmd := service.Commands[commandName] + cmd := lookupServiceCommand(service.Commands, commandName) if cmd == nil { sendNotice(fmt.Sprintf("%s /%s HELP", client.t("Unknown command. To see available commands, run"), service.ShortName)) return @@ -140,6 +160,9 @@ func serviceHelpHandler(service *ircService, server *Server, client *Client, par if 0 < len(commandInfo.capabs) && !client.HasRoleCapabs(commandInfo.capabs...) { continue } + if commandInfo.aliasOf != "" { + continue // don't show help lines for aliases + } if commandInfo.enabled != nil && !commandInfo.enabled(server) { disabledCommands = true continue @@ -164,7 +187,8 @@ func serviceHelpHandler(service *ircService, server *Server, client *Client, par sendNotice(line) } } else { - commandInfo := service.Commands[strings.ToLower(strings.TrimSpace(params))] + commandName := strings.ToLower(strings.TrimSpace(params)) + commandInfo := lookupServiceCommand(service.Commands, commandName) if commandInfo == nil { sendNotice(client.t(fmt.Sprintf("Unknown command. To see available commands, run /%s HELP", service.ShortName))) } else { @@ -196,5 +220,12 @@ func initializeServices() { Commands[ircCmd] = ircCmdDef oragonoServicesByCommandAlias[ircCmd] = service } + + // force devs to write a help entry for every command + for commandName, commandInfo := range service.Commands { + if commandInfo.aliasOf == "" && (commandInfo.help == "" || commandInfo.helpShort == "") { + log.Fatal(fmt.Sprintf("help entry missing for %s command %s", serviceName, commandName)) + } + } } } From 7cd5b8473cbe02e03449d1e072f95d10128fcbb4 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 6 Aug 2018 09:47:44 -0400 Subject: [PATCH 3/3] fix a bug in CS UNREGISTER --- irc/channelreg.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/irc/channelreg.go b/irc/channelreg.go index 6dca4bc7..723cfdaa 100644 --- a/irc/channelreg.go +++ b/irc/channelreg.go @@ -250,7 +250,7 @@ func (reg *ChannelRegistry) deleteChannel(tx *buntdb.Tx, key string, info Regist founder, _ := tx.Get(fmt.Sprintf(keyChannelFounder, key)) // to see if we're deleting the right channel, confirm the founder and the registration time - if founder == info.Founder && registeredAt == info.RegisteredAt { + if founder == info.Founder && registeredAt.Unix() == info.RegisteredAt.Unix() { for _, keyFmt := range channelKeyStrings { tx.Delete(fmt.Sprintf(keyFmt, key)) }