Merge pull request #522 from slingamn/confusable_service_names.1

check restricted nicknames against skeletons, more pedantically

irc/accounts.go

@@ -293,7 +293,7 @@ func (am *AccountManager) Register(client *Client, account string, callbackNames
 		return errAccountCreation
 	}
 
-	if restrictedNicknames[casefoldedAccount] || restrictedNicknames[skeleton] {
+	if restrictedCasefoldedNicks[casefoldedAccount] || restrictedSkeletons[skeleton] {
 		return errAccountAlreadyRegistered
 	}
 

irc/client_lookup_set.go

@@ -129,13 +129,20 @@ func (clients *ClientManager) Resume(oldClient *Client, session *Session) (err e
 
 // SetNick sets a client's nickname, validating it against nicknames in use
 func (clients *ClientManager) SetNick(client *Client, session *Session, newNick string) error {
+	if len(newNick) > client.server.Config().Limits.NickLen {
+		return errNicknameInvalid
+	}
 	newcfnick, err := CasefoldName(newNick)
 	if err != nil {
-		return err
+		return errNicknameInvalid
 	}
 	newSkeleton, err := Skeleton(newNick)
 	if err != nil {
-		return err
+		return errNicknameInvalid
+	}
+
+	if restrictedCasefoldedNicks[newcfnick] || restrictedSkeletons[newSkeleton] {
+		return errNicknameInvalid
 	}
 
 	reservedAccount, method := client.server.accounts.EnforcementStatus(newcfnick, newSkeleton)

irc/errors.go

@@ -35,6 +35,7 @@ var (
 	errInvalidChannelName             = errors.New(`Invalid channel name`)
 	errMonitorLimitExceeded           = errors.New("Monitor limit exceeded")
 	errNickMissing                    = errors.New("nick missing")
+	errNicknameInvalid                = errors.New("invalid nickname")
 	errNicknameInUse                  = errors.New("nickname in use")
 	errNicknameReserved               = errors.New("nickname is reserved")
 	errNoExistingBan                  = errors.New("Ban does not exist")

irc/nickname.go

@@ -17,17 +17,18 @@ import (
 )
 
 var (
-	// anything added here MUST be casefolded:
+	restrictedNicknames = []string{
-	restrictedNicknames = map[string]bool{
+		"=scene=",  // used for rp commands
-		"=scene=":  true, // used for rp commands
+		"HistServ", // used to play back JOIN, PART, etc. to legacy clients
-		"histserv": true, // TODO(slingamn) this should become a real service
 	}
+
+	restrictedCasefoldedNicks = make(map[string]bool)
+	restrictedSkeletons       = make(map[string]bool)
 )
 
 // returns whether the change succeeded or failed
 func performNickChange(server *Server, client *Client, target *Client, session *Session, newnick string, rb *ResponseBuffer) bool {
 	nickname := strings.TrimSpace(newnick)
-	cfnick, err := CasefoldName(nickname)
 	currentNick := client.Nick()
 
 	if len(nickname) < 1 {
@@ -35,11 +36,6 @@ func performNickChange(server *Server, client *Client, target *Client, session *
 		return false
 	}
 
-	if err != nil || len(nickname) > server.Config().Limits.NickLen || restrictedNicknames[cfnick] {
-		rb.Add(nil, server.name, ERR_ERRONEUSNICKNAME, currentNick, nickname, client.t("Erroneous nickname"))
-		return false
-	}
-
 	if target.Nick() == nickname {
 		return true
 	}
@@ -47,15 +43,17 @@ func performNickChange(server *Server, client *Client, target *Client, session *
 	hadNick := target.HasNick()
 	origNickMask := target.NickMaskString()
 	details := target.Details()
-	err = client.server.clients.SetNick(target, session, nickname)
+	err := client.server.clients.SetNick(target, session, nickname)
 	if err == errNicknameInUse {
 		rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, nickname, client.t("Nickname is already in use"))
-		return false
 	} else if err == errNicknameReserved {
 		rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, nickname, client.t("Nickname is reserved by a different account"))
-		return false
+	} else if err == errNicknameInvalid {
+		rb.Add(nil, server.name, ERR_ERRONEUSNICKNAME, currentNick, nickname, client.t("Erroneous nickname"))
 	} else if err != nil {
 		rb.Add(nil, server.name, ERR_UNKNOWNERROR, currentNick, "NICK", fmt.Sprintf(client.t("Could not set or change nickname: %s"), err.Error()))
+	}
+	if err != nil {
 		return false
 	}
 
@@ -68,7 +66,7 @@ func performNickChange(server *Server, client *Client, target *Client, session *
 	}
 	histItem.Params[0] = nickname
 
-	client.server.logger.Debug("nick", fmt.Sprintf("%s changed nickname to %s [%s]", origNickMask, nickname, cfnick))
+	client.server.logger.Debug("nick", fmt.Sprintf("%s changed nickname to %s [%s]", origNickMask, nickname, client.NickCasefolded()))
 	if hadNick {
 		if client == target {
 			target.server.snomasks.Send(sno.LocalNicks, fmt.Sprintf(ircfmt.Unescape("$%s$r changed nickname to %s"), details.nick, nickname))

irc/services.go

@@ -260,7 +260,7 @@ func initializeServices() {
 		service.Commands["help"] = &servHelpCmd
 
 		// reserve the nickname
-		restrictedNicknames[serviceName] = true
+		restrictedNicknames = append(restrictedNicknames, service.Name)
 
 		// register the protocol-level commands (NICKSERV, NS) that talk to the service
 		var ircCmdDef Command
@@ -279,4 +279,17 @@ func initializeServices() {
 			}
 		}
 	}
+
+	for _, restrictedNickname := range restrictedNicknames {
+		cfName, err := CasefoldName(restrictedNickname)
+		if err != nil {
+			panic(err)
+		}
+		restrictedCasefoldedNicks[cfName] = true
+		skeleton, err := Skeleton(restrictedNickname)
+		if err != nil {
+			panic(err)
+		}
+		restrictedSkeletons[skeleton] = true
+	}
 }