From 1519dbbf8ec9caffeccd86f0aa36846bd6592e75 Mon Sep 17 00:00:00 2001 From: JackDoan Date: Tue, 29 Sep 2026 13:19:43 -0500 Subject: [PATCH] ipv6: tighten handling of fragments wrt extension headers --- examples/config.yml | 3 +++ iputil/packet.go | 54 +++++++++++++++++-------------------------- iputil/packet_test.go | 39 ++++++++++++++++++++++--------- outside_test.go | 20 ++++++++++++++++ 4 files changed, 72 insertions(+), 44 deletions(-) diff --git a/examples/config.yml b/examples/config.yml index 20a85883..36f34137 100644 --- a/examples/config.yml +++ b/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` diff --git a/iputil/packet.go b/iputil/packet.go index 728182f9..3785ff92 100644 --- a/iputil/packet.go +++ b/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] - - //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 + offset += 8 case IPv6HeaderAuthentication: // AH + if anyFragment { + return 0, 0, false, false, ErrIPv6ExtensionHeaderAfterFragment + } if len(packet) < offset+2 { return 0, 0, false, false, ErrIPv6CouldNotFindPayload } diff --git a/iputil/packet_test.go b/iputil/packet_test.go index b88a7926..bda3290f 100644 --- a/iputil/packet_test.go +++ b/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 - 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 - 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 - 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 - extToHbH := []byte{0, 0, 0, 0, 0, 0, 0, 0} // len 0 -> 8 bytes, next = Hop-by-Hop + 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 + 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 + 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 + 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 + 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 { diff --git a/outside_test.go b/outside_test.go index cf76fc47..548d6ed5 100644 --- a/outside_test.go +++ b/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