fix: e2e port-25 check misparsed wrapped master.cf continuation lines
test / test (push) Has been cancelled
test / test (push) Has been cancelled
Found running the new checkPort25ConfigCombinations on selfpost.mixfed.ru: it compared against raw, unnormalized master.cf text. postconf -F "*/*/chroot=n" (unconditional, runs after our smtp/inet edits) rewraps the whole file using master.cf's own line-continuation convention, splitting a single -o value across lines and re-flowing what were separate -o lines into one paragraph. Valid Postfix syntax -- postfix check passes -- but plain substring matching against the raw text broke on it. parseSMTPInetOptions now joins continuation lines into one token stream and reconstructs each "-o param=value" assignment before comparing, verified against a real captured stanza in port25_config_check_test.go. Full make e2e (13 scenarios plus the two new parser unit tests) is green on the server. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2,16 +2,26 @@ package e2e
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"regexp"
|
||||||
"strings"
|
"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
|
// 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 {
|
type port25Case struct {
|
||||||
name string
|
name string
|
||||||
env []string
|
env []string
|
||||||
want []string
|
check []optionCheck
|
||||||
deny []string
|
|
||||||
}
|
}
|
||||||
|
|
||||||
var port25Cases = []port25Case{
|
var port25Cases = []port25Case{
|
||||||
@@ -21,13 +31,13 @@ var port25Cases = []port25Case{
|
|||||||
"INBOUND_RELAY_ENABLE=true",
|
"INBOUND_RELAY_ENABLE=true",
|
||||||
"DMARC_REPORTS_ENABLE=false",
|
"DMARC_REPORTS_ENABLE=false",
|
||||||
},
|
},
|
||||||
want: []string{
|
check: []optionCheck{
|
||||||
"-o smtpd_sasl_auth_enable=no",
|
{param: "smtpd_sasl_auth_enable", contains: "no", wantExact: true},
|
||||||
"-o smtpd_relay_restrictions=reject_unauth_destination",
|
{param: "smtpd_relay_restrictions", contains: "reject_unauth_destination", wantExact: true},
|
||||||
"-o smtpd_recipient_restrictions=reject_unauth_destination,reject_unlisted_recipient",
|
{param: "smtpd_recipient_restrictions", contains: "reject_unauth_destination,reject_unlisted_recipient", wantExact: true},
|
||||||
// No antispam milter configured: the submission chain (strict
|
// No antispam milter configured: the submission chain (strict
|
||||||
// OpenDKIM + journal-milter) must not reach inbound mail.
|
// 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",
|
"INBOUND_RELAY_ENABLE=false",
|
||||||
"DMARC_REPORTS_ENABLE=true",
|
"DMARC_REPORTS_ENABLE=true",
|
||||||
},
|
},
|
||||||
want: []string{
|
check: []optionCheck{
|
||||||
"-o smtpd_recipient_restrictions=check_recipient_access texthash:/data/postfix/dmarc_recipients,reject",
|
{param: "smtpd_recipient_restrictions", contains: "check_recipient_access texthash:/data/postfix/dmarc_recipients,reject", wantExact: true},
|
||||||
"-o smtpd_milters=",
|
{param: "smtpd_milters", contains: "", wantExact: true},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -47,9 +57,9 @@ var port25Cases = []port25Case{
|
|||||||
"INBOUND_RELAY_ENABLE=true",
|
"INBOUND_RELAY_ENABLE=true",
|
||||||
"DMARC_REPORTS_ENABLE=true",
|
"DMARC_REPORTS_ENABLE=true",
|
||||||
},
|
},
|
||||||
want: []string{
|
check: []optionCheck{
|
||||||
"-o smtpd_recipient_restrictions=check_recipient_access texthash:/data/postfix/dmarc_recipients,reject_unauth_destination,reject_unlisted_recipient",
|
{param: "smtpd_recipient_restrictions", contains: "check_recipient_access texthash:/data/postfix/dmarc_recipients,reject_unauth_destination,reject_unlisted_recipient", wantExact: true},
|
||||||
"-o smtpd_milters=",
|
{param: "smtpd_milters", contains: "", wantExact: true},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -60,10 +70,11 @@ var port25Cases = []port25Case{
|
|||||||
"INBOUND_ANTISPAM_MILTER=inet:antispam:11332",
|
"INBOUND_ANTISPAM_MILTER=inet:antispam:11332",
|
||||||
"INBOUND_ANTISPAM_MILTER_ACTION=accept",
|
"INBOUND_ANTISPAM_MILTER_ACTION=accept",
|
||||||
},
|
},
|
||||||
want: []string{
|
check: []optionCheck{
|
||||||
"-o smtpd_milters={inet:antispam:11332,default_action=accept}",
|
// 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 == "" {
|
if stanza == "" {
|
||||||
return fmt.Errorf("%s: no smtp/inet service in master.cf:\n%s", tc.name, master)
|
return fmt.Errorf("%s: no smtp/inet service in master.cf:\n%s", tc.name, master)
|
||||||
}
|
}
|
||||||
for _, want := range tc.want {
|
opts := parseSMTPInetOptions(stanza)
|
||||||
if !strings.Contains(stanza, want) {
|
for _, c := range tc.check {
|
||||||
return fmt.Errorf("%s: smtp/inet is missing %q:\n%s", tc.name, want, stanza)
|
got, ok := opts[c.param]
|
||||||
}
|
switch {
|
||||||
}
|
case !ok:
|
||||||
for _, deny := range tc.deny {
|
return fmt.Errorf("%s: smtp/inet has no -o %s (options: %+v)", tc.name, c.param, opts)
|
||||||
if strings.Contains(stanza, deny) {
|
case c.wantExact && got != c.contains:
|
||||||
return fmt.Errorf("%s: smtp/inet unexpectedly carries %q:\n%s", tc.name, deny, stanza)
|
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
|
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 {
|
func smtpInetStanza(master string) string {
|
||||||
var b strings.Builder
|
var b strings.Builder
|
||||||
in := false
|
in := false
|
||||||
for _, line := range strings.Split(master, "\n") {
|
for _, line := range strings.Split(master, "\n") {
|
||||||
switch {
|
switch {
|
||||||
case strings.HasPrefix(line, "smtp ") || strings.HasPrefix(line, "smtp\t"):
|
|
||||||
in = true
|
|
||||||
case in && (strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t")):
|
case in && (strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t")):
|
||||||
case in:
|
case in:
|
||||||
return b.String()
|
return b.String()
|
||||||
|
case isSMTPInetServiceLine(line):
|
||||||
|
in = true
|
||||||
default:
|
default:
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -145,3 +163,53 @@ func smtpInetStanza(master string) string {
|
|||||||
}
|
}
|
||||||
return b.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
|
||||||
|
}
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user