Fix DECnet packet filtering on big-endian hosts.

DECnet address matching works on little-endian hosts only:

$ arch
OpenBSD.amd64
$ tcpdump -nr tests/DECnet_Phone.pcap -c 1 'decnet src 1.1' | wc -l
reading from file tests/DECnet_Phone.pcap, link-type EN10MB (Ethernet),
  snapshot length 65535
       1

$ arch
OpenBSD.mips64
$ tcpdump -nr tests/DECnet_Phone.pcap -c 1 'decnet src 1.1' | wc -l
reading from file tests/DECnet_Phone.pcap, link-type EN10MB (Ethernet),
  snapshot length 65535
       0

Although the savefile is the same, the bytecode is different: in the
latter case gen_dnhostop() generates 8 statements out of 20 with 16-bit
literal values byte-swapped, e.g.:

-(006) jeq      #0x104           jt 22	jf 7
+(006) jeq      #0x401           jt 22	jf 7

For one half of those statements the root cause is feeding the provided
16-bit big-endian DECnet address, which always needs to be converted to
little-endian to match the packet encoding, to ntohs(), which leaves the
value intact on a big-endian host.  Fix this by using SWAPSHORT()
instead.

For the other half the root cause is treating two adjacent independent
bytes of the packet as a 16-bit value, hard-coding it and its 16-bit
mask byte-swapped and feeding these values to ntohs(), which on a
big-endian host does not cancel the hard-coded byte swap out.  Fix this
by hard-coding the two bytes and the bit mask as big-endian in the first
place.

Remove a number of unnecessary type casts and add a comment to explain
the packet encoding and the code logic.

diff --git a/CHANGES b/CHANGES
index ca6197c9953c810d3b4b764ecbe4329cde1143ac..35cb959d2822a428168b9114cc644911b1bd6ebe 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -41,6 +41,7 @@ DayOfTheWeek, Month DD, YYYY / The Tcpdump Group
       Add support for filtering packets encapsulated with VXLAN (pull
         request #1273).
       Eliminate trailing space in bpf_image() result.
+      Fix DECnet packet filtering on big-endian hosts.
     rpcap:
       Support user names and passwords in rpcap:// and rpcaps:// URLs.
       Add a -t flag to rpcapd to specify the data channel port; from

diff --git a/gencode.c b/gencode.c
index 7cd144cd816118dd57a11db98a1c26f67d5aac87..0c6af6a2134b916802d92d2c48129df162ebd322 100644 (file)
--- a/gencode.c
+++ b/gencode.c
@@ -5043,32 +5043,51 @@ gen_dnhostop(compiler_state_t *cstate, bpf_u_int32 addr, int dir)
                abort();
                /*NOTREACHED*/
        }
+       /*
+        * In a DECnet message inside an Ethernet frame the first two bytes
+        * immediately after EtherType are the [litle-endian] DECnet message
+        * length, which is irrelevant in this context.
+        *
+        * "pad = 1" means the third byte equals 0x81, thus it is the PLENGTH
+        * 8-bit bitmap of the optional padding before the packet route header.
+        * The bitmap always has bit 7 set to 1 and in this case has bits 0-6
+        * (TOTAL-PAD-SEQUENCE-LENGTH) set to integer value 1.  The latter
+        * means there aren't any PAD bytes after the bitmap, so the header
+        * begins at the fourth byte.  "pad = 0" means bit 7 of the third byte
+        * is set to 0, thus the header begins at the third byte.
+        *
+        * The header can be in several (as mentioned above) formats, all of
+        * which begin with the FLAGS 8-bit bitmap, which always has bit 7
+        * (PF, "pad field") set to 0 regardless of any padding present before
+        * the header.  "Short header" means bits 0-2 of the bitmap encode the
+        * integer value 2 (SFDP), and "long header" means value 6 (LFDP).
+        *
+        * For the DECnet address use SWAPSHORT(), which always swaps bytes,
+        * because the wire encoding is little-endian and this function always
+        * receives a big-endian address value.
+        */
        b0 = gen_linktype(cstate, ETHERTYPE_DN);
        /* Check for pad = 1, long header case */
-       tmp = gen_mcmp(cstate, OR_LINKPL, 2, BPF_H,
-           (bpf_u_int32)ntohs(0x0681), (bpf_u_int32)ntohs(0x07FF));
+       tmp = gen_mcmp(cstate, OR_LINKPL, 2, BPF_H, 0x8106U, 0xFF07U);
        b1 = gen_cmp(cstate, OR_LINKPL, 2 + 1 + offset_lh,
-           BPF_H, (bpf_u_int32)ntohs((u_short)addr));
+           BPF_H, SWAPSHORT(addr));
        gen_and(tmp, b1);
        /* Check for pad = 0, long header case */
-       tmp = gen_mcmp(cstate, OR_LINKPL, 2, BPF_B, (bpf_u_int32)0x06,
-           (bpf_u_int32)0x7);
+       tmp = gen_mcmp(cstate, OR_LINKPL, 2, BPF_B, 0x06U, 0x07U);
        b2 = gen_cmp(cstate, OR_LINKPL, 2 + offset_lh, BPF_H,
-           (bpf_u_int32)ntohs((u_short)addr));
+           SWAPSHORT(addr));
        gen_and(tmp, b2);
        gen_or(b2, b1);
        /* Check for pad = 1, short header case */
-       tmp = gen_mcmp(cstate, OR_LINKPL, 2, BPF_H,
-           (bpf_u_int32)ntohs(0x0281), (bpf_u_int32)ntohs(0x07FF));
+       tmp = gen_mcmp(cstate, OR_LINKPL, 2, BPF_H, 0x8102U, 0xFF07U);
        b2 = gen_cmp(cstate, OR_LINKPL, 2 + 1 + offset_sh, BPF_H,
-           (bpf_u_int32)ntohs((u_short)addr));
+           SWAPSHORT(addr));
        gen_and(tmp, b2);
        gen_or(b2, b1);
        /* Check for pad = 0, short header case */
-       tmp = gen_mcmp(cstate, OR_LINKPL, 2, BPF_B, (bpf_u_int32)0x02,
-           (bpf_u_int32)0x7);
+       tmp = gen_mcmp(cstate, OR_LINKPL, 2, BPF_B, 0x02U, 0x07U);
        b2 = gen_cmp(cstate, OR_LINKPL, 2 + offset_sh, BPF_H,
-           (bpf_u_int32)ntohs((u_short)addr));
+           SWAPSHORT(addr));
        gen_and(tmp, b2);
        gen_or(b2, b1);