PPPOE handling of packets with incorrect payload length (patch included)

[Greg Stark <gsstark () mit edu>]

At some point in the past the pppoe printer was changed to respect the payload
length provided by the pppoe encapsulation and ignore any extra data in the
ethernet frame. Originally it just parsed all the data in the ethernet frame
and ignored the payload length.

It appears this does not match actual pppoe implementations. The linux kernel
for example uses the entire ethernet frame and ignores the payload length. 

In fact I suspect *all* implementations do this because my DSL provider has
been sending incorrect payload lengths for years now and they use fairly
common enterprise scale hardware and presumably haven't been getting tons of
customers with problems or it would be fixed by now. (Any sarcasm you may have
detected in that logic is purely unintended I'm sure:)

The net result is simply that most of my packets in a tcpdump appear to be
truncated and not display all the protocol information, even though the
snaplen is set high and the data is all there.

Attached a patch to reverse this logic and use the entire ethernet frame
regardless of the payload length. It prints a note when the payload length is
incorrect. Hm, I wonder if the note should be more explicit, what's the
message when an invalid checksum is detected? 

I wrote this quite a while ago, but just tested it against current CVS and it
still works. It also seems I added some improvements to the tag printing code
for the pppoe discovery protocol, I forget why, but I think it was useful.
Just printing "UTF8" wasn't very helpful.


Index: print-pppoe.c
===================================================================
RCS file: /tcpdump/master/tcpdump/print-pppoe.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 print-pppoe.c
--- print-pppoe.c       19 Dec 2002 09:39:14 -0000      1.21
+++ print-pppoe.c       11 Jun 2003 22:02:29 -0000
@@ -88,6 +88,7 @@ static struct tok pppoetag2str[] = {
 };
 
 #define PPPOE_HDRLEN 6
+#define MAXTAGPRINT 80
 
 u_int
 pppoe_if_print(const struct pcap_pkthdr *h, register const u_char *p)
@@ -134,14 +135,19 @@ pppoe_print(register const u_char *bp, u
                printf(" [ses 0x%x]", pppoe_sessionid);
        }
 
-       if (pppoe_payload + pppoe_length < snapend) {
-#if 0
-               const u_char *x = pppoe_payload + pppoe_length;
+       if (pppoe_payload + pppoe_length < snapend && snapend-pppoe_payload+14 > 64) {
+               /* (small packets are probably just padded up to the ethernet
+                  minimum of 64 bytes) */
                printf(" [length %d (%d extra bytes)]",
                    pppoe_length, snapend - pppoe_payload - pppoe_length);
-               default_print(x, snapend - x);
-#endif
+#if RESPECT_PAYLOAD_LENGTH
                snapend = pppoe_payload+pppoe_length;
+#else
+               /* Actual PPPoE implementations appear to ignore the payload
+                  length and use the full ethernet frame anyways */
+               pppoe_length = snapend-pppoe_payload;
+#endif
+               
        }
 
        if (pppoe_code) {
@@ -162,25 +168,36 @@ pppoe_print(register const u_char *bp, u
                        /* p points to tag_value */
 
                        if (tag_len) {
-                               int isascii = 1;
+                               unsigned isascii = 0, isgarbage = 0;
                                const u_char *v = p;
                                u_short l;
+                               char tag_str[MAXTAGPRINT];
+                               unsigned tag_str_len = 0;
 
-                               for (v = p; v < p + tag_len; v++)
-                                       if (*v >= 127 || *v < 32) {
-                                               isascii = 0;
-                                               break;
+                               for (v = p; v < p + tag_len && tag_str_len < MAXTAGPRINT-1; v++)
+                                       if (*v >= 32 && *v < 127) {
+                                               tag_str[tag_str_len++] = *v;
+                                               isascii++;
+                                       } else {
+                                               tag_str[tag_str_len++] = '.';
+                                               isgarbage++;
                                        }
+                               tag_str[tag_str_len] = 0;
 
-                               /* TODO print UTF8 decoded text */
-                               if (isascii) {
-                                       l = (tag_len < 80 ? tag_len : 80);
+                               if (isascii > isgarbage) {
                                        printf(" [%s \"%*.*s\"]",
-                                           tok2str(pppoetag2str, "TAG-0x%x", tag_type),
-                                           l, l, p);
-                               } else
-                                       printf(" [%s UTF8]",
-                                           tok2str(pppoetag2str, "TAG-0x%x", tag_type));
+                                              tok2str(pppoetag2str, "TAG-0x%x", tag_type),
+                                              tag_str_len, tag_str_len, tag_str);
+                               } else {
+                                       /* Print hex, not fast to abuse printf but this doesn't get used much */
+                                       printf(" [%s 0x", tok2str(pppoetag2str, "TAG-0x%x", tag_type));
+                                       for (v=p; v<p+tag_len; v++) {
+                                               printf("%02.2X", *v);
+                                       }
+                                       printf("]");
+                               }
+                               
+
                        } else
                                printf(" [%s]", tok2str(pppoetag2str,
                                    "TAG-0x%x", tag_type));




-- 
greg

-
This is the TCPDUMP workers list. It is archived at
http://www.tcpdump.org/lists/workers/index.html
To unsubscribe use mailto:tcpdump-workers-request () tcpdump org?body=unsubscribe