ipv6: Fixed fragment packet re-assembly.

pkg/tcpip/header/ipv6.go

@@ -215,9 +215,9 @@ func (b IPv6) TransportProtocol() tcpip.TransportProtocolNumber {
 	return tcpip.TransportProtocolNumber(b.NextHeader())
 }
 
-// isExtensionHeader returns true if the next header is a known extension header.
-func isExtensionHeader(hdr uint8) bool {
-	extType := IPv6ExtensionHeaderIdentifier(hdr)
+// IsExtensionHeader returns true if the next header is a known extension header.
+func IsExtensionHeader(nextHdr uint8) bool {
+	extType := IPv6ExtensionHeaderIdentifier(nextHdr)
 	switch extType {
 	case IPv6HopByHopOptionsExtHdrIdentifier, IPv6RoutingExtHdrIdentifier, IPv6FragmentExtHdrIdentifier, IPv6DestinationOptionsExtHdrIdentifier, IPv6AuthenticationExtHdrIdentifier, IPv6NoNextHeaderIdentifier:
 		return true
@@ -238,7 +238,7 @@ func (b IPv6) TryParseTransportProtocol() (tcpip.TransportProtocolNumber, bool)
 	data := []byte(b[IPv6MinimumSize:])
 	nxtHdr := b.NextHeader()
 	maybeProto := tcpip.TransportProtocolNumber(nxtHdr)
-	for isExtensionHeader(nxtHdr) {
+	for IsExtensionHeader(nxtHdr) {
 		dataLen := len(data)
 		if dataLen < 2 {
 			return maybeProto, false

pkg/tcpip/link/fdbased/processors.go

@@ -215,34 +215,47 @@ func tcpipConnectionID(pkt *stack.PacketBuffer) (connectionID, bool) {
 			return cid, true
 		}
 		ipHdr := header.IPv6(h)
+		cid.srcAddr = ipHdr.SourceAddressSlice()
+		cid.dstAddr = ipHdr.DestinationAddressSlice()
+		cid.proto = header.IPv6ProtocolNumber
 
-		var tcpHdr header.TCP
-		if tcpip.TransportProtocolNumber(ipHdr.NextHeader()) == header.TCPProtocolNumber {
-			tcpHdr = header.TCP(h[header.IPv6FixedHeaderSize:][:tcpSrcDstPortLen])
+		if !header.IsExtensionHeader(ipHdr.NextHeader()) {
+			// Known transport protocols(not just TCP) store the src and dst ports
+			// in the first 4 bytes after the IPv6 fixed header.
+			tcpHdr := header.TCP(h[header.IPv6FixedHeaderSize:][:tcpSrcDstPortLen])
+			cid.srcPort = tcpHdr.SourcePort()
+			cid.dstPort = tcpHdr.DestinationPort()
 		} else {
 			// Slow path for IPv6 extension headers :(.
 			dataBuf := pkt.Data().ToBuffer()
 			dataBuf.TrimFront(header.IPv6MinimumSize)
 			it := header.MakeIPv6PayloadIterator(header.IPv6ExtensionHeaderIdentifier(ipHdr.NextHeader()), dataBuf)
 			defer it.Release()
+			// All fragment packets need to be processed by the same goroutine, so
+			// only record the ports if this is not a fragment packet.
+			var isFragment bool
 			for {
 				hdr, done, err := it.Next()
 				if done || err != nil {
 					break
 				}
+				if fh, ok := hdr.(header.IPv6FragmentExtHdr); ok && !fh.IsAtomic() {
+					isFragment = true
+				}
 				hdr.Release()
 			}
-			h, ok = pkt.Data().PullUp(int(it.HeaderOffset()) + tcpSrcDstPortLen)
-			if !ok {
-				return cid, true
+			if !isFragment {
+				h, ok = pkt.Data().PullUp(int(it.HeaderOffset()) + tcpSrcDstPortLen)
+				if !ok {
+					return cid, true
+				}
+				// Known transport protocols store the src and dst ports
+				//  in the first 4 bytes after the IPv6 fixed header.
+				tcpHdr := header.TCP(h[it.HeaderOffset():][:tcpSrcDstPortLen])
+				cid.srcPort = tcpHdr.SourcePort()
+				cid.dstPort = tcpHdr.DestinationPort()
 			}
-			tcpHdr = header.TCP(h[it.HeaderOffset():][:tcpSrcDstPortLen])
 		}
-		cid.srcAddr = ipHdr.SourceAddressSlice()
-		cid.dstAddr = ipHdr.DestinationAddressSlice()
-		cid.srcPort = tcpHdr.SourcePort()
-		cid.dstPort = tcpHdr.DestinationPort()
-		cid.proto = header.IPv6ProtocolNumber
 	default:
 		return cid, true
 	}

pkg/tcpip/network/internal/fragmentation/reassembler.go

@@ -103,8 +103,21 @@ func (r *reassembler) process(first, last uint16, more bool, proto uint8, pkt *s
 		}
 
 		holeFound = true
+		// IPv6: rfc8200#section-4.5
+		//  Changed the text to require that IPv6 nodes must not create
+		//  overlapping fragments.  Also, when reassembling an IPv6
+		//  datagram, if one or more its constituent fragments is
+		//  determined to be an overlapping fragment, the entire datagram
+		//  (and any constituent fragments) must be silently discarded.
+		//  Includes a clarification that no ICMP error message should be
+		//  sent if overlapping fragments are received.
 		if currentHole.filled {
+			// Incoming fragment is a subset of an existing fragment.
+			if first != currentHole.first || last != currentHole.last {
+				return nil, 0, false, 0, ErrFragmentOverlap
+			}
 			// Incoming fragment is a duplicate.
+			// Not dropping packet incase of duplicates.
 			continue
 		}
 

pkg/tcpip/network/internal/fragmentation/reassembler_test.go

@@ -55,7 +55,7 @@ func TestReassemblerProcess(t *testing.T) {
 		})
 	}
 
-	var tests = []struct {
+	tests := []struct {
 		name    string
 		params  []processParams
 		want    []hole
@@ -125,17 +125,15 @@ func TestReassemblerProcess(t *testing.T) {
 			wantPkt: pkt(2, 2),
 		},
 		{
-			name: "Two fragments completing a packet with a partial duplicate",
+			name: "Two fragments completing a packet with a partial overlap",
 			params: []processParams{
 				{first: 0, last: 3, more: true, pkt: pkt(4), wantDone: false, wantError: nil},
-				{first: 1, last: 2, more: true, pkt: pkt(2), wantDone: false, wantError: nil},
-				{first: 4, last: 5, more: false, pkt: pkt(2), wantDone: true, wantError: nil},
+				{first: 1, last: 2, more: true, pkt: pkt(2), wantDone: false, wantError: ErrFragmentOverlap},
 			},
 			want: []hole{
-				{first: 0, last: 3, filled: true, final: false},
-				{first: 4, last: 5, filled: true, final: true},
+				{first: 0, last: 3, filled: true, final: false, pkt: pkt(4)},
+				{first: 4, last: math.MaxUint16, filled: false, final: true},
 			},
-			wantPkt: pkt(4, 2),
 		},
 		{
 			name: "Two overlapping fragments",
@@ -163,7 +161,7 @@ func TestReassemblerProcess(t *testing.T) {
 			name: "Two final fragments - duplicate",
 			params: []processParams{
 				{first: 5, last: 14, more: false, pkt: pkt(10), wantDone: false, wantError: nil},
-				{first: 10, last: 14, more: false, pkt: pkt(5), wantDone: false, wantError: nil},
+				{first: 5, last: 14, more: false, pkt: pkt(10), wantDone: false, wantError: nil},
 			},
 			want: []hole{
 				{first: 5, last: 14, filled: true, final: true, pkt: pkt(10)},

test/packetimpact/tests/ipv4_fragment_reassembly_test.go

@@ -81,12 +81,18 @@ func TestIPv4FragmentReassembly(t *testing.T) {
 			description:  "fragment subset",
 			ipPayloadLen: 3000,
 			fragments: []fragmentInfo{
+				// Overlapping fragment must be dropped.
+				// rfc8200#section-4.5
+				// However, the linux kernel may accept overlaps
+				// depending on the order of the fragments.
+				// For example, if we switch the first and the second fragment,
+				// the linux kernel will accept the packet and the test will fail.
+				{offset: 512, size: 256, id: 8, more: header.IPv4FlagMoreFragments},
 				{offset: 0, size: 1000, id: 8, more: header.IPv4FlagMoreFragments},
 				{offset: 1000, size: 1000, id: 8, more: header.IPv4FlagMoreFragments},
-				{offset: 512, size: 256, id: 8, more: header.IPv4FlagMoreFragments},
 				{offset: 2000, size: 1000, id: 8, more: 0},
 			},
-			expectReply: true,
+			expectReply: false,
 		},
 		{
 			description:  "fragment overlap",

test/packetimpact/tests/ipv6_fragment_reassembly_test.go

@@ -82,12 +82,18 @@ func TestIPv6FragmentReassembly(t *testing.T) {
 			description:  "fragment subset",
 			ipPayloadLen: 3000,
 			fragments: []fragmentInfo{
+				{offset: 512, size: 256, id: 103, more: true},
+				// Overlapping fragment must be dropped.
+				// rfc8200#section-4.5
+				// However, the linux kernel may accept overlaps
+				// depending on the order of the fragments.
+				// For example, if we switch the first and the second fragment,
+				// the linux kernel will accept the packet and the test will fail.
 				{offset: 0, size: 1000, id: 103, more: true},
 				{offset: 1000, size: 1000, id: 103, more: true},
-				{offset: 512, size: 256, id: 103, more: true},
 				{offset: 2000, size: 1000, id: 103, more: false},
 			},
-			expectReply: true,
+			expectReply: false,
 		},
 		{
 			description:  "fragment overlap",