ipv6: tighten handling of fragments wrt extension headers

examples/config.yml

@@ -457,6 +457,9 @@ firewall:
   # Rules are comprised of a protocol, port, and one or more of host, group, or CIDR
   # Logical evaluation is roughly: port AND proto AND (ca_sha OR ca_name) AND (host OR group OR groups OR cidr) AND (local cidr)
   # - port: Takes `0` or `any` as any, a single number `80`, a range `200-901`, or `fragment` to match second and further fragments of fragmented packets (since there is no port available).
+  #         For IPv6, Nebula can't tell which protocol a second or further fragment belongs to. Any rule that allows them (port `any` or `fragment`,
+  #         and every icmp rule, since icmp ignores ports) allows them for every protocol, not just the rule's. The first fragment, which carries
+  #         the ports, is always checked against its real protocol and port, so this never opens a port the rules don't allow.
   #   proto: `any`, `tcp`, `udp`, or `icmp`
   #          a port specification is ignored if proto is `icmp`
   #   host: `any` or a literal hostname, ie `test-host`

iputil/packet.go

@@ -11,7 +11,7 @@ import (
 // ErrIPv6CouldNotFindPayload is returned when the ipv6 extension header chain is truncated before a terminal
 // upper layer protocol is reached.
 var ErrIPv6CouldNotFindPayload = errors.New("could not find payload in ipv6 packet")
-var ErrIPv6FragmentHeadersNotTransport = errors.New("non-transport header after fragment header in packet")
+var ErrIPv6ExtensionHeaderAfterFragment = errors.New("extension header after fragment header in packet")
 var ErrIPv6HopByHopHeadersMustBeFirst = errors.New("hop-by-hop header must be first extension header")
 
 const (
@@ -354,20 +354,15 @@ func ipv6CreateRejectTCPPacket(packet []byte, out []byte, offset int) []byte {
 	return out
 }
 
-func nextHeaderIsTransport(nextHeader uint8) error {
-	switch nextHeader {
-	case IPProtocolTCP, IPProtocolUDP, IPProtocolICMPv6, IPProtocolICMP:
-		return nil
-	default:
-		return ErrIPv6FragmentHeadersNotTransport
-	}
-}
-
 // IPv6FindUpperProtocol walks the ipv6 extension header chain and returns the upper layer protocol, the offset it begins at, and whether the packet is a non-first fragment.
 // Only the RFC 8200 and IANA extension headers below are walked.
 // Everything else, including Mobility (135), HIP (139), Shim6 (140), experimental 253/254, and real upper layer protocols like SCTP or GRE, is terminal.
 // For a non-first fragment the returned protocol is the fragmented protocol and offset points at the fragment header, there is no transport header to locate.
 // Returns ErrIPv6CouldNotFindPayload if packet is smaller than an ipv6 header, or the chain is truncated, or if more than 8 extension headers are present.
+// Returns ErrIPv6HopByHopHeadersMustBeFirst if a hop-by-hop header appears anywhere but directly after the ipv6 header (RFC 8200 section 4.1).
+// Returns ErrIPv6ExtensionHeaderAfterFragment if any walked extension header, including a second fragment header, follows the fragment header of a
+// first fragment. The upper layer header must then begin at byte 0 of the fragmentable part, beyond the reach of any non-first fragment,
+// whose offset is at least 8, so overlapping fragments cannot rewrite the ports or protocol the firewall read from the first fragment.
 func IPv6FindUpperProtocol(packet []byte) (nextHeader uint8, offset int, isFragment bool, anyFragment bool, err error) {
 	const maxIPv6ExtHeaders = 8
 	if len(packet) < ipv6.HeaderLen {
@@ -379,6 +374,9 @@ func IPv6FindUpperProtocol(packet []byte) (nextHeader uint8, offset int, isFragm
 	for range maxIPv6ExtHeaders {
 		switch nextHeader {
 		case IPv6HeaderHopByHop, IPv6HeaderRouting, IPv6HeaderDestination:
+			if anyFragment {
+				return 0, 0, false, false, ErrIPv6ExtensionHeaderAfterFragment
+			}
 			if len(packet) < offset+2 {
 				return 0, 0, false, false, ErrIPv6CouldNotFindPayload
 			}
@@ -392,37 +390,27 @@ func IPv6FindUpperProtocol(packet []byte) (nextHeader uint8, offset int, isFragm
 			offset += (int(packet[offset+1]) + 1) << 3
 
 		case IPv6HeaderFragment:
+			if anyFragment {
+				return 0, 0, false, false, ErrIPv6ExtensionHeaderAfterFragment
+			}
 			if len(packet) < offset+8 {
 				return 0, 0, false, false, ErrIPv6CouldNotFindPayload
 			}
 
 			anyFragment = true
+			// Non-first fragments carry no transport header, report the fragmented protocol and stop.
+			// Per RFC 8200 section 4.5 the receiver only uses the offset zero fragment's next header, so the value
+			// returned here can't be verified.
+			if packet[offset+2] != 0 || packet[offset+3]&0xf8 != 0 {
+				return packet[offset], offset, true, anyFragment, nil
+			}
 			nextHeader = packet[offset]
-
+			offset += 8
-			//because all these branches are terminal, we reject multiple fragment headers as well
-
-			// we always require a transport-layer header to follow a fragment header:
-			if err = nextHeaderIsTransport(nextHeader); err != nil {
-				return 0, 0, false, false, err
-			}
-			if packet[offset+2] == 0 && packet[offset+3]&0xf8 == 0 {
-				// This is a first-fragment
-				offset += 8
-
-				if offset > len(packet) {
-					// one more sanity check for length
-					return 0, 0, false, false, ErrIPv6CouldNotFindPayload
-				}
-				return nextHeader, offset, isFragment, anyFragment, nil
-			}
-			// Non-first fragments carry no transport header, and are terminal.
-			// However, per RFC 8200 §4.5, the "next header" of a non-first-fragment is not actually consulted during reassembly
-
-			// todo nextHeader is constrained to ICMP/TCP/UDP by nextHeaderIsTransport
-			// todo but should we make "fragment" a firewall-level protocol?
-			return nextHeader, offset, true, anyFragment, nil
 
 		case IPv6HeaderAuthentication: // AH
+			if anyFragment {
+				return 0, 0, false, false, ErrIPv6ExtensionHeaderAfterFragment
+			}
 			if len(packet) < offset+2 {
 				return 0, 0, false, false, ErrIPv6CouldNotFindPayload
 			}

iputil/packet_test.go

@@ -522,21 +522,29 @@ func Test_IPv6FindUpperProtocol(t *testing.T) {
 	dst := net.ParseIP("fd00::2")
 
 	// 8 byte extension/transport stand-ins, first byte is the next header, second is the length field
-	extToTCP := []byte{6, 0, 0, 0, 0, 0, 0, 0}        // len 0 -> 8 bytes, next = TCP
+	extToTCP := []byte{6, 0, 0, 0, 0, 0, 0, 0}           // len 0 -> 8 bytes, next = TCP
-	extToUDP := []byte{17, 0, 0, 0, 0, 0, 0, 0}       // len 0 -> 8 bytes, next = UDP
+	extToUDP := []byte{17, 0, 0, 0, 0, 0, 0, 0}          // len 0 -> 8 bytes, next = UDP
-	extToRouting := []byte{43, 0, 0, 0, 0, 0, 0, 0}   // len 0 -> 8 bytes, next = Routing
+	extToRouting := []byte{43, 0, 0, 0, 0, 0, 0, 0}      // len 0 -> 8 bytes, next = Routing
-	ahToUDP := []byte{17, 0, 0, 0, 0, 0, 0, 0}        // AH len 0 -> (0+2)<<2 = 8 bytes, next = UDP
+	ahToUDP := []byte{17, 0, 0, 0, 0, 0, 0, 0}           // AH len 0 -> (0+2)<<2 = 8 bytes, next = UDP
-	firstFragToUDP := []byte{17, 0, 0, 1, 0, 0, 0, 1} // frag offset 0, M=1, next = UDP
+	firstFragToUDP := []byte{17, 0, 0, 1, 0, 0, 0, 1}    // frag offset 0, M=1, next = UDP
-	nonFirstFrag := []byte{17, 0, 0, 9, 0, 0, 0, 1}   // frag offset non-zero, next = UDP
+	nonFirstFrag := []byte{17, 0, 0, 9, 0, 0, 0, 1}      // frag offset non-zero, next = UDP
-	transport := []byte{0, 80, 1, 187, 0, 0, 0, 0}    // stand-in bytes, IPv6FindUpperProtocol never reads ports
+	transport := []byte{0, 80, 1, 187, 0, 0, 0, 0}       // stand-in bytes, IPv6FindUpperProtocol never reads ports
-	extToHbH := []byte{0, 0, 0, 0, 0, 0, 0, 0}        // len 0 -> 8 bytes, next = Hop-by-Hop
+	extToHbH := []byte{0, 0, 0, 0, 0, 0, 0, 0}           // len 0 -> 8 bytes, next = Hop-by-Hop
+	extToDst := []byte{60, 0, 0, 0, 0, 0, 0, 0}          // len 0 -> 8 bytes, next = Destination
+	extToFrag := []byte{44, 0, 0, 0, 0, 0, 0, 0}         // len 0 -> 8 bytes, next = Fragment
+	firstFragToDst := []byte{60, 0, 0, 1, 0, 0, 0, 1}    // frag offset 0, M=1, next = Destination
+	firstFragToAH := []byte{51, 0, 0, 1, 0, 0, 0, 1}     // frag offset 0, M=1, next = AH
+	firstFragToFrag := []byte{44, 0, 0, 1, 0, 0, 0, 1}   // frag offset 0, M=1, next = Fragment
+	firstFragToESP := []byte{50, 0, 0, 1, 0, 0, 0, 1}    // frag offset 0, M=1, next = ESP
+	atomicFragToDst := []byte{60, 0, 0, 0, 0, 0, 0, 1}   // frag offset 0, M=0, next = Destination
+	nonFirstFragToDst := []byte{60, 0, 0, 9, 0, 0, 0, 1} // frag offset non-zero, next = Destination
 
 	// 8 walked headers spend the budget before the 9th slot's terminal is ever classified. This is the
 	// report 4040546 shape: without failing closed the walk hands back offset 104, and a caller reads ports
-	// from the attacker's bytes there.
+	// from the attacker's bytes there. Destination is the header that may legally repeat.
 	var budgetChain []byte
 	for range 7 {
-		budgetChain = append(budgetChain, extToHbH...)
+		budgetChain = append(budgetChain, extToDst...)
 	}
 	budgetChain = append(budgetChain, extToTCP...)
 	budgetChain = append(budgetChain, transport...)
@@ -559,11 +567,20 @@ func Test_IPv6FindUpperProtocol(t *testing.T) {
 		{"ah then udp", 51, append(ahToUDP, transport...), 17, ipv6.HeaderLen + 8, false, false, nil},
 		{"first fragment walks to transport", 44, append(firstFragToUDP, transport...), 17, ipv6.HeaderLen + 8, false, true, nil},
 		{"non-first fragment stops", 44, append(nonFirstFrag, transport...), 17, ipv6.HeaderLen, true, true, nil},
+		{"non-first fragment reports its claimed protocol", 44, append(nonFirstFragToDst, transport...), 60, ipv6.HeaderLen, true, true, nil},
+		{"routing then first fragment", 43, append(append(extToFrag, firstFragToUDP...), transport...), 17, ipv6.HeaderLen + 16, false, true, nil},
+		{"first fragment of a non-transport protocol is terminal", 44, append(firstFragToESP, transport...), 50, ipv6.HeaderLen + 8, false, true, nil},
+		{"destination after first fragment", 44, append(append(firstFragToDst, extToTCP...), transport...), 0, 0, false, false, ErrIPv6ExtensionHeaderAfterFragment},
+		{"ah after first fragment", 44, append(append(firstFragToAH, ahToUDP...), transport...), 0, 0, false, false, ErrIPv6ExtensionHeaderAfterFragment},
+		{"fragment after first fragment", 44, append(append(firstFragToFrag, nonFirstFrag...), transport...), 0, 0, false, false, ErrIPv6ExtensionHeaderAfterFragment},
+		{"destination after atomic fragment", 44, append(append(atomicFragToDst, extToTCP...), transport...), 0, 0, false, false, ErrIPv6ExtensionHeaderAfterFragment},
+		{"hop-by-hop after destination", 60, append(append(extToHbH, extToTCP...), transport...), 0, 0, false, false, ErrIPv6HopByHopHeadersMustBeFirst},
+		{"hop-by-hop twice", 0, append(append(extToHbH, extToTCP...), transport...), 0, 0, false, false, ErrIPv6HopByHopHeadersMustBeFirst},
 		{"unknown protocol is terminal", 132, transport, 132, ipv6.HeaderLen, false, false, nil}, // SCTP
 		{"truncated extension header", 0, nil, 0, 0, false, false, ErrIPv6CouldNotFindPayload},
 		// Destination Options with a declared length (255+1)*8 = 2048 that runs past the 48 byte buffer, next = SCTP
 		{"extension length past buffer", 0, []byte{132, 255, 0, 0, 0, 0, 0, 0}, 0, 0, false, false, ErrIPv6CouldNotFindPayload},
-		{"budget exhausted fails closed", 0, budgetChain, 0, 0, false, false, ErrIPv6CouldNotFindPayload},
+		{"budget exhausted fails closed", 60, budgetChain, 0, 0, false, false, ErrIPv6CouldNotFindPayload},
 	}
 
 	for _, tt := range tests {

outside_test.go

@@ -710,6 +710,26 @@ func Test_newPacket_v6ExtHeaderPastBuffer(t *testing.T) {
 	require.ErrorIs(t, newPacket(pkt, true, p), iputil.ErrIPv6CouldNotFindPayload)
 }
 
+// Test_newPacket_v6ExtHeaderAfterFragment rejects a first fragment whose transport header sits behind another extension
+// header. The ports would then start 8 or more bytes into the fragmentable part, where a non-first fragment, filtered
+// only as `port: fragment`, could overlap them on a receiver that splices overlapping fragments.
+func Test_newPacket_v6ExtHeaderAfterFragment(t *testing.T) {
+	p := &firewall.ParsedPacket{}
+
+	pkt := make([]byte, 60)
+	pkt[0] = 0x60                                    // version 6
+	pkt[6] = byte(layers.IPProtocolIPv6Fragment)     // NextHeader -> Fragment
+	pkt[7] = 64                                      // hop limit
+	pkt[40] = byte(layers.IPProtocolIPv6Destination) // Fragment NextHeader -> Destination Options
+	pkt[43] = 0x01                                   // offset 0, M=1: a first fragment
+	pkt[48] = byte(iputil.IPProtocolTCP)             // Destination Options NextHeader -> TCP
+	pkt[49] = 0                                      // HdrExtLen 0 -> 8 bytes, TCP begins at 56
+	binary.BigEndian.PutUint16(pkt[58:60], 443)      // TCP dst port, reachable by a fragment at offset 8
+
+	require.ErrorIs(t, newPacket(pkt, true, p), iputil.ErrIPv6ExtensionHeaderAfterFragment)
+	require.ErrorIs(t, newPacket(pkt, false, p), iputil.ErrIPv6ExtensionHeaderAfterFragment)
+}
+
 // Test_newPacket_v6ExtHeaderConfusion is a regression test for parseV6 walking any unrecognized
 // Next Header as if it were an ipv6 extension header. A real upper layer protocol Nebula doesn't
 // dissect (SCTP here) is not walkable, so applying the (len+1)*8 formula marched into the SCTP