From: Denis Ovsienko Date: Tue, 10 Dec 2024 01:27:23 +0000 (+0000) Subject: Fix DECnet packet filtering on big-endian hosts. X-Git-Url: https://git.tcpdump.org/libpcap/commitdiff_plain/6bf6fb56ffc7b76ed3c0463e6e6c83813baeb8e7 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 ca6197c9..35cb959d 100644 --- 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 7cd144cd..0c6af6a2 100644 --- 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);