diff options
author | Tamir Duberstein <tamird@google.com> | 2018-09-26 12:39:32 -0700 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2018-09-26 12:41:04 -0700 |
commit | 539df2940d4f39191b1b985e6588ca7e9529a8df (patch) | |
tree | 8eed28ba4c4e645ecc653fbd42268e828e0816f3 /pkg/tcpip/network | |
parent | bee264f0c5b41dde1f51c5a65e84dfab2ac4d40b (diff) |
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
Diffstat (limited to 'pkg/tcpip/network')
-rw-r--r-- | pkg/tcpip/network/ipv6/icmp.go | 12 | ||||
-rw-r--r-- | pkg/tcpip/network/ipv6/icmp_test.go | 24 | ||||
-rw-r--r-- | pkg/tcpip/network/ipv6/ipv6.go | 2 |
3 files changed, 32 insertions, 6 deletions
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() |