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 <mrasool@vmware.com>

diff --git a/pimd/pim_igmp.c b/pimd/pim_igmp.c
index 19d78175772c8fd422b50af7a77308cb02c0b101..9924e335b00839a82d6da220582b337dd1677604 100644 (file)
--- 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 9231b0b41fb0128120122eb1c46bc8a8ca55e22b..a0681128c0c72642c09add1a947fe80c9640047c 100644 (file)
--- 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 af598d040ddb41a86ffe693f74e65c69ec6d0bac..d836c66cbbc5d91c2f53bcb9d9506c2f7ed1bbc9 100644 (file)
--- 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 22767a8629700f22f87d6d0240320bf7d86cab9f..425adfe166686c58ef14b814b6dc2b34bbacefa9 100644 (file)
--- 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;