Implement support for DNSBL lookups

Currently lacks whitelisting support and return codes filtering.
Both should be implemented in the future.

README.md

@@ -26,6 +26,7 @@ changes happen from time to time**
   - Single process model allows more efficient implementation
 * Useful
   - [Subaddressing][subaddr] support 
+  - [DNSBL][dnsbl] checking support
   - Messages compression (LZ4, Zstd)
  
 Planned:
@@ -73,6 +74,7 @@ The code is under MIT license. See [LICENSE](LICENSE) for more information.
 [dmarc]: https://blog.returnpath.com/how-to-explain-dmarc-in-plain-english/
 [mtasts]: https://www.hardenize.com/blog/mta-sts
 [subaddr]: https://en.wikipedia.org/wiki/Email_address#Sub-addressing
+[dnsbl]: https://en.wikipedia.org/wiki/DNSBL
 [backscatter]: https://en.wikipedia.org/wiki/Backscatter_(e-mail)
 
 [setup-tutorial]: https://github.com/foxcpp/maddy/wiki/Tutorial:-Setting-up-a-mail-server-with-maddy

check/dnsbl/common.go

@@ -0,0 +1,181 @@
+package dnsbl
+
+import (
+	"context"
+	"net"
+	"strconv"
+	"strings"
+
+	"github.com/foxcpp/maddy/dns"
+	"github.com/foxcpp/maddy/exterrors"
+)
+
+type ListedErr struct {
+	Identity string
+	DNSBL    string
+	Reason   string
+}
+
+func (le ListedErr) Fields() map[string]interface{} {
+	return map[string]interface{}{
+		"check":         "dnsbl",
+		"dnsbl":         le.DNSBL,
+		"identity":      le.Identity,
+		"reason":        le.Reason,
+		"smtp_code":     554,
+		"smtp_enchcode": exterrors.EnhancedCode{5, 7, 0},
+		"smtp_msg":      le.Identity + " is listed in the used DNSBL",
+	}
+}
+
+func (le ListedErr) Error() string {
+	return le.Identity + " is listed in the used DNSBL"
+}
+
+func checkDomain(resolver dns.Resolver, cfg BL, domain string) error {
+	query := domain + "." + cfg.Zone
+
+	addrs, err := resolver.LookupHost(context.Background(), query)
+	if err != nil {
+		if dnsErr, ok := err.(*net.DNSError); ok && dnsErr.IsNotFound {
+			return nil
+		}
+
+		return err
+	}
+
+	if len(addrs) == 0 {
+		return nil
+	}
+
+	// Attempt to extract explaination string.
+	txts, err := resolver.LookupTXT(context.Background(), query)
+	if err != nil || len(txts) == 0 {
+		// Not significant, include addresses as reason. Usually they are
+		// mapped to some predefined 'reasons' by BL.
+		return ListedErr{
+			Identity: domain,
+			DNSBL:    cfg.Zone,
+			Reason:   strings.Join(addrs, "; "),
+		}
+	}
+
+	// Some BLs provide multiple reasons (meta-BLs such as Spamhaus Zen) so
+	// don't mangle them by joining with "", instead join with "; ".
+
+	return ListedErr{
+		Identity: domain,
+		DNSBL:    cfg.Zone,
+		Reason:   strings.Join(txts, "; "),
+	}
+}
+
+func checkIP(resolver dns.Resolver, cfg BL, ip net.IP) error {
+	ipv6 := true
+	if ipv4 := ip.To4(); ipv4 != nil {
+		ip = ipv4
+		ipv6 = false
+	}
+
+	if ipv6 && !cfg.ClientIPv6 {
+		return nil
+	}
+	if !ipv6 && !cfg.ClientIPv4 {
+		return nil
+	}
+
+	query := queryString(ip) + "." + cfg.Zone
+
+	addrs, err := resolver.LookupHost(context.Background(), query)
+	if err != nil {
+		if dnsErr, ok := err.(*net.DNSError); ok && dnsErr.IsNotFound {
+			return nil
+		}
+
+		return err
+	}
+
+	if len(addrs) == 0 {
+		return nil
+	}
+
+	// Attempt to extract explaination string.
+	txts, err := resolver.LookupTXT(context.Background(), query)
+	if err != nil || len(txts) == 0 {
+		// Not significant, include addresses as reason. Usually they are
+		// mapped to some predefined 'reasons' by BL.
+		return ListedErr{
+			Identity: ip.String(),
+			DNSBL:    cfg.Zone,
+			Reason:   strings.Join(addrs, "; "),
+		}
+	}
+
+	// Some BLs provide multiple reasons (meta-BLs such as Spamhaus Zen) so
+	// don't mangle them by joining with "", instead join with "; ".
+
+	return ListedErr{
+		Identity: ip.String(),
+		DNSBL:    cfg.Zone,
+		Reason:   strings.Join(txts, "; "),
+	}
+}
+
+func queryString(ip net.IP) string {
+	ipv6 := true
+	if ipv4 := ip.To4(); ipv4 != nil {
+		ip = ipv4
+		ipv6 = false
+	}
+
+	res := strings.Builder{}
+	if ipv6 {
+		res.Grow(63) // 0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0
+	} else {
+		res.Grow(15) // 000.000.000.000
+	}
+
+	for i := len(ip) - 1; i >= 0; i-- {
+		octet := ip[i]
+
+		if ipv6 {
+			// X.X
+			res.WriteString(strconv.FormatInt(int64(octet&0xf), 16))
+			res.WriteRune('.')
+			res.WriteString(strconv.FormatInt(int64((octet&0xf0)>>4), 16))
+		} else {
+			// X
+			res.WriteString(strconv.Itoa(int(octet)))
+		}
+
+		if i != 0 {
+			res.WriteRune('.')
+		}
+	}
+	return res.String()
+}
+
+// mangleErr adds smtp_* fields to DNSBL check error that mask
+// details about used DNSBL.
+func mangleErr(err error) error {
+	_, ok := err.(ListedErr)
+	if ok {
+		// ListenErr is already safe due to smtp_* fields.
+		return err
+	}
+
+	smtpCode := 554
+	smtpEnchCode := exterrors.EnhancedCode{5, 7, 0}
+	if exterrors.IsTemporary(err) {
+		smtpCode = 451
+		smtpEnchCode = exterrors.EnhancedCode{4, 7, 0}
+	}
+
+	return exterrors.WithFields(err, map[string]interface{}{
+		"check":         "dnsbl",
+		"reason":        err.Error(),
+		"smtp_code":     smtpCode,
+		"smtp_enchcode": smtpEnchCode,
+		"smtp_msg":      "Internal error during policy check",
+	})
+}

