ligang/vpp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

igmp: accept packets that have more on the wire data than IGMP reports.

Browse Source 
IGMPv3 sends a variable length of sources in a query. Today if the
amount of data on the wire does not exactly match that required for the
number of sources the packet is dropped.
Relax this check and instead accept the packet is the amount of wire
data is equal or greater than the number of sources.
Some devices on the wild internet pad small packets.

Type: feature

Change-Id: I102682814b38c0a0614d71816c9a286d90b834df
Signed-off-by: Neale Ranns <nranns@cisco.com>
This commit is contained in:
Neale Ranns
2019-06-30 09:05:05 +00:00
parent 1671d3be38
commit 01b0a05e4f
2 changed files with 38 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
src/plugins/igmp/igmp_input.c
View File

@ -276,6 +276,7 @@ igmp_parse_query (vlib_main_t * vm, vlib_node_runtime_t * node,
b = vlib_get_buffer (vm, bi); b = vlib_get_buffer (vm, bi);
igmp = vlib_buffer_get_current (b); igmp = vlib_buffer_get_current (b);
ASSERT (igmp->header.type == IGMP_TYPE_membership_query); ASSERT (igmp->header.type == IGMP_TYPE_membership_query);
len = igmp_membership_query_v3_length (igmp);
if (node->flags & VLIB_NODE_FLAG_TRACE) if (node->flags & VLIB_NODE_FLAG_TRACE)
{ {
@ -283,17 +284,17 @@ igmp_parse_query (vlib_main_t * vm, vlib_node_runtime_t * node,
tr = vlib_add_trace (vm, node, b, sizeof (*tr)); tr = vlib_add_trace (vm, node, b, sizeof (*tr));
tr->next_index = next; tr->next_index = next;
tr->sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX]; tr->sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX];
tr->len = vlib_buffer_length_in_chain (vm, b); tr->len = len;
clib_memcpy_fast (tr->packet_data, vlib_buffer_get_current (b), clib_memcpy_fast (tr->packet_data, vlib_buffer_get_current (b),
sizeof (tr->packet_data)); sizeof (tr->packet_data));
} }
len = igmp_membership_query_v3_length (igmp);
/* /*
* validate that the length on the packet on the wire * validate that the length on the packet on the wire corresponds
* corresponds to the length on the calculated v3 query * to at least the length of the calculated v3 query.
* If there's extra, then it will be ignored.
*/ */
if (vlib_buffer_length_in_chain (vm, b) == len) if (vlib_buffer_length_in_chain (vm, b) >= len)
{ {
/* /*
* copy the contents of the query, and the interface, over * copy the contents of the query, and the interface, over
@ -309,8 +310,8 @@ igmp_parse_query (vlib_main_t * vm, vlib_node_runtime_t * node,
else else
{ {
/* /*
* else a packet that is reporting more or less sources * else a packet that is reporting more sources than it really
* than it really has, bin it * has; bin it
*/ */
b->error = node->errors[IGMP_ERROR_BAD_LENGTH]; b->error = node->errors[IGMP_ERROR_BAD_LENGTH];
} }
@ -394,6 +395,7 @@ igmp_parse_report (vlib_main_t * vm, vlib_node_runtime_t * node,
igmp_input_trace_t *tr; igmp_input_trace_t *tr;
tr = vlib_add_trace (vm, node, b, sizeof (*tr)); tr = vlib_add_trace (vm, node, b, sizeof (*tr));
tr->next_index = next; tr->next_index = next;
tr->len = len;
tr->sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX]; tr->sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX];
clib_memcpy_fast (tr->packet_data, vlib_buffer_get_current (b), clib_memcpy_fast (tr->packet_data, vlib_buffer_get_current (b),
sizeof (tr->packet_data)); sizeof (tr->packet_data));
@ -403,7 +405,7 @@ igmp_parse_report (vlib_main_t * vm, vlib_node_runtime_t * node,
* validate that the length on the packet on the wire * validate that the length on the packet on the wire
* corresponds to the length on the calculated v3 query * corresponds to the length on the calculated v3 query
*/ */
if (vlib_buffer_length_in_chain (vm, b) == len) if (vlib_buffer_length_in_chain (vm, b) >= len)
{ {
/* /*
* copy the contents of the query, and the interface, over * copy the contents of the query, and the interface, over
@ -416,11 +418,15 @@ igmp_parse_report (vlib_main_t * vm, vlib_node_runtime_t * node,
vl_api_rpc_call_main_thread (igmp_handle_report, vl_api_rpc_call_main_thread (igmp_handle_report,
(u8 *) args, sizeof (*args) + len); (u8 *) args, sizeof (*args) + len);
} }
/* else
* else {
* this is a packet with more groups/sources than the /*
* header reports. bin it * this is a packet with more groups/sources than the
*/ * header reports. bin it
*/
b->error = node->errors[IGMP_ERROR_BAD_LENGTH];
}
vlib_validate_buffer_enqueue_x1 (vm, node, next_index, to_next, vlib_validate_buffer_enqueue_x1 (vm, node, next_index, to_next,
n_left_to_next, bi, next); n_left_to_next, bi, next);
} }

