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

[Windows 10] A message sent on a datagram socket was larger than the internal message buffer #168

Closed
d-Rickyy-b opened this issue Jun 6, 2021 · 12 comments

Comments

@d-Rickyy-b
Copy link

Hey there,

I just started using this library and ran into an issue on Windows 10. It seems that for certain ICMP error messages, the following error message will pop up in your terminal:

read ip4 0.0.0.0: wsarecvfrom: A message sent on a datagram socket was larger than the internal message buffer or some other network limit, or the buffer used to receive a datagram into was smaller than the datagram itself.

This seems to be related to the devices that are unreachable and a too small size of a buffer .

Steps to reproduce

  1. Install the lib (go get -t "github.com/go-ping/ping")
  2. Create a file (main.go) with this code
  3. Enter an unreachable IP address (ICMP Host unreachable - Type: 3, Code: 1)
  4. Build & Run the code

Expected behavior

The request works fine and execPing will return False because the host is not reachable.

Actual behavior

The error written earlier will appear in the terminal and execPing will not return False but instead return an error.


Assumptions

What I found out so far is that the buffer for receiving the ICMP response is calculated wrongly and hence is too small.

The buffer bytes

ping/ping.go

Line 558 in ff8be33

bytes := make([]byte, p.getMessageLength())

is calculated by

ping/utils_windows.go

Lines 10 to 16 in e4e642a

// Returns the length of an ICMP message, plus the IP packet header.
func (p *Pinger) getMessageLength() int {
if p.ipv4 {
return p.Size + 8 + ipv4.HeaderLen
}
return p.Size + 8 + ipv6.HeaderLen
}

and then being used to read the ICMP response into:

ping/ping.go

Line 564 in ff8be33

n, ttl, _, err = conn.ReadFrom(bytes)

With pinger.Size up to 547 I still get the above error message. As soon as I use a pinger.Size of 548 or bigger, I am not getting the error anymore.

Now let's assume we have the following ICMP response from a router to a request that had pinger.Size = 547:

Description Bytes
IPv4 Response Header 20
ICMP Response Header 8
IPv4 Request Header 20
ICMP Request Header 8
ICMP Request Data 520
Total 576

In the above case the ICMP Request Data (which is contained within the response) is limited to just 520 bytes (even though we defined it to be 547 Bytes) because it's being cut off for the ICMP datagram size not to exceed the limit of 576 Bytes total (RFC 1812 - section 4.3.2.3). Now we use the earlier mentioned code to calculate the message/buffer size. The buffer size is calculated as p.Size + 8 + ipv4.HeaderLen = 547 + 8 + 20 = 575. That means that our buffer is too small for the data we try to receive.

Now if we choose pinger.Size = 548 we can see that our buffer is now big enough (548 + 8 + 20 = 576) to handle the same ICMP response. Any value >= 548 works just fine. Anything below that will lead to the issue described above.

I was not able to reproduce the issue on Linux.


Suggestion

As we can see in the following picture, also the IP header gets parsed into the passed buffer.
grafik

For regular, non-error requests it's totally fine to use a buffer of the size of the request. IMHO for ICMP error messages the buffer should be of size: p.Size + (8 + ipv4.HeaderLen) * 2. Also opposed to non-error messages, it would be fine to use a maximum buffer size of 575 Bytes because error messages do not grow bigger than that, while non-error ICMP messages can theoretically reach 65507 Bytes in size.

Sorry for the wall of text. I hope you get the point and will implement a solution for this issue. Thank you!

@SuperQ
Copy link
Contributor

SuperQ commented Jun 7, 2021

Thanks for the detailed report. I need to read that RFC, but you're probably correct, we need to include some additional buffer if ICMP responses with an error code added are valid.

@zzhxgood
Copy link

I also have this problem
OS:Windows 11 22000.100

@andig
Copy link

andig commented Nov 22, 2021

Joining the discussion with another user report in evcc-io/evcc#1916 (comment). Would it hurt to just increase the buffer size or should that be done on consumer side?

@stanimirivanovde
Copy link

I get this problem sometimes on a Windows 10 VM

@andig
Copy link

andig commented Jan 22, 2022

Just had another user report this in evcc-io/evcc#1561 (comment).

@d-Rickyy-b do I read your analysis right that pinger.Size = 548 should fix this for Windows 10?

@SuperQ any chance for a permanent fix?

@stanimirivanovde
Copy link

The issue I was having on Windows was due to too many packets I was trying send out. I was doing ping sweeping and port scanning at the same time. There is some internal Windows limitation on how many concurrent connections you can have. Once I go over this limit the error message starts popping up. So it might not be related to the tool itself but a Windows limitation. I tried increasing the limits for the TCP/IP configs in Windows registry but nothing really helped. May be running it in a VM adds additional limitations based on the host.

@stanimirivanovde
Copy link

I had another workaround. Looks like this error message causes subsequent pings to fail as well even for hosts that are up. I have to retry the ping if I encounter this error in order for it to be successful on live hosts. Can we please fix the buffer size?

@CHTJonas
Copy link
Member

@stanimirivanovde PRs are open!

@CHTJonas
Copy link
Member

If what @d-Rickyy-b suggests is correct and ICMP error messages can't grow bigger than 575 bytes then I think we need to do a math.Max() between 575 and the size of the ICMP request.

@d-Rickyy-b
Copy link
Author

... I think we need to do a math.Max() between 575 and the size of the ICMP request.

First off: I am no networking expert. All I wrote/will write in this GitHub issue comes from RFCs and internet resources I read.

I checked again: ICMP error messages SHOULD contain as much of the original datagram as possible without the length of the ICMP datagram exceeding 576 bytes.. But of course there could be systems replying with ICMP error messages that are >576 bytes in length (e.g. if the original datagram was that large). So ICMP error messages can grow bigger than 576 bytes. The RFC only says "should".

According to RFC760:

This field [Total Length] allows the length of a datagram to be up to 65,535 octets. Such long datagrams are impractical for most hosts and networks. All hosts must be prepared to accept datagrams of up to 576 octets (whether they arrive whole or in fragments).

And:

Every internet destination must be able to receive a datagram of 576 octets either in one piece or in fragments to be reassembled.


From my understanding I would suggest using a buffer of at least 576 bytes to receive the ICMP packet. In a popular rust library they use 2048 as buffer size (https://github.com/aisk/ping/blob/053409b86552b82d53e57be7864534474dfd0a7a/src/ping.rs#L52-L69). This seems to be, because there are no specified packets larger than 2048 bytes. But in theory an adversary could reply with forged ICMP responses of 65,535 bytes in size.

Another C++ implementation seems to use a buffer of size 65,536 (https://github.com/john-z-yang/ping/blob/b4e888627355b66d9fb0c72805cc2a7894e002fb/src/pinger.cc#L97-L104).

This tool (https://github.com/schweikert/fping/blob/ab1ed993badc1a6978b95c9d17a5e8b56fff4598/src/fping.c#L100) uses a recv_bufsize of 4096.

I really am not sure what the best buffer size is. All I can say is that the current implementation uses a too small buffer.

@SuperQ SuperQ closed this as completed Jan 21, 2023
@andig
Copy link

andig commented Jan 21, 2023

@SuperQ how was this completed?
Update I see- moving to unmaintained. Thanks for the heads-up.

@SuperQ
Copy link
Contributor

SuperQ commented Jan 22, 2023

Yes, sorry, due to lack of admin rights on this repo, we can't maintain it anymore.

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

6 participants