From ab444a398061aea07ea757e755b0c9ce7b7d5311 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 18 Dec 2019 07:01:26 -0500 Subject: [PATCH 1/8] remove unnecessary uses of Casefold --- irc/client.go | 21 ++++++--------------- irc/config.go | 2 +- irc/handlers.go | 2 +- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/irc/client.go b/irc/client.go index e3ef9034..f23035cb 100644 --- a/irc/client.go +++ b/irc/client.go @@ -948,12 +948,7 @@ func (client *Client) updateNickMaskNoMutex() { return // pre-registration, don't bother generating the hostname } - cfhostname, err := Casefold(client.hostname) - if err != nil { - client.server.logger.Error("internal", "hostname couldn't be casefolded", client.hostname, err.Error()) - cfhostname = client.hostname // YOLO - } - + cfhostname := strings.ToLower(client.hostname) client.nickMaskString = fmt.Sprintf("%s!%s@%s", client.nick, client.username, client.hostname) client.nickMaskCasefolded = fmt.Sprintf("%s!%s@%s", client.nickCasefolded, strings.ToLower(client.username), cfhostname) } @@ -970,18 +965,14 @@ func (client *Client) AllNickmasks() (masks []string) { username = strings.ToLower(username) if len(vhost) > 0 { - cfvhost, err := Casefold(vhost) - if err == nil { - masks = append(masks, fmt.Sprintf("%s!%s@%s", nick, username, cfvhost)) - } + cfvhost := strings.ToLower(vhost) + masks = append(masks, fmt.Sprintf("%s!%s@%s", nick, username, cfvhost)) } var rawhostmask string - cfrawhost, err := Casefold(rawHostname) - if err == nil { - rawhostmask = fmt.Sprintf("%s!%s@%s", nick, username, cfrawhost) - masks = append(masks, rawhostmask) - } + cfrawhost := strings.ToLower(rawHostname) + rawhostmask = fmt.Sprintf("%s!%s@%s", nick, username, cfrawhost) + masks = append(masks, rawhostmask) if cloakedHostname != "" { masks = append(masks, fmt.Sprintf("%s!%s@%s", nick, username, cloakedHostname)) diff --git a/irc/config.go b/irc/config.go index 464bcb00..533c4582 100644 --- a/irc/config.go +++ b/irc/config.go @@ -783,7 +783,7 @@ func LoadConfig(filename string) (config *Config, err error) { } // casefold/validate server name - config.Server.nameCasefolded, err = Casefold(config.Server.Name) + config.Server.nameCasefolded = strings.ToLower(config.Server.Name) if err != nil { return nil, fmt.Errorf("Server name isn't valid [%s]: %s", config.Server.Name, err.Error()) } diff --git a/irc/handlers.go b/irc/handlers.go index dbcc5cb7..43c3273f 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2675,7 +2675,7 @@ func whoHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo } else if mask[0] == '#' { mask, err = CasefoldChannel(msg.Params[0]) } else { - mask, err = Casefold(mask) + mask, err = CanonicalizeMaskWildcard(mask) } if err != nil { From 91d6888b7e7829318b361f11c095913ecec5967f Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 18 Dec 2019 07:06:04 -0500 Subject: [PATCH 2/8] fix #693 --- irc/config.go | 22 ++++++++++++++ irc/server.go | 7 ++++- irc/strings.go | 70 +++++++++++++++++++++++++++++++++++++++++++-- irc/strings_test.go | 22 ++++++++++++++ oragono.yaml | 12 ++++++++ 5 files changed, 130 insertions(+), 3 deletions(-) diff --git a/irc/config.go b/irc/config.go index 533c4582..1a187604 100644 --- a/irc/config.go +++ b/irc/config.go @@ -182,6 +182,27 @@ func (nr *NickEnforcementMethod) UnmarshalYAML(unmarshal func(interface{}) error return err } +func (cm *Casemapping) UnmarshalYAML(unmarshal func(interface{}) error) (err error) { + var orig string + if err = unmarshal(&orig); err != nil { + return err + } + + var result Casemapping + switch strings.ToLower(orig) { + case "ascii": + result = CasemappingASCII + case "precis", "rfc7613", "rfc8265": + result = CasemappingPRECIS + case "permissive", "fun": + result = CasemappingPermissive + default: + return fmt.Errorf("invalid casemapping value: %s", orig) + } + *cm = result + return nil +} + type NickReservationConfig struct { Enabled bool AdditionalNickLimit int `yaml:"additional-nick-limit"` @@ -318,6 +339,7 @@ type Config struct { Cloaks cloaks.CloakConfig `yaml:"ip-cloaking"` supportedCaps *caps.Set capValues caps.Values + Casemapping Casemapping } Languages struct { diff --git a/irc/server.go b/irc/server.go index 5c56ac4d..c1b9aece 100644 --- a/irc/server.go +++ b/irc/server.go @@ -165,7 +165,9 @@ func (config *Config) generateISupport() (err error) { isupport.Add("STATUSMSG", "~&@%+") isupport.Add("TARGMAX", fmt.Sprintf("NAMES:1,LIST:1,KICK:1,WHOIS:1,USERHOST:10,PRIVMSG:%s,TAGMSG:%s,NOTICE:%s,MONITOR:", maxTargetsString, maxTargetsString, maxTargetsString)) isupport.Add("TOPICLEN", strconv.Itoa(config.Limits.TopicLen)) - isupport.Add("UTF8MAPPING", casemappingName) + if globalCasemappingSetting == CasemappingPRECIS { + isupport.Add("UTF8MAPPING", precisUTF8MappingToken) + } err = isupport.RegenerateCachedReply() return @@ -596,6 +598,7 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { server.configFilename = config.Filename server.name = config.Server.Name server.nameCasefolded = config.Server.nameCasefolded + globalCasemappingSetting = config.Server.Casemapping } else { // enforce configs that can't be changed after launch: currentLimits := server.Config().Limits @@ -605,6 +608,8 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { return fmt.Errorf("Server name cannot be changed after launching the server, rehash aborted") } else if server.Config().Datastore.Path != config.Datastore.Path { return fmt.Errorf("Datastore path cannot be changed after launching the server, rehash aborted") + } else if globalCasemappingSetting != config.Server.Casemapping { + return fmt.Errorf("Casemapping cannot be changed after launching the server, rehash aborted") } } diff --git a/irc/strings.go b/irc/strings.go index e7ec40ea..e03f15d2 100644 --- a/irc/strings.go +++ b/irc/strings.go @@ -8,18 +8,43 @@ package irc import ( "fmt" "strings" + "unicode" "github.com/oragono/confusables" "golang.org/x/text/cases" "golang.org/x/text/language" "golang.org/x/text/secure/precis" + "golang.org/x/text/unicode/norm" "golang.org/x/text/width" ) const ( - casemappingName = "rfc8265" + precisUTF8MappingToken = "rfc8265" ) +type Casemapping uint + +const ( + // "precis" is the default / zero value: + // casefolding/validation: PRECIS + ircd restrictions (like no *) + // confusables detection: standard skeleton algorithm + CasemappingPRECIS Casemapping = iota + // "ascii" is the traditional ircd behavior: + // casefolding/validation: must be pure ASCII and follow ircd restrictions, ASCII lowercasing + // confusables detection: none + CasemappingASCII + // "permissive" is an insecure mode: + // casefolding/validation: arbitrary unicodes that follow ircd restrictions, unicode casefolding + // confusables detection: standard skeleton algorithm (which may be ineffective + // over the larger set of permitted identifiers) + CasemappingPermissive +) + +// XXX this is a global variable without explicit synchronization. +// it gets set during the initial Server.applyConfig and cannot be changed by rehash: +// this happens-before all IRC connections and all casefolding operations. +var globalCasemappingSetting Casemapping = CasemappingPRECIS + // 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, @@ -46,7 +71,14 @@ func iterateFolding(profile *precis.Profile, oldStr string) (str string, err err // Casefold returns a casefolded string, without doing any name or channel character checks. func Casefold(str string) (string, error) { - return iterateFolding(precis.UsernameCaseMapped, str) + switch globalCasemappingSetting { + default: + return iterateFolding(precis.UsernameCaseMapped, str) + case CasemappingASCII: + return foldASCII(str) + case CasemappingPermissive: + return foldPermissive(str) + } } // CasefoldChannel returns a casefolded version of a channel name. @@ -144,6 +176,16 @@ func isIdent(name string) bool { // from the original (unfolded) identifier and stored/tracked separately from the // casefolded identifier. func Skeleton(name string) (string, error) { + switch globalCasemappingSetting { + default: + return realSkeleton(name) + case CasemappingASCII: + // identity function is fine because we independently case-normalize in Casefold + return name, nil + } +} + +func realSkeleton(name string) (string, error) { // XXX the confusables table includes some, but not all, fullwidth->standard // mappings for latin characters. do a pass of explicit width folding, // same as PRECIS: @@ -212,3 +254,27 @@ func CanonicalizeMaskWildcard(userhost string) (expanded string, err error) { } return fmt.Sprintf("%s!%s@%s", nick, user, host), nil } + +func foldASCII(str string) (result string, err error) { + if !IsPureASCII(str) { + return "", errInvalidCharacter + } + return strings.ToLower(str), nil +} + +func IsPureASCII(str string) bool { + for i := 0; i < len(str); i++ { + if unicode.MaxASCII < str[i] { + return false + } + } + return true +} + +func foldPermissive(str string) (result string, err error) { + // YOLO + str = norm.NFD.String(str) + str = cases.Fold().String(str) + str = norm.NFD.String(str) + return str, nil +} diff --git a/irc/strings_test.go b/irc/strings_test.go index fdf7ddde..c2515cb5 100644 --- a/irc/strings_test.go +++ b/irc/strings_test.go @@ -215,3 +215,25 @@ func TestCanonicalizeMaskWildcard(t *testing.T) { tester("Shivaram*", "shivaram*!*@*", nil) tester("*SHIVARAM*", "*shivaram*!*@*", nil) } + +func TestFoldPermissive(t *testing.T) { + tester := func(first, second string, equal bool) { + firstFolded, err := foldPermissive(first) + if err != nil { + panic(err) + } + secondFolded, err := foldPermissive(second) + if err != nil { + panic(err) + } + foundEqual := firstFolded == secondFolded + if foundEqual != equal { + t.Errorf("%s and %s: expected equality %t, but got %t", first, second, equal, foundEqual) + } + } + tester("SHIVARAM", "shivaram", true) + tester("shIvaram", "shivaraM", true) + tester("shivaram", "DAN-", false) + tester("dolph🐬n", "DOLPH🐬n", true) + tester("dolph🐬n", "dolph💻n", false) +} diff --git a/oragono.yaml b/oragono.yaml index 1f2d9ef4..511c14e6 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -85,6 +85,18 @@ server: # should clients include this STS policy when they ship their inbuilt preload lists? preload: false + # casemapping controls what kinds of strings are permitted as identifiers (nicknames, + # channel names, account names, etc.), and how they are normalized for case. + # with the recommended default of 'precis', utf-8 identifiers that are "sane" + # (according to RFC 8265) are allowed, and the server additionally tries to protect + # against confusable characters ("homoglyph attacks"). + # the other options are 'ascii' (traditional ASCII-only identifiers), and 'permissive', + # which allows identifiers to contain unusual characters like emoji, but makes users + # vulnerable to homoglyph attacks. unless you're really confident in your decision, + # we recommend leaving this value at its default (changing it once the network is + # already up and running is problematic). + casemapping: precis + # whether to look up user hostnames with reverse DNS # (to suppress this for privacy purposes, use the ip-cloaking options below) lookup-hostnames: true From f9b5224ae0899c42327fda0ee46d88e5299962c3 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 18 Dec 2019 08:19:36 -0500 Subject: [PATCH 3/8] have realSkeleton use cases.Fold as well --- irc/strings.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/irc/strings.go b/irc/strings.go index e03f15d2..bf229f93 100644 --- a/irc/strings.go +++ b/irc/strings.go @@ -12,7 +12,6 @@ import ( "github.com/oragono/confusables" "golang.org/x/text/cases" - "golang.org/x/text/language" "golang.org/x/text/secure/precis" "golang.org/x/text/unicode/norm" "golang.org/x/text/width" @@ -198,7 +197,7 @@ func realSkeleton(name string) (string, error) { // violate the bidi rule). We also don't care if they contain runes // that are disallowed by PRECIS, because every identifier must independently // pass PRECIS --- we are just further canonicalizing the skeleton. - return cases.Lower(language.Und).String(name), nil + return cases.Fold().String(name), nil } // maps a nickmask fragment to an expanded, casefolded wildcard: From 82e965db39284e1d69ed93528a41d63a3b29ea23 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 18 Dec 2019 12:29:57 -0500 Subject: [PATCH 4/8] avoid yaml barewords --- oragono.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oragono.yaml b/oragono.yaml index 511c14e6..1ff46757 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -95,7 +95,7 @@ server: # vulnerable to homoglyph attacks. unless you're really confident in your decision, # we recommend leaving this value at its default (changing it once the network is # already up and running is problematic). - casemapping: precis + casemapping: "precis" # whether to look up user hostnames with reverse DNS # (to suppress this for privacy purposes, use the ip-cloaking options below) From 0df25e0e30f6fbd6957d53c5541028c91da831d0 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Thu, 19 Dec 2019 18:41:46 -0500 Subject: [PATCH 5/8] remove redundant error check --- irc/config.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/irc/config.go b/irc/config.go index 1a187604..e0e0805f 100644 --- a/irc/config.go +++ b/irc/config.go @@ -618,6 +618,7 @@ func LoadConfig(filename string) (config *Config, err error) { if !utils.IsServerName(config.Server.Name) { return nil, ErrServerNameNotHostname } + config.Server.nameCasefolded = strings.ToLower(config.Server.Name) if config.Datastore.Path == "" { return nil, ErrDatastorePathMissing } @@ -804,12 +805,6 @@ func LoadConfig(filename string) (config *Config, err error) { config.Debug.recoverFromErrors = true } - // casefold/validate server name - config.Server.nameCasefolded = strings.ToLower(config.Server.Name) - if err != nil { - return nil, fmt.Errorf("Server name isn't valid [%s]: %s", config.Server.Name, err.Error()) - } - // process operator definitions, store them to config.operators operclasses, err := config.OperatorClasses() if err != nil { From 2d4dbeba1c5968adf3b8ff8a9a5a54f0c0a84318 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 22 Dec 2019 09:19:28 -0500 Subject: [PATCH 6/8] disallow some bad characters in foldPermissive --- irc/strings.go | 5 +++++ irc/strings_test.go | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/irc/strings.go b/irc/strings.go index bf229f93..f08823e0 100644 --- a/irc/strings.go +++ b/irc/strings.go @@ -271,6 +271,11 @@ func IsPureASCII(str string) bool { } func foldPermissive(str string) (result string, err error) { + for _, r := range str { + if unicode.IsSpace(r) || r == 0 { + return "", errInvalidCharacter + } + } // YOLO str = norm.NFD.String(str) str = cases.Fold().String(str) diff --git a/irc/strings_test.go b/irc/strings_test.go index c2515cb5..02e35305 100644 --- a/irc/strings_test.go +++ b/irc/strings_test.go @@ -63,6 +63,7 @@ func TestCasefoldChannel(t *testing.T) { "", "#*starpower", "# NASA", "#interro?", "OOF#", "foo", // bidi violation mixing latin and hebrew characters: "#shalomעליכם", + "#tab\tcharacter", "#\t", "#carriage\rreturn", } { testCases = append(testCases, channelTest{channel: errCase, err: true}) } @@ -237,3 +238,14 @@ func TestFoldPermissive(t *testing.T) { tester("dolph🐬n", "DOLPH🐬n", true) tester("dolph🐬n", "dolph💻n", false) } + +func TestFoldPermissiveInvalid(t *testing.T) { + _, err := foldPermissive("a\tb") + if err == nil { + t.Errorf("whitespace should be invalid in identifiers") + } + _, err = foldPermissive("a\x00b") + if err == nil { + t.Errorf("the null byte should be invalid in identifiers") + } +} From 781bb6b051afbdc6ad3f8124e0c3690a52952839 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 22 Dec 2019 09:31:51 -0500 Subject: [PATCH 7/8] more systematic bad-character check in permissive mode --- irc/strings.go | 13 +++++++++---- irc/strings_test.go | 1 + 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/irc/strings.go b/irc/strings.go index f08823e0..3f9b82c4 100644 --- a/irc/strings.go +++ b/irc/strings.go @@ -7,6 +7,7 @@ package irc import ( "fmt" + "regexp" "strings" "unicode" @@ -21,6 +22,12 @@ const ( precisUTF8MappingToken = "rfc8265" ) +var ( + // reviving the old ergonomadic nickname regex: + // in permissive mode, allow arbitrary letters, numbers, punctuation, and symbols + permissiveCharsRegex = regexp.MustCompile(`^[\pL\pN\pP\pS]*$`) +) + type Casemapping uint const ( @@ -271,10 +278,8 @@ func IsPureASCII(str string) bool { } func foldPermissive(str string) (result string, err error) { - for _, r := range str { - if unicode.IsSpace(r) || r == 0 { - return "", errInvalidCharacter - } + if !permissiveCharsRegex.MatchString(str) { + return "", errInvalidCharacter } // YOLO str = norm.NFD.String(str) diff --git a/irc/strings_test.go b/irc/strings_test.go index 02e35305..6780186f 100644 --- a/irc/strings_test.go +++ b/irc/strings_test.go @@ -237,6 +237,7 @@ func TestFoldPermissive(t *testing.T) { tester("shivaram", "DAN-", false) tester("dolph🐬n", "DOLPH🐬n", true) tester("dolph🐬n", "dolph💻n", false) + tester("9FRONT", "9front", true) } func TestFoldPermissiveInvalid(t *testing.T) { From 4391b1ba5aaa2348176a9eac0e164721ee0a761a Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 22 Dec 2019 09:57:49 -0500 Subject: [PATCH 8/8] restrict ASCII mode to printable characters only --- irc/strings.go | 10 ++++++---- irc/strings_test.go | 48 +++++++++++++++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/irc/strings.go b/irc/strings.go index 3f9b82c4..32f3f952 100644 --- a/irc/strings.go +++ b/irc/strings.go @@ -9,7 +9,6 @@ import ( "fmt" "regexp" "strings" - "unicode" "github.com/oragono/confusables" "golang.org/x/text/cases" @@ -262,15 +261,18 @@ func CanonicalizeMaskWildcard(userhost string) (expanded string, err error) { } func foldASCII(str string) (result string, err error) { - if !IsPureASCII(str) { + if !IsPrintableASCII(str) { return "", errInvalidCharacter } return strings.ToLower(str), nil } -func IsPureASCII(str string) bool { +func IsPrintableASCII(str string) bool { for i := 0; i < len(str); i++ { - if unicode.MaxASCII < str[i] { + // allow space here because it's technically printable; + // it will be disallowed later by CasefoldName/CasefoldChannel + chr := str[i] + if chr < ' ' || chr > '~' { return false } } diff --git a/irc/strings_test.go b/irc/strings_test.go index 6780186f..9bb487f5 100644 --- a/irc/strings_test.go +++ b/irc/strings_test.go @@ -217,20 +217,24 @@ func TestCanonicalizeMaskWildcard(t *testing.T) { tester("*SHIVARAM*", "*shivaram*!*@*", nil) } +func validFoldTester(first, second string, equal bool, folder func(string) (string, error), t *testing.T) { + firstFolded, err := folder(first) + if err != nil { + panic(err) + } + secondFolded, err := folder(second) + if err != nil { + panic(err) + } + foundEqual := firstFolded == secondFolded + if foundEqual != equal { + t.Errorf("%s and %s: expected equality %t, but got %t", first, second, equal, foundEqual) + } +} + func TestFoldPermissive(t *testing.T) { tester := func(first, second string, equal bool) { - firstFolded, err := foldPermissive(first) - if err != nil { - panic(err) - } - secondFolded, err := foldPermissive(second) - if err != nil { - panic(err) - } - foundEqual := firstFolded == secondFolded - if foundEqual != equal { - t.Errorf("%s and %s: expected equality %t, but got %t", first, second, equal, foundEqual) - } + validFoldTester(first, second, equal, foldPermissive, t) } tester("SHIVARAM", "shivaram", true) tester("shIvaram", "shivaraM", true) @@ -250,3 +254,23 @@ func TestFoldPermissiveInvalid(t *testing.T) { t.Errorf("the null byte should be invalid in identifiers") } } + +func TestFoldASCII(t *testing.T) { + tester := func(first, second string, equal bool) { + validFoldTester(first, second, equal, foldASCII, t) + } + tester("shivaram", "SHIVARAM", true) + tester("X|Y", "x|y", true) + tester("a != b", "A != B", true) +} + +func TestFoldASCIIInvalid(t *testing.T) { + _, err := foldASCII("\x01") + if err == nil { + t.Errorf("control characters should be invalid in identifiers") + } + _, err = foldASCII("\x7F") + if err == nil { + t.Errorf("control characters should be invalid in identifiers") + } +}