sasldb2 changed through a copy renamed over it, never in place

Cyrus SASL 2.1.28 opens sasldb2 with no Berkeley DB environment, so with
no locks (sasldb/db_berkeley.c, berkeleydb_open(): db_create(mbdb, NULL,
0)); saslpasswd2 puts and deletes in the file itself (_sasldb_putdata()),
while smtpd opens and closes it at every AUTH (_sasldb_getdata()) and the
backup copies it whenever it runs. Either could read a half-written page:
a spurious 535, or a torn sasldb2 in a backup whose restore breaks every
application's AUTH (assumptions C-S3, C-Q3).

SASLDB.Set and Delete now run saslpasswd2 on sasldb2.new - a byte copy of
the database, or no file when there is none yet (saslpasswd2 creates it,
DB_CREATE) - then chmod 0640, fsync, rename over sasldb2; a failed run
removes the copy and leaves the database; a copy a killed panel left is
removed first. A mutex keeps two changes of the panel from copying the
same old file. The backup needs no lock after all (the plan said one): it
reads each file through one descriptor, and a rename leaves that file
whole; the walk now leaves out a file that vanished between listing and
open (a writer's temporary file), while the SQLite snapshot's absence
stays an error. CHANGELOG also gains the entries of f2ad699 (supervision),
written then without them.

Tests: the copy holds the database's bytes, saslpasswd2 never gets the
live path, a reader of the old file keeps reading it (Linux), mode 0640,
nothing left behind; a failed change leaves the database; the first
account creates it. Stand (2026-10-15, make e2e, exit 0): new
sasldb_changed_by_rename - the inode of sasldb2 changes across one
password change (it cannot be the old one's: both exist at the copy),
then 35 AUTHs of app1 from three clients during 104 password changes of
another application, none refused; afterwards 640 panel selfpost, no copy
left in /data/sasl, the last password authenticates. That cannot prove a
race absent; it guards the mechanism. (A first run compared the inode
after 15 changes and found it equal: Linux reuses a freed inode number.)
go vet and go test ./... pass on the server (24 packages).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/app/sasl.go

@@ -4,23 +4,45 @@ import (
 	"bufio"
 	"bytes"
 	"encoding/hex"
+	"errors"
 	"fmt"
+	"io"
+	"os"
 	"os/exec"
 	"strings"
+	"sync"
 )
 
 // SASLDB manages the Cyrus SASL account database (sasldb2) the panel maintains
 // for application credentials (architecture.md § Mail path). The panel is the
 // only writer; Postfix reads it to authenticate SMTP clients. Accounts are
 // created and removed with the standard saslpasswd2 tool.
+//
+// saslpasswd2 is never pointed at the live file. Cyrus SASL 2.1.28 opens
+// sasldb2 with no Berkeley DB environment, so without locks
+// (sasldb/db_berkeley.c, berkeleydb_open(): db_create(mbdb, NULL, 0)), and a
+// write puts and deletes in the file itself (_sasldb_putdata()); smtpd opens
+// and closes it for every lookup (_sasldb_getdata()), and the backup reads it
+// whenever it runs. Either could read a page half written: a spurious 535 for
+// an application, or a torn sasldb2 in a backup whose restore breaks every
+// application's AUTH (assumptions C-S3, C-Q3). So each change is made to a
+// copy, which is then renamed over the file (replaceSASLDB): a reader opens
+// the old file or the new one, whole, and needs no lock. mu keeps the
+// panel's own changes from copying the same old file at once.
 type SASLDB struct {
 	path  string // sasldb2 file, under /data so it survives restarts
 	realm string // SASL realm, so lookups match what Postfix's SASL uses
 
+	mu sync.Mutex
+
 	// run executes saslpasswd2. It is a field so tests can substitute a fake;
 	// the default shells out to the real binary via runSaslpasswd2.
 	run func(args []string, stdin []byte) error
 
+	// replace makes a change to the database through a copy: edit gets the
+	// path to give saslpasswd2. The default is replaceSASLDB.
+	replace func(path string, edit func(path string) error) error
+
 	// dump reads the raw sasldb2 as db_dump key/value pairs (Berkeley DB). It is
 	// a field so tests can substitute a fake; the default runs db_dump.
 	dump func(path string) ([]byte, error)
@@ -36,12 +58,89 @@ type SASLDB struct {
 // sasldb_auxprop_lookup(), then looks a login without "@" up under the
 // server's name (common/plugin_common.c, _plug_parseuser()).
 func NewSASLDB(path, realm string) *SASLDB {
-	return &SASLDB{path: path, realm: realm, run: runSaslpasswd2, dump: dumpSASLDB}
+	return &SASLDB{path: path, realm: realm, run: runSaslpasswd2, dump: dumpSASLDB, replace: replaceSASLDB}
+}
+
+// change runs saslpasswd2 with args on a copy of the database and puts the
+// copy in its place; at in args stands for the path of the copy.
+func (s *SASLDB) change(args []string, at int, stdin []byte) error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.replace(s.path, func(path string) error {
+		a := append([]string(nil), args...)
+		a[at] = path
+		return s.run(a, stdin)
+	})
+}
+
+// replaceSASLDB copies the database at path to path+".new", lets edit change
+// the copy, and renames it over path. With no database yet, edit gets a path
+// where none exists, and saslpasswd2 creates one there (db_berkeley.c,
+// berkeleydb_open(): DB_CREATE for a writer). The copy is a byte copy of a
+// file no one else writes, which is a whole Berkeley DB file: smtpd and
+// db_dump only read it. The new file is 0640 in the setgid directory, group
+// selfpost, as build/entrypoint.sh sets the old one, so Postfix reads it.
+// A copy left by a panel killed mid-change is removed first.
+func replaceSASLDB(path string, edit func(path string) error) error {
+	tmp := path + ".new"
+	if err := os.Remove(tmp); err != nil && !errors.Is(err, os.ErrNotExist) {
+		return fmt.Errorf("remove %s: %w", tmp, err)
+	}
+	if err := copySASLDB(path, tmp); err != nil {
+		_ = os.Remove(tmp)
+		return err
+	}
+	if err := edit(tmp); err != nil {
+		_ = os.Remove(tmp)
+		return err
+	}
+	// Read-write: fsync of a file opened read-only is refused on Windows.
+	f, err := os.OpenFile(tmp, os.O_RDWR, 0)
+	if err == nil {
+		err = f.Chmod(0o640)
+		if err == nil {
+			err = f.Sync()
+		}
+		if cerr := f.Close(); err == nil {
+			err = cerr
+		}
+	}
+	if err != nil {
+		_ = os.Remove(tmp)
+		return fmt.Errorf("finish %s: %w", tmp, err)
+	}
+	if err := os.Rename(tmp, path); err != nil {
+		_ = os.Remove(tmp)
+		return fmt.Errorf("rename %s -> %s: %w", tmp, path, err)
+	}
+	return nil
+}
+
+// copySASLDB copies path to tmp; with no file at path it makes no copy.
+func copySASLDB(path, tmp string) error {
+	src, err := os.Open(path)
+	if errors.Is(err, os.ErrNotExist) {
+		return nil
+	}
+	if err != nil {
+		return fmt.Errorf("open %s: %w", path, err)
+	}
+	defer src.Close()
+	dst, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o640)
+	if err != nil {
+		return fmt.Errorf("create %s: %w", tmp, err)
+	}
+	if _, err := io.Copy(dst, src); err != nil {
+		dst.Close()
+		return fmt.Errorf("copy %s: %w", path, err)
+	}
+	return dst.Close()
 }
 
 // Set creates or updates an application's SASL account with the given password
 // (architecture.md § Mail path). Used both at creation and when a password is
-// regenerated; saslpasswd2 overwrites an existing entry in place.
+// regenerated; saslpasswd2 overwrites an existing entry (in the copy, see
+// SASLDB).
 //
 // The password is passed to saslpasswd2 on stdin (never as an argument, so it
 // cannot leak through the process table or logs). The login is passed as a
@@ -65,7 +164,7 @@ func (s *SASLDB) Set(login, password string) error {
 	//     whitelist already forbids nothing that getopt would eat, but a login
 	//     starting with '-' is legal there — this keeps it an operand).
 	args := []string{"-p", "-c", "-f", s.path, "-u", s.realm, "--", login}
-	if err := s.run(args, []byte(password)); err != nil {
+	if err := s.change(args, 3, []byte(password)); err != nil {
 		return fmt.Errorf("saslpasswd2 set %q: %w", login, err)
 	}
 	return nil
@@ -84,7 +183,7 @@ func (s *SASLDB) Delete(login string) error {
 	}
 	// -d: delete the account. "--" as in Set: the login is always an operand.
 	args := []string{"-d", "-f", s.path, "-u", s.realm, "--", login}
-	if err := s.run(args, nil); err != nil {
+	if err := s.change(args, 2, nil); err != nil {
 		return fmt.Errorf("saslpasswd2 delete %q: %w", login, err)
 	}
 	return nil
@@ -213,8 +312,13 @@ func dumpSASLDB(path string) ([]byte, error) {
 
 // WithRunner replaces the command that runs saslpasswd2 and returns the same
 // manager. It exists for tests outside this package that need an application
-// service without the real binary; the panel never calls it.
+// service without the real binary; the panel never calls it. With no real
+// database either, the runner is given the database's own path, not a copy.
 func (s *SASLDB) WithRunner(run func(args []string, stdin []byte) error) *SASLDB {
 	s.run = run
+	s.replace = inPlace
 	return s
 }
+
+// inPlace hands edit the path itself: for tests whose runner touches no file.
+func inPlace(path string, edit func(path string) error) error { return edit(path) }

internal/app/sasl_replace_test.go

@@ -0,0 +1,114 @@
+package app
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+)
+
+// A change is made to a copy and renamed over the database: saslpasswd2
+// never gets the live file, and a reader that opened the old file keeps
+// reading it whole (assumptions C-S3).
+func TestSASLChangeGoesThroughACopy(t *testing.T) {
+	dir := t.TempDir()
+	db := filepath.Join(dir, "sasldb2")
+	if err := os.WriteFile(db, []byte("old database"), 0o640); err != nil {
+		t.Fatal(err)
+	}
+	// smtpd in the middle of a lookup. Windows refuses a rename over an open
+	// file; the container is Linux, where the reader keeps the old inode.
+	var reader *os.File
+	if runtime.GOOS != "windows" {
+		var err error
+		if reader, err = os.Open(db); err != nil {
+			t.Fatal(err)
+		}
+		defer reader.Close()
+	}
+
+	s := NewSASLDB(db, "mail.example.com")
+	var gotPath string
+	s.run = func(args []string, stdin []byte) error {
+		gotPath = args[3]
+		before, err := os.ReadFile(gotPath)
+		if err != nil {
+			return err
+		}
+		if string(before) != "old database" {
+			t.Errorf("the copy holds %q, want the database's bytes", before)
+		}
+		return os.WriteFile(gotPath, []byte("new database"), 0o600)
+	}
+	if err := s.Set("alerts", "pw"); err != nil {
+		t.Fatalf("Set: %v", err)
+	}
+	if gotPath == db {
+		t.Fatal("saslpasswd2 was given the live database")
+	}
+	if got, _ := os.ReadFile(db); string(got) != "new database" {
+		t.Fatalf("database after the change: %q", got)
+	}
+	if runtime.GOOS != "windows" {
+		if fi, _ := os.Stat(db); fi.Mode().Perm() != 0o640 {
+			t.Fatalf("database mode %v, want 0640 for Postfix's group", fi.Mode().Perm())
+		}
+		old := make([]byte, 64)
+		n, _ := reader.ReadAt(old, 0)
+		if string(old[:n]) != "old database" {
+			t.Fatalf("a reader of the old file read %q", old[:n])
+		}
+	}
+	if _, err := os.Stat(db + ".new"); !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("the copy is left behind: %v", err)
+	}
+}
+
+func TestSASLFailedChangeLeavesTheDatabase(t *testing.T) {
+	dir := t.TempDir()
+	db := filepath.Join(dir, "sasldb2")
+	if err := os.WriteFile(db, []byte("old database"), 0o640); err != nil {
+		t.Fatal(err)
+	}
+	// A copy a killed panel left behind is not taken for the database.
+	if err := os.WriteFile(db+".new", []byte("stale"), 0o640); err != nil {
+		t.Fatal(err)
+	}
+	s := NewSASLDB(db, "mail.example.com")
+	s.run = func(args []string, stdin []byte) error {
+		if b, _ := os.ReadFile(args[2]); string(b) != "old database" {
+			t.Errorf("the copy holds %q", b)
+		}
+		_ = os.WriteFile(args[2], []byte("half"), 0o640)
+		return errors.New("saslpasswd2: exit status 1")
+	}
+	if err := s.Delete("alerts"); err == nil {
+		t.Fatal("Delete succeeded with saslpasswd2 failing")
+	}
+	if got, _ := os.ReadFile(db); string(got) != "old database" {
+		t.Fatalf("database after a failed change: %q", got)
+	}
+	if _, err := os.Stat(db + ".new"); !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("the copy is left behind: %v", err)
+	}
+}
+
+// The first account: there is no database to copy, and saslpasswd2 creates
+// one at the path it is given.
+func TestSASLFirstAccountCreatesTheDatabase(t *testing.T) {
+	db := filepath.Join(t.TempDir(), "sasldb2")
+	s := NewSASLDB(db, "mail.example.com")
+	s.run = func(args []string, stdin []byte) error {
+		if _, err := os.Stat(args[3]); !errors.Is(err, os.ErrNotExist) {
+			t.Errorf("saslpasswd2 given an existing file %s: %v", args[3], err)
+		}
+		return os.WriteFile(args[3], []byte("created"), 0o660)
+	}
+	if err := s.Set("alerts", "pw"); err != nil {
+		t.Fatalf("Set: %v", err)
+	}
+	if got, _ := os.ReadFile(db); string(got) != "created" {
+		t.Fatalf("database: %q", got)
+	}
+}

internal/app/sasl_test.go

@@ -17,6 +17,7 @@ type fakeRun struct {
 func newFakeSASL() (*SASLDB, *fakeRun) {
 	fr := &fakeRun{}
 	s := NewSASLDB("/data/sasl/sasldb2", "mail.example.com")
+	s.replace = inPlace
 	s.run = func(args []string, stdin []byte) error {
 		fr.calls++
 		fr.args = args

internal/app/service_test.go

@@ -44,6 +44,7 @@ func newServiceHarness(t *testing.T) (*Service, *store.Store, *saslRecorder, *fa
 
 	rec := &saslRecorder{set: map[string]string{}}
 	sasl := NewSASLDB("/data/sasl/sasldb2", "mail.example.com")
+	sasl.replace = inPlace
 	sasl.run = func(args []string, stdin []byte) error {
 		if rec.failNext {
 			rec.failNext = false

internal/backup/backup.go

@@ -323,7 +323,13 @@ func addTree(tw *tar.Writer, root, archivePrefix string, exclude map[string]bool
 			}
 			return tw.WriteHeader(hdr)
 		case info.Mode().IsRegular():
-			return writeTarFile(tw, name, info.Mode().Perm(), path)
+			err := writeTarFile(tw, name, info.Mode().Perm(), path)
+			if errors.Is(err, fs.ErrNotExist) {
+				// A writer's temporary file, renamed into place between
+				// the listing and the open: not part of /data's state.
+				return nil
+			}
+			return err
 		default:
 			return nil
 		}
@@ -349,6 +355,14 @@ func writeTarBytes(tw *tar.Writer, name string, mode int64, data []byte) error {
 }
 
 // writeTarFile streams a file from disk into the archive under name.
+//
+// The file is read through one descriptor, sized by that descriptor: the
+// panel's writers of what a backup carries put a new file in place by rename
+// (atomicfile.Write; sasldb2 through a copy, app.replaceSASLDB - the setup
+// token alone is written in place, and it is not carried; the database and
+// the queue are not walked), and a rename leaves the file open here as it
+// was: the archive holds the old file or the new one, whole (assumptions
+// C-Q3).
 func writeTarFile(tw *tar.Writer, name string, mode fs.FileMode, srcPath string) error {
 	f, err := os.Open(srcPath)
 	if err != nil {