Use the ICMP target address in responses

There is a subtle bug that is the result of two changes made when upstreaming
ICMPv6 support from Fuchsia:
1) ipv6.endpoint.WritePacket writes the local address it was initialized with,
rather than the provided route's local address
2) ipv6.endpoint.handleICMP doesn't set its route's local address to the ICMP
target address before writing the response

The result is that the ICMP response erroneously uses the target ipv6 address
(rather than icmp) as its source address in the response. When trying to debug
this by fixing (2), we ran into problems with bad ipv6 checksums because (1)
didn't respect the local address of the route being passed to it.

This fixes both problems.

PiperOrigin-RevId: 214650822
Change-Id: Ib6148bf432e6428d760ef9da35faef8e4b610d69
This commit is contained in:
Tamir Duberstein
2018-09-26 12:41:04 -07:00
committed by Shentubot
parent bee264f0c5
commit 539df2940d
3 changed files with 32 additions and 6 deletions
+12
View File
@@ -105,6 +105,18 @@ func (e *endpoint) handleICMP(r *stack.Route, vv buffer.VectorisedView) {
pkt[icmpV6OptOffset] = ndpOptDstLinkAddr
pkt[icmpV6LengthOffset] = 1
copy(pkt[icmpV6LengthOffset+1:], r.LocalLinkAddress[:])
// ICMPv6 Neighbor Solicit messages are always sent to
// specially crafted IPv6 multicast addresses. As a result, the
// route we end up with here has as its LocalAddress such a
// multicast address. It would be nonsense to claim that our
// source address is a multicast address, so we manually set
// the source address to the target address requested in the
// solicit message. Since that requires mutating the route, we
// must first clone it.
r := r.Clone()
defer r.Release()
r.LocalAddress = targetAddr
pkt.SetChecksum(icmpChecksum(pkt, r.LocalAddress, r.RemoteAddress, buffer.VectorisedView{}))
r.WritePacket(hdr, buffer.VectorisedView{}, header.ICMPv6ProtocolNumber, r.DefaultTTL())
+19 -5
View File
@@ -41,6 +41,11 @@ var (
lladdr1 = header.LinkLocalAddr(linkAddr1)
)
type icmpInfo struct {
typ header.ICMPv6Type
src tcpip.Address
}
type testContext struct {
t *testing.T
s0 *stack.Stack
@@ -49,7 +54,7 @@ type testContext struct {
linkEP0 *channel.Endpoint
linkEP1 *channel.Endpoint
icmpCh chan header.ICMPv6Type
icmpCh chan icmpInfo
}
type endpointWithResolutionCapability struct {
@@ -65,7 +70,7 @@ func newTestContext(t *testing.T) *testContext {
t: t,
s0: stack.New([]string{ProtocolName}, []string{ping.ProtocolName6}, stack.Options{}),
s1: stack.New([]string{ProtocolName}, []string{ping.ProtocolName6}, stack.Options{}),
icmpCh: make(chan header.ICMPv6Type, 10),
icmpCh: make(chan icmpInfo, 10),
}
const defaultMTU = 65536
@@ -132,7 +137,10 @@ func (c *testContext) countPacket(pkt channel.PacketInfo) {
}
b := pkt.Header[header.IPv6MinimumSize:]
icmp := header.ICMPv6(b)
c.icmpCh <- icmp.Type()
c.icmpCh <- icmpInfo{
typ: icmp.Type(),
src: ipv6.SourceAddress(),
}
}
func (c *testContext) routePackets(ch <-chan channel.PacketInfo, ep *channel.Endpoint) {
@@ -198,8 +206,14 @@ func TestLinkResolution(t *testing.T) {
case <-ctx.Done():
t.Errorf("timeout waiting for ICMP, got: %#+v", stats)
return
case typ := <-c.icmpCh:
stats[typ]++
case icmpInfo := <-c.icmpCh:
switch icmpInfo.typ {
case header.ICMPv6NeighborAdvert:
if got, want := icmpInfo.src, lladdr1; got != want {
t.Errorf("got ICMPv6NeighborAdvert.sourceAddress = %v, want = %v", got, want)
}
}
stats[icmpInfo.typ]++
if stats[header.ICMPv6NeighborSolicit] > 0 &&
stats[header.ICMPv6NeighborAdvert] > 0 &&
+1 -1
View File
@@ -91,7 +91,7 @@ func (e *endpoint) WritePacket(r *stack.Route, hdr buffer.Prependable, payload b
PayloadLength: length,
NextHeader: uint8(protocol),
HopLimit: ttl,
SrcAddr: e.id.LocalAddress,
SrcAddr: r.LocalAddress,
DstAddr: r.RemoteAddress,
})
r.Stats().IP.PacketsSent.Increment()