ARCnet filtering confuses RFC 1051 and RFC 1201 encodings for IPv4 and ARP

[infrastation]

This code in gen_linktype() treats the code points from the two specifications as equivalent:

		case ETHERTYPE_IP:
			b0 = gen_cmp(cstate, OR_LINKTYPE, 0, BPF_B,
			    ARCTYPE_IP);
			b1 = gen_cmp(cstate, OR_LINKTYPE, 0, BPF_B,
			    ARCTYPE_IP_OLD);
			gen_or(b0, b1);
			return (b1);

		case ETHERTYPE_ARP:
			b0 = gen_cmp(cstate, OR_LINKTYPE, 0, BPF_B,
			    ARCTYPE_ARP);
			b1 = gen_cmp(cstate, OR_LINKTYPE, 0, BPF_B,
			    ARCTYPE_ARP_OLD);
			gen_or(b0, b1);
			return (b1);

The resulting filter program reflects this:

$ ./testprogs/filtertest ARCNET_LINUX 'ip src host 1.2.3.4'
(000) ldb      [4]
(001) jeq      #0xd4            jt 3	jf 2
(002) jeq      #0xf0            jt 3	jf 6
(003) ld       [20]
(004) jeq      #0x1020304       jt 5	jf 6
(005) ret      #262144
(006) ret      #0

The problem here is, RFC 1051 header is 5 bytes long and RFC 1201 header is 8 bytes long. As discussed in #1515, gen_host() assumes RFC 1201 encoding, so the ARCTYPE_ARP_OLD and ARCTYPE_IP_OLD branches quietly fail (IPv4 addressing is the same and protocol exchange is very similar in the two packet captures):

$ ./tcpdump -n -r tests/arcnet-rfc1201-arp-icmp-http.pcap 'icmp and ip dst host 10.80.131.1'
reading from file tests/arcnet-rfc1201-arp-icmp-http.pcap, link-type ARCNET_LINUX (Linux ARCNET), snapshot length 65535
14:20:23.013926 IP 10.80.131.254 > 10.80.131.1: ICMP echo reply, id 59392, seq 0, length 64
14:20:24.009584 IP 10.80.131.254 > 10.80.131.1: ICMP echo reply, id 59392, seq 256, length 64

$ ./tcpdump -n -r tests/arcnet-rfc1051-arp-icmp-http.pcap 'icmp and ip dst host 10.80.131.1'
reading from file tests/arcnet-rfc1051-arp-icmp-http.pcap, link-type ARCNET_LINUX (Linux ARCNET), snapshot length 65535
$

One way to make the implementation match the specification would be using a different offset (3 fewer bytes) for ARCTYPE_ARP_OLD and ARCTYPE_IP_OLD; another would be removing RFC 1051 code points altogether, which would make more sense since RFC 1201 was published in 1991. In any case, since this would be a change in the implemented behaviour, it would need to be formally announced and documented.

[infrastation]

A few potential ways to address this problem:

1. Remove ARCTYPE_IP_OLD and ARCTYPE_ARP_OLD from gen_linktype(). Arguably, the currently implemented behaviour has been the case since commit 38b97b7 in 2001, which means these two system codes have never worked (not as specified, anyway) and nobody complained.
2. In gen_host() special-case Q_IP and Q_ARP to generate one check for RFC 1051 system code and offset and another for RFC 1201 system code and offset, and OR the two branches. Obviously, at most one branch will ever match a packet. This would probably require moving the EtherType-to-system-code translation out of gen_linktype() and to have the latter expect ll_proto to be an 8-bit ARCTYPE_ value for ARCnet DLTs similarly how other DLTs take an LLCSAP_ value rather than (or in addition to) an ETHERTYPE_ value. There may be other factors that will make the implementation more convoluted than it seems, but the logic looks simple.
3. As the comments in init_linktype() say it, both DLT_ARCNET and DLT_ARCNET_LINUX actually have a variable offset of the link-layer payload, so implement it properly and have the filter program derive the offset from the system code for each packet. This looks the most over-engineered approach, but it could be a good learning material to understand/audit/test packet offset handling in the generator and to be in a better position to progress other offset-related matters such as Enable filter functionality for DLT_DSA_TAG_DSA and DLT_DSA_TAG_EDSA. #1195 and WIP/RFC: add support for manual link layer offset changes #1477.