From 46572b871fb994b62adbebb22406f3055bba013b Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 7 Jul 2021 06:35:30 -0400 Subject: [PATCH 1/2] expose a user-visible error if direct email sending fails See #1659 --- irc/accounts.go | 26 +++++++++++++++++++++++++- irc/email/email.go | 9 +++++++-- irc/errors.go | 1 - irc/handlers.go | 16 ++++++++++------ irc/nickserv.go | 2 +- 5 files changed, 43 insertions(+), 11 deletions(-) diff --git a/irc/accounts.go b/irc/accounts.go index 473b21fa..8bf714f9 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -15,6 +15,8 @@ import ( "time" "unicode" + "github.com/ergochat/irc-go/ircutils" + "github.com/ergochat/ergo/irc/connection_limits" "github.com/ergochat/ergo/irc/email" "github.com/ergochat/ergo/irc/migrations" @@ -460,7 +462,7 @@ func (am *AccountManager) Register(client *Client, account string, callbackNames code, err := am.dispatchCallback(client, account, callbackNamespace, callbackValue) if err != nil { am.Unregister(casefoldedAccount, true) - return errCallbackFailed + return ®istrationCallbackError{underlying: err} } else { return am.server.store.Update(func(tx *buntdb.Tx) error { _, _, err = tx.Set(verificationCodeKey, code, setOptions) @@ -469,6 +471,28 @@ func (am *AccountManager) Register(client *Client, account string, callbackNames } } +type registrationCallbackError struct { + underlying error +} + +func (r *registrationCallbackError) Error() string { + return `Account verification could not be sent` +} + +func registrationCallbackErrorText(config *Config, client *Client, err error) string { + if callbackErr, ok := err.(*registrationCallbackError); ok { + // only expose a user-visible error if we are doing direct sending + if config.Accounts.Registration.EmailVerification.DirectSendingEnabled() { + errorText := ircutils.SanitizeText(callbackErr.underlying.Error(), 350) + return fmt.Sprintf(client.t("Could not dispatch registration e-mail: %s"), errorText) + } else { + return client.t("Could not dispatch registration e-mail") + } + } else { + return "" + } +} + // validatePassphrase checks whether a passphrase is allowed by our rules func validatePassphrase(passphrase string) error { // sanity check the length diff --git a/irc/email/email.go b/irc/email/email.go index dad129c2..11139fd9 100644 --- a/irc/email/email.go +++ b/irc/email/email.go @@ -15,7 +15,7 @@ import ( var ( ErrBlacklistedAddress = errors.New("Email address is blacklisted") - ErrInvalidAddress = errors.New("Email address is blacklisted") + ErrInvalidAddress = errors.New("Email address is invalid") ErrNoMXRecord = errors.New("Couldn't resolve MX record") ) @@ -73,6 +73,11 @@ func (config *MailtoConfig) Postprocess(heloDomain string) (err error) { return config.DKIM.Postprocess() } +// are we sending email directly, as opposed to deferring to an MTA? +func (config *MailtoConfig) DirectSendingEnabled() bool { + return config.MTAReal.Server == "" +} + // get the preferred MX record hostname, "" on error func lookupMX(domain string) (server string) { var minPref uint16 @@ -104,7 +109,7 @@ func SendMail(config MailtoConfig, recipient string, msg []byte) (err error) { var addr string var auth smtp.Auth - if config.MTAReal.Server != "" { + if !config.DirectSendingEnabled() { addr = fmt.Sprintf("%s:%d", config.MTAReal.Server, config.MTAReal.Port) if config.MTAReal.Username != "" && config.MTAReal.Password != "" { auth = smtp.PlainAuth("", config.MTAReal.Username, config.MTAReal.Password, config.MTAReal.Server) diff --git a/irc/errors.go b/irc/errors.go index 30eb3e54..85ce343c 100644 --- a/irc/errors.go +++ b/irc/errors.go @@ -34,7 +34,6 @@ var ( errAccountUpdateFailed = errors.New(`Error while updating your account information`) errAccountMustHoldNick = errors.New(`You must hold that nickname in order to register it`) errAuthzidAuthcidMismatch = errors.New(`authcid and authzid must be the same`) - errCallbackFailed = errors.New("Account verification could not be sent") errCertfpAlreadyExists = errors.New(`An account already exists for your certificate fingerprint`) errChannelNotOwnedByAccount = errors.New("Channel not owned by the specified account") errChannelTransferNotOffered = errors.New(`You weren't offered ownership of that channel`) diff --git a/irc/handlers.go b/irc/handlers.go index bd15175d..15e9f0c2 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -63,14 +63,16 @@ func parseCallback(spec string, config *Config) (callbackNamespace string, callb return } -func registrationErrorToMessage(err error) (message string) { +func registrationErrorToMessage(config *Config, client *Client, err error) (message string) { + if emailError := registrationCallbackErrorText(config, client, err); emailError != "" { + return emailError + } + switch err { case errAccountAlreadyRegistered, errAccountAlreadyVerified, errAccountAlreadyUnregistered, errAccountAlreadyLoggedIn, errAccountCreation, errAccountMustHoldNick, errAccountBadPassphrase, errCertfpAlreadyExists, errFeatureDisabled, errAccountBadPassphrase: message = err.Error() case errLimitExceeded: message = `There have been too many registration attempts recently; try again later` - case errCallbackFailed: - message = `Could not dispatch verification email` default: // default response: let's be risk-averse about displaying internal errors // to the clients, especially for something as sensitive as accounts @@ -2548,10 +2550,12 @@ func registerHandler(server *Server, client *Client, msg ircmsg.Message, rb *Res rb.Add(nil, server.name, "FAIL", "REGISTER", "USERNAME_EXISTS", accountName, client.t("Username is already registered or otherwise unavailable")) case errAccountBadPassphrase: rb.Add(nil, server.name, "FAIL", "REGISTER", "INVALID_PASSWORD", accountName, client.t("Password was invalid")) - case errCallbackFailed: - rb.Add(nil, server.name, "FAIL", "REGISTER", "UNACCEPTABLE_EMAIL", accountName, client.t("Could not dispatch verification e-mail")) default: - rb.Add(nil, server.name, "FAIL", "REGISTER", "UNKNOWN_ERROR", accountName, client.t("Could not register")) + if emailError := registrationCallbackErrorText(config, client, err); emailError != "" { + rb.Add(nil, server.name, "FAIL", "REGISTER", "UNACCEPTABLE_EMAIL", accountName, emailError) + } else { + rb.Add(nil, server.name, "FAIL", "REGISTER", "UNKNOWN_ERROR", accountName, client.t("Could not register")) + } } return } diff --git a/irc/nickserv.go b/irc/nickserv.go index 364435ff..2cd62fb7 100644 --- a/irc/nickserv.go +++ b/irc/nickserv.go @@ -900,7 +900,7 @@ func nsRegisterHandler(service *ircService, server *Server, client *Client, comm } } else { // details could not be stored and relevant numerics have been dispatched, abort - message := registrationErrorToMessage(err) + message := registrationErrorToMessage(config, client, err) service.Notice(rb, client.t(message)) } } From 032ca175e4d671a57cdad03c3fbab5f63dea8183 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 7 Jul 2021 07:12:15 -0400 Subject: [PATCH 2/2] add support for email timeouts --- default.yaml | 1 + irc/email/email.go | 4 +++- irc/smtp/smtp.go | 29 ++++++++++++++++++++++++----- traditional.yaml | 1 + 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/default.yaml b/default.yaml index 289165d2..d9055828 100644 --- a/default.yaml +++ b/default.yaml @@ -413,6 +413,7 @@ accounts: # password: "hunter2" blacklist-regexes: # - ".*@mailinator.com" + timeout: 60s # throttle account login attempts (to prevent either password guessing, or DoS # attacks on the server aimed at forcing repeated expensive bcrypt computations) diff --git a/irc/email/email.go b/irc/email/email.go index 11139fd9..d06f25e0 100644 --- a/irc/email/email.go +++ b/irc/email/email.go @@ -9,6 +9,7 @@ import ( "net" "regexp" "strings" + "time" "github.com/ergochat/ergo/irc/smtp" ) @@ -40,6 +41,7 @@ type MailtoConfig struct { MTAReal MTAConfig `yaml:"mta"` BlacklistRegexes []string `yaml:"blacklist-regexes"` blacklistRegexes []*regexp.Regexp + Timeout time.Duration } func (config *MailtoConfig) Postprocess(heloDomain string) (err error) { @@ -126,5 +128,5 @@ func SendMail(config MailtoConfig, recipient string, msg []byte) (err error) { addr = fmt.Sprintf("%s:smtp", mx) } - return smtp.SendMail(addr, auth, config.HeloDomain, config.Sender, []string{recipient}, msg, config.RequireTLS) + return smtp.SendMail(addr, auth, config.HeloDomain, config.Sender, []string{recipient}, msg, config.RequireTLS, config.Timeout) } diff --git a/irc/smtp/smtp.go b/irc/smtp/smtp.go index 80b76992..d17b09ae 100644 --- a/irc/smtp/smtp.go +++ b/irc/smtp/smtp.go @@ -24,6 +24,11 @@ import ( "net" "net/textproto" "strings" + "time" +) + +var ( + ErrTimedOut = errors.New("Timed out") ) // A Client represents a client connection to an SMTP server. @@ -48,11 +53,25 @@ type Client struct { // Dial returns a new Client connected to an SMTP server at addr. // The addr must include a port, as in "mail.example.com:smtp". -func Dial(addr string) (*Client, error) { - conn, err := net.Dial("tcp", addr) +func Dial(addr string, timeout time.Duration) (*Client, error) { + var conn net.Conn + var err error + start := time.Now() + if timeout == 0 { + conn, err = net.Dial("tcp", addr) + } else { + conn, err = net.DialTimeout("tcp", addr, timeout) + } if err != nil { return nil, err } + if timeout != 0 { + remaining := timeout - time.Since(start) + if remaining <= 0 { + return nil, ErrTimedOut + } + conn.SetDeadline(time.Now().Add(remaining)) + } host, _, _ := net.SplitHostPort(addr) return NewClient(conn, host) } @@ -316,8 +335,8 @@ var testHookStartTLS func(*tls.Config) // nil, except for tests // attachments (see the mime/multipart package), or other mail // functionality. Higher-level packages exist outside of the standard // library. -// XXX: modified in Oragono to add `requireTLS` and `heloDomain` arguments -func SendMail(addr string, a Auth, heloDomain string, from string, to []string, msg []byte, requireTLS bool) error { +// XXX: modified in Ergo to add `requireTLS`, `heloDomain`, and `timeout` arguments +func SendMail(addr string, a Auth, heloDomain string, from string, to []string, msg []byte, requireTLS bool, timeout time.Duration) error { if err := validateLine(from); err != nil { return err } @@ -326,7 +345,7 @@ func SendMail(addr string, a Auth, heloDomain string, from string, to []string, return err } } - c, err := Dial(addr) + c, err := Dial(addr, timeout) if err != nil { return err } diff --git a/traditional.yaml b/traditional.yaml index 5d431c60..bee90c4d 100644 --- a/traditional.yaml +++ b/traditional.yaml @@ -386,6 +386,7 @@ accounts: # password: "hunter2" blacklist-regexes: # - ".*@mailinator.com" + timeout: 60s # throttle account login attempts (to prevent either password guessing, or DoS # attacks on the server aimed at forcing repeated expensive bcrypt computations)