From: Mobashshera Rasool Date: Wed, 14 Oct 2020 13:51:32 +0000 (+0000) Subject: pimd: checksum must be validated before accepting igmp packets X-Git-Tag: base_7.6~264^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=9041c30ad1a8db6be436bc24421fabdd5ad54a6b;p=mirror%2Ffrr.git pimd: checksum must be validated before accepting igmp packets Issue: When an IGMPv2 leave packet is received, it did not validate the checksum and hence the packet is accepted and group specific query is sent out in response to this. Due to this IGMP conformance test case 6.1 failed. https://github.com/FRRouting/frr/issues/6868 Fix: Validate the checksum for all IGMP packets Signed-off-by: Mobashshera Rasool --- diff --git a/pimd/pim_igmp.c b/pimd/pim_igmp.c index 19d7817577..9924e335b0 100644 --- a/pimd/pim_igmp.c +++ b/pimd/pim_igmp.c @@ -270,6 +270,27 @@ void pim_igmp_other_querier_timer_off(struct igmp_sock *igmp) THREAD_OFF(igmp->t_other_querier_timer); } +int igmp_validate_checksum(char *igmp_msg, int igmp_msg_len) +{ + uint16_t recv_checksum; + uint16_t checksum; + + IGMP_GET_INT16((unsigned char *)(igmp_msg + IGMP_CHECKSUM_OFFSET), + recv_checksum); + + /* Clear the checksum field */ + memset(igmp_msg + IGMP_CHECKSUM_OFFSET, 0, 2); + + checksum = in_cksum(igmp_msg, igmp_msg_len); + if (ntohs(checksum) != recv_checksum) { + zlog_warn("Invalid checksum received %x, calculated %x", + recv_checksum, ntohs(checksum)); + return -1; + } + + return 0; +} + static int igmp_recv_query(struct igmp_sock *igmp, int query_version, int max_resp_code, struct in_addr from, const char *from_str, char *igmp_msg, @@ -278,8 +299,6 @@ static int igmp_recv_query(struct igmp_sock *igmp, int query_version, struct interface *ifp; struct pim_interface *pim_ifp; struct in_addr group_addr; - uint16_t recv_checksum; - uint16_t checksum; if (igmp->mtrace_only) return 0; @@ -289,17 +308,10 @@ static int igmp_recv_query(struct igmp_sock *igmp, int query_version, ifp = igmp->interface; pim_ifp = ifp->info; - recv_checksum = *(uint16_t *)(igmp_msg + IGMP_CHECKSUM_OFFSET); - - /* for computing checksum */ - *(uint16_t *)(igmp_msg + IGMP_CHECKSUM_OFFSET) = 0; - - checksum = in_cksum(igmp_msg, igmp_msg_len); - if (checksum != recv_checksum) { + if (igmp_validate_checksum(igmp_msg, igmp_msg_len) == -1) { zlog_warn( - "Recv IGMP query v%d from %s on %s: checksum mismatch: received=%x computed=%x", - query_version, from_str, ifp->name, recv_checksum, - checksum); + "Recv IGMP query v%d from %s on %s with invalid checksum", + query_version, from_str, ifp->name); return -1; } @@ -427,6 +439,13 @@ static int igmp_v1_recv_report(struct igmp_sock *igmp, struct in_addr from, return -1; } + if (igmp_validate_checksum(igmp_msg, igmp_msg_len) == -1) { + zlog_warn( + "Recv IGMP report v1 from %s on %s with invalid checksum", + from_str, ifp->name); + return -1; + } + /* Collecting IGMP Rx stats */ igmp->rx_stats.report_v1++; diff --git a/pimd/pim_igmp.h b/pimd/pim_igmp.h index 9231b0b41f..a0681128c0 100644 --- a/pimd/pim_igmp.h +++ b/pimd/pim_igmp.h @@ -69,6 +69,12 @@ #define IGMP_DEFAULT_VERSION (3) +#define IGMP_GET_INT16(ptr, output) \ + do { \ + output = *(ptr) << 8; \ + output |= *((ptr) + 1); \ + } while (0) + struct igmp_join { struct in_addr group_addr; struct in_addr source_addr; @@ -116,6 +122,8 @@ void pim_igmp_general_query_off(struct igmp_sock *igmp); void pim_igmp_other_querier_timer_on(struct igmp_sock *igmp); void pim_igmp_other_querier_timer_off(struct igmp_sock *igmp); +int igmp_validate_checksum(char *igmp_msg, int igmp_msg_len); + #define IGMP_SOURCE_MASK_FORWARDING (1 << 0) #define IGMP_SOURCE_MASK_DELETE (1 << 1) #define IGMP_SOURCE_MASK_SEND (1 << 2) diff --git a/pimd/pim_igmpv2.c b/pimd/pim_igmpv2.c index af598d040d..d836c66cbb 100644 --- a/pimd/pim_igmpv2.c +++ b/pimd/pim_igmpv2.c @@ -121,6 +121,13 @@ int igmp_v2_recv_report(struct igmp_sock *igmp, struct in_addr from, return -1; } + if (igmp_validate_checksum(igmp_msg, igmp_msg_len) == -1) { + zlog_warn( + "Recv IGMPv2 REPORT from %s on %s: size=%d with invalid checksum", + from_str, ifp->name, igmp_msg_len); + return -1; + } + /* Collecting IGMP Rx stats */ igmp->rx_stats.report_v2++; @@ -170,6 +177,13 @@ int igmp_v2_recv_leave(struct igmp_sock *igmp, struct in_addr from, return -1; } + if (igmp_validate_checksum(igmp_msg, igmp_msg_len) == -1) { + zlog_warn( + "Recv IGMPv2 LEAVE from %s on %s with invalid checksum", + from_str, ifp->name); + return -1; + } + /* Collecting IGMP Rx stats */ igmp->rx_stats.leave_v2++; diff --git a/pimd/pim_igmpv3.c b/pimd/pim_igmpv3.c index 22767a8629..425adfe166 100644 --- a/pimd/pim_igmpv3.c +++ b/pimd/pim_igmpv3.c @@ -1830,8 +1830,6 @@ void igmp_v3_recv_query(struct igmp_sock *igmp, const char *from_str, int igmp_v3_recv_report(struct igmp_sock *igmp, struct in_addr from, const char *from_str, char *igmp_msg, int igmp_msg_len) { - uint16_t recv_checksum; - uint16_t checksum; int num_groups; uint8_t *group_record; uint8_t *report_pastend = (uint8_t *)igmp_msg + igmp_msg_len; @@ -1853,16 +1851,10 @@ int igmp_v3_recv_report(struct igmp_sock *igmp, struct in_addr from, return -1; } - recv_checksum = *(uint16_t *)(igmp_msg + IGMP_CHECKSUM_OFFSET); - - /* for computing checksum */ - *(uint16_t *)(igmp_msg + IGMP_CHECKSUM_OFFSET) = 0; - - checksum = in_cksum(igmp_msg, igmp_msg_len); - if (checksum != recv_checksum) { + if (igmp_validate_checksum(igmp_msg, igmp_msg_len) == -1) { zlog_warn( - "Recv IGMP report v3 from %s on %s: checksum mismatch: received=%x computed=%x", - from_str, ifp->name, recv_checksum, checksum); + "Recv IGMPv3 report from %s on %s with invalid checksum", + from_str, ifp->name); return -1; } @@ -1880,9 +1872,8 @@ int igmp_v3_recv_report(struct igmp_sock *igmp, struct in_addr from, if (PIM_DEBUG_IGMP_PACKETS) { zlog_debug( - "Recv IGMP report v3 from %s on %s: size=%d checksum=%x groups=%d", - from_str, ifp->name, igmp_msg_len, checksum, - num_groups); + "Recv IGMP report v3 from %s on %s: size=%d groups=%d", + from_str, ifp->name, igmp_msg_len, num_groups); } group_record = (uint8_t *)igmp_msg + IGMP_V3_REPORT_GROUPPRECORD_OFFSET;