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]

diff --git a/reg-tests/checks/4be_1srv_smtpchk_httpchk_layer47errors.vtc b/reg-tests/checks/4be_1srv_smtpchk_httpchk_layer47errors.vtc
index 90a4881bcf430101154f2902a3dfe3fb8c0fb454..3d3649125fe7b240e7ebe2f60bc9f14587b69888 100644 (file)
--- 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 aea129c13f27c932770212847c8399e608916c9d..723f5f0485f80d23b764940e0703a1b3e0af09e8 100644 (file)
--- 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 deb744535470c08b13df93e2f48bf7ee96bf5e43..5ef1c69cbddf7d087e2f8355abf4daa684a6725e 100644 (file)
--- a/src/tcpcheck.c
+++ b/src/tcpcheck.c
@@ -6,6 +6,7 @@
  * Copyright 2013 Baptiste Assmann <bedis9@gmail.com>
  * Copyright 2020 Gaetan Rivet <grive@u256.net>
  * Copyright 2020 Christopher Faulet <cfaulet@haproxy.com>
+ * Crown Copyright 2022 Defence Science and Technology Laboratory <dstlipgroup@dstl.gov.uk>
  *
  * 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);