improvements to outgoing dmarc reports and displaying evaluations

- more eagerly report about overrides, so domain owners can better tell that
  switching from p=none to p=reject will not cause trouble for these messages.
- report multiple reasons, e.g. mailing list and sampled out
- in dmarc analysis for rejects from first-time senders (possibly spammers),
  fix the conditional check on nonjunk messages.
- in evaluations view in admin, show unaligned spf pass in yellow too and a few
  more small tweaks.

smtpserver/server.go

@@ -2411,24 +2411,26 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
 
 		// Any DMARC result override is stored in the evaluation for outgoing DMARC
 		// aggregate reports, and added to the Authentication-Results message header.
-		var dmarcOverride string
-		if dmarcResult.Record != nil {
-			if !dmarcUse {
-				dmarcOverride = string(dmarcrpt.PolicyOverrideSampledOut)
-			} else if a.dmarcOverrideReason != "" && (a.accept && !m.IsReject) == dmarcResult.Reject {
-				dmarcOverride = a.dmarcOverrideReason
+		// We want to tell the sender that we have an override, e.g. for mailing lists, so
+		// they don't overestimate the potential damage of switching from p=none to
+		// p=reject.
+		var dmarcOverrides []string
+		if a.dmarcOverrideReason != "" {
+			dmarcOverrides = []string{a.dmarcOverrideReason}
 		}
+		if dmarcResult.Record != nil && !dmarcUse {
+			dmarcOverrides = append(dmarcOverrides, string(dmarcrpt.PolicyOverrideSampledOut))
 		}
 
 		// Add per-recipient DMARC method to Authentication-Results. Each account can have
 		// their own override rules, e.g. based on configured mailing lists/forwards.
 		// ../rfc/7489:1486
 		rcptDMARCMethod := dmarcMethod
-		if dmarcOverride != "" {
+		if len(dmarcOverrides) > 0 {
 			if rcptDMARCMethod.Comment != "" {
 				rcptDMARCMethod.Comment += ", "
 			}
-			rcptDMARCMethod.Comment += "override " + dmarcOverride
+			rcptDMARCMethod.Comment += "override " + strings.Join(dmarcOverrides, ",")
 		}
 		rcptAuthResults := authResults
 		rcptAuthResults.Methods = append([]message.AuthMethod{}, authResults.Methods...)
@@ -2477,7 +2479,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
 					// See if we received a non-junk message from this organizational domain.
 					q := bstore.QueryTx[store.Message](tx)
 					q.FilterNonzero(store.Message{MsgFromOrgDomain: m.MsgFromOrgDomain})
-					q.FilterEqual("Notjunk", false)
+					q.FilterEqual("Notjunk", true)
 					exists, err := q.Exists()
 					if err != nil {
 						return fmt.Errorf("querying for non-junk message from organizational domain: %v", err)
@@ -2544,10 +2546,9 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
 				HeaderFrom:      msgFrom.Domain.Name(),
 			}
 
-			if dmarcOverride != "" {
-				eval.OverrideReasons = []dmarcrpt.PolicyOverrideReason{
-					{Type: dmarcrpt.PolicyOverride(dmarcOverride)},
-				}
+			for _, s := range dmarcOverrides {
+				reason := dmarcrpt.PolicyOverrideReason{Type: dmarcrpt.PolicyOverride(s)}
+				eval.OverrideReasons = append(eval.OverrideReasons, reason)
 			}
 
 			// We'll include all signatures for the organizational domain, even if they weren't

webadmin/admin.html

@@ -1148,7 +1148,7 @@ const dmarcEvaluationsDomain = async (domain) => {
 
 	const authStatus = (v) => inlineBox(v ? '' : yellow, v ? 'pass' : 'fail')
 	const formatDKIMResults = (results) => results.map(r => dom.div('selector '+r.Selector+(r.Domain !== domain ? ', domain '+r.Domain : '') + ': ', inlineBox(r.Result === "pass" ? '' : yellow, r.Result)))
-	const formatSPFResults = (results) => results.map(r => dom.div(''+r.Scope+(r.Domain !== domain ? ', domain '+r.Domain : '') + ': ', inlineBox(r.Result === "pass" ? '' : yellow, r.Result)))
+	const formatSPFResults = (alignedpass, results) => results.map(r => dom.div(''+r.Scope+(r.Domain !== domain ? ', domain '+r.Domain : '') + ': ', inlineBox(r.Result === "pass" && alignedpass ? '' : yellow, r.Result)))
 
 	const sourceIP = (ip) => {
 		const r = dom.span(ip, attr({title: 'Click to do a reverse lookup of the IP.'}), style({cursor: 'pointer'}), async function click(e) {
@@ -1198,7 +1198,7 @@ const dmarcEvaluationsDomain = async (domain) => {
 					dom.th('Policy', attr({title: 'Summary of the policy as encountered in the DMARC DNS record of the domain, and used for evaluation.'})),
 					dom.th('IP', attr({title: 'IP address of delivery attempt that was evaluated, relevant for SPF.'})),
 					dom.th('Disposition', attr({title: 'Our decision to accept/reject this message. It may be different than requested by the published policy. For example, when overriding due to delivery from a mailing list or forwarded address.'})),
-					dom.th('DKIM/SPF', attr({title: 'Whether DKIM and SPF had an aligned pass, where strict/relaxed alignment means whether the domain of an SPF pass and DKIM pass matches the exact domain (strict) or optionally a subdomain (relaxed). A DMARC pass requires at least one pass.'})),
+					dom.th('Aligned DKIM/SPF', attr({title: 'Whether DKIM and SPF had an aligned pass, where strict/relaxed alignment means whether the domain of an SPF pass and DKIM pass matches the exact domain (strict) or optionally a subdomain (relaxed). A DMARC pass requires at least one pass.'})),
 					dom.th('Envelope to', attr({title: 'Domain used in SMTP RCPT TO during delivery.'})),
 					dom.th('Envelope from', attr({title: 'Domain used in SMTP MAIL FROM during delivery.'})),
 					dom.th('Message from', attr({title: 'Domain in "From" message header.'})),
@@ -1228,13 +1228,13 @@ const dmarcEvaluationsDomain = async (domain) => {
 						dom.td(addresses),
 						dom.td(policy),
 						dom.td(sourceIP(e.SourceIP)),
-						dom.td(inlineBox(e.Disposition === 'none' ? '' : 'red', e.Disposition), (e.OverrideReasons || []).length > 0 ? ' ('+e.OverrideReasons.map(r => r.Type).join(', ')+')' : ''),
+						dom.td(inlineBox(e.Disposition === 'none' ? '' : red, e.Disposition), (e.OverrideReasons || []).length > 0 ? ' ('+e.OverrideReasons.map(r => r.Type).join(', ')+')' : ''),
 						dom.td(authStatus(e.AlignedDKIMPass), '/', authStatus(e.AlignedSPFPass)),
 						dom.td(e.EnvelopeTo),
 						dom.td(e.EnvelopeFrom),
 						dom.td(e.HeaderFrom),
 						dom.td(formatDKIMResults(e.DKIMResults || [])),
-						dom.td(formatSPFResults(e.SPFResults || [])),
+						dom.td(formatSPFResults(e.AlignedSPFPass, e.SPFResults || [])),
 					)
 				}),
 				evaluations.length === 0 ? dom.tr(dom.td(attr({colspan: '14'}), 'No evaluations.')) : [],