Implement check module for easier integration with rspamd

This replaces old rspamc-based integration script that is inefficient
and had many disadvantages.

build.sh

@@ -409,9 +409,6 @@ prepare_misc() {
 
     install -Dm 0644 -t "$PKGDIR/$PREFIX/lib/systemd/system/" systemd/maddy.service systemd/maddy@.service
 
-    install -Dm 0644 -t "$PKGDIR/$CONFDIR/integration/" integration/rspamd.conf
-    install -Dm 0755 -t "$PKGDIR/$PREFIX/lib/maddy/" scripts/rspamd-hook
-
     sed -Ei "s!/usr/bin!$PREFIX/bin!g;\
         s!/usr/lib/maddy!$PREFIX/lib/maddy!g;\
         s!/etc/maddy!$CONFDIR!g" "$PKGDIR/$SYSTEMDUNITS/system/maddy.service" "$PKGDIR/$SYSTEMDUNITS/system/maddy@.service"

dist/apparmor/dev.foxcpp.maddy.rspamd-hook

@@ -1,32 +0,0 @@
-# AppArmor profile for maddy's rspamd-hook script.
-# vim:syntax=apparmor:ts=2:sw=2:et
-
-#include <tunables/global>
-
-profile dev.foxcpp.maddy.rspamd-hook /usr{/local,}/lib/maddy/rspamd-hook {
-  #include <abstractions/base>
-
-  /usr/bin/rspamc-* Cx -> rspamc,
-  /usr/bin/cut rmix,
-  /usr/bin/grep rmix,
-
-  /usr{/local,}/lib/maddy/rspamd-hook r,
-
-  owner /dev/pts/* rw,
-  /dev/tty rw,
-  /bin/sh rmix,
-
-  profile rspamc {
-    #include <abstractions/base>
-    #include <abstractions/nameservice>
-    #include <abstractions/openssl>
-    /sys/kernel/mm/transparent_hugepage/enabled r,
-
-    /usr/bin/rspamc-* rmix,
-
-    #include if exists <local/dev.foxcpp.maddy.rspamd-hook.rspamc>
-  }
-
-  #include if exists <local/dev.foxcpp.maddy.rspamd-hook>
-}
-

dist/install.sh

@@ -22,6 +22,3 @@ install -Dm 0644 -t "$DESTDIR/$FAIL2BANDIR/jail.d/" fail2ban/jail.d/*
 install -Dm 0644 -t "$DESTDIR/$FAIL2BANDIR/filter.d/" fail2ban/filter.d/*
 
 install -Dm 0644 -t "$DESTDIR/$PREFIX/lib/systemd/system/" systemd/maddy.service systemd/maddy@.service
-
-install -Dm 0644 -t "$DESTDIR/$CONFDIR/integration/" integration/rspamd.conf
-install -Dm 0755 -t "$DESTDIR/$PREFIX/lib/maddy/" scripts/rspamd-hook

dist/integration/rspamd.conf

@@ -1,16 +0,0 @@
-# vim: ft=maddy-conf
-#
-# This configuration snippet provides integration with message rspamd filtering
-# engine via the console utility called rspamc.
-#
-# To use it, put the following directive in the smtp endpoint configuration block:
-#   import integration/rspamd
-#
-
-check {
-    command rspamd-hook {source_ip} {source_host} {sender} {auth_user} {
-        code 1 reject
-        code 2 quarantine
-        code 3 reject 450 4.7.0 "Message rejected due to a local policy"
-    }
-}

dist/scripts/rspamd-hook

@@ -1,51 +0,0 @@
-#!/bin/sh
-
-if [ "$4" != "" ]; then
-    out=$(rspamc -i "$1" --helo "$2" -F "$3" -u "$4")
-else
-    out=$(rspamc -i "$1" --helo "$2" -F "$3")
-fi
-action=$(echo "$out" | grep '^Action:' | cut -d " " -f 2-)
-score=$(echo "$out" | grep '^Score:' | cut -d " " -f 2)
-spam=$(echo "$out" | grep '^Spam:' | cut -d " " -f 2)
-
-echo 'X-Spam-Score:' "$score"
-
-case "$spam" in
-    "false")
-        echo 'X-Spam-Flag: NO'
-        ;;
-    "true")
-        echo 'X-Spam-Flag: YES'
-        ;;
-esac
-
-case "$action" in
-    "reject")
-        exit 1
-        ;;
-    "rewrite subject")
-        exit 2
-        ;;
-    "add header")
-        exit 2
-        ;;
-    "quarantine")
-        exit 2
-        ;;
-    "soft reject")
-        exit 3
-        ;;
-    "no action")
-        exit 0
-        ;;
-    "greylist")
-        # Default rspamd configuration uses 'greylist' action a lot, we ignore
-        # it explicitly since we have no support for greylisting (yet).
-        exit 0
-        ;;
-    *)
-        exit 128
-        ;;
-esac
-

docs/faq.md

@@ -81,9 +81,9 @@ are fine with alpha-quality but extremely easy to deploy webmail.
 No. maddy moves email messages around, it does not classify
 them as bad or good with the notable exception of sender policies.
 
-It should not be hard to integrate rspamd by calling `rspamc` command
-on delivery. Check
-https://github.com/foxcpp/maddy/blob/master/dist/integration/rspamd.conf
+It is possible to integrate rspamd using 'rspamd' module. Just add
+`rspamd` line to `inbound_checks` in default config, it should just work
+in most cases.
 
 ## Is it production-ready?
 

docs/man/maddy-filters.5.scd

@@ -800,3 +800,83 @@ The endpoit is specified in standard URL-like format:
 
 Toggles behavior on milter I/O errors. If false ("fail closed") - message is
 rejected with temporary error code. If true ("fail open") - check is skipped.
+
+## rspamd check (rspamd)
+
+The 'rspamd' module implements message filtering by contacting the rspamd
+server via HTTP API.
+
+```
+rspamd {
+	tls_client { ... }
+	api_path http://127.0.0.1:11333
+	settings_id whatever
+	tag maddy
+	hostname mx.example.org
+	io_error_action ignore
+	error_resp_action ignore
+	add_header_action quarantine
+	rewrite_subj_action quarantine
+	flags pass_all
+}
+
+rspamd http://127.0.0.1:11333
+```
+
+## Configuration directives
+
+*Syntax:* tls_client { ... } ++
+*Default:* not set
+
+Configure TLS client if HTTPS is used, see *maddy-tls*(5) for details.
+
+*Syntax:* api_path _url_ ++
+*Default:* http://127.0.0.1:11333
+
+URL of HTTP API endpoint. Supports both HTTP and HTTPS and can include
+path element.
+
+*Syntax:* settings_id _string_ ++
+*Default:* not set
+
+Settings ID to pass to the server.
+
+*Syntax:* tag _string_ ++
+*Default:* maddy
+
+Value to send in MTA-Tag header field.
+
+*Syntax:* hostname _string_ ++
+*Default:* value of global directive
+
+Value to send in MTA-Name header field.
+
+*Syntax:* io_error_action _action_ ++
+*Default:* ignore
+
+Action to take in case of inability to contact the rspamd server.
+
+*Syntax:* error_resp_action _action_ ++
+*Default:* ignore
+
+Action to take in case of 5xx or 4xx response received from the rspamd server.
+
+*Syntax:* add_header_action _action_ ++
+*Default:* quarantine
+
+Action to take when rspamd requests to "add header".
+
+X-Spam-Flag and X-Spam-Score are added to the header irregardless of value.
+
+*Syntax:* rewrite_subj_action _action_ ++
+*Default:* quarantine
+
+Action to take when rspamd requests to "rewrite subject".
+
+X-Spam-Flag and X-Spam-Score are added to the header irregardless of value.
+
+*Syntax:* flags _string list..._ ++
+*Default:* pass_all
+
+Flags to pass to the rspamd server.
+See https://rspamd.com/doc/architecture/protocol.html for details.

internal/check/rspamd/rspamd.go

@@ -0,0 +1,336 @@
+package rspamd
+
+import (
+	"context"
+	"crypto/tls"
+	"encoding/json"
+	"fmt"
+	"net"
+	"net/http"
+	"strconv"
+	"strings"
+
+	"github.com/emersion/go-message/textproto"
+	"github.com/foxcpp/maddy/internal/buffer"
+	"github.com/foxcpp/maddy/internal/check"
+	"github.com/foxcpp/maddy/internal/config"
+	"github.com/foxcpp/maddy/internal/exterrors"
+	"github.com/foxcpp/maddy/internal/log"
+	"github.com/foxcpp/maddy/internal/module"
+	"github.com/foxcpp/maddy/internal/target"
+)
+
+const modName = "rspamd"
+
+type Check struct {
+	instName string
+	log      log.Logger
+
+	apiPath    string
+	flags      string
+	settingsID string
+	tag        string
+	mtaName    string
+
+	ioErrAction       check.FailAction
+	errorRespAction   check.FailAction
+	addHdrAction      check.FailAction
+	rewriteSubjAction check.FailAction
+
+	client *http.Client
+}
+
+func New(modName, instName string, aliases, inlineArgs []string) (module.Module, error) {
+	c := &Check{
+		instName: instName,
+		client:   http.DefaultClient,
+	}
+
+	switch len(inlineArgs) {
+	case 1:
+		c.apiPath = inlineArgs[0]
+	case 0:
+		c.apiPath = "http://127.0.0.1:11333"
+	default:
+		return nil, fmt.Errorf("%s: unexpected amount of inline arguments", modName)
+	}
+
+	return c, nil
+}
+
+func (c *Check) Name() string {
+	return modName
+}
+
+func (c *Check) InstanceName() string {
+	return c.instName
+}
+
+func (c *Check) Init(cfg *config.Map) error {
+	var (
+		tlsConfig tls.Config
+		flags     []string
+	)
+
+	cfg.Custom("tls_client", true, false, func() (interface{}, error) {
+		return tls.Config{}, nil
+	}, config.TLSClientBlock, &tlsConfig)
+	cfg.String("api_path", false, false, c.apiPath, &c.apiPath)
+	cfg.String("settings_id", false, false, "", &c.settingsID)
+	cfg.String("tag", false, false, "maddy", &c.tag)
+	cfg.String("hostname", true, false, "", &c.mtaName)
+	cfg.Custom("io_error_action", false, false,
+		func() (interface{}, error) {
+			return check.FailAction{}, nil
+		}, check.FailActionDirective, &c.ioErrAction)
+	cfg.Custom("error_resp_action", false, false,
+		func() (interface{}, error) {
+			return check.FailAction{}, nil
+		}, check.FailActionDirective, &c.errorRespAction)
+	cfg.Custom("add_header_action", false, false,
+		func() (interface{}, error) {
+			return check.FailAction{Quarantine: true}, nil
+		}, check.FailActionDirective, &c.addHdrAction)
+	cfg.Custom("rewrite_subj_action", false, false,
+		func() (interface{}, error) {
+			return check.FailAction{Quarantine: true}, nil
+		}, check.FailActionDirective, &c.rewriteSubjAction)
+	cfg.StringList("flags", false, false, []string{"pass_all"}, &flags)
+	if _, err := cfg.Process(); err != nil {
+		return err
+	}
+
+	c.client = &http.Client{
+		Transport: &http.Transport{
+			TLSClientConfig: &tlsConfig,
+		},
+	}
+	c.flags = strings.Join(flags, ",")
+
+	return nil
+}
+
+type state struct {
+	c       *Check
+	msgMeta *module.MsgMetadata
+	log     log.Logger
+
+	mailFrom string
+	rcpt     []string
+}
+
+func (c *Check) CheckStateForMsg(ctx context.Context, msgMeta *module.MsgMetadata) (module.CheckState, error) {
+	return &state{
+		c:       c,
+		msgMeta: msgMeta,
+		log:     target.DeliveryLogger(c.log, msgMeta),
+	}, nil
+}
+
+func (s *state) CheckConnection(ctx context.Context) module.CheckResult {
+	return module.CheckResult{}
+}
+
+func (s *state) CheckSender(ctx context.Context, addr string) module.CheckResult {
+	s.mailFrom = addr
+	return module.CheckResult{}
+}
+
+func (s *state) CheckRcpt(ctx context.Context, addr string) module.CheckResult {
+	s.rcpt = append(s.rcpt, addr)
+	return module.CheckResult{}
+}
+
+func addConnHeaders(r *http.Request, meta *module.MsgMetadata, mailFrom string, rcpts []string) {
+	r.Header.Add("From", mailFrom)
+	for _, rcpt := range rcpts {
+		r.Header.Add("Rcpt", rcpt)
+	}
+
+	r.Header.Add("Queue-ID", meta.ID)
+
+	conn := meta.Conn
+	if conn != nil {
+		if meta.Conn.AuthUser != "" {
+			r.Header.Add("User", meta.Conn.AuthUser)
+		}
+
+		if tcpAddr, ok := conn.RemoteAddr.(*net.TCPAddr); ok {
+			r.Header.Add("IP", tcpAddr.IP.String())
+		}
+		r.Header.Add("Helo", conn.Hostname)
+		name, err := conn.RDNSName.Get()
+		if err == nil && name != nil {
+			r.Header.Add("Hostname", name.(string))
+		}
+
+		if conn.TLS.HandshakeComplete {
+			r.Header.Add("TLS-Cipher", tls.CipherSuiteName(conn.TLS.CipherSuite))
+			switch conn.TLS.Version {
+			case tls.VersionTLS13:
+				r.Header.Add("TLS-Version", "1.3")
+			case tls.VersionTLS12:
+				r.Header.Add("TLS-Version", "1.2")
+			case tls.VersionTLS11:
+				r.Header.Add("TLS-Version", "1.1")
+			case tls.VersionTLS10:
+				r.Header.Add("TLS-Version", "1.0")
+			}
+		}
+	}
+}
+
+func (s *state) CheckBody(ctx context.Context, hdr textproto.Header, body buffer.Buffer) module.CheckResult {
+	bodyR, err := body.Open()
+	if err != nil {
+		return module.CheckResult{
+			Reject: true,
+			Reason: exterrors.WithFields(err, map[string]interface{}{"check": modName}),
+		}
+	}
+
+	r, err := http.NewRequest("POST", s.c.apiPath+"/checkv2", bodyR)
+	if err != nil {
+		return module.CheckResult{
+			Reject: true,
+			Reason: exterrors.WithFields(err, map[string]interface{}{"check": modName}),
+		}
+	}
+
+	r.Header.Add("Pass", "all") // TODO: does that need to be configurable?
+	// TODO: include version (needs maddy.Version moved somewhere to break circular dependency)
+	r.Header.Add("User-Agent", "maddy")
+	if s.c.tag != "" {
+		r.Header.Add("MTA-Tag", s.c.tag)
+	}
+	if s.c.settingsID != "" {
+		r.Header.Add("Settings-ID", s.c.settingsID)
+	}
+	if s.c.mtaName != "" {
+		r.Header.Add("MTA-Name", s.c.mtaName)
+	}
+
+	addConnHeaders(r, s.msgMeta, s.mailFrom, s.rcpt)
+	r.Header.Add("Content-Length", strconv.Itoa(body.Len()))
+
+	resp, err := s.c.client.Do(r)
+	if err != nil {
+		return s.c.ioErrAction.Apply(module.CheckResult{
+			Reason: &exterrors.SMTPError{
+				Code:         451,
+				EnhancedCode: exterrors.EnhancedCode{4, 7, 0},
+				Message:      "Internal error during policy check",
+				CheckName:    modName,
+				Err:          err,
+			},
+		})
+	}
+	if resp.StatusCode/100 != 2 {
+		return s.c.errorRespAction.Apply(module.CheckResult{
+			Reason: &exterrors.SMTPError{
+				Code:         451,
+				EnhancedCode: exterrors.EnhancedCode{4, 7, 0},
+				Message:      "Internal error during policy check",
+				CheckName:    modName,
+				Err:          fmt.Errorf("HTTP %d", resp.StatusCode),
+			},
+		})
+	}
+	defer resp.Body.Close()
+
+	var respData response
+	if err := json.NewDecoder(resp.Body).Decode(&respData); err != nil {
+		return s.c.ioErrAction.Apply(module.CheckResult{
+			Reason: &exterrors.SMTPError{
+				Code:         451,
+				EnhancedCode: exterrors.EnhancedCode{4, 9, 0},
+				Message:      "Internal error during policy check",
+				CheckName:    modName,
+				Err:          err,
+			},
+		})
+	}
+
+	switch respData.Action {
+	case "no action":
+	case "greylist":
+		// uuh... TODO: Implement greylisting?
+		hdrAdd := textproto.Header{}
+		hdrAdd.Add("X-Spam-Score", strconv.FormatFloat(respData.Score, 'f', 2, 64))
+		return module.CheckResult{
+			Header: hdrAdd,
+		}
+	case "add header":
+		hdrAdd := textproto.Header{}
+		hdrAdd.Add("X-Spam-Flag", "Yes")
+		hdrAdd.Add("X-Spam-Score", strconv.FormatFloat(respData.Score, 'f', 2, 64))
+		return s.c.addHdrAction.Apply(module.CheckResult{
+			Reason: &exterrors.SMTPError{
+				Code:         450,
+				EnhancedCode: exterrors.EnhancedCode{4, 7, 0},
+				Message:      "Message rejected due to local policy",
+				CheckName:    modName,
+				Misc:         map[string]interface{}{"action": "add header"},
+			},
+			Header: hdrAdd,
+		})
+	case "rewrite subject":
+		hdrAdd := textproto.Header{}
+		hdrAdd.Add("X-Spam-Flag", "Yes")
+		hdrAdd.Add("X-Spam-Score", strconv.FormatFloat(respData.Score, 'f', 2, 64))
+		return s.c.rewriteSubjAction.Apply(module.CheckResult{
+			Reason: &exterrors.SMTPError{
+				Code:         450,
+				EnhancedCode: exterrors.EnhancedCode{4, 7, 0},
+				Message:      "Message rejected due to local policy",
+				CheckName:    modName,
+				Misc:         map[string]interface{}{"action": "rewrite subject"},
+			},
+			Header: hdrAdd,
+		})
+	case "soft reject":
+		return module.CheckResult{
+			Reject: true,
+			Reason: &exterrors.SMTPError{
+				Code:         450,
+				EnhancedCode: exterrors.EnhancedCode{4, 7, 0},
+				Message:      "Message rejected due to local policy",
+				CheckName:    modName,
+				Misc:         map[string]interface{}{"action": "soft reject"},
+			},
+		}
+	case "reject":
+		return module.CheckResult{
+			Reject: true,
+			Reason: &exterrors.SMTPError{
+				Code:         550,
+				EnhancedCode: exterrors.EnhancedCode{5, 7, 0},
+				Message:      "Message rejected due to local policy",
+				CheckName:    modName,
+				Misc:         map[string]interface{}{"action": "reject"},
+			},
+		}
+	}
+
+	s.log.Msg("unhandled action", respData.Action)
+
+	return module.CheckResult{}
+}
+
+type response struct {
+	Score   float64 `json:"score"`
+	Action  string  `json:"action"`
+	Subject string  `json:"subject"`
+	Symbols map[string]struct {
+		Name  string  `json:"name"`
+		Score float64 `json:"score"`
+	}
+}
+
+func (s *state) Close() error {
+	return nil
+}
+
+func init() {
+	module.Register(modName, New)
+}

maddy.go

@@ -33,6 +33,7 @@ import (
 	_ "github.com/foxcpp/maddy/internal/check/dnsbl"
 	_ "github.com/foxcpp/maddy/internal/check/milter"
 	_ "github.com/foxcpp/maddy/internal/check/requiretls"
+	_ "github.com/foxcpp/maddy/internal/check/rspamd"
 	_ "github.com/foxcpp/maddy/internal/check/spf"
 	_ "github.com/foxcpp/maddy/internal/endpoint/dovecot_sasld"
 	_ "github.com/foxcpp/maddy/internal/endpoint/imap"