diff --git a/pkg/tcpip/header/ipv6.go b/pkg/tcpip/header/ipv6.go index ae9e5d803c..06ce069f0b 100644 --- a/pkg/tcpip/header/ipv6.go +++ b/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 diff --git a/pkg/tcpip/link/fdbased/processors.go b/pkg/tcpip/link/fdbased/processors.go index de6aca8cf1..05e997b842 100644 --- a/pkg/tcpip/link/fdbased/processors.go +++ b/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 } diff --git a/pkg/tcpip/network/internal/fragmentation/reassembler.go b/pkg/tcpip/network/internal/fragmentation/reassembler.go index 233f43a00e..5dc51c93ce 100644 --- a/pkg/tcpip/network/internal/fragmentation/reassembler.go +++ b/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 } diff --git a/pkg/tcpip/network/internal/fragmentation/reassembler_test.go b/pkg/tcpip/network/internal/fragmentation/reassembler_test.go index 943bfd2757..88d2a3bead 100644 --- a/pkg/tcpip/network/internal/fragmentation/reassembler_test.go +++ b/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)}, diff --git a/test/packetimpact/tests/ipv4_fragment_reassembly_test.go b/test/packetimpact/tests/ipv4_fragment_reassembly_test.go index 150c0d0b04..8610fc51ae 100644 --- a/test/packetimpact/tests/ipv4_fragment_reassembly_test.go +++ b/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", diff --git a/test/packetimpact/tests/ipv6_fragment_reassembly_test.go b/test/packetimpact/tests/ipv6_fragment_reassembly_test.go index cd537b00a4..790972aa0c 100644 --- a/test/packetimpact/tests/ipv6_fragment_reassembly_test.go +++ b/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",