From 9a8d8a3fd0828a1cb64745318dcc5704a0b4b1a9 Mon Sep 17 00:00:00 2001 From: wrightlaw Date: Thu, 8 Sep 2022 16:10:48 +0100 Subject: [PATCH] BUG/MINOR: smtpchk: SMTP Service check should gracefully close SMTP transaction At present option smtpchk closes the TCP connection abruptly on completion of service checking, even if successful. This can result in a very high volume of errors in backend SMTP server logs. This patch ensures an SMTP QUIT is sent and a positive 2xx response is received from the SMTP server prior to disconnection. This patch depends on the following one: * MINOR: smtpchk: Update expect rule to fully match replies to EHLO commands This patch should fix the issue #1812. It may be backported as far as 2.2 with the commit above On the 2.2, proxy_parse_smtpchk_opt() function is located in src/check.c [cf: I updated reg-tests script accordingly] --- ...4be_1srv_smtpchk_httpchk_layer47errors.vtc | 7 ++++- reg-tests/checks/smtp-check.vtc | 8 ++++-- src/tcpcheck.c | 27 +++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/reg-tests/checks/4be_1srv_smtpchk_httpchk_layer47errors.vtc b/reg-tests/checks/4be_1srv_smtpchk_httpchk_layer47errors.vtc index 90a4881bc..3d3649125 100644 --- a/reg-tests/checks/4be_1srv_smtpchk_httpchk_layer47errors.vtc +++ b/reg-tests/checks/4be_1srv_smtpchk_httpchk_layer47errors.vtc @@ -8,7 +8,7 @@ barrier b cond 3 syslog S1 -level notice { recv - expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be1/srv1 succeeded.+reason: Layer7 check passed.+code: 2(20|48).+check duration: [[:digit:]]+ms.+status: 1/1 UP." + expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be1/srv1 succeeded.+reason: Layer7 check passed.+code: 221.+check duration: [[:digit:]]+ms.+status: 1/1 UP." barrier b sync recv expect ~ "Health check for server be1/srv1 failed.+reason: Layer7 timeout.+check duration: [[:digit:]]+ms.+status: 0/1 DOWN" @@ -42,6 +42,11 @@ server s1 { send "4" send "8" send "\r\n" + recv 6 + send "2" + send "2" + send "1" + send " ok\r\n" } -start server s2 { diff --git a/reg-tests/checks/smtp-check.vtc b/reg-tests/checks/smtp-check.vtc index aea129c13..723f5f048 100644 --- a/reg-tests/checks/smtp-check.vtc +++ b/reg-tests/checks/smtp-check.vtc @@ -10,6 +10,8 @@ server s1 { send "220 smtp-check.vtc SMTP Server\r\n" recv 16 send "250 smtp-check.vtc\r\n" + recv 6 + send "221 smtp-check.vtc closing\r\n" } -start server s2 { @@ -18,6 +20,8 @@ server s2 { send "250-smtp-check.vtc\r\n" send "250-KEYWORD\r\n" send "250 LAST KEYWORD\r\n" + recv 6 + send "221 smtp-check.vtc closing\r\n" } -start server s3 { @@ -36,12 +40,12 @@ server s5 { syslog S1 -level notice { recv - expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be1/srv succeeded, reason: Layer7 check passed.+code: 250.+info: \"smtp-check.vtc\".+check duration: [[:digit:]]+ms, status: 1/1 UP." + expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be1/srv succeeded, reason: Layer7 check passed.+code: 221.+info: \"smtp-check.vtc closing\".+check duration: [[:digit:]]+ms, status: 1/1 UP." } -start syslog S2 -level notice { recv - expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be2/srv succeeded, reason: Layer7 check passed.+code: 250.+info: \"smtp-check.vtc\".+check duration: [[:digit:]]+ms, status: 1/1 UP." + expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be2/srv succeeded, reason: Layer7 check passed.+code: 221.+info: \"smtp-check.vtc closing\".+check duration: [[:digit:]]+ms, status: 1/1 UP." } -start syslog S3 -level notice { diff --git a/src/tcpcheck.c b/src/tcpcheck.c index deb744535..5ef1c69cb 100644 --- a/src/tcpcheck.c +++ b/src/tcpcheck.c @@ -6,6 +6,7 @@ * Copyright 2013 Baptiste Assmann * Copyright 2020 Gaetan Rivet * Copyright 2020 Christopher Faulet + * Crown Copyright 2022 Defence Science and Technology Laboratory * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -4358,6 +4359,32 @@ int proxy_parse_smtpchk_opt(char **args, int cur_arg, struct proxy *curpx, const chk->index = 4; LIST_APPEND(&rs->rules, &chk->list); + /* Send an SMTP QUIT to ensure clean disconnect (issue 1812), and expect a 2xx response code */ + + chk = parse_tcpcheck_send((char *[]){"tcp-check", "send", "QUIT\r\n", ""}, + 1, curpx, &rs->rules, file, line, &errmsg); + if (!chk) { + ha_alert("parsing [%s:%d] : %s\n", file, line, errmsg); + goto error; + } + chk->index = 5; + LIST_APPEND(&rs->rules, &chk->list); + + chk = parse_tcpcheck_expect((char *[]){"tcp-check", "expect", "rstring", "^2[0-9]{2}[- \r]", + "min-recv", "4", + "error-status", "L7STS", + "on-error", "%[res.payload(4,0),ltrim(' '),cut_crlf]", + "on-success", "%[res.payload(4,0),ltrim(' '),cut_crlf]", + "status-code", "res.payload(0,3)", + ""}, + 1, curpx, &rs->rules, TCPCHK_RULES_SMTP_CHK, file, line, &errmsg); + if (!chk) { + ha_alert("parsing [%s:%d] : %s\n", file, line, errmsg); + goto error; + } + chk->index = 6; + LIST_APPEND(&rs->rules, &chk->list); + ruleset_found: rules->list = &rs->rules; rules->flags &= ~(TCPCHK_RULES_PROTO_CHK|TCPCHK_RULES_UNUSED_RS);