From b9b2553a2f9062df78f20252ece3a3c7f3c096f3 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 30 Jan 2019 18:59:49 -0500 Subject: [PATCH 1/4] use the TR39 skeleton algorithm to prevent confusables (#178) --- irc/accounts.go | 130 +++++++++++++++++++++++++++++---------- irc/client.go | 38 ++++++------ irc/client_lookup_set.go | 46 +++++++++++--- irc/getters.go | 15 +++-- irc/handlers.go | 1 - irc/idletimer.go | 6 +- irc/nickserv.go | 2 +- irc/strings.go | 62 +++++++++++++++++-- irc/strings_test.go | 47 ++++++++++++++ 9 files changed, 271 insertions(+), 76 deletions(-) diff --git a/irc/accounts.go b/irc/accounts.go index 0168281a..c9ee894d 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -52,17 +52,19 @@ type AccountManager struct { server *Server // track clients logged in to accounts - accountToClients map[string][]*Client - nickToAccount map[string]string - accountToMethod map[string]NickReservationMethod + accountToClients map[string][]*Client + nickToAccount map[string]string + skeletonToAccount map[string]string + accountToMethod map[string]NickReservationMethod } func NewAccountManager(server *Server) *AccountManager { am := AccountManager{ - accountToClients: make(map[string][]*Client), - nickToAccount: make(map[string]string), - accountToMethod: make(map[string]NickReservationMethod), - server: server, + accountToClients: make(map[string][]*Client), + nickToAccount: make(map[string]string), + skeletonToAccount: make(map[string]string), + accountToMethod: make(map[string]NickReservationMethod), + server: server, } am.buildNickToAccountIndex() @@ -76,6 +78,7 @@ func (am *AccountManager) buildNickToAccountIndex() { } nickToAccount := make(map[string]string) + skeletonToAccount := make(map[string]string) accountToMethod := make(map[string]NickReservationMethod) existsPrefix := fmt.Sprintf(keyAccountExists, "") @@ -91,11 +94,21 @@ func (am *AccountManager) buildNickToAccountIndex() { account := strings.TrimPrefix(key, existsPrefix) if _, err := tx.Get(fmt.Sprintf(keyAccountVerified, account)); err == nil { nickToAccount[account] = account + accountName, err := tx.Get(fmt.Sprintf(keyAccountName, account)) + if err != nil { + am.server.logger.Error("internal", "missing account name for", account) + } else { + skeleton, _ := Skeleton(accountName) + skeletonToAccount[skeleton] = account + } } if rawNicks, err := tx.Get(fmt.Sprintf(keyAccountAdditionalNicks, account)); err == nil { additionalNicks := unmarshalReservedNicks(rawNicks) for _, nick := range additionalNicks { - nickToAccount[nick] = account + cfnick, _ := CasefoldName(nick) + nickToAccount[cfnick] = account + skeleton, _ := Skeleton(nick) + skeletonToAccount[skeleton] = account } } @@ -115,6 +128,7 @@ func (am *AccountManager) buildNickToAccountIndex() { } else { am.Lock() am.nickToAccount = nickToAccount + am.skeletonToAccount = skeletonToAccount am.accountToMethod = accountToMethod am.Unlock() } @@ -171,36 +185,55 @@ func (am *AccountManager) NickToAccount(nick string) string { // Given a nick, looks up the account that owns it and the method (none/timeout/strict) // used to enforce ownership. -func (am *AccountManager) EnforcementStatus(nick string) (account string, method NickReservationMethod) { - cfnick, err := CasefoldName(nick) - if err != nil { - return - } - +func (am *AccountManager) EnforcementStatus(cfnick, skeleton string) (account string, method NickReservationMethod) { config := am.server.Config() if !config.Accounts.NickReservation.Enabled { - method = NickReservationNone - return + return "", NickReservationNone } am.RLock() defer am.RUnlock() - account = am.nickToAccount[cfnick] - if account == "" { - method = NickReservationNone + // given an account, combine stored enforcement method with the config settings + // to compute the actual enforcement method + finalEnforcementMethod := func(account_ string) (result NickReservationMethod) { + result = am.accountToMethod[account_] + // if they don't have a custom setting, or customization is disabled, use the default + if result == NickReservationOptional || !config.Accounts.NickReservation.AllowCustomEnforcement { + result = config.Accounts.NickReservation.Method + } + if result == NickReservationOptional { + // enforcement was explicitly enabled neither in the config or by the user + result = NickReservationNone + } return } - method = am.accountToMethod[account] - // if they don't have a custom setting, or customization is disabled, use the default - if method == NickReservationOptional || !config.Accounts.NickReservation.AllowCustomEnforcement { - method = config.Accounts.NickReservation.Method + + nickAccount := am.nickToAccount[cfnick] + skelAccount := am.skeletonToAccount[skeleton] + if nickAccount == "" && skelAccount == "" { + return "", NickReservationNone + } else if nickAccount != "" && skelAccount != "" && nickAccount != skelAccount { + // two people have competing claims on (this casefolding of) this nick! + nickMethod := finalEnforcementMethod(nickAccount) + skelMethod := finalEnforcementMethod(skelAccount) + switch { + case nickMethod == NickReservationNone && skelMethod == NickReservationNone: + return "", NickReservationNone + case skelMethod == NickReservationNone: + return nickAccount, nickMethod + case nickMethod == NickReservationNone: + return skelAccount, skelMethod + default: + // nobody can use this nick + return "!", NickReservationStrict + } + } else if nickAccount == "" && skelAccount != "" { + // skeleton owner is the only owner; fall through to normal case + nickAccount = skelAccount } - if method == NickReservationOptional { - // enforcement was explicitly enabled neither in the config or by the user - method = NickReservationNone - } - return + // else: nickAccount != "" && skelAccount == "", nickAccount is the only owner + return nickAccount, finalEnforcementMethod(nickAccount) } // Looks up the enforcement method stored in the database for an account @@ -264,10 +297,15 @@ func (am *AccountManager) AccountToClients(account string) (result []*Client) { func (am *AccountManager) Register(client *Client, account string, callbackNamespace string, callbackValue string, passphrase string, certfp string) error { casefoldedAccount, err := CasefoldName(account) - if err != nil || account == "" || account == "*" { + skeleton, skerr := Skeleton(account) + if err != nil || skerr != nil || account == "" || account == "*" { return errAccountCreation } + if restrictedNicknames[casefoldedAccount] || restrictedNicknames[skeleton] { + return errAccountAlreadyRegistered + } + // can't register a guest nickname config := am.server.AccountConfig() renamePrefix := strings.ToLower(config.NickReservation.RenamePrefix) @@ -535,8 +573,10 @@ func (am *AccountManager) Verify(client *Client, account string, code string) er }) if err == nil { + skeleton, _ := Skeleton(raw.Name) am.Lock() am.nickToAccount[casefoldedAccount] = casefoldedAccount + am.skeletonToAccount[skeleton] = casefoldedAccount am.Unlock() } }() @@ -567,9 +607,10 @@ func unmarshalReservedNicks(nicks string) (result []string) { func (am *AccountManager) SetNickReserved(client *Client, nick string, saUnreserve bool, reserve bool) error { cfnick, err := CasefoldName(nick) + skeleton, skerr := Skeleton(nick) // garbage nick, or garbage options, or disabled nrconfig := am.server.AccountConfig().NickReservation - if err != nil || cfnick == "" || (reserve && saUnreserve) || !nrconfig.Enabled { + if err != nil || skerr != nil || cfnick == "" || (reserve && saUnreserve) || !nrconfig.Enabled { return errAccountNickReservationFailed } @@ -591,8 +632,15 @@ func (am *AccountManager) SetNickReserved(client *Client, nick string, saUnreser return errAccountNotLoggedIn } - accountForNick := am.NickToAccount(cfnick) - if reserve && accountForNick != "" { + am.Lock() + accountForNick := am.nickToAccount[cfnick] + var accountForSkeleton string + if reserve { + accountForSkeleton = am.skeletonToAccount[skeleton] + } + am.Unlock() + + if reserve && (accountForNick != "" || accountForSkeleton != "") { return errNicknameReserved } else if !reserve && !saUnreserve && accountForNick != account { return errNicknameReserved @@ -623,12 +671,18 @@ func (am *AccountManager) SetNickReserved(client *Client, nick string, saUnreser if len(nicks) >= nrconfig.AdditionalNickLimit { return errAccountTooManyNicks } - nicks = append(nicks, cfnick) + nicks = append(nicks, nick) } else { + // compute (original reserved nicks) minus cfnick var newNicks []string for _, reservedNick := range nicks { - if reservedNick != cfnick { + cfreservednick, _ := CasefoldName(reservedNick) + if cfreservednick != cfnick { newNicks = append(newNicks, reservedNick) + } else { + // found the original, unfolded version of the nick we're dropping; + // recompute the true skeleton from it + skeleton, _ = Skeleton(reservedNick) } } nicks = newNicks @@ -650,8 +704,10 @@ func (am *AccountManager) SetNickReserved(client *Client, nick string, saUnreser defer am.Unlock() if reserve { am.nickToAccount[cfnick] = account + am.skeletonToAccount[skeleton] = account } else { delete(am.nickToAccount, cfnick) + delete(am.skeletonToAccount, skeleton) } return nil } @@ -787,8 +843,10 @@ func (am *AccountManager) Unregister(account string) error { am.serialCacheUpdateMutex.Lock() defer am.serialCacheUpdateMutex.Unlock() + var accountName string am.server.store.Update(func(tx *buntdb.Tx) error { tx.Delete(accountKey) + accountName, _ = tx.Get(accountNameKey) tx.Delete(accountNameKey) tx.Delete(verifiedKey) tx.Delete(registeredTimeKey) @@ -817,6 +875,7 @@ func (am *AccountManager) Unregister(account string) error { } } + skeleton, _ := Skeleton(accountName) additionalNicks := unmarshalReservedNicks(rawNicks) am.Lock() @@ -825,8 +884,11 @@ func (am *AccountManager) Unregister(account string) error { clients = am.accountToClients[casefoldedAccount] delete(am.accountToClients, casefoldedAccount) delete(am.nickToAccount, casefoldedAccount) + delete(am.skeletonToAccount, skeleton) for _, nick := range additionalNicks { delete(am.nickToAccount, nick) + additionalSkel, _ := Skeleton(nick) + delete(am.skeletonToAccount, additionalSkel) } for _, client := range clients { am.logoutOfAccount(client) diff --git a/irc/client.go b/irc/client.go index 8825f9ef..9fc041ae 100644 --- a/irc/client.go +++ b/irc/client.go @@ -95,6 +95,7 @@ type Client struct { saslMechanism string saslValue string server *Server + skeleton string socket *Socket stateMutex sync.RWMutex // tier 1 username string @@ -381,7 +382,7 @@ func (client *Client) Register() { client.TryResume() // finish registration - client.updateNickMask("") + client.updateNickMask() client.server.monitorManager.AlertAbout(client, true) } @@ -565,6 +566,7 @@ func (client *Client) copyResumeData(oldClient *Client) { vhost := oldClient.vhost account := oldClient.account accountName := oldClient.accountName + skeleton := oldClient.skeleton oldClient.stateMutex.RUnlock() // copy all flags, *except* TLS (in the case that the admins enabled @@ -586,6 +588,7 @@ func (client *Client) copyResumeData(oldClient *Client) { client.vhost = vhost client.account = account client.accountName = accountName + client.skeleton = skeleton client.updateNickMaskNoMutex() } @@ -696,6 +699,14 @@ func (client *Client) Friends(capabs ...caps.Capability) ClientSet { return friends } +func (client *Client) SetOper(oper *Oper) { + client.stateMutex.Lock() + defer client.stateMutex.Unlock() + client.oper = oper + // operators typically get a vhost, update the nickmask + client.updateNickMaskNoMutex() +} + // XXX: CHGHOST requires prefix nickmask to have original hostname, // this is annoying to do correctly func (client *Client) sendChghost(oldNickMask string, vhost string) { @@ -730,32 +741,23 @@ func (client *Client) SetVHost(vhost string) (updated bool) { } // updateNick updates `nick` and `nickCasefolded`. -func (client *Client) updateNick(nick string) { - casefoldedName, err := CasefoldName(nick) - if err != nil { - client.server.logger.Error("internal", "nick couldn't be casefolded", nick, err.Error()) - return - } +func (client *Client) updateNick(nick, nickCasefolded, skeleton string) { client.stateMutex.Lock() + defer client.stateMutex.Unlock() client.nick = nick - client.nickCasefolded = casefoldedName - client.stateMutex.Unlock() + client.nickCasefolded = nickCasefolded + client.skeleton = skeleton + client.updateNickMaskNoMutex() } -// updateNickMask updates the casefolded nickname and nickmask. -func (client *Client) updateNickMask(nick string) { - // on "", just regenerate the nickmask etc. - // otherwise, update the actual nick - if nick != "" { - client.updateNick(nick) - } - +// updateNickMask updates the nickmask. +func (client *Client) updateNickMask() { client.stateMutex.Lock() defer client.stateMutex.Unlock() client.updateNickMaskNoMutex() } -// updateNickMask updates the casefolded nickname and nickmask, not acquiring any mutexes. +// updateNickMaskNoMutex updates the casefolded nickname and nickmask, not acquiring any mutexes. func (client *Client) updateNickMaskNoMutex() { client.hostname = client.getVHostNoMutex() if client.hostname == "" { diff --git a/irc/client_lookup_set.go b/irc/client_lookup_set.go index 74bfb318..ec98fc5f 100644 --- a/irc/client_lookup_set.go +++ b/irc/client_lookup_set.go @@ -34,12 +34,14 @@ func ExpandUserHost(userhost string) (expanded string) { type ClientManager struct { sync.RWMutex // tier 2 byNick map[string]*Client + bySkeleton map[string]*Client } // NewClientManager returns a new ClientManager. func NewClientManager() *ClientManager { return &ClientManager{ - byNick: make(map[string]*Client), + byNick: make(map[string]*Client), + bySkeleton: make(map[string]*Client), } } @@ -65,7 +67,11 @@ func (clients *ClientManager) Get(nick string) *Client { func (clients *ClientManager) removeInternal(client *Client) (err error) { // requires holding the writable Lock() - oldcfnick := client.NickCasefolded() + oldcfnick, oldskeleton := client.uniqueIdentifiers() + if oldcfnick == "*" || oldcfnick == "" { + return errNickMissing + } + currentEntry, present := clients.byNick[oldcfnick] if present { if currentEntry == client { @@ -75,7 +81,22 @@ func (clients *ClientManager) removeInternal(client *Client) (err error) { client.server.logger.Warning("internal", "clients for nick out of sync", oldcfnick) err = errNickMissing } + } else { + err = errNickMissing } + + currentEntry, present = clients.bySkeleton[oldskeleton] + if present { + if currentEntry == client { + delete(clients.bySkeleton, oldskeleton) + } else { + client.server.logger.Warning("internal", "clients for skeleton out of sync", oldskeleton) + err = errNickMissing + } + } else { + err = errNickMissing + } + return } @@ -84,9 +105,6 @@ func (clients *ClientManager) Remove(client *Client) error { clients.Lock() defer clients.Unlock() - if !client.HasNick() { - return errNickMissing - } return clients.removeInternal(client) } @@ -105,7 +123,9 @@ func (clients *ClientManager) Resume(newClient, oldClient *Client) (err error) { } // nick has been reclaimed, grant it to the new client clients.removeInternal(newClient) - clients.byNick[oldClient.NickCasefolded()] = newClient + oldcfnick, oldskeleton := oldClient.uniqueIdentifiers() + clients.byNick[oldcfnick] = newClient + clients.bySkeleton[oldskeleton] = newClient newClient.copyResumeData(oldClient) @@ -118,8 +138,12 @@ func (clients *ClientManager) SetNick(client *Client, newNick string) error { if err != nil { return err } + newSkeleton, err := Skeleton(newNick) + if err != nil { + return err + } - reservedAccount, method := client.server.accounts.EnforcementStatus(newcfnick) + reservedAccount, method := client.server.accounts.EnforcementStatus(newcfnick, newSkeleton) clients.Lock() defer clients.Unlock() @@ -129,12 +153,18 @@ func (clients *ClientManager) SetNick(client *Client, newNick string) error { if currentNewEntry != nil && currentNewEntry != client { return errNicknameInUse } + // analogous checks for skeletons + skeletonHolder := clients.bySkeleton[newSkeleton] + if skeletonHolder != nil && skeletonHolder != client { + return errNicknameInUse + } if method == NickReservationStrict && reservedAccount != "" && reservedAccount != client.Account() { return errNicknameReserved } clients.removeInternal(client) clients.byNick[newcfnick] = client - client.updateNickMask(newNick) + clients.bySkeleton[newSkeleton] = client + client.updateNick(newNick, newcfnick, newSkeleton) return nil } diff --git a/irc/getters.go b/irc/getters.go index c1f65afa..99878a26 100644 --- a/irc/getters.go +++ b/irc/getters.go @@ -108,6 +108,15 @@ func (client *Client) Realname() string { return client.realname } +// uniqueIdentifiers returns the strings for which the server enforces per-client +// uniqueness/ownership; no two clients can have colliding casefolded nicks or +// skeletons. +func (client *Client) uniqueIdentifiers() (nickCasefolded string, skeleton string) { + client.stateMutex.RLock() + defer client.stateMutex.RUnlock() + return client.nickCasefolded, client.skeleton +} + func (client *Client) ResumeToken() string { client.stateMutex.RLock() defer client.stateMutex.RUnlock() @@ -120,12 +129,6 @@ func (client *Client) Oper() *Oper { return client.oper } -func (client *Client) SetOper(oper *Oper) { - client.stateMutex.Lock() - defer client.stateMutex.Unlock() - client.oper = oper -} - func (client *Client) Registered() bool { client.stateMutex.RLock() defer client.stateMutex.RUnlock() diff --git a/irc/handlers.go b/irc/handlers.go index b8075296..3dc8dc69 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -1706,7 +1706,6 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp oldNickmask := client.NickMaskString() client.SetOper(oper) - client.updateNickMask("") if client.NickMaskString() != oldNickmask { client.sendChghost(oldNickmask, oper.Vhost) } diff --git a/irc/idletimer.go b/irc/idletimer.go index 2aa32a58..64b22494 100644 --- a/irc/idletimer.go +++ b/irc/idletimer.go @@ -205,9 +205,9 @@ func (nt *NickTimer) Touch() { return } - nick := nt.client.NickCasefolded() + cfnick, skeleton := nt.client.uniqueIdentifiers() account := nt.client.Account() - accountForNick, method := nt.client.server.accounts.EnforcementStatus(nick) + accountForNick, method := nt.client.server.accounts.EnforcementStatus(cfnick, skeleton) enforceTimeout := method == NickReservationWithTimeout var shouldWarn bool @@ -223,7 +223,7 @@ func (nt *NickTimer) Touch() { // the timer will not reset as long as the squatter is targeting the same account accountChanged := accountForNick != nt.accountForNick // change state - nt.nick = nick + nt.nick = cfnick nt.account = account nt.accountForNick = accountForNick delinquent := accountForNick != "" && accountForNick != account diff --git a/irc/nickserv.go b/irc/nickserv.go index 602b1b1a..16d486c1 100644 --- a/irc/nickserv.go +++ b/irc/nickserv.go @@ -215,7 +215,7 @@ func nsGhostHandler(server *Server, client *Client, command string, params []str } func nsGroupHandler(server *Server, client *Client, command string, params []string, rb *ResponseBuffer) { - nick := client.NickCasefolded() + nick := client.Nick() err := server.accounts.SetNickReserved(client, nick, false, true) if err == nil { nsNotice(rb, fmt.Sprintf(client.t("Successfully grouped nick %s with your account"), nick)) diff --git a/irc/strings.go b/irc/strings.go index c5385853..87c41d6a 100644 --- a/irc/strings.go +++ b/irc/strings.go @@ -8,21 +8,25 @@ package irc import ( "strings" + "github.com/mtibben/confusables" "golang.org/x/text/secure/precis" + "golang.org/x/text/unicode/norm" ) const ( casemappingName = "rfc8265" ) -// Casefold returns a casefolded string, without doing any name or channel character checks. -func Casefold(str string) (string, error) { - var err error - oldStr := str +// Each pass of PRECIS casefolding is a composition of idempotent operations, +// but not idempotent itself. Therefore, the spec says "do it four times and hope +// it converges" (lolwtf). Golang's PRECIS implementation has a "repeat" option, +// which provides this functionality, but unfortunately it's not exposed publicly. +func iterateFolding(profile *precis.Profile, oldStr string) (str string, err error) { + str = oldStr // follow the stabilizing rules laid out here: // https://tools.ietf.org/html/draft-ietf-precis-7564bis-10.html#section-7 for i := 0; i < 4; i++ { - str, err = precis.UsernameCaseMapped.CompareKey(str) + str, err = profile.CompareKey(str) if err != nil { return "", err } @@ -37,6 +41,11 @@ func Casefold(str string) (string, error) { return str, nil } +// Casefold returns a casefolded string, without doing any name or channel character checks. +func Casefold(str string) (string, error) { + return iterateFolding(precis.UsernameCaseMapped, str) +} + // CasefoldChannel returns a casefolded version of a channel name. func CasefoldChannel(name string) (string, error) { if len(name) == 0 { @@ -96,3 +105,46 @@ func CasefoldName(name string) (string, error) { return lowered, err } + +// "boring" names are exempt from skeletonization. +// this is because confusables.txt considers various pure ASCII alphanumeric +// strings confusable: 0 and O, 1 and l, m and rn. IMO this causes more problems +// than it solves. +func isBoring(name string) bool { + for i := 0; i < len(name); i += 1 { + chr := name[i] + if (chr >= 'a' && chr <= 'z') || (chr >= 'A' && chr <= 'Z') || (chr >= '0' && chr <= '9') { + continue // alphanumerics + } + switch chr { + case '$', '%', '^', '&', '(', ')', '{', '}', '[', ']', '<', '>', '=': + continue // benign printable ascii characters + default: + return false // potentially confusable ascii like | ' `, non-ascii + } + } + return true +} + +var skeletonCasefolder = precis.NewIdentifier(precis.FoldWidth, precis.LowerCase(), precis.Norm(norm.NFC)) + +// similar to Casefold, but exempts the bidi rule, because skeletons may +// mix scripts strangely +func casefoldSkeleton(str string) (string, error) { + return iterateFolding(skeletonCasefolder, str) +} + +// Skeleton produces a canonicalized identifier that tries to catch +// homoglyphic / confusable identifiers. It's a tweaked version of the TR39 +// skeleton algorithm. We apply the skeleton algorithm first and only then casefold, +// because casefolding first would lose some information about visual confusability. +// This has the weird consequence that the skeleton is not a function of the +// casefolded identifier --- therefore it must always be computed +// from the original (unfolded) identifier and stored/tracked separately from the +// casefolded identifier. +func Skeleton(name string) (string, error) { + if !isBoring(name) { + name = confusables.Skeleton(name) + } + return casefoldSkeleton(name) +} diff --git a/irc/strings_test.go b/irc/strings_test.go index 48a7d538..0f4c87de 100644 --- a/irc/strings_test.go +++ b/irc/strings_test.go @@ -127,3 +127,50 @@ func TestCasefoldName(t *testing.T) { }) } } + +func TestIsBoring(t *testing.T) { + assertBoring := func(str string, expected bool) { + if isBoring(str) != expected { + t.Errorf("expected [%s] to have boringness [%t], but got [%t]", str, expected, !expected) + } + } + + assertBoring("warning", true) + assertBoring("phi|ip", false) + assertBoring("Νικηφόρος", false) +} + +func TestSkeleton(t *testing.T) { + skeleton := func(str string) string { + skel, err := Skeleton(str) + if err != nil { + t.Error(err) + } + return skel + } + + if skeleton("warning") == skeleton("waming") { + t.Errorf("Oragono shouldn't consider rn confusable with m") + } + + if skeleton("Phi|ip") != "philip" { + t.Errorf("but we still consider pipe confusable with l") + } + + if skeleton("ｓｍｔ") != "smt" { + t.Errorf("fullwidth characters should skeletonize to plain old ascii characters") + } + + if skeleton("ＳＭＴ") != "smt" { + t.Errorf("after skeletonizing, we should casefold") + } + + if skeleton("еvan") != "evan" { + t.Errorf("we must protect against cyrillic homoglyph attacks") + } + + if skeleton("РОТАТО") != "potato" { + t.Errorf("we must protect against cyrillic homoglyph attacks") + } + +} From 18c0c9da45e785f0eac4145441c59cfd056229de Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 30 Jan 2019 21:51:54 -0500 Subject: [PATCH 2/4] use our fork of mtibben/confusables for now --- Gopkg.lock | 9 +++++++++ Gopkg.toml | 4 ++++ irc/strings.go | 2 +- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Gopkg.lock b/Gopkg.lock index d475c98a..5f56b548 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -61,6 +61,13 @@ pruneopts = "UT" revision = "9520e82c474b0a04dd04f8a40959027271bab992" +[[projects]] + digest = "1:7caf3ea977a13cd8b9a2e1ecef1ccaa8e38f831b4f6ffcb8bd0aa909c48afb3a" + name = "github.com/oragono/confusables" + packages = ["."] + pruneopts = "UT" + revision = "d5dd03409482fae2457f0742be22782890f720c2" + [[projects]] branch = "master" digest = "1:2251e6a17ea4a6eaa708882a1cda837aae3e425edbb190ef39b761ecf15a5c3d" @@ -199,12 +206,14 @@ "github.com/goshuirc/irc-go/ircmsg", "github.com/mattn/go-colorable", "github.com/mgutz/ansi", + "github.com/oragono/confusables", "github.com/oragono/go-ident", "github.com/tidwall/buntdb", "golang.org/x/crypto/bcrypt", "golang.org/x/crypto/sha3", "golang.org/x/crypto/ssh/terminal", "golang.org/x/text/secure/precis", + "golang.org/x/text/unicode/norm", "gopkg.in/yaml.v2", ] solver-name = "gps-cdcl" diff --git a/Gopkg.toml b/Gopkg.toml index ff37dd11..380726f7 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -49,6 +49,10 @@ branch = "master" name = "github.com/oragono/go-ident" +[[constraint]] + revision = "d5dd03409482fae2457f0742be22782890f720c2" + name = "github.com/oragono/confusables" + [[constraint]] name = "github.com/tidwall/buntdb" version = "1.0.0" diff --git a/irc/strings.go b/irc/strings.go index 87c41d6a..d475120f 100644 --- a/irc/strings.go +++ b/irc/strings.go @@ -8,7 +8,7 @@ package irc import ( "strings" - "github.com/mtibben/confusables" + "github.com/oragono/confusables" "golang.org/x/text/secure/precis" "golang.org/x/text/unicode/norm" ) From bea3889a1fa94bfe7da7a7f0fb783996c2449533 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 30 Jan 2019 22:01:16 -0500 Subject: [PATCH 3/4] bump vendor to include oragono/confusables --- vendor | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor b/vendor index 77ddc3db..0d667e5d 160000 --- a/vendor +++ b/vendor @@ -1 +1 @@ -Subproject commit 77ddc3dbc1ec085c73670510a8fece80599741ce +Subproject commit 0d667e5d09fd0a2041154eca9cdd1915b9843453 From 35948d2e5ba8d895aee4dddabe914d82f5d4c6a4 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Thu, 31 Jan 2019 17:34:06 -0500 Subject: [PATCH 4/4] refactor some conditions for clarity --- irc/accounts.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/irc/accounts.go b/irc/accounts.go index c9ee894d..48c64445 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -213,13 +213,18 @@ func (am *AccountManager) EnforcementStatus(cfnick, skeleton string) (account st skelAccount := am.skeletonToAccount[skeleton] if nickAccount == "" && skelAccount == "" { return "", NickReservationNone - } else if nickAccount != "" && skelAccount != "" && nickAccount != skelAccount { + } else if nickAccount != "" && (skelAccount == nickAccount || skelAccount == "") { + return nickAccount, finalEnforcementMethod(nickAccount) + } else if skelAccount != "" && nickAccount == "" { + return skelAccount, finalEnforcementMethod(skelAccount) + } else { + // nickAccount != skelAccount and both are nonempty: // two people have competing claims on (this casefolding of) this nick! nickMethod := finalEnforcementMethod(nickAccount) skelMethod := finalEnforcementMethod(skelAccount) switch { case nickMethod == NickReservationNone && skelMethod == NickReservationNone: - return "", NickReservationNone + return nickAccount, NickReservationNone case skelMethod == NickReservationNone: return nickAccount, nickMethod case nickMethod == NickReservationNone: @@ -228,12 +233,7 @@ func (am *AccountManager) EnforcementStatus(cfnick, skeleton string) (account st // nobody can use this nick return "!", NickReservationStrict } - } else if nickAccount == "" && skelAccount != "" { - // skeleton owner is the only owner; fall through to normal case - nickAccount = skelAccount } - // else: nickAccount != "" && skelAccount == "", nickAccount is the only owner - return nickAccount, finalEnforcementMethod(nickAccount) } // Looks up the enforcement method stored in the database for an account