tcpdump mailing list archives

Re: Patches for wlan filtering


From: Guy Harris <guy () alum mit edu>
Date: Tue, 30 Oct 2007 03:42:12 -0700

Gianluca Varenni wrote:

the attached patch fixes some of the problems in the current wlan code generation of pcap_compile.
In particular it should fix these problems:

1. the 802.11 header size of a data frame has not a fixed size. When the QoS bit is set in the subtype field (QoS DATA frame), the header is two bytes longer. You can clearly see this on 802.11n APs, that usually use QoS DATA frames. The code does not take into consideration
  * 802.11n frames containing the HT control field
  * frames in a WDS (wireless bridges)
2. The optimizer generates buggy code when compiling filters in the form "wlan dst ...." or similar. The optimizer gets disabled in these cases. 3. Added some code that verifies that a packet is a data frame when accessing the LLC and above protocol fields.

I applied the patch (and fixed up the errors from "patch") and changed gen_load_ll_after_802_11_rel() so that, in the "radio header is fixed-length" branch, it sets X to off_ll and loads the Frame Control field from an offset of off_ll - off_ll is equal to the length of the fixed-length radio header, if there is one, and is otherwise 0.

With a DLT_IEEE80211 capture, the filter "tcp" generates the code

(000) ldb      [0]
(001) jset     #0x4             jt 16   jf 2
(002) ldb      [0]
(003) jset     #0x8             jt 4    jf 16
(004) ldx      #0x0
(005) ldb      [0]
(006) jset     #0x8             jt 7    jf 12
(007) jset     #0x4             jt 12   jf 8
(008) jset     #0x80            jt 9    jf 12
(009) ld       #0x2
(010) add      x
(011) tax
(012) ldh      [x + 30]
(013) jeq      #0x86dd          jt 14   jf 16
(014) ldb      [38]
(015) jeq      #0x6             jt 32   jf 16
(016) ldb      [0]
(017) jset     #0x4             jt 33   jf 18
(018) ldb      [0]
(019) jset     #0x8             jt 20   jf 33
(020) ldx      #0x0
(021) ldb      [0]
(022) jset     #0x8             jt 23   jf 28
(023) jset     #0x4             jt 28   jf 24
(024) jset     #0x80            jt 25   jf 28
(025) ld       #0x2
(026) add      x
(027) tax
(028) ldh      [x + 30]
(029) jeq      #0x800           jt 30   jf 33
(030) ldb      [41]
(031) jeq      #0x6             jt 32   jf 33
(032) ret      #2344
(033) ret      #0

Statements 14 and 15 are testing for a protocol of TCP, as are statements 30 and 31, but they're loading from a fixed offset - they need to be loading from an offset modified by the X register.

I won't be able to fix that tonight, but, if we delay the release a couple of days, I might be able to fix that.

I've attached a patch that applies to the current top of tree, and which includes my changes for fixed-length radio headers. (I also got rid of the #ifdef for the WLAN patches.)

Modifications related to 1. and 3. generate code that is not compatible with the optimizer, as gen_load_ll_XXX() generate slist*'s of instructions (where I added the code to verify if a packet is a data/QoS data frame), and not block* chunks. So optimization gets disabled.

I'm planning to commit them on the libpcap CVS as disabled patches, and also planning to enable them for the win32 build (winpcap) in the next version that is probably due within some weeks.

Any comment/test is more than welcome. Please apply the patch with "patch -p0 gencode.c < wlan_filtering.patch". The patch should work on both HEAD and the libpcap_0_9 branch (I developed it on the libpcap_0_9 branch).

Have a nice day
GV




-
This is the tcpdump-workers list.
Visit https://cod.sandelman.ca/ to unsubscribe.



Index: gencode.c
===================================================================
RCS file: /tcpdump/master/libpcap/gencode.c,v
retrieving revision 1.292
diff -c -r1.292 gencode.c
*** gencode.c   30 Oct 2007 10:16:45 -0000      1.292
--- gencode.c   30 Oct 2007 10:40:23 -0000
***************
*** 150,156 ****
        OR_NET,         /* relative to the network-layer header */
        OR_NET_NOSNAP,  /* relative to the network-layer header, with no SNAP header at the link layer */
        OR_TRAN_IPV4,   /* relative to the transport-layer header, with IPv4 network layer */
!       OR_TRAN_IPV6    /* relative to the transport-layer header, with IPv6 network layer */
  };
  
  /*
--- 150,157 ----
        OR_NET,         /* relative to the network-layer header */
        OR_NET_NOSNAP,  /* relative to the network-layer header, with no SNAP header at the link layer */
        OR_TRAN_IPV4,   /* relative to the transport-layer header, with IPv4 network layer */
