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

BUG Npcap: icmp[0]==3 causes pcap readers to not pick up icmp replies. #373

Closed
NicholasKChoi opened this issue Dec 6, 2018 · 11 comments
Closed

Comments

@NicholasKChoi
Copy link

@NicholasKChoi NicholasKChoi commented Dec 6, 2018

Information

I am running the version of npcap: 0.99-r7

I am running on the Windows Datacenter in Amazon:

  • the ami: ami-0261fc597bed67b34
  • the windows os info: Build#=14393.2608; Version=1607

I've also done the following:

  • I checked the documentation and found no answer
  • I checked to make sure that this issue has not already been filed
  • I'm reporting the issue to the correct repository (for multi-repository projects)

Expected Behavior

When running wireshark to capture on the main interface with the filter: not (icmp[0]=3), I expect to capture both the icmp request and reply traffic.

Current Behavior

I only see the request traffic. The reply ICMP traffic does not show up at all. I have confirmed this both with Wireshark (which uses Npcap), and with a custom program I wrote that uses the Npcap Driver as well.

Steps to Reproduce

  1. Launch a Windows Datacenter 2016 in Amazon us-west-1 region.
  2. Install Npcap and Wireshark using the browser of your choice.
  3. Run Wireshark to capture on the Main interface with the filter not (icmp[0]=3).
  4. Generate ICMP traffic (I used the powershell command: ping -l 100 8.8.8.8 -n 10000).
  5. Add the Display Filter to Wireshark: icmp.
  6. You will only see the ICMP request traffic.
@dmiller-nmap
Copy link
Contributor

@dmiller-nmap dmiller-nmap commented Dec 6, 2018

Thanks for this very detailed bug report. I've done a test on my own system here and was unable to reproduce, but of course it's a very different system. Still, the capture filter compilation and matching should be the same for both, so I'm guessing that the problem is not related to the capture filter. To confirm and narrow down the problem, please provide the following information:

  1. Does the ping utility indicate that a response is being received?
  2. Do you see the response in Wireshark if you do not set a capture filter? If so, please provide hex of the packet from the beginning of the IP header through the ICMP header.
  3. Do you see the response in Wireshark if you set a capture filter of icmp?
  4. Is the ping utility definitely using IPv4? There is no chance that ICMPv6 could be affecting things?
  5. Please provide output of DiagReport on your system.

Just to clarify, I understand that this packet filter is intended to filter out ICMP Destination Unreachable messages, is that correct?

@guyharris
Copy link

@guyharris guyharris commented Dec 8, 2018

  1. Does this work if you do the capturing on some flavor of UN*X (Linux, macOS, Solaris, *BSD, etc.)?

@NicholasKChoi
Copy link
Author

@NicholasKChoi NicholasKChoi commented Dec 10, 2018

  1. The ping utility indicates the response is being received. And using tshark with only an -f icmp filter shows both the reply & request.
  2. I have the pcap available now, I can provide the hex later on in the day.
  3. I do.
  4. the ping is definitely using ipv4.
  5. I'll provide this as soon as I can.
  6. This same code and the same tools works on flavors of Unix etc. No issues. there.

@guyharris
Copy link

@guyharris guyharris commented Jan 23, 2019

  1. Do the ICMP replies not being captured include VLAN headers?

@guyharris
Copy link

@guyharris guyharris commented Jan 29, 2019

So the packets not being seen are packets being received by the host running Wireshark, not being sent by that host, right? That might have the same cause as #68, but that one appears to be causing problems with packets being sent by the host running the capture program.

@dmiller-nmap
Copy link
Contributor

@dmiller-nmap dmiller-nmap commented Mar 7, 2019

I think I've found the cause of the problem, but it's going to take some time to work out a solution.

Under NDIS 5, which WinPcap used and the code was originally written for, packets were delivered as a header and a lookahead buffer. If these were contiguous, WinPcap would call bpf_filter on the header buffer directly, and all offsets would Just Work. If they were separate, there is a different function, bpf_filter_with_2_buffers, that was called to take care of translating everything.

Under NDIS 6, which is what Npcap uses, packet data is delivered as a chain of memory descriptor lists (MDLs). It looks like different network layers can just allocate a buffer for their own data/header and attach it in the appropriate place in the chain. Npcap checks for whether the data link layer header is delivered separately in the first MDL, and in this case it allocates a buffer and requests the whole packet copied contiguously into the buffer before calling bpf_filter. If the first MDL is not exactly the data link layer header length then it assumes the first MDL has enough of the header to proceed, and it just passes the first MDL's buffer to bpf_filter.

