commit 4a75d59: [Fix] Use dup fd in milter handler to avoid races with the proxy

[Vsevolod Stakhov]

Author: Vsevolod Stakhov
Date: 2020-10-09 12:31:03 +0100
URL: https://github.com/rspamd/rspamd/commit/4a75d59ccf077d08ea33695df586d81345ed2f4c

[Fix] Use dup fd in milter handler to avoid races with the proxy

---
 src/libserver/milter.c | 18 ++++++++++++++----
 src/rspamd_proxy.c     | 27 +++++++++++++++++++--------
 2 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/src/libserver/milter.c b/src/libserver/milter.c
index dc398912d..8cb7ed2f0 100644
--- a/src/libserver/milter.c
+++ b/src/libserver/milter.c
@@ -54,6 +54,8 @@ static const struct rspamd_milter_context *milter_ctx = NULL;
 static gboolean  rspamd_milter_handle_session (
 		struct rspamd_milter_session *session,
 		struct rspamd_milter_private *priv);
+static inline void rspamd_milter_plan_io (struct rspamd_milter_session *session,
+					   struct rspamd_milter_private *priv, gshort what);
 
 static GQuark
 rspamd_milter_quark (void)
@@ -183,6 +185,7 @@ rspamd_milter_session_dtor (struct rspamd_milter_session *session)
 
 		rspamd_ev_watcher_stop (priv->event_loop, &priv->ev);
 		rspamd_milter_session_reset (session, RSPAMD_MILTER_RESET_ALL);
+		close (priv->fd);
 
 		if (priv->parser.buf) {
 			rspamd_fstring_free (priv->parser.buf);
@@ -689,8 +692,6 @@ rspamd_milter_process_command (struct rspamd_milter_session *session,
 			REF_RETAIN (session);
 			priv->fin_cb (priv->fd, session, priv->ud);
 			REF_RELEASE (session);
-
-			return FALSE;
 		}
 		break;
 	case RSPAMD_MILTER_CMD_RCPT:
@@ -1104,6 +1105,15 @@ rspamd_milter_handle_socket (gint fd, ev_tstamp timeout,
 {
 	struct rspamd_milter_session *session;
 	struct rspamd_milter_private *priv;
+	gint nfd = dup (fd);
+
+	if (nfd == -1) {
+		GError *err = g_error_new (rspamd_milter_quark (), errno,
+				"dup failed: %s", strerror (errno));
+		error_cb (fd, NULL, ud, err);
+
+		return FALSE;
+	}
 
 	g_assert (finish_cb != NULL);
 	g_assert (error_cb != NULL);
@@ -1111,7 +1121,7 @@ rspamd_milter_handle_socket (gint fd, ev_tstamp timeout,
 
 	session = g_malloc0 (sizeof (*session));
 	priv = g_malloc0 (sizeof (*priv));
-	priv->fd = fd;
+	priv->fd = nfd;
 	priv->ud = ud;
 	priv->fin_cb = finish_cb;
 	priv->err_cb = error_cb;
@@ -1124,7 +1134,7 @@ rspamd_milter_handle_socket (gint fd, ev_tstamp timeout,
 	priv->quarantine_on_reject = milter_ctx->quarantine_on_reject;
 	priv->ev.timeout = timeout;
 
-	rspamd_ev_watcher_init (&priv->ev, fd, EV_READ|EV_WRITE,
+	rspamd_ev_watcher_init (&priv->ev, priv->fd, EV_READ|EV_WRITE,
 			rspamd_milter_io_handler, session);
 
 	if (pool) {
diff --git a/src/rspamd_proxy.c b/src/rspamd_proxy.c
index 4ed8cfab7..3fa5da390 100644
--- a/src/rspamd_proxy.c
+++ b/src/rspamd_proxy.c
@@ -2150,18 +2150,29 @@ proxy_milter_finish_handler (gint fd,
 
 static void
 proxy_milter_error_handler (gint fd,
-		struct rspamd_milter_session *rms,
+		struct rspamd_milter_session *rms, /* unused */
 		void *ud, GError *err)
 {
 	struct rspamd_proxy_session *session = ud;
 
-	msg_info_session ("abnormally closing milter connection from: %s, "
-			"error: %e",
-			rspamd_inet_address_to_string_pretty (session->client_addr),
-			err);
-	/* Terminate session immediately */
-	proxy_backend_close_connection (session->master_conn);
-	REF_RELEASE (session);
+	if (err && err->code != 0) {
+		msg_info_session ("abnormally closing milter connection from: %s, "
+						  "error: %e",
+				rspamd_inet_address_to_string_pretty (session->client_addr),
+				err);
+		/* Terminate session immediately */
+		proxy_backend_close_connection (session->master_conn);
+		REF_RELEASE (session);
+	}
+	else {
+		msg_info_session ("normally closing milter connection from: %s, "
+						  "%e",
+				rspamd_inet_address_to_string_pretty (session->client_addr),
+				err);
+		/* Terminate session immediately */
+		proxy_backend_close_connection (session->master_conn);
+		REF_RELEASE (session);
+	}
 }
 
 static void