tcpflow doesn't handle MPLS data

[ershov]

Subj.

Here's a patch to fix this:

diff --git a/src/datalink.cpp b/src/datalink.cpp
index 934f4c6..4b52fd5 100644
--- a/src/datalink.cpp
+++ b/src/datalink.cpp
@@ -183,6 +183,16 @@ void dl_ppp(u_char *user, const struct pcap_pkthdr *h, const u_char *p)

 #ifdef DLT_LINUX_SLL
 #define SLL_HDR_LEN       16
+
+#define SLL_ADDRLEN 8
+
+#ifndef ETHERTYPE_MPLS
+#define ETHERTYPE_MPLS      0x8847
+#endif
+#ifndef ETHERTYPE_MPLS_MULTI
+#define ETHERTYPE_MPLS_MULTI    0x8848
+#endif
+
 void dl_linux_sll(u_char *user, const struct pcap_pkthdr *h, const u_char *p){
     u_int caplen = h->caplen;
     u_int length = h->len;
@@ -196,8 +206,25 @@ void dl_linux_sll(u_char *user, const struct pcap_pkthdr *h, const u_char *p){
    DEBUG(6) ("warning: received incomplete Linux cooked frame");
    return;
     }
+
+    struct _sll_header {
+        u_int16_t   sll_pkttype;    /* packet type */
+        u_int16_t   sll_hatype; /* link-layer address type */
+        u_int16_t   sll_halen;  /* link-layer address length */
+        u_int8_t    sll_addr[SLL_ADDRLEN];  /* link-layer address */
+        u_int16_t   sll_protocol;   /* protocol */
+    };
+
+    _sll_header *sllp = (_sll_header*)p;
+    int mpls_sz = 0;
+    if (ntohs(sllp->sll_protocol) == ETHERTYPE_MPLS) {
+        // unwind MPLS stack
+        do {
+            mpls_sz += 4;
+        } while ( ((*(p+SLL_HDR_LEN + mpls_sz - 2)) & 1) == 0 );
+    }

-    packet_info pi(DLT_LINUX_SLL,h,p,tvshift(h->ts),p + SLL_HDR_LEN, caplen - SLL_HDR_LEN);
+    packet_info pi(DLT_LINUX_SLL,h,p,tvshift(h->ts),p + SLL_HDR_LEN + mpls_sz, caplen - SLL_HDR_LEN - mpls_sz);
     process_packet_info(pi);
 }
 #endif

Sorry for not providing the most common solution ;) I just needed this only case.

Just sharing back my own patch.

// Yury Ershov

[simsong]

Hi. Can you please provide me with some MPLS data so that I can test this? I need the data before I can incorporate it.

What is the "most common solution" ?

On Jan 15, 2013, at 1:16 AM, Yuriy Ershov notifications@github.com wrote:

Subj.

Here's a patch to fix this:

diff --git a/src/datalink.cpp b/src/datalink.cpp
index 934f4c6..4b52fd5 100644
--- a/src/datalink.cpp
+++ b/src/datalink.cpp
@@ -183,6 +183,16 @@ void dl_ppp(u_char *user, const struct pcap_pkthdr *h, const u_char *p)

#ifdef DLT_LINUX_SLL
#define SLL_HDR_LEN 16
+
+#define SLL_ADDRLEN 8
+
+#ifndef ETHERTYPE_MPLS
+#define ETHERTYPE_MPLS 0x8847
+#endif
+#ifndef ETHERTYPE_MPLS_MULTI
+#define ETHERTYPE_MPLS_MULTI 0x8848
+#endif
+
void dl_linux_sll(u_char *user, const struct pcap_pkthdr *h, const u_char *p){
u_int caplen = h->caplen;
u_int length = h->len;
@@ -196,8 +206,25 @@ void dl_linux_sll(u_char *user, const struct pcap_pkthdr *h, const u_char *p){
DEBUG(6) ("warning: received incomplete Linux cooked frame");
return;
}
+

• struct _sll_header {

•    u_int16_t   sll_pkttype;    /\* packet type */
  

•    u_int16_t   sll_hatype; /\* link-layer address type */
  

•    u_int16_t   sll_halen;  /\* link-layer address length */
  

•    u_int8_t    sll_addr[SLL_ADDRLEN];  /\* link-layer address */
  

•    u_int16_t   sll_protocol;   /\* protocol */
  

• };

• _sll_header _sllp = (sll_header)p;
• int mpls_sz = 0;
• if (ntohs(sllp->sll_protocol) == ETHERTYPE_MPLS) {

•    // unwind MPLS stack
  

•    do {
  

•        mpls_sz += 4;
  

•    } while ( ((*(p+SLL_HDR_LEN + mpls_sz - 2)) & 1) == 0 );
  

• }
• packet_info pi(DLT_LINUX_SLL,h,p,tvshift(h->ts),p + SLL_HDR_LEN, caplen - SLL_HDR_LEN);
• packet_info pi(DLT_LINUX_SLL,h,p,tvshift(h->ts),p + SLL_HDR_LEN + mpls_sz, caplen - SLL_HDR_LEN - mpls_sz);
  process_packet_info(pi);
  }
  #endif
  Sorry for not providing the most common solution ;) I just needed this only case.

