git » chasquid » commit e6a9410

diff --git a/chasquid.go b/chasquid.go
index e3bbe67..ac152c8 100644
--- a/chasquid.go
+++ b/chasquid.go
@@ -27,7 +27,6 @@ import (
 	"blitiri.com.ar/go/chasquid/internal/normalize"
 	"blitiri.com.ar/go/chasquid/internal/smtpsrv"
 	"blitiri.com.ar/go/chasquid/internal/sts"
-	"blitiri.com.ar/go/chasquid/internal/userdb"
 	"blitiri.com.ar/go/log"
 	"blitiri.com.ar/go/systemd"
 )
@@ -286,18 +285,18 @@ func loadDomain(name, dir string, s *smtpsrv.Server) {
 	log.Infof("  %s", name)
 	s.AddDomain(name)
 
-	udb, err := userdb.Load(dir + "/users")
-	if os.IsNotExist(err) {
-		// No users file present, that's okay.
-	} else if err != nil {
-		log.Errorf("    users file error: %v", err)
-	} else {
-		s.AddUserDB(name, udb)
+	err := s.AddUserDB(name, dir+"/users")
+	if err != nil {
+		// If there is an error loading users, fail hard to make sure this is
+		// noticed and fixed as soon as it happens.
+		log.Fatalf("    users file error: %v", err)
 	}
 
 	err = s.AddAliasesFile(name, dir+"/aliases")
 	if err != nil {
-		log.Errorf("    aliases file error: %v", err)
+		// If there's an error loading aliases, fail hard to make sure this is
+		// noticed and fixed as soon as it happens.
+		log.Fatalf("    aliases file error: %v", err)
 	}
 
 	err = loadDKIM(name, dir, s)
diff --git a/cmd/chasquid-util/chasquid-util.go b/cmd/chasquid-util/chasquid-util.go
index cb1cdcc..f319fdd 100644
--- a/cmd/chasquid-util/chasquid-util.go
+++ b/cmd/chasquid-util/chasquid-util.go
@@ -149,7 +149,14 @@ func userDBFromArgs(create bool) (string, string, *userdb.DB) {
 
 // chasquid-util check-userdb <domain>
 func checkUserDB() {
-	_, err := userdb.Load(userDBForDomain(""))
+	path := userDBForDomain("")
+	// Check if the file exists. This is because userdb.Load does not consider
+	// it an error.
+	if _, err := os.Stat(path); os.IsNotExist(err) {
+		Fatalf("Error: file %q does not exist", path)
+	}
+
+	_, err := userdb.Load(path)
 	if err != nil {
 		Fatalf("Error loading database: %v", err)
 	}
diff --git a/cmd/chasquid-util/test_bad_args.cmy b/cmd/chasquid-util/test_bad_args.cmy
index 1f9c81e..0a61d35 100644
--- a/cmd/chasquid-util/test_bad_args.cmy
+++ b/cmd/chasquid-util/test_bad_args.cmy
@@ -4,7 +4,7 @@ c <- Unknown argument "blahrarghar"
 c wait 1
 
 c = ./chasquid-util --configdir=.nonono check-userdb
-c <- Error loading database: open .nonono/domains//users: no such file or directory
+c <- Error: file ".nonono/domains//users" does not exist
 c wait 1
 
 c = ./chasquid-util --configdir=.nonono print-config
diff --git a/internal/aliases/aliases.go b/internal/aliases/aliases.go
index 1a593bb..e56054f 100644
--- a/internal/aliases/aliases.go
+++ b/internal/aliases/aliases.go
@@ -345,7 +345,8 @@ func (v *Resolver) AddDomain(domain string) {
 }
 
 // AddAliasesFile to the resolver. The file will be parsed, and an error
-// returned if it does not exist or parse correctly.
+// returned if it does not parse correctly.  Note that the file not existing
+// does NOT result in an error.
 func (v *Resolver) AddAliasesFile(domain, path string) error {
 	// We unconditionally add the domain and file on our list.
 	// Even if the file does not exist now, it may later. This makes it be
diff --git a/internal/smtpsrv/server.go b/internal/smtpsrv/server.go
index 76f7b07..b5e05ed 100644
--- a/internal/smtpsrv/server.go
+++ b/internal/smtpsrv/server.go
@@ -132,9 +132,13 @@ func (s *Server) AddDomain(d string) {
 	s.aliasesR.AddDomain(d)
 }
 
-// AddUserDB adds a userdb.DB instance as backend for the domain.
-func (s *Server) AddUserDB(domain string, db *userdb.DB) {
-	s.authr.Register(domain, auth.WrapNoErrorBackend(db))
+// AddUserDB adds a userdb file as backend for the domain.
+func (s *Server) AddUserDB(domain, f string) error {
+	// Load the userdb, and register it unconditionally (so reload works even
+	// if there are errors right now).
+	udb, err := userdb.Load(f)
+	s.authr.Register(domain, auth.WrapNoErrorBackend(udb))
+	return err
 }
 
 // AddAliasesFile adds an aliases file for the given domain.
diff --git a/internal/smtpsrv/server_test.go b/internal/smtpsrv/server_test.go
index eadbbde..348c3b9 100644
--- a/internal/smtpsrv/server_test.go
+++ b/internal/smtpsrv/server_test.go
@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"blitiri.com.ar/go/chasquid/internal/aliases"
+	"blitiri.com.ar/go/chasquid/internal/auth"
 	"blitiri.com.ar/go/chasquid/internal/domaininfo"
 	"blitiri.com.ar/go/chasquid/internal/maillog"
 	"blitiri.com.ar/go/chasquid/internal/testlib"
@@ -671,8 +672,8 @@ func realMain(m *testing.M) int {
 		udb.AddUser("testuser", "testpasswd")
 		s.aliasesR.AddAliasForTesting(
 			"to@localhost", "testuser@localhost", aliases.EMAIL)
+		s.authr.Register("localhost", auth.WrapNoErrorBackend(udb))
 		s.AddDomain("localhost")
-		s.AddUserDB("localhost", udb)
 
 		s.AddDomain("broken")
 		s.authr.Register("broken", &brokenAuthBE{})
diff --git a/internal/userdb/userdb.go b/internal/userdb/userdb.go
index 76e43d6..0c13f75 100644
--- a/internal/userdb/userdb.go
+++ b/internal/userdb/userdb.go
@@ -33,6 +33,7 @@ import (
 	"crypto/subtle"
 	"errors"
 	"fmt"
+	"os"
 	"sync"
 
 	"golang.org/x/crypto/scrypt"
@@ -59,8 +60,8 @@ func New(fname string) *DB {
 }
 
 // Load the database from the given file.
-// Return the database, and a fatal error if the database could not be
-// loaded.
+// Return the database, and an error if the database could not be loaded. If
+// the file does not exist, that is not considered an error.
 func Load(fname string) (*DB, error) {
 	db := New(fname)
 	err := protoio.ReadTextMessage(fname, db.db)
@@ -72,6 +73,12 @@ func Load(fname string) (*DB, error) {
 		db.db = &ProtoDB{Users: map[string]*Password{}}
 	}
 
+	if os.IsNotExist(err) {
+		// If the file does not exist now, it is not an error, as it might
+		// exist later and we want to be able to read it.
+		err = nil
+	}
+
 	return db, err
 }
 
diff --git a/internal/userdb/userdb_test.go b/internal/userdb/userdb_test.go
index cc1445f..478e519 100644
--- a/internal/userdb/userdb_test.go
+++ b/internal/userdb/userdb_test.go
@@ -293,14 +293,14 @@ func TestReload(t *testing.T) {
 		t.Errorf("expected 2 users, got %d", len(db.db.Users))
 	}
 
-	// Cause an even bigger error loading, check the database is not changed.
-	db.fname = "/does/not/exist"
+	// Delete the file (which is not considered an error).
+	os.Remove(fname)
 	err = db.Reload()
-	if err == nil {
-		t.Errorf("expected error, got nil")
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
 	}
-	if len(db.db.Users) != 2 {
-		t.Errorf("expected 2 users, got %d", len(db.db.Users))
+	if len(db.db.Users) != 0 {
+		t.Errorf("expected 0 users, got %d", len(db.db.Users))
 	}
 }
 
diff --git a/test/t-20-bad_configs/c-12-bad_users/.expected-error b/test/t-20-bad_configs/c-12-bad_users/.expected-error
new file mode 100644
index 0000000..7b43c1a
--- /dev/null
+++ b/test/t-20-bad_configs/c-12-bad_users/.expected-error
@@ -0,0 +1 @@
+users file error: open domains/testserver/users: permission denied
diff --git a/test/t-20-bad_configs/c-12-bad_users/chasquid.conf b/test/t-20-bad_configs/c-12-bad_users/chasquid.conf
new file mode 100644
index 0000000..a47c3db
--- /dev/null
+++ b/test/t-20-bad_configs/c-12-bad_users/chasquid.conf
@@ -0,0 +1,9 @@
+smtp_address: ":1025"
+submission_address: ":1587"
+submission_over_tls_address: ":1465"
+
+mail_delivery_agent_bin: "test-mda"
+mail_delivery_agent_args: "%to%"
+
+data_dir: "../.data"
+mail_log_path: "../.logs/mail_log"
diff --git a/test/t-20-bad_configs/c-12-bad_users/domains/testserver/users b/test/t-20-bad_configs/c-12-bad_users/domains/testserver/users
new file mode 100644
index 0000000..0ad775b
--- /dev/null
+++ b/test/t-20-bad_configs/c-12-bad_users/domains/testserver/users
@@ -0,0 +1,26 @@
+users:  {
+  key:  "someone"
+  value:  {
+    scrypt:  {
+      logN:  14
+      r:  8
+      p:  1
+      keyLen:  32
+      salt:  "J\x01\xed7]\x02\n\xe9;z[\x8d˱\x10\xc1"
+      encrypted:  "\xa50宴\xcbb\xc1!r]K\xd1yI\xa2\x99\x8d\xdaQx\x8e69\xac\xf4$\x01\x11\x03\x8d\x10"
+    }
+  }
+}
+users:  {
+  key:  "user"
+  value:  {
+    scrypt:  {
+      logN:  14
+      r:  8
+      p:  1
+      keyLen:  32
+      salt:  "\n\xc6\x1c\x8f\xb2\x0c\x15p\x8d\xa1\xc3\x05U6\xdb\xc4"
+      encrypted:  "\xc3\xe6B2\x84W\x1a\nq{\x07\xe0\x9c\x854\n\xac\xbc\xb7\x9c\x86Kyk\x8dj\x16\x1a\x8c$*N"
+    }
+  }
+}
diff --git a/test/t-20-bad_configs/c-13-bad_aliases/.expected-error b/test/t-20-bad_configs/c-13-bad_aliases/.expected-error
new file mode 100644
index 0000000..f0fcc23
--- /dev/null
+++ b/test/t-20-bad_configs/c-13-bad_aliases/.expected-error
@@ -0,0 +1 @@
+aliases file error: open domains/testserver/aliases: permission denied
diff --git a/test/t-20-bad_configs/c-13-bad_aliases/chasquid.conf b/test/t-20-bad_configs/c-13-bad_aliases/chasquid.conf
new file mode 100644
index 0000000..a47c3db
--- /dev/null
+++ b/test/t-20-bad_configs/c-13-bad_aliases/chasquid.conf
@@ -0,0 +1,9 @@
+smtp_address: ":1025"
+submission_address: ":1587"
+submission_over_tls_address: ":1465"
+
+mail_delivery_agent_bin: "test-mda"
+mail_delivery_agent_args: "%to%"
+
+data_dir: "../.data"
+mail_log_path: "../.logs/mail_log"
diff --git a/test/t-20-bad_configs/c-13-bad_aliases/domains/testserver/aliases b/test/t-20-bad_configs/c-13-bad_aliases/domains/testserver/aliases
new file mode 100644
index 0000000..26a745d
--- /dev/null
+++ b/test/t-20-bad_configs/c-13-bad_aliases/domains/testserver/aliases
@@ -0,0 +1 @@
+a: b
diff --git a/test/t-20-bad_configs/run.sh b/test/t-20-bad_configs/run.sh
index b3bbb3c..67c5693 100755
--- a/test/t-20-bad_configs/run.sh
+++ b/test/t-20-bad_configs/run.sh
@@ -19,7 +19,7 @@ mkdir -p c-04-no_cert_dirs/certs/
 # Generate certs for the tests that need them.
 for i in c-05-no_addrs c-06-bad_maillog c-07-bad_domain_info \
 	c-08-bad_sts_cache c-09-bad_queue_dir c-10-empty_listening_addr \
-	c-11-bad_dkim_key;
+	c-11-bad_dkim_key c-12-bad_users c-13-bad_aliases;
 do
 	CONFDIR=$i/ generate_certs_for testserver
 done
@@ -30,6 +30,10 @@ done
 cp c-11-bad_dkim_key/domains/testserver/dkim__selector.pem \
 	c-11-bad_dkim_key/domains/testserver/dkim:selector.pem
 
+# For the bad_users and bad_aliases test, make the relevant file unreadable.
+chmod -rw c-12-bad_users/domains/testserver/users
+chmod -rw c-13-bad_aliases/domains/testserver/aliases
+
 for i in c-*; do
 	if chasquid --config_dir="$i" > ".chasquid-$i.out" 2>&1; then
 		echo "$i failed; output:"
@@ -54,4 +58,8 @@ for i in c-*; do
 	fi
 done
 
+# Give permissions back, to avoid annoying git messages.
+chmod +rw c-12-bad_users/domains/testserver/users
+chmod +rw c-13-bad_aliases/domains/testserver/aliases
+
 success