22
test/test_igmp.py
View File

@ -2,7 +2,7 @@
import unittest import unittest
from scapy.layers.l2 import Ether from scapy.layers.l2 import Ether, Raw
from scapy.layers.inet import IP, IPOption from scapy.layers.inet import IP, IPOption
from scapy.contrib.igmpv3 import IGMPv3, IGMPv3gr, IGMPv3mq, IGMPv3mr from scapy.contrib.igmpv3 import IGMPv3, IGMPv3gr, IGMPv3mq, IGMPv3mr
@ -194,12 +194,15 @@ class TestIgmp(VppTestCase):
# #
# Send a general query (to the all router's address) # Send a general query (to the all router's address)
# expect VPP to respond with a membership report # expect VPP to respond with a membership report.
# Pad the query with 0 - some devices in the big wild
# internet are prone to this.
# #
p_g = (Ether(dst=self.pg0.local_mac, src=self.pg0.remote_mac) / p_g = (Ether(dst=self.pg0.local_mac, src=self.pg0.remote_mac) /
IP(src=self.pg0.remote_ip4, dst='224.0.0.1', tos=0xc0) / IP(src=self.pg0.remote_ip4, dst='224.0.0.1', tos=0xc0) /
IGMPv3(type="Membership Query", mrcode=100) / IGMPv3(type="Membership Query", mrcode=100) /
IGMPv3mq(gaddr="0.0.0.0")) IGMPv3mq(gaddr="0.0.0.0") /
Raw('\x00' * 10))
self.send(self.pg0, p_g) self.send(self.pg0, p_g)
@ -240,6 +243,19 @@ class TestIgmp(VppTestCase):
self.verify_report(capture[0], self.verify_report(capture[0],
[IgmpRecord(h1.sg, "Mode Is Include")]) [IgmpRecord(h1.sg, "Mode Is Include")])
#
# A group and source specific query that reports more sources
# than the packet actually has.
#
p_gs2 = (Ether(dst=self.pg0.local_mac, src=self.pg0.remote_mac) /
IP(src=self.pg0.remote_ip4, dst='239.1.1.1', tos=0xc0,
options=[IPOption(copy_flag=1, optclass="control",
option="router_alert")]) /
IGMPv3(type="Membership Query", mrcode=100) /
IGMPv3mq(gaddr="239.1.1.1", numsrc=4, srcaddrs=["1.1.1.1"]))
self.send_and_assert_no_replies(self.pg0, p_gs2, timeout=10)
# #
# A group and source specific query, with the source NOT matching # A group and source specific query, with the source NOT matching
# the source VPP has. There should be no response. # the source VPP has. There should be no response.