diff --git a/build/postfix-config.sh b/build/postfix-config.sh index dd9026d..2494bf4 100644 --- a/build/postfix-config.sh +++ b/build/postfix-config.sh @@ -285,11 +285,21 @@ if [ "${INBOUND_RELAY_ENABLE}" = "true" ] || [ "${DMARC_REPORTS_ENABLE}" = "true [ -e "$f" ] || : > "$f" done ln -sf /usr/local/bin/panel /usr/local/bin/dmarc-ingest - postconf -M "dmarc-ingest/unix-pipe=dmarc-ingest unix - n n - - pipe" - postconf -P \ - "dmarc-ingest/unix-pipe/flags=Rq" \ - "dmarc-ingest/unix-pipe/user=panel" \ - "dmarc-ingest/unix-pipe/argv=/usr/local/bin/dmarc-ingest" + # flags=, user= and argv= are arguments of pipe(8), not -o parameters: + # postconf -P refuses them ("unmatched service_name/type") and leaves a + # bare "pipe", which dies with "fatal: missing user= command-line + # attribute" on every report. They go into the service line itself. + # ${recipient} is expanded by pipe(8), not by the shell: the command + # learns which report address the message came to from its argument. + postconf -M 'dmarc-ingest/unix=dmarc-ingest unix - n n - - pipe flags=Rq user=panel argv=/usr/local/bin/dmarc-ingest ${recipient}' + # One recipient per run, so ${recipient} is one argument. + postconf -e "dmarc-ingest_destination_recipient_limit=1" + # pipe(8) starts the command with export_environment only, so what the + # command reads from the container's environment is named there. + DMARC_INGEST_ENV="DMARC_REPORTS_ENABLE=true" + [ -z "${SELFPOST_DATA_DIR:-}" ] || DMARC_INGEST_ENV="${DMARC_INGEST_ENV} SELFPOST_DATA_DIR=${SELFPOST_DATA_DIR}" + [ -z "${SELFPOST_DB_PATH:-}" ] || DMARC_INGEST_ENV="${DMARC_INGEST_ENV} SELFPOST_DB_PATH=${SELFPOST_DB_PATH}" + postconf -e "export_environment=TZ MAIL_CONFIG LANG ${DMARC_INGEST_ENV}" if [ -n "$RELAY_DOMAINS_SETTING" ]; then RELAY_DOMAINS_SETTING="${RELAY_DOMAINS_SETTING} texthash:${DMARC_RELAY_DOMAINS_MAP}" TRANSPORT_SETTING="${TRANSPORT_SETTING} texthash:${DMARC_TRANSPORT_MAP}" @@ -300,6 +310,9 @@ if [ "${INBOUND_RELAY_ENABLE}" = "true" ] || [ "${DMARC_REPORTS_ENABLE}" = "true # Comma, not a space, between check_recipient_access and its table: see # master_cf_set_smtp_inet_o. RECIPIENT_RESTRICTIONS="check_recipient_access,texthash:${DMARC_RECIPIENTS_MAP},${RECIPIENT_RESTRICTIONS}" + else + postconf -MX "dmarc-ingest/unix" 2>/dev/null || true + postconf -X dmarc-ingest_destination_recipient_limit export_environment 2>/dev/null || true fi postconf -e \ "relay_domains=${RELAY_DOMAINS_SETTING}" \ @@ -381,7 +394,8 @@ if [ "${INBOUND_RELAY_ENABLE}" = "true" ] || [ "${DMARC_REPORTS_ENABLE}" = "true master_cf_set_smtp_inet_o smtpd_milters "${INBOUND_MILTERS}" else postconf -MX "smtp/inet" 2>/dev/null || true - postconf -MX "dmarc-ingest/unix-pipe" 2>/dev/null || true + postconf -MX "dmarc-ingest/unix" 2>/dev/null || true + postconf -X dmarc-ingest_destination_recipient_limit export_environment 2>/dev/null || true postconf -e \ "relay_domains=" \ "transport_maps=" \ diff --git a/cmd/panel/dmarc_ingest.go b/cmd/panel/dmarc_ingest.go index 689969f..9d7699b 100644 --- a/cmd/panel/dmarc_ingest.go +++ b/cmd/panel/dmarc_ingest.go @@ -26,10 +26,7 @@ func runDMARCIngestMode() error { } defer st.Close() - recipient := strings.ToLower(strings.TrimSpace(os.Getenv("RECIPIENT"))) - if recipient == "" { - recipient = strings.ToLower(strings.TrimSpace(os.Getenv("ORIGINAL_RECIPIENT"))) - } + recipient := ingestRecipient(os.Args[1:]) if err := dmarc.IngestMessage(st, io.LimitReader(os.Stdin, maxIngestStdin), recipient, time.Now().UTC()); err != nil { if incrErr := st.IncrDMARCParseFailures(); incrErr != nil { log.Printf("dmarc ingest: record failure: %v", incrErr) @@ -39,6 +36,21 @@ func runDMARCIngestMode() error { return nil } +// ingestRecipient is the report address the message was delivered to. Postfix's +// pipe(8) hands it over as an argument (${recipient} in the service's argv, +// build/postfix-config.sh) - it sets no RECIPIENT variable, that is local(8). +// Without it the check that a report came to its own domain's address is +// skipped. +func ingestRecipient(args []string) string { + for _, a := range args { + if a == "-dmarc-ingest" { + continue + } + return strings.ToLower(strings.TrimSpace(a)) + } + return "" +} + func isDMARCIngestInvocation() bool { if len(os.Args) > 1 && os.Args[1] == "-dmarc-ingest" { return true diff --git a/cmd/panel/dmarc_ingest_test.go b/cmd/panel/dmarc_ingest_test.go new file mode 100644 index 0000000..12e9ff5 --- /dev/null +++ b/cmd/panel/dmarc_ingest_test.go @@ -0,0 +1,20 @@ +package main + +import "testing" + +func TestIngestRecipient(t *testing.T) { + for _, tc := range []struct { + args []string + want string + }{ + // As pipe(8) starts it: the symlink name, then ${recipient}. + {[]string{"dmarc-reports+Example.org@mail.example.org"}, "dmarc-reports+example.org@mail.example.org"}, + {[]string{"-dmarc-ingest", "dmarc-reports+example.org@mail.example.org"}, "dmarc-reports+example.org@mail.example.org"}, + {[]string{"-dmarc-ingest"}, ""}, + {nil, ""}, + } { + if got := ingestRecipient(tc.args); got != tc.want { + t.Errorf("ingestRecipient(%q) = %q, want %q", tc.args, got, tc.want) + } + } +} diff --git a/test/e2e/port25_config_check.go b/test/e2e/port25_config_check.go index 12f2954..a4e063c 100644 --- a/test/e2e/port25_config_check.go +++ b/test/e2e/port25_config_check.go @@ -25,8 +25,17 @@ type port25Case struct { // main are main.cf parameters the combination must end up with, read with // postconf -h. main []optionCheck + // ingest is set when the combination must carry the DMARC report delivery + // service, with everything the command it runs depends on. + ingest bool } +// ingestService is the dmarc-ingest service as Postfix must read it back. +// flags=, user= and argv= are arguments of pipe(8); set as -o parameters they +// are dropped, and a bare "pipe" dies on every report with "fatal: missing +// user= command-line attribute" - the state of 1.7.0 through 1.9.5. +const ingestService = "dmarc-ingest unix - n n - - pipe flags=Rq user=panel argv=/usr/local/bin/dmarc-ingest ${recipient}" + var port25Cases = []port25Case{ { name: "inbound relay only", @@ -53,6 +62,7 @@ var port25Cases = []port25Case{ {param: "smtpd_recipient_restrictions", contains: "check_recipient_access,texthash:/data/postfix/dmarc_recipients,reject", wantExact: true}, {param: "smtpd_milters", contains: "", wantExact: true}, }, + ingest: true, }, { name: "inbound relay and dmarc ingest", @@ -71,6 +81,7 @@ var port25Cases = []port25Case{ main: []optionCheck{ {param: "relay_recipient_maps", contains: "texthash:/data/postfix/relay_recipients texthash:/data/postfix/dmarc_recipients", wantExact: true}, }, + ingest: true, }, { name: "inbound relay with antispam milter", @@ -148,6 +159,9 @@ func checkPort25ConfigCombinations(s *stack) error { return fmt.Errorf("%s: %s = %q, want %q", tc.name, c.param, got, c.contains) } } + if err := checkIngestService(s, tc); err != nil { + return err + } } // Back to the stand's own configuration: flags off, no port-25 listener. @@ -157,6 +171,39 @@ func checkPort25ConfigCombinations(s *stack) error { return checkInboundRelayOff(s) } +// checkIngestService asks Postfix for the dmarc-ingest service and the two +// main.cf parameters its command depends on: one recipient per run, so +// ${recipient} is one argument, and DMARC_REPORTS_ENABLE in the environment +// pipe(8) gives the command - it passes export_environment and nothing else. +// A combination without DMARC ingest must have none of the three left over. +func checkIngestService(s *stack, tc port25Case) error { + service, _ := s.execIn("selfpost", "postconf", "-M", "dmarc-ingest/unix") + service = strings.Join(strings.Fields(service), " ") + limit, _ := s.execIn("selfpost", "postconf", "-h", "dmarc-ingest_destination_recipient_limit") + limit = strings.TrimSpace(limit) + environ, err := s.execIn("selfpost", "postconf", "-h", "export_environment") + if err != nil { + return fmt.Errorf("%s: postconf -h export_environment: %v\n%s", tc.name, err, environ) + } + exported := strings.Contains(" "+strings.TrimSpace(environ)+" ", " DMARC_REPORTS_ENABLE=true ") + if !tc.ingest { + if strings.Contains(service, "pipe") || limit == "1" || exported { + return fmt.Errorf("%s: DMARC ingest is off but left behind: service %q, recipient limit %q, export_environment %q", tc.name, service, limit, environ) + } + return nil + } + if service != ingestService { + return fmt.Errorf("%s: Postfix reads the dmarc-ingest service as %q, want %q", tc.name, service, ingestService) + } + if limit != "1" { + return fmt.Errorf("%s: dmarc-ingest_destination_recipient_limit = %q, want 1", tc.name, limit) + } + if !exported { + return fmt.Errorf("%s: export_environment = %q, want DMARC_REPORTS_ENABLE=true in it", tc.name, environ) + } + return nil +} + // runPostfixConfig re-runs the config generator with env overrides. The script // ends with `postfix check`, so a non-zero exit is exactly the start-time // failure an operator would hit.