From 40153e352fd0c5da7d81e26fb799069c5b036a6d Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Sat, 8 Oct 2016 12:11:20 +0100 Subject: [PATCH] chasquid: Detect email loops This patch implements some measures against email loops, such as keeping a limit on the lenght of an address, and rejecting email that has too many Received headers. It's not perfect (a server could be actively removing Received headers), but it should cover the normal accidents and misconfigurations. --- chasquid.go | 38 +++++++++++++++++++++-- chasquid_test.go | 2 +- test/.gitignore | 1 + test/t-04-aliases/run.sh | 14 ++++----- test/t-09-loop/A/chasquid.conf | 8 +++++ test/t-09-loop/A/domains/srv-A/aliases | 2 ++ test/t-09-loop/B/chasquid.conf | 8 +++++ test/t-09-loop/B/domains/srv-B/aliases | 1 + test/t-09-loop/content | 9 ++++++ test/t-09-loop/hosts | 2 ++ test/t-09-loop/msmtprc | 14 +++++++++ test/t-09-loop/run.sh | 43 ++++++++++++++++++++++++++ 12 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 test/t-09-loop/A/chasquid.conf create mode 100644 test/t-09-loop/A/domains/srv-A/aliases create mode 100644 test/t-09-loop/B/chasquid.conf create mode 100644 test/t-09-loop/B/domains/srv-B/aliases create mode 100644 test/t-09-loop/content create mode 100644 test/t-09-loop/hosts create mode 100644 test/t-09-loop/msmtprc create mode 100755 test/t-09-loop/run.sh diff --git a/chasquid.go b/chasquid.go index a5ecbe2..bcb018a 100644 --- a/chasquid.go +++ b/chasquid.go @@ -50,6 +50,7 @@ var ( commandCount = expvar.NewMap("chasquid/smtpIn/commandCount") responseCodeCount = expvar.NewMap("chasquid/smtpIn/responseCodeCount") spfResultCount = expvar.NewMap("chasquid/smtpIn/spfResultCount") + loopsDetected = expvar.NewInt("chasquid/smtpIn/loopsDetected") ) func main() { @@ -648,6 +649,11 @@ func (c *Conn) MAIL(params string) (code int, msg string) { return 501, "sender address must contain a domain" } + // https://tools.ietf.org/html/rfc5321#section-4.5.3.1.3 + if len(e.Address) > 256 { + return 501, "address too long" + } + // SPF check - https://tools.ietf.org/html/rfc7208#section-2.4 if tcp, ok := c.netconn.RemoteAddr().(*net.TCPAddr); ok { c.spfResult, c.spfError = spf.CheckHost( @@ -683,6 +689,10 @@ func (c *Conn) RCPT(params string) (code int, msg string) { return 500, "unknown command" } + if c.mailFrom == "" { + return 503, "sender not yet given" + } + rawAddr := "" _, err := fmt.Sscanf(params[3:], "%s ", &rawAddr) if err != nil { @@ -709,8 +719,9 @@ func (c *Conn) RCPT(params string) (code int, msg string) { return 501, "malformed address (IDNA conversion failed)" } - if c.mailFrom == "" { - return 503, "sender not yet given" + // https://tools.ietf.org/html/rfc5321#section-4.5.3.1.3 + if len(e.Address) > 256 { + return 501, "address too long" } localDst := envelope.DomainIn(e.Address, c.localDomains) @@ -755,6 +766,10 @@ func (c *Conn) DATA(params string) (code int, msg string) { c.tr.Debugf("-> ... %d bytes of data", len(c.data)) + if err := checkData(c.data); err != nil { + return 554, err.Error() + } + c.addReceivedHeader() // There are no partial failures here: we put it in the queue, and then if @@ -815,6 +830,25 @@ func (c *Conn) addReceivedHeader() { } } +// checkData performs very basic checks on the body of the email, to help +// detect very broad problems like email loops. It does not fully check the +// sanity of the headers or the structure of the payload. +func checkData(data []byte) error { + msg, err := mail.ReadMessage(bytes.NewBuffer(data)) + if err != nil { + return fmt.Errorf("error parsing message: %v", err) + } + + // This serves as a basic form of loop prevention. It's not infallible but + // should catch most instances of accidental looping. + if len(msg.Header["Received"]) > 50 { + loopsDetected.Add(1) + return fmt.Errorf("email passed through more than 50 MTAs, looping?") + } + + return nil +} + func (c *Conn) STARTTLS(params string) (code int, msg string) { if c.onTLS { return 503, "You are already wearing that!" diff --git a/chasquid_test.go b/chasquid_test.go index d44344d..78b6cb3 100644 --- a/chasquid_test.go +++ b/chasquid_test.go @@ -108,7 +108,7 @@ func sendEmailWithAuth(tb testing.TB, c *smtp.Client, auth smtp.Auth) { tb.Fatalf("Data: %v", err) } - msg := []byte("Hi! This is an email\n") + msg := []byte("Subject: Hi!\n\n This is an email\n") if _, err = w.Write(msg); err != nil { tb.Errorf("Data write: %v", err) } diff --git a/test/.gitignore b/test/.gitignore index 243d233..8495b0c 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -1,4 +1,5 @@ # Ignore the user databases - we create them each time. t-*/config/**/users +t-*/?/**/users diff --git a/test/t-04-aliases/run.sh b/test/t-04-aliases/run.sh index 5128b81..15e512f 100755 --- a/test/t-04-aliases/run.sh +++ b/test/t-04-aliases/run.sh @@ -13,13 +13,13 @@ chasquid -v=2 --log_dir=.logs --config_dir=config & wait_until_ready 1025 function send_and_check() { - run_msmtp $1@testserver < content - shift - for i in $@; do - wait_for_file .mail/$i@testserver - mail_diff content .mail/$i@testserver - rm -f .mail/$i@testserver - done + run_msmtp $1@testserver < content + shift + for i in $@; do + wait_for_file .mail/$i@testserver + mail_diff content .mail/$i@testserver + rm -f .mail/$i@testserver + done } # Test email aliases. diff --git a/test/t-09-loop/A/chasquid.conf b/test/t-09-loop/A/chasquid.conf new file mode 100644 index 0000000..2b10846 --- /dev/null +++ b/test/t-09-loop/A/chasquid.conf @@ -0,0 +1,8 @@ +smtp_address: ":1025" +submission_address: ":1587" +monitoring_address: ":1099" + +mail_delivery_agent_bin: "test-mda" +mail_delivery_agent_args: "%to%" + +data_dir: "../.data-A" diff --git a/test/t-09-loop/A/domains/srv-A/aliases b/test/t-09-loop/A/domains/srv-A/aliases new file mode 100644 index 0000000..69c81a5 --- /dev/null +++ b/test/t-09-loop/A/domains/srv-A/aliases @@ -0,0 +1,2 @@ + +aliasA: aliasB@srv-B diff --git a/test/t-09-loop/B/chasquid.conf b/test/t-09-loop/B/chasquid.conf new file mode 100644 index 0000000..0c73544 --- /dev/null +++ b/test/t-09-loop/B/chasquid.conf @@ -0,0 +1,8 @@ +smtp_address: ":2025" +submission_address: ":2587" +monitoring_address: ":2099" + +mail_delivery_agent_bin: "test-mda" +mail_delivery_agent_args: "%to%" + +data_dir: "../.data-B" diff --git a/test/t-09-loop/B/domains/srv-B/aliases b/test/t-09-loop/B/domains/srv-B/aliases new file mode 100644 index 0000000..826a5f8 --- /dev/null +++ b/test/t-09-loop/B/domains/srv-B/aliases @@ -0,0 +1 @@ +aliasB: aliasA@srv-A diff --git a/test/t-09-loop/content b/test/t-09-loop/content new file mode 100644 index 0000000..027781d --- /dev/null +++ b/test/t-09-loop/content @@ -0,0 +1,9 @@ +From: userA@srv-A +To: aliasB@srv-B +Subject: Los espejos + +Yo que sentí el horror de los espejos +no sólo ante el cristal impenetrable +donde acaba y empieza, inhabitable, +un imposible espacio de reflejos + diff --git a/test/t-09-loop/hosts b/test/t-09-loop/hosts new file mode 100644 index 0000000..b2ae8db --- /dev/null +++ b/test/t-09-loop/hosts @@ -0,0 +1,2 @@ +srv-A localhost +srv-B localhost diff --git a/test/t-09-loop/msmtprc b/test/t-09-loop/msmtprc new file mode 100644 index 0000000..a46c7eb --- /dev/null +++ b/test/t-09-loop/msmtprc @@ -0,0 +1,14 @@ +account default + +host srv-A +port 1587 + +tls on +tls_trust_file A/certs/srv-A/fullchain.pem + +from userA@srv-A + +auth on +user userA@srv-A +password userA + diff --git a/test/t-09-loop/run.sh b/test/t-09-loop/run.sh new file mode 100755 index 0000000..04c218e --- /dev/null +++ b/test/t-09-loop/run.sh @@ -0,0 +1,43 @@ +#!/bin/bash + +set -e +. $(dirname ${0})/../util/lib.sh + +init + +rm -rf .data-A .data-B .mail + +# Two servers: +# A - listens on :1025, hosts srv-A +# B - listens on :2015, hosts srv-B +# +# We cause the following loop: +# userA -> aliasB -> aliasA -> aliasB -> ... + +CONFDIR=A generate_certs_for srv-A +CONFDIR=A add_user srv-A userA userA + +CONFDIR=B generate_certs_for srv-B + +mkdir -p .logs-A .logs-B + +chasquid -v=2 --log_dir=.logs-A --config_dir=A \ + --testing__outgoing_smtp_port=2025 & +chasquid -v=2 --log_dir=.logs-B --config_dir=B \ + --testing__outgoing_smtp_port=1025 & + +wait_until_ready 1025 +wait_until_ready 2025 + +run_msmtp aliasB@srv-B < content + +# Wait until one of them has noticed and stopped the loop. +while sleep 0.1; do + wget -q -O .data-A/vars http://localhost:1099/debug/vars + wget -q -O .data-B/vars http://localhost:2099/debug/vars + if grep -q '"chasquid/smtpIn/loopsDetected": 1,' .data-?/vars; then + break + fi +done + +success