audit 5: findings of the two reviews

- The e2e's numeric-settings test ran a container that passed every gate
  and never ended: the good values are checked beside a bad one after
  them; each run is named, bounded to a minute and removed.
- The gates refuse a value under the parameter's minimum (a zero window
  kills anvil, a zero milter timeout smtpd) and one past an int, in
  seconds for a time value (src/global/conv_time.c, src/anvil/anvil.c).
- OpenDKIM waits, five seconds at most, for the relay's socket.
- The relay marks its lines `syslog: `, writes control characters out,
  reads into a queue at once and drops past 200 lines a second, saying
  how many (security F1, F2; a stuck relay would hold OpenDKIM in
  syslog(3)).
- smtp_tls_mandatory_protocols >=TLSv1.2 too.
- extract-cert.sh keeps the old files' group and sets its trap before
  mktemp.
- crashexit spares cert-reload; architecture.md "four times running" and
  what else exits the container; security.md the restart after five panel
  exits and the /dev/log socket.
- Measured against the technical review: smtpd has the selfpost group
  (Groups 104 998), so it did reach the supervisor socket before this
  stage.
Accepted, reasons in the plan: the logrotate state file in the postfix
user's directory, envdoc_test's reading of `${VAR:-}` only, crashexit's
window pruning untested.

Tested on the test server: go vet + go test of 27 packages, make e2e (all
pass, 592 s) on this tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-15 13:53:14 +03:00
co-authored by Claude Opus 5.5
parent dab7715e80
commit a39eda5a0b
19 changed files with 766 additions and 255 deletions
+45 -7
View File
@@ -1,6 +1,8 @@
package e2e
import (
"context"
"fmt"
"os"
"os/exec"
"strings"
@@ -13,7 +15,7 @@ import (
// build/entrypoint.sh runs and fails before anything else needs them) with
// hostnameEnv substituted for SELFPOST_HOSTNAME, and returns its combined
// output and whether it exited zero.
func runEntrypoint(t *testing.T, hostnameEnv string) (output string, exitedZero bool) {
func runEntrypoint(t *testing.T, hostnameEnv string, env ...string) (output string, exitedZero bool) {
t.Helper()
dataDir := t.TempDir()
// Entrypoint also chmod 755 /data; mirror that here so a pre-fix image
@@ -23,13 +25,19 @@ func runEntrypoint(t *testing.T, hostnameEnv string) (output string, exitedZero
t.Fatalf("chmod data dir: %v", err)
}
args := []string{
"run", "--rm",
"-e", "SELFPOST_HOSTNAME=" + hostnameEnv,
"-v", dataDir + ":/data",
"selfpost:e2e",
// A name of its own, so a run that outlives its minute is removed.
name := fmt.Sprintf("selfpost-gate-%d", time.Now().UnixNano())
defer exec.Command("docker", "rm", "-f", name).Run()
args := []string{"run", "--rm", "--name", name, "-e", "SELFPOST_HOSTNAME=" + hostnameEnv}
for _, e := range env {
args = append(args, "-e", e)
}
cmd := exec.Command("docker", args...)
args = append(args, "-v", dataDir+":/data", "selfpost:e2e")
// A start that gets past every gate runs on: it is ended after a minute
// rather than holding the run until the test binary's own timeout.
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
cmd := exec.CommandContext(ctx, "docker", args...)
cmd.Env = os.Environ()
out, err := cmd.CombinedOutput()
return string(out), err == nil
@@ -114,3 +122,33 @@ func runEntrypointBackground(t *testing.T, hostnameEnv string) (output string, s
logs, _ := exec.Command("docker", "logs", name).CombinedOutput()
return string(logs), err == nil
}
// A setting Postfix takes as a number or a time value stops the start when it
// is neither, naming the variable: before, it passed `postfix check` and every
// smtpd died at its first connection (audit 2026-10-12, L31). A sensible
// value of the same form gets past the check (and the start fails later, for
// what this bare run lacks).
func TestNumericSettingsGate(t *testing.T) {
for _, c := range []struct{ env, want string }{
{"RATE_LIMIT_MESSAGES_PER_IP=100/h", "RATE_LIMIT_MESSAGES_PER_IP must be a whole number, got '100/h'"},
{"INBOUND_MESSAGE_SIZE_LIMIT=10MB", "INBOUND_MESSAGE_SIZE_LIMIT must be a whole number, got '10MB'"},
{"MILTER_CONTENT_TIMEOUT=thirty", "MILTER_CONTENT_TIMEOUT must be a time value"},
// Under the parameter's minimum: anvil and smtpd die on a zero.
{"RATE_LIMIT_WINDOW_SECONDS=0", "RATE_LIMIT_WINDOW_SECONDS must be from 1 to 2147483647, got '0'"},
{"MILTER_COMMAND_TIMEOUT=0s", "MILTER_COMMAND_TIMEOUT must be a time value"},
{"INBOUND_MESSAGE_SIZE_LIMIT=99999999999", "INBOUND_MESSAGE_SIZE_LIMIT must be from 0 to 2147483647"},
} {
t.Run(c.env, func(t *testing.T) {
out, ok := runEntrypoint(t, "mail.example.com", c.env)
if ok || !strings.Contains(out, c.want) {
t.Fatalf("%s: exited zero %v, want the refusal %q\noutput:\n%s", c.env, ok, c.want, out)
}
})
}
// Good values beside a bad one checked after them: the refusal names the
// bad one alone. (A start with good values only would run on.)
out, _ := runEntrypoint(t, "mail.example.com", "RATE_LIMIT_MESSAGES_PER_IP=50", "MILTER_CONNECT_TIMEOUT=2m", "MILTER_CONTENT_TIMEOUT=bad")
if !strings.Contains(out, "MILTER_CONTENT_TIMEOUT must be") || strings.Contains(out, "RATE_LIMIT_MESSAGES_PER_IP must") || strings.Contains(out, "MILTER_CONNECT_TIMEOUT must") {
t.Errorf("valid values refused:\n%s", out)
}
}
+9 -1
View File
@@ -7,6 +7,7 @@ import (
"path/filepath"
"strings"
"testing"
"time"
)
// checkLogrotateConfigMode verifies the image pins /etc/logrotate.d/mail at 0644
@@ -21,7 +22,14 @@ func checkLogrotateConfigMode(s *stack) error {
if mode != "644" {
return fmt.Errorf("/etc/logrotate.d/mail mode is %q, want 644", mode)
}
return nil
// The state file is beside the log, under /data, so a recreated container
// keeps when mail.log was last rotated (audit 2026-10-12, M19); the loop
// runs logrotate at once when it starts.
return waitFor("logrotate's state under /data/log", 20*time.Second, 500*time.Millisecond, func() (bool, error) {
out, _ := s.execIn("selfpost", "sh", "-c", "test -s /data/log/.logrotate.status && echo yes")
out = strings.TrimSpace(out)
return out == "yes", fmt.Errorf("state file: %q", out)
})
}
// checkLogrotateRotation forces a rotation and checks the recreated mail.log is
+3
View File
@@ -116,6 +116,9 @@ func TestE2E(t *testing.T) {
if err := checkOpenDKIMLogs(h); err != nil {
t.Fatal(err)
}
if err := checkPostfixHardening(h); err != nil {
t.Fatal(err)
}
if err := checkInboundRelayOff(h); err != nil {
t.Fatal(err)
}
+15
View File
@@ -141,3 +141,18 @@ func checkOpenDKIMLogs(s *stack) error {
return strings.Contains(out, "opendkim[") && strings.Contains(out, "OpenDKIM Filter v"), fmt.Errorf("no OpenDKIM line in the log")
})
}
// checkPostfixHardening reads back what postfix-config.sh sets against
// downgrade and enumeration (audit 2026-10-12, L31).
func checkPostfixHardening(s *stack) error {
out, err := s.execIn("selfpost", "postconf", "-h", "smtpd_tls_protocols", "smtpd_tls_mandatory_protocols", "disable_vrfy_command", "smtpd_banner")
if err != nil {
return fmt.Errorf("postconf: %v\n%s", err, out)
}
got := strings.Split(strings.TrimSpace(out), "\n")
want := []string{">=TLSv1.2", ">=TLSv1.2", "yes", "$myhostname ESMTP"}
if strings.Join(got, "|") != strings.Join(want, "|") {
return fmt.Errorf("postconf -h of the hardened settings = %q, want %q", got, want)
}
return nil
}