Hi Andreas and Willy, Please find attached a patch serie adding support for both legacy and standard CIP protocol while keeping compatibility with current configuration format.
This also fixes numerous bugs spotted during this dev cycle and present since the first version of the patch. This serie applies cleanly on v1.9-dev0-76-g789691778fde but also on v1.8.1-20-gdd8ea125889d while I only tested it on v1.9. Cheers, Bertrand On 11/12/17 15:04, Bertrand Jacquin wrote: > Hi Andreas, > > I got this really side tracked, my apology. Let me take a look at that > this evening again. Some corps need to be unburied. > > I'm afraid the patch, as is, will break compatibility with other version > of the CIP protocol, I'd like haproxy to support both of them. > > Cheers, > Bertrand > > On 07/12/17 13:41, Andreas Mahnke wrote: >> Hello everybody, >> >> Is there an update regarding the merging of the patch? We are thinking >> to switch to version 1.8 in the near future and it would be nice to have >> the patch merged, so that we do not need to merge each update on our own. >> >> Best regards, >> Andreas >> >> On Fri, Jul 7, 2017 at 2:43 PM, Willy Tarreau <[email protected] >> <mailto:[email protected]>> wrote: >> >> On Fri, Jul 07, 2017 at 02:36:07PM +0200, Andreas Mahnke wrote: >> > Hi Willy, >> > >> > I had some direct mail conversations with him and he wanted to create a >> > patch based in the findings. >> > In the meantime we got it working using the patch I provided, >> therefore I >> > sent it yesterday so that it gets integrated and we do not need to >> patch >> > haproxy on our end everytime a new version comes out. >> > >> > I wrote him yesterday that the patch was sent by me, but he seems to >> be out >> > of office until monday - so maybe he will get back to us then. >> >> Yep I noticed the out-of-office notification as well. Let's wait for >> him to >> get back. I'm pretty fine with merging your patch, I just want to be >> certain >> that everything is OK on his side as well, especially before >> backporting it >> (since it's a bug fix). >> >> Thanks! >> Willy >> >> > -- Bertrand Payments Infrastructure Engineering, Amazon
From 3245e08747ed3c72fa429365010450c2ca04caac Mon Sep 17 00:00:00 2001 From: Bertrand Jacquin <[email protected]> Date: Tue, 12 Dec 2017 01:17:23 +0000 Subject: [PATCH 8/8] MEDIUM: netscaler: add support for standard NetScaler CIP protocol It looks like two version of the protocol exist as reported by Andreas Mahnke. This patch add support for both legacy and standard CIP protocol according to NetScaler specifications. --- doc/netscaler-client-ip-insertion-protocol.txt | 28 ++++++++++++++++++- src/connection.c | 38 ++++++++++++++++---------- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/doc/netscaler-client-ip-insertion-protocol.txt b/doc/netscaler-client-ip-insertion-protocol.txt index 6f77f6522c7f..559d98a82a92 100644 --- a/doc/netscaler-client-ip-insertion-protocol.txt +++ b/doc/netscaler-client-ip-insertion-protocol.txt @@ -10,7 +10,9 @@ Specifications and documentations from NetScaler: https://www.citrix.com/blogs/2016/04/25/how-to-enable-client-ip-in-tcpip-option-of-netscaler/ When CIP is enabled on the NetScaler, then a TCP packet is inserted just after -the TCP handshake. This is composed as: +the TCP handshake. Two versions of the CIP extension exist. + +Legacy (NetScaler < 10.5) - CIP magic number : 4 bytes Both sender and receiver have to agree on a magic number so that @@ -27,3 +29,27 @@ the TCP handshake. This is composed as: - TCP header : >= 20 bytes Contains the header of the last TCP packet sent by the client during TCP handshake. + +Standard (NetScaler >= 10.5) + + - CIP magic number : 4 bytes + Both sender and receiver have to agree on a magic number so that + they both handle the incoming data as a NetScaler Client IP insertion + packet. + + - CIP length : 4 bytes + Defines the total length on the CIP header. + + - CIP type: 2 bytes + Always set to 1. + + - Header length : 2 bytes + Defines the length on the remaining data. + + - IP header : >= 20 bytes if IPv4, 40 bytes if IPv6 + Contains the header of the last IP packet sent by the client during TCP + handshake. + + - TCP header : >= 20 bytes + Contains the header of the last TCP packet sent by the client during TCP + handshake. diff --git a/src/connection.c b/src/connection.c index 58bf4a5f85f5..0f8acb02dbdb 100644 --- a/src/connection.c +++ b/src/connection.c @@ -678,14 +678,8 @@ int conn_recv_proxy(struct connection *conn, int flag) } /* This handshake handler waits a NetScaler Client IP insertion header - * at the beginning of the raw data stream. The header looks like this: - * - * 4 bytes: CIP magic number - * 4 bytes: Header length - * 20+ bytes: Header of the last IP packet sent by the client during - * TCP handshake. - * 20+ bytes: Header of the last TCP packet sent by the client during - * TCP handshake. + * at the beginning of the raw data stream. The header format is + * described in doc/netscaler-client-ip-insertion-protocol.txt * * This line MUST be at the beginning of the buffer and MUST NOT be * fragmented. @@ -735,25 +729,39 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) } /* Fail if buffer length is not large enough to contain - * CIP magic, CIP length */ - if (trash.len < 8) + * CIP magic, header length or + * CIP magic, CIP length, CIP type, header length */ + if (trash.len < 12) goto missing; line = trash.str; - hdr_len = ntohl(*(uint32_t *)(line+4)); /* Decode a possible NetScaler Client IP request, fail early if * it does not match */ if (ntohl(*(uint32_t *)line) != objt_listener(conn->target)->bind_conf->ns_cip_magic) goto bad_magic; + /* Legacy CIP protocol */ + if ((trash.str[8] & 0xD0) == 0x40) { + hdr_len = ntohl(*(uint32_t *)(line+4)); + line += 8; + } + /* Standard CIP protocol */ + else if (trash.str[8] == 0x00) { + hdr_len = ntohs(*(uint32_t *)(line+10)); + line += 12; + } + /* Unknown CIP protocol */ + else { + conn->err_code = CO_ER_CIP_BAD_PROTO; + goto fail; + } + /* Fail if buffer length is not large enough to contain - * CIP magic, CIP length, minimal IP header */ - if (trash.len < 28) + * a minimal IP header */ + if (trash.len < 20) goto missing; - line += 8; - /* Get IP version from the first four bits */ ip_v = (*line & 0xf0) >> 4;
From 39b9d87524f981e3a5eb2504653fdc814b2b344a Mon Sep 17 00:00:00 2001 From: Bertrand Jacquin <[email protected]> Date: Wed, 13 Dec 2017 01:29:56 +0000 Subject: [PATCH 7/8] MEDIUM: netscaler: do not analyze original IP packet size Original informations about the client are stored in the CIP encapsulated IP header, hence there is no need to consider original IP packet length to determine if data are missing. Instead this change detect missing data if the remaining buffer is large enough to contain a minimal IP and TCP header and if the buffer has as much data as CIP is telling. --- src/connection.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/connection.c b/src/connection.c index 8d2fb77bedeb..58bf4a5f85f5 100644 --- a/src/connection.c +++ b/src/connection.c @@ -763,7 +763,7 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) hdr_ip4 = (struct ip *)line; - if (trash.len < (ntohs(hdr_ip4->ip_len) + 20)) { + if (trash.len < 40 || trash.len < hdr_len) { /* Fail if buffer length is not large enough to contain * IPv4 header, TCP header */ goto missing; @@ -793,7 +793,7 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) hdr_ip6 = (struct ip6_hdr *)line; - if (trash.len < 60) { + if (trash.len < 60 || trash.len < hdr_len) { /* Fail if buffer length is not large enough to contain * IPv6 header, TCP header */ goto missing;
From 2d5a55db9e73ec135745a583a92e646a828a6d41 Mon Sep 17 00:00:00 2001 From: Bertrand Jacquin <[email protected]> Date: Wed, 13 Dec 2017 01:15:05 +0000 Subject: [PATCH 6/8] MINOR: netscaler: check in one-shot if buffer is large enough for IP and TCP header There is minimal gain in checking first the IP header length and then the TCP header length since we always want to capture information about both protocols. IPv4 length calculation was incorrect since IPv4 ip_len actually defines the total length of IPv4 header and following data. --- src/connection.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/src/connection.c b/src/connection.c index e716e8046788..8d2fb77bedeb 100644 --- a/src/connection.c +++ b/src/connection.c @@ -763,21 +763,16 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) hdr_ip4 = (struct ip *)line; - if (trash.len < ntohs(hdr_ip4->ip_len)) { - /* Fail if buffer length is not large enough to contain - * IPv4 header */ - goto missing; - } - else if (hdr_ip4->ip_p != IPPROTO_TCP) { - /* The protocol does not include a TCP header */ - conn->err_code = CO_ER_CIP_BAD_PROTO; - goto fail; - } - else if (trash.len < (20 + ntohs(hdr_ip4->ip_len))) { + if (trash.len < (ntohs(hdr_ip4->ip_len) + 20)) { /* Fail if buffer length is not large enough to contain * IPv4 header, TCP header */ goto missing; } + else if (hdr_ip4->ip_p != IPPROTO_TCP) { + /* The protocol does not include a TCP header */ + conn->err_code = CO_ER_CIP_BAD_PROTO; + goto fail; + } hdr_tcp = (struct my_tcphdr *)(line + (hdr_ip4->ip_hl * 4)); @@ -798,21 +793,16 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) hdr_ip6 = (struct ip6_hdr *)line; - if (trash.len < 40) { - /* Fail if buffer length is not large enough to contain - * IPv6 header */ - goto missing; - } - else if (hdr_ip6->ip6_nxt != IPPROTO_TCP) { - /* The protocol does not include a TCP header */ - conn->err_code = CO_ER_CIP_BAD_PROTO; - goto fail; - } - else if (trash.len < 60) { + if (trash.len < 60) { /* Fail if buffer length is not large enough to contain * IPv6 header, TCP header */ goto missing; } + else if (hdr_ip6->ip6_nxt != IPPROTO_TCP) { + /* The protocol does not include a TCP header */ + conn->err_code = CO_ER_CIP_BAD_PROTO; + goto fail; + } hdr_tcp = (struct my_tcphdr *)(line + sizeof(struct ip6_hdr));
From bdc16a0a1a4e88a62ce6156ea966e4c8f486cbac Mon Sep 17 00:00:00 2001 From: Bertrand Jacquin <[email protected]> Date: Wed, 13 Dec 2017 00:53:33 +0000 Subject: [PATCH 5/8] BUG/MAJOR: netscaler: address truncated CIP header detection Buffer line is manually incremented in order to progress in the trash buffer but calculation are made omitting this manual offset. This leads to random packets being rejected with the following error: HTTP/1: Truncated NetScaler Client IP header received Instead, once original IP header is found, use the IP header length without considering the CIP encapsulation. --- src/connection.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/connection.c b/src/connection.c index c06babd92641..e716e8046788 100644 --- a/src/connection.c +++ b/src/connection.c @@ -763,9 +763,9 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) hdr_ip4 = (struct ip *)line; - if (trash.len < (8 + ntohs(hdr_ip4->ip_len))) { + if (trash.len < ntohs(hdr_ip4->ip_len)) { /* Fail if buffer length is not large enough to contain - * CIP magic, CIP length, IPv4 header */ + * IPv4 header */ goto missing; } else if (hdr_ip4->ip_p != IPPROTO_TCP) { @@ -773,9 +773,9 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) conn->err_code = CO_ER_CIP_BAD_PROTO; goto fail; } - else if (trash.len < (28 + ntohs(hdr_ip4->ip_len))) { + else if (trash.len < (20 + ntohs(hdr_ip4->ip_len))) { /* Fail if buffer length is not large enough to contain - * CIP magic, CIP length, IPv4 header, TCP header */ + * IPv4 header, TCP header */ goto missing; } @@ -798,9 +798,9 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) hdr_ip6 = (struct ip6_hdr *)line; - if (trash.len < 48) { + if (trash.len < 40) { /* Fail if buffer length is not large enough to contain - * CIP magic, CIP length, IPv6 header */ + * IPv6 header */ goto missing; } else if (hdr_ip6->ip6_nxt != IPPROTO_TCP) { @@ -808,9 +808,9 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) conn->err_code = CO_ER_CIP_BAD_PROTO; goto fail; } - else if (trash.len < 68) { + else if (trash.len < 60) { /* Fail if buffer length is not large enough to contain - * CIP magic, CIP length, IPv6 header, TCP header */ + * IPv6 header, TCP header */ goto missing; }
From c7c51e42ef61c7894e8cb7d6a857bc5e1d1e2de3 Mon Sep 17 00:00:00 2001 From: Bertrand Jacquin <[email protected]> Date: Wed, 13 Dec 2017 01:40:24 +0000 Subject: [PATCH 4/8] BUG/MEDIUM: netscaler: use the appropriate IPv6 header size IPv6 header has a fixed size of 40 bytes, not 20. --- src/connection.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/connection.c b/src/connection.c index a47d4d1fdb77..c06babd92641 100644 --- a/src/connection.c +++ b/src/connection.c @@ -798,7 +798,7 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) hdr_ip6 = (struct ip6_hdr *)line; - if (trash.len < 28) { + if (trash.len < 48) { /* Fail if buffer length is not large enough to contain * CIP magic, CIP length, IPv6 header */ goto missing; @@ -808,7 +808,7 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) conn->err_code = CO_ER_CIP_BAD_PROTO; goto fail; } - else if (trash.len < 48) { + else if (trash.len < 68) { /* Fail if buffer length is not large enough to contain * CIP magic, CIP length, IPv6 header, TCP header */ goto missing;
From 2062c79e2e9887470b7c3fba1cd5e8ba9d70c37e Mon Sep 17 00:00:00 2001 From: Bertrand Jacquin <[email protected]> Date: Wed, 13 Dec 2017 01:23:39 +0000 Subject: [PATCH 3/8] MINOR: netscaler: rename cip_len to clarify its uage cip_len was meant to be the length of the data encapsulated in the CIP protocol, the size the IP and TCP header --- src/connection.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/connection.c b/src/connection.c index e0b740ff92c5..a47d4d1fdb77 100644 --- a/src/connection.c +++ b/src/connection.c @@ -702,7 +702,7 @@ int conn_recv_proxy(struct connection *conn, int flag) int conn_recv_netscaler_cip(struct connection *conn, int flag) { char *line; - uint32_t cip_len; + uint32_t hdr_len; uint8_t ip_v; /* we might have been called just after an asynchronous shutr */ @@ -740,7 +740,7 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) goto missing; line = trash.str; - cip_len = ntohl(*(uint32_t *)(line+4)); + hdr_len = ntohl(*(uint32_t *)(line+4)); /* Decode a possible NetScaler Client IP request, fail early if * it does not match */ @@ -833,7 +833,7 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) goto fail; } - line += cip_len; + line += hdr_len; trash.len = line - trash.str; /* remove the NetScaler Client IP header from the request. For this
From 97a2067d90d7fed0c054b6d4c23d6481bec338c1 Mon Sep 17 00:00:00 2001 From: Bertrand Jacquin <[email protected]> Date: Wed, 13 Dec 2017 01:07:12 +0000 Subject: [PATCH 2/8] MINOR: netscaler: remove the use of cip_magic only used once --- src/connection.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/connection.c b/src/connection.c index 8637895a355f..e0b740ff92c5 100644 --- a/src/connection.c +++ b/src/connection.c @@ -702,7 +702,6 @@ int conn_recv_proxy(struct connection *conn, int flag) int conn_recv_netscaler_cip(struct connection *conn, int flag) { char *line; - uint32_t cip_magic; uint32_t cip_len; uint8_t ip_v; @@ -741,13 +740,11 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) goto missing; line = trash.str; - - cip_magic = ntohl(*(uint32_t *)line); cip_len = ntohl(*(uint32_t *)(line+4)); /* Decode a possible NetScaler Client IP request, fail early if * it does not match */ - if (cip_magic != objt_listener(conn->target)->bind_conf->ns_cip_magic) + if (ntohl(*(uint32_t *)line) != objt_listener(conn->target)->bind_conf->ns_cip_magic) goto bad_magic; /* Fail if buffer length is not large enough to contain
From caa3b184f2fe088e71dcd31001638f34727281fd Mon Sep 17 00:00:00 2001 From: Bertrand Jacquin <[email protected]> Date: Wed, 13 Dec 2017 00:58:51 +0000 Subject: [PATCH 1/8] MINOR: netscaler: respect syntax As per doc/coding-style.txt --- src/connection.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/connection.c b/src/connection.c index f8a50c3fe87d..8637895a355f 100644 --- a/src/connection.c +++ b/src/connection.c @@ -770,11 +770,13 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) /* Fail if buffer length is not large enough to contain * CIP magic, CIP length, IPv4 header */ goto missing; - } else if (hdr_ip4->ip_p != IPPROTO_TCP) { + } + else if (hdr_ip4->ip_p != IPPROTO_TCP) { /* The protocol does not include a TCP header */ conn->err_code = CO_ER_CIP_BAD_PROTO; goto fail; - } else if (trash.len < (28 + ntohs(hdr_ip4->ip_len))) { + } + else if (trash.len < (28 + ntohs(hdr_ip4->ip_len))) { /* Fail if buffer length is not large enough to contain * CIP magic, CIP length, IPv4 header, TCP header */ goto missing; @@ -803,11 +805,13 @@ int conn_recv_netscaler_cip(struct connection *conn, int flag) /* Fail if buffer length is not large enough to contain * CIP magic, CIP length, IPv6 header */ goto missing; - } else if (hdr_ip6->ip6_nxt != IPPROTO_TCP) { + } + else if (hdr_ip6->ip6_nxt != IPPROTO_TCP) { /* The protocol does not include a TCP header */ conn->err_code = CO_ER_CIP_BAD_PROTO; goto fail; - } else if (trash.len < 48) { + } + else if (trash.len < 48) { /* Fail if buffer length is not large enough to contain * CIP magic, CIP length, IPv6 header, TCP header */ goto missing;
signature.asc
Description: OpenPGP digital signature
Amazon Data Services Ireland Limited registered office: One Burlington Plaza, Burlington Road, Dublin 4, Ireland. Registered in Ireland. Registration number 390566.
