patch: compare identifier for non-root access icmp (#32)

Commit d046b245 introduces a bug which causes ping to always fail.
The source of this bug is:

```
	// Check if reply from same ID
	body := m.Body.(*icmp.Echo)
	if body.ID != p.id {
		return nil
	}
```

Which due to the selection of p.id requires that SetPrivileged is
set to true.  In the case where Privileged (i.e p.network == udp)
it is left to the kernel to set the ICMP id.

https://lwn.net/Articles/443051/  Discusses the introduction of
non-setuid-less ping.  The kernel implementation for this
interface dictates using the local port, which gets mapped into
the ping_table struct.  There is no current implementation in the
go icmp library to address this problem directly.

To address this issue, I've added a `Tracker` field for `Pinger`
as well as `IcmpData` datastructure to allow for uniquely tracking
icmp requests.  The id (as with the `id` field) is not unique,
but will statistically rare for duplicates.
This commit is contained in:
Lincoln Thurlow 2018-11-06 06:10:49 -08:00 committed by Cameron Sparr
parent 687023bdc7
commit 28a88d0810

67
ping.go
View File

@ -44,6 +44,7 @@
package ping
import (
"encoding/json"
"fmt"
"math"
"math/rand"
@ -88,11 +89,12 @@ func NewPinger(addr string) (*Pinger, error) {
Interval: time.Second,
Timeout: time.Second * 100000,
Count: -1,
id: rand.Intn(0xffff),
id: rand.Intn(math.MaxInt16),
network: "udp",
ipv4: ipv4,
Size: timeSliceLength,
done: make(chan bool),
Size: timeSliceLength,
Tracker: rand.Int63n(math.MaxInt64),
done: make(chan bool),
}, nil
}
@ -131,6 +133,9 @@ type Pinger struct {
// Size of packet being sent
Size int
// Tracker: Used to uniquely identify packet when non-priviledged
Tracker int64
// stop chan bool
done chan bool
@ -430,10 +435,24 @@ func (p *Pinger) processPacket(recv *packet) error {
return nil
}
// Check if reply from same ID
body := m.Body.(*icmp.Echo)
if body.ID != p.id {
return nil
// If we are priviledged, we can match icmp.ID
if p.network == "ip" {
// Check if reply from same ID
if body.ID != p.id {
return nil
}
} else {
// If we are not priviledged, we cannot set ID - require kernel ping_table map
// need to use contents to identify packet
data := IcmpData{}
err := json.Unmarshal(body.Data, &data)
if err != nil {
return err
}
if data.Tracker != p.Tracker {
return nil
}
}
outPkt := &Packet{
@ -444,7 +463,12 @@ func (p *Pinger) processPacket(recv *packet) error {
switch pkt := m.Body.(type) {
case *icmp.Echo:
outPkt.Rtt = time.Since(bytesToTime(pkt.Data[:timeSliceLength]))
data := IcmpData{}
err := json.Unmarshal(m.Body.(*icmp.Echo).Data, &data)
if err != nil {
return err
}
outPkt.Rtt = time.Since(bytesToTime(data.Bytes))
outPkt.Seq = pkt.Seq
p.PacketsRecv += 1
default:
@ -462,6 +486,11 @@ func (p *Pinger) processPacket(recv *packet) error {
return nil
}
type IcmpData struct {
Bytes []byte
Tracker int64
}
func (p *Pinger) sendICMP(conn *icmp.PacketConn) error {
var typ icmp.Type
if p.ipv4 {
@ -479,14 +508,22 @@ func (p *Pinger) sendICMP(conn *icmp.PacketConn) error {
if p.Size-timeSliceLength != 0 {
t = append(t, byteSliceOfSize(p.Size-timeSliceLength)...)
}
bytes, err := (&icmp.Message{
Type: typ, Code: 0,
Body: &icmp.Echo{
ID: p.id,
Seq: p.sequence,
Data: t,
},
}).Marshal(nil)
data, err := json.Marshal(IcmpData{Bytes: t, Tracker: p.Tracker})
if err != nil {
fmt.Errorf("Unable to marshal data")
}
body := &icmp.Echo{
ID: p.id,
Seq: p.sequence,
Data: data,
}
msg := &icmp.Message{
Type: typ,
Code: 0,
Body: body,
}
bytes, err := msg.Marshal(nil)
if err != nil {
return err
}