git » chasquid » commit 40153e3

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