On 3/22/22 00:37, David Gwynne wrote:
On Mon, Mar 21, 2022 at 04:37:59PM -0400, Aner Perez wrote:
I noticed that if I put an "ifname" (or "on") in a fllter expression for
tcpdump, it will show all traffic that has an ifname that *starts with* the
name I provided.?? e.g.
# tcpdump -n -l -e -ttt -i pflog0 ifname vlan1
Will show packets for vlan1 but also for vlan110, vlan140, etc (but not for
em0).
It's not clear from the man page if that is the intended behavior.
https://man.openbsd.org/tcpdump.8#ifname
|ifname| <https://man.openbsd.org/tcpdump.8#ifname> interface
True if the packet was logged as coming from the specified interface
(applies only to
packets logged by pf(4) <https://man.openbsd.org/pf.4>).
While testing I also tried using "ifname vlan" as the filter but it fails
with a syntax error.?? I'm thinking that is probably an unintended
interaction with the "vlan" primitive since "ifname em" or "ifname bnx" seem
to work with no error.
This is all tested on 6.7 so apologies if this is not the current behavior.
i think this behaviour with ifname is unintended. the diff below tries
to fix it by having the ifname comparison include the terminating nul
when doing a comparison of the supplied interface name and the one in
the pflog header.
the consequence is that it will not longer do string prefix matches,
only whole name matches.
the vlan thing is different because there's a "vlan" keyword in our
pcap filter language that lets you do things like "tcpdump vlan
123" when sniffing on a vlan parent interface to limit the packets
to those with tag 123. the parser is saying it didnt expect you to
talk about vlan when it's supposed to be a string (ie, not a keyword)
at that point.
Index: gencode.c
===================================================================
RCS file: /cvs/src/lib/libpcap/gencode.c,v
retrieving revision 1.60
diff -u -p -r1.60 gencode.c
--- gencode.c 13 Feb 2022 20:02:30 -0000 1.60
+++ gencode.c 22 Mar 2022 04:29:40 -0000
@@ -3230,7 +3246,7 @@ gen_pf_ifname(char *ifname)
len - 1);
/* NOTREACHED */
}
- b0 = gen_bcmp(off, strlen(ifname), ifname);
+ b0 = gen_bcmp(off, strlen(ifname) + 1, ifname);
return (b0);
}
That certainly seems like it would do the trick. Would your diff make it into the
official source tree for a future release or is this something that needs to be discussed
by the powers that be?
Thanks for looking into it!
- Aner
