From 8cd5db1194de54dd1eec845d5d86f81180e33e9f Mon Sep 17 00:00:00 2001 From: Daniel Oaks Date: Sun, 3 Feb 2019 18:49:42 +1000 Subject: [PATCH 1/4] Restrict idents as other servers do --- irc/client.go | 7 +++++-- irc/handlers.go | 3 +-- irc/numerics.go | 1 + irc/strings.go | 26 ++++++++++++++++++++++++++ irc/strings_test.go | 16 ++++++++++++++++ 5 files changed, 49 insertions(+), 4 deletions(-) diff --git a/irc/client.go b/irc/client.go index 9fc041ae..462deeb8 100644 --- a/irc/client.go +++ b/irc/client.go @@ -623,12 +623,15 @@ func (client *Client) HasUsername() bool { return client.username != "" && client.username != "*" } +// SetNames sets the client's ident and realname. func (client *Client) SetNames(username, realname string) error { - usernameCasefolded, err := CasefoldName(username) - if err != nil { + // do this before casefolding to ensure these are actually ascii + if !isIdent(username) { return errInvalidUsername } + usernameCasefolded := strings.ToLower(username) // only ascii is supported in idents anyway + client.stateMutex.Lock() defer client.stateMutex.Unlock() diff --git a/irc/handlers.go b/irc/handlers.go index 7fadeffa..e89fee37 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2195,8 +2195,7 @@ func userHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp err := client.SetNames(msg.Params[0], msg.Params[3]) if err == errInvalidUsername { - rb.Add(nil, "", "ERROR", client.t("Malformed username")) - return true + rb.Add(nil, server.name, ERR_INVALIDUSERNAME, client.t("Malformed username")) } return false diff --git a/irc/numerics.go b/irc/numerics.go index ec8e83cc..05c4ff21 100644 --- a/irc/numerics.go +++ b/irc/numerics.go @@ -143,6 +143,7 @@ const ( ERR_YOUREBANNEDCREEP = "465" ERR_YOUWILLBEBANNED = "466" ERR_KEYSET = "467" + ERR_INVALIDUSERNAME = "468" ERR_CHANNELISFULL = "471" ERR_UNKNOWNMODE = "472" ERR_INVITEONLYCHAN = "473" diff --git a/irc/strings.go b/irc/strings.go index 0e5e50ca..5ef67e78 100644 --- a/irc/strings.go +++ b/irc/strings.go @@ -128,6 +128,32 @@ func isBoring(name string) bool { return true } +// returns true if the given name is a valid ident, using a mix of Insp and +// Chary's ident restrictions. +func isIdent(name string) bool { + if len(name) < 1 { + return false + } + + for i := 0; i < len(name); i++ { + chr := name[i] + if (chr >= 'a' && chr <= 'z') || (chr >= 'A' && chr <= 'Z') || (chr >= '0' && chr <= '9') { + continue // alphanumerics + } + if i == 0 { + return false // first char must be alnum + } + switch chr { + case '[', '\\', ']', '^', '_', '{', '|', '}', '-', '.', '`': + continue // allowed chars + default: + return false // disallowed chars + } + } + + return true +} + // 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, diff --git a/irc/strings_test.go b/irc/strings_test.go index 6b60a0f0..757722d4 100644 --- a/irc/strings_test.go +++ b/irc/strings_test.go @@ -140,6 +140,22 @@ func TestIsBoring(t *testing.T) { assertBoring("Νικηφόρος", false) } +func TestIsIdent(t *testing.T) { + assertIdent := func(str string, expected bool) { + if isIdent(str) != expected { + t.Errorf("expected [%s] to have identness [%t], but got [%t]", str, expected, !expected) + } + } + + assertIdent("warning", true) + assertIdent("sid3225", true) + assertIdent("dan.oak25", true) + assertIdent("dan.oak[25]", true) + assertIdent("phi@#$%ip", false) + assertIdent("Νικηφόρος", false) + assertIdent("-dan56", false) +} + func TestSkeleton(t *testing.T) { skeleton := func(str string) string { skel, err := Skeleton(str) From cfbb4361dc17e5c7fc73c2d9ba31c0ca8ca93ea4 Mon Sep 17 00:00:00 2001 From: Daniel Oaks Date: Sun, 3 Feb 2019 19:24:59 +1000 Subject: [PATCH 2/4] Restrict ident length similar to other servers --- irc/client.go | 10 +++++----- irc/config.go | 7 ++++++- irc/handlers.go | 4 +++- oragono.yaml | 3 +++ 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/irc/client.go b/irc/client.go index 462deeb8..a861f021 100644 --- a/irc/client.go +++ b/irc/client.go @@ -176,12 +176,12 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) { client.Notice(client.t("*** Looking up your username")) resp, err := ident.Query(clientHost, serverPort, clientPort, IdentTimeoutSeconds) if err == nil { - username := resp.Identifier - cfusername, err := CasefoldName(username) - if err == nil { + ident := resp.Identifier[:config.Limits.IdentLen-1] + if isIdent(ident) { + identLower := strings.ToLower(ident) // idents can only be ASCII chars only client.Notice(client.t("*** Found your username")) - client.username = username - client.usernameCasefolded = cfusername + client.username = ident + client.usernameCasefolded = identLower // we don't need to updateNickMask here since nickMask is not used for anything yet } else { client.Notice(client.t("*** Got a malformed username, ignoring")) diff --git a/irc/config.go b/irc/config.go index c19bf157..41f49259 100644 --- a/irc/config.go +++ b/irc/config.go @@ -206,12 +206,13 @@ type Limits struct { AwayLen int `yaml:"awaylen"` ChanListModes int `yaml:"chan-list-modes"` ChannelLen int `yaml:"channellen"` + IdentLen int `yaml:"identlen"` KickLen int `yaml:"kicklen"` + LineLen LineLenLimits `yaml:"linelen"` MonitorEntries int `yaml:"monitor-entries"` NickLen int `yaml:"nicklen"` TopicLen int `yaml:"topiclen"` WhowasEntries int `yaml:"whowas-entries"` - LineLen LineLenLimits `yaml:"linelen"` } // STSConfig controls the STS configuration/ @@ -491,6 +492,10 @@ func LoadConfig(filename string) (config *Config, err error) { if len(config.Server.Listen) == 0 { return nil, ErrNoListenersDefined } + //dan: automagically fix identlen until a few releases in the future (from now, 0.12.0), being a newly-introduced limit + if config.Limits.IdentLen < 1 { + config.Limits.IdentLen = 10 + } if config.Limits.NickLen < 1 || config.Limits.ChannelLen < 2 || config.Limits.AwayLen < 1 || config.Limits.KickLen < 1 || config.Limits.TopicLen < 1 { return nil, ErrLimitsAreInsane } diff --git a/irc/handlers.go b/irc/handlers.go index e89fee37..8f941a35 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2193,7 +2193,9 @@ func userHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp return false } - err := client.SetNames(msg.Params[0], msg.Params[3]) + ident := msg.Params[0][:server.Limits().IdentLen-1] // -1 as SetNames adds the ~ at the start for us + + err := client.SetNames(ident, msg.Params[3]) if err == errInvalidUsername { rb.Add(nil, server.name, ERR_INVALIDUSERNAME, client.t("Malformed username")) } diff --git a/oragono.yaml b/oragono.yaml index 263d1947..e217f3e2 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -415,6 +415,9 @@ limits: # nicklen is the max nick length allowed nicklen: 32 + # identlen is the max ident length allowed + identlen: 10 + # channellen is the max channel length allowed channellen: 64 From 151002e232027395dce389025353bfc32c898c56 Mon Sep 17 00:00:00 2001 From: Daniel Oaks Date: Mon, 4 Feb 2019 05:01:46 +1000 Subject: [PATCH 3/4] Up identlen default to 20 --- irc/config.go | 2 +- oragono.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/irc/config.go b/irc/config.go index 41f49259..03399c58 100644 --- a/irc/config.go +++ b/irc/config.go @@ -494,7 +494,7 @@ func LoadConfig(filename string) (config *Config, err error) { } //dan: automagically fix identlen until a few releases in the future (from now, 0.12.0), being a newly-introduced limit if config.Limits.IdentLen < 1 { - config.Limits.IdentLen = 10 + config.Limits.IdentLen = 20 } if config.Limits.NickLen < 1 || config.Limits.ChannelLen < 2 || config.Limits.AwayLen < 1 || config.Limits.KickLen < 1 || config.Limits.TopicLen < 1 { return nil, ErrLimitsAreInsane diff --git a/oragono.yaml b/oragono.yaml index e217f3e2..45ca1748 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -416,7 +416,7 @@ limits: nicklen: 32 # identlen is the max ident length allowed - identlen: 10 + identlen: 20 # channellen is the max channel length allowed channellen: 64 From e8309aee792785968c955fcd96cf62fd7d6c7461 Mon Sep 17 00:00:00 2001 From: Daniel Oaks Date: Mon, 4 Feb 2019 05:02:13 +1000 Subject: [PATCH 4/4] Avoiding a crash when getting a short ident is a good thing --- irc/client.go | 5 ++++- irc/handlers.go | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/irc/client.go b/irc/client.go index a861f021..13b6abbf 100644 --- a/irc/client.go +++ b/irc/client.go @@ -176,7 +176,10 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) { client.Notice(client.t("*** Looking up your username")) resp, err := ident.Query(clientHost, serverPort, clientPort, IdentTimeoutSeconds) if err == nil { - ident := resp.Identifier[:config.Limits.IdentLen-1] + ident := resp.Identifier + if config.Limits.IdentLen < len(ident) { + ident = ident[:config.Limits.IdentLen] + } if isIdent(ident) { identLower := strings.ToLower(ident) // idents can only be ASCII chars only client.Notice(client.t("*** Found your username")) diff --git a/irc/handlers.go b/irc/handlers.go index 8f941a35..0841d149 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2193,7 +2193,11 @@ func userHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp return false } - ident := msg.Params[0][:server.Limits().IdentLen-1] // -1 as SetNames adds the ~ at the start for us + ident := msg.Params[0] + identLen := server.Limits().IdentLen + if identLen-1 < len(ident) { + ident = ident[:server.Limits().IdentLen-1] // -1 as SetNames adds the ~ at the start for us + } err := client.SetNames(ident, msg.Params[3]) if err == errInvalidUsername {