From 72ac1fde29fb56ac27468d51ce7987e4225f6f55 Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Tue, 5 Dec 2023 21:13:57 +0100 Subject: [PATCH] expose fewer internals in packages, for easier software reuse - prometheus is now behind an interface, they aren't dependencies for the reusable components anymore. - some dependencies have been inverted: instead of packages importing a main package to get configuration, the main package now sets configuration in these packages. that means fewer internals are pulled in. - some functions now have new parameters for values that were retrieved from package "mox-". --- dane/dane.go | 41 ++--- dane/dane_test.go | 12 +- dkim/dkim.go | 86 +++++----- dkim/dkim_test.go | 120 ++++++-------- dkim/parser.go | 8 +- dmarc/dmarc.go | 19 +-- dmarcdb/eval.go | 5 +- dns/dns.go | 7 +- dns/resolver.go | 19 +-- dnsbl/dnsbl.go | 18 +- dsn/dsn.go | 38 +---- dsn/dsn_test.go | 44 ++--- imapserver/condstore_test.go | 6 +- imapserver/fetch.go | 4 +- imapserver/server.go | 2 +- iprev/iprev.go | 15 +- main.go | 14 +- {moxio => message}/decode.go | 2 +- {moxio => message}/decode_test.go | 4 +- message/messageid.go | 3 +- message/part.go | 23 +-- message/part_test.go | 13 +- metrics.go | 267 ++++++++++++++++++++++++++++++ metrics/http.go | 65 -------- mox-/config.go | 17 +- mox-/dkimsign.go | 65 ++++++++ {mox- => moxio}/tlsinfo.go | 2 +- moxvar/pedantic.go | 4 - mtasts/mtasts.go | 21 +-- queue/direct.go | 14 +- queue/dsn.go | 14 +- queue/queue.go | 8 +- queue/submit.go | 18 +- smtp/address.go | 6 +- smtpclient/client.go | 42 ++--- smtpclient/dial.go | 10 +- smtpclient/dial_test.go | 4 +- smtpserver/dsn.go | 12 +- smtpserver/parse.go | 6 +- smtpserver/server.go | 23 +-- smtpserver/server_test.go | 3 +- spf/spf.go | 17 +- store/account.go | 3 +- stub/doc.go | 7 + stub/metrics.go | 43 +++++ subjectpass/subjectpass.go | 25 +-- tlsrpt/lookup.go | 15 +- tlsrptsend/send.go | 11 +- updates/updates.go | 30 +--- webmail/api.go | 5 +- webmail/message.go | 4 +- 51 files changed, 696 insertions(+), 568 deletions(-) rename {moxio => message}/decode.go (97%) rename {moxio => message}/decode_test.go (93%) create mode 100644 metrics.go delete mode 100644 metrics/http.go create mode 100644 mox-/dkimsign.go rename {mox- => moxio}/tlsinfo.go (97%) delete mode 100644 moxvar/pedantic.go create mode 100644 stub/doc.go create mode 100644 stub/metrics.go diff --git a/dane/dane.go b/dane/dane.go index 202ac6c..3f9ea49 100644 --- a/dane/dane.go +++ b/dane/dane.go @@ -61,29 +61,16 @@ import ( "golang.org/x/exp/slog" - "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/mlog" - "github.com/mjl-/mox/mox-" + "github.com/mjl-/mox/stub" ) var ( - metricVerify = promauto.NewCounter( - prometheus.CounterOpts{ - Name: "mox_dane_verify_total", - Help: "Total number of DANE verification attempts, including mox_dane_verify_errors_total.", - }, - ) - metricVerifyErrors = promauto.NewCounter( - prometheus.CounterOpts{ - Name: "mox_dane_verify_errors_total", - Help: "Total number of DANE verification failures, causing connections to fail.", - }, - ) + MetricVerify stub.Counter = stub.CounterIgnore{} + MetricVerifyErrors stub.Counter = stub.CounterIgnore{} ) var ( @@ -134,7 +121,7 @@ func (e VerifyError) Unwrap() error { // indicate DNSSEC errors. // - ErrInsecure // - VerifyError, potentially wrapping errors from crypto/x509. -func Dial(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, network, address string, allowedUsages []adns.TLSAUsage) (net.Conn, adns.TLSA, error) { +func Dial(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, network, address string, allowedUsages []adns.TLSAUsage, pkixRoots *x509.CertPool) (net.Conn, adns.TLSA, error) { log := mlog.New("dane", elog) // Split host and port. @@ -274,7 +261,7 @@ func Dial(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, network } var verifiedRecord adns.TLSA - config := TLSClientConfig(log.Logger, records, baseDom, moreAllowedHosts, &verifiedRecord) + config := TLSClientConfig(log.Logger, records, baseDom, moreAllowedHosts, &verifiedRecord, pkixRoots) tlsConn := tls.Client(conn, &config) if err := tlsConn.HandshakeContext(ctx); err != nil { conn.Close() @@ -297,13 +284,13 @@ func Dial(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, network // // If verifiedRecord is not nil, it is set to the record that was successfully // verified, if any. -func TLSClientConfig(elog *slog.Logger, records []adns.TLSA, allowedHost dns.Domain, moreAllowedHosts []dns.Domain, verifiedRecord *adns.TLSA) tls.Config { +func TLSClientConfig(elog *slog.Logger, records []adns.TLSA, allowedHost dns.Domain, moreAllowedHosts []dns.Domain, verifiedRecord *adns.TLSA, pkixRoots *x509.CertPool) tls.Config { log := mlog.New("dane", elog) return tls.Config{ ServerName: allowedHost.ASCII, // For SNI. InsecureSkipVerify: true, VerifyConnection: func(cs tls.ConnectionState) error { - verified, record, err := Verify(log.Logger, records, cs, allowedHost, moreAllowedHosts) + verified, record, err := Verify(log.Logger, records, cs, allowedHost, moreAllowedHosts, pkixRoots) log.Debugx("dane verification", err, slog.Bool("verified", verified), slog.Any("record", record)) if verified { if verifiedRecord != nil { @@ -335,23 +322,23 @@ func TLSClientConfig(elog *slog.Logger, records []adns.TLSA, allowedHost dns.Dom // If an error is encountered while verifying a record, e.g. for x509 // trusted-anchor verification, an error may be returned, typically one or more // (wrapped) errors of type VerifyError. -func Verify(elog *slog.Logger, records []adns.TLSA, cs tls.ConnectionState, allowedHost dns.Domain, moreAllowedHosts []dns.Domain) (verified bool, matching adns.TLSA, rerr error) { +func Verify(elog *slog.Logger, records []adns.TLSA, cs tls.ConnectionState, allowedHost dns.Domain, moreAllowedHosts []dns.Domain, pkixRoots *x509.CertPool) (verified bool, matching adns.TLSA, rerr error) { log := mlog.New("dane", elog) - metricVerify.Inc() + MetricVerify.Inc() if len(records) == 0 { - metricVerifyErrors.Inc() + MetricVerifyErrors.Inc() return false, adns.TLSA{}, fmt.Errorf("verify requires at least one tlsa record") } var errs []error for _, r := range records { - ok, err := verifySingle(log, r, cs, allowedHost, moreAllowedHosts) + ok, err := verifySingle(log, r, cs, allowedHost, moreAllowedHosts, pkixRoots) if err != nil { errs = append(errs, VerifyError{err, r}) } else if ok { return true, r, nil } } - metricVerifyErrors.Inc() + MetricVerifyErrors.Inc() return false, adns.TLSA{}, errors.Join(errs...) } @@ -364,7 +351,7 @@ func Verify(elog *slog.Logger, records []adns.TLSA, cs tls.ConnectionState, allo // errors while verifying certificates against a trust-anchor, an error can be // returned with one or more underlying x509 verification errors. A nil-nil error // is only returned when verified is false. -func verifySingle(log mlog.Log, tlsa adns.TLSA, cs tls.ConnectionState, allowedHost dns.Domain, moreAllowedHosts []dns.Domain) (verified bool, rerr error) { +func verifySingle(log mlog.Log, tlsa adns.TLSA, cs tls.ConnectionState, allowedHost dns.Domain, moreAllowedHosts []dns.Domain, pkixRoots *x509.CertPool) (verified bool, rerr error) { if len(cs.PeerCertificates) == 0 { return false, fmt.Errorf("no server certificate") } @@ -401,7 +388,7 @@ func verifySingle(log mlog.Log, tlsa adns.TLSA, cs tls.ConnectionState, allowedH opts := x509.VerifyOptions{ DNSName: host.ASCII, Intermediates: x509.NewCertPool(), - Roots: mox.Conf.Static.TLS.CertPool, + Roots: pkixRoots, } for _, cert := range cs.PeerCertificates[1:] { opts.Intermediates.AddCert(cert) diff --git a/dane/dane_test.go b/dane/dane_test.go index 67f1558..1795186 100644 --- a/dane/dane_test.go +++ b/dane/dane_test.go @@ -26,7 +26,6 @@ import ( "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" - "github.com/mjl-/mox/mox-" ) func tcheckf(t *testing.T, err error, format string, args ...any) { @@ -137,11 +136,13 @@ func TestDial(t *testing.T) { dialHost := "localhost" var allowedUsages []adns.TLSAUsage + pkixRoots := x509.NewCertPool() + // Helper function for dialing with DANE. test := func(resolver dns.Resolver, expRecord adns.TLSA, expErr any) { t.Helper() - conn, record, err := Dial(context.Background(), log.Logger, resolver, "tcp", net.JoinHostPort(dialHost, portstr), allowedUsages) + conn, record, err := Dial(context.Background(), log.Logger, resolver, "tcp", net.JoinHostPort(dialHost, portstr), allowedUsages, pkixRoots) if err == nil { conn.Close() } @@ -457,11 +458,8 @@ func TestDial(t *testing.T) { } test(resolver, zeroRecord, &x509.UnknownAuthorityError{}) - // Now we add the TA to the "system" trusted roots and try again. - pool, err := x509.SystemCertPool() - tcheckf(t, err, "get system certificate pool") - mox.Conf.Static.TLS.CertPool = pool - pool.AddCert(taCert) + // Now we add the TA to the "pkix" trusted roots and try again. + pkixRoots.AddCert(taCert) // PKIXEE, will now succeed. resolver = dns.MockResolver{ diff --git a/dkim/dkim.go b/dkim/dkim.go index 931fc2d..dedced9 100644 --- a/dkim/dkim.go +++ b/dkim/dkim.go @@ -26,38 +26,17 @@ import ( "golang.org/x/exp/slog" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - - "github.com/mjl-/mox/config" "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/moxio" "github.com/mjl-/mox/publicsuffix" "github.com/mjl-/mox/smtp" + "github.com/mjl-/mox/stub" ) var ( - metricSign = promauto.NewCounterVec( - prometheus.CounterOpts{ - Name: "mox_dkim_sign_total", - Help: "DKIM messages signings, label key is the type of key, rsa or ed25519.", - }, - []string{ - "key", - }, - ) - metricVerify = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_dkim_verify_duration_seconds", - Help: "DKIM verify, including lookup, duration and result.", - Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20}, - }, - []string{ - "algorithm", - "status", - }, - ) + MetricSign stub.CounterVec = stub.CounterVecIgnore{} + MetricVerify stub.HistogramVec = stub.HistogramVecIgnore{} ) var timeNow = time.Now // Replaced during tests. @@ -122,8 +101,28 @@ type Result struct { // todo: use some io.Writer to hash the body and the header. +// Selector holds selectors and key material to generate DKIM signatures. +type Selector struct { + Hash string // "sha256" or the older "sha1". + HeaderRelaxed bool // If the header is canonicalized in relaxed instead of simple mode. + BodyRelaxed bool // If the body is canonicalized in relaxed instead of simple mode. + Headers []string // Headers to include in signature. + + // Whether to "oversign" headers, ensuring additional/new values of existing + // headers cannot be added. + SealHeaders bool + + // If > 0, 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. + Expiration time.Duration + + PrivateKey crypto.Signer // Either an *rsa.PrivateKey or ed25519.PrivateKey. + Domain dns.Domain // Of selector only, not FQDN. +} + // Sign returns line(s) with DKIM-Signature headers, generated according to the configuration. -func Sign(ctx context.Context, elog *slog.Logger, localpart smtp.Localpart, domain dns.Domain, c config.DKIM, smtputf8 bool, msg io.ReaderAt) (headers string, rerr error) { +func Sign(ctx context.Context, elog *slog.Logger, localpart smtp.Localpart, domain dns.Domain, selectors []Selector, smtputf8 bool, msg io.ReaderAt) (headers string, rerr error) { log := mlog.New("dkim", elog) start := timeNow() defer func() { @@ -155,26 +154,25 @@ func Sign(ctx context.Context, elog *slog.Logger, localpart smtp.Localpart, doma var bodyHashes = map[hashKey][]byte{} - for _, sign := range c.Sign { - sel := c.Selectors[sign] + for _, sel := range selectors { sig := newSigWithDefaults() sig.Version = 1 - switch sel.Key.(type) { + switch sel.PrivateKey.(type) { case *rsa.PrivateKey: sig.AlgorithmSign = "rsa" - metricSign.WithLabelValues("rsa").Inc() + MetricSign.IncLabels("rsa") case ed25519.PrivateKey: sig.AlgorithmSign = "ed25519" - metricSign.WithLabelValues("ed25519").Inc() + MetricSign.IncLabels("ed25519") default: - return "", fmt.Errorf("internal error, unknown pivate key %T", sel.Key) + return "", fmt.Errorf("internal error, unknown pivate key %T", sel.PrivateKey) } - sig.AlgorithmHash = sel.HashEffective + sig.AlgorithmHash = sel.Hash sig.Domain = domain sig.Selector = sel.Domain sig.Identity = &Identity{&localpart, domain} - sig.SignedHeaders = append([]string{}, sel.HeadersEffective...) - if !sel.DontSealHeaders { + sig.SignedHeaders = append([]string{}, sel.Headers...) + if sel.SealHeaders { // ../rfc/6376:2156 // Each time a header name is added to the signature, the next unused value is // signed (in reverse order as they occur in the message). So we can add each @@ -184,23 +182,23 @@ func Sign(ctx context.Context, elog *slog.Logger, localpart smtp.Localpart, doma for _, h := range hdrs { counts[h.lkey]++ } - for _, h := range sel.HeadersEffective { + for _, h := range sel.Headers { for j := counts[strings.ToLower(h)]; j > 0; j-- { sig.SignedHeaders = append(sig.SignedHeaders, h) } } } sig.SignTime = timeNow().Unix() - if sel.ExpirationSeconds > 0 { - sig.ExpireTime = sig.SignTime + int64(sel.ExpirationSeconds) + if sel.Expiration > 0 { + sig.ExpireTime = sig.SignTime + int64(sel.Expiration/time.Second) } sig.Canonicalization = "simple" - if sel.Canonicalization.HeaderRelaxed { + if sel.HeaderRelaxed { sig.Canonicalization = "relaxed" } sig.Canonicalization += "/" - if sel.Canonicalization.BodyRelaxed { + if sel.BodyRelaxed { sig.Canonicalization += "relaxed" } else { sig.Canonicalization += "simple" @@ -217,12 +215,12 @@ func Sign(ctx context.Context, elog *slog.Logger, localpart smtp.Localpart, doma // DKIM-Signature header. // ../rfc/6376:1700 - hk := hashKey{!sel.Canonicalization.BodyRelaxed, strings.ToLower(sig.AlgorithmHash)} + hk := hashKey{!sel.BodyRelaxed, strings.ToLower(sig.AlgorithmHash)} if bh, ok := bodyHashes[hk]; ok { sig.BodyHash = bh } else { br := bufio.NewReader(&moxio.AtReader{R: msg, Offset: int64(bodyOffset)}) - bh, err = bodyHash(h.New(), !sel.Canonicalization.BodyRelaxed, br) + bh, err = bodyHash(h.New(), !sel.BodyRelaxed, br) if err != nil { return "", err } @@ -236,12 +234,12 @@ func Sign(ctx context.Context, elog *slog.Logger, localpart smtp.Localpart, doma } verifySig := []byte(strings.TrimSuffix(sigh, "\r\n")) - dh, err := dataHash(h.New(), !sel.Canonicalization.HeaderRelaxed, sig, hdrs, verifySig) + dh, err := dataHash(h.New(), !sel.HeaderRelaxed, sig, hdrs, verifySig) if err != nil { return "", err } - switch key := sel.Key.(type) { + switch key := sel.PrivateKey.(type) { case *rsa.PrivateKey: sig.Signature, err = key.Sign(cryptorand.Reader, dh, h) if err != nil { @@ -358,7 +356,7 @@ func Verify(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, smtpu alg = r.Sig.Algorithm() } status := string(r.Status) - metricVerify.WithLabelValues(alg, status).Observe(duration) + MetricVerify.ObserveLabels(duration, alg, status) } if len(results) == 0 { diff --git a/dkim/dkim_test.go b/dkim/dkim_test.go index 34dcd80..8cc66ef 100644 --- a/dkim/dkim_test.go +++ b/dkim/dkim_test.go @@ -15,7 +15,6 @@ import ( "strings" "testing" - "github.com/mjl-/mox/config" "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" ) @@ -222,50 +221,42 @@ test rsaKey := getRSAKey(t) ed25519Key := ed25519.NewKeyFromSeed(make([]byte, 32)) - selrsa := config.Selector{ - HashEffective: "sha256", - Key: rsaKey, - HeadersEffective: strings.Split("From,To,Cc,Bcc,Reply-To,References,In-Reply-To,Subject,Date,Message-ID,Content-Type", ","), - Domain: dns.Domain{ASCII: "testrsa"}, + selrsa := Selector{ + Hash: "sha256", + PrivateKey: rsaKey, + Headers: strings.Split("From,To,Cc,Bcc,Reply-To,References,In-Reply-To,Subject,Date,Message-ID,Content-Type", ","), + Domain: dns.Domain{ASCII: "testrsa"}, } // Now with sha1 and relaxed canonicalization. - selrsa2 := config.Selector{ - HashEffective: "sha1", - Key: rsaKey, - HeadersEffective: strings.Split("From,To,Cc,Bcc,Reply-To,References,In-Reply-To,Subject,Date,Message-ID,Content-Type", ","), - Domain: dns.Domain{ASCII: "testrsa2"}, + selrsa2 := Selector{ + Hash: "sha1", + PrivateKey: rsaKey, + Headers: strings.Split("From,To,Cc,Bcc,Reply-To,References,In-Reply-To,Subject,Date,Message-ID,Content-Type", ","), + Domain: dns.Domain{ASCII: "testrsa2"}, } - selrsa2.Canonicalization.HeaderRelaxed = true - selrsa2.Canonicalization.BodyRelaxed = true + selrsa2.HeaderRelaxed = true + selrsa2.BodyRelaxed = true // Ed25519 key. - seled25519 := config.Selector{ - HashEffective: "sha256", - Key: ed25519Key, - HeadersEffective: strings.Split("From,To,Cc,Bcc,Reply-To,References,In-Reply-To,Subject,Date,Message-ID,Content-Type", ","), - Domain: dns.Domain{ASCII: "tested25519"}, + seled25519 := Selector{ + Hash: "sha256", + PrivateKey: ed25519Key, + Headers: strings.Split("From,To,Cc,Bcc,Reply-To,References,In-Reply-To,Subject,Date,Message-ID,Content-Type", ","), + Domain: dns.Domain{ASCII: "tested25519"}, } // Again ed25519, but without sealing headers. Use sha256 again, for reusing the body hash from the previous dkim-signature. - seled25519b := config.Selector{ - HashEffective: "sha256", - Key: ed25519Key, - HeadersEffective: strings.Split("From,To,Cc,Bcc,Reply-To,Subject,Date", ","), - DontSealHeaders: true, - Domain: dns.Domain{ASCII: "tested25519b"}, - } - dkimConf := config.DKIM{ - Selectors: map[string]config.Selector{ - "testrsa": selrsa, - "testrsa2": selrsa2, - "tested25519": seled25519, - "tested25519b": seled25519b, - }, - Sign: []string{"testrsa", "testrsa2", "tested25519", "tested25519b"}, + seled25519b := Selector{ + Hash: "sha256", + PrivateKey: ed25519Key, + Headers: strings.Split("From,To,Cc,Bcc,Reply-To,Subject,Date", ","), + SealHeaders: true, + Domain: dns.Domain{ASCII: "tested25519b"}, } + selectors := []Selector{selrsa, selrsa2, seled25519, seled25519b} ctx := context.Background() - headers, err := Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, dkimConf, false, strings.NewReader(message)) + headers, err := Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, selectors, false, strings.NewReader(message)) if err != nil { t.Fatalf("sign: %v", err) } @@ -307,31 +298,31 @@ test //log.Infof("nmsg\n%s", nmsg) // Multiple From headers. - _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, dkimConf, false, strings.NewReader("From: \r\nFrom: \r\n\r\ntest")) + _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, selectors, false, strings.NewReader("From: \r\nFrom: \r\n\r\ntest")) if !errors.Is(err, ErrFrom) { t.Fatalf("sign, got err %v, expected ErrFrom", err) } // No From header. - _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, dkimConf, false, strings.NewReader("Brom: \r\n\r\ntest")) + _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, selectors, false, strings.NewReader("Brom: \r\n\r\ntest")) if !errors.Is(err, ErrFrom) { t.Fatalf("sign, got err %v, expected ErrFrom", err) } // Malformed headers. - _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, dkimConf, false, strings.NewReader(":\r\n\r\ntest")) + _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, selectors, false, strings.NewReader(":\r\n\r\ntest")) if !errors.Is(err, ErrHeaderMalformed) { t.Fatalf("sign, got err %v, expected ErrHeaderMalformed", err) } - _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, dkimConf, false, strings.NewReader(" From:\r\n\r\ntest")) + _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, selectors, false, strings.NewReader(" From:\r\n\r\ntest")) if !errors.Is(err, ErrHeaderMalformed) { t.Fatalf("sign, got err %v, expected ErrHeaderMalformed", err) } - _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, dkimConf, false, strings.NewReader("Frøm:\r\n\r\ntest")) + _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, selectors, false, strings.NewReader("Frøm:\r\n\r\ntest")) if !errors.Is(err, ErrHeaderMalformed) { t.Fatalf("sign, got err %v, expected ErrHeaderMalformed", err) } - _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, dkimConf, false, strings.NewReader("From:")) + _, err = Sign(ctx, pkglog.Logger, "mjl", dns.Domain{ASCII: "mox.example"}, selectors, false, strings.NewReader("From:")) if !errors.Is(err, ErrHeaderMalformed) { t.Fatalf("sign, got err %v, expected ErrHeaderMalformed", err) } @@ -358,9 +349,9 @@ test var record *Record var recordTxt string var msg string - var sel config.Selector - var dkimConf config.DKIM var policy func(*Sig) error + var sel Selector + var selectors []Selector var signed bool var signDomain dns.Domain @@ -389,18 +380,13 @@ test }, } - sel = config.Selector{ - HashEffective: "sha256", - Key: key, - HeadersEffective: strings.Split("From,To,Cc,Bcc,Reply-To,References,In-Reply-To,Subject,Date,Message-ID,Content-Type", ","), - Domain: dns.Domain{ASCII: "test"}, - } - dkimConf = config.DKIM{ - Selectors: map[string]config.Selector{ - "test": sel, - }, - Sign: []string{"test"}, + sel = Selector{ + Hash: "sha256", + PrivateKey: key, + Headers: strings.Split("From,To,Cc,Bcc,Reply-To,References,In-Reply-To,Subject,Date,Message-ID,Content-Type", ","), + Domain: dns.Domain{ASCII: "test"}, } + selectors = []Selector{sel} msg = message signed = false @@ -411,7 +397,7 @@ test msg = strings.ReplaceAll(msg, "\n", "\r\n") - headers, err := Sign(context.Background(), pkglog.Logger, "mjl", signDomain, dkimConf, false, strings.NewReader(msg)) + headers, err := Sign(context.Background(), pkglog.Logger, "mjl", signDomain, selectors, false, strings.NewReader(msg)) if err != nil { t.Fatalf("sign: %v", err) } @@ -515,11 +501,9 @@ test }) // Unknown canonicalization. test(nil, StatusPermerror, ErrCanonicalizationUnknown, func() { - sel.Canonicalization.HeaderRelaxed = true - sel.Canonicalization.BodyRelaxed = true - dkimConf.Selectors = map[string]config.Selector{ - "test": sel, - } + sel.HeaderRelaxed = true + sel.BodyRelaxed = true + selectors = []Selector{sel} sign() msg = strings.ReplaceAll(msg, "relaxed/relaxed", "bogus/bogus") @@ -577,10 +561,8 @@ test resolver.TXT = map[string][]string{ "test._domainkey.mox.example.": {txt}, } - sel.Key = key - dkimConf.Selectors = map[string]config.Selector{ - "test": sel, - } + sel.PrivateKey = key + selectors = []Selector{sel} }) // Key not allowed for email by DNS record. ../rfc/6376:1541 test(nil, StatusPermerror, ErrKeyNotForEmail, func() { @@ -603,18 +585,14 @@ test // Check that last-occurring header field is used. test(nil, StatusFail, ErrSigVerify, func() { - sel.DontSealHeaders = true - dkimConf.Selectors = map[string]config.Selector{ - "test": sel, - } + sel.SealHeaders = false + selectors = []Selector{sel} sign() msg = strings.ReplaceAll(msg, "\r\n\r\n", "\r\nsubject: another\r\n\r\n") }) test(nil, StatusPass, nil, func() { - sel.DontSealHeaders = true - dkimConf.Selectors = map[string]config.Selector{ - "test": sel, - } + sel.SealHeaders = false + selectors = []Selector{sel} sign() msg = "subject: another\r\n" + msg }) diff --git a/dkim/parser.go b/dkim/parser.go index 7e60254..dbbc1d6 100644 --- a/dkim/parser.go +++ b/dkim/parser.go @@ -7,10 +7,12 @@ import ( "strings" "github.com/mjl-/mox/dns" - "github.com/mjl-/mox/moxvar" "github.com/mjl-/mox/smtp" ) +// Pedantic enables stricter parsing. +var Pedantic bool + type parseErr string func (e parseErr) Error() string { @@ -200,7 +202,7 @@ func (p *parser) xdomainselector(isselector bool) dns.Domain { // domain names must always be a-labels, ../rfc/6376:1115 ../rfc/6376:1187 ../rfc/6376:1303 // dkim selectors with underscores happen in the wild, accept them when not in // pedantic mode. ../rfc/6376:581 ../rfc/5321:2303 - return isalphadigit(c) || (i > 0 && (c == '-' || isselector && !moxvar.Pedantic && c == '_') && p.o+1 < len(p.s)) + return isalphadigit(c) || (i > 0 && (c == '-' || isselector && !Pedantic && c == '_') && p.o+1 < len(p.s)) } s := p.xtakefn1(false, subdomain) for p.hasPrefix(".") { @@ -273,7 +275,7 @@ func (p *parser) xlocalpart() smtp.Localpart { } } // In the wild, some services use large localparts for generated (bounce) addresses. - if moxvar.Pedantic && len(s) > 64 || len(s) > 128 { + if Pedantic && len(s) > 64 || len(s) > 128 { // ../rfc/5321:3486 p.xerrorf("localpart longer than 64 octets") } diff --git a/dmarc/dmarc.go b/dmarc/dmarc.go index b688815..20ba825 100644 --- a/dmarc/dmarc.go +++ b/dmarc/dmarc.go @@ -17,31 +17,18 @@ import ( mathrand "math/rand" "time" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/mjl-/mox/dkim" "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/publicsuffix" "github.com/mjl-/mox/spf" + "github.com/mjl-/mox/stub" "golang.org/x/exp/slog" ) var ( - metricDMARCVerify = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_dmarc_verify_duration_seconds", - Help: "DMARC verify, including lookup, duration and result.", - Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20}, - }, - []string{ - "status", - "reject", // yes/no - "use", // yes/no, if policy is used after random selection - }, - ) + MetricVerify stub.HistogramVec = stub.HistogramVecIgnore{} ) // link errata: @@ -248,7 +235,7 @@ func Verify(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, from if result.Reject { reject = "yes" } - metricDMARCVerify.WithLabelValues(string(result.Status), reject, use).Observe(float64(time.Since(start)) / float64(time.Second)) + MetricVerify.ObserveLabels(float64(time.Since(start))/float64(time.Second), string(result.Status), reject, use) log.Debugx("dmarc verify result", result.Err, slog.Any("fromdomain", from), slog.Any("dkimresults", dkimResults), diff --git a/dmarcdb/eval.go b/dmarcdb/eval.go index 1c12dde..85cf599 100644 --- a/dmarcdb/eval.go +++ b/dmarcdb/eval.go @@ -1068,8 +1068,9 @@ func dkimSign(ctx context.Context, log mlog.Log, fromAddr smtp.Address, smtputf8 var zerodom dns.Domain for fd != zerodom { confDom, ok := mox.Conf.Domain(fd) - if len(confDom.DKIM.Sign) > 0 { - dkimHeaders, err := dkim.Sign(ctx, log.Logger, fromAddr.Localpart, fd, confDom.DKIM, smtputf8, mf) + selectors := mox.DKIMSelectors(confDom.DKIM) + if len(selectors) > 0 { + dkimHeaders, err := dkim.Sign(ctx, log.Logger, fromAddr.Localpart, fd, selectors, smtputf8, mf) if err != nil { log.Errorx("dkim-signing dmarc report, continuing without signature", err) metricReportError.Inc() diff --git a/dns/dns.go b/dns/dns.go index 3247ba3..df7641d 100644 --- a/dns/dns.go +++ b/dns/dns.go @@ -10,10 +10,11 @@ import ( "golang.org/x/net/idna" "github.com/mjl-/adns" - - "github.com/mjl-/mox/moxvar" ) +// Pedantic enables stricter parsing. +var Pedantic bool + var ( errTrailingDot = errors.New("dns name has trailing dot") errUnderscore = errors.New("domain name with underscore") @@ -113,7 +114,7 @@ func ParseDomain(s string) (Domain, error) { // is not enabled. Used for interoperability, e.g. domains may specify MX // targets with underscores. func ParseDomainLax(s string) (Domain, error) { - if moxvar.Pedantic || !strings.Contains(s, "_") { + if Pedantic || !strings.Contains(s, "_") { return ParseDomain(s) } diff --git a/dns/resolver.go b/dns/resolver.go index 9a1d100..0c5deab 100644 --- a/dns/resolver.go +++ b/dns/resolver.go @@ -12,12 +12,10 @@ import ( "golang.org/x/exp/slog" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/mjl-/adns" "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/stub" ) // todo future: replace with a dnssec capable resolver @@ -29,18 +27,7 @@ func init() { } var ( - metricLookup = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_dns_lookup_duration_seconds", - Help: "DNS lookups.", - Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, - }, - []string{ - "pkg", - "type", // Lower-case Resolver method name without leading Lookup. - "result", // ok, nxdomain, temporary, timeout, canceled, error - }, - ) + MetricLookup stub.HistogramVec = stub.HistogramVecIgnore{} ) // Resolver is the interface strict resolver implements. @@ -106,7 +93,7 @@ func metricLookupObserve(pkg, typ string, err error, start time.Time) { default: result = "error" } - metricLookup.WithLabelValues(pkg, typ, result).Observe(float64(time.Since(start)) / float64(time.Second)) + MetricLookup.ObserveLabels(float64(time.Since(start))/float64(time.Second), pkg, typ, result) } func (r StrictResolver) WithPackage(name string) Resolver { diff --git a/dnsbl/dnsbl.go b/dnsbl/dnsbl.go index 8114f70..8f39ac7 100644 --- a/dnsbl/dnsbl.go +++ b/dnsbl/dnsbl.go @@ -12,25 +12,13 @@ import ( "golang.org/x/exp/slog" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/stub" ) var ( - metricLookup = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_dnsbl_lookup_duration_seconds", - Help: "DNSBL lookup", - Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20}, - }, - []string{ - "zone", - "status", - }, - ) + MetricLookup stub.HistogramVec = stub.HistogramVecIgnore{} ) var ErrDNS = errors.New("dnsbl: dns error") @@ -49,7 +37,7 @@ func Lookup(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, zone log := mlog.New("dnsbl", elog) start := time.Now() defer func() { - metricLookup.WithLabelValues(zone.Name(), string(rstatus)).Observe(float64(time.Since(start)) / float64(time.Second)) + MetricLookup.ObserveLabels(float64(time.Since(start))/float64(time.Second), zone.Name(), string(rstatus)) log.Debugx("dnsbl lookup result", rerr, slog.Any("zone", zone), slog.Any("ip", ip), diff --git a/dsn/dsn.go b/dsn/dsn.go index 2e6605b..d36e429 100644 --- a/dsn/dsn.go +++ b/dsn/dsn.go @@ -5,7 +5,6 @@ package dsn import ( "bufio" "bytes" - "context" "encoding/base64" "errors" "fmt" @@ -16,13 +15,8 @@ import ( "strings" "time" - "golang.org/x/exp/slog" - - "github.com/mjl-/mox/dkim" - "github.com/mjl-/mox/dns" "github.com/mjl-/mox/message" "github.com/mjl-/mox/mlog" - "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/smtp" ) @@ -48,7 +42,6 @@ type Message struct { // Message subject header, e.g. describing mail delivery failure. Subject string - // Set when message is composed. MessageID string // References header, with Message-ID of original message this DSN is about. So @@ -136,7 +129,7 @@ type Recipient struct { // supports smtputf8. This influences the message media (sub)types used for the // DSN. // -// DKIM signatures are added if DKIM signing is configured for the "from" domain. +// Called may want to add DKIM-Signature headers. func (m *Message) Compose(log mlog.Log, smtputf8 bool) ([]byte, error) { // ../rfc/3462:119 // ../rfc/3464:377 @@ -168,7 +161,9 @@ func (m *Message) Compose(log mlog.Log, smtputf8 bool) ([]byte, error) { header("From", fmt.Sprintf("<%s>", m.From.XString(smtputf8))) // todo: would be good to have a local ascii-only name for this address. header("To", fmt.Sprintf("<%s>", m.To.XString(smtputf8))) // todo: we could just leave this out if it has utf-8 and remote does not support utf-8. header("Subject", m.Subject) - m.MessageID = mox.MessageIDGen(smtputf8) + if m.MessageID == "" { + return nil, fmt.Errorf("missing message-id") + } header("Message-Id", fmt.Sprintf("<%s>", m.MessageID)) if m.References != "" { header("References", m.References) @@ -367,31 +362,6 @@ func (m *Message) Compose(log mlog.Log, smtputf8 bool) ([]byte, error) { } data := msgw.w.Bytes() - - // Add DKIM signature for domain, even if higher up than the full mail hostname. - // This helps with an assumed (because default) relaxed DKIM policy. If the DMARC - // policy happens to be strict, the signature won't help, but won't hurt either. - fd := m.From.IPDomain.Domain - var zerodom dns.Domain - for fd != zerodom { - confDom, ok := mox.Conf.Domain(fd) - if !ok { - var nfd dns.Domain - _, nfd.ASCII, _ = strings.Cut(fd.ASCII, ".") - _, nfd.Unicode, _ = strings.Cut(fd.Unicode, ".") - fd = nfd - continue - } - - dkimHeaders, err := dkim.Sign(context.Background(), log.Logger, m.From.Localpart, fd, confDom.DKIM, smtputf8, bytes.NewReader(data)) - if err != nil { - log.Errorx("dsn: dkim sign for domain, returning unsigned dsn", err, slog.Any("domain", fd)) - } else { - data = append([]byte(dkimHeaders), data...) - } - break - } - return data, nil } diff --git a/dsn/dsn_test.go b/dsn/dsn_test.go index c3edc3f..4f57d17 100644 --- a/dsn/dsn_test.go +++ b/dsn/dsn_test.go @@ -2,21 +2,17 @@ package dsn import ( "bytes" - "context" "fmt" "io" "net" - "path/filepath" "reflect" "strings" "testing" "time" - "github.com/mjl-/mox/dkim" "github.com/mjl-/mox/dns" "github.com/mjl-/mox/message" "github.com/mjl-/mox/mlog" - "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/smtp" ) @@ -83,10 +79,11 @@ func TestDSN(t *testing.T) { m := Message{ SMTPUTF8: false, - From: smtp.Path{Localpart: "postmaster", IPDomain: xparseIPDomain("mox.example")}, - To: smtp.Path{Localpart: "mjl", IPDomain: xparseIPDomain("remote.example")}, - Subject: "dsn", - TextBody: "delivery failure\n", + From: smtp.Path{Localpart: "postmaster", IPDomain: xparseIPDomain("mox.example")}, + To: smtp.Path{Localpart: "mjl", IPDomain: xparseIPDomain("remote.example")}, + Subject: "dsn", + MessageID: "test@localhost", + TextBody: "delivery failure\n", ReportingMTA: "mox.example", ReceivedFromMTA: smtp.Ehlo{Name: xparseIPDomain("relay.example"), ConnIP: net.ParseIP("10.10.10.10")}, @@ -107,6 +104,7 @@ func TestDSN(t *testing.T) { if err != nil { t.Fatalf("composing dsn: %v", err) } + pmsg, part := tparseMessage(t, msgbuf, 3) tcheckType(t, part, "multipart", "report", "") tcheckType(t, &part.Parts[0], "text", "plain", "7bit") @@ -130,35 +128,15 @@ func TestDSN(t *testing.T) { tcompareReader(t, part.Parts[2].Reader(), m.Original) tcompare(t, pmsg.Recipients[0].FinalRecipient, m.Recipients[0].FinalRecipient) - // Test for valid DKIM signature. - mox.Context = context.Background() - mox.ConfigStaticPath = filepath.FromSlash("../testdata/dsn/mox.conf") - mox.MustLoadConfig(true, false) - msgbuf, err = m.Compose(log, false) - if err != nil { - t.Fatalf("composing utf-8 dsn with utf-8 support: %v", err) - } - resolver := &dns.MockResolver{ - TXT: map[string][]string{ - "testsel._domainkey.mox.example.": {"v=DKIM1;h=sha256;t=s;p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA3ZId3ys70VFspp/VMFaxMOrNjHNPg04NOE1iShih16b3Ex7hHBOgC1UvTGSmrMlbCB1OxTXkvf6jW6S4oYRnZYVNygH6zKUwYYhaSaGIg1xA/fDn+IgcTRyLoXizMUgUgpTGyxhNrwIIWv+i7jjbs3TKpP3NU4owQ/rxowmSNqg+fHIF1likSvXvljYS" + "jaFXXnWfYibW7TdDCFFpN4sB5o13+as0u4vLw6MvOi59B1tLype1LcHpi1b9PfxNtznTTdet3kL0paxIcWtKHT0LDPUos8YYmiPa5nGbUqlC7d+4YT2jQPvwGxCws1oo2Tw6nj1UaihneYGAyvEky49FBwIDAQAB"}, - }, - } - results, err := dkim.Verify(context.Background(), log.Logger, resolver, false, func(*dkim.Sig) error { return nil }, bytes.NewReader(msgbuf), false) - if err != nil { - t.Fatalf("dkim verify: %v", err) - } - if len(results) != 1 || results[0].Status != dkim.StatusPass { - t.Fatalf("dkim result not pass, %#v", results) - } - // An utf-8 message. m = Message{ SMTPUTF8: true, - From: smtp.Path{Localpart: "postmæster", IPDomain: xparseIPDomain("møx.example")}, - To: smtp.Path{Localpart: "møx", IPDomain: xparseIPDomain("remøte.example")}, - Subject: "dsn¡", - TextBody: "delivery failure¿\n", + From: smtp.Path{Localpart: "postmæster", IPDomain: xparseIPDomain("møx.example")}, + To: smtp.Path{Localpart: "møx", IPDomain: xparseIPDomain("remøte.example")}, + Subject: "dsn¡", + MessageID: "test@localhost", + TextBody: "delivery failure¿\n", ReportingMTA: "mox.example", ReceivedFromMTA: smtp.Ehlo{Name: xparseIPDomain("reläy.example"), ConnIP: net.ParseIP("10.10.10.10")}, diff --git a/imapserver/condstore_test.go b/imapserver/condstore_test.go index c9a3215..d2dd312 100644 --- a/imapserver/condstore_test.go +++ b/imapserver/condstore_test.go @@ -8,7 +8,7 @@ import ( "github.com/mjl-/bstore" "github.com/mjl-/mox/imapclient" - "github.com/mjl-/mox/moxvar" + "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/store" ) @@ -150,9 +150,9 @@ func testCondstoreQresync(t *testing.T, qresync bool) { imapclient.UntaggedFetch{Seq: 6, Attrs: []imapclient.FetchAttr{imapclient.FetchUID(6), noflags, imapclient.FetchModSeq(clientModseq + 4)}}, ) - moxvar.Pedantic = true + mox.SetPedantic(true) tc.transactf("bad", `Fetch 1 Flags (Changedsince 0)`) // 0 not allowed in syntax. - moxvar.Pedantic = false + mox.SetPedantic(false) tc.transactf("ok", "Uid fetch 1 (Flags) (Changedsince 0)") tc.xuntagged(imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{imapclient.FetchUID(1), noflags, imapclient.FetchModSeq(clientModseq)}}) diff --git a/imapserver/fetch.go b/imapserver/fetch.go index 5541fda..7b09eae 100644 --- a/imapserver/fetch.go +++ b/imapserver/fetch.go @@ -17,8 +17,8 @@ import ( "github.com/mjl-/bstore" "github.com/mjl-/mox/message" + "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/moxio" - "github.com/mjl-/mox/moxvar" "github.com/mjl-/mox/store" ) @@ -102,7 +102,7 @@ func (c *conn) cmdxFetch(isUID bool, tag, cmdstr string, p *parser) { p.xspace() changedSince = p.xnumber64() // workaround: ios mail (16.5.1) was seen sending changedSince 0 on an existing account that got condstore enabled. - if changedSince == 0 && moxvar.Pedantic { + if changedSince == 0 && mox.Pedantic { // ../rfc/7162:2551 xsyntaxErrorf("changedsince modseq must be > 0") } diff --git a/imapserver/server.go b/imapserver/server.go index ac7b641..fb9a5fe 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -1456,7 +1456,7 @@ func (c *conn) cmdStarttls(tag, cmd string, p *parser) { panic(fmt.Errorf("starttls handshake: %s (%w)", err, errIO)) } cancel() - tlsversion, ciphersuite := mox.TLSInfo(tlsConn) + tlsversion, ciphersuite := moxio.TLSInfo(tlsConn) c.log.Debug("tls server handshake done", slog.String("tls", tlsversion), slog.String("ciphersuite", ciphersuite)) c.conn = tlsConn diff --git a/iprev/iprev.go b/iprev/iprev.go index e9de926..953470c 100644 --- a/iprev/iprev.go +++ b/iprev/iprev.go @@ -11,24 +11,15 @@ import ( "golang.org/x/exp/slog" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/stub" ) var xlog = mlog.New("iprev", nil) var ( - metricIPRev = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_iprev_lookup_total", - Help: "Number of iprev lookups.", - Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, - }, - []string{"status"}, - ) + MetricIPRev stub.HistogramVec = stub.HistogramVecIgnore{} ) // Lookup errors. @@ -62,7 +53,7 @@ func Lookup(ctx context.Context, resolver dns.Resolver, ip net.IP) (rstatus Stat log := xlog.WithContext(ctx) start := time.Now() defer func() { - metricIPRev.WithLabelValues(string(rstatus)).Observe(float64(time.Since(start)) / float64(time.Second)) + MetricIPRev.ObserveLabels(float64(time.Since(start))/float64(time.Second), string(rstatus)) log.Debugx("iprev lookup result", rerr, slog.Any("ip", ip), slog.Any("status", rstatus), diff --git a/main.go b/main.go index 6739508..8e99144 100644 --- a/main.go +++ b/main.go @@ -394,7 +394,7 @@ func mustLoadConfig() { log.Fatal("unknown loglevel", slog.String("loglevel", loglevel)) } if pedantic { - moxvar.Pedantic = true + mox.SetPedantic(true) } } @@ -445,7 +445,7 @@ func main() { defer profile(cpuprofile, memprofile)() if pedantic { - moxvar.Pedantic = true + mox.SetPedantic(true) } mox.ConfigDynamicPath = filepath.Join(filepath.Dir(mox.ConfigStaticPath), "domains.conf") @@ -1600,8 +1600,11 @@ connection. } } + pkixRoots, err := x509.SystemCertPool() + xcheckf(err, "get system pkix certificate pool") + resolver := dns.StrictResolver{Pkg: "danedial"} - conn, record, err := dane.Dial(context.Background(), c.log.Logger, resolver, "tcp", args[0], allowedUsages) + conn, record, err := dane.Dial(context.Background(), c.log.Logger, resolver, "tcp", args[0], allowedUsages, pkixRoots) xcheckf(err, "dial") log.Printf("(connected, verified with %s)", record) @@ -1756,7 +1759,7 @@ sharing most of its code. log.Printf("gathered valid tls certificate names for potential verification with dane-ta: %s", strings.Join(l, ", ")) dialer := &net.Dialer{Timeout: 5 * time.Second} - conn, _, err := smtpclient.Dial(ctxbg, c.log.Logger, dialer, dns.IPDomain{Domain: expandedHost}, ips, 25, dialedIPs) + conn, _, err := smtpclient.Dial(ctxbg, c.log.Logger, dialer, dns.IPDomain{Domain: expandedHost}, ips, 25, dialedIPs, nil) if err != nil { log.Printf("dial %s: %v, skipping", expandedHost, err) continue @@ -2238,7 +2241,8 @@ headers prepended. log.Fatalf("domain %s not configured", dom) } - headers, err := dkim.Sign(context.Background(), c.log.Logger, localpart, dom, domConf.DKIM, false, msgf) + selectors := mox.DKIMSelectors(domConf.DKIM) + headers, err := dkim.Sign(context.Background(), c.log.Logger, localpart, dom, selectors, false, msgf) xcheckf(err, "signing message with dkim") if headers == "" { log.Fatalf("no DKIM configured for domain %s", dom) diff --git a/moxio/decode.go b/message/decode.go similarity index 97% rename from moxio/decode.go rename to message/decode.go index cfc1b73..39a89bb 100644 --- a/moxio/decode.go +++ b/message/decode.go @@ -1,4 +1,4 @@ -package moxio +package message import ( "io" diff --git a/moxio/decode_test.go b/message/decode_test.go similarity index 93% rename from moxio/decode_test.go rename to message/decode_test.go index 0c165c5..c51fe00 100644 --- a/moxio/decode_test.go +++ b/message/decode_test.go @@ -1,4 +1,4 @@ -package moxio +package message import ( "io" @@ -10,7 +10,7 @@ func TestDecodeReader(t *testing.T) { check := func(charset, input, output string) { t.Helper() buf, err := io.ReadAll(DecodeReader(charset, strings.NewReader(input))) - tcheckf(t, err, "decode") + tcheck(t, err, "decode") if string(buf) != output { t.Fatalf("decoding %q with charset %q, got %q, expected %q", input, charset, buf, output) } diff --git a/message/messageid.go b/message/messageid.go index aba06d0..c5e8b93 100644 --- a/message/messageid.go +++ b/message/messageid.go @@ -5,7 +5,6 @@ import ( "fmt" "strings" - "github.com/mjl-/mox/moxvar" "github.com/mjl-/mox/smtp" ) @@ -29,7 +28,7 @@ func MessageIDCanonical(s string) (string, bool, error) { // Seen in practice: Message-ID: (added by postmaster@some.example) // Doesn't seem valid, but we allow it. s, rem, have := strings.Cut(s, ">") - if !have || (rem != "" && (moxvar.Pedantic || !strings.HasPrefix(rem, " "))) { + if !have || (rem != "" && (Pedantic || !strings.HasPrefix(rem, " "))) { return "", false, fmt.Errorf("%w: missing >", errBadMessageID) } // We canonicalize the Message-ID: lower-case, no unneeded quoting. diff --git a/message/part.go b/message/part.go index cf318fa..2a4fe36 100644 --- a/message/part.go +++ b/message/part.go @@ -25,11 +25,12 @@ import ( "golang.org/x/text/encoding/ianaindex" "github.com/mjl-/mox/mlog" - "github.com/mjl-/mox/moxio" - "github.com/mjl-/mox/moxvar" "github.com/mjl-/mox/smtp" ) +// Pedantic enables stricter parsing. +var Pedantic bool + var ( ErrBadContentType = errors.New("bad content-type") ) @@ -207,7 +208,7 @@ func (p *Part) Walk(elog *slog.Logger, parent *Part) error { // If this is a DSN and we are not in pedantic mode, accept unexpected end of // message. This is quite common because MTA's sometimes just truncate the original // message in a place that makes the message invalid. - if errors.Is(err, errUnexpectedEOF) && !moxvar.Pedantic && parent != nil && len(parent.Parts) >= 3 && p == &parent.Parts[2] && parent.MediaType == "MULTIPART" && parent.MediaSubType == "REPORT" { + if errors.Is(err, errUnexpectedEOF) && !Pedantic && parent != nil && len(parent.Parts) >= 3 && p == &parent.Parts[2] && parent.MediaType == "MULTIPART" && parent.MediaSubType == "REPORT" { mp, err = fallbackPart(mp, br, int64(len(buf))) if err != nil { return fmt.Errorf("parsing invalid embedded message: %w", err) @@ -305,7 +306,7 @@ func newPart(log mlog.Log, strict bool, r io.ReaderAt, offset int64, parent *Par ct := p.header.Get("Content-Type") mt, params, err := mime.ParseMediaType(ct) if err != nil && ct != "" { - if moxvar.Pedantic || strict { + if Pedantic || strict { return p, fmt.Errorf("%w: %s: %q", ErrBadContentType, err, ct) } @@ -337,7 +338,7 @@ func newPart(log mlog.Log, strict bool, r io.ReaderAt, offset int64, parent *Par } else if mt != "" { t := strings.SplitN(strings.ToUpper(mt), "/", 2) if len(t) != 2 { - if moxvar.Pedantic || strict { + if Pedantic || strict { return p, fmt.Errorf("bad content-type: %q (content-type %q)", mt, ct) } log.Debug("malformed media-type, ignoring and continuing", slog.String("type", mt)) @@ -584,7 +585,7 @@ func (p *Part) Reader() io.Reader { // already). For unknown or missing character sets/encodings, the original reader // is returned. func (p *Part) ReaderUTF8OrBinary() io.Reader { - return moxio.DecodeReader(p.ContentTypeParams["charset"], p.Reader()) + return DecodeReader(p.ContentTypeParams["charset"], p.Reader()) } func (p *Part) bodyReader(r io.Reader) io.Reader { @@ -689,7 +690,7 @@ func (r *crlfReader) Read(buf []byte) (int, error) { if b == '\n' && !r.prevcr { err = errBareLF break - } else if b != '\n' && r.prevcr && (r.strict || moxvar.Pedantic) { + } else if b != '\n' && r.prevcr && (r.strict || Pedantic) { err = errBareCR break } @@ -719,7 +720,7 @@ type bufAt struct { const maxLineLength = 8 * 1024 func (b *bufAt) maxLineLength() int { - if b.strict || moxvar.Pedantic { + if b.strict || Pedantic { return 1000 } return maxLineLength @@ -777,7 +778,7 @@ func (b *bufAt) line(consume, requirecrlf bool) (buf []byte, crlf bool, err erro } i++ if i >= b.nbuf || b.buf[i] != '\n' { - if b.strict || moxvar.Pedantic { + if b.strict || Pedantic { return nil, false, errBareCR } continue @@ -833,7 +834,7 @@ func (r *offsetReader) Read(buf []byte) (int, error) { if n > 0 { r.offset += int64(n) max := maxLineLength - if r.strict || moxvar.Pedantic { + if r.strict || Pedantic { max = 1000 } @@ -844,7 +845,7 @@ func (r *offsetReader) Read(buf []byte) (int, error) { if err == nil || err == io.EOF { if c == '\n' && !r.prevcr { err = errBareLF - } else if c != '\n' && r.prevcr && (r.strict || moxvar.Pedantic) { + } else if c != '\n' && r.prevcr && (r.strict || Pedantic) { err = errBareCR } } diff --git a/message/part_test.go b/message/part_test.go index 6935cfd..4bf1334 100644 --- a/message/part_test.go +++ b/message/part_test.go @@ -12,7 +12,6 @@ import ( "testing" "github.com/mjl-/mox/mlog" - "github.com/mjl-/mox/moxvar" ) var pkglog = mlog.New("message", nil) @@ -54,7 +53,7 @@ func TestBadContentType(t *testing.T) { expBody := "test" // Pedantic is like strict. - moxvar.Pedantic = true + Pedantic = true s := "content-type: text/html;;\r\n\r\ntest" p, err := EnsurePart(pkglog.Logger, false, strings.NewReader(s), int64(len(s))) tfail(t, err, ErrBadContentType) @@ -63,7 +62,7 @@ func TestBadContentType(t *testing.T) { tcompare(t, string(buf), expBody) tcompare(t, p.MediaType, "APPLICATION") tcompare(t, p.MediaSubType, "OCTET-STREAM") - moxvar.Pedantic = false + Pedantic = false // Strict s = "content-type: text/html;;\r\n\r\ntest" @@ -111,12 +110,12 @@ func TestBareCR(t *testing.T) { expBody := "bare\rcr\r\n" // Pedantic is like strict. - moxvar.Pedantic = true + Pedantic = true p, err := EnsurePart(pkglog.Logger, false, strings.NewReader(s), int64(len(s))) tfail(t, err, errBareCR) _, err = io.ReadAll(p.Reader()) tfail(t, err, errBareCR) - moxvar.Pedantic = false + Pedantic = false // Strict. p, err = EnsurePart(pkglog.Logger, true, strings.NewReader(s), int64(len(s))) @@ -291,12 +290,12 @@ func TestBareCrLf(t *testing.T) { err = parse(false, "\r\ntest\ntest\r\n") tfail(t, err, errBareLF) - moxvar.Pedantic = true + Pedantic = true err = parse(false, "subject: test\rtest\r\n") tfail(t, err, errBareCR) err = parse(false, "\r\ntest\rtest\r\n") tfail(t, err, errBareCR) - moxvar.Pedantic = false + Pedantic = false err = parse(true, "subject: test\rtest\r\n") tfail(t, err, errBareCR) diff --git a/metrics.go b/metrics.go new file mode 100644 index 0000000..693f611 --- /dev/null +++ b/metrics.go @@ -0,0 +1,267 @@ +package main + +import ( + "context" + "errors" + "fmt" + "os" + "time" + + "golang.org/x/exp/slog" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + + "github.com/mjl-/mox/dane" + "github.com/mjl-/mox/dkim" + "github.com/mjl-/mox/dmarc" + "github.com/mjl-/mox/dns" + "github.com/mjl-/mox/dnsbl" + "github.com/mjl-/mox/iprev" + "github.com/mjl-/mox/metrics" + "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/mtasts" + "github.com/mjl-/mox/smtpclient" + "github.com/mjl-/mox/spf" + "github.com/mjl-/mox/subjectpass" + "github.com/mjl-/mox/tlsrpt" + "github.com/mjl-/mox/updates" +) + +var metricHTTPClient = promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_httpclient_request_duration_seconds", + Help: "HTTP requests lookups.", + Buckets: []float64{0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, + }, + []string{ + "pkg", + "method", + "code", + "result", + }, +) + +// httpClientObserve tracks the result of an HTTP transaction in a metric, and +// logs the result. +func httpClientObserve(ctx context.Context, elog *slog.Logger, pkg, method string, statusCode int, err error, start time.Time) { + log := mlog.New("metrics", elog) + var result string + switch { + case err == nil: + switch statusCode / 100 { + case 2: + result = "ok" + case 4: + result = "usererror" + case 5: + result = "servererror" + default: + result = "other" + } + case errors.Is(err, os.ErrDeadlineExceeded) || errors.Is(err, context.DeadlineExceeded): + result = "timeout" + case errors.Is(err, context.Canceled): + result = "canceled" + default: + result = "error" + } + metricHTTPClient.WithLabelValues(pkg, method, result, fmt.Sprintf("%d", statusCode)).Observe(float64(time.Since(start)) / float64(time.Second)) + log.Debugx("httpclient result", err, + slog.String("pkg", pkg), + slog.String("method", method), + slog.Int("code", statusCode), + slog.Duration("duration", time.Since(start))) +} + +func init() { + dane.MetricVerify = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "mox_dane_verify_total", + Help: "Total number of DANE verification attempts, including mox_dane_verify_errors_total.", + }, + ) + dane.MetricVerifyErrors = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "mox_dane_verify_errors_total", + Help: "Total number of DANE verification failures, causing connections to fail.", + }, + ) + + dkim.MetricSign = counterVec{promauto.NewCounterVec( + prometheus.CounterOpts{ + Name: "mox_dkim_sign_total", + Help: "DKIM messages signings, label key is the type of key, rsa or ed25519.", + }, + []string{ + "key", + }, + )} + dkim.MetricVerify = histogramVec{ + promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_dkim_verify_duration_seconds", + Help: "DKIM verify, including lookup, duration and result.", + Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20}, + }, + []string{ + "algorithm", + "status", + }, + ), + } + + dmarc.MetricVerify = histogramVec{promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_dmarc_verify_duration_seconds", + Help: "DMARC verify, including lookup, duration and result.", + Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20}, + }, + []string{ + "status", + "reject", // yes/no + "use", // yes/no, if policy is used after random selection + }, + )} + dns.MetricLookup = histogramVec{ + promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_dns_lookup_duration_seconds", + Help: "DNS lookups.", + Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, + }, + []string{ + "pkg", + "type", // Lower-case Resolver method name without leading Lookup. + "result", // ok, nxdomain, temporary, timeout, canceled, error + }, + ), + } + + dnsbl.MetricLookup = histogramVec{promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_dnsbl_lookup_duration_seconds", + Help: "DNSBL lookup", + Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20}, + }, + []string{ + "zone", + "status", + }, + )} + + iprev.MetricIPRev = histogramVec{promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_iprev_lookup_total", + Help: "Number of iprev lookups.", + Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, + }, + []string{"status"}, + )} + + mtasts.MetricGet = histogramVec{promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_mtasts_get_duration_seconds", + Help: "MTA-STS get of policy, including lookup, duration and result.", + Buckets: []float64{0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20}, + }, + []string{ + "result", // ok, lookuperror, fetcherror + }, + )} + mtasts.HTTPClientObserve = httpClientObserve + + smtpclient.MetricCommands = histogramVec{promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_smtpclient_command_duration_seconds", + Help: "SMTP client command duration and result codes in seconds.", + Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30, 60, 120}, + }, + []string{ + "cmd", + "code", + "secode", + }, + )} + smtpclient.MetricTLSRequiredNoIgnored = counterVec{promauto.NewCounterVec( + prometheus.CounterOpts{ + Name: "mox_smtpclient_tlsrequiredno_ignored_total", + Help: "Connection attempts with TLS policy findings ignored due to message with TLS-Required: No header. Does not cover case where TLS certificate cannot be PKIX-verified.", + }, + []string{ + "ignored", // daneverification (no matching tlsa record) + }, + )} + smtpclient.MetricPanicInc = func() { + metrics.PanicInc(metrics.Smtpclient) + } + + spf.MetricVerify = histogramVec{promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_spf_verify_duration_seconds", + Help: "SPF verify, including lookup, duration and result.", + Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20}, + }, + []string{ + "status", + }, + )} + + subjectpass.MetricGenerate = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "mox_subjectpass_generate_total", + Help: "Number of generated subjectpass challenges.", + }, + ) + subjectpass.MetricVerify = counterVec{promauto.NewCounterVec( + prometheus.CounterOpts{ + Name: "mox_subjectpass_verify_total", + Help: "Number of subjectpass verifications.", + }, + []string{ + "result", // ok, fail + }, + )} + + tlsrpt.MetricLookup = histogramVec{promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_tlsrpt_lookup_duration_seconds", + Help: "TLSRPT lookups with result.", + Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, + }, + []string{"result"}, + )} + + updates.MetricLookup = histogramVec{promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_updates_lookup_duration_seconds", + Help: "Updates lookup with result.", + Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, + }, + []string{"result"}, + )} + updates.MetricFetchChangelog = histogramVec{promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "mox_updates_fetchchangelog_duration_seconds", + Help: "Fetch changelog with result.", + Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, + }, + []string{"result"}, + )} +} + +type counterVec struct { + *prometheus.CounterVec +} + +func (m counterVec) IncLabels(labels ...string) { + m.CounterVec.WithLabelValues(labels...).Inc() +} + +type histogramVec struct { + *prometheus.HistogramVec +} + +func (m histogramVec) ObserveLabels(v float64, labels ...string) { + m.HistogramVec.WithLabelValues(labels...).Observe(v) +} diff --git a/metrics/http.go b/metrics/http.go deleted file mode 100644 index 9fe2bf4..0000000 --- a/metrics/http.go +++ /dev/null @@ -1,65 +0,0 @@ -// Package metrics has prometheus metric variables/functions. -package metrics - -import ( - "context" - "errors" - "fmt" - "os" - "time" - - "golang.org/x/exp/slog" - - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - - "github.com/mjl-/mox/mlog" -) - -var ( - metricHTTPClient = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_httpclient_request_duration_seconds", - Help: "HTTP requests lookups.", - Buckets: []float64{0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, - }, - []string{ - "pkg", - "method", - "code", - "result", - }, - ) -) - -// HTTPClientObserve tracks the result of an HTTP transaction in a metric, and -// logs the result. -func HTTPClientObserve(ctx context.Context, log mlog.Log, pkg, method string, statusCode int, err error, start time.Time) { - log = log.WithPkg("metrics") - var result string - switch { - case err == nil: - switch statusCode / 100 { - case 2: - result = "ok" - case 4: - result = "usererror" - case 5: - result = "servererror" - default: - result = "other" - } - case errors.Is(err, os.ErrDeadlineExceeded) || errors.Is(err, context.DeadlineExceeded): - result = "timeout" - case errors.Is(err, context.Canceled): - result = "canceled" - default: - result = "error" - } - metricHTTPClient.WithLabelValues(pkg, method, result, fmt.Sprintf("%d", statusCode)).Observe(float64(time.Since(start)) / float64(time.Second)) - log.Debugx("httpclient result", err, - slog.String("pkg", pkg), - slog.String("method", method), - slog.Int("code", statusCode), - slog.Duration("duration", time.Since(start))) -} diff --git a/mox-/config.go b/mox-/config.go index 026171a..1e689d7 100644 --- a/mox-/config.go +++ b/mox-/config.go @@ -37,16 +37,20 @@ import ( "github.com/mjl-/mox/autotls" "github.com/mjl-/mox/config" + "github.com/mjl-/mox/dkim" "github.com/mjl-/mox/dns" + "github.com/mjl-/mox/message" "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/moxio" - "github.com/mjl-/mox/moxvar" "github.com/mjl-/mox/mtasts" "github.com/mjl-/mox/smtp" ) var pkglog = mlog.New("mox", nil) +// Pedantic enables stricter parsing. +var Pedantic bool + // Config paths are set early in program startup. They will point to files in // the same directory. var ( @@ -397,7 +401,16 @@ func SetConfig(c *Config) { } } - moxvar.Pedantic = c.Static.Pedantic + SetPedantic(c.Static.Pedantic) +} + +// Set pedantic in all packages. +func SetPedantic(p bool) { + dkim.Pedantic = p + dns.Pedantic = p + message.Pedantic = p + smtp.Pedantic = p + Pedantic = p } // ParseConfig parses the static config at path p. If checkOnly is true, no changes diff --git a/mox-/dkimsign.go b/mox-/dkimsign.go new file mode 100644 index 0000000..94f29b5 --- /dev/null +++ b/mox-/dkimsign.go @@ -0,0 +1,65 @@ +package mox + +import ( + "bytes" + "context" + "fmt" + "strings" + "time" + + "github.com/mjl-/mox/config" + "github.com/mjl-/mox/dkim" + "github.com/mjl-/mox/dns" + "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/smtp" +) + +// DKIMSelectors returns the selectors to use for signing. +func DKIMSelectors(dkimConf config.DKIM) []dkim.Selector { + var l []dkim.Selector + for _, sign := range dkimConf.Sign { + sel := dkimConf.Selectors[sign] + s := dkim.Selector{ + Hash: sel.HashEffective, + HeaderRelaxed: sel.Canonicalization.HeaderRelaxed, + BodyRelaxed: sel.Canonicalization.BodyRelaxed, + Headers: sel.HeadersEffective, + SealHeaders: !sel.DontSealHeaders, + Expiration: time.Duration(sel.ExpirationSeconds) * time.Second, + PrivateKey: sel.Key, + Domain: sel.Domain, + } + l = append(l, s) + } + return l +} + +// DKIMSign looks up the domain for "from", and uses its DKIM configuration to +// generate DKIM-Signature headers, for inclusion in a message. The +// DKIM-Signatur headers, are returned. If no domain was found an empty string and +// nil error is returned. +func DKIMSign(ctx context.Context, log mlog.Log, from smtp.Path, smtputf8 bool, data []byte) (string, error) { + // Add DKIM signature for domain, even if higher up than the full mail hostname. + // This helps with an assumed (because default) relaxed DKIM policy. If the DMARC + // policy happens to be strict, the signature won't help, but won't hurt either. + fd := from.IPDomain.Domain + var zerodom dns.Domain + for fd != zerodom { + confDom, ok := Conf.Domain(fd) + if !ok { + var nfd dns.Domain + _, nfd.ASCII, _ = strings.Cut(fd.ASCII, ".") + _, nfd.Unicode, _ = strings.Cut(fd.Unicode, ".") + fd = nfd + continue + } + + selectors := DKIMSelectors(confDom.DKIM) + dkimHeaders, err := dkim.Sign(ctx, log.Logger, from.Localpart, fd, selectors, smtputf8, bytes.NewReader(data)) + if err != nil { + return "", fmt.Errorf("dkim sign for domain %s: %v", fd, err) + } + return dkimHeaders, nil + } + return "", nil +} diff --git a/mox-/tlsinfo.go b/moxio/tlsinfo.go similarity index 97% rename from mox-/tlsinfo.go rename to moxio/tlsinfo.go index 5c60671..960a3e3 100644 --- a/mox-/tlsinfo.go +++ b/moxio/tlsinfo.go @@ -1,4 +1,4 @@ -package mox +package moxio import ( "crypto/tls" diff --git a/moxvar/pedantic.go b/moxvar/pedantic.go deleted file mode 100644 index fbabaff..0000000 --- a/moxvar/pedantic.go +++ /dev/null @@ -1,4 +0,0 @@ -package moxvar - -// Pendantic mode, in moxvar to prevent cyclic imports. -var Pedantic bool diff --git a/mtasts/mtasts.go b/mtasts/mtasts.go index 598457b..f60364b 100644 --- a/mtasts/mtasts.go +++ b/mtasts/mtasts.go @@ -22,28 +22,17 @@ import ( "golang.org/x/exp/slog" - "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" "github.com/mjl-/mox/moxio" + "github.com/mjl-/mox/stub" ) var ( - metricGet = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_mtasts_get_duration_seconds", - Help: "MTA-STS get of policy, including lookup, duration and result.", - Buckets: []float64{0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20}, - }, - []string{ - "result", // ok, lookuperror, fetcherror - }, - ) + MetricGet stub.HistogramVec = stub.HistogramVecIgnore{} + HTTPClientObserve func(ctx context.Context, log *slog.Logger, pkg, method string, statusCode int, err error, start time.Time) = stub.HTTPClientObserveIgnore ) // Pair is an extension key/value pair in a MTA-STS DNS record or policy. @@ -298,7 +287,7 @@ func FetchPolicy(ctx context.Context, elog *slog.Logger, domain dns.Domain) (pol // We pass along underlying TLS certificate errors. return nil, "", fmt.Errorf("%w: http get: %w", ErrPolicyFetch, err) } - metrics.HTTPClientObserve(ctx, log, "mtasts", req.Method, resp.StatusCode, err, start) + HTTPClientObserve(ctx, log.Logger, "mtasts", req.Method, resp.StatusCode, err, start) defer resp.Body.Close() if resp.StatusCode == http.StatusNotFound { return nil, "", ErrNoPolicy @@ -341,7 +330,7 @@ func Get(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, domain d start := time.Now() result := "lookuperror" defer func() { - metricGet.WithLabelValues(result).Observe(float64(time.Since(start)) / float64(time.Second)) + MetricGet.ObserveLabels(float64(time.Since(start))/float64(time.Second), result) log.Debugx("mtasts get result", err, slog.Any("domain", domain), slog.Any("record", record), diff --git a/queue/direct.go b/queue/direct.go index 796fd3e..4b3586c 100644 --- a/queue/direct.go +++ b/queue/direct.go @@ -90,14 +90,14 @@ var ( ) // todo: rename function, perhaps put some of the params in a delivery struct so we don't pass all the params all the time? -func fail(qlog mlog.Log, m Msg, backoff time.Duration, permanent bool, remoteMTA dsn.NameIP, secodeOpt, errmsg string) { +func fail(ctx context.Context, qlog mlog.Log, m Msg, backoff time.Duration, permanent bool, remoteMTA dsn.NameIP, secodeOpt, errmsg string) { // todo future: when we implement relaying, we should be able to send DSNs to non-local users. and possibly specify a null mailfrom. ../rfc/5321:1503 // todo future: when we implement relaying, and a dsn cannot be delivered, and requiretls was active, we cannot drop the message. instead deliver to local postmaster? though ../rfc/8689:383 may intend to say the dsn should be delivered without requiretls? // todo future: when we implement smtp dsn extension, parameter RET=FULL must be disregarded for messages with REQUIRETLS. ../rfc/8689:379 if permanent || m.MaxAttempts == 0 && m.Attempts >= 8 || m.MaxAttempts > 0 && m.Attempts >= m.MaxAttempts { qlog.Errorx("permanent failure delivering from queue", errors.New(errmsg)) - deliverDSNFailure(qlog, m, remoteMTA, secodeOpt, errmsg) + deliverDSNFailure(ctx, qlog, m, remoteMTA, secodeOpt, errmsg) if err := queueDelete(context.Background(), m.ID); err != nil { qlog.Errorx("deleting message from queue after permanent failure", err) @@ -117,7 +117,7 @@ func fail(qlog mlog.Log, m Msg, backoff time.Duration, permanent bool, remoteMTA qlog.Errorx("temporary failure delivering from queue, sending delayed dsn", errors.New(errmsg), slog.Duration("backoff", backoff)) retryUntil := m.LastAttempt.Add((4 + 8 + 16) * time.Hour) - deliverDSNDelay(qlog, m, remoteMTA, secodeOpt, errmsg, retryUntil) + deliverDSNDelay(ctx, qlog, m, remoteMTA, secodeOpt, errmsg, retryUntil) } else { qlog.Errorx("temporary failure delivering from queue", errors.New(errmsg), slog.Duration("backoff", backoff), slog.Time("nextattempt", m.NextAttempt)) } @@ -169,7 +169,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale recipientDomainResult.Summary.TotalFailureSessionCount++ } - fail(qlog, m, backoff, permanent, dsn.NameIP{}, "", err.Error()) + fail(ctx, qlog, m, backoff, permanent, dsn.NameIP{}, "", err.Error()) return } @@ -189,7 +189,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale } else { qlog.Infox("mtasts lookup temporary error, aborting delivery attempt", err, slog.Any("domain", origNextHop)) recipientDomainResult.Summary.TotalFailureSessionCount++ - fail(qlog, m, backoff, false, dsn.NameIP{}, "", err.Error()) + fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", err.Error()) return } } @@ -333,7 +333,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale permanent = true } - fail(qlog, m, backoff, permanent, remoteMTA, secodeOpt, errmsg) + fail(ctx, qlog, m, backoff, permanent, remoteMTA, secodeOpt, errmsg) return } @@ -527,7 +527,7 @@ func deliverHost(log mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, if m.DialedIPs == nil { m.DialedIPs = map[string][]net.IP{} } - conn, remoteIP, err = smtpclient.Dial(ctx, log.Logger, dialer, host, ips, 25, m.DialedIPs) + conn, remoteIP, err = smtpclient.Dial(ctx, log.Logger, dialer, host, ips, 25, m.DialedIPs, mox.Conf.Static.SpecifiedSMTPListenIPs) } cancel() diff --git a/queue/dsn.go b/queue/dsn.go index 886b8cc..b238dff 100644 --- a/queue/dsn.go +++ b/queue/dsn.go @@ -2,6 +2,7 @@ package queue import ( "bufio" + "context" "fmt" "os" "time" @@ -29,7 +30,7 @@ var ( ) ) -func deliverDSNFailure(log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string) { +func deliverDSNFailure(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string) { const subject = "mail delivery failed" message := fmt.Sprintf(` Delivery has failed permanently for your email to: @@ -43,10 +44,10 @@ Error during the last delivery attempt: %s `, m.Recipient().XString(m.SMTPUTF8), errmsg) - deliverDSN(log, m, remoteMTA, secodeOpt, errmsg, true, nil, subject, message) + deliverDSN(ctx, log, m, remoteMTA, secodeOpt, errmsg, true, nil, subject, message) } -func deliverDSNDelay(log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string, retryUntil time.Time) { +func deliverDSNDelay(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string, retryUntil time.Time) { // Should not happen, but doesn't hurt to prevent sending delayed delivery // notifications for DMARC reports. We don't want to waste postmaster attention. if m.IsDMARCReport { @@ -67,14 +68,14 @@ Error during the last delivery attempt: %s `, m.Recipient().XString(false), errmsg) - deliverDSN(log, m, remoteMTA, secodeOpt, errmsg, false, &retryUntil, subject, message) + deliverDSN(ctx, log, m, remoteMTA, secodeOpt, errmsg, false, &retryUntil, subject, message) } // We only queue DSNs for delivery failures for emails submitted by authenticated // users. So we are delivering to local users. ../rfc/5321:1466 // ../rfc/5321:1494 // ../rfc/7208:490 -func deliverDSN(log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string, permanent bool, retryUntil *time.Time, subject, textBody string) { +func deliverDSN(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string, permanent bool, retryUntil *time.Time, subject, textBody string) { kind := "delayed delivery" if permanent { kind = "failure" @@ -124,6 +125,7 @@ func deliverDSN(log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg str From: smtp.Path{Localpart: "postmaster", IPDomain: dns.IPDomain{Domain: mox.Conf.Static.HostnameDomain}}, To: m.Sender(), Subject: subject, + MessageID: mox.MessageIDGen(false), References: m.MessageID, TextBody: textBody, @@ -150,7 +152,7 @@ func deliverDSN(log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg str return } - msgData = append(msgData, []byte("Return-Path: <"+dsnMsg.From.XString(m.SMTPUTF8)+">\r\n")...) + msgData = append([]byte("Return-Path: <"+dsnMsg.From.XString(m.SMTPUTF8)+">\r\n"), msgData...) mailbox := "Inbox" senderAccount := m.SenderAccount diff --git a/queue/queue.go b/queue/queue.go index 5c77ded..c6e459e 100644 --- a/queue/queue.go +++ b/queue/queue.go @@ -525,6 +525,8 @@ func queueDelete(ctx context.Context, msgID int64) error { // The queue is updated, either by removing a delivered or permanently failed // message, or updating the time for the next attempt. A DSN may be sent. func deliver(log mlog.Log, resolver dns.Resolver, m Msg) { + ctx := mox.Shutdown + qlog := log.WithCid(mox.Cid()).With(slog.Any("from", m.Sender()), slog.Any("recipient", m.Recipient()), slog.Int("attempts", m.Attempts), @@ -572,7 +574,7 @@ func deliver(log mlog.Log, resolver dns.Resolver, m Msg) { transport, ok = mox.Conf.Static.Transports[m.Transport] if !ok { var remoteMTA dsn.NameIP // Zero value, will not be included in DSN. ../rfc/3464:1027 - fail(qlog, m, backoff, false, remoteMTA, "", fmt.Sprintf("cannot find transport %q", m.Transport)) + fail(ctx, qlog, m, backoff, false, remoteMTA, "", fmt.Sprintf("cannot find transport %q", m.Transport)) return } transportName = m.Transport @@ -683,10 +685,10 @@ func deliver(log mlog.Log, resolver dns.Resolver, m Msg) { if transport.Socks != nil { socksdialer, err := proxy.SOCKS5("tcp", transport.Socks.Address, nil, &net.Dialer{}) if err != nil { - fail(qlog, m, backoff, false, dsn.NameIP{}, "", fmt.Sprintf("socks dialer: %v", err)) + fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", fmt.Sprintf("socks dialer: %v", err)) return } else if d, ok := socksdialer.(smtpclient.Dialer); !ok { - fail(qlog, m, backoff, false, dsn.NameIP{}, "", "socks dialer is not a contextdialer") + fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", "socks dialer is not a contextdialer") return } else { dialer = d diff --git a/queue/submit.go b/queue/submit.go index aed3122..40ed32a 100644 --- a/queue/submit.go +++ b/queue/submit.go @@ -70,16 +70,18 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale // todo: for submission, understand SRV records, and even DANE. + ctx := mox.Shutdown + // 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.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) + fail(ctx, qlog, m, backoff, true, dsn.NameIP{}, smtp.SePol7MissingReqTLS, errmsg) return } - dialctx, dialcancel := context.WithTimeout(context.Background(), 30*time.Second) + dialctx, dialcancel := context.WithTimeout(ctx, 30*time.Second) defer dialcancel() if m.DialedIPs == nil { m.DialedIPs = map[string][]net.IP{} @@ -90,7 +92,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale if m.DialedIPs == nil { m.DialedIPs = map[string][]net.IP{} } - conn, _, err = smtpclient.Dial(dialctx, qlog.Logger, dialer, dns.IPDomain{Domain: transport.DNSHost}, ips, port, m.DialedIPs) + conn, _, err = smtpclient.Dial(dialctx, qlog.Logger, dialer, dns.IPDomain{Domain: transport.DNSHost}, ips, port, m.DialedIPs, mox.Conf.Static.SpecifiedSMTPListenIPs) } addr := net.JoinHostPort(transport.Host, fmt.Sprintf("%d", port)) var result string @@ -112,7 +114,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale } qlog.Errorx("dialing for submission", err, slog.String("remote", addr)) errmsg = fmt.Sprintf("transport %s: dialing %s for submission: %v", transportName, addr, err) - fail(qlog, m, backoff, false, dsn.NameIP{}, "", errmsg) + fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", errmsg) return } dialcancel() @@ -134,7 +136,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale // Should not happen. qlog.Error("missing smtp authentication mechanisms implementation", slog.String("mechanism", mech)) errmsg = fmt.Sprintf("transport %s: authentication mechanisms %q not implemented", transportName, mech) - fail(qlog, m, backoff, false, dsn.NameIP{}, "", errmsg) + fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", errmsg) return } } @@ -155,7 +157,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale qlog.Errorx("establishing smtp session for submission", err, slog.String("remote", addr)) errmsg = fmt.Sprintf("transport %s: establishing smtp session with %s for submission: %v", transportName, addr, err) secodeOpt = smtperr.Secode - fail(qlog, m, backoff, false, remoteMTA, secodeOpt, errmsg) + fail(ctx, qlog, m, backoff, false, remoteMTA, secodeOpt, errmsg) return } defer func() { @@ -180,7 +182,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale if err != nil { qlog.Errorx("opening message for delivery", err, slog.String("remote", addr), slog.String("path", p)) errmsg = fmt.Sprintf("transport %s: opening message file for submission: %v", transportName, err) - fail(qlog, m, backoff, false, dsn.NameIP{}, "", errmsg) + fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", errmsg) return } msgr = store.FileMsgReader(m.MsgPrefix, f) @@ -223,7 +225,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale permanent = smtperr.Permanent secodeOpt = smtperr.Secode errmsg = fmt.Sprintf("transport %s: submitting email to %s: %v", transportName, addr, err) - fail(qlog, m, backoff, permanent, remoteMTA, secodeOpt, errmsg) + fail(ctx, qlog, m, backoff, permanent, remoteMTA, secodeOpt, errmsg) return } qlog.Info("delivered from queue with transport") diff --git a/smtp/address.go b/smtp/address.go index 0d3a245..a4164c1 100644 --- a/smtp/address.go +++ b/smtp/address.go @@ -7,9 +7,11 @@ import ( "strings" "github.com/mjl-/mox/dns" - "github.com/mjl-/mox/moxvar" ) +// Pedantic enables stricter parsing. +var Pedantic bool + var ErrBadAddress = errors.New("invalid email address") // Localpart is a decoded local part of an email address, before the "@". @@ -262,7 +264,7 @@ func (p *parser) xlocalpart() Localpart { } } // In the wild, some services use large localparts for generated (bounce) addresses. - if moxvar.Pedantic && len(s) > 64 || len(s) > 128 { + if Pedantic && len(s) > 64 || len(s) > 128 { // ../rfc/5321:3486 p.xerrorf("localpart longer than 64 octets") } diff --git a/smtpclient/client.go b/smtpclient/client.go index a3ca81f..e871775 100644 --- a/smtpclient/client.go +++ b/smtpclient/client.go @@ -18,46 +18,24 @@ import ( "golang.org/x/exp/slog" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/mjl-/adns" "github.com/mjl-/mox/dane" "github.com/mjl-/mox/dns" - "github.com/mjl-/mox/metrics" "github.com/mjl-/mox/mlog" - "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/moxio" "github.com/mjl-/mox/sasl" "github.com/mjl-/mox/smtp" + "github.com/mjl-/mox/stub" "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 var ( - metricCommands = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_smtpclient_command_duration_seconds", - Help: "SMTP client command duration and result codes in seconds.", - Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30, 60, 120}, - }, - []string{ - "cmd", - "code", - "secode", - }, - ) - metricTLSRequiredNoIgnored = promauto.NewCounterVec( - prometheus.CounterOpts{ - Name: "mox_smtpclient_tlsrequiredno_ignored_total", - Help: "Connection attempts with TLS policy findings ignored due to message with TLS-Required: No header. Does not cover case where TLS certificate cannot be PKIX-verified.", - }, - []string{ - "ignored", // daneverification (no matching tlsa record) - }, - ) + MetricCommands stub.HistogramVec = stub.HistogramVecIgnore{} + MetricTLSRequiredNoIgnored stub.CounterVec = stub.CounterVecIgnore{} + MetricPanicInc = func() {} ) var ( @@ -281,7 +259,7 @@ func New(ctx context.Context, elog *slog.Logger, conn net.Conn, tlsMode TLSMode, c.firstReadAfterHandshake = true c.tlsResultAdd(1, 0, nil) c.conn = tlsconn - tlsversion, ciphersuite := mox.TLSInfo(tlsconn) + tlsversion, ciphersuite := moxio.TLSInfo(tlsconn) c.log.Debug("tls client handshake done", slog.String("tls", tlsversion), slog.String("ciphersuite", ciphersuite), @@ -334,7 +312,7 @@ func (c *Client) tlsConfig() *tls.Config { // DANE verification. // daneRecords can be non-nil and empty, that's intended. if c.daneRecords != nil { - verified, record, err := dane.Verify(c.log.Logger, c.daneRecords, cs, c.remoteHostname, c.daneMoreHostnames) + verified, record, err := dane.Verify(c.log.Logger, c.daneRecords, cs, c.remoteHostname, c.daneMoreHostnames, c.rootCAs) c.log.Debugx("dane verification", err, slog.Bool("verified", verified), slog.Any("record", record)) if verified { if c.daneVerifiedRecord != nil { @@ -355,7 +333,7 @@ func (c *Client) tlsConfig() *tls.Config { 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() + MetricTLSRequiredNoIgnored.IncLabels("daneverification") } else { // This connection will fail. daneErr = dane.ErrNoMatch @@ -547,7 +525,7 @@ func (c *Client) readecode(ecodes bool) (code int, secode, lastLine string, text c.cmds = c.cmds[1:] } } - metricCommands.WithLabelValues(cmd, fmt.Sprintf("%d", co), sec).Observe(float64(time.Since(c.cmdStart)) / float64(time.Second)) + MetricCommands.ObserveLabels(float64(time.Since(c.cmdStart))/float64(time.Second), cmd, fmt.Sprintf("%d", co), sec) c.log.Debug("smtpclient command result", slog.String("cmd", cmd), slog.Int("code", co), @@ -651,7 +629,7 @@ func (c *Client) recover(rerr *error) { } cerr, ok := x.(Error) if !ok { - metrics.PanicInc(metrics.Smtpclient) + MetricPanicInc() panic(x) } *rerr = cerr @@ -779,7 +757,7 @@ func (c *Client) hello(ctx context.Context, tlsMode TLSMode, ehloHostname dns.Do c.r = bufio.NewReader(c.tr) c.w = bufio.NewWriter(c.tw) - tlsversion, ciphersuite := mox.TLSInfo(nconn) + tlsversion, ciphersuite := moxio.TLSInfo(nconn) c.log.Debug("starttls client handshake done", slog.Any("tlsmode", tlsMode), slog.Bool("verifypkix", c.tlsVerifyPKIX), diff --git a/smtpclient/dial.go b/smtpclient/dial.go index 809b855..e56fd89 100644 --- a/smtpclient/dial.go +++ b/smtpclient/dial.go @@ -10,7 +10,6 @@ import ( "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" - "github.com/mjl-/mox/mox-" ) // DialHook can be used during tests to override the regular dialer from being used. @@ -50,10 +49,9 @@ type Dialer interface { // Dial updates dialedIPs, callers may want to save it so it can be taken into // account for future delivery attempts. // -// If we have fully specified local SMTP listener IPs, we set those for the -// outgoing connection. The admin probably configured these same IPs in SPF, but -// others possibly not. -func Dial(ctx context.Context, elog *slog.Logger, dialer Dialer, host dns.IPDomain, ips []net.IP, port int, dialedIPs map[string][]net.IP) (conn net.Conn, ip net.IP, rerr error) { +// The first matching protocol family from localIPs is set for the local side +// of the TCP connection. +func Dial(ctx context.Context, elog *slog.Logger, dialer Dialer, host dns.IPDomain, ips []net.IP, port int, dialedIPs map[string][]net.IP, localIPs []net.IP) (conn net.Conn, ip net.IP, rerr error) { log := mlog.New("smtpclient", elog) timeout := 30 * time.Second if deadline, ok := ctx.Deadline(); ok && len(ips) > 0 { @@ -66,7 +64,7 @@ func Dial(ctx context.Context, elog *slog.Logger, dialer Dialer, host dns.IPDoma addr := net.JoinHostPort(ip.String(), fmt.Sprintf("%d", port)) log.Debug("dialing host", slog.String("addr", addr)) var laddr net.Addr - for _, lip := range mox.Conf.Static.SpecifiedSMTPListenIPs { + for _, lip := range localIPs { ipIs4 := ip.To4() != nil lipIs4 := lip.To4() != nil if ipIs4 == lipIs4 { diff --git a/smtpclient/dial_test.go b/smtpclient/dial_test.go index b660bc6..1d014e9 100644 --- a/smtpclient/dial_test.go +++ b/smtpclient/dial_test.go @@ -41,7 +41,7 @@ func TestDialHost(t *testing.T) { if err != nil || !reflect.DeepEqual(ips, []net.IP{net.ParseIP("10.0.0.1"), net.ParseIP("2001:db8::1")}) || !dualstack { t.Fatalf("expected err nil, address 10.0.0.1,2001:db8::1, dualstack true, got %v %v %v", err, ips, dualstack) } - _, ip, err := Dial(ctxbg, log.Logger, nil, ipdomain("dualstack.example"), ips, 25, dialedIPs) + _, ip, err := Dial(ctxbg, log.Logger, nil, ipdomain("dualstack.example"), ips, 25, dialedIPs, nil) if err != nil || ip.String() != "10.0.0.1" { t.Fatalf("expected err nil, address 10.0.0.1, dualstack true, got %v %v %v", err, ip, dualstack) } @@ -50,7 +50,7 @@ func TestDialHost(t *testing.T) { if err != nil || !reflect.DeepEqual(ips, []net.IP{net.ParseIP("2001:db8::1"), net.ParseIP("10.0.0.1")}) || !dualstack { t.Fatalf("expected err nil, address 2001:db8::1,10.0.0.1, dualstack true, got %v %v %v", err, ips, dualstack) } - _, ip, err = Dial(ctxbg, log.Logger, nil, ipdomain("dualstack.example"), ips, 25, dialedIPs) + _, ip, err = Dial(ctxbg, log.Logger, nil, ipdomain("dualstack.example"), ips, 25, dialedIPs, nil) if err != nil || ip.String() != "2001:db8::1" { t.Fatalf("expected err nil, address 2001:db8::1, dualstack true, got %v %v %v", err, ip, dualstack) } diff --git a/smtpserver/dsn.go b/smtpserver/dsn.go index 98fee1d..3dd798a 100644 --- a/smtpserver/dsn.go +++ b/smtpserver/dsn.go @@ -5,22 +5,32 @@ import ( "fmt" "github.com/mjl-/mox/dsn" + "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/queue" "github.com/mjl-/mox/smtp" "github.com/mjl-/mox/store" ) // compose dsn message and add it to the queue for delivery to rcptTo. -func queueDSN(ctx context.Context, c *conn, rcptTo smtp.Path, m dsn.Message, requireTLS bool) error { +func queueDSN(ctx context.Context, log mlog.Log, c *conn, rcptTo smtp.Path, m dsn.Message, requireTLS bool) error { buf, err := m.Compose(c.log, false) if err != nil { return err } + bufDKIM, err := mox.DKIMSign(ctx, c.log, m.From, false, buf) + log.Check(err, "dkim signing dsn") + buf = append([]byte(bufDKIM), buf...) + var bufUTF8 []byte if c.smtputf8 { bufUTF8, err = m.Compose(c.log, true) if err != nil { c.log.Errorx("composing dsn with utf-8 for incoming delivery for unknown user, continuing with ascii-only dsn", err) + } else { + bufUTF8DKIM, err := mox.DKIMSign(ctx, log, m.From, true, bufUTF8) + log.Check(err, "dkim signing dsn with utf8") + bufUTF8 = append([]byte(bufUTF8DKIM), bufUTF8...) } } diff --git a/smtpserver/parse.go b/smtpserver/parse.go index 932ecee..c076471 100644 --- a/smtpserver/parse.go +++ b/smtpserver/parse.go @@ -8,7 +8,7 @@ import ( "strings" "github.com/mjl-/mox/dns" - "github.com/mjl-/mox/moxvar" + "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/smtp" ) @@ -308,7 +308,7 @@ func (p *parser) xipdomain(isehlo bool) dns.IPDomain { // Mail user agents that submit are relatively likely to use IPs in EHLO and forget // that an IPv6 address needs to be tagged as such. We can forgive them. For // SMTP servers we are strict. - return isehlo && p.conn.submission && !moxvar.Pedantic && ip.To16() != nil + return isehlo && p.conn.submission && !mox.Pedantic && ip.To16() != nil } if ipv6 && isv4 { p.xerrorf("ip address is not ipv6") @@ -337,7 +337,7 @@ func (p *parser) xlocalpart() smtp.Localpart { } } // In the wild, some services use large localparts for generated (bounce) addresses. - if moxvar.Pedantic && len(s) > 64 || len(s) > 128 { + if mox.Pedantic && len(s) > 64 || len(s) > 128 { // ../rfc/5321:3486 p.xerrorf("localpart longer than 64 octets") } diff --git a/smtpserver/server.go b/smtpserver/server.go index 15b05dd..5b844d2 100644 --- a/smtpserver/server.go +++ b/smtpserver/server.go @@ -804,7 +804,7 @@ func (c *conn) cmdEhlo(p *parser) { // ../rfc/5321:1783 func (c *conn) cmdHello(p *parser, ehlo bool) { var remote dns.IPDomain - if c.submission && !moxvar.Pedantic { + if c.submission && !mox.Pedantic { // Mail clients regularly put bogus information in the hostname/ip. For submission, // the value is of no use, so there is not much point in annoying the user with // errors they cannot fix themselves. Except when in pedantic mode. @@ -907,7 +907,7 @@ func (c *conn) cmdStarttls(p *parser) { panic(fmt.Errorf("starttls handshake: %s (%w)", err, errIO)) } cancel() - tlsversion, ciphersuite := mox.TLSInfo(tlsConn) + tlsversion, ciphersuite := moxio.TLSInfo(tlsConn) c.log.Debug("tls server handshake done", slog.String("tls", tlsversion), slog.String("ciphersuite", ciphersuite)) c.conn = tlsConn c.tr = moxio.NewTraceReader(c.log, "RC: ", c) @@ -982,7 +982,7 @@ func (c *conn) cmdAuth(p *parser) { } } else { p.xspace() - if !moxvar.Pedantic { + if !mox.Pedantic { // Windows Mail 16005.14326.21606.0 sends two spaces between "AUTH PLAIN" and the // base64 data. for p.space() { @@ -1304,7 +1304,7 @@ func (c *conn) cmdMail(p *parser) { // note: no space allowed after colon. ../rfc/5321:1093 // Microsoft Outlook 365 Apps for Enterprise sends it with submission. For delivery // it is mostly used by spammers, but has been seen with legitimate senders too. - if !moxvar.Pedantic { + if !mox.Pedantic { p.space() } rawRevPath := p.xrawReversePath() @@ -1445,7 +1445,7 @@ func (c *conn) cmdRcpt(p *parser) { // note: no space allowed after colon. ../rfc/5321:1093 // Microsoft Outlook 365 Apps for Enterprise sends it with submission. For delivery // it is mostly used by spammers, but has been seen with legitimate senders too. - if !moxvar.Pedantic { + if !mox.Pedantic { p.space() } var fpath smtp.Path @@ -1639,13 +1639,13 @@ func (c *conn) cmdData(p *parser) { } // Check only for pedantic mode because ios mail will attempt to send smtputf8 with // non-ascii in message from localpart without using 8bitmime. - if moxvar.Pedantic && msgWriter.Has8bit && !c.has8bitmime { + if mox.Pedantic && msgWriter.Has8bit && !c.has8bitmime { // ../rfc/5321:906 xsmtpUserErrorf(smtp.C500BadSyntax, smtp.SeMsg6Other0, "message with non-us-ascii requires 8bitmime extension") } } - if Localserve && moxvar.Pedantic { + if Localserve && mox.Pedantic { // Require that message can be parsed fully. p, err := message.Parse(c.log.Logger, false, dataFile) if err == nil { @@ -1855,11 +1855,11 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr xsmtpServerErrorf(codes{smtp.C451LocalErr, smtp.SeSys3Other0}, "internal error") } - dkimConfig := confDom.DKIM - if len(dkimConfig.Sign) > 0 { + selectors := mox.DKIMSelectors(confDom.DKIM) + if len(selectors) > 0 { if canonical, err := mox.CanonicalLocalpart(msgFrom.Localpart, confDom); err != nil { c.log.Errorx("determining canonical localpart for dkim signing", err, slog.Any("localpart", msgFrom.Localpart)) - } else if dkimHeaders, err := dkim.Sign(ctx, c.log.Logger, canonical, msgFrom.Domain, dkimConfig, c.smtputf8, store.FileMsgReader(msgPrefix, dataFile)); err != nil { + } else if dkimHeaders, err := dkim.Sign(ctx, c.log.Logger, canonical, msgFrom.Domain, selectors, c.smtputf8, store.FileMsgReader(msgPrefix, dataFile)); err != nil { c.log.Errorx("dkim sign for domain", err, slog.Any("domain", msgFrom.Domain)) metricServerErrors.WithLabelValues("dkimsign").Inc() } else { @@ -2841,6 +2841,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW From: smtp.Path{Localpart: "postmaster", IPDomain: deliverErrors[0].rcptTo.IPDomain}, To: *c.mailFrom, Subject: "mail delivery failure", + MessageID: mox.MessageIDGen(false), References: messageID, // Per-message details. @@ -2876,7 +2877,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW if Localserve { c.log.Error("not queueing dsn for incoming delivery due to localserve") - } else if err := queueDSN(context.TODO(), c, *c.mailFrom, dsnMsg, c.requireTLS != nil && *c.requireTLS); err != nil { + } else if err := queueDSN(context.TODO(), c.log, c, *c.mailFrom, dsnMsg, c.requireTLS != nil && *c.requireTLS); err != nil { metricServerErrors.WithLabelValues("queuedsn").Inc() c.log.Errorx("queuing DSN for incoming delivery, no DSN sent", err) } diff --git a/smtpserver/server_test.go b/smtpserver/server_test.go index 5ec59fd..4e60e84 100644 --- a/smtpserver/server_test.go +++ b/smtpserver/server_test.go @@ -1008,7 +1008,8 @@ func TestTLSReport(t *testing.T) { tcheck(t, xerr, "write msg") msg := msgb.String() - headers, xerr := dkim.Sign(ctxbg, pkglog.Logger, "remote", dns.Domain{ASCII: "example.org"}, dkimConf, false, strings.NewReader(msg)) + selectors := mox.DKIMSelectors(dkimConf) + headers, xerr := dkim.Sign(ctxbg, pkglog.Logger, "remote", dns.Domain{ASCII: "example.org"}, selectors, false, strings.NewReader(msg)) tcheck(t, xerr, "dkim sign") msg = headers + msg diff --git a/spf/spf.go b/spf/spf.go index 49784d4..d62d556 100644 --- a/spf/spf.go +++ b/spf/spf.go @@ -18,12 +18,10 @@ import ( "golang.org/x/exp/slog" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/smtp" + "github.com/mjl-/mox/stub" ) // The net package always returns DNS names in absolute, lower-case form. We make @@ -31,16 +29,7 @@ import ( // verify names relative to our local search domain. var ( - metricSPFVerify = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_spf_verify_duration_seconds", - Help: "SPF verify, including lookup, duration and result.", - Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20}, - }, - []string{ - "status", - }, - ) + MetricVerify stub.HistogramVec = stub.HistogramVecIgnore{} ) // cross-link rfc and errata @@ -202,7 +191,7 @@ func Verify(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, args log := mlog.New("spf", elog) start := time.Now() defer func() { - metricSPFVerify.WithLabelValues(string(received.Result)).Observe(float64(time.Since(start)) / float64(time.Second)) + MetricVerify.ObserveLabels(float64(time.Since(start))/float64(time.Second), string(received.Result)) log.Debugx("spf verify result", rerr, slog.Any("domain", args.domain), slog.Any("ip", args.RemoteIP), diff --git a/store/account.go b/store/account.go index c06b2eb..e8c2173 100644 --- a/store/account.go +++ b/store/account.go @@ -56,7 +56,6 @@ import ( "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/moxio" - "github.com/mjl-/mox/moxvar" "github.com/mjl-/mox/publicsuffix" "github.com/mjl-/mox/scram" "github.com/mjl-/mox/smtp" @@ -2086,7 +2085,7 @@ func ParseFlagsKeywords(l []string) (flags Flags, keywords []string, rerr error) if field, ok := fields[f]; ok { *field = true } else if seen[f] { - if moxvar.Pedantic { + if mox.Pedantic { return Flags{}, nil, fmt.Errorf("duplicate keyword %s", f) } } else { diff --git a/stub/doc.go b/stub/doc.go new file mode 100644 index 0000000..c249c2c --- /dev/null +++ b/stub/doc.go @@ -0,0 +1,7 @@ +// Package stub provides interfaces and stub implementations. +// +// Packages in mox use these interfaces and implementations so other software +// reusing these packages won't have to take on unwanted dependencies. +// +// Stubs are provided for: metrics (prometheus). +package stub diff --git a/stub/metrics.go b/stub/metrics.go new file mode 100644 index 0000000..e3e16a2 --- /dev/null +++ b/stub/metrics.go @@ -0,0 +1,43 @@ +package stub + +import ( + "context" + "time" + + "golang.org/x/exp/slog" +) + +func HTTPClientObserveIgnore(ctx context.Context, log *slog.Logger, pkg, method string, statusCode int, err error, start time.Time) { +} + +type Counter interface { + Inc() +} + +type CounterIgnore struct{} + +func (CounterIgnore) Inc() {} + +type CounterVec interface { + IncLabels(labels ...string) +} + +type CounterVecIgnore struct{} + +func (CounterVecIgnore) IncLabels(labels ...string) {} + +type Histogram interface { + Observe(float64) +} + +type HistogramIgnore struct{} + +func (HistogramIgnore) Observe(float64) {} + +type HistogramVec interface { + ObserveLabels(v float64, labels ...string) +} + +type HistogramVecIgnore struct{} + +func (HistogramVecIgnore) ObserveLabels(v float64, labels ...string) {} diff --git a/subjectpass/subjectpass.go b/subjectpass/subjectpass.go index 208c79a..88d4688 100644 --- a/subjectpass/subjectpass.go +++ b/subjectpass/subjectpass.go @@ -13,31 +13,16 @@ import ( "golang.org/x/exp/slog" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/mjl-/mox/dns" "github.com/mjl-/mox/message" "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/smtp" + "github.com/mjl-/mox/stub" ) var ( - metricGenerate = promauto.NewCounter( - prometheus.CounterOpts{ - Name: "mox_subjectpass_generate_total", - Help: "Number of generated subjectpass challenges.", - }, - ) - metricVerify = promauto.NewCounterVec( - prometheus.CounterOpts{ - Name: "mox_subjectpass_verify_total", - Help: "Number of subjectpass verifications.", - }, - []string{ - "result", // ok, fail - }, - ) + MetricGenerate stub.Counter = stub.CounterIgnore{} + MetricVerify stub.CounterVec = stub.CounterVecIgnore{} ) var ( @@ -57,7 +42,7 @@ var Explanation = "Your message resembles spam. If your email is legitimate, ple func Generate(elog *slog.Logger, mailFrom smtp.Address, key []byte, tm time.Time) string { log := mlog.New("subjectpass", elog) - metricGenerate.Inc() + MetricGenerate.Inc() log.Debug("subjectpass generate", slog.Any("mailfrom", mailFrom)) // We discard the lower 8 bits of the time, we can do with less precision. @@ -88,7 +73,7 @@ func Verify(elog *slog.Logger, r io.ReaderAt, key []byte, period time.Duration) if rerr == nil { result = "ok" } - metricVerify.WithLabelValues(result).Inc() + MetricVerify.IncLabels(result) log.Debugx("subjectpass verify result", rerr, slog.String("token", token), slog.Duration("period", period)) }() diff --git a/tlsrpt/lookup.go b/tlsrpt/lookup.go index db5e2ce..3c255b9 100644 --- a/tlsrpt/lookup.go +++ b/tlsrpt/lookup.go @@ -8,22 +8,13 @@ import ( "golang.org/x/exp/slog" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/mjl-/mox/dns" "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/stub" ) var ( - metricLookup = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_tlsrpt_lookup_duration_seconds", - Help: "TLSRPT lookups with result.", - Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, - }, - []string{"result"}, - ) + MetricLookup stub.HistogramVec = stub.HistogramVecIgnore{} ) var ( @@ -53,7 +44,7 @@ func Lookup(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, domai result = "error" } } - metricLookup.WithLabelValues(result).Observe(float64(time.Since(start)) / float64(time.Second)) + MetricLookup.ObserveLabels(float64(time.Since(start))/float64(time.Second), result) log.Debugx("tlsrpt lookup result", rerr, slog.Any("domain", domain), slog.Any("record", rrecord), diff --git a/tlsrptsend/send.go b/tlsrptsend/send.go index 378b7de..47bdf92 100644 --- a/tlsrptsend/send.go +++ b/tlsrptsend/send.go @@ -688,15 +688,14 @@ func composeMessage(ctx context.Context, log mlog.Log, mf *os.File, policyDomain xc.Flush() - selectors := map[string]config.Selector{} - for name, sel := range confDKIM.Selectors { + selectors := mox.DKIMSelectors(confDKIM) + for i, sel := range selectors { // Also sign the TLS-Report headers. ../rfc/8460:940 - sel.HeadersEffective = append(append([]string{}, sel.HeadersEffective...), "TLS-Report-Domain", "TLS-Report-Submitter") - selectors[name] = sel + sel.Headers = append(append([]string{}, sel.Headers...), "TLS-Report-Domain", "TLS-Report-Submitter") + selectors[i] = sel } - confDKIM.Selectors = selectors - dkimHeader, err := dkim.Sign(ctx, log.Logger, fromAddr.Localpart, fromAddr.Domain, confDKIM, smtputf8, mf) + dkimHeader, err := dkim.Sign(ctx, log.Logger, fromAddr.Localpart, fromAddr.Domain, selectors, smtputf8, mf) xc.Checkf(err, "dkim-signing report message") return dkimHeader, xc.Has8bit, xc.SMTPUTF8, messageID, nil diff --git a/updates/updates.go b/updates/updates.go index e837e29..0abe607 100644 --- a/updates/updates.go +++ b/updates/updates.go @@ -23,32 +23,16 @@ import ( "golang.org/x/exp/slog" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/mjl-/mox/dns" - "github.com/mjl-/mox/metrics" "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/moxio" + "github.com/mjl-/mox/stub" ) var ( - metricLookup = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_updates_lookup_duration_seconds", - Help: "Updates lookup with result.", - Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, - }, - []string{"result"}, - ) - metricFetchChangelog = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "mox_updates_fetchchangelog_duration_seconds", - Help: "Fetch changelog with result.", - Buckets: []float64{0.001, 0.005, 0.01, 0.05, 0.100, 0.5, 1, 5, 10, 20, 30}, - }, - []string{"result"}, - ) + MetricLookup stub.HistogramVec = stub.HistogramVecIgnore{} + MetricFetchChangelog stub.HistogramVec = stub.HistogramVecIgnore{} + HTTPClientObserve func(ctx context.Context, log *slog.Logger, pkg, method string, statusCode int, err error, start time.Time) = stub.HTTPClientObserveIgnore ) var ( @@ -89,7 +73,7 @@ func Lookup(ctx context.Context, elog *slog.Logger, resolver dns.Resolver, domai if rerr != nil { result = "error" } - metricLookup.WithLabelValues(result).Observe(float64(time.Since(start)) / float64(time.Second)) + MetricLookup.ObserveLabels(float64(time.Since(start))/float64(time.Second), result) log.Debugx("updates lookup result", rerr, slog.Any("domain", domain), slog.Any("version", rversion), @@ -144,7 +128,7 @@ func FetchChangelog(ctx context.Context, elog *slog.Logger, baseURL string, base if rerr != nil { result = "error" } - metricFetchChangelog.WithLabelValues(result).Observe(float64(time.Since(start)) / float64(time.Second)) + MetricFetchChangelog.ObserveLabels(float64(time.Since(start))/float64(time.Second), result) log.Debugx("updates fetch changelog result", rerr, slog.String("baseurl", baseURL), slog.Any("base", base), @@ -163,7 +147,7 @@ func FetchChangelog(ctx context.Context, elog *slog.Logger, baseURL string, base if resp == nil { resp = &http.Response{StatusCode: 0} } - metrics.HTTPClientObserve(ctx, log, "updates", req.Method, resp.StatusCode, err, start) + HTTPClientObserve(ctx, log.Logger, "updates", req.Method, resp.StatusCode, err, start) if err != nil { return nil, fmt.Errorf("%w: making http request: %s", ErrChangelogFetch, err) } diff --git a/webmail/api.go b/webmail/api.go index fe81eed..f59ec19 100644 --- a/webmail/api.go +++ b/webmail/api.go @@ -557,8 +557,9 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { var msgPrefix string fd := fromAddr.Address.Domain confDom, _ := mox.Conf.Domain(fd) - if len(confDom.DKIM.Sign) > 0 { - dkimHeaders, err := dkim.Sign(ctx, log.Logger, fromAddr.Address.Localpart, fd, confDom.DKIM, smtputf8, dataFile) + selectors := mox.DKIMSelectors(confDom.DKIM) + if len(selectors) > 0 { + dkimHeaders, err := dkim.Sign(ctx, log.Logger, fromAddr.Address.Localpart, fd, selectors, smtputf8, dataFile) if err != nil { metricServerErrors.WithLabelValues("dkimsign").Inc() } diff --git a/webmail/message.go b/webmail/message.go index 76f7a3b..0a5aa01 100644 --- a/webmail/message.go +++ b/webmail/message.go @@ -13,8 +13,8 @@ import ( "github.com/mjl-/mox/dns" "github.com/mjl-/mox/message" "github.com/mjl-/mox/mlog" + "github.com/mjl-/mox/mox-" "github.com/mjl-/mox/moxio" - "github.com/mjl-/mox/moxvar" "github.com/mjl-/mox/smtp" "github.com/mjl-/mox/store" ) @@ -39,7 +39,7 @@ func tryDecodeParam(log mlog.Log, name string) string { return name } // todo: find where this is allowed. it seems quite common. perhaps we should remove the pedantic check? - if moxvar.Pedantic { + if mox.Pedantic { log.Debug("attachment contains rfc2047 q/b-word-encoded mime parameter instead of rfc2231-encoded", slog.String("name", name)) return name }