Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

avoid processing incorrect ICMP message on Unix #34084

Merged
merged 2 commits into from
Dec 19, 2018

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented Dec 13, 2018

fixes #33699 Ping.Unix falsely returns successful pings

When we use RAW socket implementation we simply create socket to get all ICMP.
When there are multiple pings going (even from different process) the raw socket would receive all ICMP responses and than it can incorrectly report success even for non existing host.

This change effectively implements address filtering. Instead of doing it explicitly as discussed in issue, it uses Connect() to ping socket to only that address for Unicast. (similar behavior to UDP)

I don't know if there is better way to determine multicast (leading 1110) IP for IPv4.

Aside from running tests, I did verify that ping to multicast (224.0.0.1 all nodes) still works as it used to. There is currently not good API to know what nodes responded to the query. So successful response means that there is at least one responding node.

I also did tests with broadcast and directed broadcast (xx.xx.xx.255)
It fails with even for root:

Unhandled Exception: System.Net.Sockets.SocketException: Permission denied
at System.Net.Sockets.Socket.DoBeginSendTo(Byte[] buffer, Int32 offset, Int32 size, SocketFlags socketFlags, EndPoint endPointSnapshot, SocketAddress socketAddress, OverlappedAsyncResult asyncResult)

this is because we would need to do ioctl to enable broadcast functions on socket.
This behavior remains same with this change.

@wfurt wfurt added area-System.Net os-linux Linux OS (any supported distro) labels Dec 13, 2018
@wfurt wfurt self-assigned this Dec 13, 2018
@wfurt wfurt requested a review from a team December 13, 2018 22:46
// Disable warning about obsolete property. We could use GetAddressBytes but that allocates.
if (!ep.Address.IsIPv6Multicast && !(addrFamily == AddressFamily.InterNetwork && (ep.Address.Address & 0xf0) == 0xe0))
{
// If it is not multicast, use Connect to scope responses only tho target address.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: tho => to the

@wfurt wfurt merged commit 3172386 into dotnet:master Dec 19, 2018
@karelz karelz added this to the 3.0 milestone Dec 21, 2018
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
* use connect on unicast ping addresses to avoild processing incorrect responses

* feedback from review


Commit migrated from dotnet/corefx@3172386
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net os-linux Linux OS (any supported distro)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ping.Unix falsely returns successful pings
3 participants