Re: [PATCH net v4] ipv6: Implement limits on extension header parsing

[Justin Iurman <justin.iurman@gmail.com>]

On 4/28/26 17:37, Daniel Borkmann wrote:
> ipv6_{skip_exthdr,find_hdr}() and ip6_{tnl_parse_tlv_enc_lim,
> protocol_deliver_rcu}() iterate over IPv6 extension headers until they
> find a non-extension-header protocol or run out of packet data. The
> loops have no iteration counter, relying solely on the packet length
> to bound them. For a crafted packet with 8-byte extension headers
> filling a 64KB jumbogram, this means a worst case of up to ~8k
> iterations with a skb_header_pointer call each. ipv6_skip_exthdr(),
> for example, is used where it parses the inner quoted packet inside
> an incoming ICMPv6 error:
> 
>    - icmpv6_rcv
>      - checksum validation
>      - case ICMPV6_DEST_UNREACH
>        - icmpv6_notify
>          - pskb_may_pull()       <- pull inner IPv6 header
>          - ipv6_skip_exthdr()    <- iterates here
>          - pskb_may_pull()
>          - ipprot->err_handler() <- sk lookup
> 
> The per-iteration cost of ipv6_skip_exthdr itself is generally
> light, but skb_header_pointer becomes more costly on reassembled
> packets: the first ~1232 bytes of the inner packet are in the skb's
> linear area, but the remaining ~63KB are in the frag_list where
> skb_copy_bits is needed to read data.
> 
> Initially, the idea was to add a configurable limit via a new
> sysctl knob with default 8, in line with knobs from commit
> 47d3d7ac656a ("ipv6: Implement limits on Hop-by-Hop and Destination
> options"), but two reasons eventually argued against it:
> 
> - It adds to UAPI that needs to be maintained forever, and
>    upcoming work is restricting extension header ordering anyway,
>    leaving little reason for another sysctl knob
> - exthdrs_core.c is always built-in even when CONFIG_IPV6=n,
>    where struct net has no .ipv6 member, so the read site would
>    need an ifdef'd fallback to a constant anyway
> 
> Therefore, just use a constant (IP6_MAX_EXT_HDRS_CNT). All four
> extension header walking functions are now bound by this limit.
> 
> Note that the check in ip6_protocol_deliver_rcu() happens right
> before the goto resubmit, such that we don't have to have a test
> for ipv6_ext_hdr() in the fast-path.
> 
> There's an ongoing IETF draft-iurman-6man-eh-occurrences to enforce
> IPv6 extension headers ordering and occurrence. The latter also
> discusses security implications. As per RFC8200 section 4.1, the
> occurrence rules for extension headers provide a practical upper
> bound which is 8. In order to be conservative, let's define
> IP6_MAX_EXT_HDRS_CNT as 4x that to leave enough room for quirky
> setups. In the unlikely event that this is still not enough, then
> we might need to reconsider a sysctl.
> 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

Reviewed-by: Justin Iurman <justin.iurman@gmail.com>

FWIW, I prefer v4 over v3. I didn't like the idea of a new sysctl for 
several reasons I already mentioned. So, thanks for this new version, 
Daniel.

However, I can't help but think 8 would pose absolutely no problem. I 
would be very surprised to see someone complain. We're choosing 32 over 
8 to be extra safe, at the price of security. RFC8200 provides the 
following ordered list which is recommended for senders (without 
normative language, though, which was a mistake):

       Hop-by-Hop Options header
       Destination Options header
       Routing header
       Fragment header
       Authentication header
       Encapsulating Security Payload header
       Destination Options header

So this is the maximum you can have theoretically***, although you 
wouldn't for instance use the Authentication header with ESP. I'm not 
even talking about ordering or specific number of occurrences here, just 
the total number of Extension Headers in a packet (as your patch does). 
It's also worth mentioning that it's highly unlikely to see someone use 
them all at the same time (in production, of course). This is why I 
still think that 8 is safe too, and would provide security as expected.

***well, you also have 3 others [1] (Mobility Header, Host Identity 
Protocol, Shim6 Protocol), but they're not widely used (not to say 
dead). And they would probably not be used with other Extension Headers 
anyway, as they are not specified to be present with a layer-4 in most 
cases.

  [1] 
https://www.iana.org/assignments/ipv6-parameters/ipv6-parameters.xhtml#extension-header