!       OR_TRAN_IPV6,   /* relative to the transport-layer header, with IPv6 network layer */
!       OR_LINK_AFTER_WIRELESS_HDR /* After the 802.11 variable length header */
  };
  
  /*
***************
*** 191,196 ****
--- 192,198 ----
  static struct block *gen_ncmp(enum e_offrel, bpf_u_int32, bpf_u_int32,
      bpf_u_int32, bpf_u_int32, int, bpf_int32);
  static struct slist *gen_load_llrel(u_int, u_int);
+ static struct slist *gen_load_ll_after_802_11_rel(u_int, u_int);
  static struct slist *gen_load_a(enum e_offrel, u_int, u_int);
  static struct slist *gen_loadx_iphdrlen(void);
  static struct block *gen_uncond(int);
***************
*** 205,210 ****
--- 207,213 ----
  static struct block *gen_linktype(int);
  static struct block *gen_snap(bpf_u_int32, bpf_u_int32, u_int);
  static struct block *gen_llc_linktype(int);
+ static struct block *gen_802_11_llc_linktype(int);
  static struct block *gen_hostop(bpf_u_int32, bpf_u_int32, int, int, u_int, u_int);
  #ifdef INET6
  static struct block *gen_hostop6(struct in6_addr *, struct in6_addr *, int, int, u_int, u_int);
***************
*** 248,253 ****
--- 251,257 ----
  static struct slist *xfer_to_a(struct arth *);
  static struct block *gen_mac_multicast(int);
  static struct block *gen_len(int, int);
+ static struct block *gen_check_802_11_data_frame(void);
  
  static struct block *gen_ppi_dlt_check(void);
  static struct block *gen_msg_abbrev(int type);
***************
*** 1497,1502 ****
--- 1501,1519 ----
                s = gen_load_llrel(offset, size);
                break;
  
+       case OR_LINK_AFTER_WIRELESS_HDR:
+               if (linktype != DLT_IEEE802_11_RADIO 
+                       && linktype != DLT_PPI 
+                       && linktype != DLT_IEEE802_11 
+                       && linktype != DLT_PRISM_HEADER
+                       && linktype != DLT_IEEE802_11_RADIO_AVS)
+               {
+                       abort();
+                       return NULL;
+               }
+               s = gen_load_ll_after_802_11_rel(offset + 24, size);
+               break;
+ 
        case OR_NET:
                s = gen_load_llrel(off_nl + offset, size);
                break;
***************
*** 2160,2165 ****
--- 2177,2302 ----
        }
  }
  
+ /*
+  * Load a value relative to the beginning of the link-layer header after the 802.11
+  * header, i.e. LLC_SNAP.
+  * The link-layer header doesn't necessarily begin at the beginning
+  * of the packet data; there might be a variable-length prefix containing
+  * radio information.
+  */
+ static struct slist *
+ gen_load_ll_after_802_11_rel(offset, size)
+       u_int offset, size;
+ {
+       struct slist *s, *s_load_fc;
+       struct slist *sjset_qos;
+       struct slist *s_load;
+       struct slist *s_ld_a_2;
+       struct slist *s_add_a_x;
+       struct slist *s_a_to_x;
+       struct slist *sjset_data_frame_1;
+       struct slist *sjset_data_frame_2;
+       struct slist *s_load_x_off_ll;  
+ 
+       /*
+        * This code is not compatible with the optimizer, as
+        * we are generating jmp instructions within a normal
+        * slist of instructions
+        *
+        */
+       no_optimize = 1;
+       
+       s = gen_llprefixlen();
+ 
+       /*
+        * If "s" is non-null, it has code to arrange that the X register
+        * contains the length of the prefix preceding the link-layer
+        * header.
+        *
+        * Otherwise, the length of the prefix preceding the link-layer
+        * header is "off_ll".
+        */
+       if (s != NULL) {
+               /*
+                * There's a variable-length prefix preceding the
+                * link-layer header.  "s" points to a list of statements
+                * that put the length of that prefix into the X register.
+                * do an indirect load, to use the X register as an offset.
+                */
+ 
+               /*
+                * Load the Frame Control field
+                */
+               s_load_fc = new_stmt(BPF_LD|BPF_IND|BPF_B);
+               s_load_fc->s.k = 0;
+       } else {
+               /*
+                * There is no variable-length header preceding the
+                * link-layer header; add in off_ll, which, if there's
+                * a fixed-length header preceding the link-layer header,
+                * is the length of that header.
+                */
+ 
+               /*
+                * We need to load the Frame control directly, and
+                * then load X with the length of the fixed-length
+                * header preceding the link-layer header.
+                */
+ 
+               /* Load off_ll into X */
+               s_load_x_off_ll = new_stmt(BPF_LDX|BPF_IMM);
+               s_load_x_off_ll->s.k = off_ll;
+ 
+               /*
+                * load the Frame Control with absolute access
+                */
+               s_load_fc = new_stmt(BPF_LD|BPF_ABS|BPF_B);
+               s_load_fc->s.k = off_ll;
+               s = s_load_x_off_ll;
+       }
+ 
+       /*
+        * Generate the common instructions to check if it's a data frame
+        * and if so compute the 802.11 header length
+        */
+       sjset_data_frame_1 = new_stmt(JMP(BPF_JSET));   /* b3 should be 1 */
+       sjset_data_frame_1->s.k = 0x8;
+               
+       sjset_data_frame_2 = new_stmt(JMP(BPF_JSET));   /* b2 should be 0 */
+       sjset_data_frame_2->s.k = 0x04;
+ 
+       sjset_qos = new_stmt(JMP(BPF_JSET));
+       sjset_qos->s.k = 0x80; /* QoS bit */
+               
+       s_ld_a_2 = new_stmt(BPF_LD|BPF_IMM);
+       s_ld_a_2->s.k = 2;
+ 
+       s_add_a_x = new_stmt(BPF_ALU|BPF_ADD|BPF_X);
+       s_a_to_x = new_stmt(BPF_MISC|BPF_TAX);
+ 
+       s_load = new_stmt(BPF_LD|BPF_IND|size);
+       s_load->s.k = offset;
+ 
+       sjset_data_frame_1->s.jt = sjset_data_frame_2;
+       sjset_data_frame_1->s.jf = s_load;
+               
+       sjset_data_frame_2->s.jt = s_load;
+       sjset_data_frame_2->s.jf = sjset_qos;
+               
+       sjset_qos->s.jt = s_ld_a_2;
+       sjset_qos->s.jf = s_load;
+ 
+       sappend(s, s_load_fc);
+       sappend(s_load_fc, sjset_data_frame_1);
+       sappend(sjset_data_frame_1, sjset_data_frame_2);
+       sappend(sjset_data_frame_2, sjset_qos);
+       sappend(sjset_qos, s_ld_a_2);
+       sappend(s_ld_a_2, s_add_a_x);
+       sappend(s_add_a_x,s_a_to_x);
+       sappend(s_a_to_x, s_load);
+       
+       return s;
+ }
  
  static struct slist *
  gen_radiotap_llprefixlen(void)
