BUG/MINOR: connection: parse PROXY TLV for LOCAL mode

conn_recv_proxy() is responsible to parse PROXY protocol header. For v2
of the protocol, TLVs parsing is implemented. However, this step was
only done inside 'PROXY' command label. TLVs were never extracted for
'LOCAL' command mode.

Fix this by extracting TLV parsing loop outside of the switch case. Of
notable importance, tlv_offset is updated on LOCAL label to point to
first TLV location.

This bug should be backported up to 2.9 at least. It should even
probably be backported to every stable versions. Note however that this
code has changed much over time. It may be useful to use option
'--ignore-all-space' to have a clearer overview of the git diff.

diff --git a/src/connection.c b/src/connection.c
index 46526b141f0fdf6bc56e3797acf50caaf8b3f861..97378632c33a3bbfc66d46227d58087624eb1d42 100644 (file)
--- a/src/connection.c
+++ b/src/connection.c
@@ -1129,111 +1129,112 @@ int conn_recv_proxy(struct connection *conn, int flag)
                        break;
                }
 
-               /* TLV parsing */
-               while (tlv_offset < total_v2_len) {
-                       struct ist tlv;
-                       struct tlv *tlv_packet = NULL;
-                       struct conn_tlv_list *new_tlv = NULL;
-                       size_t data_len = 0;
-
-                       /* Verify that we have at least TLV_HEADER_SIZE bytes left */
-                       if (tlv_offset + TLV_HEADER_SIZE > total_v2_len)
-                               goto bad_header;
+               /* unsupported protocol, keep local connection address */
+               break;
+       case 0x00: /* LOCAL command */
+               /* keep local connection address for LOCAL */
 
-                       tlv_packet = (struct tlv *) &trash.area[tlv_offset];
-                       tlv = ist2((const char *)tlv_packet->value, get_tlv_length(tlv_packet));
-                       tlv_offset += istlen(tlv) + TLV_HEADER_SIZE;
+               tlv_offset = PP2_HEADER_LEN;
+               break;
+       default:
+               goto bad_header; /* not a supported command */
+       }
 
-                       /* Verify that the TLV length does not exceed the total PROXYv2 length */
-                       if (tlv_offset > total_v2_len)
-                               goto bad_header;
+       /* TLV parsing */
+       while (tlv_offset < total_v2_len) {
+               struct ist tlv;
+               struct tlv *tlv_packet = NULL;
+               struct conn_tlv_list *new_tlv = NULL;
+               size_t data_len = 0;
 
-                       /* Prepare known TLV types */
-                       switch (tlv_packet->type) {
-                       case PP2_TYPE_CRC32C: {
-                               uint32_t n_crc32c;
+               /* Verify that we have at least TLV_HEADER_SIZE bytes left */
+               if (tlv_offset + TLV_HEADER_SIZE > total_v2_len)
+                       goto bad_header;
 
-                               /* Verify that this TLV is exactly 4 bytes long */
-                               if (istlen(tlv) != PP2_CRC32C_LEN)
-                                       goto bad_header;
+               tlv_packet = (struct tlv *) &trash.area[tlv_offset];
+               tlv = ist2((const char *)tlv_packet->value, get_tlv_length(tlv_packet));
+               tlv_offset += istlen(tlv) + TLV_HEADER_SIZE;
 
-                               n_crc32c = read_n32(istptr(tlv));
-                               write_n32(istptr(tlv), 0); // compute with CRC==0
+               /* Verify that the TLV length does not exceed the total PROXYv2 length */
+               if (tlv_offset > total_v2_len)
+                       goto bad_header;
 
-                               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;
+               /* Prepare known TLV types */
+               switch (tlv_packet->type) {
+               case PP2_TYPE_CRC32C: {
+                       uint32_t n_crc32c;
 
-                               ns = netns_store_lookup(istptr(tlv), istlen(tlv));
-                               if (ns)
-                                       conn->proxy_netns = ns;
-                               break;
-                       }
-#endif
-                       case PP2_TYPE_AUTHORITY: {
-                               /* For now, keep the length restriction by HAProxy */
-                               if (istlen(tlv) > HA_PP2_AUTHORITY_MAX)
-                                       goto bad_header;
+                       /* Verify that this TLV is exactly 4 bytes long */
+                       if (istlen(tlv) != PP2_CRC32C_LEN)
+                               goto bad_header;
 
-                               break;
-                       }
-                       case PP2_TYPE_UNIQUE_ID: {
-                               if (istlen(tlv) > UNIQUEID_LEN)
-                                       goto bad_header;
-                               break;
-                       }
-                       default:
-                               break;
-                       }
+                       n_crc32c = read_n32(istptr(tlv));
+                       write_n32(istptr(tlv), 0); // compute with CRC==0
 
-                       /* If we did not find a known TLV type that we can optimize for, we generically allocate it */
-                       data_len = get_tlv_length(tlv_packet);
+                       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;
 
-                       /* Prevent attackers from allocating too much memory */
-                       if (unlikely(data_len > HA_PP2_MAX_ALLOC))
-                               goto fail;
+                       ns = netns_store_lookup(istptr(tlv), istlen(tlv));
+                       if (ns)
+                               conn->proxy_netns = ns;
+                       break;
+               }
+#endif
+               case PP2_TYPE_AUTHORITY: {
+                       /* For now, keep the length restriction by HAProxy */
+                       if (istlen(tlv) > HA_PP2_AUTHORITY_MAX)
+                               goto bad_header;
 
-                       /* Alloc memory based on data_len */
-                       if (data_len > HA_PP2_TLV_VALUE_256)
-                               new_tlv = malloc(get_tlv_length(tlv_packet) + sizeof(struct conn_tlv_list));
-                       else if (data_len <= HA_PP2_TLV_VALUE_128)
-                               new_tlv = pool_alloc(pool_head_pp_tlv_128);
-                       else
-                               new_tlv = pool_alloc(pool_head_pp_tlv_256);
+                       break;
+               }
+               case PP2_TYPE_UNIQUE_ID: {
+                       if (istlen(tlv) > UNIQUEID_LEN)
+                               goto bad_header;
+                       break;
+               }
+               default:
+                       break;
+               }
 
-                       if (unlikely(!new_tlv))
-                               goto fail;
+               /* If we did not find a known TLV type that we can optimize for, we generically allocate it */
+               data_len = get_tlv_length(tlv_packet);
 
-                       new_tlv->type = tlv_packet->type;
+               /* Prevent attackers from allocating too much memory */
+               if (unlikely(data_len > HA_PP2_MAX_ALLOC))
+                       goto fail;
 
-                       /* Save TLV to make it accessible via sample fetch */
-                       memcpy(new_tlv->value, tlv.ptr, data_len);
-                       new_tlv->len = data_len;
+               /* Alloc memory based on data_len */
+               if (data_len > HA_PP2_TLV_VALUE_256)
+                       new_tlv = malloc(get_tlv_length(tlv_packet) + sizeof(struct conn_tlv_list));
+               else if (data_len <= HA_PP2_TLV_VALUE_128)
+                       new_tlv = pool_alloc(pool_head_pp_tlv_128);
+               else
+                       new_tlv = pool_alloc(pool_head_pp_tlv_256);
 
-                       LIST_APPEND(&conn->tlv_list, &new_tlv->list);
-               }
+               if (unlikely(!new_tlv))
+                       goto fail;
 
+               new_tlv->type = tlv_packet->type;
 
-               /* Verify that the PROXYv2 header ends at a TLV boundary.
-                * This is can not be true, 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.
-                */
-               BUG_ON(tlv_offset != total_v2_len);
+               /* Save TLV to make it accessible via sample fetch */
+               memcpy(new_tlv->value, tlv.ptr, data_len);
+               new_tlv->len = data_len;
 
-               /* unsupported protocol, keep local connection address */
-               break;
-       case 0x00: /* LOCAL command */
-               /* keep local connection address for LOCAL */
-               break;
-       default:
-               goto bad_header; /* not a supported command */
+               LIST_APPEND(&conn->tlv_list, &new_tlv->list);
        }
 
+       /* Verify that the PROXYv2 header ends at a TLV boundary.
+        * This is can not be true, 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.
+        */
+       BUG_ON(tlv_offset != total_v2_len);
+
        trash.data = total_v2_len;
        goto eat_header;