base-trixie: findings of the reviews
test / test (push) Canceled after 0s

Security review (1c6e04b):
- The panel's database and its -wal/-shm were 0644 under a traversable
  /data, holding the panel users' password hashes and session token
  hashes. store.Open now makes them 0600 (SQLite gives the next -wal/-shm
  the database's mode: findCreateFileMode(), unixOpenSharedMemory());
  TestOpenKeepsTheDatabasePrivate.
- The entrypoint removes a symbolic link the panel user could leave where
  a /data directory, table, map, the queue or mail.log is expected, before
  its root steps chmod or create them (drop_links).
- The panel user is 10999:10999, outside the system range: in trixie
  useradd gave it group 998, the host's systemd-network on the test
  server; bookworm's 999 was the host's systemd-journal.
- security.md said the database was 0640; it was 0644.

Technical review (1c6e04b):
- mail_helpers.go: smtp-sink does add a Received: header, "(smtp-sink)"
  (smtp-sink.c:520-531); the "(Postfix)" filter leaves it out.
- CHANGELOG/roadmap: OpenDKIM's signing and From:-parsing code is
  unchanged, not all of it; the retry-policy bug predates 3.9 (any
  postconf warning); three entries moved from Fixed to Changed, as no
  released image had them.
- internal/app/sasl.go: db_dump's stdout alone is parsed.
- mynetworks comment: postinst:180, fset_all_changed.

Tests: on the server, make e2e exit 0 (712 s), go vet + go test 27
packages ok.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-15 20:05:01 +03:00
co-authored by Claude Opus 5.5
parent 1c6e04b7d3
commit a3050afb66
10 changed files with 164 additions and 31 deletions
+8 -2
View File
@@ -309,11 +309,17 @@ func runSaslpasswd2(args []string, stdin []byte) error {
// dumpSASLDB runs db_dump to export the sasldb2 as key/value hex pairs. The path
// is our own sasldb2 file (never user input) and is passed as a fixed argument
// with no shell (security.md).
//
// Only standard output is the dump: a message db_dump writes to standard
// error is not data and is left out of what is parsed (the same rule as
// postconf's values in internal/postfix, retry.go; review of base-trixie).
func dumpSASLDB(path string) ([]byte, error) {
cmd := exec.Command("db_dump", path)
out, err := cmd.CombinedOutput()
var errOut bytes.Buffer
cmd.Stderr = &errOut
out, err := cmd.Output()
if err != nil {
return nil, fmt.Errorf("db_dump: %w: %s", err, strings.TrimSpace(string(out)))
return nil, fmt.Errorf("db_dump: %w: %s", err, strings.TrimSpace(errOut.String()))
}
return out, nil
}
+43
View File
@@ -0,0 +1,43 @@
package store
import (
"os"
"path/filepath"
"runtime"
"testing"
)
// The database and its WAL and shared-memory files are the panel's alone:
// a file that was 0644 is 0600 after Open, and the -wal and -shm files
// SQLite opens are 0600 too (keepPrivate).
func TestOpenKeepsTheDatabasePrivate(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("file modes are POSIX")
}
path := filepath.Join(t.TempDir(), "selfpost.db")
st, err := Open(path)
if err != nil {
t.Fatal(err)
}
st.Close()
if err := os.Chmod(path, 0o644); err != nil {
t.Fatal(err)
}
st, err = Open(path)
if err != nil {
t.Fatal(err)
}
defer st.Close()
if _, err := st.db.Exec(`PRAGMA user_version`); err != nil {
t.Fatal(err)
}
for _, p := range []string{path, path + "-wal", path + "-shm"} {
fi, err := os.Stat(p)
if err != nil {
t.Fatalf("%s: %v", filepath.Base(p), err)
}
if m := fi.Mode().Perm(); m != 0o600 {
t.Errorf("%s mode %o, want 600", filepath.Base(p), m)
}
}
}
+28
View File
@@ -11,6 +11,7 @@ import (
"errors"
"fmt"
"io/fs"
"os"
"sort"
_ "modernc.org/sqlite" // pure-Go SQLite driver (no cgo), keeps the static build
@@ -63,9 +64,36 @@ func Open(path string) (*Store, error) {
db.Close()
return nil, err
}
if err := keepPrivate(path); err != nil {
db.Close()
return nil, err
}
return s, nil
}
// keepPrivate makes the database file and its WAL and shared-memory files
// the panel's alone (0600). They hold the panel users' password hashes and
// the session token hashes, and /data is traversable by every user of the
// container (entrypoint.sh) - Postfix and OpenDKIM among them. SQLite
// creates a file with 0644 less the umask, and the panel sets no umask; the
// -wal and -shm files take the database file's mode when they are created
// (SQLite 3.53.4 as modernc.org/sqlite v1.60.1 builds it,
// lib/sqlite_linux_386.go: findCreateFileMode(), "WAL and journal files are
// created using the same permissions as the associated database file";
// unixOpenSharedMemory(), robust_open(... st_mode&0777) of the database's
// fstat). So the database file is chmodded and the two files beside it,
// where they exist already; the next ones follow its mode. Found by the
// security review of base-trixie (2026-10-15): the file was 0644 on the
// test server.
func keepPrivate(path string) error {
for _, p := range []string{path, path + "-wal", path + "-shm"} {
if err := os.Chmod(p, 0o600); err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("database mode: %w", err)
}
}
return nil
}
// Close closes the underlying database.
func (s *Store) Close() error {
return s.db.Close()