diff --git a/.gitignore b/.gitignore index 00d5a2f..060178d 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,7 @@ /testdata/smtp/postmaster/ /testdata/smtpserverfuzz/data/ /testdata/store/data/ +/testdata/tlsrptsend/data/ /testdata/train/ /testdata/webmail/data/ /testdata/upgradetest.mbox.gz diff --git a/README.md b/README.md index bef2f39..6f5e5a4 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ See Quickstart below to get started. ("localparts"), and in domain names (IDNA). - Automatic TLS with ACME, for use with Let's Encrypt and other CA's. - DANE and MTA-STS for inbound and outbound delivery over SMTP with STARTTLS, - including REQUIRETLS and with incoming TLSRPT reporting. + including REQUIRETLS and with incoming/outgoing TLSRPT reporting. - Web admin interface that helps you set up your domains and accounts (instructions to create DNS records, configure SPF/DKIM/DMARC/TLSRPT/MTA-STS), for status information, managing @@ -114,7 +114,6 @@ https://nlnet.nl/project/Mox/. ## Roadmap -- Sending TLS reports (currently only receiving) - Authentication other than HTTP-basic for webmail/webadmin/webaccount - Per-domain webmail and IMAP/SMTP host name (and TLS cert) and client settings - Make mox Go packages more easily reusable, each pulling in fewer (internal) diff --git a/backup.go b/backup.go index a4dd60b..6a56cb6 100644 --- a/backup.go +++ b/backup.go @@ -282,7 +282,8 @@ func backupctl(ctx context.Context, ctl *ctl) { backupDB(dmarcdb.ReportsDB, "dmarcrpt.db") backupDB(dmarcdb.EvalDB, "dmarceval.db") backupDB(mtastsdb.DB, "mtasts.db") - backupDB(tlsrptdb.DB, "tlsrpt.db") + backupDB(tlsrptdb.ReportDB, "tlsrpt.db") + backupDB(tlsrptdb.ResultDB, "tlsrptresult.db") backupFile("receivedid.key") // Acme directory is optional. @@ -530,7 +531,7 @@ func backupctl(ctx context.Context, ctl *ctl) { } switch p { - case "dmarcrpt.db", "dmarceval.db", "mtasts.db", "tlsrpt.db", "receivedid.key", "ctl": + case "dmarcrpt.db", "dmarceval.db", "mtasts.db", "tlsrpt.db", "tlsrptresult.db", "receivedid.key", "ctl": // Already handled. return nil case "lastknownversion": // Optional file, not yet handled. diff --git a/config/config.go b/config/config.go index 625eae3..286e962 100644 --- a/config/config.go +++ b/config/config.go @@ -57,10 +57,18 @@ type Static struct { Account string Mailbox string `sconf-doc:"E.g. Postmaster or Inbox."` } `sconf-doc:"Destination for emails delivered to postmaster addresses: a plain 'postmaster' without domain, 'postmaster@' (also for each listener with SMTP enabled), and as fallback for each domain without explicitly configured postmaster destination."` + HostTLSRPT struct { + Account string `sconf-doc:"Account to deliver TLS reports to. Typically same account as for postmaster."` + Mailbox string `sconf-doc:"Mailbox to deliver TLS reports to. Recommended value: TLSRPT."` + Localpart string `sconf-doc:"Localpart at hostname to accept TLS reports at. Recommended value: tls-reports."` + + ParsedLocalpart smtp.Localpart `sconf:"-"` + } `sconf:"optional" sconf-doc:"Destination for per-host TLS reports (TLSRPT). TLS reports can be per recipient domain (for MTA-STS), or per MX host (for DANE). The per-domain TLS reporting configuration is in domains.conf. This is the TLS reporting configuration for this host. If absent, no host-based TLSRPT address is configured, and no host TLSRPT DNS record is suggested."` InitialMailboxes InitialMailboxes `sconf:"optional" sconf-doc:"Mailboxes to create for new accounts. Inbox is always created. Mailboxes can be given a 'special-use' role, which are understood by most mail clients. If absent/empty, the following mailboxes are created: Sent, Archive, Trash, Drafts and Junk."` DefaultMailboxes []string `sconf:"optional" sconf-doc:"Deprecated in favor of InitialMailboxes. Mailboxes to create when adding an account. Inbox is always created. If no mailboxes are specified, the following are automatically created: Sent, Archive, Trash, Drafts and Junk."` Transports map[string]Transport `sconf:"optional" sconf-doc:"Transport are mechanisms for delivering messages. Transports can be referenced from Routes in accounts, domains and the global configuration. There is always an implicit/fallback delivery transport doing direct delivery with SMTP from the outgoing message queue. Transports are typically only configured when using smarthosts, i.e. when delivering through another SMTP server. Zero or one transport methods must be set in a transport, never multiple. When using an external party to send email for a domain, keep in mind you may have to add their IP address to your domain's SPF record, and possibly additional DKIM records."` NoOutgoingDMARCReports bool `sconf:"optional" sconf-doc:"Do not send DMARC reports (aggregate only). By default, aggregate reports on DMARC evaluations are sent to domains if their DMARC policy requests them. Reports are sent at whole hours, with a minimum of 1 hour and maximum of 24 hours, rounded up so a whole number of intervals cover 24 hours, aligned at whole days in UTC."` + NoOutgoingTLSReports bool `sconf:"optional" sconf-doc:"Do not send TLS reports. By default, reports about successful and failed SMTP STARTTLS connections are sent to domains if their TLSRPT DNS record requests them. Reports covering a 24 hour UTC interval are sent daily."` // All IPs that were explicitly listen on for external SMTP. Only set when there // are no unspecified external SMTP listeners and there is at most one for IPv4 and @@ -126,7 +134,7 @@ type Listener struct { Enabled bool Port int `sconf:"optional" sconf-doc:"Default 25."` NoSTARTTLS bool `sconf:"optional" sconf-doc:"Do not offer STARTTLS to secure the connection. Not recommended."` - RequireSTARTTLS bool `sconf:"optional" sconf-doc:"Do not accept incoming messages if STARTTLS is not active. Can be used in combination with a strict MTA-STS policy. A remote SMTP server may not support TLS and may not be able to deliver messages."` + RequireSTARTTLS bool `sconf:"optional" sconf-doc:"Do not accept incoming messages if STARTTLS is not active. Consider using in combination with an MTA-STS policy and/or DANE. A remote SMTP server may not support TLS and may not be able to deliver messages. Incoming messages for TLS reporting addresses ignore this setting and do not require TLS."` NoRequireTLS bool `sconf:"optional" sconf-doc:"Do not announce the REQUIRETLS SMTP extension. Messages delivered using the REQUIRETLS extension should only be distributed onwards to servers also implementing the REQUIRETLS extension. In some situations, such as hosting mailing lists, this may not be feasible due to lack of support for the extension by mailing list subscribers."` // Reoriginated messages (such as messages sent to mailing list subscribers) should // keep REQUIRETLS. ../rfc/8689:412 @@ -306,7 +314,7 @@ type Selector struct { BodyRelaxed bool `sconf-doc:"If set, some whitespace modifications to the message body are allowed."` } `sconf:"optional"` Headers []string `sconf:"optional" sconf-doc:"Headers to sign with DKIM. If empty, a reasonable default set of headers is selected."` - HeadersEffective []string `sconf:"-"` + HeadersEffective []string `sconf:"-"` // Used when signing. Based on Headers from config, or the reasonable default. DontSealHeaders bool `sconf:"optional" sconf-doc:"If set, don't prevent duplicate headers from being added. Not recommended."` Expiration string `sconf:"optional" sconf-doc:"Period a signature is valid after signing, as duration, e.g. 72h. The period should be enough for delivery at the final destination, potentially with several hops/relays. In the order of days at least."` PrivateKeyFile string `sconf-doc:"Either an RSA or ed25519 private key file in PKCS8 PEM form."` @@ -371,8 +379,9 @@ type Destination struct { Rulesets []Ruleset `sconf:"optional" sconf-doc:"Delivery rules based on message and SMTP transaction. You may want to match each mailing list by SMTP MailFrom address, VerifiedDomain and/or List-ID header (typically if the list address is listname@example.org), delivering them to their own mailbox."` FullName string `sconf:"optional" sconf-doc:"Full name to use in message From header when composing messages coming from this address with webmail."` - DMARCReports bool `sconf:"-" json:"-"` - TLSReports bool `sconf:"-" json:"-"` + DMARCReports bool `sconf:"-" json:"-"` + HostTLSReports bool `sconf:"-" json:"-"` + DomainTLSReports bool `sconf:"-" json:"-"` } // Equal returns whether d and o are equal, only looking at their user-changeable fields. diff --git a/config/doc.go b/config/doc.go index 80241a3..4099828 100644 --- a/config/doc.go +++ b/config/doc.go @@ -179,9 +179,10 @@ describe-static" and "mox config describe-domains": # Do not offer STARTTLS to secure the connection. Not recommended. (optional) NoSTARTTLS: false - # Do not accept incoming messages if STARTTLS is not active. Can be used in - # combination with a strict MTA-STS policy. A remote SMTP server may not support - # TLS and may not be able to deliver messages. (optional) + # Do not accept incoming messages if STARTTLS is not active. Consider using in + # combination with an MTA-STS policy and/or DANE. A remote SMTP server may not + # support TLS and may not be able to deliver messages. Incoming messages for TLS + # reporting addresses ignore this setting and do not require TLS. (optional) RequireSTARTTLS: false # Do not announce the REQUIRETLS SMTP extension. Messages delivered using the @@ -391,6 +392,22 @@ describe-static" and "mox config describe-domains": # E.g. Postmaster or Inbox. Mailbox: + # Destination for per-host TLS reports (TLSRPT). TLS reports can be per recipient + # domain (for MTA-STS), or per MX host (for DANE). The per-domain TLS reporting + # configuration is in domains.conf. This is the TLS reporting configuration for + # this host. If absent, no host-based TLSRPT address is configured, and no host + # TLSRPT DNS record is suggested. (optional) + HostTLSRPT: + + # Account to deliver TLS reports to. Typically same account as for postmaster. + Account: + + # Mailbox to deliver TLS reports to. Recommended value: TLSRPT. + Mailbox: + + # Localpart at hostname to accept TLS reports at. Recommended value: tls-reports. + Localpart: + # Mailboxes to create for new accounts. Inbox is always created. Mailboxes can be # given a 'special-use' role, which are understood by most mail clients. If # absent/empty, the following mailboxes are created: Sent, Archive, Trash, Drafts @@ -556,6 +573,11 @@ describe-static" and "mox config describe-domains": # whole days in UTC. (optional) NoOutgoingDMARCReports: false + # Do not send TLS reports. By default, reports about successful and failed SMTP + # STARTTLS connections are sent to domains if their TLSRPT DNS record requests + # them. Reports covering a 24 hour UTC interval are sent daily. (optional) + NoOutgoingTLSReports: false + # domains.conf # NOTE: This config file is in 'sconf' format. Indent with tabs. Comments must be diff --git a/dmarcdb/eval.go b/dmarcdb/eval.go index 2f175ba..bc9a184 100644 --- a/dmarcdb/eval.go +++ b/dmarcdb/eval.go @@ -1,5 +1,7 @@ package dmarcdb +// Sending TLS reports and DMARC reports is very similar. See ../dmarcdb/eval.go:/similar and ../tlsrptsend/send.go:/similar. + import ( "compress/gzip" "context" diff --git a/gentestdata.go b/gentestdata.go index 7d20e3c..cec8561 100644 --- a/gentestdata.go +++ b/gentestdata.go @@ -209,7 +209,7 @@ Accounts: }, MaxAgeSeconds: 1296000, } - err = mtastsdb.Upsert(ctxbg, dns.Domain{ASCII: "mox.example"}, "123", &mtastsPolicy) + err = mtastsdb.Upsert(ctxbg, dns.Domain{ASCII: "mox.example"}, "123", &mtastsPolicy, mtastsPolicy.String()) xcheckf(err, "adding mtastsdb report") // Populate tlsrpt.db. @@ -217,7 +217,7 @@ Accounts: xcheckf(err, "tlsrptdb init") tlsr, err := tlsrpt.Parse(strings.NewReader(tlsReport)) xcheckf(err, "parsing tls report") - err = tlsrptdb.AddReport(ctxbg, dns.Domain{ASCII: "mox.example"}, "tlsrpt@mox.example", tlsr) + err = tlsrptdb.AddReport(ctxbg, dns.Domain{ASCII: "mox.example"}, "tlsrpt@mox.example", false, tlsr) xcheckf(err, "adding tls report") // Populate queue, with a message. diff --git a/go.mod b/go.mod index 1ea6b20..c20c0ba 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/mjl-/mox go 1.20 require ( - github.com/mjl-/adns v0.0.0-20231013194548-ea0378d616ab + github.com/mjl-/adns v0.0.0-20231109160910-82839fe3e6ae github.com/mjl-/autocert v0.0.0-20231013072455-c361ae2e20a6 github.com/mjl-/bstore v0.0.2 github.com/mjl-/sconf v0.0.5 diff --git a/go.sum b/go.sum index a3f59e4..fde8640 100644 --- a/go.sum +++ b/go.sum @@ -145,8 +145,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= -github.com/mjl-/adns v0.0.0-20231013194548-ea0378d616ab h1:fL+dZP+IxX08+ugLq42bkvOfV42muXET+T+Ei1K16bI= -github.com/mjl-/adns v0.0.0-20231013194548-ea0378d616ab/go.mod h1:v47qUMJnipnmDTRGaHwpCwzE6oypa5K33mUvBfzZBn8= +github.com/mjl-/adns v0.0.0-20231109160910-82839fe3e6ae h1:P/kTaQbDFSbmDK+RVjwu7mPyr6a6XyHqHu1PlRMEbAU= +github.com/mjl-/adns v0.0.0-20231109160910-82839fe3e6ae/go.mod h1:v47qUMJnipnmDTRGaHwpCwzE6oypa5K33mUvBfzZBn8= github.com/mjl-/autocert v0.0.0-20231013072455-c361ae2e20a6 h1:TEXyTghAN9pmV2ffzdnhmzkML08e1Z/oGywJ9eunbRI= github.com/mjl-/autocert v0.0.0-20231013072455-c361ae2e20a6/go.mod h1:taMFU86abMxKLPV4Bynhv8enbYmS67b8LG80qZv2Qus= github.com/mjl-/bstore v0.0.2 h1:4fdpIOY/+Dv1dBHyzdqa4PD90p8Mz86FeyRpI4qcehw= diff --git a/integration_test.go b/integration_test.go index b07199a..2af61db 100644 --- a/integration_test.go +++ b/integration_test.go @@ -129,7 +129,7 @@ This is the message. `, mailfrom, rcptto) msg = strings.ReplaceAll(msg, "\n", "\r\n") auth := []sasl.Client{sasl.NewClientPlain(mailfrom, password)} - c, err := smtpclient.New(mox.Context, xlog, conn, smtpclient.TLSSkip, ourHostname, dns.Domain{ASCII: desthost}, auth, nil, nil, nil) + c, err := smtpclient.New(mox.Context, xlog, conn, smtpclient.TLSSkip, false, ourHostname, dns.Domain{ASCII: desthost}, smtpclient.Opts{Auth: auth}) tcheck(t, err, "smtp hello") err = c.Deliver(mox.Context, mailfrom, rcptto, int64(len(msg)), strings.NewReader(msg), false, false, false) tcheck(t, err, "deliver with smtp") diff --git a/localserve.go b/localserve.go index 52cd661..5768600 100644 --- a/localserve.go +++ b/localserve.go @@ -139,8 +139,9 @@ during those commands instead of during "data". const mtastsdbRefresher = false const sendDMARCReports = false + const sendTLSReports = false const skipForkExec = true - if err := start(mtastsdbRefresher, sendDMARCReports, skipForkExec); err != nil { + if err := start(mtastsdbRefresher, sendDMARCReports, sendTLSReports, skipForkExec); err != nil { log.Fatalx("starting mox", err) } golog.Printf("mox, version %s", moxvar.Version) diff --git a/main.go b/main.go index 39316e9..14264d3 100644 --- a/main.go +++ b/main.go @@ -842,9 +842,10 @@ func cmdConfigDNSCheck(c *cmd) { printResult("SPF", result.SPF.Result) printResult("DKIM", result.DKIM.Result) printResult("DMARC", result.DMARC.Result) - printResult("TLSRPT", result.TLSRPT.Result) + printResult("Host TLSRPT", result.HostTLSRPT.Result) + printResult("Domain TLSRPT", result.DomainTLSRPT.Result) printResult("MTASTS", result.MTASTS.Result) - printResult("SRV", result.SRVConf.Result) + printResult("SRV conf", result.SRVConf.Result) printResult("Autoconf", result.Autoconf.Result) printResult("Autodiscover", result.Autodiscover.Result) } @@ -1728,14 +1729,14 @@ sharing most of its code. log.Printf("looking up tlsa records: %s, skipping", err) continue } - tlsMode := smtpclient.TLSStrictStartTLS + tlsMode := smtpclient.TLSRequiredStartTLS if len(daneRecords) == 0 { if !daneRequired { log.Printf("host %s has no tlsa records, skipping", expandedHost) continue } log.Printf("warning: only unusable tlsa records found, continuing with required tls without certificate verification") - tlsMode = smtpclient.TLSUnverifiedStartTLS + daneRecords = nil } else { var l []string for _, r := range daneRecords { @@ -1744,9 +1745,9 @@ sharing most of its code. log.Printf("tlsa records: %s", strings.Join(l, "; ")) } - tlsRemoteHostnames := smtpclient.GatherTLSANames(haveMX, expandedNextHopAuthentic, expandedAuthentic, origNextHop, expandedNextHop, host.Domain, tlsaBaseDomain) + tlsHostnames := smtpclient.GatherTLSANames(haveMX, expandedNextHopAuthentic, expandedAuthentic, origNextHop, expandedNextHop, host.Domain, tlsaBaseDomain) var l []string - for _, name := range tlsRemoteHostnames { + for _, name := range tlsHostnames { l = append(l, name.String()) } log.Printf("gathered valid tls certificate names for potential verification with dane-ta: %s", strings.Join(l, ", ")) @@ -1760,7 +1761,14 @@ sharing most of its code. log.Printf("connected to %s, %s, starting smtp session with ehlo and starttls with dane verification", expandedHost, conn.RemoteAddr()) var verifiedRecord adns.TLSA - sc, err := smtpclient.New(ctxbg, clog, conn, tlsMode, ehloDomain, tlsRemoteHostnames[0], nil, daneRecords, tlsRemoteHostnames[1:], &verifiedRecord) + opts := smtpclient.Opts{ + DANERecords: daneRecords, + DANEMoreHostnames: tlsHostnames[1:], + DANEVerifiedRecord: &verifiedRecord, + RootCAs: mox.Conf.Static.TLS.CertPool, + } + tlsPKIX := false + sc, err := smtpclient.New(ctxbg, clog, conn, tlsMode, tlsPKIX, ehloDomain, tlsHostnames[0], opts) if err != nil { log.Printf("setting up smtp session: %v, skipping", err) conn.Close() @@ -2672,7 +2680,7 @@ should be used, and how long the policy can be cached. domain := xparseDomain(args[0], "domain") - record, policy, err := mtasts.Get(context.Background(), dns.StrictResolver{}, domain) + record, policy, _, err := mtasts.Get(context.Background(), dns.StrictResolver{}, domain) if err != nil { fmt.Printf("error: %s\n", err) } @@ -2712,6 +2720,8 @@ func cmdTLSRPTDBAddReport(c *cmd) { c.unlisted = true c.params = "< message" c.help = "Parse a TLS report from the message and add it to the database." + var hostReport bool + c.flag.BoolVar(&hostReport, "hostreport", false, "report for a host instead of domain") args := c.Parse() if len(args) != 0 { c.Usage() @@ -2737,7 +2747,7 @@ func cmdTLSRPTDBAddReport(c *cmd) { xcheckf(err, "parsing tls report in message") mailfrom := from.User + "@" + from.Host // todo future: should escape and such - err = tlsrptdb.AddReport(context.Background(), domain, mailfrom, report) + err = tlsrptdb.AddReport(context.Background(), domain, mailfrom, hostReport, report) xcheckf(err, "add tls report to database") } diff --git a/metrics/panic.go b/metrics/panic.go index f9a1443..59ca8d2 100644 --- a/metrics/panic.go +++ b/metrics/panic.go @@ -27,6 +27,7 @@ const ( Queue Panic = "queue" Smtpclient Panic = "smtpclient" Smtpserver Panic = "smtpserver" + Tlsrptdb Panic = "tlsrptdb" Dkimverify Panic = "dkimverify" Spfverify Panic = "spfverify" Upgradethreads Panic = "upgradethreads" diff --git a/mox-/admin.go b/mox-/admin.go index f2bc0cb..a7670e9 100644 --- a/mox-/admin.go +++ b/mox-/admin.go @@ -523,11 +523,28 @@ func DomainRecords(domConf config.Domain, domain dns.Domain, hasDNSSEC bool) ([] if d != h { records = append(records, - "; For the machine, only needs to be created once, for the first domain added.", + "; For the machine, only needs to be created once, for the first domain added:", + "; ", + "; SPF-allow host for itself, resulting in relaxed DMARC pass for (postmaster)", + "; messages (DSNs) sent from host:", fmt.Sprintf(`%-*s TXT "v=spf1 a -all"`, 20+len(d), h+"."), // ../rfc/7208:2263 ../rfc/7208:2287 "", ) } + if d != h && Conf.Static.HostTLSRPT.ParsedLocalpart != "" { + uri := url.URL{ + Scheme: "mailto", + Opaque: smtp.NewAddress(Conf.Static.HostTLSRPT.ParsedLocalpart, Conf.Static.HostnameDomain).Pack(false), + } + tlsrptr := tlsrpt.Record{Version: "TLSRPTv1", RUAs: [][]string{{uri.String()}}} + records = append(records, + "; For the machine, only needs to be created once, for the first domain added:", + "; ", + "; Request reporting about success/failures of TLS connections to (MX) host, for DANE.", + fmt.Sprintf(`_smtp._tls.%-*s TXT "%s"`, 20+len(d)-len("_smtp._tls."), h+".", tlsrptr.String()), + "", + ) + } records = append(records, "; Deliver email for the domain to this host.", diff --git a/mox-/config.go b/mox-/config.go index 457fd0e..ef1aef7 100644 --- a/mox-/config.go +++ b/mox-/config.go @@ -501,6 +501,18 @@ func PrepareStaticConfig(ctx context.Context, configFile string, conf *Config, c } c.HostnameDomain = hostname + if c.HostTLSRPT.Account != "" { + tlsrptLocalpart, err := smtp.ParseLocalpart(c.HostTLSRPT.Localpart) + if err != nil { + addErrorf("invalid localpart %q for host tlsrpt: %v", c.HostTLSRPT.Localpart, err) + } else if tlsrptLocalpart.IsInternational() { + // Does not appear documented in ../rfc/8460, but similar to DMARC it makes sense + // to keep this ascii-only addresses. + addErrorf("host TLSRPT localpart %q is an internationalized address, only conventional ascii-only address allowed for interopability", tlsrptLocalpart) + } + c.HostTLSRPT.ParsedLocalpart = tlsrptLocalpart + } + // Return private key for host name for use with an ACME. Used to return the same // private key as pre-generated for use with DANE, with its public key in DNS. // We only use this key for Listener's that have this ACME configured, and for @@ -942,6 +954,25 @@ func prepareDynamicConfig(ctx context.Context, dynamicPath string, static config } checkMailboxNormf(static.Postmaster.Mailbox, "postmaster mailbox") + accDests = map[string]AccountDestination{} + + // Validate host TLSRPT account/address. + if static.HostTLSRPT.Account != "" { + if _, ok := c.Accounts[static.HostTLSRPT.Account]; !ok { + addErrorf("host tlsrpt account %q does not exist", static.HostTLSRPT.Account) + } + checkMailboxNormf(static.HostTLSRPT.Mailbox, "host tlsrpt mailbox") + + // Localpart has been parsed already. + + addrFull := smtp.NewAddress(static.HostTLSRPT.ParsedLocalpart, static.HostnameDomain).String() + dest := config.Destination{ + Mailbox: static.HostTLSRPT.Mailbox, + HostTLSReports: true, + } + accDests[addrFull] = AccountDestination{false, static.HostTLSRPT.ParsedLocalpart, static.HostTLSRPT.Account, dest} + } + var haveSTSListener, haveWebserverListener bool for _, l := range static.Listeners { if l.MTASTSHTTPS.Enabled { @@ -1111,7 +1142,6 @@ func prepareDynamicConfig(ctx context.Context, dynamicPath string, static config } // Validate email addresses. - accDests = map[string]AccountDestination{} for accName, acc := range c.Accounts { var err error acc.DNSDomain, err = dns.ParseDomain(acc.Domain) @@ -1366,8 +1396,8 @@ func prepareDynamicConfig(ctx context.Context, dynamicPath string, static config c.Domains[d] = domain addrFull := smtp.NewAddress(lp, addrdom).String() dest := config.Destination{ - Mailbox: tlsrpt.Mailbox, - TLSReports: true, + Mailbox: tlsrpt.Mailbox, + DomainTLSReports: true, } checkMailboxNormf(tlsrpt.Mailbox, "TLSRPT mailbox for account %q", tlsrpt.Account) accDests[addrFull] = AccountDestination{false, lp, tlsrpt.Account, dest} diff --git a/mtasts/mtasts.go b/mtasts/mtasts.go index 3f91fe5..e47e713 100644 --- a/mtasts/mtasts.go +++ b/mtasts/mtasts.go @@ -23,6 +23,8 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/mjl-/adns" + "github.com/mjl-/mox/dns" "github.com/mjl-/mox/metrics" "github.com/mjl-/mox/mlog" @@ -153,6 +155,30 @@ func (p *Policy) Matches(host dns.Domain) bool { return false } +// TLSReportFailureReason returns a concise error for known error types, or an +// empty string. For use in TLSRPT. +func TLSReportFailureReason(err error) string { + // If this is a DNSSEC authentication error, we'll collect it for TLS reporting. + // We can also use this reason for STS, not only DANE. ../rfc/8460:580 + var errCode adns.ErrorCode + if errors.As(err, &errCode) && errCode.IsAuthentication() { + return fmt.Sprintf("dns-extended-error-%d-%s", errCode, strings.ReplaceAll(errCode.String(), " ", "-")) + } + + for _, e := range mtastsErrors { + if errors.Is(err, e) { + s := strings.TrimPrefix(e.Error(), "mtasts: ") + return strings.ReplaceAll(s, " ", "-") + } + } + return "" +} + +var mtastsErrors = []error{ + ErrNoRecord, ErrMultipleRecords, ErrDNS, ErrRecordSyntax, // Lookup + ErrNoPolicy, ErrPolicyFetch, ErrPolicySyntax, // Fetch +} + // Lookup errors. var ( ErrNoRecord = errors.New("mtasts: no mta-sts dns txt record") // Domain does not implement MTA-STS. If a cached non-expired policy is available, it should still be used. @@ -262,7 +288,8 @@ func FetchPolicy(ctx context.Context, domain dns.Domain) (policy *Policy, policy return nil, "", ErrNoPolicy } if err != nil { - return nil, "", fmt.Errorf("%w: http get: %s", ErrPolicyFetch, err) + // We pass along underlying TLS certificate errors. + return nil, "", fmt.Errorf("%w: http get: %w", ErrPolicyFetch, err) } metrics.HTTPClientObserve(ctx, "mtasts", req.Method, resp.StatusCode, err, start) defer resp.Body.Close() @@ -302,7 +329,7 @@ func FetchPolicy(ctx context.Context, domain dns.Domain) (policy *Policy, policy // record is still returned. // // Also see Get in package mtastsdb. -func Get(ctx context.Context, resolver dns.Resolver, domain dns.Domain) (record *Record, policy *Policy, err error) { +func Get(ctx context.Context, resolver dns.Resolver, domain dns.Domain) (record *Record, policy *Policy, policyText string, err error) { log := xlog.WithContext(ctx) start := time.Now() result := "lookuperror" @@ -313,15 +340,15 @@ func Get(ctx context.Context, resolver dns.Resolver, domain dns.Domain) (record record, _, err = LookupRecord(ctx, resolver, domain) if err != nil { - return nil, nil, err + return nil, nil, "", err } result = "fetcherror" - policy, _, err = FetchPolicy(ctx, domain) + policy, policyText, err = FetchPolicy(ctx, domain) if err != nil { - return record, nil, err + return record, nil, "", err } result = "ok" - return record, policy, nil + return record, policy, policyText, nil } diff --git a/mtasts/mtasts_test.go b/mtasts/mtasts_test.go index 6018d7b..1a51ef0 100644 --- a/mtasts/mtasts_test.go +++ b/mtasts/mtasts_test.go @@ -247,7 +247,7 @@ func TestFetch(t *testing.T) { expErr = ErrNoRecord } - _, p, err = Get(context.Background(), resolver, dns.Domain{ASCII: domain}) + _, p, _, err = Get(context.Background(), resolver, dns.Domain{ASCII: domain}) if (err == nil) != (expErr == nil) || err != nil && !errors.Is(err, expErr) { t.Fatalf("get: got err %#v, expected %#v", err, expErr) } diff --git a/mtastsdb/db.go b/mtastsdb/db.go index cef3828..5c87e0c 100644 --- a/mtastsdb/db.go +++ b/mtastsdb/db.go @@ -7,10 +7,12 @@ package mtastsdb import ( "context" + "crypto/tls" "errors" "fmt" "os" "path/filepath" + "strings" "sync" "time" @@ -23,6 +25,7 @@ import ( "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/mtasts" + "github.com/mjl-/mox/tlsrpt" ) var xlog = mlog.New("mtastsdb") @@ -49,6 +52,10 @@ type PolicyRecord struct { Backoff bool RecordID string // As retrieved from DNS. mtasts.Policy // As retrieved from the well-known HTTPS url. + + // Text that make up the policy, as retrieved. We didn't store this in the past. If + // empty, policy can be reconstructed from Policy field. Needed by TLSRPT. + PolicyText string } var ( @@ -145,7 +152,7 @@ func lookup(ctx context.Context, domain dns.Domain) (*PolicyRecord, error) { // Upsert adds the policy to the database, overwriting an existing policy for the domain. // Policy can be nil, indicating a failure to fetch the policy. -func Upsert(ctx context.Context, domain dns.Domain, recordID string, policy *mtasts.Policy) error { +func Upsert(ctx context.Context, domain dns.Domain, recordID string, policy *mtasts.Policy, policyText string) error { db, err := database(ctx) if err != nil { return err @@ -172,7 +179,7 @@ func Upsert(ctx context.Context, domain dns.Domain, recordID string, policy *mta validEnd := now.Add(time.Duration(p.MaxAgeSeconds) * time.Second) if err == bstore.ErrAbsent { - pr = PolicyRecord{domain.Name(), now, validEnd, now, now, backoff, recordID, p} + pr = PolicyRecord{domain.Name(), now, validEnd, now, now, backoff, recordID, p, policyText} return tx.Insert(&pr) } @@ -182,6 +189,7 @@ func Upsert(ctx context.Context, domain dns.Domain, recordID string, policy *mta pr.Backoff = backoff pr.RecordID = recordID pr.Policy = p + pr.PolicyText = policyText return tx.Update(&pr) }) } @@ -210,7 +218,11 @@ func PolicyRecords(ctx context.Context) ([]PolicyRecord, error) { // // Some errors are logged but not otherwise returned, e.g. if a new policy is // supposedly published but could not be retrieved. -func Get(ctx context.Context, resolver dns.Resolver, domain dns.Domain) (policy *mtasts.Policy, fresh bool, err error) { +// +// Get returns an "sts" or "no-policy-found" in reportResult in most cases (when +// not a local/internal error). It may add an "sts" result without policy contents +// ("policy-string") in case of errors while fetching the policy. +func Get(ctx context.Context, resolver dns.Resolver, domain dns.Domain) (policy *mtasts.Policy, reportResult tlsrpt.Result, fresh bool, err error) { log := xlog.WithContext(ctx) defer func() { result := "ok" @@ -231,17 +243,37 @@ func Get(ctx context.Context, resolver dns.Resolver, domain dns.Domain) (policy // and should backoff. So attempt to fetch policy. nctx, cancel := context.WithTimeout(ctx, time.Minute) defer cancel() - record, p, err := mtasts.Get(nctx, resolver, domain) + record, p, ptext, err := mtasts.Get(nctx, resolver, domain) if err != nil { switch { case errors.Is(err, mtasts.ErrNoRecord) || errors.Is(err, mtasts.ErrMultipleRecords) || errors.Is(err, mtasts.ErrRecordSyntax) || errors.Is(err, mtasts.ErrNoPolicy) || errors.Is(err, mtasts.ErrPolicyFetch) || errors.Is(err, mtasts.ErrPolicySyntax): // Remote is not doing MTA-STS, continue below. ../rfc/8461:333 ../rfc/8461:574 log.Debugx("interpreting mtasts error to mean remote is not doing mta-sts", err) + + if errors.Is(err, mtasts.ErrNoRecord) { + reportResult = tlsrpt.MakeResult(tlsrpt.NoPolicyFound, domain) + } else { + fd := policyFetchFailureDetails(err) + reportResult = tlsrpt.MakeResult(tlsrpt.STS, domain, fd) + } + default: // Interpret as temporary error, e.g. mtasts.ErrDNS, try again later. - return nil, false, fmt.Errorf("lookup up mta-sts policy: %w", err) + + // Temporary DNS error could be an operational issue on our side, but we can still + // report it. + // Result: ../rfc/8460:594 + fd := tlsrpt.Details(tlsrpt.ResultSTSPolicyFetch, mtasts.TLSReportFailureReason(err)) + reportResult = tlsrpt.MakeResult(tlsrpt.STS, domain, fd) + + return nil, reportResult, false, fmt.Errorf("lookup up mta-sts policy: %w", err) } + } else if p.Mode == mtasts.ModeNone { + reportResult = tlsrpt.MakeResult(tlsrpt.NoPolicyFound, domain) + } else { + reportResult = tlsrpt.Result{Policy: tlsrptPolicy(p, ptext, domain)} } + // Insert policy into database. If we could not fetch the policy itself, we back // off for 5 minutes. ../rfc/8461:555 if err == nil || errors.Is(err, mtasts.ErrNoPolicy) || errors.Is(err, mtasts.ErrPolicyFetch) || errors.Is(err, mtasts.ErrPolicySyntax) { @@ -249,17 +281,22 @@ func Get(ctx context.Context, resolver dns.Resolver, domain dns.Domain) (policy if record != nil { recordID = record.ID } - if err := Upsert(ctx, domain, recordID, p); err != nil { + if err := Upsert(ctx, domain, recordID, p, ptext); err != nil { log.Errorx("inserting policy into cache, continuing", err) } } - return p, true, nil + + return p, reportResult, true, nil } else if err != nil && errors.Is(err, ErrBackoff) { // ../rfc/8461:552 // We recently failed to fetch a policy, act as if MTA-STS is not implemented. - return nil, false, nil + // Result: ../rfc/8460:594 + fd := tlsrpt.Details(tlsrpt.ResultSTSPolicyFetch, "back-off-after-recent-fetch-error") + reportResult = tlsrpt.MakeResult(tlsrpt.STS, domain, fd) + return nil, reportResult, false, nil } else if err != nil { - return nil, false, fmt.Errorf("looking up mta-sts policy in cache: %w", err) + // We don't add the result to the report, this is an internal error. + return nil, reportResult, false, fmt.Errorf("looking up mta-sts policy in cache: %w", err) } // Policy was found in database. Check in DNS it is still fresh. @@ -268,25 +305,96 @@ func Get(ctx context.Context, resolver dns.Resolver, domain dns.Domain) (policy defer cancel() record, _, err := mtasts.LookupRecord(nctx, resolver, domain) if err != nil { - if !errors.Is(err, mtasts.ErrNoRecord) { + if errors.Is(err, mtasts.ErrNoRecord) { + if policy.Mode != mtasts.ModeNone { + log.Errorx("no mtasts dns record while checking non-none policy for freshness, either domain owner removed mta-sts without phasing out policy with a none-policy for period of previous max-age, or this could be an attempt to downgrade to connection without mtasts, continuing with previous policy", err) + } + // else, policy will be removed by periodic refresher in the near future. + } else { // Could be a temporary DNS or configuration error. log.Errorx("checking for freshness of cached mta-sts dns txt record for domain, continuing with previously cached policy", err) } - return policy, false, nil - } else if record.ID == cachedPolicy.RecordID { - return policy, true, nil + + // Result: ../rfc/8460:594 + fd := tlsrpt.Details(tlsrpt.ResultSTSPolicyFetch, mtasts.TLSReportFailureReason(err)) + if policy.Mode != mtasts.ModeNone { + fd.FailureReasonCode += "+fallback-to-cached-policy" + } + reportResult = tlsrpt.Result{ + Policy: tlsrptPolicy(policy, cachedPolicy.PolicyText, domain), + FailureDetails: []tlsrpt.FailureDetails{fd}, + } + return policy, reportResult, false, nil + } else if record.ID == cachedPolicy.RecordID && cachedPolicy.PolicyText != "" { + // In the past, we didn't store the raw policy lines in cachedPolicy.Lines. We only + // stop now if we do have policy lines in the cache. + reportResult = tlsrpt.Result{Policy: tlsrptPolicy(policy, cachedPolicy.PolicyText, domain)} + return policy, reportResult, true, nil } - // New policy should be available. + // New policy should be available, or we are fetching the policy again because we + // didn't store the raw policy lines in the past. nctx, cancel = context.WithTimeout(ctx, 30*time.Second) defer cancel() - p, _, err := mtasts.FetchPolicy(nctx, domain) + p, ptext, err := mtasts.FetchPolicy(nctx, domain) if err != nil { log.Errorx("fetching updated policy for domain, continuing with previously cached policy", err) - return policy, false, nil + + fd := policyFetchFailureDetails(err) + fd.FailureReasonCode += "+fallback-to-cached-policy" + reportResult = tlsrpt.Result{ + Policy: tlsrptPolicy(policy, cachedPolicy.PolicyText, domain), + FailureDetails: []tlsrpt.FailureDetails{fd}, + } + return policy, reportResult, false, nil } - if err := Upsert(ctx, domain, record.ID, p); err != nil { + if err := Upsert(ctx, domain, record.ID, p, ptext); err != nil { log.Errorx("inserting refreshed policy into cache, continuing with fresh policy", err) } - return p, true, nil + reportResult = tlsrpt.Result{Policy: tlsrptPolicy(p, ptext, domain)} + return p, reportResult, true, nil +} + +func policyFetchFailureDetails(err error) tlsrpt.FailureDetails { + var verificationErr *tls.CertificateVerificationError + if errors.As(err, &verificationErr) { + resultType, reasonCode := tlsrpt.TLSFailureDetails(verificationErr) + // Result: ../rfc/8460:601 + reason := string(resultType) + if reasonCode != "" { + reason += "+" + reasonCode + } + return tlsrpt.Details(tlsrpt.ResultSTSWebPKIInvalid, reason) + } else if errors.Is(err, mtasts.ErrPolicySyntax) { + // Result: ../rfc/8460:598 + return tlsrpt.Details(tlsrpt.ResultSTSPolicyInvalid, mtasts.TLSReportFailureReason(err)) + } + // Result: ../rfc/8460:594 + return tlsrpt.Details(tlsrpt.ResultSTSPolicyFetch, mtasts.TLSReportFailureReason(err)) +} + +func tlsrptPolicy(p *mtasts.Policy, policyText string, domain dns.Domain) tlsrpt.ResultPolicy { + if policyText == "" { + // We didn't always store original policy lines. Reconstruct. + policyText = p.String() + } + lines := strings.Split(strings.TrimSuffix(policyText, "\n"), "\n") + for i, line := range lines { + lines[i] = strings.TrimSuffix(line, "\r") + } + + rp := tlsrpt.ResultPolicy{ + Type: tlsrpt.STS, + Domain: domain.ASCII, + String: lines, + } + rp.MXHost = make([]string, len(p.MX)) + for i, mx := range p.MX { + s := mx.Domain.ASCII + if mx.Wildcard { + s = "*." + s + } + rp.MXHost[i] = s + } + return rp } diff --git a/mtastsdb/db_test.go b/mtastsdb/db_test.go index 43989d8..8fb32b9 100644 --- a/mtastsdb/db_test.go +++ b/mtastsdb/db_test.go @@ -56,7 +56,7 @@ func TestDB(t *testing.T) { }, MaxAgeSeconds: 1296000, } - if err := Upsert(ctxbg, dns.Domain{ASCII: "example.com"}, "123", &policy1); err != nil { + if err := Upsert(ctxbg, dns.Domain{ASCII: "example.com"}, "123", &policy1, policy1.String()); err != nil { t.Fatalf("upsert record: %s", err) } if got, err := lookup(ctxbg, dns.Domain{ASCII: "example.com"}); err != nil { @@ -73,7 +73,7 @@ func TestDB(t *testing.T) { }, MaxAgeSeconds: 360000, } - if err := Upsert(ctxbg, dns.Domain{ASCII: "example.com"}, "124", &policy2); err != nil { + if err := Upsert(ctxbg, dns.Domain{ASCII: "example.com"}, "124", &policy2, policy2.String()); err != nil { t.Fatalf("upsert record: %s", err) } if got, err := lookup(ctxbg, dns.Domain{ASCII: "example.com"}); err != nil { @@ -86,7 +86,7 @@ func TestDB(t *testing.T) { records, err := PolicyRecords(ctxbg) tcheckf(t, err, "policyrecords") expRecords := []PolicyRecord{ - {"example.com", now, now.Add(time.Duration(policy2.MaxAgeSeconds) * time.Second), now, now, false, "124", policy2}, + {"example.com", now, now.Add(time.Duration(policy2.MaxAgeSeconds) * time.Second), now, now, false, "124", policy2, policy2.String()}, } records[0].Policy = mtasts.Policy{} expRecords[0].Policy = mtasts.Policy{} @@ -94,14 +94,15 @@ func TestDB(t *testing.T) { t.Fatalf("records mismatch, got %#v, expected %#v", records, expRecords) } - if err := Upsert(ctxbg, dns.Domain{ASCII: "other.example.com"}, "", nil); err != nil { + if err := Upsert(ctxbg, dns.Domain{ASCII: "other.example.com"}, "", nil, ""); err != nil { t.Fatalf("upsert record: %s", err) } records, err = PolicyRecords(ctxbg) tcheckf(t, err, "policyrecords") + policyNone := mtasts.Policy{Mode: mtasts.ModeNone, MaxAgeSeconds: 5 * 60} expRecords = []PolicyRecord{ - {"other.example.com", now, now.Add(5 * 60 * time.Second), now, now, true, "", mtasts.Policy{Mode: mtasts.ModeNone, MaxAgeSeconds: 5 * 60}}, - {"example.com", now, now.Add(time.Duration(policy2.MaxAgeSeconds) * time.Second), now, now, false, "124", policy2}, + {"other.example.com", now, now.Add(5 * 60 * time.Second), now, now, true, "", policyNone, ""}, + {"example.com", now, now.Add(time.Duration(policy2.MaxAgeSeconds) * time.Second), now, now, false, "124", policy2, policy2.String()}, } if !reflect.DeepEqual(records, expRecords) { t.Fatalf("records mismatch, got %#v, expected %#v", records, expRecords) @@ -124,7 +125,7 @@ func TestDB(t *testing.T) { testGet := func(domain string, expPolicy *mtasts.Policy, expFresh bool, expErr error) { t.Helper() - p, fresh, err := Get(ctxbg, resolver, dns.Domain{ASCII: domain}) + p, _, fresh, err := Get(ctxbg, resolver, dns.Domain{ASCII: domain}) if (err == nil) != (expErr == nil) || err != nil && !errors.Is(err, expErr) { t.Fatalf("got err %v, expected %v", err, expErr) } diff --git a/mtastsdb/refresh.go b/mtastsdb/refresh.go index 219b82e..b321cc9 100644 --- a/mtastsdb/refresh.go +++ b/mtastsdb/refresh.go @@ -141,8 +141,15 @@ func refreshDomain(ctx context.Context, db *bstore.DB, resolver dns.Resolver, pr } return } - // ../rfc/8461:587 if err != nil && pr.Mode == mtasts.ModeNone { + if errors.Is(err, mtasts.ErrNoRecord) { + // Policy was in mode "none". Now it doesn't have a policy anymore. Remove from our + // database so we don't keep refreshing it. + err := db.Delete(ctx, &pr) + log.Check(err, "removing mta-sts policy with mode none, dns record is gone") + } + // Else, don't bother operator with temporary error about policy none. + // ../rfc/8461:587 return } else if err != nil { log.Errorx("looking up mta-sts record for domain", err, mlog.Field("domain", d)) diff --git a/mtastsdb/refresh_test.go b/mtastsdb/refresh_test.go index 5531b21..376c5e8 100644 --- a/mtastsdb/refresh_test.go +++ b/mtastsdb/refresh_test.go @@ -68,7 +68,7 @@ func TestRefresh(t *testing.T) { Extensions: nil, } - pr := PolicyRecord{domain, time.Time{}, validEnd, lastUpdate, lastUse, backoff, recordID, policy} + pr := PolicyRecord{domain, time.Time{}, validEnd, lastUpdate, lastUse, backoff, recordID, policy, policy.String()} if err := db.Insert(ctxbg, &pr); err != nil { t.Fatalf("insert policy: %s", err) } diff --git a/queue/direct.go b/queue/direct.go index a752654..e0fe515 100644 --- a/queue/direct.go +++ b/queue/direct.go @@ -26,6 +26,7 @@ import ( "github.com/mjl-/mox/smtp" "github.com/mjl-/mox/smtpclient" "github.com/mjl-/mox/store" + "github.com/mjl-/mox/tlsrpt" ) var ( @@ -121,7 +122,14 @@ func fail(qlog *mlog.Log, m Msg, backoff time.Duration, permanent bool, remoteMT } // Delivery by directly dialing (MX) hosts for destination domain of message. -func deliverDirect(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, ourHostname dns.Domain, transportName string, m Msg, backoff time.Duration) { +// +// The returned results are for use in a TLSRPT report, it holds success/failure +// counts and failure details for delivery/connection attempts. The +// recipientDomainResult is for policies/counts/failures about the whole recipient +// domain (MTA-STS), its policy type can be empty, in which case there is no +// information (e.g. internal failure). hostResults are per-host details (DANE, one +// per MX target). +func deliverDirect(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, ourHostname dns.Domain, transportName string, m Msg, backoff time.Duration) (recipientDomainResult tlsrpt.Result, hostResults []tlsrpt.Result) { // High-level approach: // - Resolve domain to deliver to (CNAME), and determine hosts to try to deliver to (MX) // - Get MTA-STS policy for domain (optional). If present, only deliver to its @@ -146,24 +154,40 @@ func deliverDirect(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtp ctx := context.WithValue(mox.Context, mlog.CidKey, cid) haveMX, origNextHopAuthentic, expandedNextHopAuthentic, expandedNextHop, hosts, permanent, err := smtpclient.GatherDestinations(ctx, qlog, resolver, m.RecipientDomain) if err != nil { + // If this is a DNSSEC authentication error, we'll collect it for TLS reporting. + // Hopefully it's a temporary misconfiguration that is solve before we try to send + // our report. We don't report as "dnssec-invalid", because that is defined as + // being for DANE. ../rfc/8460:580 + var errCode adns.ErrorCode + if errors.As(err, &errCode) && errCode.IsAuthentication() { + // Result: ../rfc/8460:567 + reasonCode := fmt.Sprintf("dns-extended-error-%d-%s", errCode, strings.ReplaceAll(errCode.String(), " ", "-")) + fd := tlsrpt.Details(tlsrpt.ResultValidationFailure, reasonCode) + recipientDomainResult = tlsrpt.MakeResult(tlsrpt.NoPolicyFound, origNextHop, fd) + recipientDomainResult.Summary.TotalFailureSessionCount++ + } + fail(qlog, m, backoff, permanent, dsn.NameIP{}, "", err.Error()) return } + tlsRequiredNo := m.RequireTLS != nil && !*m.RequireTLS + // Check for MTA-STS policy and enforce it if needed. // We must check at the original next-hop, i.e. recipient domain, not following any // CNAMEs. If we were to follow CNAMEs and ask for MTA-STS at that domain, it // would only take a single CNAME DNS response to direct us to an unrelated domain. - var policy *mtasts.Policy + var policy *mtasts.Policy // Policy can have mode enforce, testing and none. if !origNextHop.IsZero() { cidctx := context.WithValue(mox.Shutdown, mlog.CidKey, cid) - policy, _, err = mtastsdb.Get(cidctx, resolver, origNextHop) + policy, recipientDomainResult, _, err = mtastsdb.Get(cidctx, resolver, origNextHop) if err != nil { - if m.RequireTLS != nil && !*m.RequireTLS { + if tlsRequiredNo { qlog.Infox("mtasts lookup temporary error, continuing due to tls-required-no message header", err, mlog.Field("domain", origNextHop)) metricTLSRequiredNoIgnored.WithLabelValues("mtastspolicy").Inc() } else { qlog.Infox("mtasts lookup temporary error, aborting delivery attempt", err, mlog.Field("domain", origNextHop)) + recipientDomainResult.Summary.TotalFailureSessionCount++ fail(qlog, m, backoff, false, dsn.NameIP{}, "", err.Error()) return } @@ -187,18 +211,27 @@ func deliverDirect(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtp // todo: should make distinction between host permanently not accepting the message, and the message not being deliverable permanently. e.g. a mx host may have a size limit, or not accept 8bitmime, while another host in the list does accept the message. same for smtputf8, ../rfc/6531:555 for _, h := range hosts { // ../rfc/8461:913 - if policy != nil && !policy.Matches(h.Domain) { + if policy != nil && policy.Mode != mtasts.ModeNone && !policy.Matches(h.Domain) { + // todo: perhaps only send tlsrpt failure if none of the mx hosts matched? reporting about each mismatch seems useful for domain owners, to discover mtasts policies they didn't update after changing mx. there is a risk a domain owner intentionally didn't put all mx'es in the mtasts policy, but they probably won't mind being reported about that. + // Other error: Surprising that TLSRPT doesn't have an MTA-STS specific error code + // for this case, it's a big part of the reason to have MTA-STS. ../rfc/8460:610 + // Result: ../rfc/8460:567 todo spec: propose adding a result for this case? + fd := tlsrpt.Details(tlsrpt.ResultValidationFailure, "mtasts-policy-mx-mismatch") + fd.ReceivingMXHostname = h.Domain.ASCII + recipientDomainResult.Add(0, 0, fd) + var policyHosts []string for _, mx := range policy.MX { policyHosts = append(policyHosts, mx.LogString()) } if policy.Mode == mtasts.ModeEnforce { - if m.RequireTLS != nil && !*m.RequireTLS { + if tlsRequiredNo { qlog.Info("mx host does not match mta-sts policy in mode enforce, ignoring due to tls-required-no message header", mlog.Field("host", h.Domain), mlog.Field("policyhosts", policyHosts)) metricTLSRequiredNoIgnored.WithLabelValues("mtastsmx").Inc() } else { errmsg = fmt.Sprintf("mx host %s does not match enforced mta-sts policy with hosts %s", h.Domain, strings.Join(policyHosts, ",")) qlog.Error("mx host does not match mta-sts policy in mode enforce, skipping", mlog.Field("host", h.Domain), mlog.Field("policyhosts", policyHosts)) + recipientDomainResult.Summary.TotalFailureSessionCount++ continue } } else { @@ -211,9 +244,13 @@ func deliverDirect(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtp nqlog := qlog.WithCid(cid) var remoteIP net.IP + enforceMTASTS := policy != nil && policy.Mode == mtasts.ModeEnforce tlsMode := smtpclient.TLSOpportunistic - if policy != nil && policy.Mode == mtasts.ModeEnforce { - tlsMode = smtpclient.TLSStrictStartTLS + tlsPKIX := false + if enforceMTASTS { + tlsMode = smtpclient.TLSRequiredStartTLS + tlsPKIX = true + // note: smtpclient will still go through PKIX verification, and report about it, but not fail the connection if not passing. } // Try to deliver to host. We can get various errors back. Like permanent failure @@ -221,32 +258,43 @@ func deliverDirect(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtp // without), etc. It's a balancing act to handle these situations correctly. We // don't want to bounce unnecessarily. But also not keep trying if there is no // chance of success. + // + // deliverHost will report generic TLS and MTA-STS-specific failures in + // recipientDomainResult. If DANE is encountered, it will add a DANE reporting + // result for generic TLS and DANE-specific errors. - // Set if there TLSA records were found. Means TLS is required for this host, - // usually with verification of the certificate. - var daneRequired bool + // Set if TLSA records were found. Means TLS is required for this host, usually + // with verification of the certificate, and that we cannot fall back to + // opportunistic TLS. + var tlsDANE bool var badTLS, ok bool - enforceMTASTS := policy != nil && policy.Mode == mtasts.ModeEnforce - permanent, daneRequired, badTLS, secodeOpt, remoteIP, errmsg, ok = deliverHost(nqlog, resolver, dialer, cid, ourHostname, transportName, h, enforceMTASTS, haveMX, origNextHopAuthentic, origNextHop, expandedNextHopAuthentic, expandedNextHop, &m, tlsMode) + var hostResult tlsrpt.Result + permanent, tlsDANE, badTLS, secodeOpt, remoteIP, errmsg, hostResult, ok = deliverHost(nqlog, resolver, dialer, cid, ourHostname, transportName, h, enforceMTASTS, haveMX, origNextHopAuthentic, origNextHop, expandedNextHopAuthentic, expandedNextHop, &m, tlsMode, tlsPKIX, &recipientDomainResult) + + var zerotype tlsrpt.PolicyType + if hostResult.Policy.Type != zerotype { + hostResults = append(hostResults, hostResult) + } // If we had a TLS-related failure when doing TLS, and we don't have a requirement // for MTA-STS/DANE, we try again without TLS. This could be an old server that // only does ancient TLS versions, or has a misconfiguration. Note that // opportunistic TLS does not do regular certificate verification, so that can't be // the problem. + // ../rfc/7435:459 // We don't fall back to plain text for DMARC reports. ../rfc/7489:1768 ../rfc/7489:2683 - if !ok && badTLS && (!enforceMTASTS && tlsMode == smtpclient.TLSOpportunistic && !daneRequired && !m.IsDMARCReport || m.RequireTLS != nil && !*m.RequireTLS) { + // We queue outgoing TLS reports with tlsRequiredNo, so reports can be delivered in + // case of broken TLS. + if !ok && badTLS && (!enforceMTASTS && tlsMode == smtpclient.TLSOpportunistic && !tlsDANE && !m.IsDMARCReport || tlsRequiredNo) { metricPlaintextFallback.Inc() - if m.RequireTLS != nil && !*m.RequireTLS { + if tlsRequiredNo { metricTLSRequiredNoIgnored.WithLabelValues("badtls").Inc() } - // In case of failure with opportunistic TLS, try again without TLS. ../rfc/7435:459 // todo future: add a configuration option to not fall back? - nqlog.Info("connecting again for delivery attempt without tls", mlog.Field("enforcemtasts", enforceMTASTS), mlog.Field("danerequired", daneRequired), mlog.Field("requiretls", m.RequireTLS)) - tlsMode = smtpclient.TLSSkip - permanent, _, _, secodeOpt, remoteIP, errmsg, ok = deliverHost(nqlog, resolver, dialer, cid, ourHostname, transportName, h, enforceMTASTS, haveMX, origNextHopAuthentic, origNextHop, expandedNextHopAuthentic, expandedNextHop, &m, tlsMode) + nqlog.Info("connecting again for delivery attempt without tls", mlog.Field("enforcemtasts", enforceMTASTS), mlog.Field("tlsdane", tlsDANE), mlog.Field("requiretls", m.RequireTLS)) + permanent, _, _, secodeOpt, remoteIP, errmsg, _, ok = deliverHost(nqlog, resolver, dialer, cid, ourHostname, transportName, h, enforceMTASTS, haveMX, origNextHopAuthentic, origNextHop, expandedNextHopAuthentic, expandedNextHop, &m, smtpclient.TLSSkip, false, &tlsrpt.Result{}) } if ok { @@ -283,33 +331,48 @@ func deliverDirect(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtp } fail(qlog, m, backoff, permanent, remoteMTA, secodeOpt, errmsg) + return } -// deliverHost attempts to deliver m to host. Depending on tlsMode, we'll do -// required TLS with WebPKI verification (with MTA-STS), opportunistic DANE TLS -// (opportunistic TLS), non-verifying TLS (opportunistic TLS) or skip TLS -// altogether due to previous TLS errors. +// deliverHost attempts to deliver m to host. Depending on tlsMode we'll do +// opportunistic or required STARTTLS or skip TLS entirely. Based on tlsPKIX we do +// PKIX/WebPKI verification (for MTA-STS). If we encounter DANE records, we verify +// those. If the message has a message header "TLS-Required: No", we ignore TLS +// verification errors. // // deliverHost updates m.DialedIPs, which must be saved in case of failure to // deliver. // -// With TLS-Required no header, we ignore verification failures and continue -// delivering. -// // The haveMX and next-hop-authentic fields are used to determine if DANE is // applicable. The next-hop fields themselves are used to determine valid names // during DANE TLS certificate verification. -func deliverHost(log *mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, cid int64, ourHostname dns.Domain, transportName string, host dns.IPDomain, enforceMTASTS, haveMX, origNextHopAuthentic bool, origNextHop dns.Domain, expandedNextHopAuthentic bool, expandedNextHop dns.Domain, m *Msg, tlsMode smtpclient.TLSMode) (permanent, daneRequired, badTLS bool, secodeOpt string, remoteIP net.IP, errmsg string, ok bool) { +// +// The returned hostResult holds TLSRPT reporting results for the connection +// attempt. Its policy type can be the zero value, indicating there was no finding +// (e.g. internal error). +func deliverHost(log *mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, cid int64, ourHostname dns.Domain, transportName string, host dns.IPDomain, enforceMTASTS, haveMX, origNextHopAuthentic bool, origNextHop dns.Domain, expandedNextHopAuthentic bool, expandedNextHop dns.Domain, m *Msg, tlsMode smtpclient.TLSMode, tlsPKIX bool, recipientDomainResult *tlsrpt.Result) (permanent, tlsDANE, badTLS bool, secodeOpt string, remoteIP net.IP, errmsg string, hostResult tlsrpt.Result, ok bool) { // About attempting delivery to multiple addresses of a host: ../rfc/5321:3898 + tlsRequiredNo := m.RequireTLS != nil && !*m.RequireTLS + start := time.Now() var deliveryResult string defer func() { - metricDelivery.WithLabelValues(fmt.Sprintf("%d", m.Attempts), transportName, string(tlsMode), deliveryResult).Observe(float64(time.Since(start)) / float64(time.Second)) + mode := string(tlsMode) + if tlsPKIX { + mode += "+mtasts" + } + if tlsDANE { + mode += "+dane" + } + metricDelivery.WithLabelValues(fmt.Sprintf("%d", m.Attempts), transportName, mode, deliveryResult).Observe(float64(time.Since(start)) / float64(time.Second)) log.Debug("queue deliverhost result", mlog.Field("host", host), mlog.Field("attempt", m.Attempts), mlog.Field("tlsmode", tlsMode), + mlog.Field("tlspkix", tlsPKIX), + mlog.Field("tlsdane", tlsDANE), + mlog.Field("tlsrequiredno", tlsRequiredNo), mlog.Field("permanent", permanent), mlog.Field("badtls", badTLS), mlog.Field("secodeopt", secodeOpt), @@ -321,7 +384,7 @@ func deliverHost(log *mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, // Open message to deliver. f, err := os.Open(m.MessagePath()) if err != nil { - return false, false, false, "", nil, fmt.Sprintf("open message file: %s", err), false + return false, false, false, "", nil, fmt.Sprintf("open message file: %s", err), hostResult, false } msgr := store.FileMsgReader(m.MsgPrefix, f) defer func() { @@ -338,79 +401,122 @@ func deliverHost(log *mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, // name servers returning an error for TLSA requests or letting it timeout (not // sending a response). ../rfc/7672:879 var daneRecords []adns.TLSA - var tlsRemoteHostnames []dns.Domain + var tlsHostnames []dns.Domain if host.IsDomain() { - tlsRemoteHostnames = []dns.Domain{host.Domain} + tlsHostnames = []dns.Domain{host.Domain} } if m.DialedIPs == nil { m.DialedIPs = map[string][]net.IP{} } + + countResultFailure := func() { + recipientDomainResult.Summary.TotalFailureSessionCount++ + hostResult.Summary.TotalFailureSessionCount++ + } + metricDestinations.Inc() authentic, expandedAuthentic, expandedHost, ips, dualstack, err := smtpclient.GatherIPs(ctx, log, resolver, host, m.DialedIPs) - if err == nil && authentic && origNextHopAuthentic && (!haveMX || expandedNextHopAuthentic) && host.IsDomain() { + destAuthentic := err == nil && authentic && origNextHopAuthentic && (!haveMX || expandedNextHopAuthentic) && host.IsDomain() + if !destAuthentic { + log.Debugx("not attempting verification with dane", err, mlog.Field("authentic", authentic), mlog.Field("expandedauthentic", expandedAuthentic)) + + // Track a DNSSEC error if found. + var errCode adns.ErrorCode + if err != nil { + if errors.As(err, &errCode) && errCode.IsAuthentication() { + // Result: ../rfc/8460:567 + reasonCode := fmt.Sprintf("dns-extended-error-%d-%s", errCode, strings.ReplaceAll(errCode.String(), " ", "-")) + fd := tlsrpt.Details(tlsrpt.ResultValidationFailure, reasonCode) + hostResult = tlsrpt.MakeResult(tlsrpt.TLSA, host.Domain, fd) + countResultFailure() + } + } else { + // todo: we could lookup tlsa records, and log an error when they are not dnssec-signed. this should be interpreted simply as "not doing dane", but it could be useful to warn domain owners about, they may be under the impression they are dane-protected. + hostResult = tlsrpt.MakeResult(tlsrpt.NoPolicyFound, host.Domain) + } + } else if tlsMode == smtpclient.TLSSkip { metricDestinationsAuthentic.Inc() - switch tlsMode { - case smtpclient.TLSSkip: - // No TLS, so clearly no DANE. This can happen if we've dialed TLS before but a TLS - // connection couldn't be established. - case smtpclient.TLSUnverifiedStartTLS: - // Fallback mode for DANE without usable records, so skip DANE. - // We shouldn't be able to get here, but no harm handling it. - default: - // Look for TLSA records in either the expandedHost, or otherwise the original - // host. ../rfc/7672:912 - var tlsaBaseDomain dns.Domain - daneRequired, daneRecords, tlsaBaseDomain, err = smtpclient.GatherTLSA(ctx, log, resolver, host.Domain, expandedNextHopAuthentic && expandedAuthentic, expandedHost) - if daneRequired { - metricDestinationDANERequired.Inc() - } - if err != nil { - metricDestinationDANEGatherTLSAErrors.Inc() - } - if err == nil && daneRequired { - tlsMode = smtpclient.TLSStrictStartTLS - if len(daneRecords) == 0 { - // If there are no usable DANE records, we still have to use TLS, but without - // verifying its certificate. At least when there is no MTA-STS. Why? Perhaps to - // prevent ossification? The SMTP TLSA specification has different behaviour than - // the generic TLSA. "Usable" means different things in different places. - // ../rfc/7672:718 ../rfc/6698:1845 ../rfc/6698:660 - if !enforceMTASTS { - tlsMode = smtpclient.TLSUnverifiedStartTLS - log.Debug("no usable dane records, not verifying dane records, but doing required non-verifying opportunistic tls") - metricDestinationDANESTARTTLSUnverified.Inc() - } - daneRecords = nil - } else { - // Based on CNAMEs followed and DNSSEC-secure status, we must allow up to 4 host - // names. - tlsRemoteHostnames = smtpclient.GatherTLSANames(haveMX, expandedNextHopAuthentic, expandedAuthentic, origNextHop, expandedNextHop, host.Domain, tlsaBaseDomain) - log.Debug("delivery with required starttls with dane verification", mlog.Field("allowedtlshostnames", tlsRemoteHostnames)) - } - } else if !daneRequired { - log.Debugx("not doing opportunistic dane after gathering tlsa records", err) - err = nil - } else if err != nil && m.RequireTLS != nil && !*m.RequireTLS { - log.Debugx("error gathering dane tlsa records with dane required, but continuing without validation due to tls-required-no message header", err) + // TLSSkip is used to fallback to plaintext, which is used with a TLS-Required: No + // header to ignore the recipient domain's DANE policy. + + // possible err is propagated to below. + } else { + metricDestinationsAuthentic.Inc() + + // Look for TLSA records in either the expandedHost, or otherwise the original + // host. ../rfc/7672:912 + var tlsaBaseDomain dns.Domain + tlsDANE, daneRecords, tlsaBaseDomain, err = smtpclient.GatherTLSA(ctx, log, resolver, host.Domain, expandedNextHopAuthentic && expandedAuthentic, expandedHost) + if tlsDANE { + metricDestinationDANERequired.Inc() + } + if err != nil { + metricDestinationDANEGatherTLSAErrors.Inc() + } + if err == nil && tlsDANE { + tlsMode = smtpclient.TLSRequiredStartTLS + hostResult = tlsrpt.Result{Policy: tlsrpt.TLSAPolicy(daneRecords, tlsaBaseDomain)} + if len(daneRecords) == 0 { + // If there are no usable DANE records, we still have to use TLS, but without + // verifying its certificate. At least when there is no MTA-STS. Why? Perhaps to + // prevent ossification? The SMTP TLSA specification has different behaviour than + // the generic TLSA. "Usable" means different things in different places. + // ../rfc/7672:718 ../rfc/6698:1845 ../rfc/6698:660 + log.Debug("no usable dane records, requiring starttls but not verifying with dane") + metricDestinationDANESTARTTLSUnverified.Inc() daneRecords = nil + // Result: ../rfc/8460:576 (this isn't technicall invalid, only all-unusable...) + hostResult.FailureDetails = []tlsrpt.FailureDetails{ + { + ResultType: tlsrpt.ResultTLSAInvalid, + ReceivingMXHostname: host.XString(false), + FailureReasonCode: "all-unusable-records+ignored", + }, + } + } else { + log.Debug("delivery with required starttls with dane verification", mlog.Field("allowedtlshostnames", tlsHostnames)) + } + // Based on CNAMEs followed and DNSSEC-secure status, we must allow up to 4 host + // names. + tlsHostnames = smtpclient.GatherTLSANames(haveMX, expandedNextHopAuthentic, expandedAuthentic, origNextHop, expandedNextHop, host.Domain, tlsaBaseDomain) + } else if !tlsDANE { + log.Debugx("not doing opportunistic dane after gathering tlsa records", err) + err = nil + hostResult = tlsrpt.MakeResult(tlsrpt.NoPolicyFound, tlsaBaseDomain) + } else if err != nil { + fd := tlsrpt.Details(tlsrpt.ResultTLSAInvalid, "") + var errCode adns.ErrorCode + if errors.As(err, &errCode) { + fd.FailureReasonCode = fmt.Sprintf("extended-dns-error-%d-%s", errCode, strings.ReplaceAll(errCode.String(), " ", "-")) + if errCode.IsAuthentication() { + // Result: ../rfc/8460:580 + fd.ResultType = tlsrpt.ResultDNSSECInvalid + countResultFailure() + } + } + hostResult = tlsrpt.Result{ + Policy: tlsrpt.TLSAPolicy(daneRecords, tlsaBaseDomain), + FailureDetails: []tlsrpt.FailureDetails{fd}, + } + + if tlsRequiredNo { + log.Debugx("error gathering dane tlsa records with dane required, but continuing without validation due to tls-required-no message header", err) err = nil metricTLSRequiredNoIgnored.WithLabelValues("badtlsa").Inc() } - // else, err is propagated below. } - } else { - log.Debugx("not attempting verification with dane", err, mlog.Field("authentic", authentic), mlog.Field("expandedauthentic", expandedAuthentic)) + // else, err is propagated below. } // todo: for requiretls, should an MTA-STS policy in mode testing be treated as good enough for requiretls? let's be strict and assume not. // todo: ../rfc/8689:276 seems to specify stricter requirements on name in certificate than DANE (which allows original recipient domain name and cname-expanded name, and hints at following CNAME for MX targets as well, allowing both their original and expanded names too). perhaps the intent was just to say the name must be validated according to the relevant specifications? // todo: for requiretls, should we allow no usable dane records with requiretls? dane allows it, but doesn't seem in spirit of requiretls, so not allowing it. - if err == nil && m.RequireTLS != nil && *m.RequireTLS && !(daneRequired && len(daneRecords) > 0) && !enforceMTASTS { + if err == nil && m.RequireTLS != nil && *m.RequireTLS && !(tlsDANE && len(daneRecords) > 0) && !enforceMTASTS { log.Info("verified tls is required, but destination has no usable dane records and no mta-sts policy, canceling delivery attempt to host") metricRequireTLSUnsupported.WithLabelValues("nopolicy").Inc() // Resond with proper enhanced status code. ../rfc/8689:301 - return false, daneRequired, false, smtp.SePol7MissingReqTLS, remoteIP, "missing required tls verification mechanism", false + return false, tlsDANE, false, smtp.SePol7MissingReqTLS, remoteIP, "missing required tls verification mechanism", hostResult, false } // Dial the remote host given the IPs if no error yet. @@ -438,7 +544,7 @@ func deliverHost(log *mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, metricConnection.WithLabelValues(result).Inc() if err != nil { log.Debugx("connecting to remote smtp", err, mlog.Field("host", host)) - return false, daneRequired, false, "", remoteIP, fmt.Sprintf("dialing smtp server: %v", err), false + return false, tlsDANE, false, "", remoteIP, fmt.Sprintf("dialing smtp server: %v", err), hostResult, false } var mailFrom string @@ -456,16 +562,22 @@ func deliverHost(log *mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, // Initialize SMTP session, sending EHLO/HELO and STARTTLS with specified tls mode. var firstHost dns.Domain var moreHosts []dns.Domain - if len(tlsRemoteHostnames) > 0 { + if len(tlsHostnames) > 0 { // For use with DANE-TA. - firstHost = tlsRemoteHostnames[0] - moreHosts = tlsRemoteHostnames[1:] + firstHost = tlsHostnames[0] + moreHosts = tlsHostnames[1:] } var verifiedRecord adns.TLSA - if m.RequireTLS != nil && !*m.RequireTLS && tlsMode != smtpclient.TLSSkip { - tlsMode = smtpclient.TLSUnverifiedStartTLS + opts := smtpclient.Opts{ + IgnoreTLSVerifyErrors: tlsRequiredNo, + RootCAs: mox.Conf.Static.TLS.CertPool, + DANERecords: daneRecords, + DANEMoreHostnames: moreHosts, + DANEVerifiedRecord: &verifiedRecord, + RecipientDomainResult: recipientDomainResult, + HostResult: &hostResult, } - sc, err := smtpclient.New(ctx, log, conn, tlsMode, ourHostname, firstHost, nil, daneRecords, moreHosts, &verifiedRecord) + sc, err := smtpclient.New(ctx, log, conn, tlsMode, tlsPKIX, ourHostname, firstHost, opts) defer func() { if sc == nil { conn.Close() @@ -523,7 +635,7 @@ func deliverHost(log *mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, deliveryResult = "error" } if err == nil { - return false, daneRequired, false, "", remoteIP, "", true + return false, tlsDANE, false, "", remoteIP, "", hostResult, true } else if cerr, ok := err.(smtpclient.Error); ok { // If we are being rejected due to policy reasons on the first // attempt and remote has both IPv4 and IPv6, we'll give it @@ -539,9 +651,9 @@ func deliverHost(log *mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, secode = smtp.SePol7MissingReqTLS metricRequireTLSUnsupported.WithLabelValues("norequiretls").Inc() } - return permanent, daneRequired, errors.Is(cerr, smtpclient.ErrTLS), secode, remoteIP, cerr.Error(), false + return permanent, tlsDANE, errors.Is(cerr, smtpclient.ErrTLS), secode, remoteIP, cerr.Error(), hostResult, false } else { - return false, daneRequired, errors.Is(cerr, smtpclient.ErrTLS), "", remoteIP, err.Error(), false + return false, tlsDANE, errors.Is(cerr, smtpclient.ErrTLS), "", remoteIP, err.Error(), hostResult, false } } diff --git a/queue/queue.go b/queue/queue.go index f615cb0..66142c3 100644 --- a/queue/queue.go +++ b/queue/queue.go @@ -32,6 +32,8 @@ import ( "github.com/mjl-/mox/smtp" "github.com/mjl-/mox/smtpclient" "github.com/mjl-/mox/store" + "github.com/mjl-/mox/tlsrpt" + "github.com/mjl-/mox/tlsrptdb" ) var xlog = mlog.New("queue") @@ -55,7 +57,7 @@ var ( []string{ "attempt", // Number of attempts. "transport", // empty for default direct delivery. - "tlsmode", // strict, opportunistic, skip + "tlsmode", // immediate, requiredstarttls, opportunistic, skip (from smtpclient.TLSMode), with optional +mtasts and/or +dane. "result", // ok, timeout, canceled, temperror, permerror, error }, ) @@ -580,12 +582,71 @@ func deliver(resolver dns.Resolver, m Msg) { qlog.Debug("delivering with transport", mlog.Field("transport", transportName)) } + // We gather TLS connection successes and failures during delivery, and we store + // them in tlsrptb. Every 24 hours we send an email with a report to the recipient + // domains that opt in via a TLSRPT DNS record. For us, the tricky part is + // collecting all reporting information. We've got several TLS modes + // (opportunistic, DANE and/or MTA-STS (PKIX), overrides due to Require TLS). + // Failures can happen at various levels: MTA-STS policies (apply to whole delivery + // attempt/domain), MX targets (possibly multiple per delivery attempt, both for + // MTA-STS and DANE). + // + // Once the SMTP client has tried a TLS handshake, we register success/failure, + // regardless of what happens next on the connection. We also register failures + // when they happen before we get to the SMTP client, but only if they are related + // to TLS (and some DNSSEC). + var recipientDomainResult tlsrpt.Result + var hostResults []tlsrpt.Result + defer func() { + if mox.Conf.Static.NoOutgoingTLSReports || !m.RecipientDomain.IsDomain() { + return + } + + now := time.Now() + dayUTC := now.UTC().Format("20060102") + + results := make([]tlsrptdb.TLSResult, 0, 1+len(hostResults)) + addResult := func(r tlsrpt.Result, isHost bool) { + var zerotype tlsrpt.PolicyType + if r.Policy.Type == zerotype { + return + } + + // Ensure we store policy domain in unicode in database. + policyDomain, err := dns.ParseDomain(r.Policy.Domain) + if err != nil { + qlog.Errorx("parsing policy domain for tls result", err, mlog.Field("policydomain", r.Policy.Domain)) + return + } + + tlsResult := tlsrptdb.TLSResult{ + PolicyDomain: policyDomain.Name(), + DayUTC: dayUTC, + RecipientDomain: m.RecipientDomain.Domain.Name(), + IsHost: isHost, + SendReport: !m.IsTLSReport, + Results: []tlsrpt.Result{r}, + } + results = append(results, tlsResult) + } + addResult(recipientDomainResult, false) + for _, result := range hostResults { + addResult(result, true) + } + + if len(results) > 0 { + err := tlsrptdb.AddTLSResults(context.Background(), results) + qlog.Check(err, "adding tls results to database for upcoming tlsrpt report") + } + }() + var dialer smtpclient.Dialer = &net.Dialer{} if transport.Submissions != nil { deliverSubmit(cid, qlog, resolver, dialer, m, backoff, transportName, transport.Submissions, true, 465) } else if transport.Submission != nil { deliverSubmit(cid, qlog, resolver, dialer, m, backoff, transportName, transport.Submission, false, 587) } else if transport.SMTP != nil { + // todo future: perhaps also gather tlsrpt results for submissions. deliverSubmit(cid, qlog, resolver, dialer, m, backoff, transportName, transport.SMTP, false, 25) } else { ourHostname := mox.Conf.Static.HostnameDomain @@ -602,7 +663,7 @@ func deliver(resolver dns.Resolver, m Msg) { } ourHostname = transport.Socks.Hostname } - deliverDirect(cid, qlog, resolver, dialer, ourHostname, transportName, m, backoff) + recipientDomainResult, hostResults = deliverDirect(cid, qlog, resolver, dialer, ourHostname, transportName, m, backoff) } } diff --git a/queue/queue_test.go b/queue/queue_test.go index 02906cb..243de0a 100644 --- a/queue/queue_test.go +++ b/queue/queue_test.go @@ -27,6 +27,8 @@ import ( "github.com/mjl-/mox/smtp" "github.com/mjl-/mox/smtpclient" "github.com/mjl-/mox/store" + "github.com/mjl-/mox/tlsrpt" + "github.com/mjl-/mox/tlsrptdb" ) var ctxbg = context.Background() @@ -150,14 +152,16 @@ func TestQueue(t *testing.T) { t.Fatalf("launchWork launched %d deliveries, expected 0", nn) } - // Override dial function. We'll make connecting fail for now. + mailDomain := dns.Domain{ASCII: "mox.example"} + mailHost := dns.Domain{ASCII: "mail.mox.example"} resolver := dns.MockResolver{ A: map[string][]string{ - "mox.example.": {"127.0.0.1"}, + "mail.mox.example.": {"127.0.0.1"}, "submission.example.": {"127.0.0.1"}, }, - MX: map[string][]*net.MX{"mox.example.": {{Host: "mox.example", Pref: 10}}}, + MX: map[string][]*net.MX{"mox.example.": {{Host: "mail.mox.example", Pref: 10}}}, } + // Override dial function. We'll make connecting fail for now. dialed := make(chan struct{}, 1) smtpclient.DialHook = func(ctx context.Context, dialer smtpclient.Dialer, timeout time.Duration, addr string, laddr net.Addr) (net.Conn, error) { dialed <- struct{}{} @@ -169,7 +173,7 @@ func TestQueue(t *testing.T) { launchWork(resolver, map[string]struct{}{}) - moxCert := fakeCert(t, "mox.example", false) + moxCert := fakeCert(t, "mail.mox.example", false) // Wait until we see the dial and the failed attempt. timer := time.NewTimer(time.Second) @@ -226,7 +230,7 @@ func TestQueue(t *testing.T) { }() // We do a minimal fake smtp server. We cannot import smtpserver.Serve due to cyclic dependencies. - fmt.Fprintf(server, "220 mox.example\r\n") + fmt.Fprintf(server, "220 mail.mox.example\r\n") br := bufio.NewReader(server) readline := func(cmd string) { @@ -240,7 +244,7 @@ func TestQueue(t *testing.T) { } readline("ehlo") - writeline("250 mox.example") + writeline("250 mail.mox.example") readline("mail") writeline("250 ok") readline("rcpt") @@ -265,7 +269,7 @@ func TestQueue(t *testing.T) { attempt++ // We do a minimal fake smtp server. We cannot import smtpserver.Serve due to cyclic dependencies. - fmt.Fprintf(server, "220 mox.example\r\n") + fmt.Fprintf(server, "220 mail.mox.example\r\n") br := bufio.NewReader(server) readline := func(cmd string) { @@ -279,7 +283,7 @@ func TestQueue(t *testing.T) { } readline("ehlo") - writeline("250-mox.example") + writeline("250-mail.mox.example") writeline("250 starttls") if nstarttls == 0 || attempt <= nstarttls { readline("starttls") @@ -294,10 +298,10 @@ func TestQueue(t *testing.T) { readline("ehlo") if requiretls { - writeline("250-mox.example") + writeline("250-mail.mox.example") writeline("250 requiretls") } else { - writeline("250 mox.example") + writeline("250 mail.mox.example") } } readline("mail") @@ -325,7 +329,7 @@ func TestQueue(t *testing.T) { }() // We do a minimal fake smtp server. We cannot import smtpserver.Serve due to cyclic dependencies. - fmt.Fprintf(server, "220 mox.example\r\n") + fmt.Fprintf(server, "220 mail.mox.example\r\n") br := bufio.NewReader(server) br.ReadString('\n') // Should be EHLO. fmt.Fprintf(server, "250-localhost\r\n") @@ -477,6 +481,30 @@ func TestQueue(t *testing.T) { t.Fatalf("expected net.Dialer as dialer") } + // Various failure reasons. + fdNotTrusted := tlsrpt.FailureDetails{ + ResultType: tlsrpt.ResultCertificateNotTrusted, + SendingMTAIP: "", // Missing due to pipe. + ReceivingMXHostname: "mail.mox.example", + ReceivingMXHelo: "mail.mox.example", + ReceivingIP: "", // Missing due to pipe. + FailedSessionCount: 1, + FailureReasonCode: "", + } + fdTLSAUnusable := tlsrpt.FailureDetails{ + ResultType: tlsrpt.ResultTLSAInvalid, + ReceivingMXHostname: "mail.mox.example", + FailedSessionCount: 0, + FailureReasonCode: "all-unusable-records+ignored", + } + fdBadProtocol := tlsrpt.FailureDetails{ + ResultType: tlsrpt.ResultValidationFailure, + ReceivingMXHostname: "mail.mox.example", + ReceivingMXHelo: "mail.mox.example", + FailedSessionCount: 1, + FailureReasonCode: "tls-remote-alert-70-protocol-version-not-supported", + } + // Add a message to be delivered with socks. qm = MakeMsg("mjl", path, path, false, false, int64(len(testmsg)), "", nil, nil) err = Add(ctxbg, xlog, &qm, mf) @@ -493,6 +521,7 @@ func TestQueue(t *testing.T) { } // Add message to be delivered with opportunistic TLS verification. + clearTLSResults(t) qm = MakeMsg("mjl", path, path, false, false, int64(len(testmsg)), "", nil, nil) err = Add(ctxbg, xlog, &qm, mf) tcheck(t, err, "add message to queue for delivery") @@ -502,8 +531,11 @@ func TestQueue(t *testing.T) { t.Fatalf("kick changed %d messages, expected 1", n) } testDeliver(fakeSMTPSTARTTLSServer) + checkTLSResults(t, "mox.example", "mox.example", false, addCounts(1, 0, tlsrpt.MakeResult(tlsrpt.NoPolicyFound, mailDomain, fdNotTrusted))) + checkTLSResults(t, "mail.mox.example", "mox.example", true, addCounts(1, 0, tlsrpt.MakeResult(tlsrpt.NoPolicyFound, mailHost))) // Test fallback to plain text with TLS handshake fails. + clearTLSResults(t) qm = MakeMsg("mjl", path, path, false, false, int64(len(testmsg)), "", nil, nil) err = Add(ctxbg, xlog, &qm, mf) tcheck(t, err, "add message to queue for delivery") @@ -513,11 +545,14 @@ func TestQueue(t *testing.T) { t.Fatalf("kick changed %d messages, expected 1", n) } testDeliver(makeBadFakeSMTPSTARTTLSServer(true)) + checkTLSResults(t, "mox.example", "mox.example", false, addCounts(0, 1, tlsrpt.MakeResult(tlsrpt.NoPolicyFound, mailDomain, fdBadProtocol))) + checkTLSResults(t, "mail.mox.example", "mox.example", true, addCounts(0, 1, tlsrpt.MakeResult(tlsrpt.NoPolicyFound, mailHost, fdBadProtocol))) // Add message to be delivered with DANE verification. + clearTLSResults(t) resolver.AllAuthentic = true resolver.TLSA = map[string][]adns.TLSA{ - "_25._tcp.mox.example.": { + "_25._tcp.mail.mox.example.": { {Usage: adns.TLSAUsageDANEEE, Selector: adns.TLSASelectorSPKI, MatchType: adns.TLSAMatchTypeFull, CertAssoc: moxCert.Leaf.RawSubjectPublicKeyInfo}, }, } @@ -530,6 +565,8 @@ func TestQueue(t *testing.T) { t.Fatalf("kick changed %d messages, expected 1", n) } testDeliver(fakeSMTPSTARTTLSServer) + checkTLSResults(t, "mox.example", "mox.example", false, addCounts(1, 0, tlsrpt.MakeResult(tlsrpt.NoPolicyFound, mailDomain, fdNotTrusted))) + checkTLSResults(t, "mail.mox.example", "mox.example", true, addCounts(1, 0, tlsrpt.Result{Policy: tlsrpt.TLSAPolicy(resolver.TLSA["_25._tcp.mail.mox.example."], mailHost), FailureDetails: []tlsrpt.FailureDetails{}})) // We should know starttls/requiretls by now. rdt := store.RecipientDomainTLS{Domain: "mox.example"} @@ -551,8 +588,9 @@ func TestQueue(t *testing.T) { testDeliver(fakeSMTPSTARTTLSServer) // Check that message is delivered with all unusable DANE records. + clearTLSResults(t) resolver.TLSA = map[string][]adns.TLSA{ - "_25._tcp.mox.example.": { + "_25._tcp.mail.mox.example.": { {}, }, } @@ -565,12 +603,15 @@ func TestQueue(t *testing.T) { t.Fatalf("kick changed %d messages, expected 1", n) } testDeliver(fakeSMTPSTARTTLSServer) + checkTLSResults(t, "mox.example", "mox.example", false, addCounts(1, 0, tlsrpt.MakeResult(tlsrpt.NoPolicyFound, mailDomain, fdNotTrusted))) + checkTLSResults(t, "mail.mox.example", "mox.example", true, addCounts(1, 0, tlsrpt.Result{Policy: tlsrpt.TLSAPolicy([]adns.TLSA{}, mailHost), FailureDetails: []tlsrpt.FailureDetails{fdTLSAUnusable}})) // Check that message is delivered with insecure TLSA records. They should be // ignored and regular STARTTLS tried. - resolver.Inauthentic = []string{"tlsa _25._tcp.mox.example."} + clearTLSResults(t) + resolver.Inauthentic = []string{"tlsa _25._tcp.mail.mox.example."} resolver.TLSA = map[string][]adns.TLSA{ - "_25._tcp.mox.example.": { + "_25._tcp.mail.mox.example.": { {Usage: adns.TLSAUsageDANEEE, Selector: adns.TLSASelectorSPKI, MatchType: adns.TLSAMatchTypeFull, CertAssoc: make([]byte, sha256.Size)}, }, } @@ -584,6 +625,8 @@ func TestQueue(t *testing.T) { } testDeliver(makeBadFakeSMTPSTARTTLSServer(true)) resolver.Inauthentic = nil + checkTLSResults(t, "mox.example", "mox.example", false, addCounts(0, 1, tlsrpt.MakeResult(tlsrpt.NoPolicyFound, mailDomain, fdBadProtocol))) + checkTLSResults(t, "mail.mox.example", "mox.example", true, addCounts(0, 1, tlsrpt.MakeResult(tlsrpt.NoPolicyFound, mailHost, fdBadProtocol))) // STARTTLS failed, so not known supported. rdt = store.RecipientDomainTLS{Domain: "mox.example"} @@ -669,8 +712,8 @@ func TestQueue(t *testing.T) { } } - conn2, cleanup2 := prepServer(func(conn net.Conn) { fmt.Fprintf(conn, "220 mox.example\r\n") }) - conn3, cleanup3 := prepServer(func(conn net.Conn) { fmt.Fprintf(conn, "451 mox.example\r\n") }) + conn2, cleanup2 := prepServer(func(conn net.Conn) { fmt.Fprintf(conn, "220 mail.mox.example\r\n") }) + conn3, cleanup3 := prepServer(func(conn net.Conn) { fmt.Fprintf(conn, "451 mail.mox.example\r\n") }) conn4, cleanup4 := prepServer(fakeSMTPSTARTTLSServer) defer func() { cleanup2() @@ -704,7 +747,7 @@ func TestQueue(t *testing.T) { if i == 4 { resolver.AllAuthentic = true resolver.TLSA = map[string][]adns.TLSA{ - "_25._tcp.mox.example.": { + "_25._tcp.mail.mox.example.": { // Non-matching zero CertAssoc, should cause failure. {Usage: adns.TLSAUsageDANEEE, Selector: adns.TLSASelectorSPKI, MatchType: adns.TLSAMatchTypeSHA256, CertAssoc: make([]byte, sha256.Size)}, }, @@ -755,6 +798,37 @@ func TestQueue(t *testing.T) { } } +func addCounts(success, failure int64, result tlsrpt.Result) tlsrpt.Result { + result.Summary.TotalSuccessfulSessionCount += success + result.Summary.TotalFailureSessionCount += failure + return result +} + +func clearTLSResults(t *testing.T) { + _, err := bstore.QueryDB[tlsrptdb.TLSResult](ctxbg, tlsrptdb.ResultDB).Delete() + tcheck(t, err, "delete tls results") +} + +func checkTLSResults(t *testing.T, policyDomain, expRecipientDomain string, expIsHost bool, expResults ...tlsrpt.Result) { + t.Helper() + q := bstore.QueryDB[tlsrptdb.TLSResult](ctxbg, tlsrptdb.ResultDB) + q.FilterNonzero(tlsrptdb.TLSResult{PolicyDomain: policyDomain}) + result, err := q.Get() + tcheck(t, err, "get tls result") + tcompare(t, result.RecipientDomain, expRecipientDomain) + tcompare(t, result.IsHost, expIsHost) + + // Before comparing, compensate for go1.20 vs go1.21 difference. + for i, r := range result.Results { + for j, fd := range r.FailureDetails { + if fd.FailureReasonCode == "tls-remote-alert-70" { + result.Results[i].FailureDetails[j].FailureReasonCode = "tls-remote-alert-70-protocol-version-not-supported" + } + } + } + tcompare(t, result.Results, expResults) +} + // test Start and that it attempts to deliver. func TestQueueStart(t *testing.T) { // Override dial function. We'll make connecting fail and check the attempt. diff --git a/queue/submit.go b/queue/submit.go index 2c78e53..66f53d0 100644 --- a/queue/submit.go +++ b/queue/submit.go @@ -33,13 +33,16 @@ func deliverSubmit(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtp port = defaultPort } - tlsMode := smtpclient.TLSStrictStartTLS + tlsMode := smtpclient.TLSRequiredStartTLS + tlsPKIX := true if dialTLS { - tlsMode = smtpclient.TLSStrictImmediate + tlsMode = smtpclient.TLSImmediate } else if transport.STARTTLSInsecureSkipVerify { tlsMode = smtpclient.TLSOpportunistic + tlsPKIX = false } else if transport.NoSTARTTLS { tlsMode = smtpclient.TLSSkip + tlsPKIX = false } start := time.Now() var deliveryResult string @@ -60,7 +63,7 @@ func deliverSubmit(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtp // If submit was done with REQUIRETLS extension for SMTP, we must verify TLS // certificates. If our submission connection is not configured that way, abort. requireTLS := m.RequireTLS != nil && *m.RequireTLS - if requireTLS && tlsMode != smtpclient.TLSStrictStartTLS && tlsMode != smtpclient.TLSStrictImmediate { + if requireTLS && (tlsMode != smtpclient.TLSRequiredStartTLS && tlsMode != smtpclient.TLSImmediate || !tlsPKIX) { errmsg = fmt.Sprintf("transport %s: message requires verified tls but transport does not verify tls", transportName) fail(qlog, m, backoff, true, dsn.NameIP{}, smtp.SePol7MissingReqTLS, errmsg) return @@ -128,7 +131,11 @@ func deliverSubmit(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtp } clientctx, clientcancel := context.WithTimeout(context.Background(), 60*time.Second) defer clientcancel() - client, err := smtpclient.New(clientctx, qlog, conn, tlsMode, mox.Conf.Static.HostnameDomain, transport.DNSHost, auth, nil, nil, nil) + opts := smtpclient.Opts{ + Auth: auth, + RootCAs: mox.Conf.Static.TLS.CertPool, + } + client, err := smtpclient.New(clientctx, qlog, conn, tlsMode, tlsPKIX, mox.Conf.Static.HostnameDomain, transport.DNSHost, opts) if err != nil { smtperr, ok := err.(smtpclient.Error) var remoteMTA dsn.NameIP diff --git a/quickstart.go b/quickstart.go index 6f1c797..ef64880 100644 --- a/quickstart.go +++ b/quickstart.go @@ -734,6 +734,9 @@ and check the admin page for the needed DNS records.`) } sc.Postmaster.Account = accountName sc.Postmaster.Mailbox = "Postmaster" + sc.HostTLSRPT.Account = accountName + sc.HostTLSRPT.Localpart = "tls-reports" + sc.HostTLSRPT.Mailbox = "TLSRPT" mox.ConfigStaticPath = filepath.FromSlash("config/mox.conf") mox.ConfigDynamicPath = filepath.FromSlash("config/domains.conf") diff --git a/sendmail.go b/sendmail.go index 49d4c40..3adb5db 100644 --- a/sendmail.go +++ b/sendmail.go @@ -18,6 +18,7 @@ import ( "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/sasl" "github.com/mjl-/mox/smtp" "github.com/mjl-/mox/smtpclient" @@ -274,10 +275,13 @@ binary should be setgid that group: ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() tlsMode := smtpclient.TLSSkip + tlsPKIX := false if submitconf.TLS { - tlsMode = smtpclient.TLSStrictImmediate + tlsMode = smtpclient.TLSImmediate + tlsPKIX = true } else if submitconf.STARTTLS { - tlsMode = smtpclient.TLSStrictStartTLS + tlsMode = smtpclient.TLSRequiredStartTLS + tlsPKIX = true } else if submitconf.RequireTLS == RequireTLSYes { xsavecheckf(errors.New("cannot submit with requiretls enabled without tls to submission server"), "checking tls configuration") } @@ -292,7 +296,11 @@ binary should be setgid that group: } // todo: implement SRV and DANE, allowing for a simpler config file (just the email address & password) - client, err := smtpclient.New(ctx, mlog.New("sendmail"), conn, tlsMode, ourHostname, remoteHostname, auth, nil, nil, nil) + opts := smtpclient.Opts{ + Auth: auth, + RootCAs: mox.Conf.Static.TLS.CertPool, + } + client, err := smtpclient.New(ctx, mlog.New("sendmail"), conn, tlsMode, tlsPKIX, ourHostname, remoteHostname, opts) xsavecheckf(err, "open smtp session") err = client.Deliver(ctx, submitconf.From, recipient, int64(len(msg)), strings.NewReader(msg), true, false, submitconf.RequireTLS == RequireTLSYes) diff --git a/serve.go b/serve.go index 511ed3c..7f50f90 100644 --- a/serve.go +++ b/serve.go @@ -16,6 +16,7 @@ import ( "github.com/mjl-/mox/smtpserver" "github.com/mjl-/mox/store" "github.com/mjl-/mox/tlsrptdb" + "github.com/mjl-/mox/tlsrptsend" ) func shutdown(log *mlog.Log) { @@ -52,7 +53,7 @@ func shutdown(log *mlog.Log) { // start initializes all packages, starts all listeners and the switchboard // goroutine, then returns. -func start(mtastsdbRefresher, sendDMARCReports, skipForkExec bool) error { +func start(mtastsdbRefresher, sendDMARCReports, sendTLSReports, skipForkExec bool) error { smtpserver.Listen() imapserver.Listen() http.Listen() @@ -90,6 +91,10 @@ func start(mtastsdbRefresher, sendDMARCReports, skipForkExec bool) error { dmarcdb.Start(dns.StrictResolver{Pkg: "dmarcdb"}) } + if sendTLSReports { + tlsrptsend.Start(dns.StrictResolver{Pkg: "tlsrptsend"}) + } + store.StartAuthCache() smtpserver.Serve() imapserver.Serve() diff --git a/serve_unix.go b/serve_unix.go index bb66ff8..63fad09 100644 --- a/serve_unix.go +++ b/serve_unix.go @@ -224,7 +224,7 @@ Only implemented on unix systems, not Windows. // taken. const mtastsdbRefresher = true const skipForkExec = false - if err := start(mtastsdbRefresher, !mox.Conf.Static.NoOutgoingDMARCReports, skipForkExec); err != nil { + if err := start(mtastsdbRefresher, !mox.Conf.Static.NoOutgoingDMARCReports, !mox.Conf.Static.NoOutgoingTLSReports, skipForkExec); err != nil { log.Fatalx("start", err) } log.Print("ready to serve") diff --git a/smtpclient/client.go b/smtpclient/client.go index a4c67eb..f407cb3 100644 --- a/smtpclient/client.go +++ b/smtpclient/client.go @@ -5,6 +5,7 @@ import ( "bufio" "context" "crypto/tls" + "crypto/x509" "encoding/base64" "errors" "fmt" @@ -27,6 +28,7 @@ import ( "github.com/mjl-/mox/moxio" "github.com/mjl-/mox/sasl" "github.com/mjl-/mox/smtp" + "github.com/mjl-/mox/tlsrpt" ) // todo future: add function to deliver message to multiple recipients. requires more elaborate return value, indicating success per message: some recipients may succeed, others may fail, and we should still deliver. to prevent backscatter, we also sometimes don't allow multiple recipients. ../rfc/5321:1144 @@ -71,22 +73,17 @@ var ( type TLSMode string const ( - // Required TLS with STARTTLS for SMTP servers, with either verified DANE TLSA - // record, or a WebPKI-verified certificate (with matching name, not expired, etc). - TLSStrictStartTLS TLSMode = "strictstarttls" + // TLS immediately ("implicit TLS"), directly starting TLS on the TCP connection, + // so not using STARTTLS. Whether PKIX and/or DANE is verified is specified + // separately. + TLSImmediate TLSMode = "immediate" - // Required TLS with STARTTLS for SMTP servers, without verifiying the certificate. - // This mode is needed to fallback after only unusable DANE records were found - // (e.g. with unknown parameters in the TLSA records). Also for allowing - // verification errors with DANE with message header TLS-Required no. - TLSUnverifiedStartTLS TLSMode = "unverifiedstarttls" + // Required TLS with STARTTLS for SMTP servers. The STARTTLS command is always + // executed, even if the server does not announce support. + // Whether PKIX and/or DANE is verified is specified separately. + TLSRequiredStartTLS TLSMode = "requiredstarttls" - // TLS immediately ("implicit TLS"), with either verified DANE TLSA records or a - // verified certificate: matching name, not expired, trusted by CA. - TLSStrictImmediate TLSMode = "strictimmediate" - - // Use TLS if remote claims to support it, but do not verify the certificate - // (not trusted by CA, different host name or expired certificate is accepted). + // Use TLS with STARTTLS if remote claims to support it. TLSOpportunistic TLSMode = "opportunistic" // TLS must not be attempted, e.g. due to earlier TLS handshake error. @@ -101,12 +98,20 @@ type Client struct { // can be wrapped in a tls.Client. We close origConn instead of conn because // closing the TLS connection would send a TLS close notification, which may block // for 5s if the server isn't reading it (because it is also sending it). - origConn net.Conn - conn net.Conn - remoteHostname dns.Domain // TLS with SNI and name verification. - daneRecords []adns.TLSA // For authenticating (START)TLS connection. - moreRemoteHostnames []dns.Domain // Additional allowed names in TLS certificate. - verifiedRecord *adns.TLSA // If non-nil, then will be set to verified DANE record if any. + origConn net.Conn + conn net.Conn + tlsVerifyPKIX bool + ignoreTLSVerifyErrors bool + rootCAs *x509.CertPool + remoteHostname dns.Domain // TLS with SNI and name verification. + daneRecords []adns.TLSA // For authenticating (START)TLS connection. + daneMoreHostnames []dns.Domain // Additional allowed names in TLS certificate for DANE-TA. + daneVerifiedRecord *adns.TLSA // If non-nil, then will be set to verified DANE record if any. + + // TLS connection success/failure are added. These are always non-nil, regardless + // of what was passed in opts. It lets us unconditionally dereference them. + recipientDomainResult *tlsrpt.Result // Either "sts" or "no-policy-found". + hostResult *tlsrpt.Result // Either "dane" or "no-policy-found". r *bufio.Reader w *bufio.Writer @@ -121,8 +126,9 @@ type Client struct { botched bool // If set, protocol is out of sync and no further commands can be sent. needRset bool // If set, a new delivery requires an RSET command. - extEcodes bool // Remote server supports sending extended error codes. - extStartTLS bool // Remote server supports STARTTLS. + remoteHelo string // From 220 greeting line. + extEcodes bool // Remote server supports sending extended error codes. + extStartTLS bool // Remote server supports STARTTLS. ext8bitmime bool extSize bool // Remote server supports SIZE parameter. maxSize int64 // Max size of email message. @@ -177,6 +183,33 @@ func (e Error) Error() string { return s } +// Opts influence behaviour of Client. +type Opts struct { + // If auth is non-empty, authentication will be done with the first algorithm + // supported by the server. If none of the algorithms are supported, an error is + // returned. + Auth []sasl.Client + + DANERecords []adns.TLSA // If not nil, DANE records to verify. + DANEMoreHostnames []dns.Domain // For use with DANE, where additional certificate host names are allowed. + DANEVerifiedRecord *adns.TLSA // If non-empty, set to the DANE record that verified the TLS connection. + + // If set, TLS verification errors (for DANE or PKIX) are ignored. Useful for + // delivering messages with message header "TLS-Required: No". + // Certificates are still verified, and results are still tracked for TLS + // reporting, but the connections will continue. + IgnoreTLSVerifyErrors bool + + // If not nil, used instead of the system default roots for TLS PKIX verification. + RootCAs *x509.CertPool + + // TLS verification successes/failures is added to these TLS reporting results. + // Once the STARTTLS handshake is attempted, a successful/failed connection is + // tracked. + RecipientDomainResult *tlsrpt.Result // MTA-STS or no policy. + HostResult *tlsrpt.Result // DANE or no policy. +} + // New initializes an SMTP session on the given connection, returning a client that // can be used to deliver messages. // @@ -191,29 +224,38 @@ func (e Error) Error() string { // records with preferences, other DNS records, MTA-STS, retries and special // cases into account. // -// tlsMode indicates if TLS is required, optional or should not be used. Only for -// strict TLS modes is the certificate verified: Either with DANE, or through -// the trusted CA pool with matching remoteHostname and not expired. For DANE, -// additional host names in moreRemoteHostnames are allowed during TLS certificate -// verification. By default, SMTP does not verify TLS for interopability reasons, -// but MTA-STS or DANE can require it. If opportunistic TLS is used, and a TLS -// error is encountered, the caller may want to try again (on a new connection) -// without TLS. For messages with header TLS-Required no, DANE records may be -// passed along with tlsMode TLSUnverifiedStartTLS. In that case, failing DANE -// verification causes an error to be logged, but the connection won't be aborted. -// -// If auth is non-empty, authentication will be done with the first algorithm -// supported by the server. If none of the algorithms are supported, an error is -// returned. -func New(ctx context.Context, log *mlog.Log, conn net.Conn, tlsMode TLSMode, ehloHostname, remoteHostname dns.Domain, auth []sasl.Client, daneRecords []adns.TLSA, moreRemoteHostnames []dns.Domain, verifiedRecord *adns.TLSA) (*Client, error) { +// tlsMode indicates if and how TLS may/must (not) be used. tlsVerifyPKIX +// indicates if TLS certificates must be validated against the PKIX/WebPKI +// certificate authorities (if TLS is done). DANE-verification is done when +// opts.DANERecords is not nil. TLS verification errors will be ignored if +// opts.IgnoreTLSVerification is set. If TLS is done, PKIX verification is +// always performed for tracking the results for TLS reporting, but if +// tlsVerifyPKIX is false, the verification result does not affect the +// connection. At the time of writing, delivery of email on the internet is done +// with opportunistic TLS without PKIX verification by default. Recipient domains +// can opt-in to PKIX verification by publishing an MTA-STS policy, or opt-in to +// DANE verification by publishing DNSSEC-protected TLSA records in DNS. +func New(ctx context.Context, log *mlog.Log, conn net.Conn, tlsMode TLSMode, tlsVerifyPKIX bool, ehloHostname, remoteHostname dns.Domain, opts Opts) (*Client, error) { + ensureResult := func(r *tlsrpt.Result) *tlsrpt.Result { + if r == nil { + return &tlsrpt.Result{} + } + return r + } + c := &Client{ - origConn: conn, - remoteHostname: remoteHostname, - daneRecords: daneRecords, - moreRemoteHostnames: moreRemoteHostnames, - verifiedRecord: verifiedRecord, - lastlog: time.Now(), - cmds: []string{"(none)"}, + origConn: conn, + tlsVerifyPKIX: tlsVerifyPKIX, + ignoreTLSVerifyErrors: opts.IgnoreTLSVerifyErrors, + rootCAs: opts.RootCAs, + remoteHostname: remoteHostname, + daneRecords: opts.DANERecords, + daneMoreHostnames: opts.DANEMoreHostnames, + daneVerifiedRecord: opts.DANEVerifiedRecord, + lastlog: time.Now(), + cmds: []string{"(none)"}, + recipientDomainResult: ensureResult(opts.RecipientDomainResult), + hostResult: ensureResult(opts.HostResult), } c.log = log.Fields(mlog.Field("smtpclient", "")).MoreFields(func() []mlog.Pair { now := time.Now() @@ -224,13 +266,15 @@ func New(ctx context.Context, log *mlog.Log, conn net.Conn, tlsMode TLSMode, ehl return l }) - if tlsMode == TLSStrictImmediate { - // todo: we could also verify DANE here. not applicable to SMTP delivery. - config := c.tlsConfig(tlsMode) + if tlsMode == TLSImmediate { + config := c.tlsConfig() tlsconn := tls.Client(conn, config) + // The tlsrpt tracking isn't used by caller, but won't hurt. if err := tlsconn.HandshakeContext(ctx); err != nil { + c.tlsResultAdd(0, 1, err) return nil, err } + c.tlsResultAdd(1, 0, nil) c.conn = tlsconn tlsversion, ciphersuite := mox.TLSInfo(tlsconn) c.log.Debug("tls client handshake done", mlog.Field("tls", tlsversion), mlog.Field("ciphersuite", ciphersuite), mlog.Field("servername", remoteHostname)) @@ -249,36 +293,103 @@ func New(ctx context.Context, log *mlog.Log, conn net.Conn, tlsMode TLSMode, ehl c.tw = moxio.NewTraceWriter(c.log, "LC: ", timeoutWriter{c.conn, 30 * time.Second, c.log}) c.w = bufio.NewWriter(c.tw) - if err := c.hello(ctx, tlsMode, ehloHostname, auth); err != nil { + if err := c.hello(ctx, tlsMode, ehloHostname, opts.Auth); err != nil { return nil, err } return c, nil } -func (c *Client) tlsConfig(tlsMode TLSMode) *tls.Config { - if c.daneRecords != nil { - config := dane.TLSClientConfig(c.log, c.daneRecords, c.remoteHostname, c.moreRemoteHostnames, c.verifiedRecord) - if tlsMode == TLSUnverifiedStartTLS { - // In case of delivery with header "TLS-Required: No", the connection should not be - // aborted. - origVerify := config.VerifyConnection - config.VerifyConnection = func(cs tls.ConnectionState) error { - err := origVerify(cs) - if err != nil { - c.log.Infox("verifying dane failed, continuing due to tls mode unverified starttls, due to tls-required-no message header", err) - metricTLSRequiredNoIgnored.WithLabelValues("daneverification").Inc() +// reportedError wraps an error while indicating it was already tracked for TLS +// reporting. +type reportedError struct{ err error } + +func (e reportedError) Error() string { + return e.err.Error() +} + +func (e reportedError) Unwrap() error { + return e.err +} + +func (c *Client) tlsConfig() *tls.Config { + // We always manage verification ourselves: We need to report in detail about + // failures. And we may have to verify both PKIX and DANE, record errors for + // each, and possibly ignore the errors. + + verifyConnection := func(cs tls.ConnectionState) error { + // Collect verification errors. If there are none at the end, TLS validation + // succeeded. We may find validation problems below, record them for a TLS report + // but continue due to policies. We track the TLS reporting result in this + // function, wrapping errors in a reportedError. + var daneErr, pkixErr error + + // DANE verification. + // daneRecords can be non-nil and empty, that's intended. + if c.daneRecords != nil { + verified, record, err := dane.Verify(c.log, c.daneRecords, cs, c.remoteHostname, c.daneMoreHostnames) + c.log.Debugx("dane verification", err, mlog.Field("verified", verified), mlog.Field("record", record)) + if verified { + if c.daneVerifiedRecord != nil { + *c.daneVerifiedRecord = record + } + } else { + // Track error for reports. + // todo spec: may want to propose adding a result for no-dane-match. dane allows multiple records, some mismatching/failing isn't fatal and reporting on each record is probably not productive. ../rfc/8460:541 + fd := c.tlsrptFailureDetails(tlsrpt.ResultValidationFailure, "dane-no-match") + if err != nil { + // todo future: potentially add more details. e.g. dane-ta verification errors. tlsrpt does not have "result types" to indicate those kinds of errors. we would probably have to pass c.daneResult to dane.Verify. + + // We may have encountered errors while evaluation some of the TLSA records. + fd.FailureReasonCode += "+errors" + } + c.hostResult.Add(0, 0, fd) + + if c.ignoreTLSVerifyErrors { + // We ignore the failure and continue the connection. + c.log.Infox("verifying dane failed, continuing with connection", err) + metricTLSRequiredNoIgnored.WithLabelValues("daneverification").Inc() + } else { + // This connection will fail. + daneErr = dane.ErrNoMatch } - return nil } } - return &config + + // PKIX verification. + opts := x509.VerifyOptions{ + DNSName: cs.ServerName, + Intermediates: x509.NewCertPool(), + Roots: c.rootCAs, + } + for _, cert := range cs.PeerCertificates[1:] { + opts.Intermediates.AddCert(cert) + } + if _, err := cs.PeerCertificates[0].Verify(opts); err != nil { + resultType, reasonCode := tlsrpt.TLSFailureDetails(err) + fd := c.tlsrptFailureDetails(resultType, reasonCode) + c.recipientDomainResult.Add(0, 0, fd) + + if c.tlsVerifyPKIX && !c.ignoreTLSVerifyErrors { + pkixErr = err + } + } + + if daneErr != nil && pkixErr != nil { + return reportedError{errors.Join(daneErr, pkixErr)} + } else if daneErr != nil { + return reportedError{daneErr} + } else if pkixErr != nil { + return reportedError{pkixErr} + } + return nil } - // todo: possibly accept older TLS versions for TLSOpportunistic? + return &tls.Config{ - ServerName: c.remoteHostname.ASCII, - RootCAs: mox.Conf.Static.TLS.CertPool, - InsecureSkipVerify: tlsMode == TLSOpportunistic || tlsMode == TLSUnverifiedStartTLS, + ServerName: c.remoteHostname.ASCII, // For SNI. + // todo: possibly accept older TLS versions for TLSOpportunistic? or would our private key be at risk? MinVersion: tls.VersionTLS12, // ../rfc/8996:31 ../rfc/8997:66 + InsecureSkipVerify: true, // VerifyConnection below is called and will do all verification. + VerifyConnection: verifyConnection, } } @@ -589,16 +700,18 @@ func (c *Client) hello(ctx context.Context, tlsMode TLSMode, ehloHostname dns.Do // Read greeting. c.cmds = []string{"(greeting)"} c.cmdStart = time.Now() - code, _, lastLine, _ := c.xreadecode(false) + code, _, lastLine, lines := c.xreadecode(false) if code != smtp.C220ServiceReady { c.xerrorf(code/100 == 5, code, "", lastLine, "%w: expected 220, got %d", ErrStatus, code) } + // ../rfc/5321:2588 + c.remoteHelo, _, _ = strings.Cut(lines[0], " ") // Write EHLO, falling back to HELO if server doesn't appear to support it. hello(true) // Attempt TLS if remote understands STARTTLS and we aren't doing immediate TLS or if caller requires it. - if c.extStartTLS && (tlsMode != TLSSkip && tlsMode != TLSStrictImmediate) || tlsMode == TLSStrictStartTLS || tlsMode == TLSUnverifiedStartTLS { + if c.extStartTLS && tlsMode == TLSOpportunistic || tlsMode == TLSRequiredStartTLS { c.log.Debug("starting tls client", mlog.Field("tlsmode", tlsMode), mlog.Field("servername", c.remoteHostname)) c.cmds[0] = "starttls" c.cmdStart = time.Now() @@ -606,6 +719,7 @@ func (c *Client) hello(ctx context.Context, tlsMode TLSMode, ehloHostname dns.Do code, secode, lastLine, _ := c.xread() // ../rfc/3207:107 if code != smtp.C220ServiceReady { + c.tlsResultAddFailureDetails(0, 1, c.tlsrptFailureDetails(tlsrpt.ResultSTARTTLSNotSupported, fmt.Sprintf("smtp-starttls-reply-code-%d", code))) c.xerrorf(code/100 == 5, code, secode, lastLine, "%w: STARTTLS: got %d, expected 220", ErrTLS, code) } @@ -621,9 +735,7 @@ func (c *Client) hello(ctx context.Context, tlsMode TLSMode, ehloHostname dns.Do } } - // For TLSStrictStartTLS, the Go TLS library performs the checks needed for MTA-STS. - // ../rfc/8461:646 - tlsConfig := c.tlsConfig(tlsMode) + tlsConfig := c.tlsConfig() nconn := tls.Client(conn, tlsConfig) c.conn = nconn @@ -631,6 +743,10 @@ func (c *Client) hello(ctx context.Context, tlsMode TLSMode, ehloHostname dns.Do defer cancel() err := nconn.HandshakeContext(nctx) if err != nil { + // For each STARTTLS failure, we track a failed TLS session. For deliveries with + // multiple MX targets, we may add multiple failures, and delivery may succeed with + // a later MX target with which we can do STARTTLS. ../rfc/8460:524 + c.tlsResultAdd(0, 1, err) c.xerrorf(false, 0, "", "", "%w: STARTTLS TLS handshake: %s", ErrTLS, err) } cancel() @@ -640,10 +756,23 @@ func (c *Client) hello(ctx context.Context, tlsMode TLSMode, ehloHostname dns.Do c.w = bufio.NewWriter(c.tw) tlsversion, ciphersuite := mox.TLSInfo(nconn) - c.log.Debug("starttls client handshake done", mlog.Field("tlsmode", tlsMode), mlog.Field("tls", tlsversion), mlog.Field("ciphersuite", ciphersuite), mlog.Field("servername", c.remoteHostname), mlog.Field("danerecord", c.verifiedRecord)) + c.log.Debug("starttls client handshake done", + mlog.Field("tlsmode", tlsMode), + mlog.Field("verifypkix", c.tlsVerifyPKIX), + mlog.Field("verifydane", c.daneRecords != nil), + mlog.Field("ignoretlsverifyerrors", c.ignoreTLSVerifyErrors), + mlog.Field("tls", tlsversion), + mlog.Field("ciphersuite", ciphersuite), + mlog.Field("servername", c.remoteHostname), + mlog.Field("danerecord", c.daneVerifiedRecord)) c.tls = true + // Track successful TLS connection. ../rfc/8460:515 + c.tlsResultAdd(1, 0, nil) hello(false) + } else if tlsMode == TLSOpportunistic { + // Result: ../rfc/8460:538 + c.tlsResultAddFailureDetails(0, 0, c.tlsrptFailureDetails(tlsrpt.ResultSTARTTLSNotSupported, "")) } if len(auth) > 0 { @@ -652,6 +781,50 @@ func (c *Client) hello(ctx context.Context, tlsMode TLSMode, ehloHostname dns.Do return } +func addrIP(addr net.Addr) string { + if t, ok := addr.(*net.TCPAddr); ok { + return t.IP.String() + } + host, _, _ := net.SplitHostPort(addr.String()) + ip := net.ParseIP(host) + if ip == nil { + return "" // For pipe during tests. + } + return ip.String() +} + +// tlsrptFailureDetails returns FailureDetails with connection details (such as +// IP addresses) for inclusion in a TLS report. +func (c *Client) tlsrptFailureDetails(resultType tlsrpt.ResultType, reasonCode string) tlsrpt.FailureDetails { + return tlsrpt.FailureDetails{ + ResultType: resultType, + SendingMTAIP: addrIP(c.origConn.LocalAddr()), + ReceivingMXHostname: c.remoteHostname.ASCII, + ReceivingMXHelo: c.remoteHelo, + ReceivingIP: addrIP(c.origConn.RemoteAddr()), + FailedSessionCount: 1, + FailureReasonCode: reasonCode, + } +} + +// tlsResultAdd adds TLS success/failure to all results. +func (c *Client) tlsResultAdd(success, failure int64, err error) { + // Only track failure if not already done so in tls.Config.VerifyConnection. + var fds []tlsrpt.FailureDetails + var repErr reportedError + if err != nil && !errors.As(err, &repErr) { + resultType, reasonCode := tlsrpt.TLSFailureDetails(err) + fd := c.tlsrptFailureDetails(resultType, reasonCode) + fds = []tlsrpt.FailureDetails{fd} + } + c.tlsResultAddFailureDetails(success, failure, fds...) +} + +func (c *Client) tlsResultAddFailureDetails(success, failure int64, fds ...tlsrpt.FailureDetails) { + c.recipientDomainResult.Add(success, failure, fds...) + c.hostResult.Add(success, failure, fds...) +} + // ../rfc/4954:139 func (c *Client) auth(auth []sasl.Client) (rerr error) { defer c.recover(&rerr) diff --git a/smtpclient/client_test.go b/smtpclient/client_test.go index 192d66e..af28924 100644 --- a/smtpclient/client_test.go +++ b/smtpclient/client_test.go @@ -23,7 +23,6 @@ import ( "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" - "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/sasl" "github.com/mjl-/mox/scram" "github.com/mjl-/mox/smtp" @@ -49,6 +48,8 @@ func TestClient(t *testing.T) { ehlo bool tlsMode TLSMode + tlsPKIX bool + roots *x509.CertPool tlsHostname dns.Domain need8bitmime bool needsmtputf8 bool @@ -60,8 +61,8 @@ func TestClient(t *testing.T) { // Make fake cert, and make it trusted. cert := fakeCert(t, false) - mox.Conf.Static.TLS.CertPool = x509.NewCertPool() - mox.Conf.Static.TLS.CertPool.AddCert(cert.Leaf) + roots := x509.NewCertPool() + roots.AddCert(cert.Leaf) tlsConfig := tls.Config{ Certificates: []tls.Certificate{cert}, } @@ -280,7 +281,7 @@ func TestClient(t *testing.T) { result <- err panic("stop") } - c, err := New(ctx, log, clientConn, opts.tlsMode, localhost, opts.tlsHostname, auths, nil, nil, nil) + c, err := New(ctx, log, clientConn, opts.tlsMode, opts.tlsPKIX, localhost, opts.tlsHostname, Opts{Auth: auths, RootCAs: opts.roots}) if (err == nil) != (expClientErr == nil) || err != nil && !errors.As(err, reflect.New(reflect.ValueOf(expClientErr).Type()).Interface()) && !errors.Is(err, expClientErr) { fail("new client: got err %v, expected %#v", err, expClientErr) } @@ -338,7 +339,9 @@ test ehlo: true, requiretls: true, - tlsMode: TLSStrictStartTLS, + tlsMode: TLSRequiredStartTLS, + tlsPKIX: true, + roots: roots, tlsHostname: dns.Domain{ASCII: "mox.example"}, need8bitmime: true, needsmtputf8: true, @@ -350,7 +353,7 @@ test test(msg, options{ehlo: true, eightbitmime: true}, nil, nil, nil, nil) test(msg, options{ehlo: true, eightbitmime: false, need8bitmime: true, nodeliver: true}, nil, nil, Err8bitmimeUnsupported, nil) test(msg, options{ehlo: true, smtputf8: false, needsmtputf8: true, nodeliver: true}, nil, nil, ErrSMTPUTF8Unsupported, nil) - test(msg, options{ehlo: true, starttls: true, tlsMode: TLSStrictStartTLS, tlsHostname: dns.Domain{ASCII: "mismatch.example"}, nodeliver: true}, nil, ErrTLS, nil, &net.OpError{}) // Server TLS handshake is a net.OpError with "remote error" as text. + test(msg, options{ehlo: true, starttls: true, tlsMode: TLSRequiredStartTLS, tlsPKIX: true, tlsHostname: dns.Domain{ASCII: "mismatch.example"}, nodeliver: true}, nil, ErrTLS, nil, &net.OpError{}) // Server TLS handshake is a net.OpError with "remote error" as text. test(msg, options{ehlo: true, maxSize: len(msg) - 1, nodeliver: true}, nil, nil, ErrSize, nil) test(msg, options{ehlo: true, auths: []string{"PLAIN"}}, []sasl.Client{sasl.NewClientPlain("test", "test")}, nil, nil, nil) test(msg, options{ehlo: true, auths: []string{"CRAM-MD5"}}, []sasl.Client{sasl.NewClientCRAMMD5("test", "test")}, nil, nil, nil) @@ -362,19 +365,19 @@ test // Set an expired certificate. For non-strict TLS, we should still accept it. // ../rfc/7435:424 cert = fakeCert(t, true) - mox.Conf.Static.TLS.CertPool = x509.NewCertPool() - mox.Conf.Static.TLS.CertPool.AddCert(cert.Leaf) + roots = x509.NewCertPool() + roots.AddCert(cert.Leaf) tlsConfig = tls.Config{ Certificates: []tls.Certificate{cert}, } - test(msg, options{ehlo: true, starttls: true}, nil, nil, nil, nil) + test(msg, options{ehlo: true, starttls: true, roots: roots}, nil, nil, nil, nil) // Again with empty cert pool so it isn't trusted in any way. - mox.Conf.Static.TLS.CertPool = x509.NewCertPool() + roots = x509.NewCertPool() tlsConfig = tls.Config{ Certificates: []tls.Certificate{cert}, } - test(msg, options{ehlo: true, starttls: true}, nil, nil, nil, nil) + test(msg, options{ehlo: true, starttls: true, roots: roots}, nil, nil, nil, nil) } func TestErrors(t *testing.T) { @@ -385,7 +388,7 @@ func TestErrors(t *testing.T) { run(t, func(s xserver) { s.writeline("bogus") // Invalid, should be "220 ". }, func(conn net.Conn) { - _, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + _, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) var xerr Error if err == nil || !errors.Is(err, ErrProtocol) || !errors.As(err, &xerr) || xerr.Permanent { panic(fmt.Errorf("got %#v, expected ErrProtocol without Permanent", err)) @@ -396,7 +399,7 @@ func TestErrors(t *testing.T) { run(t, func(s xserver) { s.conn.Close() }, func(conn net.Conn) { - _, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + _, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) var xerr Error if err == nil || !errors.Is(err, io.ErrUnexpectedEOF) || !errors.As(err, &xerr) || xerr.Permanent { panic(fmt.Errorf("got %#v (%v), expected ErrUnexpectedEOF without Permanent", err, err)) @@ -407,7 +410,7 @@ func TestErrors(t *testing.T) { run(t, func(s xserver) { s.writeline("521 not accepting connections") }, func(conn net.Conn) { - _, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + _, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) var xerr Error if err == nil || !errors.Is(err, ErrStatus) || !errors.As(err, &xerr) || !xerr.Permanent { panic(fmt.Errorf("got %#v, expected ErrStatus with Permanent", err)) @@ -418,7 +421,7 @@ func TestErrors(t *testing.T) { run(t, func(s xserver) { s.writeline("2200 mox.example") // Invalid, too many digits. }, func(conn net.Conn) { - _, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + _, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) var xerr Error if err == nil || !errors.Is(err, ErrProtocol) || !errors.As(err, &xerr) || xerr.Permanent { panic(fmt.Errorf("got %#v, expected ErrProtocol without Permanent", err)) @@ -432,7 +435,7 @@ func TestErrors(t *testing.T) { s.writeline("250-mox.example") s.writeline("500 different code") // Invalid. }, func(conn net.Conn) { - _, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + _, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) var xerr Error if err == nil || !errors.Is(err, ErrProtocol) || !errors.As(err, &xerr) || xerr.Permanent { panic(fmt.Errorf("got %#v, expected ErrProtocol without Permanent", err)) @@ -448,7 +451,7 @@ func TestErrors(t *testing.T) { s.readline("MAIL FROM:") s.writeline("550 5.7.0 not allowed") }, func(conn net.Conn) { - c, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + c, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) if err != nil { panic(err) } @@ -468,7 +471,7 @@ func TestErrors(t *testing.T) { s.readline("MAIL FROM:") s.writeline("451 bad sender") }, func(conn net.Conn) { - c, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + c, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) if err != nil { panic(err) } @@ -490,7 +493,7 @@ func TestErrors(t *testing.T) { s.readline("RCPT TO:") s.writeline("451") }, func(conn net.Conn) { - c, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + c, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) if err != nil { panic(err) } @@ -514,7 +517,7 @@ func TestErrors(t *testing.T) { s.readline("DATA") s.writeline("550 no!") }, func(conn net.Conn) { - c, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + c, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) if err != nil { panic(err) } @@ -534,7 +537,7 @@ func TestErrors(t *testing.T) { s.readline("STARTTLS") s.writeline("502 command not implemented") }, func(conn net.Conn) { - _, err := New(ctx, log, conn, TLSStrictStartTLS, localhost, dns.Domain{ASCII: "mox.example"}, nil, nil, nil, nil) + _, err := New(ctx, log, conn, TLSRequiredStartTLS, true, localhost, dns.Domain{ASCII: "mox.example"}, Opts{}) var xerr Error if err == nil || !errors.Is(err, ErrTLS) || !errors.As(err, &xerr) || !xerr.Permanent { panic(fmt.Errorf("got %#v, expected ErrTLS with Permanent", err)) @@ -550,7 +553,7 @@ func TestErrors(t *testing.T) { s.readline("MAIL FROM:") s.writeline("451 enough") }, func(conn net.Conn) { - c, err := New(ctx, log, conn, TLSSkip, localhost, dns.Domain{ASCII: "mox.example"}, nil, nil, nil, nil) + c, err := New(ctx, log, conn, TLSSkip, false, localhost, dns.Domain{ASCII: "mox.example"}, Opts{}) if err != nil { panic(err) } @@ -580,7 +583,7 @@ func TestErrors(t *testing.T) { s.readline("DATA") s.writeline("550 not now") }, func(conn net.Conn) { - c, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + c, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) if err != nil { panic(err) } @@ -610,7 +613,7 @@ func TestErrors(t *testing.T) { s.readline("MAIL FROM:") s.writeline("550 ok") }, func(conn net.Conn) { - c, err := New(ctx, log, conn, TLSOpportunistic, localhost, zerohost, nil, nil, nil, nil) + c, err := New(ctx, log, conn, TLSOpportunistic, false, localhost, zerohost, Opts{}) if err != nil { panic(err) } diff --git a/smtpclient/gather.go b/smtpclient/gather.go index 7d1ba7a..8807cc9 100644 --- a/smtpclient/gather.go +++ b/smtpclient/gather.go @@ -266,14 +266,16 @@ func GatherIPs(ctx context.Context, log *mlog.Log, resolver dns.Resolver, host d // Only usable records are returned. If any record was found, DANE is required and // this is indicated with daneRequired. If no usable records remain, the caller // must do TLS, but not verify the remote TLS certificate. +// +// Returned values are always meaningful, also when an error was returned. func GatherTLSA(ctx context.Context, log *mlog.Log, resolver dns.Resolver, host dns.Domain, expandedAuthentic bool, expandedHost dns.Domain) (daneRequired bool, daneRecords []adns.TLSA, tlsaBaseDomain dns.Domain, err error) { // ../rfc/7672:912 // This function is only called when the lookup of host was authentic. var l []adns.TLSA + tlsaBaseDomain = host if host == expandedHost || !expandedAuthentic { - tlsaBaseDomain = host l, err = lookupTLSACNAME(ctx, log, resolver, 25, "tcp", host) } else if expandedAuthentic { // ../rfc/7672:934 @@ -286,8 +288,8 @@ func GatherTLSA(ctx context.Context, log *mlog.Log, resolver dns.Resolver, host } if len(l) == 0 || err != nil { daneRequired = err != nil - log.Debugx("gathering tlsa records failed", err, mlog.Field("danerequired", daneRequired)) - return daneRequired, nil, dns.Domain{}, err + log.Debugx("gathering tlsa records failed", err, mlog.Field("danerequired", daneRequired), mlog.Field("basedomain", tlsaBaseDomain)) + return daneRequired, nil, tlsaBaseDomain, err } daneRequired = len(l) > 0 l = filterUsableTLSARecords(log, l) @@ -329,7 +331,7 @@ func lookupTLSACNAME(ctx context.Context, log *mlog.Log, resolver dns.Resolver, return nil, fmt.Errorf("looking up tlsa records for tlsa candidate base domain: %w", err) } else if !result.Authentic { log.Debugx("tlsa lookup not authentic, not doing dane for host", err, mlog.Field("host", host), mlog.Field("name", name)) - return nil, err + return nil, nil } return l, nil } diff --git a/smtpclient/gather_test.go b/smtpclient/gather_test.go index 85b0686..20b0dcf 100644 --- a/smtpclient/gather_test.go +++ b/smtpclient/gather_test.go @@ -283,12 +283,12 @@ func TestGatherTLSA(t *testing.T) { test(domain("cnameloop.example"), false, domain("cnameloop.example"), true, nil, zerohost, errCNAMELimit) test(domain("host0.example"), false, domain("inauthentic.example"), true, list0, domain("host0.example"), nil) - test(domain("inauthentic.example"), false, domain("inauthentic.example"), false, nil, zerohost, nil) - test(domain("temperror-cname.example"), false, domain("temperror-cname.example"), true, nil, zerohost, &adns.DNSError{}) + test(domain("inauthentic.example"), false, domain("inauthentic.example"), false, nil, domain("inauthentic.example"), nil) + test(domain("temperror-cname.example"), false, domain("temperror-cname.example"), true, nil, domain("temperror-cname.example"), &adns.DNSError{}) test(domain("host1.example"), true, domain("cname-to-inauthentic.example"), true, list1, domain("host1.example"), nil) test(domain("host1.example"), true, domain("danglingcname.example"), true, list1, domain("host1.example"), nil) - test(domain("danglingcname.example"), true, domain("danglingcname.example"), false, nil, zerohost, nil) + test(domain("danglingcname.example"), true, domain("danglingcname.example"), false, nil, domain("danglingcname.example"), nil) } func TestGatherTLSANames(t *testing.T) { diff --git a/smtpserver/analyze.go b/smtpserver/analyze.go index 51adfff..8cdd010 100644 --- a/smtpserver/analyze.go +++ b/smtpserver/analyze.go @@ -210,7 +210,7 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive // Similar to DMARC reporting, we check for the required DKIM. We'll check // reputation, defaulting to accept. var tlsReport *tlsrpt.Report - if d.rcptAcc.destination.TLSReports { + if d.rcptAcc.destination.HostTLSReports || d.rcptAcc.destination.DomainTLSReports { // Valid DKIM signature for domain must be present. We take "valid" to assume // "passing", not "syntactically valid". We also check for "tlsrpt" as service. // This check is optional, but if anyone goes through the trouble to explicitly @@ -218,6 +218,13 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive // ../rfc/8460:320 ok := false for _, r := range d.dkimResults { + // The record should have an allowed service "tlsrpt". The RFC mentions it as if + // the service must be specified explicitly, but the default allowed services for a + // DKIM record are "*", which includes "tlsrpt". Unless a the DKIM record + // explicitly specifies services (e.g. s=email), a record will work for TLS + // reports. The DKIM records seen used for TLS reporting in the wild don't + // explicitly set "s" for services. + // ../rfc/8460:326 if r.Status == dkim.StatusPass && r.Sig.Domain == d.msgFrom.Domain && r.Sig.Length < 0 && r.Record.ServiceAllowed("tlsrpt") { ok = true break diff --git a/smtpserver/server.go b/smtpserver/server.go index f1e4e9a..27e2f3f 100644 --- a/smtpserver/server.go +++ b/smtpserver/server.go @@ -295,7 +295,7 @@ type conn struct { log *mlog.Log maxMessageSize int64 requireTLSForAuth bool - requireTLSForDelivery bool + requireTLSForDelivery bool // If set, delivery is only allowed with TLS (STARTTLS), except if delivery is to a TLS reporting address. cmd string // Current command. cmdStart time.Time // Start of current command. ncmds int // Number of commands processed. Used to abort connection when first incoming command is unknown/invalid. @@ -761,14 +761,22 @@ func (c *conn) xneedHello() { } } -// If smtp server is configured to require TLS for all mail delivery, abort command. -func (c *conn) xneedTLSForDelivery() { - if c.requireTLSForDelivery && !c.tls { +// If smtp server is configured to require TLS for all mail delivery (except to TLS +// reporting address), abort command. +func (c *conn) xneedTLSForDelivery(rcpt smtp.Path) { + // For TLS reports, we allow the message in even without TLS, because there may be + // TLS interopability problems. ../rfc/8460:316 + if c.requireTLSForDelivery && !c.tls && !isTLSReportRecipient(rcpt) { // ../rfc/3207:148 xsmtpUserErrorf(smtp.C530SecurityRequired, smtp.SePol7Other0, "STARTTLS required for mail delivery") } } +func isTLSReportRecipient(rcpt smtp.Path) bool { + _, _, dest, err := mox.FindAccount(rcpt.Localpart, rcpt.IPDomain.Domain, false) + return err == nil && (dest.HostTLSReports || dest.DomainTLSReports) +} + func (c *conn) cmdHelo(p *parser) { c.cmdHello(p, false) } @@ -1219,7 +1227,6 @@ func (c *conn) cmdMail(p *parser) { c.xneedHello() c.xcheckAuth() - c.xneedTLSForDelivery() if c.mailFrom != nil { // ../rfc/5321:2507, though ../rfc/5321:1029 contradicts, implying a MAIL would also reset, but ../rfc/5321:1160 decides. xsmtpUserErrorf(smtp.C503BadCmdSeq, smtp.SeProto5BadCmdOrSeq1, "already have MAIL") @@ -1368,7 +1375,6 @@ func (c *conn) cmdMail(p *parser) { func (c *conn) cmdRcpt(p *parser) { c.xneedHello() c.xcheckAuth() - c.xneedTLSForDelivery() if c.mailFrom == nil { // ../rfc/5321:1088 xsmtpUserErrorf(smtp.C503BadCmdSeq, smtp.SeProto5BadCmdOrSeq1, "missing MAIL FROM") @@ -1398,6 +1404,12 @@ func (c *conn) cmdRcpt(p *parser) { } p.xend() + // Check if TLS is enabled if required. It's not great that sender/recipient + // addresses may have been exposed in plaintext before we can reject delivery. The + // recipient could be the tls reporting addresses, which must always be able to + // receive in plain text. + c.xneedTLSForDelivery(fpath) + // todo future: for submission, should we do explicit verification that domains are fully qualified? also for mail from. ../rfc/6409:420 if len(c.recipients) >= 100 { @@ -1496,7 +1508,6 @@ func (c *conn) cmdRcpt(p *parser) { func (c *conn) cmdData(p *parser) { c.xneedHello() c.xcheckAuth() - c.xneedTLSForDelivery() if c.mailFrom == nil { // ../rfc/5321:1130 xsmtpUserErrorf(smtp.C503BadCmdSeq, smtp.SeProto5BadCmdOrSeq1, "missing MAIL FROM") @@ -2525,7 +2536,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW // loop. We also don't want to be used for sending reports to unsuspecting domains // we have no relation with. // todo: would it make sense to also mark some percentage of mailing-list-policy-overrides optional? to lower the load on mail servers of folks sending to large mailing lists. - Optional: rcptAcc.destination.DMARCReports || rcptAcc.destination.TLSReports || a.reason == reasonDMARCPolicy && unknownDomain(), + Optional: rcptAcc.destination.DMARCReports || rcptAcc.destination.HostTLSReports || rcptAcc.destination.DomainTLSReports || a.reason == reasonDMARCPolicy && unknownDomain(), Addresses: addresses, @@ -2643,7 +2654,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW } if a.tlsReport != nil { // todo future: add rate limiting to prevent DoS attacks. - if err := tlsrptdb.AddReport(ctx, msgFrom.Domain, c.mailFrom.String(), a.tlsReport); err != nil { + if err := tlsrptdb.AddReport(ctx, msgFrom.Domain, c.mailFrom.String(), rcptAcc.destination.HostTLSReports, a.tlsReport); err != nil { log.Errorx("saving TLSRPT report in database", err) } else { log.Info("tlsrpt report processed") diff --git a/smtpserver/server_test.go b/smtpserver/server_test.go index 22963c7..74225b1 100644 --- a/smtpserver/server_test.go +++ b/smtpserver/server_test.go @@ -93,6 +93,7 @@ type testserver struct { requiretls bool dnsbls []dns.Domain tlsmode smtpclient.TLSMode + tlspkix bool } func newTestServer(t *testing.T, configPath string, resolver dns.Resolver) *testserver { @@ -164,7 +165,11 @@ func (ts *testserver) run(fn func(helloErr error, client *smtpclient.Client)) { ourHostname := mox.Conf.Static.HostnameDomain remoteHostname := dns.Domain{ASCII: "mox.example"} - client, err := smtpclient.New(ctxbg, xlog.WithCid(ts.cid-1), clientConn, ts.tlsmode, ourHostname, remoteHostname, auth, nil, nil, nil) + opts := smtpclient.Opts{ + Auth: auth, + RootCAs: mox.Conf.Static.TLS.CertPool, + } + client, err := smtpclient.New(ctxbg, xlog.WithCid(ts.cid-1), clientConn, ts.tlsmode, ts.tlspkix, ourHostname, remoteHostname, opts) if err != nil { clientConn.Close() } else { diff --git a/testdata/tlsrptsend/domains.conf b/testdata/tlsrptsend/domains.conf new file mode 100644 index 0000000..2b8bb28 --- /dev/null +++ b/testdata/tlsrptsend/domains.conf @@ -0,0 +1,13 @@ +Domains: + mox.example: + DKIM: + Selectors: + testsel: + PrivateKeyFile: testsel.rsakey.pkcs8.pem + Sign: + - testsel +Accounts: + mjl: + Domain: mox.example + Destinations: + mjl@mox.example: nil diff --git a/testdata/tlsrptsend/mox.conf b/testdata/tlsrptsend/mox.conf new file mode 100644 index 0000000..3f6fdcf --- /dev/null +++ b/testdata/tlsrptsend/mox.conf @@ -0,0 +1,11 @@ +DataDir: data +User: 1000 +LogLevel: trace +Hostname: mail.mox.example +Listeners: + local: + IPs: + - 0.0.0.0 +Postmaster: + Account: mjl + Mailbox: postmaster diff --git a/testdata/tlsrptsend/testsel.rsakey.pkcs8.pem b/testdata/tlsrptsend/testsel.rsakey.pkcs8.pem new file mode 100644 index 0000000..73d742c --- /dev/null +++ b/testdata/tlsrptsend/testsel.rsakey.pkcs8.pem @@ -0,0 +1,30 @@ +-----BEGIN PRIVATE KEY----- +Note: RSA private key for use with DKIM, generated by mox + +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDdkh3fKzvRUWym +n9UwVrEw6s2Mc0+DTg04TWJKGKHXpvcTHuEcE6ALVS9MZKasyVsIHU7FNeS9/qNb +pLihhGdlhU3KAfrMpTBhiFpJoYiDXED98Of4iBxNHIuheLMxSBSClMbLGE2vAgha +/6LuONuzdMqk/c1TijBD+vGjCZI2qD58cgXWWKRK9e+WNhKNoVdedZ9iJtbtN0MI +UWk3iwHmjXf5qzS7i8vDoy86Ln0HW0vKl7UtwemLVv09/E23OdNN163eQvSlrEhx +a0odPQsM9SizxhiaI9rmcZtSqULt37hhPaNA+/AbELCzWijZPDqePVRqKGd5gYDK +8STLj0UHAgMBAAECggEBAKVkJJgplYUx2oCmXmSu0aVKIBTvHjNNV+DnIq9co7Ju +F5BWRILIw3ayJ5RGrYPc6e6ssdfT2uNX6GjIFGm8g9HsJ5zazXNk+zBSr9K2mUg0 +3O6xnPaP41BMNo5ZoqjuvSCcHagMhDBWvBXxLJXWK2lRjNKMAXCSfmTANQ8WXeYd +XG2nYTPtBu6UgY8W6sKAx1xetxBrzk8q6JTxb5eVG22BSiUniWYif+XVmAj1u6TH +0m6X0Kb6zsMYYgKPC2hmDsxD3uZ7qBNxxJzzLjpK6eP9aeFKzNyfnaoO4s+9K6Di +31oxTBpqLI4dcrvg4xWl+YkEknXXaomMqM8hyDzfcAECgYEA9/zmjRpoTAoY3fu9 +mn16wxReFXZZZhqV0+c+gyYtao2Kf2pUNAdhD62HQv7KtAPPHKvLfL8PH0u7bzK0 +vVNzBUukwxGI7gsoTMdc3L5x4v9Yb6jUx7RrDZn93sDod/1f/sb56ARCFQoqbUck +dSjnVUyF/l5oeh6CgKhvtghJ/AcCgYEA5Lq4kL82qWjIuNUT/C3lzjPfQVU+WvQ9 +wa+x4B4mxm5r4na3AU1T8H+peh4YstAJUgscGfYnLzxuMGuP1ReIuWYy29eDptKl +WTzVZDcZrAPciP1FOL6jm03PT2UAEuoPRr4OHLg8DxoOqG8pxqk1izDSHG2Tof6l +0ToafeIALwECgYEA8wvLTgnOpI/U1WNP7aUDd0Rz/WbzsW1m4Lsn+lOleWPllIE6 +q4974mi5Q8ECG7IL/9aj5cw/XvXTauVwXIn4Ff2QKpr58AvBYJaX/cUtS0PlgfIf +MOczcK43MWUxscADoGmVLn9V4NcIw/dQ1P7U0zXfsXEHxoA2eTAb5HV1RWsCgYBd +TcXoVfgIV1Q6AcGrR1XNLd/OmOVc2PEwR2l6ERKkM3sS4HZ6s36gRpNt20Ub/D0x +GJMYDA+j9zTDz7zWokkFyCjLATkVHiyRIH2z6b4xK0oVH6vTIAFBYxZEPuEu1gfx +RaogEQ9+4ZRFJUOXZIMRCpNLQW/Nz0D4/oi7/SsyAQKBgHEA27Js8ivt+EFCBjwB +UbkW+LonDAXuUbw91lh5jICCigqUg73HNmV5xpoYI9JNPc6fy6wLyInVUC2w9tpO +eH2Rl8n79vQMLbzsFClGEC/Q1kAbK5bwUjlfvKBZjvE0RknWX9e1ZY04DSsunSrM +prS2eHVZ24hecd7j9XfAbHLC +-----END PRIVATE KEY----- diff --git a/tlsrpt/alert.go b/tlsrpt/alert.go new file mode 100644 index 0000000..3f4ca8b --- /dev/null +++ b/tlsrpt/alert.go @@ -0,0 +1,21 @@ +//go:build go1.21 + +// From go1.21 and onwards. + +package tlsrpt + +import ( + "crypto/tls" + "fmt" + "strings" +) + +func formatAlert(alert uint8) string { + s := fmt.Sprintf("alert-%d", alert) + err := tls.AlertError(alert) // Since go1.21.0 + // crypto/tls returns messages like "tls: short message" or "tls: alert(321)". + if str := err.Error(); !strings.Contains(str, "alert(") { + s += "-" + strings.ReplaceAll(strings.TrimPrefix(str, "tls: "), " ", "-") + } + return s +} diff --git a/tlsrpt/alert_go120.go b/tlsrpt/alert_go120.go new file mode 100644 index 0000000..c686a52 --- /dev/null +++ b/tlsrpt/alert_go120.go @@ -0,0 +1,13 @@ +//go:build !go1.21 + +// For go1.20 and earlier. + +package tlsrpt + +import ( + "fmt" +) + +func formatAlert(alert uint8) string { + return fmt.Sprintf("alert-%d", alert) +} diff --git a/tlsrpt/parse.go b/tlsrpt/parse.go index 9216689..bcff02b 100644 --- a/tlsrpt/parse.go +++ b/tlsrpt/parse.go @@ -18,8 +18,13 @@ type Extension struct { // // v=TLSRPTv1; rua=mailto:tlsrpt@mox.example; type Record struct { - Version string // "TLSRPTv1", for "v=". - RUAs [][]string // Aggregate reporting URI, for "rua=". "rua=" can occur multiple times, each can be a list. Must be URL-encoded strings, with ",", "!" and ";" encoded. + Version string // "TLSRPTv1", for "v=". + + // Aggregate reporting URI, for "rua=". "rua=" can occur multiple times, each can + // be a list. Must be URL-encoded strings, with ",", "!" and ";" encoded. + RUAs [][]string + // ../rfc/8460:383 + Extensions []Extension } diff --git a/tlsrpt/report.go b/tlsrpt/report.go index d3bb891..b402c0c 100644 --- a/tlsrpt/report.go +++ b/tlsrpt/report.go @@ -2,13 +2,25 @@ package tlsrpt import ( "compress/gzip" + "context" + "crypto/tls" + "crypto/x509" "encoding/json" "errors" "fmt" "io" + "net" + "os" + "reflect" + "sort" "strings" "time" + "golang.org/x/exp/slices" + + "github.com/mjl-/adns" + + "github.com/mjl-/mox/dns" "github.com/mjl-/mox/message" "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/moxio" @@ -27,6 +39,79 @@ type Report struct { Policies []Result `json:"policies"` } +// Merge combines the counts and failure details of results into the report. +// Policies are merged if identical and added otherwise. Same for failure details +// within a result. +func (r *Report) Merge(results ...Result) { +Merge: + for _, nr := range results { + for i, p := range r.Policies { + if !p.Policy.equal(nr.Policy) { + continue + } + + r.Policies[i].Add(nr.Summary.TotalSuccessfulSessionCount, nr.Summary.TotalFailureSessionCount, nr.FailureDetails...) + continue Merge + } + + r.Policies = append(r.Policies, nr) + } +} + +// Add increases the success/failure counts of a result, and adds any failure +// details. +func (r *Result) Add(success, failure int64, fds ...FailureDetails) { + r.Summary.TotalSuccessfulSessionCount += success + r.Summary.TotalFailureSessionCount += failure + +Merge: + for _, nfd := range fds { + for i, fd := range r.FailureDetails { + if !fd.equalKey(nfd) { + continue + } + + fd.FailedSessionCount += nfd.FailedSessionCount + r.FailureDetails[i] = fd + continue Merge + } + r.FailureDetails = append(r.FailureDetails, nfd) + } +} + +// Add is a convenience function for merging making a Result and merging it into +// the report. +func (r *Report) Add(policy ResultPolicy, success, failure int64, fds ...FailureDetails) { + r.Merge(Result{policy, Summary{success, failure}, fds}) +} + +// TLSAPolicy returns a policy for DANE. +func TLSAPolicy(records []adns.TLSA, tlsaBaseDomain dns.Domain) ResultPolicy { + // The policy domain is the TLSA base domain. ../rfc/8460:251 + + l := make([]string, len(records)) + for i, r := range records { + l[i] = r.Record() + } + sort.Strings(l) // For consistent equals. + return ResultPolicy{ + Type: TLSA, + String: l, + Domain: tlsaBaseDomain.ASCII, + MXHost: []string{}, + } +} + +func MakeResult(policyType PolicyType, domain dns.Domain, fds ...FailureDetails) Result { + if fds == nil { + fds = []FailureDetails{} + } + return Result{ + Policy: ResultPolicy{Type: policyType, Domain: domain.ASCII, String: []string{}, MXHost: []string{}}, + FailureDetails: fds, + } +} + // note: with TLSRPT prefix to prevent clash in sherpadoc types. type TLSRPTDateRange struct { Start time.Time `json:"start-datetime"` @@ -80,11 +165,32 @@ type Result struct { FailureDetails []FailureDetails `json:"failure-details"` } +// todo spec: ../rfc/8460:437 says policy is a string, with rules for turning dane records into a single string. perhaps a remnant of an earlier version (for mtasts a single string would have made more sense). i doubt the intention is to always have a single element in policy-string (though the field name is singular). + type ResultPolicy struct { - Type string `json:"policy-type"` - String []string `json:"policy-string"` - Domain string `json:"policy-domain"` - MXHost []string `json:"mx-host"` // Example in RFC has errata, it originally was a single string. ../rfc/8460-eid6241 ../rfc/8460:1779 + Type PolicyType `json:"policy-type"` + String []string `json:"policy-string"` + Domain string `json:"policy-domain"` + MXHost []string `json:"mx-host"` // Example in RFC has errata, it originally was a single string. ../rfc/8460-eid6241 ../rfc/8460:1779 +} + +// PolicyType indicates the policy success/failure results are for. +type PolicyType string + +const ( + // For DANE, against a mail host (not recipient domain). + TLSA PolicyType = "tlsa" + + // For MTA-STS, against a recipient domain (not a mail host). + STS PolicyType = "sts" + + // Recipient domain did not have MTA-STS policy, or mail host (TSLA base domain) + // did not have DANE TLSA records. + NoPolicyFound PolicyType = "no-policy-found" +) + +func (rp ResultPolicy) equal(orp ResultPolicy) bool { + return rp.Type == orp.Type && slices.Equal(rp.String, orp.String) && rp.Domain == orp.Domain && slices.Equal(rp.MXHost, orp.MXHost) } type Summary struct { @@ -112,17 +218,131 @@ const ( ResultSTSPolicyFetch ResultType = "sts-policy-fetch-error" ) +// todo spec: ../rfc/8460:719 more of these fields should be optional. some sts failure details, like failed policy fetches, won't have an ip or mx, the failure happens earlier in the delivery process. + type FailureDetails struct { ResultType ResultType `json:"result-type"` SendingMTAIP string `json:"sending-mta-ip"` ReceivingMXHostname string `json:"receiving-mx-hostname"` - ReceivingMXHelo string `json:"receiving-mx-helo"` + ReceivingMXHelo string `json:"receiving-mx-helo,omitempty"` ReceivingIP string `json:"receiving-ip"` FailedSessionCount int64 `json:"failed-session-count"` AdditionalInformation string `json:"additional-information"` FailureReasonCode string `json:"failure-reason-code"` } +// equalKey returns whether FailureDetails have the same values, expect for +// FailedSessionCount. Useful for aggregating FailureDetails. +func (fd FailureDetails) equalKey(ofd FailureDetails) bool { + fd.FailedSessionCount = 0 + ofd.FailedSessionCount = 0 + return fd == ofd +} + +// Details is a convenience function to compose a FailureDetails. +func Details(t ResultType, r string) FailureDetails { + return FailureDetails{ResultType: t, FailedSessionCount: 1, FailureReasonCode: r} +} + +var invalidReasons = map[x509.InvalidReason]string{ + x509.NotAuthorizedToSign: "not-authorized-to-sign", + x509.Expired: "certificate-expired", + x509.CANotAuthorizedForThisName: "ca-not-authorized-for-this-name", + x509.TooManyIntermediates: "too-many-intermediates", + x509.IncompatibleUsage: "incompatible-key-usage", + x509.NameMismatch: "parent-subject-child-issuer-mismatch", + x509.NameConstraintsWithoutSANs: "name-constraint-without-sans", + x509.UnconstrainedName: "unconstrained-name", + x509.TooManyConstraints: "too-many-constraints", + x509.CANotAuthorizedForExtKeyUsage: "ca-not-authorized-for-ext-key-usage", +} + +// TLSFailureDetails turns errors encountered during TLS handshakes into a result +// type and failure reason code for use with FailureDetails. +// +// Errors from crypto/tls, including local and remote alerts, from crypto/x509, +// and generic i/o and timeout errors are recognized. +func TLSFailureDetails(err error) (ResultType, string) { + var invalidErr x509.CertificateInvalidError + var hostErr x509.HostnameError + var unknownAuthErr x509.UnknownAuthorityError + var rootsErr x509.SystemRootsError + var verifyErr *tls.CertificateVerificationError + var netErr *net.OpError + var recordHdrErr tls.RecordHeaderError + if errors.As(err, &invalidErr) { + if invalidErr.Reason == x509.Expired { + // Result: ../rfc/8460:546 + return ResultCertificateExpired, "" + } + s, ok := invalidReasons[invalidErr.Reason] + if !ok { + s = fmt.Sprintf("go-x509-invalid-reason-%d", invalidErr.Reason) + } + // Result: ../rfc/8460:549 + return ResultCertificateNotTrusted, s + } else if errors.As(err, &hostErr) { + // Result: ../rfc/8460:541 + return ResultCertificateHostMismatch, "" + } else if errors.As(err, &unknownAuthErr) { + // Result: ../rfc/8460:549 + return ResultCertificateNotTrusted, "" + } else if errors.As(err, &rootsErr) { + // Result: ../rfc/8460:549 + return ResultCertificateNotTrusted, "no-system-roots" + } else if errors.As(err, &verifyErr) { + // We don't know a more specific error. ../rfc/8460:610 + // Result: ../rfc/8460:567 + return ResultValidationFailure, "unknown-go-certificate-verification-error" + } else if errors.As(err, &netErr) && netErr.Op == "remote error" { + // This is how TLS errors from the server (through an alert) are represented by + // crypto/tls. Err will usually be tls.alert error that is a type around uint8. + reasonCode := "tls-remote-error" + if netErr.Err != nil { + // todo: ideally, crypto/tls would let us check if this is an alert. it could be another uint8-typed error. + v := reflect.ValueOf(netErr.Err) + if v.Kind() == reflect.Uint8 && v.Type().Name() == "alert" { + reasonCode = "tls-remote-" + formatAlert(uint8(v.Uint())) + } + } + return ResultValidationFailure, reasonCode + } else if errors.As(err, &recordHdrErr) { + // Like for AlertError, not a lot of details, but better than nothing. + // Result: ../rfc/8460:567 + return ResultValidationFailure, "tls-record-header-error" + } + + // Consider not adding failure details at all for transient errors? It probably + // isn't very common to have an accidental connection failure during STARTTL setup + // after having completed SMTP TCP setup and having exchanged commands. Seems best + // to report on them. ../rfc/8460:625 + // Could be any other kind of error, we try to report on i/o errors, but best not to claim any + // other reason we don't know about. ../rfc/8460:610 + // Result: ../rfc/8460:567 + var reasonCode string + if errors.Is(err, os.ErrDeadlineExceeded) || errors.Is(err, context.DeadlineExceeded) { + reasonCode = "io-timeout-during-handshake" + } else if moxio.IsClosed(err) || errors.Is(err, io.ErrClosedPipe) { + reasonCode = "connection-closed-during-handshake" + } else { + // Attempt to get a local, outgoing TLS alert. + // We unwrap the error to the end (not multiple errors), and check for uint8 of a + // type named "alert". + for { + uerr := errors.Unwrap(err) + if uerr == nil { + break + } + err = uerr + } + v := reflect.ValueOf(err) + if v.Kind() == reflect.Uint8 && v.Type().Name() == "alert" { + reasonCode = "tls-local-" + formatAlert(uint8(v.Uint())) + } + } + return ResultValidationFailure, reasonCode +} + // Parse parses a Report. // The maximum size is 20MB. func Parse(r io.Reader) (*Report, error) { diff --git a/tlsrpt/report_test.go b/tlsrpt/report_test.go index f5f507f..99206d2 100644 --- a/tlsrpt/report_test.go +++ b/tlsrpt/report_test.go @@ -1,10 +1,19 @@ package tlsrpt import ( + "context" + "crypto/ed25519" + cryptorand "crypto/rand" + "crypto/tls" + "crypto/x509" "encoding/json" + "io" + "math/big" + "net" "os" "strings" "testing" + "time" ) const reportJSON = `{ @@ -141,6 +150,165 @@ func TestReport(t *testing.T) { } } +func TestTLSFailureDetails(t *testing.T) { + const alert70 = "tls-remote-alert-70-protocol-version-not-supported" + + test := func(expResultType ResultType, expReasonCode string, client func(net.Conn) error, server func(net.Conn)) { + t.Helper() + + cconn, sconn := net.Pipe() + defer cconn.Close() + defer sconn.Close() + go server(sconn) + err := client(cconn) + if err == nil { + t.Fatalf("expected tls error") + } + + resultType, reasonCode := TLSFailureDetails(err) + if resultType != expResultType || !(reasonCode == expReasonCode || expReasonCode == alert70 && reasonCode == "tls-remote-alert-70") { + t.Fatalf("got %v %v, expected %v %v", resultType, reasonCode, expResultType, expReasonCode) + } + } + + newPool := func(certs ...tls.Certificate) *x509.CertPool { + pool := x509.NewCertPool() + for _, cert := range certs { + pool.AddCert(cert.Leaf) + } + return pool + } + + // Expired certificate. + expiredCert := fakeCert(t, "localhost", true) + test(ResultCertificateExpired, "", + func(conn net.Conn) error { + config := tls.Config{ServerName: "localhost", RootCAs: newPool(expiredCert)} + return tls.Client(conn, &config).Handshake() + }, + func(conn net.Conn) { + config := tls.Config{Certificates: []tls.Certificate{expiredCert}} + tls.Server(conn, &config).Handshake() + }, + ) + + // Hostname mismatch. + okCert := fakeCert(t, "localhost", false) + test(ResultCertificateHostMismatch, "", func(conn net.Conn) error { + config := tls.Config{ServerName: "otherhost", RootCAs: newPool(okCert)} + return tls.Client(conn, &config).Handshake() + }, + func(conn net.Conn) { + config := tls.Config{Certificates: []tls.Certificate{okCert}} + tls.Server(conn, &config).Handshake() + }, + ) + + // Not signed by trusted CA. + test(ResultCertificateNotTrusted, "", func(conn net.Conn) error { + config := tls.Config{ServerName: "localhost", RootCAs: newPool()} + return tls.Client(conn, &config).Handshake() + }, + func(conn net.Conn) { + config := tls.Config{Certificates: []tls.Certificate{okCert}} + tls.Server(conn, &config).Handshake() + }, + ) + + // We don't support the right protocol version. + test(ResultValidationFailure, alert70, func(conn net.Conn) error { + config := tls.Config{ServerName: "localhost", RootCAs: newPool(okCert), MinVersion: tls.VersionTLS10, MaxVersion: tls.VersionTLS10} + return tls.Client(conn, &config).Handshake() + }, + func(conn net.Conn) { + config := tls.Config{Certificates: []tls.Certificate{okCert}, MinVersion: tls.VersionTLS12} + tls.Server(conn, &config).Handshake() + }, + ) + + // todo: ideally a test for tls-local-alert-* + + // Remote is not speaking TLS. + test(ResultValidationFailure, "tls-record-header-error", func(conn net.Conn) error { + config := tls.Config{ServerName: "localhost", RootCAs: newPool(okCert)} + return tls.Client(conn, &config).Handshake() + }, + func(conn net.Conn) { + go io.Copy(io.Discard, conn) + buf := make([]byte, 128) + for { + _, err := conn.Write(buf) + if err != nil { + break + } + } + }, + ) + + // Context deadline exceeded during handshake. + test(ResultValidationFailure, "io-timeout-during-handshake", + func(conn net.Conn) error { + config := tls.Config{ServerName: "localhost", RootCAs: newPool(okCert)} + ctx, cancel := context.WithTimeout(context.Background(), 1) + defer cancel() + return tls.Client(conn, &config).HandshakeContext(ctx) + }, + func(conn net.Conn) {}, + ) + + // Timeout during handshake. + test(ResultValidationFailure, "io-timeout-during-handshake", + func(conn net.Conn) error { + config := tls.Config{ServerName: "localhost", RootCAs: newPool(okCert)} + conn.SetDeadline(time.Now()) + return tls.Client(conn, &config).Handshake() + }, + func(conn net.Conn) {}, + ) + + // Closing connection during handshake. + test(ResultValidationFailure, "connection-closed-during-handshake", func(conn net.Conn) error { + config := tls.Config{ServerName: "localhost", RootCAs: newPool(okCert)} + return tls.Client(conn, &config).Handshake() + }, + func(conn net.Conn) { + conn.Close() + }, + ) +} + +// Just a cert that appears valid. +func fakeCert(t *testing.T, name string, expired bool) tls.Certificate { + notAfter := time.Now() + if expired { + notAfter = notAfter.Add(-time.Hour) + } else { + notAfter = notAfter.Add(time.Hour) + } + + privKey := ed25519.NewKeyFromSeed(make([]byte, ed25519.SeedSize)) // Fake key, don't use this for real! + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), // Required field... + DNSNames: []string{name}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: notAfter, + } + localCertBuf, err := x509.CreateCertificate(cryptorand.Reader, template, template, privKey.Public(), privKey) + if err != nil { + t.Fatalf("making certificate: %s", err) + } + cert, err := x509.ParseCertificate(localCertBuf) + if err != nil { + t.Fatalf("parsing generated certificate: %s", err) + } + c := tls.Certificate{ + Certificate: [][]byte{localCertBuf}, + PrivateKey: privKey, + Leaf: cert, + } + return c +} + func FuzzParseMessage(f *testing.F) { f.Add(tlsrptMessage) f.Fuzz(func(t *testing.T, s string) { diff --git a/tlsrptdb/db.go b/tlsrptdb/db.go index cb340a5..a803657 100644 --- a/tlsrptdb/db.go +++ b/tlsrptdb/db.go @@ -1,194 +1,50 @@ -// Package tlsrptdb stores reports from "SMTP TLS Reporting" in its database. package tlsrptdb import ( - "context" - "fmt" - "os" - "path/filepath" "sync" - "time" - - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" "github.com/mjl-/bstore" - "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/mox-" - "github.com/mjl-/mox/tlsrpt" ) var ( xlog = mlog.New("tlsrptdb") - DBTypes = []any{TLSReportRecord{}} - DB *bstore.DB - mutex sync.Mutex + ReportDBTypes = []any{TLSReportRecord{}} + ReportDB *bstore.DB + mutex sync.Mutex - metricSession = promauto.NewCounterVec( - prometheus.CounterOpts{ - Name: "mox_tlsrptdb_session_total", - Help: "Number of sessions, both success and known result types.", - }, - []string{"type"}, // Known result types, and "success" - ) - - knownResultTypes = map[tlsrpt.ResultType]struct{}{ - tlsrpt.ResultSTARTTLSNotSupported: {}, - tlsrpt.ResultCertificateHostMismatch: {}, - tlsrpt.ResultCertificateExpired: {}, - tlsrpt.ResultTLSAInvalid: {}, - tlsrpt.ResultDNSSECInvalid: {}, - tlsrpt.ResultDANERequired: {}, - tlsrpt.ResultCertificateNotTrusted: {}, - tlsrpt.ResultSTSPolicyInvalid: {}, - tlsrpt.ResultSTSWebPKIInvalid: {}, - tlsrpt.ResultValidationFailure: {}, - tlsrpt.ResultSTSPolicyFetch: {}, - } + // Accessed directly by tlsrptsend. + ResultDBTypes = []any{TLSResult{}} + ResultDB *bstore.DB ) -// TLSReportRecord is a TLS report as a database record, including information -// about the sender. -// -// todo: should be named just Record, but it would cause a sherpa type name conflict. -type TLSReportRecord struct { - ID int64 `bstore:"typename Record"` - Domain string `bstore:"index"` // Domain to which the TLS report applies. - FromDomain string - MailFrom string - Report tlsrpt.Report -} - -func database(ctx context.Context) (rdb *bstore.DB, rerr error) { - mutex.Lock() - defer mutex.Unlock() - if DB == nil { - p := mox.DataDirPath("tlsrpt.db") - os.MkdirAll(filepath.Dir(p), 0770) - db, err := bstore.Open(ctx, p, &bstore.Options{Timeout: 5 * time.Second, Perm: 0660}, DBTypes...) - if err != nil { - return nil, err - } - DB = db - } - return DB, nil -} - -// Init opens and possibly initializes the database. +// Init opens and possibly initializes the databases. func Init() error { - _, err := database(mox.Shutdown) - return err -} - -// Close closes the database connection. -func Close() { - mutex.Lock() - defer mutex.Unlock() - if DB != nil { - err := DB.Close() - xlog.Check(err, "closing database") - DB = nil - } -} - -// AddReport adds a TLS report to the database. -// -// The report should have come in over SMTP, with a DKIM-validated -// verifiedFromDomain. Using HTTPS for reports is not recommended as there is no -// authentication on the reports origin. -// -// The report is currently required to only cover a single domain in its policy -// domain. Only reports for known domains are added to the database. -// -// Prometheus metrics are updated only for configured domains. -func AddReport(ctx context.Context, verifiedFromDomain dns.Domain, mailFrom string, r *tlsrpt.Report) error { - log := xlog.WithContext(ctx) - - db, err := database(ctx) - if err != nil { + if _, err := reportDB(mox.Shutdown); err != nil { return err } - - if len(r.Policies) == 0 { - return fmt.Errorf("no policies in report") + if _, err := resultDB(mox.Shutdown); err != nil { + return err } - - var reportdom, zerodom dns.Domain - record := TLSReportRecord{0, "", verifiedFromDomain.Name(), mailFrom, *r} - - for _, p := range r.Policies { - pp := p.Policy - - // Check domain, they must all be the same for now (in future, with DANE, this may - // no longer apply). - d, err := dns.ParseDomain(pp.Domain) - if err != nil { - log.Errorx("invalid domain in tls report", err, mlog.Field("domain", pp.Domain), mlog.Field("mailfrom", mailFrom)) - continue - } - if _, ok := mox.Conf.Domain(d); !ok { - log.Info("unknown domain in tls report, not storing", mlog.Field("domain", d), mlog.Field("mailfrom", mailFrom)) - return fmt.Errorf("unknown domain") - } - if reportdom != zerodom && d != reportdom { - return fmt.Errorf("multiple domains in report %s and %s", reportdom, d) - } - reportdom = d - - metricSession.WithLabelValues("success").Add(float64(p.Summary.TotalSuccessfulSessionCount)) - for _, f := range p.FailureDetails { - var result string - if _, ok := knownResultTypes[f.ResultType]; ok { - result = string(f.ResultType) - } else { - result = "other" - } - metricSession.WithLabelValues(result).Add(float64(f.FailedSessionCount)) - } - } - record.Domain = reportdom.Name() - return db.Insert(ctx, &record) + return nil } -// Records returns all TLS reports in the database. -func Records(ctx context.Context) ([]TLSReportRecord, error) { - db, err := database(ctx) - if err != nil { - return nil, err +// Close closes the database connections. +func Close() { + if ResultDB != nil { + err := ResultDB.Close() + xlog.Check(err, "closing result database") + ResultDB = nil + } + + mutex.Lock() + defer mutex.Unlock() + if ReportDB != nil { + err := ReportDB.Close() + xlog.Check(err, "closing report database") + ReportDB = nil } - return bstore.QueryDB[TLSReportRecord](ctx, db).List() -} - -// RecordID returns the report for the ID. -func RecordID(ctx context.Context, id int64) (TLSReportRecord, error) { - db, err := database(ctx) - if err != nil { - return TLSReportRecord{}, err - } - - e := TLSReportRecord{ID: id} - err = db.Get(ctx, &e) - return e, err -} - -// RecordsPeriodDomain returns the reports overlapping start and end, for the given -// domain. If domain is empty, all records match for domain. -func RecordsPeriodDomain(ctx context.Context, start, end time.Time, domain string) ([]TLSReportRecord, error) { - db, err := database(ctx) - if err != nil { - return nil, err - } - - q := bstore.QueryDB[TLSReportRecord](ctx, db) - if domain != "" { - q.FilterNonzero(TLSReportRecord{Domain: domain}) - } - q.FilterFn(func(r TLSReportRecord) bool { - dr := r.Report.DateRange - return !dr.Start.Before(start) && dr.Start.Before(end) || dr.End.After(start) && !dr.End.After(end) - }) - return q.List() } diff --git a/tlsrptdb/report.go b/tlsrptdb/report.go new file mode 100644 index 0000000..15f499b --- /dev/null +++ b/tlsrptdb/report.go @@ -0,0 +1,171 @@ +// Package tlsrptdb stores reports from "SMTP TLS Reporting" in its database. +package tlsrptdb + +import ( + "context" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + + "github.com/mjl-/bstore" + + "github.com/mjl-/mox/dns" + "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/mox-" + "github.com/mjl-/mox/tlsrpt" +) + +var ( + metricSession = promauto.NewCounterVec( + prometheus.CounterOpts{ + Name: "mox_tlsrptdb_session_total", + Help: "Number of sessions, both success and known result types.", + }, + []string{"type"}, // Known result types, and "success" + ) + + knownResultTypes = map[tlsrpt.ResultType]struct{}{ + tlsrpt.ResultSTARTTLSNotSupported: {}, + tlsrpt.ResultCertificateHostMismatch: {}, + tlsrpt.ResultCertificateExpired: {}, + tlsrpt.ResultTLSAInvalid: {}, + tlsrpt.ResultDNSSECInvalid: {}, + tlsrpt.ResultDANERequired: {}, + tlsrpt.ResultCertificateNotTrusted: {}, + tlsrpt.ResultSTSPolicyInvalid: {}, + tlsrpt.ResultSTSWebPKIInvalid: {}, + tlsrpt.ResultValidationFailure: {}, + tlsrpt.ResultSTSPolicyFetch: {}, + } +) + +// TLSReportRecord is a TLS report as a database record, including information +// about the sender. +// +// todo: should be named just Record, but it would cause a sherpa type name conflict. +type TLSReportRecord struct { + ID int64 `bstore:"typename Record"` + Domain string `bstore:"index"` // Domain to which the TLS report applies. + FromDomain string + MailFrom string + HostReport bool // Report for host TLSRPT record, as opposed to domain TLSRPT record. + Report tlsrpt.Report +} + +func reportDB(ctx context.Context) (rdb *bstore.DB, rerr error) { + mutex.Lock() + defer mutex.Unlock() + if ReportDB == nil { + p := mox.DataDirPath("tlsrpt.db") + os.MkdirAll(filepath.Dir(p), 0770) + db, err := bstore.Open(ctx, p, &bstore.Options{Timeout: 5 * time.Second, Perm: 0660}, ReportDBTypes...) + if err != nil { + return nil, err + } + ReportDB = db + } + return ReportDB, nil +} + +// AddReport adds a TLS report to the database. +// +// The report should have come in over SMTP, with a DKIM-validated +// verifiedFromDomain. Using HTTPS for reports is not recommended as there is no +// authentication on the reports origin. +// +// The report is currently required to only cover a single domain in its policy +// domain. Only reports for known domains are added to the database. +// +// Prometheus metrics are updated only for configured domains. +func AddReport(ctx context.Context, verifiedFromDomain dns.Domain, mailFrom string, hostReport bool, r *tlsrpt.Report) error { + log := xlog.WithContext(ctx) + + db, err := reportDB(ctx) + if err != nil { + return err + } + + if len(r.Policies) == 0 { + return fmt.Errorf("no policies in report") + } + + var reportdom, zerodom dns.Domain + record := TLSReportRecord{0, "", verifiedFromDomain.Name(), mailFrom, hostReport, *r} + + for _, p := range r.Policies { + pp := p.Policy + + // Check domain, they must all be the same for now (in future, with DANE, this may + // no longer apply). + d, err := dns.ParseDomain(pp.Domain) + if err != nil { + log.Errorx("invalid domain in tls report", err, mlog.Field("domain", pp.Domain), mlog.Field("mailfrom", mailFrom)) + continue + } + if _, ok := mox.Conf.Domain(d); !ok { + log.Info("unknown domain in tls report, not storing", mlog.Field("domain", d), mlog.Field("mailfrom", mailFrom)) + return fmt.Errorf("unknown domain") + } + if reportdom != zerodom && d != reportdom { + return fmt.Errorf("multiple domains in report %s and %s", reportdom, d) + } + reportdom = d + + metricSession.WithLabelValues("success").Add(float64(p.Summary.TotalSuccessfulSessionCount)) + for _, f := range p.FailureDetails { + var result string + if _, ok := knownResultTypes[f.ResultType]; ok { + result = string(f.ResultType) + } else { + result = "other" + } + metricSession.WithLabelValues(result).Add(float64(f.FailedSessionCount)) + } + } + record.Domain = reportdom.Name() + return db.Insert(ctx, &record) +} + +// Records returns all TLS reports in the database. +func Records(ctx context.Context) ([]TLSReportRecord, error) { + db, err := reportDB(ctx) + if err != nil { + return nil, err + } + return bstore.QueryDB[TLSReportRecord](ctx, db).List() +} + +// RecordID returns the report for the ID. +func RecordID(ctx context.Context, id int64) (TLSReportRecord, error) { + db, err := reportDB(ctx) + if err != nil { + return TLSReportRecord{}, err + } + + e := TLSReportRecord{ID: id} + err = db.Get(ctx, &e) + return e, err +} + +// RecordsPeriodDomain returns the reports overlapping start and end, for the given +// domain. If domain is empty, all records match for domain. +func RecordsPeriodDomain(ctx context.Context, start, end time.Time, domain string) ([]TLSReportRecord, error) { + db, err := reportDB(ctx) + if err != nil { + return nil, err + } + + q := bstore.QueryDB[TLSReportRecord](ctx, db) + if domain != "" { + q.FilterNonzero(TLSReportRecord{Domain: domain}) + } + q.FilterFn(func(r TLSReportRecord) bool { + dr := r.Report.DateRange + return !dr.Start.Before(start) && dr.Start.Before(end) || dr.End.After(start) && !dr.End.After(end) + }) + return q.List() +} diff --git a/tlsrptdb/db_test.go b/tlsrptdb/report_test.go similarity index 95% rename from tlsrptdb/db_test.go rename to tlsrptdb/report_test.go index 5f32ca8..818065f 100644 --- a/tlsrptdb/db_test.go +++ b/tlsrptdb/report_test.go @@ -73,6 +73,7 @@ func TestReport(t *testing.T) { dbpath := mox.DataDirPath("tlsrpt.db") os.MkdirAll(filepath.Dir(dbpath), 0770) defer os.Remove(dbpath) + defer os.Remove(mox.DataDirPath("tlsrptresult.db")) if err := Init(); err != nil { t.Fatalf("init database: %s", err) @@ -93,7 +94,7 @@ func TestReport(t *testing.T) { if err != nil { t.Fatalf("parsing TLSRPT from message %q: %s", file.Name(), err) } - if err := AddReport(ctxbg, dns.Domain{ASCII: "mox.example"}, "tlsrpt@mox.example", report); err != nil { + if err := AddReport(ctxbg, dns.Domain{ASCII: "mox.example"}, "tlsrpt@mox.example", false, report); err != nil { t.Fatalf("adding report to database: %s", err) } } @@ -101,7 +102,7 @@ func TestReport(t *testing.T) { report, err := tlsrpt.Parse(strings.NewReader(reportJSON)) if err != nil { t.Fatalf("parsing report: %v", err) - } else if err := AddReport(ctxbg, dns.Domain{ASCII: "company-y.example"}, "tlsrpt@company-y.example", report); err != nil { + } else if err := AddReport(ctxbg, dns.Domain{ASCII: "company-y.example"}, "tlsrpt@company-y.example", false, report); err != nil { t.Fatalf("adding report to database: %s", err) } diff --git a/tlsrptdb/result.go b/tlsrptdb/result.go new file mode 100644 index 0000000..7ea13be --- /dev/null +++ b/tlsrptdb/result.go @@ -0,0 +1,161 @@ +package tlsrptdb + +import ( + "context" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/mjl-/bstore" + + "github.com/mjl-/mox/dns" + "github.com/mjl-/mox/mox-" + "github.com/mjl-/mox/tlsrpt" +) + +// TLSResult is stored in the database to track TLS results per policy domain, day +// and recipient domain. These records will be included in TLS reports. +type TLSResult struct { + ID int64 + + // Domain with TLSRPT DNS record, with addresses that will receive reports. Either + // a recipient domain (for MTA-STS policies) or an (MX) host (for DANE policies). + // Unicode. + PolicyDomain string `bstore:"unique PolicyDomain+DayUTC+RecipientDomain,nonzero"` + + // DayUTC is of the form yyyymmdd. + DayUTC string `bstore:"nonzero"` + // We send per 24h UTC-aligned days. ../rfc/8460:474 + + // Reports are sent per policy domain. When delivering a message to a recipient + // domain, we can get multiple TLSResults, typically one for MTA-STS, and one or + // more for DANE (one for each MX target, or actually TLSA base domain). We track + // recipient domain so we can display successes/failures for delivery of messages + // to a recipient domain in the admin pages. Unicode. + RecipientDomain string `bstore:"index,nonzero"` + + Created time.Time `bstore:"default now"` + Updated time.Time `bstore:"default now"` + + IsHost bool // Result is for host (e.g. DANE), not recipient domain (e.g. MTA-STS). + + // Whether to send a report. TLS results for delivering messages with TLS reports + // will be recorded, but will not cause a report to be sent. + SendReport bool + // ../rfc/8460:318 says we should not include TLS results for sending a TLS report, + // but presumably that's to prevent mail servers sending a report every day once + // they start. + + // Results is updated for each TLS attempt. + Results []tlsrpt.Result +} + +func resultDB(ctx context.Context) (rdb *bstore.DB, rerr error) { + mutex.Lock() + defer mutex.Unlock() + if ResultDB == nil { + p := mox.DataDirPath("tlsrptresult.db") + os.MkdirAll(filepath.Dir(p), 0770) + db, err := bstore.Open(ctx, p, &bstore.Options{Timeout: 5 * time.Second, Perm: 0660}, ResultDBTypes...) + if err != nil { + return nil, err + } + ResultDB = db + } + return ResultDB, nil +} + +// AddTLSResults adds or merges all tls results for delivering to a policy domain, +// on its UTC day to a recipient domain to the database. Results may cause multiple +// separate reports to be sent. +func AddTLSResults(ctx context.Context, results []TLSResult) error { + db, err := resultDB(ctx) + if err != nil { + return err + } + + now := time.Now() + + err = db.Write(ctx, func(tx *bstore.Tx) error { + for _, result := range results { + // Ensure all slices are non-nil. We do this now so all readers will marshal to + // compliant with the JSON schema. And also for consistent equality checks when + // merging policies created in different places. + for i, r := range result.Results { + if r.Policy.String == nil { + r.Policy.String = []string{} + } + if r.Policy.MXHost == nil { + r.Policy.MXHost = []string{} + } + if r.FailureDetails == nil { + r.FailureDetails = []tlsrpt.FailureDetails{} + } + result.Results[i] = r + } + + q := bstore.QueryTx[TLSResult](tx) + q.FilterNonzero(TLSResult{PolicyDomain: result.PolicyDomain, DayUTC: result.DayUTC, RecipientDomain: result.RecipientDomain}) + r, err := q.Get() + if err == bstore.ErrAbsent { + result.ID = 0 + if err := tx.Insert(&result); err != nil { + return fmt.Errorf("insert: %w", err) + } + continue + } else if err != nil { + return err + } + + report := tlsrpt.Report{Policies: r.Results} + report.Merge(result.Results...) + r.Results = report.Policies + + r.IsHost = result.IsHost + if result.SendReport { + r.SendReport = true + } + r.Updated = now + if err := tx.Update(&r); err != nil { + return fmt.Errorf("update: %w", err) + } + } + return nil + }) + return err +} + +// Results returns all TLS results in the database, for all policy domains each +// with potentially multiple days. Sorted by RecipientDomain and day. +func Results(ctx context.Context) ([]TLSResult, error) { + db, err := resultDB(ctx) + if err != nil { + return nil, err + } + + return bstore.QueryDB[TLSResult](ctx, db).SortAsc("PolicyDomain", "DayUTC", "RecipientDomain").List() +} + +// ResultsPolicyDomain returns all TLSResults for a policy domain, potentially for +// multiple days. +func ResultsPolicyDomain(ctx context.Context, policyDomain dns.Domain) ([]TLSResult, error) { + db, err := resultDB(ctx) + if err != nil { + return nil, err + } + + return bstore.QueryDB[TLSResult](ctx, db).FilterNonzero(TLSResult{PolicyDomain: policyDomain.Name()}).SortAsc("DayUTC", "RecipientDomain").List() +} + +// RemoveResultsPolicyDomain removes all TLSResults for the policy domain on the +// day from the database. +func RemoveResultsPolicyDomain(ctx context.Context, policyDomain dns.Domain, dayUTC string) error { + db, err := resultDB(ctx) + if err != nil { + return err + } + + _, err = bstore.QueryDB[TLSResult](ctx, db).FilterNonzero(TLSResult{PolicyDomain: policyDomain.Name(), DayUTC: dayUTC}).Delete() + return err +} diff --git a/tlsrptsend/send.go b/tlsrptsend/send.go new file mode 100644 index 0000000..540a716 --- /dev/null +++ b/tlsrptsend/send.go @@ -0,0 +1,549 @@ +// Package tlsrptsend sends TLS reports based on success/failure statistics and +// details gathering while making SMTP STARTTLS connections for delivery. See RFC +// 8460. +package tlsrptsend + +// tlsrptsend is a separate package instead of being in tlsrptdb because it imports +// queue and queue imports tlsrptdb to store tls results, so that would cause a +// cyclic dependency. + +// Sending TLS reports and DMARC reports is very similar. See ../dmarcdb/eval.go:/similar and ../tlsrptsend/send.go:/similar. + +// todo spec: ../rfc/8460:441 ../rfc/8460:463 may lead reader to believe they can find a DANE or MTA-STS policy at the same place, while in practice you'll get an MTA-STS policy at a recipient domain and a DANE policy at a mail host, and that's where the TLSRPT policy is defined. it would have helped with this implementation if the distinction was mentioned explicitly, also earlier in the document (i realized it late in the implementation process based on the terminology entry for the policy domain). examples with a tlsrpt record at a mail host would have helped too. +// todo spec: ../rfc/8460:1017 example report message misses the required DKIM signature. + +import ( + "compress/gzip" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "mime" + "mime/multipart" + "net/textproto" + "net/url" + "os" + "runtime/debug" + "strings" + "sync" + "time" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + + "github.com/mjl-/bstore" + + "github.com/mjl-/mox/config" + "github.com/mjl-/mox/dkim" + "github.com/mjl-/mox/dns" + "github.com/mjl-/mox/message" + "github.com/mjl-/mox/metrics" + "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/mox-" + "github.com/mjl-/mox/moxio" + "github.com/mjl-/mox/moxvar" + "github.com/mjl-/mox/queue" + "github.com/mjl-/mox/smtp" + "github.com/mjl-/mox/store" + "github.com/mjl-/mox/tlsrpt" + "github.com/mjl-/mox/tlsrptdb" +) + +var ( + metricReport = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "mox_tlsrptsend_report_queued_total", + Help: "Total messages with TLS reports queued.", + }, + ) + metricReportError = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "mox_tlsrptsend_report_error_total", + Help: "Total errors while composing or queueing TLS reports.", + }, + ) +) + +var jitterRand = mox.NewPseudoRand() + +// time to sleep until sending reports at midnight t, replaced by tests. +// Jitter so we don't cause load at exactly midnight, other processes may +// already be doing that. +var jitteredTimeUntil = func(t time.Time) time.Duration { + return time.Until(t.Add(time.Duration(240+jitterRand.Intn(120)) * time.Second)) +} + +// Start launches a goroutine that wakes up just after 00:00 UTC to send TLSRPT +// reports. Reports are sent spread out over a 4 hour period. +func Start(resolver dns.Resolver) { + go func() { + log := mlog.New("tlsrptsend") + + defer func() { + // In case of panic don't take the whole program down. + x := recover() + if x != nil { + log.Error("recover from panic", mlog.Field("panic", x)) + debug.PrintStack() + metrics.PanicInc(metrics.Tlsrptdb) + } + }() + + timer := time.NewTimer(time.Hour) // Reset below. + defer timer.Stop() + + ctx := mox.Shutdown + + db := tlsrptdb.ResultDB + if db == nil { + log.Error("no tlsrpt results database for tls reports, not sending reports") + return + } + + // We start sending for previous day, if there are any reports left. + endUTC := midnightUTC(time.Now()) + + for { + dayUTC := endUTC.Add(-12 * time.Hour).Format("20060102") + + // Remove evaluations older than 48 hours (2 reports with 24 hour interval) + // They should have been processed by now. We may have kept them + // during temporary errors, but persistent temporary errors shouldn't fill up our + // database and we don't want to send old reports either. + _, err := bstore.QueryDB[tlsrptdb.TLSResult](ctx, db).FilterLess("DayUTC", endUTC.Add((-48-12)*time.Hour).Format("20060102")).Delete() + log.Check(err, "removing stale tls results from database") + + log.Info("sending tls reports", mlog.Field("day", dayUTC)) + if err := sendReports(ctx, log.WithCid(mox.Cid()), resolver, db, dayUTC, endUTC); err != nil { + log.Errorx("sending tls reports", err) + metricReportError.Inc() + } else { + log.Info("finished sending tls reports") + } + + endUTC = endUTC.Add(24 * time.Hour) + timer.Reset(jitteredTimeUntil(endUTC)) + + select { + case <-ctx.Done(): + log.Info("tls report sender shutting down") + return + case <-timer.C: + } + } + }() +} + +func midnightUTC(now time.Time) time.Time { + t := now.UTC() + return time.Date(t.Year(), t.Month(), t.Day(), 0, 0, 0, 0, t.Location()) +} + +// Sleep in between sending two reports. +// Replaced by tests. +var sleepBetween = func(ctx context.Context, between time.Duration) (ok bool) { + t := time.NewTimer(between) + select { + case <-ctx.Done(): + t.Stop() + return false + case <-t.C: + return true + } +} + +// sendReports gathers all policy domains that have results that should receive a +// TLS report and sends a report to each if their TLSRPT DNS record has reporting +// addresses. +func sendReports(ctx context.Context, log *mlog.Log, resolver dns.Resolver, db *bstore.DB, dayUTC string, endTimeUTC time.Time) error { + type key struct { + policyDomain string + dayUTC string + } + destDomains := map[key]bool{} + + // Gather all policy domains we plan to send to. + var nsend int + q := bstore.QueryDB[tlsrptdb.TLSResult](ctx, db) + q.FilterLessEqual("DayUTC", dayUTC) + q.SortAsc("PolicyDomain", "DayUTC", "RecipientDomain") // Sort for testability. + err := q.ForEach(func(e tlsrptdb.TLSResult) error { + k := key{e.PolicyDomain, dayUTC} + if e.SendReport && !destDomains[k] { + nsend++ + } + destDomains[k] = destDomains[k] || e.SendReport + return nil + }) + if err != nil { + return fmt.Errorf("looking for domains to send tls reports to: %v", err) + } + + // Send report to each domain. We stretch sending over 4 hours, but only if there + // are quite a few message. ../rfc/8460:479 + between := 4 * time.Hour + if nsend > 0 { + between = between / time.Duration(nsend) + } + if between > 5*time.Minute { + between = 5 * time.Minute + } + + var wg sync.WaitGroup + + var n int + for k, send := range destDomains { + // Cleanup results for domain that doesn't need to get a report (e.g. for TLS + // connections that were the result of delivering TLSRPT messages). + if !send { + removeResults(ctx, log, db, k.policyDomain, k.dayUTC) + continue + } + + if n > 0 { + ok := sleepBetween(ctx, between) + if !ok { + return nil + } + } + n++ + + // In goroutine, so our timing stays independent of how fast we process. + wg.Add(1) + go func(policyDomain string, dayUTC string) { + defer func() { + // In case of panic don't take the whole program down. + x := recover() + if x != nil { + log.Error("unhandled panic in tlsrptsend sendReports", mlog.Field("panic", x)) + debug.PrintStack() + metrics.PanicInc(metrics.Tlsrptdb) + } + }() + defer wg.Done() + + rlog := log.WithCid(mox.Cid()).Fields(mlog.Field("policydomain", policyDomain), mlog.Field("daytutc", dayUTC)) + if _, err := sendReportDomain(ctx, rlog, resolver, db, endTimeUTC, policyDomain, dayUTC); err != nil { + rlog.Errorx("sending tls report to domain", err) + metricReportError.Inc() + } + }(k.policyDomain, k.dayUTC) + } + + wg.Wait() + + return nil +} + +func removeResults(ctx context.Context, log *mlog.Log, db *bstore.DB, policyDomain string, dayUTC string) { + q := bstore.QueryDB[tlsrptdb.TLSResult](ctx, db) + q.FilterNonzero(tlsrptdb.TLSResult{PolicyDomain: policyDomain, DayUTC: dayUTC}) + _, err := q.Delete() + log.Check(err, "removing tls results from database") +} + +// replaceable for testing. +var queueAdd = queue.Add + +func sendReportDomain(ctx context.Context, log *mlog.Log, resolver dns.Resolver, db *bstore.DB, endUTC time.Time, policyDomain, dayUTC string) (cleanup bool, rerr error) { + dom, err := dns.ParseDomain(policyDomain) + if err != nil { + return false, fmt.Errorf("parsing policy domain for sending tls reports: %v", err) + } + + // We'll cleanup records by default. + cleanup = true + // But if we encounter a temporary error we cancel cleanup of evaluations on error. + tempError := false + + defer func() { + if !cleanup || tempError { + log.Debug("not cleaning up results after attempting to send tls report") + } else { + removeResults(ctx, log, db, policyDomain, dayUTC) + } + }() + + // Get TLSRPT record. If there are no reporting addresses, we're not going to send at all. + record, _, err := tlsrpt.Lookup(ctx, resolver, dom) + if err != nil { + // If there is no TLSRPT record, that's fine, we'll remove what we tracked. + if errors.Is(err, tlsrpt.ErrNoRecord) { + return true, nil + } + cleanup = errors.Is(err, tlsrpt.ErrDNS) + return cleanup, fmt.Errorf("looking up current tlsrpt record for reporting addresses: %v", err) + } + + var recipients []smtp.Address + + for _, l := range record.RUAs { + for _, s := range l { + u, err := url.Parse(s) + if err != nil { + log.Debugx("parsing rua uri in tlsrpt dns record, ignoring", err, mlog.Field("rua", s)) + continue + } + + if u.Scheme == "mailto" { + addr, err := smtp.ParseAddress(u.Opaque) + if err != nil { + log.Debugx("parsing mailto uri in tlsrpt record rua value, ignoring", err, mlog.Field("rua", s)) + continue + } + recipients = append(recipients, addr) + } else if u.Scheme == "https" { + // Although "report" is ambiguous and could mean both only the JSON data or an + // entire message (including DKIM-Signature) with the JSON data, it appears the + // intention of the RFC is that the HTTPS transport sends only the JSON data, given + // mention of the media type to use (for the HTTP POST). It is the type of the + // report, not of a message. TLS reports sent over email must have a DKIM + // signature, i.e. must be authenticated, for understandable reasons. No such + // requirement is specified for HTTPS, but no one is going to accept + // unauthenticated TLS reports over HTTPS. So there seems little point in sending + // them. + // ../rfc/8460:320 ../rfc/8460:1055 + // todo spec: would be good to have clearer distinction between "report" (JSON) and "report message" (message with report attachment, that can be DKIM signed). propose sending report message over https that includes DKIM signature so authenticity can be verified and the report used. ../rfc/8460:310 + log.Debug("https scheme in rua uri in tlsrpt record, ignoring since they will likey not be used to due lack of authentication", mlog.Field("rua", s)) + } else { + log.Debug("unknown scheme in rua uri in tlsrpt record, ignoring", mlog.Field("rua", s)) + } + } + } + + if len(recipients) == 0 { + // No reports requested, perfectly fine, no work to do for us. + log.Debug("no tlsrpt reporting addresses configured") + return true, nil + } + + log.Info("sending tlsrpt report") + + q := bstore.QueryDB[tlsrptdb.TLSResult](ctx, db) + q.FilterNonzero(tlsrptdb.TLSResult{PolicyDomain: policyDomain, DayUTC: dayUTC}) + tlsResults, err := q.List() + if err != nil { + return true, fmt.Errorf("get tls results from database: %v", err) + } + + if len(tlsResults) == 0 { + // Should not happen. But no point in sending messages with empty reports. + return true, fmt.Errorf("no tls results found") + } + + beginUTC := endUTC.Add(-24 * time.Hour) + + report := tlsrpt.Report{ + OrganizationName: mox.Conf.Static.HostnameDomain.ASCII, + DateRange: tlsrpt.TLSRPTDateRange{ + Start: beginUTC, + End: endUTC.Add(-time.Second), // Per example, ../rfc/8460:1769 + }, + ContactInfo: "postmaster@" + mox.Conf.Static.HostnameDomain.ASCII, + // todo spec: ../rfc/8460:968 ../rfc/8460:1772 ../rfc/8460:691 subject header assumes a report-id in the form of a msg-id, but example and report-id json field explanation allows free-form report-id's (assuming we're talking about the same report-id here). + ReportID: endUTC.Format("20060102") + "." + dom.ASCII + "@" + mox.Conf.Static.HostnameDomain.ASCII, + } + + // Merge all results into this report. + for _, tlsResult := range tlsResults { + report.Merge(tlsResult.Results...) + } + + reportFile, err := store.CreateMessageTemp("tlsreportout") + if err != nil { + return false, fmt.Errorf("creating temporary file for outgoing tls report: %v", err) + } + defer store.CloseRemoveTempFile(log, reportFile, "generated tls report") + + // ../rfc/8460:905 + gzw := gzip.NewWriter(reportFile) + enc := json.NewEncoder(gzw) + enc.SetIndent("", "\t") + if err == nil { + err = enc.Encode(report) + } + if err == nil { + err = gzw.Close() + } + if err != nil { + return false, fmt.Errorf("writing tls report as json with gzip: %v", err) + } + + msgf, err := store.CreateMessageTemp("tlsreportmsgout") + if err != nil { + return false, fmt.Errorf("creating temporary message file with outgoing tls report: %v", err) + } + defer store.CloseRemoveTempFile(log, msgf, "message with generated tls report") + + // We are sending reports from our host's postmaster address. In a + // typical setup the host is a subdomain of a configured domain with + // DKIM keys, so we can DKIM-sign our reports. SPF should pass anyway. + // todo future: when sending, use an SMTP MAIL FROM that we can relate back to recipient reporting address so we can stop trying to send reports in case of repeated delivery failure DSNs. + from := smtp.Address{Localpart: "postmaster", Domain: mox.Conf.Static.HostnameDomain} + + // Subject follows the form from RFC. ../rfc/8460:959 + subject := fmt.Sprintf("Report Domain: %s Submitter: %s Report-ID: <%s>", dom.ASCII, mox.Conf.Static.HostnameDomain.ASCII, report.ReportID) + + // Human-readable part for convenience. ../rfc/8460:917 + text := fmt.Sprintf(` +Attached is a TLS report with a summary of connection successes and failures +during attempts to securely deliver messages to your mail server, including +details about errors encountered. You are receiving this message because your +address is specified in the "rua" field of the TLSRPT record for your +domain/host. + +Policy Domain: %s +Submitter: %s +Report-ID: %s +Period: %s - %s UTC +`, dom, mox.Conf.Static.HostnameDomain, report.ReportID, beginUTC.Format(time.DateTime), endUTC.Format(time.DateTime)) + + // The attached file follows the naming convention from the RFC. ../rfc/8460:849 + reportFilename := fmt.Sprintf("%s!%s!%d!%d.json.gz", mox.Conf.Static.HostnameDomain.ASCII, dom.ASCII, beginUTC.Unix(), endUTC.Add(-time.Second).Unix()) + + // Compose the message. + msgPrefix, has8bit, smtputf8, messageID, err := composeMessage(ctx, log, msgf, dom, from, recipients, subject, text, reportFilename, reportFile) + if err != nil { + return false, fmt.Errorf("composing message with outgoing tls report: %v", err) + } + msgInfo, err := msgf.Stat() + if err != nil { + return false, fmt.Errorf("stat message with outgoing tls report: %v", err) + } + msgSize := int64(len(msgPrefix)) + msgInfo.Size() + + for _, rcpt := range recipients { + qm := queue.MakeMsg(mox.Conf.Static.Postmaster.Account, from.Path(), rcpt.Path(), has8bit, smtputf8, msgSize, messageID, []byte(msgPrefix), nil) + // Don't try as long as regular deliveries, and stop before we would send the + // delayed DSN. Though we also won't send that due to IsTLSReport. + // ../rfc/8460:1077 + qm.MaxAttempts = 5 + qm.IsTLSReport = true + // TLS failures should be ignored. ../rfc/8460:317 ../rfc/8460:1050 + no := false + qm.RequireTLS = &no + + err := queueAdd(ctx, log, &qm, msgf) + if err != nil { + tempError = true + log.Errorx("queueing message with tls report", err) + metricReportError.Inc() + } else { + log.Debug("tls report queued", mlog.Field("recipient", rcpt)) + metricReport.Inc() + } + } + + // Regardless of whether we queued a report, we are not going to keep the + // evaluations around. Though this can be overridden if tempError is set. + // ../rfc/7489:1785 + + return true, nil +} + +func composeMessage(ctx context.Context, log *mlog.Log, mf *os.File, policyDomain dns.Domain, fromAddr smtp.Address, recipients []smtp.Address, subject, text, filename string, reportFile *os.File) (msgPrefix string, has8bit, smtputf8 bool, messageID string, rerr error) { + xc := message.NewComposer(mf) + defer xc.Recover(&rerr) + + // We only use smtputf8 if we have to, with a utf-8 localpart. For IDNA, we use ASCII domains. + for _, a := range recipients { + if a.Localpart.IsInternational() { + xc.SMTPUTF8 = true + break + } + } + + xc.HeaderAddrs("From", []smtp.Address{fromAddr}) + xc.HeaderAddrs("To", recipients) + xc.Subject(subject) + // ../rfc/8460:926 + xc.Header("TLS-Report-Domain", policyDomain.ASCII) + xc.Header("TLS-Report-Submitter", mox.Conf.Static.HostnameDomain.ASCII) + // TLS failures should be ignored. ../rfc/8460:317 ../rfc/8460:1050 + xc.Header("TLS-Required", "No") + messageID = fmt.Sprintf("<%s>", mox.MessageIDGen(xc.SMTPUTF8)) + xc.Header("Message-Id", messageID) + xc.Header("Date", time.Now().Format(message.RFC5322Z)) + xc.Header("User-Agent", "mox/"+moxvar.Version) + xc.Header("MIME-Version", "1.0") + + // Multipart message, with a text/plain and the report attached. + mp := multipart.NewWriter(xc) + // ../rfc/8460:916 + xc.Header("Content-Type", fmt.Sprintf(`multipart/report; report-type="tlsrpt"; boundary="%s"`, mp.Boundary())) + xc.Line() + + // Textual part, just mentioning this is a TLS report. + textBody, ct, cte := xc.TextPart(text) + textHdr := textproto.MIMEHeader{} + textHdr.Set("Content-Type", ct) + textHdr.Set("Content-Transfer-Encoding", cte) + textp, err := mp.CreatePart(textHdr) + xc.Checkf(err, "adding text part to message") + _, err = textp.Write(textBody) + xc.Checkf(err, "writing text part") + + // TLS report as attachment. + ahdr := textproto.MIMEHeader{} + ct = mime.FormatMediaType("application/tlsrpt+gzip", map[string]string{"name": filename}) + ahdr.Set("Content-Type", ct) + cd := mime.FormatMediaType("attachment", map[string]string{"filename": filename}) + ahdr.Set("Content-Disposition", cd) + ahdr.Set("Content-Transfer-Encoding", "base64") + ap, err := mp.CreatePart(ahdr) + xc.Checkf(err, "adding tls report to message") + wc := moxio.Base64Writer(ap) + _, err = io.Copy(wc, &moxio.AtReader{R: reportFile}) + xc.Checkf(err, "adding attachment") + err = wc.Close() + xc.Checkf(err, "flushing attachment") + + err = mp.Close() + xc.Checkf(err, "closing multipart") + + xc.Flush() + + // Also sign the TLS-Report headers. ../rfc/8460:940 + extraHeaders := []string{"TLS-Report-Domain", "TLS-Report-Submitter"} + msgPrefix = dkimSign(ctx, log, fromAddr, smtputf8, mf, extraHeaders) + + return msgPrefix, xc.Has8bit, xc.SMTPUTF8, messageID, nil +} + +func dkimSign(ctx context.Context, log *mlog.Log, fromAddr smtp.Address, smtputf8 bool, mf *os.File, extraHeaders []string) string { + // Add DKIM-Signature headers if we have a key for (a higher) domain than the from + // address, which is a host name. A signature will only be useful with higher-level + // domains if they have a relaxed dkim check (which is the default). If the dkim + // check is strict, there is no harm, there will simply not be a dkim pass. + fd := fromAddr.Domain + var zerodom dns.Domain + for fd != zerodom { + confDom, ok := mox.Conf.Domain(fd) + if ok && len(confDom.DKIM.Sign) == 0 { + return "" + } + if len(confDom.DKIM.Sign) > 0 { + selectors := map[string]config.Selector{} + for name, sel := range confDom.DKIM.Selectors { + sel.HeadersEffective = append(append([]string{}, sel.HeadersEffective...), extraHeaders...) + selectors[name] = sel + } + confDom.DKIM.Selectors = selectors + + dkimHeaders, err := dkim.Sign(ctx, fromAddr.Localpart, fd, confDom.DKIM, smtputf8, mf) + if err != nil { + log.Errorx("dkim-signing dmarc report, continuing without signature", err) + metricReportError.Inc() + return "" + } + return dkimHeaders + } + + var nfd dns.Domain + _, nfd.ASCII, _ = strings.Cut(fd.ASCII, ".") + _, nfd.Unicode, _ = strings.Cut(fd.Unicode, ".") + fd = nfd + } + return "" +} diff --git a/tlsrptsend/send_test.go b/tlsrptsend/send_test.go new file mode 100644 index 0000000..e4a99d9 --- /dev/null +++ b/tlsrptsend/send_test.go @@ -0,0 +1,384 @@ +package tlsrptsend + +import ( + "context" + "fmt" + "io" + "os" + "path/filepath" + "reflect" + "sync" + "testing" + "time" + + "github.com/mjl-/mox/dns" + "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/mox-" + "github.com/mjl-/mox/moxio" + "github.com/mjl-/mox/queue" + "github.com/mjl-/mox/tlsrpt" + "github.com/mjl-/mox/tlsrptdb" +) + +var ctxbg = context.Background() + +func tcheckf(t *testing.T, err error, format string, args ...any) { + t.Helper() + if err != nil { + t.Fatalf("%s: %s", fmt.Sprintf(format, args...), err) + } +} + +func tcompare(t *testing.T, got, expect any) { + t.Helper() + if !reflect.DeepEqual(got, expect) { + t.Fatalf("got:\n%v\nexpected:\n%v", got, expect) + } +} + +func TestSendReports(t *testing.T) { + mlog.SetConfig(map[string]mlog.Level{"": mlog.LevelDebug}) + + os.RemoveAll("../testdata/tlsrptsend/data") + mox.Context = ctxbg + mox.ConfigStaticPath = filepath.FromSlash("../testdata/tlsrptsend/mox.conf") + mox.MustLoadConfig(true, false) + + err := tlsrptdb.Init() + tcheckf(t, err, "init database") + + db := tlsrptdb.ResultDB + + resolver := dns.MockResolver{ + TXT: map[string][]string{ + "_smtp._tls.sender.example.": { + "v=TLSRPTv1; rua=mailto:tls-reports@sender.example,https://ignored.example/", + }, + "_smtp._tls.mailhost.sender.example.": { + "v=TLSRPTv1; rua=mailto:tls-reports1@mailhost.sender.example,mailto:tls-reports2@mailhost.sender.example; rua=mailto:tls-reports3@mailhost.sender.example", + }, + "_smtp._tls.noreport.example.": { + "v=TLSRPTv1; rua=mailto:tls-reports@noreport.example", + }, + "_smtp._tls.mailhost.norua.example.": { + "v=TLSRPTv1;", + }, + }, + } + + endUTC := midnightUTC(time.Now()) + dayUTC := endUTC.Add(-12 * time.Hour).Format("20060102") + + tlsResults := []tlsrptdb.TLSResult{ + // For report1 below. + { + PolicyDomain: "sender.example", + DayUTC: dayUTC, + RecipientDomain: "sender.example", + IsHost: false, + SendReport: true, + Results: []tlsrpt.Result{ + { + Policy: tlsrpt.ResultPolicy{ + Type: tlsrpt.STS, + Domain: "sender.example", + String: []string{"... mtasts policy ..."}, + MXHost: []string{"*.sender.example"}, + }, + Summary: tlsrpt.Summary{ + TotalSuccessfulSessionCount: 10, + TotalFailureSessionCount: 3, + }, + FailureDetails: []tlsrpt.FailureDetails{ + { + ResultType: tlsrpt.ResultCertificateExpired, + SendingMTAIP: "1.2.3.4", + ReceivingMXHostname: "mailhost.sender.example", + ReceivingMXHelo: "mailhost.sender.example", + ReceivingIP: "4.3.2.1", + FailedSessionCount: 3, + }, + }, + }, + }, + }, + + // For report2 below. + { + PolicyDomain: "mailhost.sender.example", + DayUTC: dayUTC, + RecipientDomain: "sender.example", + IsHost: true, + SendReport: false, // Would be ignored if on its own, but we have another result for this policy domain. + Results: []tlsrpt.Result{ + { + Policy: tlsrpt.ResultPolicy{ + Type: tlsrpt.TLSA, + Domain: "mailhost.sender.example", + String: []string{"... tlsa record ..."}, + }, + Summary: tlsrpt.Summary{ + TotalSuccessfulSessionCount: 10, + TotalFailureSessionCount: 1, + }, + FailureDetails: []tlsrpt.FailureDetails{ + { + ResultType: tlsrpt.ResultValidationFailure, + SendingMTAIP: "1.2.3.4", + ReceivingMXHostname: "mailhost.sender.example", + ReceivingMXHelo: "mailhost.sender.example", + ReceivingIP: "4.3.2.1", + FailedSessionCount: 1, + FailureReasonCode: "dns-extended-error-7-signature-expired", + }, + }, + }, + }, + }, + { + PolicyDomain: "mailhost.sender.example", + DayUTC: dayUTC, + RecipientDomain: "sharedsender.example", + IsHost: true, + SendReport: true, // Causes previous result to be included in this report. + Results: []tlsrpt.Result{ + { + Policy: tlsrpt.ResultPolicy{ + Type: tlsrpt.TLSA, + Domain: "mailhost.sender.example", + String: []string{"... tlsa record ..."}, + }, + Summary: tlsrpt.Summary{ + TotalSuccessfulSessionCount: 10, + TotalFailureSessionCount: 1, + }, + FailureDetails: []tlsrpt.FailureDetails{ + { + ResultType: tlsrpt.ResultValidationFailure, + SendingMTAIP: "1.2.3.4", + ReceivingMXHostname: "mailhost.sender.example", + ReceivingMXHelo: "mailhost.sender.example", + ReceivingIP: "4.3.2.1", + FailedSessionCount: 1, + FailureReasonCode: "dns-extended-error-7-signature-expired", + }, + }, + }, + }, + }, + + // No report due to SendReport false. + { + PolicyDomain: "mailhost.noreport.example", + DayUTC: dayUTC, + RecipientDomain: "noreport.example", + IsHost: true, + SendReport: false, // No report. + Results: []tlsrpt.Result{ + { + Policy: tlsrpt.ResultPolicy{ + Type: tlsrpt.NoPolicyFound, + Domain: "mailhost.noreport.example", + }, + Summary: tlsrpt.Summary{ + TotalSuccessfulSessionCount: 2, + TotalFailureSessionCount: 1, + }, + }, + }, + }, + + // No report due to no mailto rua. + { + PolicyDomain: "mailhost.norua.example", + DayUTC: dayUTC, + RecipientDomain: "norua.example", + IsHost: true, + SendReport: false, // No report. + Results: []tlsrpt.Result{ + { + Policy: tlsrpt.ResultPolicy{ + Type: tlsrpt.NoPolicyFound, + Domain: "mailhost.norua.example", + }, + Summary: tlsrpt.Summary{ + TotalSuccessfulSessionCount: 2, + TotalFailureSessionCount: 1, + }, + }, + }, + }, + + // No report due to no TLSRPT record. + { + PolicyDomain: "mailhost.notlsrpt.example", + DayUTC: dayUTC, + RecipientDomain: "notlsrpt.example", + IsHost: true, + SendReport: true, + Results: []tlsrpt.Result{ + { + Policy: tlsrpt.ResultPolicy{ + Type: tlsrpt.NoPolicyFound, + Domain: "mailhost.notlsrpt.example", + }, + Summary: tlsrpt.Summary{ + TotalSuccessfulSessionCount: 2, + TotalFailureSessionCount: 1, + }, + }, + }, + }, + } + + report1 := tlsrpt.Report{ + OrganizationName: "mail.mox.example", + DateRange: tlsrpt.TLSRPTDateRange{ + Start: endUTC.Add(-24 * time.Hour), + End: endUTC.Add(-time.Second), + }, + ContactInfo: "postmaster@mail.mox.example", + ReportID: endUTC.Format("20060102") + ".sender.example@mail.mox.example", + Policies: []tlsrpt.Result{ + { + Policy: tlsrpt.ResultPolicy{ + Type: tlsrpt.STS, + Domain: "sender.example", + String: []string{"... mtasts policy ..."}, + MXHost: []string{"*.sender.example"}, + }, + Summary: tlsrpt.Summary{ + TotalSuccessfulSessionCount: 10, + TotalFailureSessionCount: 3, + }, + FailureDetails: []tlsrpt.FailureDetails{ + { + ResultType: tlsrpt.ResultCertificateExpired, + SendingMTAIP: "1.2.3.4", + ReceivingMXHostname: "mailhost.sender.example", + ReceivingMXHelo: "mailhost.sender.example", + ReceivingIP: "4.3.2.1", + FailedSessionCount: 3, + }, + }, + }, + }, + } + report2 := tlsrpt.Report{ + OrganizationName: "mail.mox.example", + DateRange: tlsrpt.TLSRPTDateRange{ + Start: endUTC.Add(-24 * time.Hour), + End: endUTC.Add(-time.Second), + }, + ContactInfo: "postmaster@mail.mox.example", + ReportID: endUTC.Format("20060102") + ".mailhost.sender.example@mail.mox.example", + Policies: []tlsrpt.Result{ + { + Policy: tlsrpt.ResultPolicy{ + Type: tlsrpt.TLSA, + Domain: "mailhost.sender.example", + String: []string{"... tlsa record ..."}, + }, + Summary: tlsrpt.Summary{ + TotalSuccessfulSessionCount: 20, + TotalFailureSessionCount: 2, + }, + FailureDetails: []tlsrpt.FailureDetails{ + { + ResultType: tlsrpt.ResultValidationFailure, + SendingMTAIP: "1.2.3.4", + ReceivingMXHostname: "mailhost.sender.example", + ReceivingMXHelo: "mailhost.sender.example", + ReceivingIP: "4.3.2.1", + FailedSessionCount: 2, + FailureReasonCode: "dns-extended-error-7-signature-expired", + }, + }, + }, + }, + } + + // Set a timeUntil that we steplock and that causes the actual sleep to return + // immediately when we want to. + wait := make(chan struct{}) + step := make(chan time.Duration) + jitteredTimeUntil = func(_ time.Time) time.Duration { + wait <- struct{}{} + return <-step + } + + sleepBetween = func(ctx context.Context, d time.Duration) (ok bool) { return true } + + test := func(results []tlsrptdb.TLSResult, expReports map[string]tlsrpt.Report) { + // t.Helper() + + mox.Shutdown, mox.ShutdownCancel = context.WithCancel(ctxbg) + + for _, r := range results { + err := db.Insert(ctxbg, &r) + tcheckf(t, err, "inserting tlsresult") + } + + haveReports := map[string]tlsrpt.Report{} + + var mutex sync.Mutex + + var index int + queueAdd = func(ctx context.Context, log *mlog.Log, qm *queue.Msg, msgFile *os.File) error { + mutex.Lock() + defer mutex.Unlock() + + // Read message file. Also write copy to disk for inspection. + buf, err := io.ReadAll(&moxio.AtReader{R: msgFile}) + tcheckf(t, err, "read report message") + p := fmt.Sprintf("../testdata/tlsrptsend/data/report%d.eml", index) + index++ + err = os.WriteFile(p, append(append([]byte{}, qm.MsgPrefix...), buf...), 0600) + tcheckf(t, err, "write report message") + + report, err := tlsrpt.ParseMessage(log, msgFile) + tcheckf(t, err, "parsing generated report message") + + addr := qm.Recipient().String() + + if _, ok := haveReports[addr]; ok { + t.Fatalf("report for address %s already seen", addr) + } else if expReport, ok := expReports[addr]; !ok { + t.Fatalf("unexpected report for address %s", addr) + } else { + tcompare(t, *report, expReport) + } + haveReports[addr] = *report + + return nil + } + + Start(resolver) + // Run first loop. + <-wait + step <- 0 + <-wait + + tcompare(t, haveReports, expReports) + + // Second loop. Evaluations cleaned, should not result in report messages. + haveReports = map[string]tlsrpt.Report{} + step <- 0 + <-wait + tcompare(t, haveReports, map[string]tlsrpt.Report{}) + + // Caus Start to stop. + mox.ShutdownCancel() + step <- time.Minute + } + + // Multiple results, some are combined into a single report, another result + // generates a separate report to multiple rua's, and the last don't send a report. + expReports := map[string]tlsrpt.Report{ + "tls-reports@sender.example": report1, + "tls-reports1@mailhost.sender.example": report2, + "tls-reports2@mailhost.sender.example": report2, + "tls-reports3@mailhost.sender.example": report2, + } + test(tlsResults, expReports) +} diff --git a/vendor/github.com/mjl-/adns/ede.go b/vendor/github.com/mjl-/adns/ede.go index 1a7f13e..cbb7071 100644 --- a/vendor/github.com/mjl-/adns/ede.go +++ b/vendor/github.com/mjl-/adns/ede.go @@ -109,6 +109,14 @@ func (e ErrorCode) Error() string { return fmt.Sprintf("error from name server: %s", errorCodeStrings[e]) } +// String returns a short text string for known error codes, or "unknown". +func (e ErrorCode) String() string { + if int(e) >= 0 && int(e) < len(errorCodeStrings) { + return errorCodeStrings[e] + } + return "unknown" +} + // short strings, always included in error messages. var errorCodeStrings = []string{ "other", diff --git a/vendor/modules.txt b/vendor/modules.txt index 5c7fedd..74f01f1 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -11,7 +11,7 @@ github.com/golang/protobuf/ptypes/timestamp # github.com/matttproud/golang_protobuf_extensions v1.0.1 ## explicit github.com/matttproud/golang_protobuf_extensions/pbutil -# github.com/mjl-/adns v0.0.0-20231013194548-ea0378d616ab +# github.com/mjl-/adns v0.0.0-20231109160910-82839fe3e6ae ## explicit; go 1.20 github.com/mjl-/adns github.com/mjl-/adns/internal/bytealg diff --git a/verifydata.go b/verifydata.go index dc3bec1..714bddb 100644 --- a/verifydata.go +++ b/verifydata.go @@ -402,7 +402,7 @@ possibly making them potentially no longer readable by the previous version. p = p[len(dataDir)+1:] } switch p { - case "dmarcrpt.db", "dmarceval.db", "mtasts.db", "tlsrpt.db", "receivedid.key", "lastknownversion": + case "dmarcrpt.db", "dmarceval.db", "mtasts.db", "tlsrpt.db", "tlsrptresult.db", "receivedid.key", "lastknownversion": return nil case "acme", "queue", "accounts", "tmp", "moved": return fs.SkipDir @@ -423,7 +423,8 @@ possibly making them potentially no longer readable by the previous version. checkDB(true, filepath.Join(dataDir, "dmarcrpt.db"), dmarcdb.ReportsDBTypes) checkDB(false, filepath.Join(dataDir, "dmarceval.db"), dmarcdb.EvalDBTypes) // After v0.0.7. checkDB(true, filepath.Join(dataDir, "mtasts.db"), mtastsdb.DBTypes) - checkDB(true, filepath.Join(dataDir, "tlsrpt.db"), tlsrptdb.DBTypes) + checkDB(true, filepath.Join(dataDir, "tlsrpt.db"), tlsrptdb.ReportDBTypes) + checkDB(false, filepath.Join(dataDir, "tlsrptresult.db"), tlsrptdb.ResultDBTypes) // After v0.0.7. checkQueue() checkAccounts() checkOther() diff --git a/webaccount/account.go b/webaccount/account.go index 511c3e4..88084b2 100644 --- a/webaccount/account.go +++ b/webaccount/account.go @@ -393,7 +393,8 @@ func (Account) DestinationSave(ctx context.Context, destName string, oldDest, ne // Keep fields we manage. newDest.DMARCReports = curDest.DMARCReports - newDest.TLSReports = curDest.TLSReports + newDest.HostTLSReports = curDest.HostTLSReports + newDest.DomainTLSReports = curDest.DomainTLSReports err := mox.DestinationSave(ctx, accountName, destName, newDest) xcheckf(ctx, err, "saving destination") diff --git a/webadmin/admin.go b/webadmin/admin.go index ab2e929..6ed8dbe 100644 --- a/webadmin/admin.go +++ b/webadmin/admin.go @@ -365,7 +365,8 @@ type CheckResult struct { SPF SPFCheckResult DKIM DKIMCheckResult DMARC DMARCCheckResult - TLSRPT TLSRPTCheckResult + HostTLSRPT TLSRPTCheckResult + DomainTLSRPT TLSRPTCheckResult MTASTS MTASTSCheckResult SRVConf SRVConfCheckResult Autoconf AutoconfCheckResult @@ -1130,28 +1131,27 @@ EOF } }() - // TLSRPT - wg.Add(1) - go func() { + checkTLSRPT := func(result *TLSRPTCheckResult, dom dns.Domain, address smtp.Address, isHost bool) { defer logPanic(ctx) defer wg.Done() - record, txt, err := tlsrpt.Lookup(ctx, resolver, domain) + record, txt, err := tlsrpt.Lookup(ctx, resolver, dom) if err != nil { - addf(&r.TLSRPT.Errors, "Looking up TLSRPT record: %s", err) + addf(&result.Errors, "Looking up TLSRPT record: %s", err) } - r.TLSRPT.TXT = txt + result.TXT = txt if record != nil { - r.TLSRPT.Record = &TLSRPTRecord{*record} + result.Record = &TLSRPTRecord{*record} } - instr := `TLSRPT is an opt-in mechanism to request feedback about TLS connectivity from remote SMTP servers when they connect to us. It allows detecting delivery problems and unwanted downgrades to plaintext SMTP connections. With TLSRPT you configure an email address to which reports should be sent. Remote SMTP servers will send a report once a day with the number of successful connections, and the number of failed connections including details that should help debugging/resolving any issues.` - if domConf.TLSRPT != nil { + instr := `TLSRPT is an opt-in mechanism to request feedback about TLS connectivity from remote SMTP servers when they connect to us. It allows detecting delivery problems and unwanted downgrades to plaintext SMTP connections. With TLSRPT you configure an email address to which reports should be sent. Remote SMTP servers will send a report once a day with the number of successful connections, and the number of failed connections including details that should help debugging/resolving any issues. Both the mail host (e.g. mail.domain.example) and a recipient domain (e.g. domain.example, with an MX record pointing to mail.domain.example) can have a TLSRPT record. The TLSRPT record for the hosts is for reporting about DANE, the TLSRPT record for the domain is for MTA-STS.` + var zeroaddr smtp.Address + if address != zeroaddr { // TLSRPT does not require validation of reporting addresses outside the domain. // ../rfc/8460:1463 uri := url.URL{ Scheme: "mailto", - Opaque: smtp.NewAddress(domConf.TLSRPT.ParsedLocalpart, domConf.TLSRPT.DNSDomain).Pack(false), + Opaque: address.Pack(false), } uristr := uri.String() uristr = strings.ReplaceAll(uristr, ",", "%2C") @@ -1167,11 +1167,29 @@ Ensure a DNS TXT record like the following exists: _smtp._tls TXT %s `, mox.TXTStrings(tlsrptr.String())) + } else if isHost { + addf(&result.Errors, `Configure a host TLSRPT localpart in static mox.conf config file.`) } else { - addf(&r.TLSRPT.Errors, `Configure a TLSRPT destination in domain in config file.`) + addf(&result.Errors, `Configure a domain TLSRPT destination in domains.conf config file.`) } - addf(&r.TLSRPT.Instructions, instr) - }() + addf(&result.Instructions, instr) + } + + // Hots TLSRPT + wg.Add(1) + var hostTLSRPTAddr smtp.Address + if mox.Conf.Static.HostTLSRPT.Localpart != "" { + hostTLSRPTAddr = smtp.NewAddress(mox.Conf.Static.HostTLSRPT.ParsedLocalpart, mox.Conf.Static.HostnameDomain) + } + go checkTLSRPT(&r.HostTLSRPT, mox.Conf.Static.HostnameDomain, hostTLSRPTAddr, true) + + // Domain TLSRPT + wg.Add(1) + var domainTLSRPTAddr smtp.Address + if domConf.TLSRPT != nil { + domainTLSRPTAddr = smtp.NewAddress(domConf.TLSRPT.ParsedLocalpart, domain) + } + go checkTLSRPT(&r.DomainTLSRPT, domain, domainTLSRPTAddr, false) // MTA-STS wg.Add(1) @@ -1960,3 +1978,51 @@ func (Admin) DMARCRemoveEvaluations(ctx context.Context, domain string) { err = dmarcdb.RemoveEvaluationsDomain(ctx, dom) xcheckf(ctx, err, "removing evaluations for domain") } + +// TLSRPTResults returns all TLSRPT results in the database. +func (Admin) TLSRPTResults(ctx context.Context) []tlsrptdb.TLSResult { + results, err := tlsrptdb.Results(ctx) + xcheckf(ctx, err, "get results") + return results +} + +// TLSRPTResultsPolicyDomain returns the TLS results for a domain. +func (Admin) TLSRPTResultsPolicyDomain(ctx context.Context, policyDomain string) (dns.Domain, []tlsrptdb.TLSResult) { + dom, err := dns.ParseDomain(policyDomain) + xcheckf(ctx, err, "parsing domain") + + results, err := tlsrptdb.ResultsPolicyDomain(ctx, dom) + xcheckf(ctx, err, "get result for policy domain") + return dom, results +} + +// LookupTLSRPTRecord looks up a TLSRPT record and returns the parsed form, original txt +// form from DNS, and error with the TLSRPT record as a string. +func (Admin) LookupTLSRPTRecord(ctx context.Context, domain string) (record *TLSRPTRecord, txt string, errstr string) { + dom, err := dns.ParseDomain(domain) + xcheckf(ctx, err, "parsing domain") + + resolver := dns.StrictResolver{Pkg: "webadmin"} + r, txt, err := tlsrpt.Lookup(ctx, resolver, dom) + if err != nil && (errors.Is(err, tlsrpt.ErrNoRecord) || errors.Is(err, tlsrpt.ErrMultipleRecords) || errors.Is(err, tlsrpt.ErrRecordSyntax)) { + errstr = err.Error() + err = nil + } + xcheckf(ctx, err, "fetching tlsrpt record") + + if r != nil { + record = &TLSRPTRecord{Record: *r} + } + + return record, txt, errstr +} + +// TLSRPTRemoveResults removes the TLS results for a domain for the given day. If +// day is empty, all results are removed. +func (Admin) TLSRPTRemoveResults(ctx context.Context, domain string, day string) { + dom, err := dns.ParseDomain(domain) + xcheckf(ctx, err, "parsing domain") + + err = tlsrptdb.RemoveResultsPolicyDomain(ctx, dom, day) + xcheckf(ctx, err, "removing tls results") +} diff --git a/webadmin/admin.html b/webadmin/admin.html index 3be1fbb..5e87d8a 100644 --- a/webadmin/admin.html +++ b/webadmin/admin.html @@ -19,6 +19,7 @@ table table td, table table th { padding: 0 0.1em; } table.long >tbody >tr >td { padding: 1em .5em; } table.long td { vertical-align: top; } table > tbody > tr:nth-child(odd) { background-color: #f8f8f8; } +table.hover > tbody > tr:hover { background-color: #f0f0f0; } .text { max-width: 50em; } p { margin-bottom: 1em; max-width: 50em; } [title] { text-decoration: underline; text-decoration-style: dotted; } @@ -262,12 +263,12 @@ const index = async () => { dom.br(), dom.h2('Reports'), dom.div(dom.a('DMARC', attr({href: '#dmarc/reports'}))), - dom.div(dom.a('TLS', attr({href: '#tlsrpt'}))), + dom.div(dom.a('TLS', attr({href: '#tlsrpt/reports'}))), dom.br(), dom.h2('Operations'), dom.div(dom.a('MTA-STS policies', attr({href: '#mtasts'}))), dom.div(dom.a('DMARC evaluations', attr({href: '#dmarc/evaluations'}))), - // todo: outgoing TLSRPT findings + dom.div(dom.a('TLS connection results', attr({href: '#tlsrpt/results'}))), // todo: routing, globally, per domain and per account dom.br(), dom.h2('DNS blocklist status'), @@ -960,8 +961,8 @@ const domainDNSCheck = async (d) => { dom.div('Domain: ' + checks.DMARC.Domain), !checks.DMARC.TXT ? [] : dom.div('TXT record: ' + checks.DMARC.TXT), ] - const detailsTLSRPT = !checks.TLSRPT.TXT ? [] : [ - dom.div('TXT record: ' + checks.TLSRPT.TXT), + const detailsTLSRPT = (checksTLSRPT) => !checksTLSRPT.TXT ? [] : [ + dom.div('TXT record: ' + checksTLSRPT.TXT), ] const detailsMTASTS = !checks.MTASTS.TXT && !checks.MTASTS.PolicyText ? [] : [ !checks.MTASTS.TXT ? [] : dom.div('MTA-STS record: ' + checks.MTASTS.TXT), @@ -1007,7 +1008,8 @@ const domainDNSCheck = async (d) => { resultSection('SPF', checks.SPF, detailsSPF), resultSection('DKIM', checks.DKIM, detailsDKIM), resultSection('DMARC', checks.DMARC, detailsDMARC), - resultSection('TLSRPT', checks.TLSRPT, detailsTLSRPT), + resultSection('Host TLSRPT', checks.HostTLSRPT, detailsTLSRPT(checks.HostTLSRPT)), + resultSection('Domain TLSRPT', checks.DomainTLSRPT, detailsTLSRPT(checks.DomainTLSRPT)), resultSection('MTA-STS', checks.MTASTS, detailsMTASTS), resultSection('SRV conf', checks.SRVConf, detailsSRVConf), resultSection('Autoconf', checks.Autoconf, detailsAutoconf), @@ -1458,7 +1460,149 @@ const domainDMARCReport = async (d, reportID) => { ) } -const tlsrpt = async () => { +const tlsrptIndex = async () => { + const page = document.getElementById('page') + dom._kids(page, + crumbs( + crumblink('Mox Admin', '#'), + 'TLSRPT reports and connectivity results', + ), + dom.ul( + dom.li( + dom.a(attr({href: '#tlsrpt/reports'}), 'Reports'), ', incoming TLS reports.', + ), + dom.li( + dom.a(attr({href: '#tlsrpt/results'}), 'Results'), ', for outgoing TLS reports.', + ), + ), + ) +} + +const tlsrptResults = async () => { + const results = await api.TLSRPTResults() + + // todo: add a view where results are grouped by policy domain+dayutc. now each recipient domain gets a row. + + const page = document.getElementById('page') + dom._kids(page, + crumbs( + crumblink('Mox Admin', '#'), + crumblink('TLSRPT', '#tlsrpt'), + 'Results', + ), + dom.p('Messages are delivered with SMTP with TLS using STARTTLS if supported and/or required by the recipient domain\'s mail server. TLS connections may fail for various reasons, such as mismatching certificate host name, expired certificates or TLS protocol version/cipher suite incompatibilities. Statistics about successful connections and failed connections are tracked. Results can be tracked for recipient domains (for MTA-STS policies), and per MX host (for DANE). A domain/host can publish a TLSRPT DNS record with addresses that should receive TLS reports. Reports are sent every 24 hours. Not all results are enough reason to send a report, but if a report is sent all results are included.'), + dom('table.hover', + dom.thead( + dom.tr( + dom.th('Day (UTC)', attr({title: 'Day covering these results, a whole day from 00:00 UTC to 24:00 UTC.'})), + dom.th('Recipient domain', attr({title: 'Domain of addressee. For delivery to a recipient, the recipient and policy domains will match for reporting on MTA-STS policies, but can also result in reports for hosts from the MX record of the recipient to report on DANE policies.'})), + dom.th('Policy domain', attr({title: 'Domain for TLSRPT policy, specifying URIs to which reports should be sent.'})), + dom.th('Host', attr({title: 'Whether policy domain is an (MX) host (for DANE), or a recipient domain (for MTA-STS).'})), + dom.th('Success', attr({title: 'Total number of successful connections.'})), + dom.th('Failure', attr({title: 'Total number of failed connection attempts.'})), + dom.th('Failure details', attr({title: 'Total number of details about failures.'})), + dom.th('Send report', attr({title: 'Whether the current results will cause a report to be sent. A report is only sent if the domain has a TLSRPT with reporting addresses configured.'})), + ), + ), + dom.tbody( + results.sort((a, b) => { + if (a.DayUTC !== b.DayUTC) { + return a.DayUTC < b.DayUTC ? -1 : 1 + } + if (a.RecipientDomain !== b.RecipientDomain) { + return a.RecipientDomain < b.RecipientDomain ? -1 : 1 + } + return a.PolicyDomain < b.PolicyDomain ? -1 : 1 + }).map(r => { + let success = 0 + let failed = 0 + let failureDetails = 0 + r.Results.forEach(result => { + success += result.summary['total-successful-session-count'] + failed += result.summary['total-failure-session-count'] + failureDetails += (result['failure-details'] || []).length + }) + return dom.tr( + dom.td(r.DayUTC), + dom.td(r.RecipientDomain), + dom.td(dom.a(attr({href: '#tlsrpt/results/'+r.PolicyDomain}), r.PolicyDomain)), + dom.td(r.IsHost ? '✓' : ''), + dom.td(style({textAlign: 'right'}), ''+success), + dom.td(style({textAlign: 'right'}), ''+failed), + dom.td(style({textAlign: 'right'}), ''+failureDetails), + dom.td(style({textAlign: 'right'}), r.SendReport ? '✓' : ''), + ) + }), + results.length === 0 ? dom.tr(dom.td(attr({colspan: '8'}), 'No results.')) : [], + ), + ), + ) +} + +const tlsrptResultsPolicyDomain = async (domain) => { + const [d, tlsresults] = await api.TLSRPTResultsPolicyDomain(domain) + const recordPromise = api.LookupTLSRPTRecord(domain) + + let recordBox + const page = document.getElementById('page') + dom._kids(page, + crumbs( + crumblink('Mox Admin', '#'), + crumblink('TLSRPT', '#tlsrpt'), + crumblink('Results', '#tlsrpt/results'), + 'Policy domain '+domainString(d), + ), + dom.div( + dom.button('Remove results', async function click(e) { + e.preventDefault() + e.target.disabled = true + try { + await api.TLSRPTRemoveResults(domain, '') + window.location.reload() // todo: only clear the table? + } catch (err) { + console.log({err}) + window.alert('Error: ' + err.message) + } finally { + e.target.disabled = false + } + }), + ), + dom.br(), + dom.div('Fetching TLSRPT DNS record for policy domain...'), + recordBox=dom.div(), + dom.br(), + dom.p('Below are the results per day and recipient domain that may be sent in a report.'), + tlsresults.map(tlsresult => [ + dom.h2(tlsresult.DayUTC, ' - ', dom.span(attr({title: 'Recipient domain, as used in SMTP MAIL TO, usually based on message To/Cc/Bcc.'}), tlsresult.RecipientDomain)), + dom.p( + 'Send report (if TLSRPT exists and has address): '+(tlsresult.SendReport ? 'Yes' : 'No'), + dom.br(), + 'Report about (MX) host (instead of recipient domain): '+(tlsresult.IsHost ? 'Yes' : 'No'), + ), + dom('div.literal', JSON.stringify(tlsresult.Results, null, '\t')), + ]) + ) + + // In background so page load fade doesn't look weird. + ;(async () => { + let record, txt, errmsg + try { + [record, txt, errmsg] = await recordPromise + } catch (err) { + errmsg = 'error: '+err.message + } + const l = [] + if (txt) { + l.push(dom('div.literal', txt)) + } + if (errmsg) { + l.push(box(red, errmsg)) + } + dom._kids(recordBox, l) + })() +} + +const tlsrptReports = async () => { const end = new Date().toISOString() const start = new Date(new Date().getTime() - 30*24*3600*1000).toISOString() const summaries = await api.TLSRPTSummaries(start, end, '') @@ -2431,7 +2575,13 @@ const init = async () => { } else if (h === 'queue') { await queueList() } else if (h === 'tlsrpt') { - await tlsrpt() + await tlsrptIndex() + } else if (h === 'tlsrpt/reports') { + await tlsrptReports() + } else if (h === 'tlsrpt/results') { + await tlsrptResults() + } else if (t[0] == 'tlsrpt' && t[1] == 'results' && t.length === 3) { + await tlsrptResultsPolicyDomain(t[2]) } else if (h === 'dmarc') { await dmarcIndex() } else if (h === 'dmarc/reports') { diff --git a/webadmin/adminapi.json b/webadmin/adminapi.json index f0c2028..2fc5ab9 100644 --- a/webadmin/adminapi.json +++ b/webadmin/adminapi.json @@ -807,6 +807,99 @@ } ], "Returns": [] + }, + { + "Name": "TLSRPTResults", + "Docs": "TLSRPTResults returns all TLSRPT results in the database.", + "Params": [], + "Returns": [ + { + "Name": "r0", + "Typewords": [ + "[]", + "TLSResult" + ] + } + ] + }, + { + "Name": "TLSRPTResultsPolicyDomain", + "Docs": "TLSRPTResultsPolicyDomain returns the TLS results for a domain.", + "Params": [ + { + "Name": "policyDomain", + "Typewords": [ + "string" + ] + } + ], + "Returns": [ + { + "Name": "r0", + "Typewords": [ + "Domain" + ] + }, + { + "Name": "r1", + "Typewords": [ + "[]", + "TLSResult" + ] + } + ] + }, + { + "Name": "LookupTLSRPTRecord", + "Docs": "LookupTLSRPTRecord looks up a TLSRPT record and returns the parsed form, original txt\nform from DNS, and error with the TLSRPT record as a string.", + "Params": [ + { + "Name": "domain", + "Typewords": [ + "string" + ] + } + ], + "Returns": [ + { + "Name": "record", + "Typewords": [ + "nullable", + "TLSRPTRecord" + ] + }, + { + "Name": "txt", + "Typewords": [ + "string" + ] + }, + { + "Name": "errstr", + "Typewords": [ + "string" + ] + } + ] + }, + { + "Name": "TLSRPTRemoveResults", + "Docs": "TLSRPTRemoveResults removes the TLS results for a domain for the given day. If\nday is empty, all results are removed.", + "Params": [ + { + "Name": "domain", + "Typewords": [ + "string" + ] + }, + { + "Name": "day", + "Typewords": [ + "string" + ] + } + ], + "Returns": [] } ], "Sections": [], @@ -879,7 +972,14 @@ ] }, { - "Name": "TLSRPT", + "Name": "HostTLSRPT", + "Docs": "", + "Typewords": [ + "TLSRPTCheckResult" + ] + }, + { + "Name": "DomainTLSRPT", "Docs": "", "Typewords": [ "TLSRPTCheckResult" @@ -2148,6 +2248,13 @@ "[]", "Pair" ] + }, + { + "Name": "PolicyText", + "Docs": "Text that make up the policy, as retrieved. We didn't store this in the past. If empty, policy can be reconstructed from Policy field. Needed by TLSRPT.", + "Typewords": [ + "string" + ] } ] }, @@ -2183,6 +2290,13 @@ "string" ] }, + { + "Name": "HostReport", + "Docs": "Report for host TLSRPT record, as opposed to domain TLSRPT record.", + "Typewords": [ + "bool" + ] + }, { "Name": "Report", "Docs": "", @@ -2290,7 +2404,7 @@ "Name": "policy-type", "Docs": "", "Typewords": [ - "string" + "PolicyType" ] }, { @@ -2367,6 +2481,7 @@ "Name": "receiving-mx-helo", "Docs": "", "Typewords": [ + "nullable", "string" ] }, @@ -3672,6 +3787,76 @@ ] } ] + }, + { + "Name": "TLSResult", + "Docs": "TLSResult is stored in the database to track TLS results per policy domain, day\nand recipient domain. These records will be included in TLS reports.", + "Fields": [ + { + "Name": "ID", + "Docs": "", + "Typewords": [ + "int64" + ] + }, + { + "Name": "PolicyDomain", + "Docs": "Domain with TLSRPT DNS record, with addresses that will receive reports. Either a recipient domain (for MTA-STS policies) or an (MX) host (for DANE policies). Unicode.", + "Typewords": [ + "string" + ] + }, + { + "Name": "DayUTC", + "Docs": "DayUTC is of the form yyyymmdd.", + "Typewords": [ + "string" + ] + }, + { + "Name": "RecipientDomain", + "Docs": "Reports are sent per policy domain. When delivering a message to a recipient domain, we can get multiple TLSResults, typically one for MTA-STS, and one or more for DANE (one for each MX target, or actually TLSA base domain). We track recipient domain so we can display successes/failures for delivery of messages to a recipient domain in the admin pages. Unicode.", + "Typewords": [ + "string" + ] + }, + { + "Name": "Created", + "Docs": "", + "Typewords": [ + "timestamp" + ] + }, + { + "Name": "Updated", + "Docs": "", + "Typewords": [ + "timestamp" + ] + }, + { + "Name": "IsHost", + "Docs": "Result is for host (e.g. DANE), not recipient domain (e.g. MTA-STS).", + "Typewords": [ + "bool" + ] + }, + { + "Name": "SendReport", + "Docs": "Whether to send a report. TLS results for delivering messages with TLS reports will be recorded, but will not cause a report to be sent.", + "Typewords": [ + "bool" + ] + }, + { + "Name": "Results", + "Docs": "Results is updated for each TLS attempt.", + "Typewords": [ + "[]", + "Result" + ] + } + ] } ], "Ints": [], @@ -3739,6 +3924,27 @@ } ] }, + { + "Name": "PolicyType", + "Docs": "PolicyType indicates the policy success/failure results are for.", + "Values": [ + { + "Name": "TLSA", + "Value": "tlsa", + "Docs": "For DANE, against a mail host (not recipient domain)." + }, + { + "Name": "STS", + "Value": "sts", + "Docs": "For MTA-STS, against a recipient domain (not a mail host)." + }, + { + "Name": "NoPolicyFound", + "Value": "no-policy-found", + "Docs": "Recipient domain did not have MTA-STS policy, or mail host (TSLA base domain)\ndid not have DANE TLSA records." + } + ] + }, { "Name": "ResultType", "Docs": "ResultType represents a TLS error.", diff --git a/webmail/api.go b/webmail/api.go index f72539b..66a1847 100644 --- a/webmail/api.go +++ b/webmail/api.go @@ -1708,7 +1708,7 @@ func recipientSecurity(ctx context.Context, resolver dns.Resolver, messageAddres defer logPanic(ctx) defer wg.Done() - policy, _, err := mtastsdb.Get(ctx, resolver, addr.Domain) + policy, _, _, err := mtastsdb.Get(ctx, resolver, addr.Domain) if policy != nil && policy.Mode == mtasts.ModeEnforce { rs.MTASTS = SecurityResultYes } else if err == nil {