diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dcc8d6..ce48622 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,20 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Security +- **The panel's database was readable by every user of the container.** + `selfpost.db` and its `-wal` and `-shm` files were 0644 under a + traversable `/data`; they hold the panel users' password hashes and the + session token hashes, and Postfix's smtpd - which faces the Internet - + and OpenDKIM run as other users there. The panel now makes them 0600 at + every start; SQLite gives the next `-wal` and `-shm` the database's mode. + Found by the security review of this release. +- **The entrypoint no longer follows a symbolic link the panel user could + leave under `/data`.** Several of its steps run as root and chmod, + create or regroup paths in a tree the panel writes; a link where a + directory or table is expected is now removed first. No escalation was + found - a compromised panel could only have turned it against the + container itself. + - **The image runs Debian trixie instead of bookworm** (base-trixie). Bookworm left regular security support on 2026-07-12, and its Postfix 3.7.11 lacks upstream's 2026 security fixes, which carry no CVE ids and @@ -14,7 +28,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version August batch (among them a BDAT memory exhaustion, a crash by DATA after a BDAT error, and a policy and milter bypass after an end-of-data reject), Debian's security support until 2028, OpenSSL 3.5 and libmilter - 8.18.1. OpenDKIM's code is unchanged. The build stage is + 8.18.1. OpenDKIM's signing and From:-parsing code is unchanged (Debian's + new patches touch the verifier's Authentication-Results and librbl). + The build stage is `golang:1.26-trixie`. ### Changed @@ -54,7 +70,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version the trixie image a package took it and the build failed, and on a host it is often a system group of the host's own, which could then read the key as well (on the test server's host it is `systemd-network`). 10998 is - outside that range. **On upgrade, give the private key in `certs/` to + outside that range. The panel user is 10999:10999 for the same reason + (it was 999:999, the host's `systemd-journal` group on the test server). + **On upgrade, give the private key in `certs/` to the new group on the host** - `chgrp 10998` where the guide said `chgrp 998`, in a certbot deploy hook too - or a full backup is refused until then; Postfix reads its own copy and is not affected. The other @@ -63,22 +81,27 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version - `opendkim-tools` is no longer installed: nothing in the image ran it. `openssl` is installed by name, for the certificate reload. +- **Made ready for what trixie's programs do differently:** the log tailer + reads Postfix 3.10's refusal lines that carry `sasl_method=`, + `sasl_username=` and `sasl_sender=`, and cleanup's `removed (canceled)` + for a transaction that did not complete; the rotation loop refuses on + logrotate 3.22's "Ignoring" output, now with exit status 1 where 3.21 + gave 0; Debian's chroot sync, which trixie's postfix runs at every + start, is turned off (`SYNC_CHROOT=` in `/etc/default/postfix`; nothing + runs chrooted), so `/etc/resolv.conf` and the like are not copied into + the queue under `/data`; a mail.log Postfix creates itself is `0640`, as + the entrypoint and logrotate create it. + ### Fixed -- **The retry policy shown in the panel could fall back to Postfix's - compiled-in defaults:** the panel read `postconf`'s warnings together - with its values, and since Postfix 3.9 `postconf` warns about deprecated - parameters. It reads the values alone now and logs the warning. -- **logrotate 3.22 exits 1 when it skips a config it does not trust;** - 3.21 exited 0. The rotation loop now refuses to run on that output - whatever the exit code, instead of retrying for ever. -- **The log tailer reads Postfix 3.10's new line shapes:** refusal lines - that carry `sasl_method=`, `sasl_username=` and `sasl_sender=`, and - cleanup's `removed (canceled)` for a transaction that did not complete. -- Debian's chroot sync no longer copies `/etc/resolv.conf` and the like - into the queue under `/data` at every start (`SYNC_CHROOT=` in - `/etc/default/postfix`; nothing runs chrooted). A Postfix-created - mail.log is `0640`, as the entrypoint and logrotate create it. +- **The retry policy shown in the panel fell back to Postfix's compiled-in + defaults whenever `postconf` warned:** the panel read `postconf`'s + standard error together with its values, and a warning there - an + unused parameter in main.cf, and since Postfix 3.9 a deprecated one - + was one value too many. The image's own main.cf sets none, so it took a + parameter an operator added. The panel reads the values alone now and + logs the warning. `db_dump`'s output, which the panel parses to list the + SASL accounts, is read the same way. ## [2.6.1] - 2026-10-15 diff --git a/build/Dockerfile b/build/Dockerfile index c535ef4..fef9931 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -85,10 +85,14 @@ RUN echo "postfix postfix/mailname string localhost" | debconf-set-selections \ # it off (debian/configure-instance.in:22-24 and :44; debian/README.Debian:136-138). RUN printf 'SYNC_CHROOT=\n' > /etc/default/postfix -# Unprivileged user for the panel process. Its ids are left to useradd: the -# entrypoint gives /data back to it by name on every start, so a number -# that differs from one image to the next changes nothing. -RUN useradd --system --no-create-home --shell /usr/sbin/nologin panel +# Unprivileged user for the panel process, with ids of its own outside the +# system range for the same reason as selfpost's below: its files under +# /data show on the host by number, and a number from that range is a +# system group of the host's too (useradd gave the panel group 998 in the +# trixie image, systemd-network on the test server's host; bookworm's +# 999:999 was the host's systemd-journal group). The entrypoint gives /data +# back to it by name on every start, so files of the old ids follow. +RUN groupadd --system --gid 10999 panel && useradd --system --uid 10999 --gid panel --no-create-home --shell /usr/sbin/nologin panel # Shared group bridging the unprivileged services: the panel generates # per-domain DKIM keys, application SASL accounts (sasldb2) and the Postfix diff --git a/build/entrypoint.sh b/build/entrypoint.sh index 617c874..cc7ec8c 100644 --- a/build/entrypoint.sh +++ b/build/entrypoint.sh @@ -66,6 +66,31 @@ esac # mode 0o700".) chown panel:panel /data chmod 755 /data + +# What follows runs as root over a tree the panel user writes: /data is the +# panel's, and so is /data/postfix. A symbolic link the panel put where a +# directory or a file of its own is expected would lead root's mkdir, ": >", +# chmod and chgrp to whatever it names (chmod follows a link; so do a +# redirect and mkdir -p; chown -R and find do not descend into one given as +# their operand). None of these paths is ever a link in a working +# installation, so a link there is removed and the path made anew below. +# (Security review of base-trixie, 2026-10-15: no escalation found, as the +# TLS key copy and the read-only mounts stand, but nothing for root to +# follow either.) +drop_links() { + for p in "$@"; do + if [ -L "$p" ]; then + echo "entrypoint: removing the symbolic link $p" >&2 + rm -f "$p" + fi + done +} +drop_links /data/opendkim /data/sasl /data/postfix /data/antispam /data/log \ + /data/opendkim/keys /data/opendkim/KeyTable /data/opendkim/SigningTable \ + /data/sasl/sasldb2 /data/postfix/queue \ + /data/postfix/sender_login_maps /data/postfix/relay_domains \ + /data/postfix/transport /data/postfix/relay_recipients /data/postfix/tls_policy \ + /data/log/mail.log # Restored backups or previously-created state may contain panel-owned files # under /data; make sure they stay writable without disturbing anything that a # later phase deliberately hands to another service. /data/log is exempt: it is @@ -73,7 +98,7 @@ chmod 755 /data # normalised on its own below, as are the trees shared through group selfpost. # A file's owner is a number: an entry whose user or group is not the panel's # by that number - unpacked by root, or written under another image whose -# useradd gave the panel other ids (bookworm's 999:999, until 2.7.0) - is +# image gave the panel other ids (999:999 until 2.7.0, 10999:10999 since) - is # handed over whole. find /data -mindepth 1 -maxdepth 1 ! -name log ! -name postfix ! -name opendkim ! -name sasl ! -name antispam \ \( ! -user panel -o ! -group panel \) -exec chown -R panel:panel {} + diff --git a/build/postfix-config.sh b/build/postfix-config.sh index c0dad4e..a63ebd5 100644 --- a/build/postfix-config.sh +++ b/build/postfix-config.sh @@ -387,7 +387,8 @@ postconf -e \ # what the exceptions above and smtpd_forbid_bare_newline_exclusions (below) # spare. The value is the one Debian's debconf default gives (postfix # 3.10.13-0+deb13u1, debian/templates:124-126, "postfix/mynetworks", written -# by debian/postfix.postinst:249-262; debian/main.cf.in:28 has 127.0.0.0/8 +# by debian/postfix.postinst:249-262 on a new main.cf, where :180, +# fset_all_changed, marks every debconf value to be written; main.cf.in:28 has 127.0.0.0/8 # alone), so on this image it changes nothing. postconf -e \ "mynetworks=127.0.0.0/8 [::ffff:127.0.0.0]/104 [::1]/128" \ diff --git a/docs/roadmap.md b/docs/roadmap.md index 752eaed..37be878 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -825,8 +825,9 @@ attention to 3.10's reject logging (`sasl_method`, `sasl_username`, cleanup's `canceled` line, 3.8's `postfix(1)` stderr logging under supervisord; `dev/upstream` refetched from trixie; the cited-package table and `checkCitedPackageVersions`; the e2e sink image; `opendkim-tools` -dropped from the image (used by nothing at run time). OpenDKIM's code is -the same in trixie, its citations stand. One stage, a MINOR release. Also +dropped from the image (used by nothing at run time). OpenDKIM's signing +and From:-parsing code is the same in trixie (its new Debian patches touch +the verifier and librbl), its citations stand. One stage, a MINOR release. Also gained: `smtpd_hide_client_session` (3.10); the owner decided on 2026-10-15 to turn it on for 465 and 587 in the same release, so the `Received:` header no longer shows the application's address. diff --git a/docs/security.md b/docs/security.md index 0297ab5..b115a47 100644 --- a/docs/security.md +++ b/docs/security.md @@ -518,11 +518,11 @@ deferred item from the roadmap. - **The filter lists are files readable by anyone on the host.** `/data/antispam` is 0755 and its six files 0644, unlike the rest of `/data`: the filter runs under another user in another container (rspamd as - `_rspamd`, uid 11333, in `rspamd/rspamd:3.11.1-3`; the panel is uid 999 - - measured on a live server, 2026-10-15). They hold sender + `_rspamd`, uid 11333, in `rspamd/rspamd:3.11.1-3`, measured on a live + server, 2026-10-15; the panel is uid 10999 since `2.7.0`). They hold sender addresses, domains and networks - who the operator blocks and whom he lets through - but no message data. The inbound journal itself is in the - database, 0640. Trigger to revisit: a shared host, where the sidecar should + database, which is 0600 since `2.7.0` (0644 before). Trigger to revisit: a shared host, where the sidecar should instead be given the files through a group it shares with the panel. - ~~**An application can learn which domains are senders of this server.**~~ Struck in `2.5.2` (audit 2026-10-12, stage 2c; L8 closed): the sender diff --git a/internal/app/sasl.go b/internal/app/sasl.go index 4659a0d..d9cbda2 100644 --- a/internal/app/sasl.go +++ b/internal/app/sasl.go @@ -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 } diff --git a/internal/store/mode_test.go b/internal/store/mode_test.go new file mode 100644 index 0000000..8e78c67 --- /dev/null +++ b/internal/store/mode_test.go @@ -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) + } + } +} diff --git a/internal/store/store.go b/internal/store/store.go index 75bb218..83d4101 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -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() diff --git a/test/e2e/mail_helpers.go b/test/e2e/mail_helpers.go index fbef5f9..443a74e 100644 --- a/test/e2e/mail_helpers.go +++ b/test/e2e/mail_helpers.go @@ -78,8 +78,10 @@ func verifyDKIM(raw []byte, domain string) error { // postfixReceived returns the Received: headers of raw that a Postfix smtpd // wrote - "(Postfix)" after the host name, the only form it writes (Postfix // 3.10.13, src/smtpd/smtpd.c:3627-3645, "by %s (%s)" with var_mail_name). -// In the stand only SelfPost's Postfix writes one: the sink is smtp-sink, -// which adds none, and the tests compose their messages without any. +// In the stand only SelfPost's Postfix writes one: the tests compose their +// messages without any, and the sink, smtp-sink, adds its own with "by +// (smtp-sink)" (src/smtpstone/smtp-sink.c:520-531, +// mail_file_finish_header()), which the "(Postfix)" filter leaves out. func postfixReceived(raw []byte) ([]string, error) { msg, err := mail.ReadMessage(bytes.NewReader(raw)) if err != nil {