From 539df2940d4f39191b1b985e6588ca7e9529a8df Mon Sep 17 00:00:00 2001
From: Tamir Duberstein <tamird@google.com>
Date: Wed, 26 Sep 2018 12:39:32 -0700
Subject: 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
---
 pkg/tcpip/network/ipv6/icmp.go      | 12 ++++++++++++
 pkg/tcpip/network/ipv6/icmp_test.go | 24 +++++++++++++++++++-----
 pkg/tcpip/network/ipv6/ipv6.go      |  2 +-
 3 files changed, 32 insertions(+), 6 deletions(-)

(limited to 'pkg')

diff --git a/pkg/tcpip/network/ipv6/icmp.go b/pkg/tcpip/network/ipv6/icmp.go
index 9b5fa3f6e..81aba0923 100644
--- a/pkg/tcpip/network/ipv6/icmp.go
+++ b/pkg/tcpip/network/ipv6/icmp.go
@@ -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())
 
diff --git a/pkg/tcpip/network/ipv6/icmp_test.go b/pkg/tcpip/network/ipv6/icmp_test.go
index e548784ac..a8eef4cf2 100644
--- a/pkg/tcpip/network/ipv6/icmp_test.go
+++ b/pkg/tcpip/network/ipv6/icmp_test.go
@@ -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 &&
diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go
index e1f4ea863..25bd998e5 100644
--- a/pkg/tcpip/network/ipv6/ipv6.go
+++ b/pkg/tcpip/network/ipv6/ipv6.go
@@ -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()
-- 
cgit v1.2.3