***************
*** 2293,2304 ****
                break;
  
        case DLT_PPI:
!       case DLT_FDDI:
!       case DLT_IEEE802:
        case DLT_IEEE802_11:
        case DLT_IEEE802_11_RADIO_AVS:
-       case DLT_IEEE802_11_RADIO:
        case DLT_PRISM_HEADER:
        case DLT_ATM_RFC1483:
        case DLT_ATM_CLIP:
        case DLT_IP_OVER_FC:
--- 2430,2445 ----
                break;
  
        case DLT_PPI:
!       case DLT_IEEE802_11_RADIO:
        case DLT_IEEE802_11:
+       case DLT_IEEE802:
        case DLT_IEEE802_11_RADIO_AVS:
        case DLT_PRISM_HEADER:
+               return gen_802_11_llc_linktype(proto);
+               /*NOTREACHED*/
+               break;
+ 
+       case DLT_FDDI:
        case DLT_ATM_RFC1483:
        case DLT_ATM_CLIP:
        case DLT_IP_OVER_FC:
***************
*** 2849,2854 ****
--- 2990,3129 ----
  }
  
  static struct block *
+ gen_check_802_11_data_frame()
+ {
+       struct slist *s;
+       struct block *b0, *b1;
+ 
+       /*
+        * Check for a data frame.
+        * I.e, check "link[0] & 0x08".
+        */
+       s = gen_load_a(OR_LINK, 0, BPF_B);
+       b0 = new_block(JMP(BPF_JSET));
+       b0->s.k = 0x08;
+       b0->stmts = s;
+       
+       s = gen_load_a(OR_LINK, 0, BPF_B);
+       b1 = new_block(JMP(BPF_JSET));
+       b1->s.k = 0x04;
+       b1->stmts = s;
+       gen_not(b1);
+       
+ 
+       gen_and(b1, b0);
+ 
+       return b0;
+ }
+ 
+ /*
+  * Generate code to match a particular packet type, for link-layer types
+  * using 802.2 LLC headers.
+  *
+  * This is *NOT* used for Ethernet; "gen_ether_linktype()" is used
+  * for that - it handles the D/I/X Ethernet vs. 802.3+802.2 issues.
+  *
+  * "proto" is an Ethernet type value, if > ETHERMTU, or an LLC SAP
+  * value, if <= ETHERMTU.  We use that to determine whether to
+  * match the DSAP or both DSAP and LSAP or to check the OUI and
+  * protocol ID in a SNAP header.
+  */
+ static struct block *
+ gen_802_11_llc_linktype(proto)
+       int proto;
+ {
+       struct block *b_check_data_frame;
+       struct block *b_check_linktype;
+ 
+       b_check_data_frame = gen_check_802_11_data_frame();
+ 
+       /*
+        * XXX - generate the code that discards non data frames
+        */
+       switch (proto) {
+ 
+       case LLCSAP_IP:
+       case LLCSAP_ISONS:
+       case LLCSAP_NETBEUI:
+               /*
+                * XXX - should we check both the DSAP and the
+                * SSAP, like this, or should we check just the
+                * DSAP, as we do for other types <= ETHERMTU
+                * (i.e., other SAP values)?
+                */
+               b_check_linktype = gen_cmp(OR_LINK_AFTER_WIRELESS_HDR, 0, BPF_H, (bpf_u_int32)
+                            ((proto << 8) | proto));
+               break;
+ 
+       case LLCSAP_IPX:
+               /*
+                * XXX - are there ever SNAP frames for IPX on
+                * non-Ethernet 802.x networks?
+                */
+               b_check_linktype = gen_cmp(OR_LINK_AFTER_WIRELESS_HDR, 0, BPF_B,
+                   (bpf_int32)LLCSAP_IPX);
+ 
+               break;
+ 
+ #if 0
+       case ETHERTYPE_ATALK:
+               /*
+                * 802.2-encapsulated ETHERTYPE_ATALK packets are
+                * SNAP packets with an organization code of
+                * 0x080007 (Apple, for Appletalk) and a protocol
+                * type of ETHERTYPE_ATALK (Appletalk).
+                *
+                * XXX - check for an organization code of
+                * encapsulated Ethernet as well?
+                */
+               return gen_snap(0x080007, ETHERTYPE_ATALK, off_linktype);
+ #endif
+       default:
+               /*
+                * XXX - we don't have to check for IPX 802.3
+                * here, but should we check for the IPX Ethertype?
+                */
+               if (proto <= ETHERMTU) {
+                       /*
+                        * This is an LLC SAP value, so check
+                        * the DSAP.
+                        */
+                       b_check_linktype = gen_cmp(OR_LINK_AFTER_WIRELESS_HDR, 0, BPF_B,
+                           (bpf_int32)proto);
+               } else {
+                       /*
+                        * This is an Ethernet type; we assume that it's
+                        * unlikely that it'll appear in the right place
+                        * at random, and therefore check only the
+                        * location that would hold the Ethernet type
+                        * in a SNAP frame with an organization code of
+                        * 0x000000 (encapsulated Ethernet).
+                        *
+                        * XXX - if we were to check for the SNAP DSAP and
+                        * LSAP, as per XXX, and were also to check for an
+                        * organization code of 0x000000 (encapsulated
+                        * Ethernet), we'd do
+                        *
+                        *      return gen_snap(0x000000, proto,
+                        *          off_linktype);
+                        *
+                        * here; for now, we don't, as per the above.
+                        * I don't know whether it's worth the extra CPU
+                        * time to do the right check or not.
+                        */
+                       b_check_linktype = gen_cmp(OR_LINK_AFTER_WIRELESS_HDR, 0+6, BPF_H,
+                           (bpf_int32)proto);
+               }
+       }
+ 
+       gen_and(b_check_data_frame, b_check_linktype);
+       return b_check_linktype;
+ 
+ }
+ 
+ 
+ 
+ static struct block *
  gen_hostop(addr, mask, dir, proto, src_off, dst_off)
        bpf_u_int32 addr;
        bpf_u_int32 mask;
***************
*** 3062,3067 ****
--- 3337,3351 ----
        register struct block *b0, *b1, *b2;
        register struct slist *s;
  
+       /*
+        * TODO GV 20070613
+        * We need to disable the optimizer because the optimizer is buggy
+        * and wipes out some LD instructions generated by the below
+        * code to validate the Frame Control bits
+        *
+        */
+       no_optimize = 1;
+ 
        switch (dir) {
        case Q_SRC:
                /*
***************
*** 5367,5373 ****
                else
                        bpf_error("unknown protocol: %s", name);
  
- 
        case Q_UNDEF:
                syntax();
                /* NOTREACHED */
--- 5651,5656 ----
-
This is the tcpdump-workers list.
Visit https://cod.sandelman.ca/ to unsubscribe.

Current thread: