diff --git a/CHANGELOG.md b/CHANGELOG.md index a1f363c..811fd9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,22 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Upgrading + +- **Give the TLS private key to group 10997** (audit I1): `chgrp 10997 + certs/privkey.pem` - or certbot's `archive//privkey*.pem` - and the + same in whatever renews it. Group 10998, which the guide named until now, + has Postfix in it; the backup keeps working with either, but only 10997 + keeps the key from Postfix's daemons. + ### Security +- **The TLS private key no longer has to be readable by Postfix** (audit + I1): the guide gave it to group 10998 for the panel's backup, and + `postfix` is in that group, so every Postfix daemon - an smtpd talking to + the internet included - could read it. A new group, `backupkey` (10997), + holds the panel alone; the guide, Help and the backup's refusal name it. + - **A stolen `global` session no longer takes a full backup** (security audit 2026-10-17, A2, high): the user form set the signed-in user's own password without the current one, which then passed the backup's password diff --git a/build/Dockerfile b/build/Dockerfile index ed1842c..5056805 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -109,9 +109,10 @@ RUN groupadd --system --gid 10999 panel && useradd --system --uid 10999 --gid p # (set up in entrypoint.sh) - lets Postfix read the sasldb2/sender map and # connect to the panel's milters. The DKIM keys are the panel's alone. (The # supervisor control socket has a group of its own, `supervisor`, below.) -# The id is fixed, because the host sees it: an operator gives the TLS -# private key on the host to this group by number, so that the panel can put -# it into a full backup (guide § Taking a full backup). It lies outside the +# The id is fixed, because the host saw it: until 2.14.4 an operator gave the +# TLS private key on the host to this group by number - now to backupkey, +# below, which Postfix is not in - and a key given then still names it. It +# lies outside the # range the distribution hands out to system groups, 100-999 (adduser 3.152, # /etc/adduser.conf: "Default: FIRST_SYSTEM_GID=100, LAST_SYSTEM_GID=999"; # passwd 1:4.17.4-2, /etc/login.defs: SYS_GID_MIN 101, SYS_GID_MAX 999 - both @@ -127,6 +128,16 @@ RUN groupadd --system --gid 10998 selfpost \ && usermod -aG selfpost panel \ && usermod -aG selfpost postfix +# The host's TLS private key is given to this group, not to selfpost, so that +# the panel can put it into a full backup (guide § Taking a full backup) and +# no Postfix daemon can read it: postfix is in selfpost, and a memory-safety +# bug in an smtpd reached from the internet would otherwise yield the key of +# the mail and of the panel (security audit 2026-10-17, I1). Postfix itself +# serves a root-only copy (postfix-config.sh). The panel is the one member. +# Fixed for the same reason as selfpost's, next to it. +RUN groupadd --system --gid 10997 backupkey \ + && usermod -aG backupkey panel + # The panel reads the mail queue with `postqueue -p` (and the status page's # queue card). postqueue is setgid postdrop, which normally gives it the group # needed to reach Postfix's showq socket - but the documented deployment runs diff --git a/build/postfix-config.sh b/build/postfix-config.sh index 9323bb7..27c95d0 100644 --- a/build/postfix-config.sh +++ b/build/postfix-config.sh @@ -94,7 +94,7 @@ HOSTNAME_VALUE="${SELFPOST_HOSTNAME:-$(hostname -f 2>/dev/null || hostname)}" # other user; Postfix does not enforce it (smtpd loads the key as root in # pre_jail_init(), Postfix 3.10.13 src/smtpd/smtpd.c), but the mount cannot # give that mode - it is :ro and owned by the host's user, often readable by -# group 10998 for the panel's backup. So the pair is copied into a directory of +# group 10997 (the panel's alone) for the panel's backup. So the pair is copied into a directory of # Postfix's own, the key root:root 0600, before postconf and `postfix check` # (assumptions D-arch:154). TLS_CERT_SRC="${TLS_CERT_FILE:-/etc/postfix/tls/fullchain.pem}" diff --git a/deploy/traefik/extract-cert.sh b/deploy/traefik/extract-cert.sh index 84074e1..8e521d8 100755 --- a/deploy/traefik/extract-cert.sh +++ b/deploy/traefik/extract-cert.sh @@ -59,7 +59,7 @@ grep -q -- '-----BEGIN CERTIFICATE-----' "$tmp_cert" || die "the certificate of grep -q -- '-----BEGIN .*PRIVATE KEY-----' "$tmp_key" || die "the key of $DOMAIN is not PEM" chmod 0640 "$tmp_cert" "$tmp_key" -# The group the files had stays theirs: the guide gives the key to group 10998 +# The group the files had stays theirs: the guide gives the key to group 10997 # so the panel can read it for a full backup, and a run from root's cron would # otherwise hand it to root's group (review of audit stage 5). for f in fullchain privkey; do diff --git a/docs/guide.md b/docs/guide.md index 544f2f9..0d92f6f 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -336,13 +336,13 @@ ln -s /etc/letsencrypt/live/mail.example.com/privkey.pem certs/privkey.pem ``` The panel reads them as its own user (Health shows the certificate, a full -backup carries both files), through the container's group 10998. certbot's +backup carries both files), through the container's group 10997. certbot's documentation gives the way for a server that is not root, and certbot keeps the key's group and mode at every renewal: ```sh sudo chmod 0755 /etc/letsencrypt/live /etc/letsencrypt/archive -sudo chgrp 10998 /etc/letsencrypt/archive/mail.example.com/privkey*.pem +sudo chgrp 10997 /etc/letsencrypt/archive/mail.example.com/privkey*.pem sudo chmod 0640 /etc/letsencrypt/archive/mail.example.com/privkey*.pem ``` @@ -363,7 +363,7 @@ docker compose -f docker-compose.yml -f nginx/docker-compose.nginx.yml \ The fragment mounts certbot's directory into both nginx and SelfPost, at `/etc/letsencrypt`; `./certs` holds two links to `/etc/letsencrypt/live/mail.example.com/` — a path that exists inside the -container, not on the host — and the panel reads the key through group 10998, +container, not on the host — and the panel reads the key through group 10997, as in the Apache case. Then start everything: ```sh @@ -371,7 +371,7 @@ mkdir certs ln -s /etc/letsencrypt/live/mail.example.com/fullchain.pem certs/fullchain.pem ln -s /etc/letsencrypt/live/mail.example.com/privkey.pem certs/privkey.pem sudo chmod 0755 nginx/certbot-etc/live nginx/certbot-etc/archive -sudo chgrp 10998 nginx/certbot-etc/archive/mail.example.com/privkey*.pem +sudo chgrp 10997 nginx/certbot-etc/archive/mail.example.com/privkey*.pem sudo chmod 0640 nginx/certbot-etc/archive/mail.example.com/privkey*.pem docker compose -f docker-compose.yml -f nginx/docker-compose.nginx.yml up -d @@ -887,11 +887,13 @@ instance keeps running, what goes into the archive, and what happens on restore. → *Full backup*. *Encrypt with a password* is ticked; untick it for a plain archive. Same archive as the CLI below. The panel runs as its own user, not as root, so **the private key in `certs/` has to be readable by it**: give the -file to the container's `selfpost` group, which has the fixed id 10998 and holds -both the panel and Postfix — +file to the container's `backupkey` group, which has the fixed id 10997 and +holds the panel alone - not Postfix, whose daemons face the internet (until +`2.14.4` the guide said group 10998, `selfpost`, which Postfix is in; a key +given to it then is to be given to 10997 now) — ```sh -chgrp 10998 certs/privkey.pem && chmod 640 certs/privkey.pem +chgrp 10997 certs/privkey.pem && chmod 640 certs/privkey.pem ``` — and have whatever renews the certificate do the same; certbot does it by @@ -968,7 +970,7 @@ methods avoid it by stopping first. | SQLite consistency | `VACUUM INTO` snapshot | Safe when stopped | Safe when stopped | | `data/log/` | Excluded | Included (optional exclude) | Included (optional exclude) | | Postfix queue | No | Yes | Yes | -| `certs/` private key | Yes (the panel needs it readable by group 10998; links are followed) | Yes when `certs/` holds the files; links are stored as links unless `tar -h` | No — deploy files separate | +| `certs/` private key | Yes (the panel needs it readable by group 10997; links are followed) | Yes when `certs/` holds the files; links are stored as links unless `tar -h` | No — deploy files separate | | `data/manifest.json` | Written (version stamp) | Absent | Absent | | Restore version check | Yes — must match image tag | No | No | | Post-restore *Resync* | Yes, on first boot | No | No | diff --git a/docs/security-audit-2026-10-17.md b/docs/security-audit-2026-10-17.md index df5faf7..782094f 100644 --- a/docs/security-audit-2026-10-17.md +++ b/docs/security-audit-2026-10-17.md @@ -755,7 +755,7 @@ the argument. ### Stage 2 — the key group (~2.5 h) → patch release, then the test server's `certs/` -- [ ] **I1.** `groupadd --system --gid 10997 backupkey; usermod -aG backupkey +- [x] **I1.** `groupadd --system --gid 10997 backupkey; usermod -aG backupkey panel` in the Dockerfile; the guide's two `chgrp 10998` become `10997` with a sentence on why; `handlers_backup.go:398-399` likewise; the e2e stage gives the key to 10997 and `restore_test` asserts the `postfix` uid diff --git a/docs/security.md b/docs/security.md index 58b270e..435abed 100644 --- a/docs/security.md +++ b/docs/security.md @@ -266,6 +266,12 @@ operator can act on it. `mail.log`, which `postfix` writes, through it the other way ([build/Dockerfile](../build/Dockerfile), `build/entrypoint.sh`). The DKIM keys are not in it (§ Mail path). +- **Group `backupkey` (gid 10997) has one member, `panel`.** The operator + gives the host's TLS private key to it so that a full backup carries the + key; Postfix serves a root-only copy and has no way to the original. Until + `2.14.4` the guide gave the key to `selfpost`, so every Postfix daemon - + an smtpd talking to the internet included - could read it (security audit + 2026-10-17, I1; measured on the test server with `docker exec -u postfix`). - **The container keeps `KILL`** (`deploy/docker-compose.yml`, `cap_add`; every other capability not listed there is dropped). Without it root cannot signal a process of another uid (kill(2), capabilities(7); measured diff --git a/internal/web/handlers/handlers_backup.go b/internal/web/handlers/handlers_backup.go index 523a228..c6219c2 100644 --- a/internal/web/handlers/handlers_backup.go +++ b/internal/web/handlers/handlers_backup.go @@ -397,7 +397,7 @@ func decryptErrorMessage(err error) string { func unreadableBackupErr(err error, deployRoot string) string { if name, ok := backup.UnreadableCert(err, deployRoot); ok { return "The panel is not allowed to read " + name + ", and a backup without it would not bring the server back. " + - "On the host, give the file to group 10998 and make it group-readable (chgrp 10998, chmod 640), then try again. Nothing was downloaded." + "On the host, give the file to group 10997 and make it group-readable (chgrp 10997, chmod 640), then try again. Nothing was downloaded." } if errors.Is(err, backup.ErrNoCerts) { return "The project's certs/ holds no certificate, so a backup would bring the server back without one. " + diff --git a/internal/web/view/help.go b/internal/web/view/help.go index bff6c05..c049fe7 100644 --- a/internal/web/view/help.go +++ b/internal/web/view/help.go @@ -269,12 +269,12 @@ var serverTopics = []HelpTopic{ {ID: HelpBackup, Title: "Backup and moving a domain", Body: []Text{ Rich("Full backup downloads one archive of the whole instance: ", Code("data/"), " (the database, DKIM keys and application credentials), ", Code("docker-compose.yml"), ", ", Code(".env"), " and ", Code("certs/"), - ". Both it and a domain's export ask for your account password - the one you sign in with - at every download, so a session left open is not enough to take them; every attempt, right or wrong, counts against the limit of a password change, ten in 15 minutes."+ + ". Both it and a domain's export ask for your account password - the one you sign in with - at every download, so a session left open is not enough to take them; a wrong one counts against the limit of a password change, ten in 15 minutes."+ " The Postfix queue is not in it. The backup is taken while the server is running, and Postfix keeps moving messages through the queue, "+ "so a copy could hold a message twice or miss it; mail still waiting for delivery is therefore not carried over, and a restored server "+ "starts with an empty queue. To keep the queue, stop the container and archive the project directory instead. The private key in ", Code("certs/"), - " has to be readable by the panel: give it to group 10998 on the host (", Code("chgrp 10998"), ", ", Code("chmod 640"), - "). Until then the panel refuses to make a backup and names the file, rather than hand over an archive without the key. It holds secrets, so leave Encrypt with a password ticked: the file is sealed as ", Code(".spbk"), + " has to be readable by the panel: give it to group 10997 on the host (", Code("chgrp 10997"), ", ", Code("chmod 640"), + "), a group only the panel is in, so Postfix's daemons cannot read it. Until then the panel refuses to make a backup and names the file, rather than hand over an archive without the key. It holds secrets, so leave Encrypt with a password ticked: the file is sealed as ", Code(".spbk"), ", the password is not stored, and without it nothing can be recovered. To restore, unpack it into an empty project directory (decrypt a ", Code(".spbk"), " first with ", Code("selfpost-backup -decrypt"), ") and start the same SelfPost version that made it; the panel refuses data from another version. The reverse-proxy configuration and ", diff --git a/test/e2e/restore_test.go b/test/e2e/restore_test.go index 1823164..282ca21 100644 --- a/test/e2e/restore_test.go +++ b/test/e2e/restore_test.go @@ -24,7 +24,7 @@ import ( // record that was published before the backup. func testFullBackupRestore(t *testing.T, sc *scenario) { // A private key the panel may not read - the state a server is in until - // its operator gives the key to group 10998. The panel does not make an + // its operator gives the key to group 10997. The panel does not make an // archive without it: it refuses, names the file and says what to do. keyPath := filepath.Join(h.stageDir, "certs", "privkey.pem") if err := os.Chmod(keyPath, 0o600); err != nil { @@ -46,12 +46,23 @@ func testFullBackupRestore(t *testing.T, sc *scenario) { if err != nil { t.Fatalf("full backup with an unreadable key: %v", err) } - if resp.StatusCode != 500 || !strings.Contains(body, "not allowed to read certs/privkey.pem") || !strings.Contains(body, "chgrp 10998") { + if resp.StatusCode != 500 || !strings.Contains(body, "not allowed to read certs/privkey.pem") || !strings.Contains(body, "chgrp 10997") { t.Fatalf("full backup with an unreadable key: status %d, want a refusal that names the file and the way out:\n%.800s", resp.StatusCode, body) } - if err := os.Chmod(keyPath, 0o644); err != nil { + if err := stageKeyMode(keyPath); err != nil { t.Fatal(err) } + // Given to group 10997 the key is the panel's to read and no Postfix + // daemon's: postfix is in selfpost, not in backupkey (security audit + // 2026-10-17, I1). Only where the runner could give it that group. + if keyGroupStaged { + if out, err := h.compose("exec", "-T", "-u", "postfix", "selfpost", "head", "-c", "1", "/etc/postfix/tls/privkey.pem"); err == nil { + t.Errorf("the postfix uid reads the TLS key given to group %d:\n%s", keyGroup, out) + } + if out, err := h.compose("exec", "-T", "-u", "panel", "selfpost", "head", "-c", "1", "/etc/postfix/tls/privkey.pem"); err != nil { + t.Errorf("the panel cannot read the TLS key given to group %d: %v\n%s", keyGroup, err, out) + } + } // A map under data/postfix by a name the entrypoint does not list - the // DMARC maps were such, with DMARC on (review of audit stage 3, H1). The diff --git a/test/e2e/stage.go b/test/e2e/stage.go index ab93d84..98f150c 100644 --- a/test/e2e/stage.go +++ b/test/e2e/stage.go @@ -116,13 +116,32 @@ func writeSelfSignedCert(certPath, keyPath string) error { return err } keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) - // Readable by everyone, which a real key must never be. On a server the - // operator gives the key to the container's selfpost group (10998) so the - // panel can put it into a full backup; the runner of this suite is not - // always root and cannot hand a file to a group it is not in. The key is + // As the guide has the operator do: group 10997, the panel's alone, and + // 0640 - the panel can back it up and no Postfix daemon can read it + // (security audit 2026-10-17, I1). A runner that is not root cannot hand + // a file to a group it is not in; then the key is readable by everyone, + // which a real key must never be, and keyGroupStaged says so. The key is // thrown away with the stage directory. - if err := os.WriteFile(keyPath, keyPEM, 0o644); err != nil { + if err := os.WriteFile(keyPath, keyPEM, 0o600); err != nil { return err } + return stageKeyMode(keyPath) +} + +// keyGroup is the container's group for the TLS key (build/Dockerfile). +const keyGroup = 10997 + +// keyGroupStaged is whether the stage's key went to keyGroup at 0640 (the +// runner is root) rather than 0644. +var keyGroupStaged bool + +// stageKeyMode gives a key the group and mode the guide gives a real one, or +// 0644 when the runner may not. +func stageKeyMode(keyPath string) error { + if err := os.Chown(keyPath, -1, keyGroup); err == nil { + keyGroupStaged = true + return os.Chmod(keyPath, 0o640) + } + keyGroupStaged = false return os.Chmod(keyPath, 0o644) }