RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2025-01-14 17:50:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

BUG/MAJOR: proxy_protocol: Properly validate TLV lengths

Browse Source 
This patch fixes PROXYv2 parsing when the payload of the TCP connection is
fused with the PROXYv2 header within a single recv() call.

Previously HAProxy ignored the PROXYv2 header length when attempting to
parse the TLV, possibly interpreting the first byte of the payload as a
TLV type.

This patch adds proper validation. It ensures that:

1. TLV parsing stops when the end of the PROXYv2 header is reached.
2. TLV lengths cannot exceed the PROXYv2 header length.
3. The PROXYv2 header ends together with the last TLV, not allowing for
   "stray bytes" to be ignored.

A reg-test was added to ensure proper behavior.

This patch tries to find the sweat spot between a small and easily
backportable one, and a cleaner one that's more easily adaptable to
older versions, hence why it merges the "if" and "while" blocks which
causes a reindent of the whole block. It should be used as-is for
versions 1.9 to 2.1, the block about PP2_TYPE_AUTHORITY should be
dropped for 2.0 and the block about CRC32C should be dropped for 1.8.

This bug was introduced when TLV parsing was added. This happened in commit
b3e54fe387. This commit was first released
with HAProxy 1.6-dev1.

A similar issue was fixed in commit 7209c204bd.

This patch must be backported to HAProxy 1.6+.
This commit is contained in:
Tim Duesterhus 2020-03-05 22:55:20 +01:00 committed by Willy Tarreau
parent b1beaa302c
commit 488ee7fb6e
2 changed files with 200 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

138
reg-tests/connection/proxy_protocol_tlv_validation.vtc Normal file
View File

@ -0,0 +1,138 @@
varnishtest "Check that the TLVs are properly validated"
feature ignore_unknown_macro
# We need one HAProxy for each test, because apparently the connection by
# the client is reused, leading to connection resets.
haproxy h1 -conf {
defaults
mode http
timeout connect 1s
timeout client 1s
timeout server 1s
frontend a
bind "fd@${fe1}" accept-proxy
http-after-response set-header echo %[fc_pp_authority,hex]
http-request return status 200
} -start
# Validate that a correct header passes
client c1 -connect ${h1_fe1_sock} {
# PROXY v2 signature
sendhex "0d 0a 0d 0a 00 0d 0a 51 55 49 54 0a"
# version + PROXY
sendhex "21"
# TCP4
sendhex "11"
# length of the address (12) + length of the TLV (8)
sendhex "00 14"
# 127.0.0.1 42 127.0.0.1 1337
sendhex "7F 00 00 01 7F 00 00 01 00 2A 05 39"
# PP2_TYPE_AUTHORITY + length of the value + "12345"
sendhex "02 00 05 31 32 33 34 35"
txreq -url "/"
rxresp
expect resp.http.echo == "3132333435"
} -run
haproxy h2 -conf {
defaults
mode http
timeout connect 1s
timeout client 1s
timeout server 1s
frontend a
bind "fd@${fe1}" accept-proxy
http-after-response set-header echo %[fc_pp_authority,hex]
http-request return status 200
} -start
# Validate that a TLV after the end of the PROXYv2 header is ignored
client c2 -connect ${h2_fe1_sock} {
# PROXY v2 signature
sendhex "0d 0a 0d 0a 00 0d 0a 51 55 49 54 0a"
# version + PROXY
sendhex "21"
# TCP4
sendhex "11"
# length of the address (12) + length of the TLV (8)
sendhex "00 14"
# 127.0.0.1 42 127.0.0.1 1337
sendhex "7F 00 00 01 7F 00 00 01 00 2A 05 39"
# PP2_TYPE_AUTHORITY + length of the value + "12345"
sendhex "02 00 05 31 32 33 34 35"
# after the end of the PROXYv2 header: PP2_TYPE_AUTHORITY + length of the value + "54321"
sendhex "02 00 05 35 34 33 32 31"
txreq -url "/"
rxresp
expect resp.http.echo == "3132333435"
} -run
haproxy h3 -conf {
defaults
mode http
timeout connect 1s
timeout client 1s
timeout server 1s
frontend a
bind "fd@${fe1}" accept-proxy
http-after-response set-header echo %[fc_pp_authority,hex]
http-request return status 200
} -start
# Validate that a TLV length exceeding the PROXYv2 length fails
client c3 -connect ${h3_fe1_sock} {
# PROXY v2 signature
sendhex "0d 0a 0d 0a 00 0d 0a 51 55 49 54 0a"
# version + PROXY
sendhex "21"
# TCP4
sendhex "11"
# length of the address (12) + too small length of the TLV (8)
sendhex "00 14"
# 127.0.0.1 42 127.0.0.1 1337
sendhex "7F 00 00 01 7F 00 00 01 00 2A 05 39"
# PP2_TYPE_AUTHORITY + length of the value + "1234512345"
sendhex "02 00 0A 31 32 33 34 35 31 32 33 34 35"
txreq -url "/"
expect_close
} -run
haproxy h4 -conf {
defaults
mode http
timeout connect 1s
timeout client 1s
timeout server 1s
frontend a
bind "fd@${fe1}" accept-proxy
http-after-response set-header echo %[fc_pp_authority,hex]
http-request return status 200
} -start
# Validate that TLVs not ending with the PROXYv2 header fail
client c4 -connect ${h4_fe1_sock} {
# PROXY v2 signature
sendhex "0d 0a 0d 0a 00 0d 0a 51 55 49 54 0a"
# version + PROXY
sendhex "21"
# TCP4
sendhex "11"
# length of the address (12) + too big length of the TLV (8)
sendhex "00 14"
# 127.0.0.1 42 127.0.0.1 1337
sendhex "7F 00 00 01 7F 00 00 01 00 2A 05 39"
# PP2_TYPE_AUTHORITY + length of the value + "1234"
sendhex "02 00 04 31 32 33 34"
txreq -url "/"
expect_close
} -run

