Merge pull request #1741 from slingamn/greylisting.3

user visible email errors, email timeouts

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)

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 &registrationCallbackError{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

irc/email/email.go

@@ -9,13 +9,14 @@ import (
 	"net"
 	"regexp"
 	"strings"
+	"time"
 
 	"github.com/ergochat/ergo/irc/smtp"
 )
 
 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")
 )
 
@@ -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) {
@@ -73,6 +75,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 +111,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)
@@ -121,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)
 }

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`)

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
@@ -2557,10 +2559,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
 }

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))
 	}
 }

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) {
+func Dial(addr string, timeout time.Duration) (*Client, error) {
-	conn, err := net.Dial("tcp", addr)
+	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
+// 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) error {
+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
 	}

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)