BUG/MINOR: tcpcheck: Improve LDAP response parsing to fix LDAP check

When the LDAP response is parsed, the message length is not properly
decoded. While it works for LDAP servers encoding it on 1 byte, it does not
work for those using a multi-bytes encoding. Among others, Active Directory
servers seems to encode messages or elements length on 4 bytes.

In this patch, we only handle length of BindResponse messages encoded on 1,
2 or 4 bytes. In theory, it may be encoded on any bytes number less than 127
bytes. But it is useless to make this part too complex. It should be ok this
way.

This patch should fix the issue #1390. It should be backported to all stable
versions. While it should be easy to backport it as far as 2.2, the patch
will have to be totally rewritten for lower versions.

diff --git a/reg-tests/checks/ldap-check.vtc b/reg-tests/checks/ldap-check.vtc
index 4c0db11a6b7fbfd80bfd2129088d26473c15898f..c50c23e35d3c42ae139570e61711fab8c9bf64cd 100644 (file)
--- a/reg-tests/checks/ldap-check.vtc
+++ b/reg-tests/checks/ldap-check.vtc
@@ -23,6 +23,13 @@ server s3 {
   sendhex "300C020101 61 070A01 01 04000400"
 } -start
 
+server s4 {
+  recv 14
+  sendhex "308400000010020101 61 84000000070A01"
+  delay 0.1
+  sendhex "00 04000400"
+} -start
+
 syslog S1 -level notice {
     recv
     expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be1/srv succeeded, reason: Layer7 check passed.+info: \"Success\".+check duration: [[:digit:]]+ms, status: 1/1 UP."
@@ -38,6 +45,11 @@ syslog S3 -level notice {
     expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be3/srv failed, reason: Layer7 wrong status.+code: 1.+info: \"See RFC: http://tools.ietf.org/html/rfc4511#section-4.1.9\".+check duration: [[:digit:]]+ms, status: 0/1 DOWN."
 } -start
 
+syslog S4 -level notice {
+    recv
+    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be4/srv succeeded, reason: Layer7 check passed.+info: \"Success\".+check duration: [[:digit:]]+ms, status: 1/1 UP."
+} -start
+
 haproxy h1 -conf {
     defaults
         mode tcp
@@ -63,6 +75,12 @@ haproxy h1 -conf {
         option ldap-check
         server srv ${s3_addr}:${s3_port} check inter 1s rise 1 fall 1
 
+    backend be4
+        log ${S4_addr}:${S4_port} daemon
+        option log-health-checks
+        option ldap-check
+        server srv ${s4_addr}:${s4_port} check inter 1s rise 1 fall 1
+
     listen ldap1
         bind "fd@${ldap1}"
         tcp-request inspect-delay 100ms
@@ -75,3 +93,4 @@ haproxy h1 -conf {
 syslog S1 -wait
 syslog S2 -wait
 syslog S3 -wait
+syslog S4 -wait

diff --git a/src/tcpcheck.c b/src/tcpcheck.c
index a3b946c8ced932f3647e027423b3332ca0709989..2b93af5461da45d25be1f585f6db83e0b73e42b3 100644 (file)
--- a/src/tcpcheck.c
+++ b/src/tcpcheck.c
@@ -48,6 +48,7 @@
 #include <haproxy/istbuf.h>
 #include <haproxy/list.h>
 #include <haproxy/log.h>
+#include <haproxy/net_helper.h>
 #include <haproxy/protocol.h>
 #include <haproxy/proxy-t.h>
 #include <haproxy/regex.h>
@@ -656,7 +657,9 @@ enum tcpcheck_eval_ret tcpcheck_ldap_expect_bindrsp(struct check *check, struct
        enum healthcheck_status status;
        struct buffer *msg = NULL;
        struct ist desc = IST_NULL;
-       unsigned short msglen = 0;
+       char *ptr;
+       unsigned short nbytes = 0;
+       size_t msglen = 0;
 
        TRACE_ENTER(CHK_EV_TCPCHK_EXP, check);
 
@@ -664,35 +667,65 @@ enum tcpcheck_eval_ret tcpcheck_ldap_expect_bindrsp(struct check *check, struct
         * http://en.wikipedia.org/wiki/Basic_Encoding_Rules
         * http://tools.ietf.org/html/rfc4511
         */
+       ptr = b_head(&check->bi) + 1;
+
        /* size of LDAPMessage */
-       msglen = (*(b_head(&check->bi) + 1) & 0x80) ? (*(b_head(&check->bi) + 1) & 0x7f) : 0;
+       if (*ptr & 0x80) {
+               /* For message size encoded on several bytes, we only handle
+                * size encoded on 2 or 4 bytes. There is no reason to make this
+                * part to complex because only Active Directory is known to
+                * encode BindReponse length on 4 bytes.
+                */
+               nbytes = (*ptr & 0x7f);
+               if (b_data(&check->bi) < 1 + nbytes)
+                       goto too_short;
+               switch (nbytes) {
+                       case 4: msglen = read_n32(ptr+1); break;
+                       case 2: msglen = read_n16(ptr+1); break;
+                       default:
+                               status = HCHK_STATUS_L7RSP;
+                               desc = ist("Not LDAPv3 protocol");
+                               goto error;
+               }
+       }
+       else
+               msglen = *ptr;
+       ptr += 1 + nbytes;
+
+       if (b_data(&check->bi) < 2 + nbytes + msglen)
+               goto too_short;
 
        /* http://tools.ietf.org/html/rfc4511#section-4.2.2
         *   messageID: 0x02 0x01 0x01: INTEGER 1
         *   protocolOp: 0x61: bindResponse
         */
-       if ((msglen > 2) || (memcmp(b_head(&check->bi) + 2 + msglen, "\x02\x01\x01\x61", 4) != 0)) {
+       if (memcmp(ptr, "\x02\x01\x01\x61", 4) != 0) {
                status = HCHK_STATUS_L7RSP;
                desc = ist("Not LDAPv3 protocol");
                goto error;
        }
+       ptr += 4;
 
-       /* size of bindResponse */
-       msglen += (*(b_head(&check->bi) + msglen + 6) & 0x80) ? (*(b_head(&check->bi) + msglen + 6) & 0x7f) : 0;
+       /* skip size of bindResponse */
+       nbytes = 0;
+       if (*ptr & 0x80)
+               nbytes = (*ptr & 0x7f);
+       ptr += 1 + nbytes;
 
        /* http://tools.ietf.org/html/rfc4511#section-4.1.9
         *   ldapResult: 0x0a 0x01: ENUMERATION
         */
-       if ((msglen > 4) || (memcmp(b_head(&check->bi) + 7 + msglen, "\x0a\x01", 2) != 0)) {
+       if (memcmp(ptr, "\x0a\x01", 2) != 0) {
                status = HCHK_STATUS_L7RSP;
                desc = ist("Not LDAPv3 protocol");
                goto error;
        }
+       ptr += 2;
 
        /* http://tools.ietf.org/html/rfc4511#section-4.1.9
         *   resultCode
         */
-       check->code = *(b_head(&check->bi) + msglen + 9);
+       check->code = *ptr;
        if (check->code) {
                status = HCHK_STATUS_L7STS;
                desc = ist("See RFC: http://tools.ietf.org/html/rfc4511#section-4.1.9");
@@ -714,6 +747,18 @@ enum tcpcheck_eval_ret tcpcheck_ldap_expect_bindrsp(struct check *check, struct
                tcpcheck_expect_onerror_message(msg, check, rule, 0, desc);
        set_server_check_status(check, status, (msg ? b_head(msg) : NULL));
        goto out;
+
+  too_short:
+       if (!last_read)
+               goto wait_more_data;
+       /* invalid length or truncated response */
+       status = HCHK_STATUS_L7RSP;
+       goto error;
+
+  wait_more_data:
+       TRACE_DEVEL("waiting for more data", CHK_EV_TCPCHK_EXP, check);
+       ret = TCPCHK_EVAL_WAIT;
+       goto out;
 }
 
 /* Custom tcp-check expect function to parse and validate the SPOP hello agent