diff --git a/reg-tests/connection/proxy_protocol_tlv_validation.vtc b/reg-tests/connection/proxy_protocol_tlv_validation.vtc new file mode 100644 index 000000000..65a251cb8 --- /dev/null +++ b/reg-tests/connection/proxy_protocol_tlv_validation.vtc @@ -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 diff --git a/src/connection.c b/src/connection.c index a6e2b3468..1ea8e9f2a 100644 --- a/src/connection.c +++ b/src/connection.c @@ -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: