diff --git a/test/e2e/port25_config_check.go b/test/e2e/port25_config_check.go index 7a2358c..44b82d8 100644 --- a/test/e2e/port25_config_check.go +++ b/test/e2e/port25_config_check.go @@ -2,16 +2,26 @@ package e2e import ( "fmt" + "regexp" "strings" ) +// optionCheck asserts one smtpd `-o` option on the generated smtp/inet +// stanza. When want is non-empty the option's value must contain it; when +// wantExact is set the value must equal it exactly (used for options — like +// smtpd_milters — where an unexpectedly non-empty value is itself the bug). +type optionCheck struct { + param string + contains string + wantExact bool +} + // port25Case is one combination of the optional port-25 flags, with the -// master.cf `-o` lines its generated smtp/inet stanza must carry. +// options its generated smtp/inet stanza must carry. type port25Case struct { - name string - env []string - want []string - deny []string + name string + env []string + check []optionCheck } var port25Cases = []port25Case{ @@ -21,13 +31,13 @@ var port25Cases = []port25Case{ "INBOUND_RELAY_ENABLE=true", "DMARC_REPORTS_ENABLE=false", }, - want: []string{ - "-o smtpd_sasl_auth_enable=no", - "-o smtpd_relay_restrictions=reject_unauth_destination", - "-o smtpd_recipient_restrictions=reject_unauth_destination,reject_unlisted_recipient", + check: []optionCheck{ + {param: "smtpd_sasl_auth_enable", contains: "no", wantExact: true}, + {param: "smtpd_relay_restrictions", contains: "reject_unauth_destination", wantExact: true}, + {param: "smtpd_recipient_restrictions", contains: "reject_unauth_destination,reject_unlisted_recipient", wantExact: true}, // No antispam milter configured: the submission chain (strict // OpenDKIM + journal-milter) must not reach inbound mail. - "-o smtpd_milters=", + {param: "smtpd_milters", contains: "", wantExact: true}, }, }, { @@ -36,9 +46,9 @@ var port25Cases = []port25Case{ "INBOUND_RELAY_ENABLE=false", "DMARC_REPORTS_ENABLE=true", }, - want: []string{ - "-o smtpd_recipient_restrictions=check_recipient_access texthash:/data/postfix/dmarc_recipients,reject", - "-o smtpd_milters=", + check: []optionCheck{ + {param: "smtpd_recipient_restrictions", contains: "check_recipient_access texthash:/data/postfix/dmarc_recipients,reject", wantExact: true}, + {param: "smtpd_milters", contains: "", wantExact: true}, }, }, { @@ -47,9 +57,9 @@ var port25Cases = []port25Case{ "INBOUND_RELAY_ENABLE=true", "DMARC_REPORTS_ENABLE=true", }, - want: []string{ - "-o smtpd_recipient_restrictions=check_recipient_access texthash:/data/postfix/dmarc_recipients,reject_unauth_destination,reject_unlisted_recipient", - "-o smtpd_milters=", + check: []optionCheck{ + {param: "smtpd_recipient_restrictions", contains: "check_recipient_access texthash:/data/postfix/dmarc_recipients,reject_unauth_destination,reject_unlisted_recipient", wantExact: true}, + {param: "smtpd_milters", contains: "", wantExact: true}, }, }, { @@ -60,10 +70,11 @@ var port25Cases = []port25Case{ "INBOUND_ANTISPAM_MILTER=inet:antispam:11332", "INBOUND_ANTISPAM_MILTER_ACTION=accept", }, - want: []string{ - "-o smtpd_milters={inet:antispam:11332,default_action=accept}", + check: []optionCheck{ + // Exact equality also covers the absence of OpenDKIM/journal-milter + // sockets — anything else there would fail the comparison. + {param: "smtpd_milters", contains: "{inet:antispam:11332,default_action=accept}", wantExact: true}, }, - deny: []string{"opendkim.sock", "journal.sock"}, }, } @@ -94,14 +105,16 @@ func checkPort25ConfigCombinations(s *stack) error { if stanza == "" { return fmt.Errorf("%s: no smtp/inet service in master.cf:\n%s", tc.name, master) } - for _, want := range tc.want { - if !strings.Contains(stanza, want) { - return fmt.Errorf("%s: smtp/inet is missing %q:\n%s", tc.name, want, stanza) - } - } - for _, deny := range tc.deny { - if strings.Contains(stanza, deny) { - return fmt.Errorf("%s: smtp/inet unexpectedly carries %q:\n%s", tc.name, deny, stanza) + opts := parseSMTPInetOptions(stanza) + for _, c := range tc.check { + got, ok := opts[c.param] + switch { + case !ok: + return fmt.Errorf("%s: smtp/inet has no -o %s (options: %+v)", tc.name, c.param, opts) + case c.wantExact && got != c.contains: + return fmt.Errorf("%s: -o %s = %q, want exactly %q", tc.name, c.param, got, c.contains) + case !c.wantExact && !strings.Contains(got, c.contains): + return fmt.Errorf("%s: -o %s = %q, want it to contain %q", tc.name, c.param, got, c.contains) } } } @@ -126,17 +139,22 @@ func runPostfixConfig(s *stack, env []string) error { return nil } -// smtpInetStanza returns the smtp/inet service line and its -o options. +// smtpInetStanza returns the smtp/inet service line and its -o options. The +// match requires the second whitespace-separated field to be "inet" — the +// same test master_cf_set_smtp_inet_o uses (build/postfix-config.sh) — +// because master.cf also has a "smtp unix ... smtp" delivery-agent service +// (the outbound SMTP client) whose line starts with the same "smtp" prefix +// and would otherwise be matched instead. func smtpInetStanza(master string) string { var b strings.Builder in := false for _, line := range strings.Split(master, "\n") { switch { - case strings.HasPrefix(line, "smtp ") || strings.HasPrefix(line, "smtp\t"): - in = true case in && (strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t")): case in: return b.String() + case isSMTPInetServiceLine(line): + in = true default: continue } @@ -145,3 +163,53 @@ func smtpInetStanza(master string) string { } return b.String() } + +// isSMTPInetServiceLine reports whether line is master.cf's "smtp inet ..." +// service definition (as opposed to "smtp unix ..." or anything else). +func isSMTPInetServiceLine(line string) bool { + fields := strings.Fields(line) + return len(fields) >= 2 && fields[0] == "smtp" && fields[1] == "inet" +} + +// optionBoundary matches the start of one "-o param=" assignment in the +// option stream (see parseSMTPInetOptions). +var optionBoundary = regexp.MustCompile(`-o\s+([A-Za-z0-9_]+)=`) + +// parseSMTPInetOptions reconstructs each logical "-o param=value" assignment +// from a stanza, joining Postfix's own master.cf line-continuations first. +// +// postconf wraps long master.cf lines at a fixed column using master.cf's +// whitespace-continuation convention — any line-writing postconf call (here, +// the unconditional `postconf -F "*/*/chroot=n"` later in +// build/postfix-config.sh) rewraps the *whole* file, including options this +// script wrote as one raw line via master_cf_set_smtp_inet_o. The wrap does +// not respect option boundaries: it can split an option's own value (e.g. +// smtpd_recipient_restrictions's two space-separated tokens) across lines, +// and it re-flows several originally separate "-o " lines into one paragraph. +// This is valid, functional master.cf syntax (`postfix check` accepts it) — +// so the fix is on the reading side: join every stanza line after the header +// into one continuous token stream before splitting it back into options. +func parseSMTPInetOptions(stanza string) map[string]string { + lines := strings.Split(strings.TrimRight(stanza, "\n"), "\n") + var stream strings.Builder + for _, line := range lines[1:] { // skip the "smtp inet ..." header + if stream.Len() > 0 { + stream.WriteByte(' ') + } + stream.WriteString(strings.TrimSpace(line)) + } + text := stream.String() + + opts := make(map[string]string) + matches := optionBoundary.FindAllStringSubmatchIndex(text, -1) + for i, m := range matches { + param := text[m[2]:m[3]] + valueStart := m[1] + valueEnd := len(text) + if i+1 < len(matches) { + valueEnd = matches[i+1][0] + } + opts[param] = strings.TrimSpace(text[valueStart:valueEnd]) + } + return opts +} diff --git a/test/e2e/port25_config_check_test.go b/test/e2e/port25_config_check_test.go new file mode 100644 index 0000000..256f982 --- /dev/null +++ b/test/e2e/port25_config_check_test.go @@ -0,0 +1,62 @@ +package e2e + +import "testing" + +// realMasterCfSample is captured verbatim (via `cat -A`) from a container +// generated with INBOUND_RELAY_ENABLE=false DMARC_REPORTS_ENABLE=true: after +// the unconditional `postconf -F "*/*/chroot=n"` rewraps the file, the +// smtp/inet stanza's options are spread across several continuation lines +// that split values mid-token and re-flow what were originally separate +// "-o " lines into one paragraph. Confirms parseSMTPInetOptions reconstructs +// the logical options from that rather than a plain single-option-per-line +// master.cf. +const realMasterCfSample = `smtp inet n - n - - smtpd + -o smtpd_milters= + -o smtpd_recipient_restrictions=check_recipient_access + texthash:/data/postfix/dmarc_recipients,reject -o smtpd_sasl_auth_enable=no + -o smtpd_tls_auth_only=no -o smtpd_sender_login_maps= -o + smtpd_sender_restrictions= -o smtpd_client_restrictions= -o + smtpd_relay_restrictions=reject_unauth_destination -o + smtpd_client_message_rate_limit=20 -o message_size_limit=5242880 +pickup unix n - n 60 1 pickup +` + +func TestParseSMTPInetOptionsAcrossContinuationLines(t *testing.T) { + stanza := smtpInetStanza(realMasterCfSample) + opts := parseSMTPInetOptions(stanza) + + want := map[string]string{ + "smtpd_milters": "", + "smtpd_recipient_restrictions": "check_recipient_access texthash:/data/postfix/dmarc_recipients,reject", + "smtpd_sasl_auth_enable": "no", + "smtpd_tls_auth_only": "no", + "smtpd_sender_login_maps": "", + "smtpd_sender_restrictions": "", + "smtpd_client_restrictions": "", + "smtpd_relay_restrictions": "reject_unauth_destination", + "smtpd_client_message_rate_limit": "20", + "message_size_limit": "5242880", + } + for param, wantVal := range want { + if got, ok := opts[param]; !ok || got != wantVal { + t.Errorf("opts[%q] = %q (present=%v), want %q", param, got, ok, wantVal) + } + } + if len(opts) != len(want) { + t.Errorf("parsed %d options, want %d: %+v", len(opts), len(want), opts) + } +} + +func TestSMTPInetStanzaSkipsDeliveryAgent(t *testing.T) { + master := "smtp unix - - n - - smtp\n" + + "# -o smtp_helo_timeout=5\n" + + realMasterCfSample + stanza := smtpInetStanza(master) + if stanza == "" { + t.Fatal("no smtp/inet stanza found") + } + opts := parseSMTPInetOptions(stanza) + if v, ok := opts["smtpd_milters"]; !ok || v != "" { + t.Fatalf("picked up the wrong \"smtp\" service: opts = %+v", opts) + } +}