Just sharing back my own patch.

// Yury Ershov

—
Reply to this email directly or view it on GitHub.

[ershov]

Hi. Can you please provide me with some MPLS data so that I can test this? I need the data before I can incorporate it.

I'll try to get some but I'm not sure because it might contain some "sensitive data".

How do I give it to you if I do?

What is the "most common solution" ?

Hmm.. Maybe.. A special layer to decode the whole set of all known protocols, possibly, one wrapped in another?

I understand that tcpflow is rather a quick lightweight hack than a thorough heavy solution, which is an advantage and drawback at the same time.

In general, the most proper way to do it is possibly to hack into tcpdump itself at point where it extracts an IP frame and pass it to tcpflow module to do it's work.
This behavior could be controlled by one more print mode switch.

If you do so, you would be able to process any network data known to tcpdump, including all future versions.

[simsong]

Hi. I am interested in adding this fix, but I can't do so without some test data. Just a few packets is all I need. Perhaps a Google query?

[ershov]

Oh.. Sorry, I forgot about it :(
Will now try to make some data for you.

[ershov]

Hi!
I made a tcpdump record with MPLS data.
This is a real recording. I truncated it and changed the source IP addresses.
https://dl.dropbox.com/u/57339249/mpls2.tcpdump

[simsong]

Thanks. I'll get to this within the next few days.

On Mar 14, 2013, at 8:30 AM, Yuriy Ershov notifications@github.com wrote:

Hi!
I made a tcpdump record with MPLS data.
This is a real recording. I truncated it and changed the source IP addresses.
https://dl.dropbox.com/u/57339249/mpls2.tcpdump

—
Reply to this email directly or view it on GitHub.

[simsong]

The patch you gave me is susceptible to buffer overruns when it encounters malformed mpls packets.

[ershov]

You're right.
This is easy to fix:

    struct _sll_header {
        u_int16_t   sll_pkttype;    /* packet type */
        u_int16_t   sll_hatype; /* link-layer address type */
        u_int16_t   sll_halen;  /* link-layer address length */
        u_int8_t    sll_addr[SLL_ADDRLEN];  /* link-layer address */
        u_int16_t   sll_protocol;   /* protocol */
    };

    _sll_header *sllp = (_sll_header*)p;
    int mpls_sz = 0;
    if (htons(sllp->sll_protocol) == ETHERTYPE_MPLS) {
        // unwind MPLS stack
        do {
            mpls_sz += 4;
            if (caplen < SLL_HDR_LEN + mpls_sz) {
                DEBUG(6) ("warning: MPLS stack overrun");
                return;
            }
        } while ( ((*(p+SLL_HDR_LEN + mpls_sz - 2)) & 1) == 0 );
    }

    demux.process_ip(&h->ts,p + SLL_HDR_LEN + mpls_sz, caplen - SLL_HDR_LEN - mpls_sz,flow::NO_VLAN);

[simsong]

Thanks. I prefer this:

    } while ( ((p[SLL_HDR_LEN + mpls_sz - 2]) & 1) == 0 );

Any objection?

On Mar 14, 2013, at 10:10 AM, Yuriy Ershov notifications@github.com wrote:

You're right.
This is easy to fix:

struct _sll_header {
    u_int16_t   sll_pkttype;    /* packet type */
    u_int16_t   sll_hatype; /* link-layer address type */
    u_int16_t   sll_halen;  /* link-layer address length */
    u_int8_t    sll_addr[SLL_ADDRLEN];  /* link-layer address */
    u_int16_t   sll_protocol;   /* protocol */
};

_sll_header *sllp = (_sll_header*)p;
int mpls_sz = 0;
if (htons(sllp->sll_protocol) == ETHERTYPE_MPLS) {
    // unwind MPLS stack
    do {
        mpls_sz += 4;
        if (caplen < SLL_HDR_LEN + mpls_sz) {
            DEBUG(6) ("warning: MPLS stack overrun");
            return;
        }
    } while ( ((*(p+SLL_HDR_LEN + mpls_sz - 2)) & 1) == 0 );
}

demux.process_ip(&h->ts,p + SLL_HDR_LEN + mpls_sz, caplen - SLL_HDR_LEN - mpls_sz,flow::NO_VLAN);

—
Reply to this email directly or view it on GitHub.

[ershov]

Maybe remove one more ()'s then?

         } while ( (p[SLL_HDR_LEN + mpls_sz - 2] & 1) == 0 );

[simsong]

Yep. That's what I had, if not what I emailed.
Thanks again.Will be uploaded shortly.
On Mar 15, 2013, at 1:05 AM, Yuriy Ershov notifications@github.com wrote:

Maybe remove one more ()'s then?

     } while ( (p[SLL_HDR_LEN + mpls_sz - 2] & 1) == 0 );

—
Reply to this email directly or view it on GitHub.