fix #913

irc/accounts.go

@@ -15,6 +15,7 @@ import (
 	"unicode"
 
 	"github.com/oragono/oragono/irc/caps"
+	"github.com/oragono/oragono/irc/connection_limits"
 	"github.com/oragono/oragono/irc/ldap"
 	"github.com/oragono/oragono/irc/passwd"
 	"github.com/oragono/oragono/irc/utils"
@@ -62,6 +63,7 @@ type AccountManager struct {
 	nickToAccount     map[string]string
 	skeletonToAccount map[string]string
 	accountToMethod   map[string]NickEnforcementMethod
+	registerThrottle  connection_limits.GenericThrottle
 }
 
 func (am *AccountManager) Initialize(server *Server) {
@@ -75,6 +77,24 @@ func (am *AccountManager) Initialize(server *Server) {
 	am.buildNickToAccountIndex(config)
 	am.initVHostRequestQueue(config)
 	am.createAlwaysOnClients(config)
+	am.resetRegisterThrottle(config)
+}
+
+func (am *AccountManager) resetRegisterThrottle(config *Config) {
+	am.Lock()
+	defer am.Unlock()
+
+	am.registerThrottle = connection_limits.GenericThrottle{
+		Duration: config.Accounts.Registration.Throttling.Duration,
+		Limit:    config.Accounts.Registration.Throttling.MaxAttempts,
+	}
+}
+
+func (am *AccountManager) touchRegisterThrottle() (throttled bool) {
+	am.Lock()
+	defer am.Unlock()
+	throttled, _ = am.registerThrottle.Touch()
+	return
 }
 
 func (am *AccountManager) createAlwaysOnClients(config *Config) {
@@ -363,6 +383,15 @@ func (am *AccountManager) Register(client *Client, account string, callbackNames
 		return errFeatureDisabled
 	}
 
+	if client != nil && client.Account() != "" {
+		return errAccountAlreadyLoggedIn
+	}
+
+	if client != nil && am.touchRegisterThrottle() {
+		am.server.logger.Warning("accounts", "global registration throttle exceeded by client", client.Nick())
+		return errLimitExceeded
+	}
+
 	// if nick reservation is enabled, you can only register your current nickname
 	// as an account; this prevents "land-grab" situations where someone else
 	// registers your nick out from under you and then NS GHOSTs you
@@ -725,6 +754,10 @@ func (am *AccountManager) Verify(client *Client, account string, code string) er
 		return errAccountVerificationFailed
 	}
 
+	if client != nil && client.Account() != "" {
+		return errAccountAlreadyLoggedIn
+	}
+
 	verifiedKey := fmt.Sprintf(keyAccountVerified, casefoldedAccount)
 	accountKey := fmt.Sprintf(keyAccountExists, casefoldedAccount)
 	accountNameKey := fmt.Sprintf(keyAccountName, casefoldedAccount)

irc/config.go

@@ -229,6 +229,29 @@ type MulticlientConfig struct {
 	AlwaysOn         PersistentStatus `yaml:"always-on"`
 }
 
+type throttleConfig struct {
+	Enabled     bool
+	Duration    time.Duration
+	MaxAttempts int `yaml:"max-attempts"`
+}
+
+type ThrottleConfig struct {
+	throttleConfig
+}
+
+func (t *ThrottleConfig) UnmarshalYAML(unmarshal func(interface{}) error) (err error) {
+	// note that this technique only works if the zero value of the struct
+	// doesn't need any postprocessing (because if the field is omitted entirely
+	// from the YAML, then UnmarshalYAML won't be called at all)
+	if err = unmarshal(&t.throttleConfig); err != nil {
+		return
+	}
+	if !t.Enabled {
+		t.MaxAttempts = 0 // limit of 0 means disabled
+	}
+	return
+}
+
 type AccountConfig struct {
 	Registration          AccountRegistrationConfig
 	AuthenticationEnabled bool `yaml:"authentication-enabled"`
@@ -237,13 +260,9 @@ type AccountConfig struct {
 		Exempted     []string
 		exemptedNets []net.IPNet
 	} `yaml:"require-sasl"`
-	LDAP            ldap.ServerConfig
+	LDAP               ldap.ServerConfig
-	LoginThrottling struct {
+	LoginThrottling    ThrottleConfig `yaml:"login-throttling"`
-		Enabled     bool
+	SkipServerPassword bool           `yaml:"skip-server-password"`
-		Duration    time.Duration
-		MaxAttempts int `yaml:"max-attempts"`
-	} `yaml:"login-throttling"`
-	SkipServerPassword bool `yaml:"skip-server-password"`
 	NickReservation    struct {
 		Enabled                bool
 		AdditionalNickLimit    int `yaml:"additional-nick-limit"`
@@ -266,6 +285,7 @@ type AccountConfig struct {
 // AccountRegistrationConfig controls account registration.
 type AccountRegistrationConfig struct {
 	Enabled                bool
+	Throttling             ThrottleConfig
 	EnabledCallbacks       []string         `yaml:"enabled-callbacks"`
 	EnabledCredentialTypes []string         `yaml:"-"`
 	VerifyTimeout          custime.Duration `yaml:"verify-timeout"`
@@ -997,10 +1017,6 @@ func LoadConfig(filename string) (config *Config, err error) {
 		}
 	}
 
-	if !config.Accounts.LoginThrottling.Enabled {
-		config.Accounts.LoginThrottling.MaxAttempts = 0 // limit of 0 means disabled
-	}
-
 	config.Server.capValues[caps.SASL] = "PLAIN,EXTERNAL"
 	if !config.Accounts.AuthenticationEnabled {
 		config.Server.supportedCaps.Disable(caps.SASL)

irc/errors.go

@@ -22,6 +22,7 @@ var (
 	errAccountBadPassphrase           = errors.New(`Passphrase contains forbidden characters or is otherwise invalid`)
 	errAccountNickReservationFailed   = errors.New("Could not (un)reserve nick")
 	errAccountNotLoggedIn             = errors.New("You're not logged into an account")
+	errAccountAlreadyLoggedIn         = errors.New("You're already logged into an account")
 	errAccountTooManyNicks            = errors.New("Account has too many reserved nicks")
 	errAccountUnverified              = errors.New(`Account is not yet verified`)
 	errAccountVerificationFailed      = errors.New("Account verification failed")

irc/handlers.go

@@ -66,10 +66,10 @@ func registrationErrorToMessageAndCode(err error) (message, code string) {
 	case errAccountBadPassphrase:
 		code = "REG_INVALID_CREDENTIAL"
 		message = err.Error()
-	case errAccountAlreadyRegistered, errAccountAlreadyVerified, errAccountAlreadyUnregistered:
+	case errAccountAlreadyRegistered, errAccountAlreadyVerified, errAccountAlreadyUnregistered, errAccountAlreadyLoggedIn, errAccountCreation, errAccountMustHoldNick, errAccountBadPassphrase, errCertfpAlreadyExists, errFeatureDisabled:
-		message = err.Error()
-	case errAccountCreation, errAccountMustHoldNick, errAccountBadPassphrase, errCertfpAlreadyExists, errFeatureDisabled:
 		message = err.Error()
+	case errLimitExceeded:
+		message = `There have been too many registration attempts recently; try again later`
 	}
 	return
 }

irc/nickserv.go

@@ -616,7 +616,9 @@ func nsGroupHandler(server *Server, client *Client, command string, params []str
 }
 
 func nsLoginThrottleCheck(client *Client, rb *ResponseBuffer) (success bool) {
+	client.stateMutex.Lock()
 	throttled, remainingTime := client.loginThrottle.Touch()
+	client.stateMutex.Unlock()
 	if throttled {
 		nsNotice(rb, fmt.Sprintf(client.t("Please wait at least %v and try again"), remainingTime))
 		return false
@@ -741,11 +743,6 @@ func nsRegisterHandler(server *Server, client *Client, command string, params []
 		}
 	}
 
-	if details.account != "" {
-		nsNotice(rb, client.t("You're already logged into an account"))
-		return
-	}
-
 	if !nsLoginThrottleCheck(client, rb) {
 		return
 	}
@@ -793,13 +790,10 @@ func nsRegisterHandler(server *Server, client *Client, command string, params []
 			message := fmt.Sprintf(messageTemplate, fmt.Sprintf("%s:%s", callbackNamespace, callbackValue))
 			nsNotice(rb, message)
 		}
-	}
+	} else {
-
+		// details could not be stored and relevant numerics have been dispatched, abort
-	// details could not be stored and relevant numerics have been dispatched, abort
+		message, _ := registrationErrorToMessageAndCode(err)
-	message, _ := registrationErrorToMessageAndCode(err)
-	if err != nil {
 		nsNotice(rb, client.t(message))
-		return
 	}
 }
 
@@ -894,10 +888,13 @@ func nsVerifyHandler(server *Server, client *Client, command string, params []st
 	err := server.accounts.Verify(client, username, code)
 
 	var errorMessage string
-	if err == errAccountVerificationInvalidCode || err == errAccountAlreadyVerified {
+	if err != nil {
-		errorMessage = err.Error()
+		switch err {
-	} else if err != nil {
+		case errAccountAlreadyLoggedIn, errAccountVerificationInvalidCode, errAccountAlreadyVerified:
-		errorMessage = errAccountVerificationFailed.Error()
+			errorMessage = err.Error()
+		default:
+			errorMessage = errAccountVerificationFailed.Error()
+		}
 	}
 
 	if errorMessage != "" {

irc/server.go

@@ -634,6 +634,9 @@ func (server *Server) applyConfig(config *Config) (err error) {
 				client.resizeHistory(config)
 			}
 		}
+		if oldConfig.Accounts.Registration.Throttling != config.Accounts.Registration.Throttling {
+			server.accounts.resetRegisterThrottle(config)
+		}
 	}
 
 	// activate the new config

oragono.yaml

@@ -266,6 +266,14 @@ accounts:
         # the `accreg` capability can still create accounts with `/NICKSERV SAREGISTER`
         enabled: true
 
+        # global throttle on new account creation
+        throttling:
+            enabled: true
+            # window
+            duration: 10m
+            # number of attempts allowed within the window
+            max-attempts: 30
+
         # this is the bcrypt cost we'll use for account passwords
         bcrypt-cost: 9