go-ping / ping

ICMP Ping library for Go
MIT License
1.32k stars 344 forks source link

Deserialisation issue on Windows #143

Closed xerox-xeon closed 3 years ago

xerox-xeon commented 3 years ago

PING www.google.com (192.217.24.68):

--- www.google.com ping statistics --- 1 packets transmitted, 0 packets received, 0 duplicates, 100% packet loss round-trip min/avg/max/stddev = 0s/0s/0s/0s

CHTJonas commented 3 years ago

Can you please include some code that reproduces this?

```go you can put your code inside three backticks like so and it will auto-format it nicely ```

xerox-xeon commented 3 years ago
func DoPing(target string, t int) {
    timeout := time.Second * time.Duration(t)
    interval := time.Second
    count := -1
    privileged := true
    flag.Usage = func() {
        fmt.Print(usage)
    }
    flag.Parse()

    if flag.NArg() == 0 {
        flag.Usage()
        return
    }

    host := GetHost(target)
    pinger, err := ping.NewPinger(host)
    if err != nil {
        fmt.Printf("ERROR: %s\n", err.Error())
        return
    }

    // listen for ctrl-C signal
    c := make(chan os.Signal, 1)
    signal.Notify(c, os.Interrupt)
    go func() {
        for range c {
            pinger.Stop()
        }
    }()

    pinger.OnRecv = func(pkt *ping.Packet) {
        fmt.Printf("%d bytes from %s: icmp_seq=%d time=%v ttl=%v\n",
            pkt.Nbytes, pkt.IPAddr, pkt.Seq, pkt.Rtt, pkt.Ttl)
    }
    pinger.OnDuplicateRecv = func(pkt *ping.Packet) {
        fmt.Printf("%d bytes from %s: icmp_seq=%d time=%v ttl=%v (DUP!)\n",
            pkt.Nbytes, pkt.IPAddr, pkt.Seq, pkt.Rtt, pkt.Ttl)
    }
    pinger.OnFinish = func(stats *ping.Statistics) {
        fmt.Printf("\n--- %s ping statistics ---\n", stats.Addr)
        fmt.Printf("%d packets transmitted, %d packets received, %d duplicates, %v%% packet loss\n",
            stats.PacketsSent, stats.PacketsRecv, stats.PacketsRecvDuplicates, stats.PacketLoss)
        fmt.Printf("round-trip min/avg/max/stddev = %v/%v/%v/%v\n",
            stats.MinRtt, stats.AvgRtt, stats.MaxRtt, stats.StdDevRtt)
    }

    pinger.Count = count
    pinger.Interval = interval
    pinger.Timeout = timeout
    pinger.SetPrivileged(privileged)

    fmt.Printf("PING %s (%s):\n", pinger.Addr(), pinger.IPAddr())
    err = pinger.Run()
    if err != nil {
        fmt.Printf("Failed to ping target host: %s", err)
    }
}
scientiacoder commented 3 years ago

I changed this code in recvICMP function if you are importing go-ping as local packages:

default:
    // ICMP messages have an 8-byte header.
    bytes := make([]byte, p.Size+8)    // change p.Size + 8 to a larger value like 512

to

default:
    // ICMP messages have an 8-byte header.
    bytes := make([]byte, 512)    // change p.Size + 8 to a larger value

and it works well on my windows 10 laptop.

xerox-xeon commented 3 years ago

I changed this code in recvICMP function if you are importing go-ping as local packages:

default:
  // ICMP messages have an 8-byte header.
  bytes := make([]byte, p.Size+8)    // change p.Size + 8 to a larger value like 512

to

default:
  // ICMP messages have an 8-byte header.
  bytes := make([]byte, 512)    // change p.Size + 8 to a larger value

and it works well on my windows 10 laptop.

When I change it, the program can return recv message, but not have TTL value.

PING www.google.com (172.217.31.228): 24 bytes from 172.217.31.228: icmp_seq=0 time=93.7401ms ttl=0 24 bytes from 172.217.31.228: icmp_seq=1 time=84.5403ms ttl=0 24 bytes from 172.217.31.228: icmp_seq=2 time=86.0818ms ttl=0

--- www.google.com ping statistics --- 3 packets transmitted, 3 packets received, 0 duplicates, 0% packet loss round-trip min/avg/max/stddev = 84.5403ms/88.120733ms/93.7401ms/4.023018ms

scientiacoder commented 3 years ago

Found some old issues about TTLs on the Windows platform, #105, #108. Hope this can help...

And it seems like still an issue here since net package does not implement it Source: https://pkg.go.dev/golang.org/x/net@v0.0.0-20210129194117-4acb7895a057/ipv4 and search Windows

Notes

Bugs

On Windows, the ReadBatch and WriteBatch methods of PacketConn are not implemented.

On Windows, the ReadBatch and WriteBatch methods of RawConn are not implemented.

This package is not implemented on JS, NaCl and Plan 9.

On Windows, the JoinSourceSpecificGroup, LeaveSourceSpecificGroup, ExcludeSourceSpecificGroup and IncludeSourceSpecificGroup methods of PacketConn and RawConn are not implemented.

On Windows, the ReadFrom and WriteTo methods of RawConn are not implemented.

[X] On Windows, the ControlMessage for ReadFrom and WriteTo methods of PacketConn is not implemented.

CHTJonas commented 3 years ago

Thanks for the repro code @lifeblood! I can replicate reliably and have a fix that I'll submit in a PR shortly.

@scientiacoder using a fixed size of 512 bytes is sub-optimal for a couple of reasons (see #137 and #138).

The real issue here is some kind of strange Windows-only incompatibility in the underlying Go standard libraries -- if you test this on Linux or macOS then it works fine. But for some reason when we run on Windows (*ipv4.PacketConn).ReadFrom([]byte) wants to deserialise a full IP packet rather than just the payload. But we can work around this...

The lack of TTLs on Windows is a known upstream issue that's out of scope for us here -- there's nothing we can do to fix it ourselves I'm afraid.

xerox-xeon commented 3 years ago

ping www.google.com PING www.google.com (172.217.31.228) 56(84) bytes of data. 64 bytes from hkg07s28-in-f4.1e100.net (172.217.31.228): icmp_seq=1 ttl=55 time=20.4 ms 64 bytes from hkg07s28-in-f4.1e100.net (172.217.31.228): icmp_seq=2 ttl=55 time=20.7 ms

PING www.google.com (172.217.31.228): 24 bytes from 172.217.31.228: icmp_seq=0 time=93.7401ms ttl=0 24 bytes from 172.217.31.228: icmp_seq=1 time=84.5403ms ttl=0 24 bytes from 172.217.31.228: icmp_seq=2 time=86.0818ms ttl=0

In addition,go-ping can not get the CNAME address of the ping domain.

CHTJonas commented 3 years ago

The code at cmd/ping/ping.go is intended as a demo only and not a wholesale replacement of /bin/ping on *NIX systems or C:\Windows\System32\PING.EXE on Windows.

By "get the CNAME address of the ping domain" I presume you mean performing a reverse lookup by querying the arpa. DNS zone for PTR records? If so then I think this is out of scope for a simple ICMP library. My opinion is that the downstream application should handle doing the DNS bit.

CHTJonas commented 3 years ago

If you update to the latest commit on master this should now be fixed.