The bug, it seems, is that the first MDL must contain the data link header and the IP header, but not the ICMP header and data. Therefore the capture filter cannot succeed since there is not enough data in the buffer that is passed to it.

A simple fix would be to always copy the packet data into a contiguous buffer before calling bpf_filter, but that seems like a lot of extra work that could slow down capturing unnecessarily in some cases. It would be great if we could know beforehand how much data the packet filter needs in order to test, but I don't think that's something simple to add. We could use the "snaplen" concept, which is not very well supported in Npcap (e.g. nmap/nmap#339). We would add a new IoCtl and extend PacketSetSnapLen to use it. Currently, PacketSetSnapLen only works on DAG cards, and only when DAG API support is built in, which we currently don't do.

I'm going to think about this for a bit, refactor the NPF_TapExForEachOpen function to better fit the current data model, and then choose a way ahead. I think this is something we can take care of in the next release.

@NicholasKChoi
Copy link
Author

@NicholasKChoi NicholasKChoi commented Mar 7, 2019

@guyharris
Copy link

@guyharris guyharris commented Mar 7, 2019

Under NDIS 6, which is what Npcap uses, packet data is delivered as a chain of memory descriptor lists (MDLs).

In the kernel of *BSD-flavored operating systems, packet data is delivered as a chain of mbufs (struct mbuf`).

A simple fix would be to always copy the packet data into a contiguous buffer before calling bpf_filter, but that seems like a lot of extra work that could slow down capturing unnecessarily in some cases.

In the kernel of *BSD-flavored operating systems, the bpf_filter() code has routines - named m_xword() and m_xhalf() in the FreeBSD kernel - that take, as arguments, a pointer into an mbuf chain, a byte offset into the data, and a pointer to an int. They walk through the mbuf chain to find the mbuf that contains the byte at the offset, check whether there are 3 more bytes in the mbuf in the "fetch a 4-byte word" routine or 1 more byte in the mbuf in the "fetch a 1-byte halfword" routine, and:

  • if so, fetch the 4-byte word or 2-byte halfword from the mbuf's data;
  • if not, get a pointer to the first data byte in the next mbuf (if there is one) and, depending on how many bytes of data remain in the mbuf containing the first byte, reassemble the 4-byte word or 2-byte halfword from the data in the two mbufs.

In both of those cases, it sets the pointed-to int to 0, meaning "no error".

If it doesn't find the mbuf with the first byte, or if not all the bytes are in the mbuf it found and there is no next mbuf or there aren't enough bytes in the next mbuf, it fails, setting the pointed-to int to 1, meaning "error".

Here's the code in question:

  • FreeBSD;
  • NetBSD (look for EXTRACT_SHORT);
  • OpenBSD: generic filtering code, BPF code that calls it - OpenBSD passes a table of generic memory access ops to _bpf_filter(), with bpf_filter() passing a set of ops that assume a contiguous blob of memory, and bpf_mfilter() in bpf.c passing a set of ops that work on mbuf chains;
  • DragonFly BSD (look for EXTRACT_SHORT);
  • macOS (look for get_word_from_buffers).

@dmiller-nmap
Copy link
Contributor

@dmiller-nmap dmiller-nmap commented Mar 7, 2019

@guyharris Thanks, that's really helpful! I'm often hesitant to change certain parts of code (like the BPF stuff) and so I look for solutions at layers that I feel more comfortable with. But this certainly seems like the right way to do it, and ought to make things faster, too.

dmiller-nmap referenced this issue Mar 14, 2019
Takes the guesswork out of pulling packet data out of the MDL chain and
into buffers. Solves the problem where packet filters can't reference
data in a later MDL than the initial one. See nmap/nmap#1406
mattlknight referenced this issue in mattlknight/npcap Mar 14, 2019
Takes the guesswork out of pulling packet data out of the MDL chain and
into buffers. Solves the problem where packet filters can't reference
data in a later MDL than the initial one. See nmap/nmap#1406
@dmiller-nmap
Copy link
Contributor

@dmiller-nmap dmiller-nmap commented Mar 19, 2019

This issue ought to be fixed in Npcap 0.991. @NicholasKChoi We would be very grateful if you could confirm the fix, since we were unable to reproduce the issue in our lab.

@dmiller-nmap
Copy link
Contributor

@dmiller-nmap dmiller-nmap commented Sep 4, 2019

Since there has been no further comment on this bug, we will assume the fix applied in 0.991 and corrected in 0.992 was sufficient to resolve it. If you experience further issues with Npcap 0.9983 or later, please open a new issue and reference this one if applicable.

@fyodor fyodor transferred this issue from nmap/nmap May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants