Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

various improvements and bug fixes for Hurd #1275

Merged
merged 5 commits into from
Feb 14, 2024

Conversation

infrastation
Copy link
Member

This among other things includes a workaround for #1269 and seems to work, but could use more eyeballs, so please proof-read if you can.

@infrastation
Copy link
Member Author

One thing I notice now is that pcap_setfilter_hurd() does not need to test if the first argument is NULL because the function call happens from a function pointer in the same pcap_t, which implies the latter is not NULL.

Another thing is that the calls to pcap_strerror() should have been pcapint_fmt_errmsg_for_errno().

On GNU/Hurd "tcpdump -D" does not include any network interfaces in the
list of capture devices because pcap_findalldevs() indeed does not
return any network interfaces (it can return other, e.g. D-Bus, capture
devices as expected though) because pcapint_platform_finddevs() in
pcap-hurd.c is a no-op.

Fix that by using pcapint_findalldevs_interfaces() (which in this case
comes from fad-getad.c) with two appropriate helper functions.  Omit the
loopback interface because it does not work, at least not with the
existing implementation of pcap_activate_hurd().
Giving a valid index to tcpdump "-i" flag usually works on other OSes,
but not on GNU/Hurd:

root@debian:~# tcpdump -D
1./dev/eth0 [Up, Running]
2.dbus-system (D-Bus system bus) [none]
3.dbus-session (D-Bus session bus) [none]

root@debian:~# tcpdump -n -i 1
tcpdump: device_open: (os/device) no such device

The correct behaviour depends on pcap_activate() returning the more
specific PCAP_ERROR_NO_SUCH_DEVICE, not just PCAP_ERROR.  Likewise,
tcpdump has a code path for the more specific PCAP_ERROR_PERM_DENIED,
which it does not receive either, so the user sees a generic error
message:

$ tcpdump -n -i /dev/eth0
tcpdump: get_privileged_ports: Operation not permitted

Also the snapshot length value 0, which is the default, does not mean
the usual MAXIMUM_SNAPLEN:

root@debian:~# tcpdump -n -i /dev/eth0
tcpdump: snaplen of 0 rejects all packets

root@debian:~# tcpdump -n -i /dev/eth0 -s 0
tcpdump: snaplen of 0 rejects all packets

In pcap_activate_hurd() add the required error code translations and
adjust snapshot length the same way other modules do.
The expected behaviour of "tcpdump -c 10 tcp" is to exit after capturing
10 TCP packets no matter how many other packets were on the wire, which
works on other OSes.  On GNU/Hurd the actual behaviour is exiting after
seeing any 10 packets on the wire no matter how many were TCP, if any.
This is because pcap_read_hurd() always returns 1 for a packet, and it
receives all packets because the filtering occurs in userland.  To fix
this, return 1 iff the packet matched the filter.

While at it, obtain the packet timestamp ASAP to make it a bit more
accurate.
Given the current value of NET_MAX_FILTER, Hurd kernel allows at most 31
BPF statements, which is not much, but better than nothing.  Make the
existing code around device_set_filter() a helper function and implement
the first revision of a setfilter_op function to use kernel filtering
when possible.

Make the existing userland filtering in pcap_read_hurd() conditional on
pcap_hurd.filtering_in_kernel and use the correct buffer length to fix
erroneous discarding of packets.  For example, the following invocation
did not capture any packets because the UDP header is within the packet
buffer, but beyond the specified snapshot length:

tcpdump -n -i /dev/eth0 -s 10 'udp[0] >= 0'

Add some comments and update pcap-filter(3PCAP).
GNU/Hurd, rather logically, implements GNU strerror_r(), so let
pcapint_fmt_errmsg_for_errno() make the necessary moves.
@infrastation
Copy link
Member Author

This is going to be merged in one hour unless anyone sees anything else that requires improvement.

@fxlb
Copy link
Member

fxlb commented Feb 14, 2024

I look at the output of findalldevstest on a Hurd system.
It shows no loopback interface. Is it OK?

@infrastation
Copy link
Member Author

This is intended, until someone finds a way to add support for loopback packet capture. The existing code cannot do that.

@infrastation
Copy link
Member Author

That is, on GNU/Hurd the attempt to capture on loopback fails with an error. On Haiku it "succeeds", but all packet counters are zero, so perhaps meanwhile it would make sense to disable it there as well.

@infrastation infrastation merged commit 3b0edbc into the-tcpdump-group:master Feb 14, 2024
4 checks passed
@fxlb
Copy link
Member

fxlb commented Feb 14, 2024

so perhaps meanwhile it would make sense to disable it there as well.

Perhaps, indeed.

@fxlb
Copy link
Member

fxlb commented Feb 14, 2024

The existing code cannot do that.

OK.

@infrastation
Copy link
Member Author

Loopback capture in Haiku turns out to be a bug, the bug fix is in the development branch, but is not yet in the beta branch. So that particular issue does not require a workaround.

@guyharris
Copy link
Member

Loopback capture in Haiku turns out to be a bug, the bug fix is in the development branch, but is not yet in the beta branch. So that particular issue does not require a workaround.

Should it have a note in doc/README.haiku.md?

(Speaking of "you can open the device but you get no packets", I should add a note in doc/README.macos about monitor-mode captures on newer versions of macOS, at least with newer machines, not giving any packets. There's apparently something that allows packet capturing, given that the "Sniffer" window in Apple's Wireless Diagnostics captures traffic by doing something and then running tcpdump on the Wi-Fi adapter with -I; it may be that it just disassociates the adapter from the network it's on.)

@infrastation
Copy link
Member Author

After the bug fix makes it into beta4 updates, it should not be difficult to tell the minimal "hrev" that has loopback capture working. I do not know when this will happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants