Bug #835 false unsupported dlt warnings on 802.3 (Ethernet I) and LLC

docs/CHANGELOG

@@ -11,6 +11,7 @@
     - incorrect checksum for certain IPv4 packets - fixed by #846 (#844)
     - add check for IPv6 extension header length (#827 #842)
     - GitHub template for pull requests (#839)
+    - improved 802.3 (Ethernet I) handling and warning messages (#835)
     - handle IPv6 fragment extension header (#832 #837)
     - Linux tap interfaces fail intermittently (#828)
     - Infinite loop in tcprewrite at get.c (#827 #842)

src/common/flows.c

@@ -157,8 +157,13 @@
 /*
  * Decode the packet, study it's flow status and report
  */
-flow_entry_type_t flow_decode(flow_hash_table_t *fht, const struct pcap_pkthdr *pkthdr,
-        const u_char *pktdata, const int datalink, const int expiry)
+flow_entry_type_t
+flow_decode(flow_hash_table_t *fht,
+            const struct pcap_pkthdr *pkthdr,
+            const u_char *pktdata,
+            const int datalink,
+            const int expiry,
+            COUNTER packetnum)
 {
     uint32_t pkt_len = pkthdr->caplen;
     const u_char *packet = pktdata;
@@ -195,30 +200,52 @@
                              &vlan_offset);
 
     if (res == -1) {
-        warnx("Unable to process unsupported DLT type: %s (0x%x)",
-              pcap_datalink_val_to_description(datalink), datalink);
+        warnx("flow_decode failed to determine %s header length for packet " COUNTER_SPEC "",
+              pcap_datalink_val_to_description(datalink),
+              packetnum);
         return FLOW_ENTRY_INVALID;
     }
 
     if (ether_type == ETHERTYPE_IP) {
-        if (pkt_len < l2len + sizeof(ipv4_hdr_t))
-                return FLOW_ENTRY_INVALID;
+        size_t required_len = sizeof(ipv4_hdr_t) + l2len;
+        if (pkt_len < required_len) {
+            warnx("flow_decode: packet " COUNTER_SPEC " needs at least %zd bytes for IPv4 header but only %d available",
+                  packetnum,
+                  required_len,
+                  pkt_len);
+            return FLOW_ENTRY_INVALID;
+        }
 
         ip_hdr = (ipv4_hdr_t *)(packet + l2len);
 
-        if (ip_hdr->ip_v != 4)
+        if (ip_hdr->ip_v != 4) {
+            warnx("flow_decode: packet " COUNTER_SPEC " IPv4 header version should be 4 but instead is %u",
+                  packetnum,
+                  ip_hdr->ip_v);
             return FLOW_ENTRY_NON_IP;
+        }
 
         ip_len = ip_hdr->ip_hl * 4;
         protocol = ip_hdr->ip_p;
         entry.src_ip.in = ip_hdr->ip_src;
         entry.dst_ip.in = ip_hdr->ip_dst;
     } else if (ether_type == ETHERTYPE_IP6) {
-        if (pkt_len < l2len + sizeof(ipv6_hdr_t))
-                return FLOW_ENTRY_INVALID;
+        size_t required_len = sizeof(ipv6_hdr_t) + l2len;
+        if (pkt_len < required_len) {
+            warnx("flow_decode: packet " COUNTER_SPEC " needs at least %zd bytes for IPv6 header but only %d available",
+                  packetnum,
+                  required_len,
+                  pkt_len);
+            return FLOW_ENTRY_INVALID;
+        }
 
-        if ((packet[0] >> 4) != 6)
+        uint8_t ip6_version = packet[0] >> 4;
+        if (ip6_version != 6) {
+            warnx("flow_decode: packet " COUNTER_SPEC " IPv6 header version should be 6 but instead is %u",
+                  packetnum,
+                  ip6_version);
             return FLOW_ENTRY_NON_IP;
+        }
 
         ip6_hdr = (ipv6_hdr_t *)(packet + l2len);
         ip_len = sizeof(*ip6_hdr);
@@ -238,30 +265,50 @@
     entry.protocol = protocol;
 
     switch (protocol) {
-    case IPPROTO_UDP:
-        if (pkt_len < (l2len + ip_len + sizeof(udp_hdr_t)))
+    case IPPROTO_UDP: {
+        size_t required_len = sizeof(udp_hdr_t) + l2len + ip_len;
+        if (pkt_len < required_len) {
+            warnx("flow_decode: packet " COUNTER_SPEC " needs at least %zd bytes for UDP header but only %d available",
+                  packetnum,
+                  required_len,
+                  pkt_len);
             return FLOW_ENTRY_INVALID;
-        udp_hdr = (udp_hdr_t*)(packet + ip_len + l2len);
+        }
+        udp_hdr = (udp_hdr_t *)(packet + ip_len + l2len);
         entry.src_port = udp_hdr->uh_sport;
         entry.dst_port = udp_hdr->uh_dport;
         break;
-
-    case IPPROTO_TCP:
-        if (pkt_len < (l2len + ip_len + sizeof(tcp_hdr_t)))
+    }
+    case IPPROTO_TCP: {
+        size_t required_len = sizeof(tcp_hdr_t) + l2len + ip_len;
+        if (pkt_len < required_len) {
+            warnx("flow_decode: packet " COUNTER_SPEC " needs at least %zd bytes for TCP header but only %d available",
+                  packetnum,
+                  required_len,
+                  pkt_len);
             return FLOW_ENTRY_INVALID;
-        tcp_hdr = (tcp_hdr_t*)(packet + ip_len + l2len);
+        }
+        tcp_hdr = (tcp_hdr_t *)(packet + ip_len + l2len);
         entry.src_port = tcp_hdr->th_sport;
         entry.dst_port = tcp_hdr->th_dport;
         break;
-
+    }
     case IPPROTO_ICMP:
-    case IPPROTO_ICMPV6:
-        if (pkt_len < (l2len + ip_len + sizeof(icmpv4_hdr_t)))
+    case IPPROTO_ICMPV6: {
+        size_t required_len = sizeof(icmpv4_hdr_t) + l2len + ip_len;
+        if (pkt_len < required_len) {
+            warnx("flow_decode: packet " COUNTER_SPEC " needs at least %zd bytes for %s header but only %d available",
+                  packetnum,
+                  required_len,
+                  (protocol == IPPROTO_ICMP) ? "ICMP" : "ICMPv6",
+                  pkt_len);
             return FLOW_ENTRY_INVALID;
-        icmp_hdr = (icmpv4_hdr_t*)(packet + ip_len + l2len);
+        }
+        icmp_hdr = (icmpv4_hdr_t *)(packet + ip_len + l2len);
         entry.src_port = icmp_hdr->icmp_type;
         entry.dst_port = icmp_hdr->icmp_code;
         break;
+    }
     default:
         entry.src_port = 0;
         entry.dst_port = 0;

src/common/flows.h

@@ -17,7 +17,7 @@
  */
 #pragma once
 
 #include "defines.h"
 #include "common.h"
 
 #define DEFAULT_FLOW_HASH_BUCKET_SIZE (1 << 16) /* 64K - must be a power of two */
@@ -38,4 +38,5 @@
                               const struct pcap_pkthdr *pkthdr,
                               const u_char *pktdata,
                               const int datalink,
-                              const int expiry);
+                              const int expiry,
+                              COUNTER packetnum);

src/common/get.c

@@ -18,7 +18,7 @@
  *   along with the Tcpreplay Suite.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 #include "defines.h"
 #include "config.h"
 #include "common.h"
 #include <lib/sll.h>
@@ -103,8 +103,12 @@
 
     /* move over MPLS labels until we get to the last one */
     while (!bos) {
-        if (pktdata + len + sizeof(*mpls_label) > end_ptr)
+        if (pktdata + len + sizeof(*mpls_label) > end_ptr) {
+            warnx("parse_mpls: Need at least %zu bytes for MPLS header but only %u available",
+                  sizeof(*mpls_label) + len,
+                  datalen);
             return -1;
+        }
 
         mpls_label = (struct tcpr_mpls_label *)(pktdata + len);
         len += sizeof(*mpls_label);
@@ -117,8 +121,12 @@
         }
     }
 
-    if ((u_char *)(mpls_label + 1) + 1 > end_ptr)
+    if ((u_char *)(mpls_label + 1) + 1 > end_ptr) {
+        warnx("parse_mpls: Need at least %zu bytes for MPLS label but only %u available",
+              sizeof(*mpls_label) + 1,
+              datalen);
         return -1;
+    }
 
     first_nibble = *((u_char *)(mpls_label + 1)) >> 4;
     switch (first_nibble) {
@@ -132,8 +140,12 @@
         /* EoMPLS - jump over PW Ethernet Control Word and handle
          * inner Ethernet header
          */
-        if (pktdata + len + 4 + sizeof(*eth_hdr) > end_ptr)
+        if (pktdata + len + 4 + sizeof(*eth_hdr) > end_ptr) {
+            warnx("parse_mpls: Need at least %zu bytes for EoMPLS header but only %u available",
+                  sizeof(*eth_hdr) + len + 4,
+                  datalen);
             return -1;
+        }
 
         len += 4;
         *l2offset = len;
@@ -142,7 +154,7 @@
         *next_protocol = ntohs(eth_hdr->ether_type);
         break;
     default:
-        /* suspect Generic Associated Channel Header */
+        warn("parse_mpls:suspect Generic Associated Channel Header");
         return -1;
     }
 
@@ -165,9 +177,10 @@
 parse_vlan(const u_char *pktdata, uint32_t datalen, uint16_t *next_protocol, uint32_t *l2len)
 {
     vlan_hdr_t *vlan_hdr;
-    if ((size_t)datalen < *l2len + sizeof(*vlan_hdr))
+    if ((size_t)datalen < *l2len + sizeof(*vlan_hdr)) {
+        warnx("parse_vlan: Need at least %zu bytes for VLAN header but only %u available", sizeof(*vlan_hdr), datalen);
         return -1;
-
+    }
     vlan_hdr = (vlan_hdr_t *)(pktdata + *l2len);
     *next_protocol = ntohs(vlan_hdr->vlan_tpid);
     *l2len += sizeof(vlan_hdr_t);
@@ -247,7 +260,7 @@
  * return 0 on success, -1 on failure
  */
 int
 get_l2len_protocol(const u_char *pktdata,
                    uint32_t datalen,
                    int datalink,
                    uint16_t *protocol,
@@ -277,19 +290,29 @@
             *protocol = ETHERTYPE_IP6;
         break;
     case DLT_JUNIPER_ETHER:
-        if (datalen < 4)
+        if (datalen < 4) {
+            warnx("%s (0x%x): Need at least 4 bytes for DLT_JUNIPER_ETHER but only %u available",
+                  pcap_datalink_val_to_description(datalink),
+                  datalink,
+                  datalen);
             return -1;
+        }
 
         if (memcmp(pktdata, JUNIPER_PCAP_MAGIC, 3) != 0) {
-            warnx("No Magic Number found during protocol lookup: %s (0x%x)",
+            warnx("%s (0x%x): No JUNIPER_PCAP_MAGIC Magic Number found during protocol lookup",
                   pcap_datalink_val_to_description(datalink),
                   datalink);
             return -1;
         }
 
         if ((pktdata[3] & JUNIPER_FLAG_EXT) == JUNIPER_FLAG_EXT) {
-            if (datalen < 6)
+            if (datalen < 6) {
+                warnx("%s (0x%x): Need at least 6 bytes for JUNIPER_FLAG_EXT but only %u available",
+                      pcap_datalink_val_to_description(datalink),
+                      datalink,
+                      datalen);
                 return -1;
+            }
 
             *l2offset = ntohs(*((uint16_t *)&pktdata[4]));
             *l2offset += 6; /* MGC + flags + ext_total_len */
@@ -300,8 +323,15 @@
         if ((pktdata[3] & JUNIPER_FLAG_NO_L2) == JUNIPER_FLAG_NO_L2) {
             /* no L2 header present - *l2offset is actually IP offset */
             uint32_t ip_hdr_offset = *l2offset;
-            if (datalen < ip_hdr_offset + 1)
+            uint32_t hdrSpaceNeeded = ip_hdr_offset + 1;
+            if (datalen < hdrSpaceNeeded) {
+                warnx("%s (0x%x): Need at least %u bytes for JUNIPER_FLAG_NO_L2 but only %u available",
+                      pcap_datalink_val_to_description(datalink),
+                      hdrSpaceNeeded,
+                      datalink,
+                      datalen);
                 return -1;
+            }
 
             if ((pktdata[ip_hdr_offset] >> 4) == 4)
                 *protocol = ETHERTYPE_IP;
@@ -317,36 +347,46 @@
         uint16_t ether_type;
         uint32_t l2_net_off = sizeof(*eth_hdr) + *l2offset;
 
-        if (datalen <= l2_net_off)
+        if (datalen <= l2_net_off + 4) {
+            warnx("%s (0x%x): Need at least %u bytes for DLT_EN10MB but only %u available",
+                  pcap_datalink_val_to_description(datalink),
+                  datalink,
+                  l2_net_off + 4,
+                  datalen);
             return -1;
+        }
 
         eth_hdr = (eth_hdr_t *)(pktdata + *l2offset);
         ether_type = ntohs(eth_hdr->ether_type);
         if (parse_metadata(pktdata, datalen, &ether_type, &l2_net_off, l2offset, vlan_offset))
             return -1;
 
-        if (datalen <= l2_net_off)
-            return -1;
-
         *l2len = l2_net_off;
-        if (ether_type > 1500) {
+        if (ether_type >= 1536) {
             /* Ethernet II frame - return in host order */
             *protocol = ether_type;
+        } else if (ether_type > 1500) {
+            warnx("%s (0x%x): unsupported 802.3 length %u",
+                  pcap_datalink_val_to_description(datalink),
+                  datalink,
+                  ether_type);
+            return -1;
         } else {
             /* 803.3 frame */
-            if ((pktdata[l2_net_off] >> 4) == 4)
-                *protocol = ETHERTYPE_IP;
-            else if ((pktdata[l2_net_off] >> 4) == 6)
-                *protocol = ETHERTYPE_IP6;
-            else
-                /* unsupported 802.3 protocol */
-                return -1;
+            /* we don't modify 802.3 protocols */
+            return -1;
         }
         break;
     }
     case DLT_PPP_SERIAL:
-        if ((size_t)datalen < sizeof(struct tcpr_pppserial_hdr))
+        if ((size_t)datalen < sizeof(struct tcpr_pppserial_hdr)) {
+            warnx("%s (0x%x): Need at least %zu bytes for DLT_PPP_SERIAL but only %u available",
+                  pcap_datalink_val_to_description(datalink),
+                  datalink,
+                  sizeof(struct tcpr_pppserial_hdr),
+                  datalen);
             return -1;
+        }
 
         struct tcpr_pppserial_hdr *ppp = (struct tcpr_pppserial_hdr *)pktdata;
         *l2len = sizeof(*ppp);
@@ -357,24 +397,42 @@
 
         break;
     case DLT_C_HDLC:
-        if (datalen < CISCO_HDLC_LEN)
+        if (datalen < CISCO_HDLC_LEN) {
+            warnx("%s (0x%x): Need at least %u bytes for DLT_C_HDLC but only %u available",
+                  pcap_datalink_val_to_description(datalink),
+                  datalink,
+                  CISCO_HDLC_LEN,
+                  datalen);
             return -1;
+        }
 
         hdlc_hdr_t *hdlc_hdr = (hdlc_hdr_t *)pktdata;
         *l2len = sizeof(*hdlc_hdr);
         *protocol = ntohs(hdlc_hdr->protocol);
         break;
     case DLT_LINUX_SLL:
-        if (datalen < SLL_HDR_LEN)
+        if (datalen < SLL_HDR_LEN) {
+            warnx("%s (0x%x): Need at least %u bytes for DLT_LINUX_SLL but only %u available",
+                  pcap_datalink_val_to_description(datalink),
+                  datalink,
+                  SLL_HDR_LEN,
+                  datalen);
             return -1;
+        }
 
         *l2len = SLL_HDR_LEN;
         sll_hdr_t *sll_hdr = (sll_hdr_t *)pktdata;
         *protocol = ntohs(sll_hdr->sll_protocol);
         break;
     case DLT_LINUX_SLL2:
-        if (datalen < SLL2_HDR_LEN)
+        if (datalen < SLL2_HDR_LEN) {
+            warnx("%s (0x%x): Need at least %u bytes for DLT_LINUX_SLL2 but only %u available",
+                  pcap_datalink_val_to_description(datalink),
+                  datalink,
+                  SLL2_HDR_LEN,
+                  datalen);
             return -1;
+        }
 
         *l2len = SLL2_HDR_LEN;
         sll2_hdr_t *sll2_hdr = (sll2_hdr_t *)pktdata;