From 6d65335071bd5b3d9c8c822ae32ccfa3ec951e35 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 16 Mar 2020 23:37:52 -0400 Subject: [PATCH] fix various bugs --- irc/accounts.go | 4 ++++ irc/errors.go | 3 +-- irc/handlers.go | 6 +++++- irc/nickname.go | 24 ++++++++++++++++++++++-- irc/nickserv.go | 31 ++++++++----------------------- 5 files changed, 40 insertions(+), 28 deletions(-) diff --git a/irc/accounts.go b/irc/accounts.go index d2c38386..80ad1d39 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -943,6 +943,10 @@ func (am *AccountManager) checkPassphrase(accountName, passphrase string) (accou } func (am *AccountManager) AuthenticateByPassphrase(client *Client, accountName string, passphrase string) (err error) { + // XXX check this now, so we don't allow a redundant login for an always-on client + // even for a brief period. the other potential source of nick-account conflicts + // is from force-nick-equals-account, but those will be caught later by + // fixupNickEqualsAccount and if there is a conflict, they will be logged out. if client.registered { if clientAlready := am.server.clients.Get(accountName); clientAlready != nil && clientAlready.AlwaysOn() { return errNickAccountMismatch diff --git a/irc/errors.go b/irc/errors.go index ee283419..54faecd2 100644 --- a/irc/errors.go +++ b/irc/errors.go @@ -41,8 +41,7 @@ var ( errNicknameInvalid = errors.New("invalid nickname") errNicknameInUse = errors.New("nickname in use") errNicknameReserved = errors.New("nickname is reserved") - errCantChangeNick = errors.New(`You must use your account name as your nickname`) - errNickAccountMismatch = errors.New(`Your nickname must match your account name`) + errNickAccountMismatch = errors.New(`Your nickname must match your account name; try logging out and logging back in with SASL`) errNoExistingBan = errors.New("Ban does not exist") errNoSuchChannel = errors.New(`No such channel`) errChannelPurged = errors.New(`This channel was purged by the server operators and cannot be used`) diff --git a/irc/handlers.go b/irc/handlers.go index fcf09293..1c4178a7 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -237,6 +237,8 @@ func authPlainHandler(server *Server, client *Client, mechanism string, value [] msg := authErrorToMessage(server, err) rb.Add(nil, server.name, ERR_SASLFAIL, client.Nick(), fmt.Sprintf("%s: %s", client.t("SASL authentication failed"), client.t(msg))) return false + } else if !fixupNickEqualsAccount(client, rb, server.Config()) { + return false } sendSuccessfulAccountAuth(client, rb, false, true) @@ -245,7 +247,7 @@ func authPlainHandler(server *Server, client *Client, mechanism string, value [] func authErrorToMessage(server *Server, err error) (msg string) { switch err { - case errAccountDoesNotExist, errAccountUnverified, errAccountInvalidCredentials, errAuthzidAuthcidMismatch: + case errAccountDoesNotExist, errAccountUnverified, errAccountInvalidCredentials, errAuthzidAuthcidMismatch, errNickAccountMismatch: return err.Error() default: // don't expose arbitrary error messages to the user @@ -280,6 +282,8 @@ func authExternalHandler(server *Server, client *Client, mechanism string, value msg := authErrorToMessage(server, err) rb.Add(nil, server.name, ERR_SASLFAIL, client.nick, fmt.Sprintf("%s: %s", client.t("SASL authentication failed"), client.t(msg))) return false + } else if !fixupNickEqualsAccount(client, rb, server.Config()) { + return false } sendSuccessfulAccountAuth(client, rb, false, true) diff --git a/irc/nickname.go b/irc/nickname.go index 2c26b86b..1a1677b6 100644 --- a/irc/nickname.go +++ b/irc/nickname.go @@ -37,9 +37,9 @@ func performNickChange(server *Server, client *Client, target *Client, session * assignedNickname, 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")) + rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, utils.SafeErrorParam(nickname), client.t("Nickname is already in use")) } else if err == errNicknameReserved { - rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, nickname, client.t("Nickname is reserved by a different account")) + rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, utils.SafeErrorParam(nickname), client.t("Nickname is reserved by a different account")) } else if err == errNicknameInvalid { rb.Add(nil, server.name, ERR_ERRONEUSNICKNAME, currentNick, utils.SafeErrorParam(nickname), client.t("Erroneous nickname")) } else if err == errNickAccountMismatch { @@ -108,3 +108,23 @@ func (server *Server) RandomlyRename(client *Client) { // technically performNickChange can fail to change the nick, // but if they're still delinquent, the timer will get them later } + +// if force-nick-equals-account is set, account name and nickname must be equal, +// so we need to re-NICK automatically on every login event (IDENTIFY, +// VERIFY, and a REGISTER that auto-verifies). if we can't get the nick +// then we log them out (they will be able to reattach with SASL) +func fixupNickEqualsAccount(client *Client, rb *ResponseBuffer, config *Config) (success bool) { + if !config.Accounts.NickReservation.ForceNickEqualsAccount { + return true + } + if !client.registered { + return true + } + // don't need to supply a nickname, SetNick will use the account name + if !performNickChange(client.server, client, client, rb.session, "", rb) { + client.server.accounts.Logout(client) + nsNotice(rb, client.t("A client is already using that account; try logging out and logging back in with SASL")) + return false + } + return true +} diff --git a/irc/nickserv.go b/irc/nickserv.go index c6dfa663..166c7b37 100644 --- a/irc/nickserv.go +++ b/irc/nickserv.go @@ -610,23 +610,6 @@ func nsLoginThrottleCheck(client *Client, rb *ResponseBuffer) (success bool) { return true } -// if force-nick-equals-account is set, account name and nickname must be equal, -// so we need to re-NICK automatically on every login event (IDENTIFY, -// VERIFY, and a REGISTER that auto-verifies). if we can't get the nick -// then we log them out (they will be able to reattach with SASL) -func nsFixNickname(client *Client, rb *ResponseBuffer, config *Config) (success bool) { - if !config.Accounts.NickReservation.ForceNickEqualsAccount { - return true - } - // don't need to supply a nickname, SetNick will use the account name - if !performNickChange(client.server, client, client, rb.session, "", rb) { - client.server.accounts.Logout(client) - nsNotice(rb, client.t("A client is already using that account; try logging out and logging back in with SASL")) - return false - } - return true -} - func nsIdentifyHandler(server *Server, client *Client, command string, params []string, rb *ResponseBuffer) { if client.LoggedIntoAccount() { nsNotice(rb, client.t("You're already logged into an account")) @@ -666,17 +649,19 @@ func nsIdentifyHandler(server *Server, client *Client, command string, params [] loginSuccessful = (err == nil) } + nickFixupFailed := false if loginSuccessful { - if !nsFixNickname(client, rb, server.Config()) { + if !fixupNickEqualsAccount(client, rb, server.Config()) { loginSuccessful = false - err = errNickAccountMismatch + // fixupNickEqualsAccount sends its own error message, don't send another + nickFixupFailed = true } } if loginSuccessful { sendSuccessfulAccountAuth(client, rb, true, true) - } else if err != errNickAccountMismatch { - nsNotice(rb, client.t("Could not login with your TLS certificate or supplied username/password")) + } else if !nickFixupFailed { + nsNotice(rb, fmt.Sprintf(client.t("Authentication failed: %s"), authErrorToMessage(server, err))) } } @@ -786,7 +771,7 @@ func nsRegisterHandler(server *Server, client *Client, command string, params [] if err == nil { if callbackNamespace == "*" { err = server.accounts.Verify(client, account, "") - if err == nil && nsFixNickname(client, rb, config) { + if err == nil && fixupNickEqualsAccount(client, rb, config) { sendSuccessfulRegResponse(client, rb, true) } } else { @@ -892,7 +877,7 @@ func nsVerifyHandler(server *Server, client *Client, command string, params []st return } - if nsFixNickname(client, rb, server.Config()) { + if fixupNickEqualsAccount(client, rb, server.Config()) { sendSuccessfulRegResponse(client, rb, true) } }