101
src/connection.c
View File

@ -490,7 +490,7 @@ int conn_recv_proxy(struct connection *conn, int flag)
char *line, *end;
struct proxy_hdr_v2 *hdr_v2;
const char v2sig[] = PP2_SIGNATURE;
int tlv_length = 0;
size_t total_v2_len;
int tlv_offset = 0;
int ret;
@ -668,7 +668,8 @@ int conn_recv_proxy(struct connection *conn, int flag)
goto fail;
}
if (trash.data < PP2_HEADER_LEN + ntohs(hdr_v2->len))
total_v2_len = PP2_HEADER_LEN + ntohs(hdr_v2->len);
if (trash.data < total_v2_len)
goto missing;
switch (hdr_v2->ver_cmd & PP2_CMD_MASK) {
@ -686,7 +687,6 @@ int conn_recv_proxy(struct connection *conn, int flag)
((struct sockaddr_in *)conn->dst)->sin_port = hdr_v2->addr.ip4.dst_port;
conn->flags |= CO_FL_ADDR_FROM_SET | CO_FL_ADDR_TO_SET;
tlv_offset = PP2_HEADER_LEN + PP2_ADDR_LEN_INET;
tlv_length = ntohs(hdr_v2->len) - PP2_ADDR_LEN_INET;
break;
case 0x21: /* TCPv6 */
if (ntohs(hdr_v2->len) < PP2_ADDR_LEN_INET6)
@ -700,51 +700,74 @@ int conn_recv_proxy(struct connection *conn, int flag)
((struct sockaddr_in6 *)conn->dst)->sin6_port = hdr_v2->addr.ip6.dst_port;
conn->flags |= CO_FL_ADDR_FROM_SET | CO_FL_ADDR_TO_SET;
tlv_offset = PP2_HEADER_LEN + PP2_ADDR_LEN_INET6;
tlv_length = ntohs(hdr_v2->len) - PP2_ADDR_LEN_INET6;
break;
}
/* TLV parsing */
if (tlv_length > 0) {
while (tlv_offset + TLV_HEADER_SIZE <= trash.data) {
const struct tlv *tlv_packet = (struct tlv *) &trash.area[tlv_offset];
const int tlv_len = get_tlv_length(tlv_packet);
tlv_offset += tlv_len + TLV_HEADER_SIZE;
while (tlv_offset < total_v2_len) {
struct tlv *tlv_packet;
int tlv_len;
switch (tlv_packet->type) {
case PP2_TYPE_CRC32C: {
void *tlv_crc32c_p = (void *)tlv_packet->value;
uint32_t n_crc32c = ntohl(read_u32(tlv_crc32c_p));
write_u32(tlv_crc32c_p, 0);
if (hash_crc32c(trash.area, PP2_HEADER_LEN + ntohs(hdr_v2->len)) != n_crc32c)
goto bad_header;
break;
}
/* Verify that we have at least TLV_HEADER_SIZE bytes left */
if (tlv_offset + TLV_HEADER_SIZE > total_v2_len)
goto bad_header;
tlv_packet = (struct tlv *) &trash.area[tlv_offset];
tlv_len = get_tlv_length(tlv_packet);
tlv_offset += tlv_len + TLV_HEADER_SIZE;
/* Verify that the TLV length does not exceed the total PROXYv2 length */
if (tlv_offset > total_v2_len)
goto bad_header;
switch (tlv_packet->type) {
case PP2_TYPE_CRC32C: {
uint32_t n_crc32c;
/* Verify that this TLV is exactly 4 bytes long */
if (tlv_len != 4)
goto bad_header;
n_crc32c = read_n32(tlv_packet->value);
write_n32(tlv_packet->value, 0); // compute with CRC==0
if (hash_crc32c(trash.area, total_v2_len) != n_crc32c)
goto bad_header;
break;
}
#ifdef USE_NS
case PP2_TYPE_NETNS: {
const struct netns_entry *ns;
ns = netns_store_lookup((char*)tlv_packet->value, tlv_len);
if (ns)
conn->proxy_netns = ns;
break;
}
case PP2_TYPE_NETNS: {
const struct netns_entry *ns;
ns = netns_store_lookup((char*)tlv_packet->value, tlv_len);
if (ns)
conn->proxy_netns = ns;
break;
}
#endif
case PP2_TYPE_AUTHORITY: {
if (tlv_len > PP2_AUTHORITY_MAX)
goto bad_header;
conn->proxy_authority = pool_alloc(pool_head_authority);
if (conn->proxy_authority == NULL)
goto fail;
memcpy(conn->proxy_authority, (const char *)tlv_packet->value, tlv_len);
conn->proxy_authority_len = tlv_len;
break;
}
default:
break;
}
case PP2_TYPE_AUTHORITY: {
if (tlv_len > PP2_AUTHORITY_MAX)
goto bad_header;
conn->proxy_authority = pool_alloc(pool_head_authority);
if (conn->proxy_authority == NULL)
goto fail;
memcpy(conn->proxy_authority, (const char *)tlv_packet->value, tlv_len);
conn->proxy_authority_len = tlv_len;
break;
}
default:
break;
}
}
/* Verify that the PROXYv2 header ends at a TLV boundary.
* This is technically unreachable, because the TLV parsing already
* verifies that a TLV does not exceed the total length and also
* that there is space for a TLV header.
*/
if (tlv_offset != total_v2_len)
goto bad_header;
/* unsupported protocol, keep local connection address */
break;
case 0x00: /* LOCAL command */
@ -754,7 +777,7 @@ int conn_recv_proxy(struct connection *conn, int flag)
goto bad_header; /* not a supported command */
}
trash.data = PP2_HEADER_LEN + ntohs(hdr_v2->len);
trash.data = total_v2_len;
goto eat_header;
eat_header: