[WireGuard] [PATCH] go test: add ICMP ping
Jason A. Donenfeld
Jason at zx2c4.com
Thu Jul 7 12:20:06 CEST 2016
This is awesome. What fun it was making this yesterday. A couple nits, however:
On Thu, Jul 7, 2016 at 4:57 AM, Jonathan Rudenberg
<jonathan at titanous.com> wrote:
> + Checksum: 0xa15b, // the packet is always the same, hard-code checksum
Isn't there some Go lib we can use to calculate this for us?
> + // read ICMP Echo Reply packet
> + replyPacket := make([]byte, 128)
Is this 128 exact?
> + replyPacket = replyPacket[:n]
I guess not, since you truncate it to n here.
> + replyHeaderLen := int(replyPacket[0]&0x0f) << 2
> + replyLen := binary.BigEndian.Uint16(replyPacket[2:])
> + replyMessage, err := icmp.ParseMessage(1, replyPacket[replyHeaderLen:replyLen])
I realize this is Go, where nobody cares about indices because the
runtime does checks for you, but... this pops out at me as ugly and
dangerous. replyHeaderLen and replyLen are attacker controlled!
> + if echo.ID != 1 || echo.Seq != 1 || string(echo.Data) != "WireGuard" {
Can we set ID and Seq to random numbers, and check that we get them back here?
More information about the WireGuard
mailing list