From 67150bc8f7d35178c3a3db2d33a2e9f8f1c59194 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 17 May 2020 18:06:20 -0400 Subject: [PATCH 1/2] fix #1020 --- conventional.yaml | 4 ++++ irc/client.go | 17 +++++++++++++---- irc/config.go | 16 ++++++++++------ irc/handlers.go | 35 ++++++++++++++++++++++++++++++++--- oragono.yaml | 4 ++++ 5 files changed, 63 insertions(+), 13 deletions(-) diff --git a/conventional.yaml b/conventional.yaml index bf587af7..777d9275 100644 --- a/conventional.yaml +++ b/conventional.yaml @@ -330,6 +330,10 @@ accounts: # PASS as well, so it can be configured to authenticate with SASL only. skip-server-password: false + # enable login to accounts via the PASS command, e.g., PASS account:password + # this is sometimes useful for compatibility with old clients that don't support SASL + login-via-pass-command: false + # require-sasl controls whether clients are required to have accounts # (and sign into them using SASL) to connect to the server require-sasl: diff --git a/irc/client.go b/irc/client.go index b5204b3d..e013063d 100644 --- a/irc/client.go +++ b/irc/client.go @@ -97,6 +97,15 @@ func (s *saslStatus) Clear() { *s = saslStatus{} } +// what stage the client is at w.r.t. the PASS command: +type serverPassStatus uint + +const ( + serverPassUnsent serverPassStatus = iota + serverPassSuccessful + serverPassFailed +) + // Session is an individual client connection to the server (TCP connection // and associated per-connection data, such as capabilities). There is a // many-one relationship between sessions and clients. @@ -117,9 +126,9 @@ type Session struct { deferredFakelagCount int destroyed uint32 - certfp string - sasl saslStatus - sentPassCommand bool + certfp string + sasl saslStatus + passStatus serverPassStatus batchCounter uint32 @@ -510,7 +519,7 @@ const ( func (client *Client) isAuthorized(config *Config, session *Session) AuthOutcome { saslSent := client.account != "" // PASS requirement - if (config.Server.passwordBytes != nil) && !session.sentPassCommand && !(config.Accounts.SkipServerPassword && saslSent) { + if (config.Server.passwordBytes != nil) && session.passStatus != serverPassSuccessful && !(config.Accounts.SkipServerPassword && saslSent) { return authFailPass } // Tor connections may be required to authenticate with SASL diff --git a/irc/config.go b/irc/config.go index 6966fcf6..81512b7b 100644 --- a/irc/config.go +++ b/irc/config.go @@ -254,12 +254,13 @@ type AccountConfig struct { Exempted []string exemptedNets []net.IPNet } `yaml:"require-sasl"` - DefaultUserModes *string `yaml:"default-user-modes"` - defaultUserModes modes.ModeChanges - LDAP ldap.ServerConfig - LoginThrottling ThrottleConfig `yaml:"login-throttling"` - SkipServerPassword bool `yaml:"skip-server-password"` - NickReservation struct { + DefaultUserModes *string `yaml:"default-user-modes"` + defaultUserModes modes.ModeChanges + LDAP ldap.ServerConfig + LoginThrottling ThrottleConfig `yaml:"login-throttling"` + SkipServerPassword bool `yaml:"skip-server-password"` + LoginViaPassCommand bool `yaml:"login-via-pass-command"` + NickReservation struct { Enabled bool AdditionalNickLimit int `yaml:"additional-nick-limit"` Method NickEnforcementMethod @@ -1078,6 +1079,9 @@ func LoadConfig(filename string) (config *Config, err error) { if err != nil { return nil, err } + if config.Accounts.LoginViaPassCommand && !config.Accounts.SkipServerPassword { + return nil, errors.New("Using a server password and login-via-pass-command requires skip-server-password as well") + } } if config.Accounts.Registration.BcryptCost == 0 { diff --git a/irc/handlers.go b/irc/handlers.go index 467881e8..93a93307 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2159,16 +2159,45 @@ func passHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp rb.Add(nil, server.name, ERR_ALREADYREGISTRED, client.nick, client.t("You may not reregister")) return false } + if rb.session.passStatus != serverPassUnsent { + return false + } + + password := msg.Params[0] + config := server.Config() + + if config.Accounts.LoginViaPassCommand { + colonIndex := strings.IndexByte(password, ':') + if colonIndex != -1 && client.Account() == "" { + // TODO consolidate all login throttle checks into AccountManager + throttled, _ := client.loginThrottle.Touch() + if !throttled { + account, accountPass := password[:colonIndex], password[colonIndex+1:] + err := server.accounts.AuthenticateByPassphrase(client, account, accountPass) + if err == nil { + sendSuccessfulAccountAuth(client, rb, false, true) + // login-via-pass-command entails that we do not need to check + // an actual server password (either no password or skip-server-password) + rb.session.passStatus = serverPassSuccessful + return false + } + } + } + } + + serverPassword := config.Server.passwordBytes // if no password exists, skip checking - serverPassword := server.Config().Server.passwordBytes if serverPassword == nil { return false } // check the provided password - password := []byte(msg.Params[0]) - rb.session.sentPassCommand = bcrypt.CompareHashAndPassword(serverPassword, password) == nil + if bcrypt.CompareHashAndPassword(serverPassword, []byte(password)) == nil { + rb.session.passStatus = serverPassSuccessful + } else { + rb.session.passStatus = serverPassFailed + } // if they failed the check, we'll bounce them later when they try to complete registration return false diff --git a/oragono.yaml b/oragono.yaml index dfb799b4..5b454f2e 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -351,6 +351,10 @@ accounts: # PASS as well, so it can be configured to authenticate with SASL only. skip-server-password: false + # enable login to accounts via the PASS command, e.g., PASS account:password + # this is sometimes useful for compatibility with old clients that don't support SASL + login-via-pass-command: false + # require-sasl controls whether clients are required to have accounts # (and sign into them using SASL) to connect to the server require-sasl: From 4d21d78f49a62d7673c76ac9b868c9a45789e5eb Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 18 May 2020 03:35:58 -0400 Subject: [PATCH 2/2] explanatory comments --- irc/handlers.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/irc/handlers.go b/irc/handlers.go index 93a93307..a4425d12 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2159,6 +2159,8 @@ func passHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp rb.Add(nil, server.name, ERR_ALREADYREGISTRED, client.nick, client.t("You may not reregister")) return false } + // only give them one try to run the PASS command (all code paths end with this + // variable being set): if rb.session.passStatus != serverPassUnsent { return false } @@ -2184,6 +2186,8 @@ func passHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp } } } + // if login-via-PASS failed for any reason, proceed to try and interpret the + // provided password as the server password serverPassword := config.Server.passwordBytes @@ -2200,6 +2204,8 @@ func passHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp } // if they failed the check, we'll bounce them later when they try to complete registration + // note in particular that with skip-server-password, you can give the wrong server + // password here, then successfully SASL and be admitted return false }