chasquid: Fail at RCPT TO time if a user does not exist

It's more convenient and in line with standard practice to fail RCPT TO if the
user does not exist.

This involves making the server and client aware of aliases, but it doesn't
end up being very convoluted, and simplifies other code.

chasquid.go

@@ -74,9 +74,8 @@ func main() {
 	s.Hostname = conf.Hostname
 	s.MaxDataSize = conf.MaxDataSizeMb * 1024 * 1024
 
-	aliasesR := aliases.NewResolver()
-	aliasesR.SuffixSep = conf.SuffixSeparators
-	aliasesR.DropChars = conf.DropCharacters
+	s.aliasesR.SuffixSep = conf.SuffixSeparators
+	s.aliasesR.DropChars = conf.DropCharacters
 
 	// Load domains.
 	// They live inside the config directory, so the relative path works.
@@ -92,7 +91,7 @@ func main() {
 	for _, info := range domainDirs {
 		name := info.Name()
 		dir := filepath.Join("domains", name)
-		loadDomain(name, dir, s, aliasesR)
+		loadDomain(name, dir, s)
 	}
 
 	// Always include localhost as local domain.
@@ -106,9 +105,9 @@ func main() {
 		Timeout: 30 * time.Second,
 	}
 	remoteC := &courier.SMTP{}
-	s.InitQueue(conf.DataDir+"/queue", aliasesR, localC, remoteC)
+	s.InitQueue(conf.DataDir+"/queue", localC, remoteC)
 
-	go s.periodicallyReload(aliasesR)
+	go s.periodicallyReload()
 
 	// Load the addresses and listeners.
 	systemdLs, err := systemd.Listeners()
@@ -144,10 +143,10 @@ func loadAddresses(srv *Server, addrs []string, ls []net.Listener, mode SocketMo
 }
 
 // Helper to load a single domain configuration into the server.
-func loadDomain(name, dir string, s *Server, aliasesR *aliases.Resolver) {
+func loadDomain(name, dir string, s *Server) {
 	glog.Infof("  %s", name)
 	s.AddDomain(name)
-	aliasesR.AddDomain(name)
+	s.aliasesR.AddDomain(name)
 	s.AddCerts(dir+"/cert.pem", dir+"/key.pem")
 
 	if _, err := os.Stat(dir + "/users"); err == nil {
@@ -161,7 +160,7 @@ func loadDomain(name, dir string, s *Server, aliasesR *aliases.Resolver) {
 	}
 
 	glog.Infof("    adding aliases")
-	err := aliasesR.AddAliasesFile(name, dir+"/aliases")
+	err := s.aliasesR.AddAliasesFile(name, dir+"/aliases")
 	if err != nil {
 		glog.Errorf("      error: %v", err)
 	}
@@ -220,6 +219,9 @@ type Server struct {
 	// User databases (per domain).
 	userDBs map[string]*userdb.DB
 
+	// Aliases resolver.
+	aliasesR *aliases.Resolver
+
 	// Time before we give up on a connection, even if it's sending data.
 	connTimeout time.Duration
 
@@ -238,6 +240,7 @@ func NewServer() *Server {
 		commandTimeout: 1 * time.Minute,
 		localDomains:   &set.String{},
 		userDBs:        map[string]*userdb.DB{},
+		aliasesR:       aliases.NewResolver(),
 	}
 }
 
@@ -262,10 +265,8 @@ func (s *Server) AddUserDB(domain string, db *userdb.DB) {
 	s.userDBs[domain] = db
 }
 
-func (s *Server) InitQueue(path string, aliasesR *aliases.Resolver,
-	localC, remoteC courier.Courier) {
-
-	q := queue.New(path, s.localDomains, aliasesR, localC, remoteC)
+func (s *Server) InitQueue(path string, localC, remoteC courier.Courier) {
+	q := queue.New(path, s.localDomains, s.aliasesR, localC, remoteC)
 	err := q.Load()
 	if err != nil {
 		glog.Fatalf("Error loading queue: %v", err)
@@ -275,9 +276,9 @@ func (s *Server) InitQueue(path string, aliasesR *aliases.Resolver,
 
 // periodicallyReload some of the server's information, such as aliases and
 // the user databases.
-func (s *Server) periodicallyReload(aliasesR *aliases.Resolver) {
+func (s *Server) periodicallyReload() {
 	for range time.Tick(30 * time.Second) {
-		err := aliasesR.Reload()
+		err := s.aliasesR.Reload()
 		if err != nil {
 			glog.Errorf("Error reloading aliases: %v", err)
 		}
@@ -363,6 +364,7 @@ func (s *Server) serve(l net.Listener, mode SocketMode) {
 			mode:           mode,
 			tlsConfig:      s.tlsConfig,
 			userDBs:        s.userDBs,
+			aliasesR:       s.aliasesR,
 			localDomains:   s.localDomains,
 			deadline:       time.Now().Add(s.connTimeout),
 			commandTimeout: s.commandTimeout,
@@ -401,9 +403,11 @@ type Conn struct {
 	// Are we using TLS?
 	onTLS bool
 
-	// User databases and local domains, taken from the server at creation time.
+	// User databases, aliases and local domains, taken from the server at
+	// creation time.
 	userDBs      map[string]*userdb.DB
 	localDomains *set.String
+	aliasesR     *aliases.Resolver
 
 	// Have we successfully completed AUTH?
 	completedAuth bool
@@ -644,11 +648,15 @@ func (c *Conn) RCPT(params string) (code int, msg string) {
 		return 503, "sender not yet given"
 	}
 
-	remoteDst := !envelope.DomainIn(e.Address, c.localDomains)
-	if remoteDst && !c.completedAuth {
+	localDst := envelope.DomainIn(e.Address, c.localDomains)
+	if !localDst && !c.completedAuth {
 		return 503, "relay not allowed"
 	}
 
+	if localDst && !c.userExists(e.Address) {
+		return 550, "recipient unknown, please check the address for typos"
+	}
+
 	c.rcptTo = append(c.rcptTo, e.Address)
 	return 250, "You have an eerie feeling..."
 }
@@ -854,6 +862,25 @@ func (c *Conn) resetEnvelope() {
 	c.data = nil
 }
 
+func (c *Conn) userExists(addr string) bool {
+	var ok bool
+	addr, ok = c.aliasesR.Exists(addr)
+	if ok {
+		return true
+	}
+
+	// Note we used the address returned by the aliases resolver, which has
+	// cleaned it up. This means that a check for "us.er@domain" will have us
+	// look up "user" in our databases if the domain is local, which is what
+	// we want.
+	user, domain := envelope.Split(addr)
+	udb := c.userDBs[domain]
+	if udb == nil {
+		return false
+	}
+	return udb.HasUser(user)
+}
+
 func (c *Conn) readCommand() (cmd, params string, err error) {
 	var msg string
 

chasquid_test.go

@@ -431,13 +431,14 @@ func realMain(m *testing.M) int {
 		s.AddAddr(smtpAddr, ModeSMTP)
 		s.AddAddr(submissionAddr, ModeSubmission)
 
-		ars := aliases.NewResolver()
 		localC := &courier.Procmail{}
 		remoteC := &courier.SMTP{}
-		s.InitQueue(tmpDir+"/queue", ars, localC, remoteC)
+		s.InitQueue(tmpDir+"/queue", localC, remoteC)
 
 		udb := userdb.New("/dev/null")
 		udb.AddUser("testuser", "testpasswd")
+		s.aliasesR.AddAliasForTesting(
+			"to@localhost", "testuser@localhost", aliases.EMAIL)
 		s.AddDomain("localhost")
 		s.AddUserDB("localhost", udb)
 

internal/aliases/aliases.go

@@ -120,6 +120,19 @@ func (v *Resolver) Resolve(addr string) ([]Recipient, error) {
 	return v.resolve(0, addr)
 }
 
+// Exists check that the address exists in the database.
+// It returns the cleaned address, and a boolean indicating the result.
+// The clean address can be used to look it up in other databases, even if it
+// doesn't exist.
+func (v *Resolver) Exists(addr string) (string, bool) {
+	v.mu.Lock()
+	defer v.mu.Unlock()
+
+	addr = v.cleanIfLocal(addr)
+	_, ok := v.aliases[addr]
+	return addr, ok
+}
+
 func (v *Resolver) resolve(rcount int, addr string) ([]Recipient, error) {
 	if rcount >= recursionLimit {
 		return nil, ErrRecursionLimitExceeded

internal/aliases/aliases_test.go

@@ -25,6 +25,22 @@ func (cases Cases) check(t *testing.T, r *Resolver) {
 	}
 }
 
+func mustExist(t *testing.T, r *Resolver, addrs ...string) {
+	for _, addr := range addrs {
+		if _, ok := r.Exists(addr); !ok {
+			t.Errorf("address %q does not exist, it should", addr)
+		}
+	}
+}
+
+func mustNotExist(t *testing.T, r *Resolver, addrs ...string) {
+	for _, addr := range addrs {
+		if _, ok := r.Exists(addr); ok {
+			t.Errorf("address %q exists, it should not", addr)
+		}
+	}
+}
+
 func TestBasic(t *testing.T) {
 	resolver := NewResolver()
 	resolver.aliases = map[string][]Recipient{
@@ -39,6 +55,9 @@ func TestBasic(t *testing.T) {
 		{"x@y", []Recipient{{"x@y", EMAIL}}},
 	}
 	cases.check(t, resolver)
+
+	mustExist(t, resolver, "a@b", "e@f", "cmd")
+	mustNotExist(t, resolver, "x@y")
 }
 
 func TestAddrRewrite(t *testing.T) {
@@ -81,6 +100,48 @@ func TestAddrRewrite(t *testing.T) {
 	cases.check(t, resolver)
 }
 
+func TestExistsRewrite(t *testing.T) {
+	resolver := NewResolver()
+	resolver.AddDomain("def")
+	resolver.AddDomain("p-q.com")
+	resolver.aliases = map[string][]Recipient{
+		"abc@def":  {{"x@y", EMAIL}},
+		"ñoño@def": {{"x@y", EMAIL}},
+		"recu@def": {{"ab+cd@p-q.com", EMAIL}},
+	}
+	resolver.DropChars = ".~"
+	resolver.SuffixSep = "-+"
+
+	mustExist(t, resolver, "abc@def", "a.bc+blah@def", "ño.ño@def")
+	mustNotExist(t, resolver, "abc@d.ef", "nothere@def")
+
+	cases := []struct {
+		addr         string
+		expectAddr   string
+		expectExists bool
+	}{
+		{"abc@def", "abc@def", true},
+		{"abc+blah@def", "abc@def", true},
+		{"a.b~c@def", "abc@def", true},
+		{"a.bc+blah@def", "abc@def", true},
+
+		{"a.bc@unknown", "a.bc@unknown", false},
+		{"x.yz@def", "xyz@def", false},
+		{"x.yz@d.ef", "x.yz@d.ef", false},
+	}
+	for _, c := range cases {
+		addr, exists := resolver.Exists(c.addr)
+		if addr != c.expectAddr {
+			t.Errorf("%q: expected addr %q, got %q",
+				c.addr, c.expectAddr, addr)
+		}
+		if exists != c.expectExists {
+			t.Errorf("%q: expected exists %v, got %v",
+				c.addr, c.expectExists, exists)
+		}
+	}
+}
+
 func TestTooMuchRecursion(t *testing.T) {
 	resolver := Resolver{}
 	resolver.aliases = map[string][]Recipient{

internal/userdb/userdb.go

@@ -202,6 +202,14 @@ func (db *DB) RemoveUser(name string) bool {
 	return present
 }
 
+// HasUser returns true if the user is present, False otherwise.
+func (db *DB) HasUser(name string) bool {
+	db.mu.Lock()
+	_, present := db.db.Users[name]
+	db.mu.Unlock()
+	return present
+}
+
 ///////////////////////////////////////////////////////////
 // Encryption schemes
 //

internal/userdb/userdb_test.go

@@ -283,3 +283,29 @@ func TestRemoveUser(t *testing.T) {
 		t.Errorf("removal of unknown user succeeded")
 	}
 }
+
+func TestHasUser(t *testing.T) {
+	fname := mustCreateDB(t, "")
+	defer removeIfSuccessful(t, fname)
+	db := mustLoad(t, fname)
+
+	if ok := db.HasUser("unknown"); ok {
+		t.Errorf("unknown user exists")
+	}
+
+	if err := db.AddUser("user", "passwd"); err != nil {
+		t.Fatalf("error adding user: %v", err)
+	}
+
+	if ok := db.HasUser("unknown"); ok {
+		t.Errorf("unknown user exists")
+	}
+
+	if ok := db.HasUser("user"); !ok {
+		t.Errorf("known user does not exist")
+	}
+
+	if ok := db.HasUser("user"); !ok {
+		t.Errorf("known user does not exist")
+	}
+}

test/t-01-simple_local/run.sh

@@ -7,6 +7,7 @@ init
 
 generate_certs_for testserver
 add_user testserver user secretpassword
+add_user testserver someone secretpassword
 
 mkdir -p .logs
 chasquid -v=2 --log_dir=.logs --config_dir=config &
@@ -25,6 +26,11 @@ if ! run_msmtp -a smtpport someone@testserver < content 2> /dev/null; then
 	exit 1
 fi
 
+if run_msmtp nobody@testserver < content 2> /dev/null; then
+	echo "ERROR: successfuly sent an email to a non-existent user"
+	exit 1
+fi
+
 if run_msmtp -a baduser someone@testserver < content 2> /dev/null; then
 	echo "ERROR: successfully sent an email with a bad password"
 	exit 1

test/t-02-exim/run.sh

@@ -39,6 +39,7 @@ EXIMDIR="$PWD/.exim4" envsubst < config/exim4.in > .exim4/config
 
 generate_certs_for srv-chasquid
 add_user srv-chasquid user secretpassword
+add_user srv-chasquid someone secretpassword
 
 # Launch chasquid at port 1025 (in config).
 # Use outgoing port 2025 which is where exim will be at.