It makes much more sense to do the loop checking on incoming mails rather

than on outgoing mails...

ok gilles@

2 files changed, 29 insertions, 59 deletions

diff --git a/usr.sbin/smtpd/mta_session.c b/usr.sbin/smtpd/mta_session.c
index 8d85ccf0a2c..e6e42a00078 100644
--- a/usr.sbin/smtpd/mta_session.c
+++ b/usr.sbin/smtpd/mta_session.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mta_session.c,v 1.67 2014/07/04 13:25:00 eric Exp $	*/
+/*	$OpenBSD: mta_session.c,v 1.68 2014/07/04 15:24:46 eric Exp $	*/
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -144,7 +144,6 @@ static void mta_send(struct mta_session *, char *, ...);
 static ssize_t mta_queue_data(struct mta_session *);
 static void mta_response(struct mta_session *, char *);
 static const char * mta_strstate(int);
-static int mta_check_loop(FILE *);
 static void mta_start_tls(struct mta_session *);
 static int mta_verify_certificate(struct mta_session *);
 static struct mta_session *mta_tree_pop(struct tree *, uint64_t);
@@ -286,16 +285,7 @@ mta_session_imsg(struct mproc *p, struct imsg *imsg)
 		if (s->datafp == NULL)
 			fatal("mta: fdopen");
 
-		if (mta_check_loop(s->datafp)) {
-			log_debug("debug: mta: loop detected");
-			fclose(s->datafp);
-			s->datafp = NULL;
-			mta_flush_task(s, IMSG_MTA_DELIVERY_LOOP,
-			    "Loop detected", 0, 0);
-			mta_enter_state(s, MTA_READY);
-		} else {
-			mta_enter_state(s, MTA_MAIL);
-		}
+		mta_enter_state(s, MTA_MAIL);
 		io_reload(&s->io);
 		return;
 
@@ -1490,48 +1480,6 @@ mta_error(struct mta_session *s, const char *fmt, ...)
 	free(error);
 }
 
-static int
-mta_check_loop(FILE *fp)
-{
-	char	*buf, *lbuf;
-	size_t	 len;
-	uint32_t rcvcount = 0;
-	int	 ret = 0;
-
-	lbuf = NULL;
-	while ((buf = fgetln(fp, &len))) {
-		if (buf[len - 1] == '\n')
-			buf[len - 1] = '\0';
-		else {
-			/* EOF without EOL, copy and add the NUL */
-			lbuf = xmalloc(len + 1, "mta_check_loop");
-			memcpy(lbuf, buf, len);
-			lbuf[len] = '\0';
-			buf = lbuf;
-		}
-
-		if (strchr(buf, ':') == NULL && !isspace((unsigned char)*buf))
-			break;
-
-		if (strncasecmp("Received: ", buf, 10) == 0) {
-			rcvcount++;
-			if (rcvcount == MAX_HOPS_COUNT) {
-				ret = 1;
-				break;
-			}
-		}
-		if (lbuf) {
-			free(lbuf);
-			lbuf  = NULL;
-		}
-	}
-	if (lbuf)
-		free(lbuf);
-
-	fseek(fp, SEEK_SET, 0);
-	return ret;
-}
-
 static void
 mta_start_tls(struct mta_session *s)
 {
diff --git a/usr.sbin/smtpd/smtp_session.c b/usr.sbin/smtpd/smtp_session.c
index 34eac0f423c..55dff532287 100644
--- a/usr.sbin/smtpd/smtp_session.c
+++ b/usr.sbin/smtpd/smtp_session.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtp_session.c,v 1.211 2014/05/17 20:07:54 chl Exp $	*/
+/*	$OpenBSD: smtp_session.c,v 1.212 2014/07/04 15:24:46 eric Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -85,8 +85,9 @@ enum message_flags {
 	MF_QUEUE_ENVELOPE_FAIL	= 0x0001,
 	MF_ERROR_SIZE		= 0x1000,
 	MF_ERROR_IO		= 0x2000,
-	MF_ERROR_MFA		= 0x4000,
+	MF_ERROR_LOOP		= 0x4000,
 };
+#define MF_ERROR	(MF_ERROR_SIZE | MF_ERROR_IO | MF_ERROR_LOOP)
 
 enum smtp_command {
 	CMD_HELO = 0,
@@ -140,9 +141,10 @@ struct smtp_session {
 	size_t			 rcptfail;
 	TAILQ_HEAD(, smtp_rcpt)	 rcpts;
 
-
 	size_t			 datalen;
 	FILE			*ofile;
+	int			 hdrdone;
+	int			 rcvcount;
 
 	struct event		 pause;
 };
@@ -1542,9 +1544,23 @@ smtp_message_write(struct smtp_session *s, const char *line)
 	log_trace(TRACE_SMTP, "<<< [MSG] %s", line);
 
 	/* Don't waste resources on message if it's going to bin anyway. */
-	if (s->msgflags & (MF_ERROR_IO | MF_ERROR_SIZE | MF_ERROR_MFA))
+	if (s->msgflags & MF_ERROR)
 		return;
 
+	if (*line == '\0')
+		s->hdrdone = 1;
+
+	/* check for loops */
+	if (!s->hdrdone) {
+		if (strncasecmp("Received: ", line, 10) == 0)
+			s->rcvcount++;
+		if (s->rcvcount == MAX_HOPS_COUNT) {
+			s->msgflags |= MF_ERROR_LOOP;
+			log_warn("warn: loop detected");
+			return;
+		}
+	}
+
 	len = strlen(line) + 1;
 
 	if (s->datalen + len > env->sc_maxsize) {
@@ -1572,7 +1588,7 @@ smtp_message_end(struct smtp_session *s)
 	fclose(s->ofile);
 	s->ofile = NULL;
 
-	if (s->msgflags & (MF_ERROR_SIZE | MF_ERROR_MFA | MF_ERROR_IO)) {
+	if (s->msgflags & MF_ERROR) {
 		m_create(p_queue, IMSG_SMTP_MESSAGE_ROLLBACK, 0, 0, -1);
 		m_add_msgid(p_queue, evpid_to_msgid(s->evp.id));
 		m_close(p_queue);
@@ -1580,6 +1596,10 @@ smtp_message_end(struct smtp_session *s)
 			smtp_reply(s, "554 %s %s: Transaction failed, message too big",
 			    esc_code(ESC_STATUS_PERMFAIL, ESC_MESSAGE_TOO_BIG_FOR_SYSTEM),
 			    esc_description(ESC_MESSAGE_TOO_BIG_FOR_SYSTEM));
+		else if (s->msgflags & MF_ERROR_LOOP)
+			smtp_reply(s, "500 %s %s: Loop detected",
+			    esc_code(ESC_STATUS_PERMFAIL, ESC_ROUTING_LOOP_DETECTED),
+			    esc_description(ESC_ROUTING_LOOP_DETECTED));
 		else
 			smtp_reply(s, "%d Message rejected", s->msgcode);
 		smtp_message_reset(s, 0);
@@ -1609,6 +1629,8 @@ smtp_message_reset(struct smtp_session *s, int prepare)
 	s->destcount = 0;
 	s->rcptcount = 0;
 	s->datalen = 0;
+	s->rcvcount = 0;
+	s->hdrdone = 0;
 
 	if (prepare) {
 		s->evp.ss = s->ss;