check/dnsbl/common_test.go

@@ -0,0 +1,28 @@
+package dnsbl
+
+import (
+	"net"
+	"testing"
+)
+
+// TODO: Tests for checkIP and checkDomain once we have proper DNS mocking.
+
+func TestQueryString(t *testing.T) {
+	test := func(ip, queryStr string) {
+		t.Helper()
+
+		parsed := net.ParseIP(ip)
+		if parsed == nil {
+			panic("Malformed IP in test")
+		}
+
+		actual := queryString(parsed)
+		if actual != queryStr {
+			t.Errorf("want queryString(%s) to be %s, got %s", ip, queryStr, actual)
+		}
+	}
+
+	test("2001:db8:1:2:3:4:567:89ab", "b.a.9.8.7.6.5.0.4.0.0.0.3.0.0.0.2.0.0.0.1.0.0.0.8.b.d.0.1.0.0.2")
+	test("2001::1:2:3:4:567:89ab", "b.a.9.8.7.6.5.0.4.0.0.0.3.0.0.0.2.0.0.0.1.0.0.0.0.0.0.0.1.0.0.2")
+	test("192.0.2.99", "99.2.0.192")
+}

check/dnsbl/dnsbl.go

@@ -0,0 +1,316 @@
+package dnsbl
+
+import (
+	"net"
+	"strings"
+
+	"github.com/emersion/go-message/textproto"
+	"github.com/emersion/go-smtp"
+	"github.com/foxcpp/maddy/address"
+	"github.com/foxcpp/maddy/buffer"
+	"github.com/foxcpp/maddy/check"
+	"github.com/foxcpp/maddy/config"
+	"github.com/foxcpp/maddy/dns"
+	"github.com/foxcpp/maddy/log"
+	"github.com/foxcpp/maddy/module"
+	"github.com/foxcpp/maddy/target"
+	"golang.org/x/sync/errgroup"
+)
+
+type BL struct {
+	Zone string
+
+	ClientIPv4 bool
+	ClientIPv6 bool
+
+	EHLO     bool
+	MAILFROM bool
+}
+
+var defaultBL = BL{
+	ClientIPv4: true,
+}
+
+type DNSBL struct {
+	instName     string
+	checkEarly   bool
+	listedAction check.FailAction
+	inlineBls    []string
+	bls          []BL
+
+	resolver dns.Resolver
+	log      log.Logger
+}
+
+func NewDNSBL(_, instName string, _, inlineArgs []string) (module.Module, error) {
+	return &DNSBL{
+		instName:  instName,
+		inlineBls: inlineArgs,
+
+		resolver: net.DefaultResolver,
+		log:      log.Logger{Name: "dnsbl"},
+	}, nil
+}
+
+func (bl *DNSBL) Name() string {
+	return "dnsbl"
+}
+
+func (bl *DNSBL) InstanceName() string {
+	return bl.instName
+}
+
+func (bl *DNSBL) Init(cfg *config.Map) error {
+	cfg.Bool("debug", false, false, &bl.log.Debug)
+	cfg.Bool("check_early", false, false, &bl.checkEarly)
+	cfg.Custom("listed_action", false, false,
+		func() (interface{}, error) {
+			return check.FailAction{Reject: true}, nil
+		}, check.FailActionDirective, &bl.listedAction)
+	cfg.AllowUnknown()
+	unmatched, err := cfg.Process()
+	if err != nil {
+		return err
+	}
+
+	for _, inlineBl := range bl.inlineBls {
+		cfg := defaultBL
+		cfg.Zone = inlineBl
+		go bl.testBL(cfg)
+		bl.bls = append(bl.bls, cfg)
+	}
+
+	for _, node := range unmatched {
+		if err := bl.readBLCfg(node); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (bl *DNSBL) readBLCfg(node config.Node) error {
+	var blCfg BL
+
+	cfg := config.NewMap(nil, &node)
+	cfg.Bool("client_ipv4", false, defaultBL.ClientIPv4, &blCfg.ClientIPv4)
+	cfg.Bool("client_ipv6", false, defaultBL.ClientIPv4, &blCfg.ClientIPv6)
+	cfg.Bool("ehlo", false, defaultBL.EHLO, &blCfg.EHLO)
+	cfg.Bool("mailfrom", false, defaultBL.EHLO, &blCfg.MAILFROM)
+	if _, err := cfg.Process(); err != nil {
+		return err
+	}
+
+	for _, zone := range append([]string{node.Name}, node.Args...) {
+		// From RFC 5782 Section 7:
+		// >To avoid this situation, systems that use
+		// >DNSxLs SHOULD check for the test entries described in Section 5 to
+		// >ensure that a domain actually has the structure of a DNSxL, and
+		// >SHOULD NOT use any DNSxL domain that does not have correct test
+		// >entries.
+		// Sadly, however, many DNSBLs lack test records so at most we can
+		// log a warning. Also, DNS is kinda slow so we do checks
+		// asynchronously to prevent slowing down server start-up.
+
+		zoneCfg := blCfg
+		zoneCfg.Zone = zone
+		go bl.testBL(zoneCfg)
+		bl.bls = append(bl.bls, zoneCfg)
+	}
+
+	return nil
+}
+
+func (bl *DNSBL) testBL(listCfg BL) {
+	// Check RFC 5782 Section 5 requirements.
+
+	bl.log.DebugMsg("testing BL for RFC 5782 requirements...", "dnsbl", listCfg.Zone)
+
+	// 1. IPv4-based DNSxLs MUST contain an entry for 127.0.0.2 for testing purposes.
+	if listCfg.ClientIPv4 {
+		err := checkIP(bl.resolver, listCfg, net.IPv4(127, 0, 0, 2))
+		if err == nil {
+			bl.log.Msg("BL does not contain a test record for 127.0.0.2", "dnsbl", listCfg.Zone)
+		} else if _, ok := err.(ListedErr); !ok {
+			bl.log.Error("lookup error, bailing out", err, "dnsbl", listCfg.Zone)
+			return
+		}
+
+		// 2. IPv4-based DNSxLs MUST NOT contain an entry for 127.0.0.1.
+		err = checkIP(bl.resolver, listCfg, net.IPv4(127, 0, 0, 1))
+		if err != nil {
+			_, ok := err.(ListedErr)
+			if !ok {
+				bl.log.Error("lookup error, bailing out", err, "dnsbl", listCfg.Zone)
+				return
+			}
+			bl.log.Msg("BL contains a record for 127.0.0.1", "dnsbl", listCfg.Zone)
+		}
+	}
+
+	if listCfg.ClientIPv6 {
+		// 1. IPv6-based DNSxLs MUST contain an entry for ::FFFF:7F00:2
+		mustIP := net.ParseIP("::FFFF:7F00:2")
+
+		err := checkIP(bl.resolver, listCfg, mustIP)
+		if err == nil {
+			bl.log.Msg("BL does not contain a test record for ::FFFF:7F00:2", "dnsbl", listCfg.Zone)
+		} else if _, ok := err.(ListedErr); !ok {
+			bl.log.Error("lookup error, bailing out", err, "dnsbl", listCfg.Zone)
+			return
+		}
+
+		// 2. IPv4-based DNSxLs MUST NOT contain an entry for 127.0.0.1.
+		mustNotIP := net.ParseIP("::FFFF:7F00:1")
+		err = checkIP(bl.resolver, listCfg, mustNotIP)
+		if err != nil {
+			_, ok := err.(ListedErr)
+			if !ok {
+				bl.log.Error("lookup error, bailing out", err, "dnsbl", listCfg.Zone)
+				return
+			}
+			bl.log.Msg("BL contains a record for ::FFFF:7F00:1", "dnsbl", listCfg.Zone)
+		}
+	}
+
+	if listCfg.EHLO || listCfg.MAILFROM {
+		// Domain-name-based DNSxLs MUST contain an entry for the reserved
+		// domain name "TEST".
+		err := checkDomain(bl.resolver, listCfg, "test")
+		if err == nil {
+			bl.log.Msg("BL does not contain a test record for 'test' TLD", "dnsbl", listCfg.Zone)
+		} else if _, ok := err.(ListedErr); !ok {
+			bl.log.Error("lookup error, bailing out", err, "dnsbl", listCfg.Zone)
+			return
+		}
+
+		// ... and MUST NOT contain an entry for the reserved domain name
+		// "INVALID".
+		err = checkDomain(bl.resolver, listCfg, "invalid")
+		if err != nil {
+			_, ok := err.(ListedErr)
+			if !ok {
+				bl.log.Error("lookup error, bailing out", err, "dnsbl", listCfg.Zone)
+				return
+			}
+			bl.log.Msg("BL contains a record for 'invalid' TLD", "dnsbl", listCfg.Zone)
+		}
+	}
+}
+
+func (bl *DNSBL) checkPreBody(ip net.IP, ehlo, mailFrom string) error {
+	eg := errgroup.Group{}
+
+	for _, list := range bl.bls {
+		list := list
+		eg.Go(func() error {
+			if list.ClientIPv4 || list.ClientIPv6 {
+				if err := checkIP(bl.resolver, list, ip); err != nil {
+					return err
+				}
+			}
+
+			if list.EHLO && ehlo != "" {
+				// Skip IPs in EHLO.
+				if strings.HasPrefix(ehlo, "[") && strings.HasSuffix(ehlo, "]") {
+					return nil
+				}
+
+				if err := checkDomain(bl.resolver, list, ehlo); err != nil {
+					return err
+				}
+			}
+
+			if list.MAILFROM && mailFrom != "" {
+				_, domain, err := address.Split(mailFrom)
+				if err != nil || domain == "" {
+					// Probably <postmaster> or <>, not much we can check.
+					return nil
+				}
+
+				if err := checkDomain(bl.resolver, list, domain); err != nil {
+					return err
+				}
+			}
+
+			return nil
+
+		})
+	}
+
+	// TODO: Whitelists support.
+	// ... if there is error and it is a ListenErr, then check whitelists
+	// for whether it is whitelisted.
+
+	return eg.Wait()
+}
+
+// CheckConnection implements module.EarlyCheck.
+func (bl *DNSBL) CheckConnection(state *smtp.ConnectionState) error {
+	ip, ok := state.RemoteAddr.(*net.TCPAddr)
+	if !ok {
+		bl.log.Msg("non-TCP/IP source",
+			"src_addr", state.RemoteAddr,
+			"src_host", state.Hostname)
+		return nil
+	}
+
+	if err := bl.checkPreBody(ip.IP, state.Hostname, ""); err != nil {
+		return mangleErr(err)
+	}
+
+	return nil
+}
+
+type state struct {
+	bl      *DNSBL
+	msgMeta *module.MsgMetadata
+	log     log.Logger
+}
+
+func (bl *DNSBL) CheckStateForMsg(msgMeta *module.MsgMetadata) (module.CheckState, error) {
+	return state{
+		bl:      bl,
+		msgMeta: msgMeta,
+		log:     target.DeliveryLogger(bl.log, msgMeta),
+	}, nil
+}
+
+func (s state) CheckConnection() module.CheckResult {
+	ip, ok := s.msgMeta.SrcAddr.(*net.TCPAddr)
+	if !ok {
+		s.log.Msg("non-TCP/IP source")
+		return module.CheckResult{}
+	}
+
+	if err := s.bl.checkPreBody(ip.IP, s.msgMeta.SrcHostname, s.msgMeta.OriginalFrom); err != nil {
+		// TODO: Support per-list actions?
+		return s.bl.listedAction.Apply(module.CheckResult{
+			Reason: mangleErr(err),
+		})
+	}
+
+	s.log.DebugMsg("ok")
+
+	return module.CheckResult{}
+}
+
+func (state) CheckSender(string) module.CheckResult {
+	return module.CheckResult{}
+}
+
+func (state) CheckRcpt(string) module.CheckResult {
+	return module.CheckResult{}
+}
+
+func (state) CheckBody(textproto.Header, buffer.Buffer) module.CheckResult {
+	return module.CheckResult{}
+}
+
+func (state) Close() error {
+	return nil
+}
+
+func init() {
+	module.Register("dnsbl", NewDNSBL)
+}

go.mod

@@ -26,6 +26,7 @@ require (
 	github.com/urfave/cli v1.20.0
 	golang.org/x/crypto v0.0.0-20190923035154-9ee001bba392
 	golang.org/x/net v0.0.0-20190923162816-aa69164e4478
+	golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
 	google.golang.org/appengine v1.6.2 // indirect
 	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect
 )

go.sum

@@ -137,6 +137,8 @@ golang.org/x/net v0.0.0-20190923162816-aa69164e4478 h1:l5EDrHhldLYb3ZRHDUhXF7Om7
 golang.org/x/net v0.0.0-20190923162816-aa69164e4478/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=
+golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d h1:+R4KGOnez64A81RvjARKc4UT5/tI9ujCIVX+P5KiHuI=

maddy.go

@@ -12,6 +12,7 @@ import (
 	_ "github.com/foxcpp/maddy/auth/shadow"
 	_ "github.com/foxcpp/maddy/check/dkim"
 	_ "github.com/foxcpp/maddy/check/dns"
+	_ "github.com/foxcpp/maddy/check/dnsbl"
 	_ "github.com/foxcpp/maddy/check/spf"
 	_ "github.com/foxcpp/maddy/endpoint/imap"
 	_ "github.com/foxcpp/maddy/endpoint/smtp"

man/maddy-filters.5.scd

@@ -230,6 +230,126 @@ Action to take when SPF policy evaluates to a 'permerror' result.
 
 Action to take when SPF policy evaluates to a 'temperror' result.
 
+# DNSBL lookup module (dnsbl)
+
+The dnsbl module implements checking of source IP and hostnames against a set
+of DNS-based Blackhole lists (DNSBLs).
+
+Its configuration consists of module configuration directives and a set
+of blocks specifing lists to use and kind of lookups to perform on them.
+
+```
+dnsbl {
+    debug no
+    check_early no
+    listed_action reject
+
+    # Lists configuration example.
+    dnsbl.example.org {
+        client_ipv4 yes
+        client_ipv6 no
+        ehlo no
+        mailfrom no
+    }
+    hsrbl.example.org {
+        client_ipv4 no
+        client_ipv6 no
+        ehlo yes
+        mailfrom yes
+    }
+}
+```
+
+## Inline arguments
+
+When used inline, arguments specify the list of IP-based BLs to use.
+
+The following configurations are equivalent.
+
+```
+check {
+    dnsbl dnsbl.example.org dnsbl2.example.org
+}
+```
+
+```
+check {
+    dnsbl {
+        dnsbl.example.org dnsbl2.example.org {
+            client_ipv4 yes
+            client_ipv6 no
+            ehlo no
+            mailfrom no
+        }
+    }
+}
+```
+
+## Configuration directives
+
+*Syntax*: debug _boolean_ ++
+*Default*: global directive value
+
+Enable verbose logging.
+
+*Syntax*: check_early _boolean_ ++
+*Default*: no
+
+Check BLs before mail delivery starts and silently reject blacklisted clients.
+
+In particular, this means:
+- No logging is done for rejected messages.
+- listed_action takes no effect. It is always reject.
+- defer_sender_reject from SMTP configuration takes no effect.
+
+If you often get hit by spam attacks, this is recommended to enable this
+setting to save server resources.
+
+*Syntax*: listed_action reject|quarantine|ignore ++
+*Default*: reject
+
+Action to take when one of the client identifiers is listed on the DNSBL.
+
+## List configuration
+
+```
+dnsbl.example.org dnsbl.example.com {
+    client_ipv4 yes
+    client_ipv6 no
+    ehlo no
+    mailfrom no
+}
+```
+
+Directive name and arguments specify the actual DNS zone to query when checking
+the list. Using multiple arguments is equivalent to specifying the same
+configuration separately for each list. 
+
+*Syntax*: client_ipv4 _boolean_ ++
+*Default*: yes
+
+Whether to check address of the IPv4 clients against the list.
+
+*Syntax*: client_ipv6 _boolean_ ++
+*Default*: yes
+
+Whether to check address of the IPv6 clients against the list.
+
+*Syntax*: ehlo _boolean_ ++
+*Default*: no
+
+Whether to check hostname specified n the HELO/EHLO command
+against the list.
+
+This works correctly only with domain-based DNSBLs.
+
+*Syntax*: mailfrom _boolean_ ++
+*Default*: no
+
+Whether to check domain part of the MAIL FROM address against the list.
+
+This works correctly only with domain-based DNSBLs.
+
 # DKIM signing module (sign_dkim)
 
 sign_dkim module is a modifier that signs messages using DKIM