Re: [PATCH net-next v5 6/7] ipv6: Enforce Extension Header ordering

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

On 1/29/26 06:18, Willem de Bruijn wrote:
> Tom Herbert wrote:
>> RFC8200 highly recommends that different Extension Headers be send in
>> a prescibed order and all Extension Header types occur at most once
>> in a packet with the exception of Destination Options that may
>> occur twice. This patch enforces the ordering be folowed in received
>> packets.
>>
>> The allowed order of Extension Headers is:
>>
>>      IPv6 header
>>      Hop-by-Hop Options header
>>      Destination Options before the Routing Header
>>      Routing header
>>      Fragment header
>>      Authentication header
>>      Encapsulating Security Payload header
>>      Destination Options header
>>      Upper-Layer header
>>
>> Each Extension Header may be present only once in a packet.
>>
>> net.ipv6.enforce_ext_hdr_order is a sysctl to enable or disable
>> enforcement of xtension Header order. If it is set to zero then
> 
> [e]xtension. There are a few more typos in the various commit
> messages.
> 
>> Extension Header order and number of occurences is not checked
>> in receive processeing (except for Hop-by-Hop Options that
>> must be the first Extension Header and can only occur once in
>> a packet.
> 
> RFC 8200 also states
> 
>     "IPv6 nodes must accept and attempt to process extension headers in
>     any order and occurring any number of times in the same packet,
>     except for the Hop-by-Hop Options header, which is restricted to
>     appear immediately after an IPv6 header only. Nonetheless, it is
>     strongly advised that sources of IPv6 packets adhere to the above
>     recommended order until and unless subsequent specifications revise
>     that recommendation."
> 
> A case of be strict in what you send, liberal in what you accept.
> 
> This new sysctl has a chance of breaking existing users.

Willem,

Note that RFC8200 does not use normative language, which is part of the 
problem. It could theoretically break existing users, but I don't think 
it will in reality. For that, you would need users to beginning with 
(joke aside, I like EHs). Anyway, if the order is enforced at sending, 
why would any receiver accept a different order? In this case, being 
liberal in what we accept might be a security risk (see below).

> The series as a whole is framed as a security improvement. Does
> enforcing order help with that?

IMHO, any packet with EHs in a different order than the one specified in 
RFC8200 looks suspicious. So, yes.

Justin

>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>> ---
>>   include/net/netns/ipv6.h   |  1 +
>>   include/net/protocol.h     | 14 +++++++++++++
>>   net/ipv6/af_inet6.c        |  1 +
>>   net/ipv6/exthdrs.c         |  2 ++
>>   net/ipv6/ip6_input.c       | 42 ++++++++++++++++++++++++++++++++++++++
>>   net/ipv6/reassembly.c      |  1 +
>>   net/ipv6/sysctl_net_ipv6.c |  7 +++++++
>>   net/ipv6/xfrm6_protocol.c  |  2 ++
>>   8 files changed, 70 insertions(+)
>>
>> diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
>> index 34bdb1308e8f..2db56718ea60 100644
>> --- a/include/net/netns/ipv6.h
>> +++ b/include/net/netns/ipv6.h
>> @@ -61,6 +61,7 @@ struct netns_sysctl_ipv6 {
>>   	u8 fib_notify_on_flag_change;
>>   	u8 icmpv6_error_anycast_as_unicast;
>>   	u8 icmpv6_errors_extension_mask;
>> +	u8 enforce_ext_hdr_order;
>>   };
>>   
>>   struct netns_ipv6 {
>> diff --git a/include/net/protocol.h b/include/net/protocol.h
>> index b2499f88f8f8..0f1676625570 100644
>> --- a/include/net/protocol.h
>> +++ b/include/net/protocol.h
>> @@ -50,6 +50,19 @@ struct net_protocol {
>>   };
>>   
>>   #if IS_ENABLED(CONFIG_IPV6)
>> +
>> +/* Order of extension headers as prescribed in RFC8200. The ordering and
>> + * number of extension headers in a packet can be enforced in IPv6 receive
>> + * processing.
>> + */
>> +#define IPV6_EXT_HDR_ORDER_HOP			BIT(0)
>> +#define IPV6_EXT_HDR_ORDER_DEST_BEFORE_RH	BIT(1)
>> +#define IPV6_EXT_HDR_ORDER_ROUTING		BIT(2)
>> +#define IPV6_EXT_HDR_ORDER_FRAGMENT		BIT(3)
>> +#define IPV6_EXT_HDR_ORDER_AUTH			BIT(4)
>> +#define IPV6_EXT_HDR_ORDER_ESP			BIT(5)
>> +#define IPV6_EXT_HDR_ORDER_DEST			BIT(6)
>> +
>>   struct inet6_protocol {
>>   	int	(*handler)(struct sk_buff *skb);
>>   
>> @@ -61,6 +74,7 @@ struct inet6_protocol {
>>   
>>   	unsigned int	flags;	/* INET6_PROTO_xxx */
>>   	u32		secret;
>> +	u32		ext_hdr_order;
>>   };
>>   
>>   #define INET6_PROTO_NOPOLICY	0x1
>> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
>> index bd29840659f3..43097360ce64 100644
>> --- a/net/ipv6/af_inet6.c
>> +++ b/net/ipv6/af_inet6.c
>> @@ -980,6 +980,7 @@ static int __net_init inet6_net_init(struct net *net)
>>   	net->ipv6.sysctl.max_dst_opts_len = IP6_DEFAULT_MAX_DST_OPTS_LEN;
>>   	net->ipv6.sysctl.max_hbh_opts_len = IP6_DEFAULT_MAX_HBH_OPTS_LEN;
>>   	net->ipv6.sysctl.fib_notify_on_flag_change = 0;
>> +	net->ipv6.sysctl.enforce_ext_hdr_order = 1;
>>   	atomic_set(&net->ipv6.fib6_sernum, 1);
>>   
>>   	net->ipv6.sysctl.ioam6_id = IOAM6_DEFAULT_ID;
>> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
>> index 6925cfad94d2..4ab94c8cddb9 100644
>> --- a/net/ipv6/exthdrs.c
>> +++ b/net/ipv6/exthdrs.c
>> @@ -845,11 +845,13 @@ static int ipv6_rthdr_rcv(struct sk_buff *skb)
>>   static const struct inet6_protocol rthdr_protocol = {
>>   	.handler	=	ipv6_rthdr_rcv,
>>   	.flags		=	INET6_PROTO_NOPOLICY,
>> +	.ext_hdr_order	=	IPV6_EXT_HDR_ORDER_ROUTING,
>>   };
>>   
>>   static const struct inet6_protocol destopt_protocol = {
>>   	.handler	=	ipv6_destopt_rcv,
>>   	.flags		=	INET6_PROTO_NOPOLICY,
>> +	.ext_hdr_order	=	IPV6_EXT_HDR_ORDER_DEST,
>>   };
>>   
>>   static const struct inet6_protocol nodata_protocol = {
>> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
>> index 168ec07e31cc..ab921c0a94af 100644
>> --- a/net/ipv6/ip6_input.c
>> +++ b/net/ipv6/ip6_input.c
>> @@ -355,6 +355,27 @@ void ipv6_list_rcv(struct list_head *head, struct packet_type *pt,
>>   		ip6_sublist_rcv(&sublist, curr_dev, curr_net);
>>   }
>>   
>> +static u32 check_dst_opts_before_rh(const struct inet6_protocol *ipprot,
>> +				    u32 ext_hdrs)
>> +{
>> +	/* Check if Destination Options before the Routing Header are
>> +	 * present.
>> +	 */
>> +	if (ipprot->ext_hdr_order != IPV6_EXT_HDR_ORDER_ROUTING ||
>> +	    !(ext_hdrs | IPV6_EXT_HDR_ORDER_DEST))
>> +		return ext_hdrs;
>> +
>> +	/* We have Destination Options before the Routing Header. Set
>> +	 * the mask of recived extension headers to reflect that. We promote
>> +	 * the bit from indicating just Destination Options present to
>> +	 * Destination Options before the Routing Header being present
>> +	 */
>> +	ext_hdrs = (ext_hdrs & ~IPV6_EXT_HDR_ORDER_DEST) |
>> +		IPV6_EXT_HDR_ORDER_DEST_BEFORE_RH;
>> +
>> +	return ext_hdrs;
>> +}
>> +
>>   INDIRECT_CALLABLE_DECLARE(int tcp_v6_rcv(struct sk_buff *));
>>   
>>   /*
>> @@ -366,6 +387,7 @@ void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int nexthdr,
>>   	const struct inet6_protocol *ipprot;
>>   	struct inet6_dev *idev;
>>   	unsigned int nhoff;
>> +	u32 ext_hdrs = 0;
>>   	SKB_DR(reason);
>>   	bool raw;
>>   
>> @@ -427,6 +449,26 @@ void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int nexthdr,
>>   				goto discard;
>>   			}
>>   		}
>> +
>> +		if (ipprot->ext_hdr_order &&
>> +		    READ_ONCE(net->ipv6.sysctl.enforce_ext_hdr_order)) {
>> +			/* The protocol is an extension header and EH ordering
>> +			 * is being enforced. Discard packet if we've already
>> +			 * seen this EH or one that is lower in the order list
>> +			 */
>> +			if (ipprot->ext_hdr_order <= ext_hdrs) {
>> +				/* Check if there's Destination Options
>> +				 * before the Routing Header
>> +				 */
>> +				ext_hdrs = check_dst_opts_before_rh(ipprot,
>> +								    ext_hdrs);
>> +				if (ipprot->ext_hdr_order <= ext_hdrs)
>> +					goto discard;
>> +			}
>> +
>> +			ext_hdrs |= ipprot->ext_hdr_order;
>> +		}
>> +
>>   		if (!(ipprot->flags & INET6_PROTO_NOPOLICY)) {
>>   			if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) {
>>   				SKB_DR_SET(reason, XFRM_POLICY);
>> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
>> index 25ec8001898d..91dba72c5a3c 100644
>> --- a/net/ipv6/reassembly.c
>> +++ b/net/ipv6/reassembly.c
>> @@ -414,6 +414,7 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
>>   static const struct inet6_protocol frag_protocol = {
>>   	.handler	=	ipv6_frag_rcv,
>>   	.flags		=	INET6_PROTO_NOPOLICY,
>> +	.ext_hdr_order	=	IPV6_EXT_HDR_ORDER_FRAGMENT,
>>   };
>>   
>>   #ifdef CONFIG_SYSCTL
>> diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
>> index d2cd33e2698d..543b6acdb11d 100644
>> --- a/net/ipv6/sysctl_net_ipv6.c
>> +++ b/net/ipv6/sysctl_net_ipv6.c
>> @@ -213,6 +213,13 @@ static struct ctl_table ipv6_table_template[] = {
>>   		.proc_handler	= proc_doulongvec_minmax,
>>   		.extra2		= &ioam6_id_wide_max,
>>   	},
>> +	{
>> +		.procname	= "enforce_ext_hdr_order",
>> +		.data		= &init_net.ipv6.sysctl.enforce_ext_hdr_order,
>> +		.maxlen		= sizeof(u8),
>> +		.mode		= 0644,
>> +		.proc_handler	= proc_dou8vec_minmax,
>> +	},
>>   };
>>   
>>   static struct ctl_table ipv6_rotable[] = {
>> diff --git a/net/ipv6/xfrm6_protocol.c b/net/ipv6/xfrm6_protocol.c
>> index ea2f805d3b01..5826edf67f64 100644
>> --- a/net/ipv6/xfrm6_protocol.c
>> +++ b/net/ipv6/xfrm6_protocol.c
>> @@ -197,12 +197,14 @@ static const struct inet6_protocol esp6_protocol = {
>>   	.handler	=	xfrm6_esp_rcv,
>>   	.err_handler	=	xfrm6_esp_err,
>>   	.flags		=	INET6_PROTO_NOPOLICY,
>> +	.ext_hdr_order	=	IPV6_EXT_HDR_ORDER_ESP,
>>   };
>>   
>>   static const struct inet6_protocol ah6_protocol = {
>>   	.handler	=	xfrm6_ah_rcv,
>>   	.err_handler	=	xfrm6_ah_err,
>>   	.flags		=	INET6_PROTO_NOPOLICY,
>> +	.ext_hdr_order	=	IPV6_EXT_HDR_ORDER_AUTH
>>   };
>>   
>>   static const struct inet6_protocol ipcomp6_protocol = {
>> -- 
>> 2.43.0
>>
> 
>