smtpsrv: Return a temporary error when we fail to check if a user exists

When we fail to check if a user exists, we currently return a permanent
error, which can be misleading and also make things more difficult to
troubleshoot.

This patch makes chasquid return a temporary error in that case.

Thanks to Thor77 (thor77@thor77.org) for suggesting this change.

internal/smtpsrv/conn.go

@@ -578,7 +578,14 @@ func (c *Conn) RCPT(params string) (code int, msg string) {
 			return 550, "5.1.3 Destination address is invalid"
 		}
 
-		if !c.userExists(addr) {
+		ok, err := c.userExists(addr)
+		if err != nil {
+			c.tr.Errorf("error checking if user %q exists: %v", addr, err)
+			maillog.Rejected(c.remoteAddr, c.mailFrom, []string{addr},
+				fmt.Sprintf("error checking if user exists: %v", err))
+			return 451, "4.4.3 Temporary error checking address"
+		}
+		if !ok {
 			maillog.Rejected(c.remoteAddr, c.mailFrom, []string{addr},
 				"local user does not exist")
 			return 550, "5.1.1 Destination address is unknown (user does not exist)"
@@ -1081,11 +1088,11 @@ func (c *Conn) resetEnvelope() {
 	c.spfError = nil
 }
 
-func (c *Conn) userExists(addr string) bool {
+func (c *Conn) userExists(addr string) (bool, error) {
 	var ok bool
 	addr, ok = c.aliasesR.Exists(addr)
 	if ok {
-		return true
+		return true, nil
 	}
 
 	// Note we used the address returned by the aliases resolver, which has
@@ -1093,11 +1100,7 @@ func (c *Conn) userExists(addr string) bool {
 	// look up "user" in our databases if the domain is local, which is what
 	// we want.
 	user, domain := envelope.Split(addr)
-	ok, err := c.authr.Exists(user, domain)
+	return c.authr.Exists(user, domain)
-	if err != nil {
-		c.tr.Errorf("error checking if user %q exists: %v", addr, err)
-	}
-	return ok
 }
 
 func (c *Conn) readCommand() (cmd, params string, err error) {

internal/smtpsrv/server_test.go

@@ -308,7 +308,7 @@ func TestTooManyRecipients(t *testing.T) {
 	}
 }
 
-func TestRcptFailsExistsCheck(t *testing.T) {
+func TestRcptBrokenExists(t *testing.T) {
 	c := mustDial(t, ModeSMTP, true)
 	defer c.Close()
 
@@ -320,6 +320,24 @@ func TestRcptFailsExistsCheck(t *testing.T) {
 	if err == nil {
 		t.Errorf("Accepted RCPT with broken Exists")
 	}
+	expect := "451 4.4.3 Temporary error checking address"
+	if err.Error() != expect {
+		t.Errorf("RCPT returned unexpected error %q", err.Error())
+	}
+}
+
+func TestRcptUserDoesNotExist(t *testing.T) {
+	c := mustDial(t, ModeSMTP, true)
+	defer c.Close()
+
+	if err := c.Mail("from@localhost"); err != nil {
+		t.Fatalf("Mail: %v", err)
+	}
+
+	err := c.Rcpt("doesnotexist@localhost")
+	if err == nil {
+		t.Errorf("Accepted RCPT for non-existent user")
+	}
 	expect := "550 5.1.1 Destination address is unknown (user does not exist)"
 	if err.Error() != expect {
 		t.Errorf("RCPT returned unexpected error %q", err.Error())