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

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

On 1/26/26 20:48, 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
> 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.
> 
> 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))

Just curious, did you test it? Seems weird, not sure about this code. By 
the way, how about adding a selftest (python+scapy) to test all EH 
combinations? Should be straightforward. Could be added later, not 
necessarily in this series.

I think you inverted IPV6_EXT_HDR_ORDER_ROUTING and 
IPV6_EXT_HDR_ORDER_DEST. Also, (ext_hdrs | IPV6_EXT_HDR_ORDER_DEST) 
should probably be (ext_hdrs & IPV6_EXT_HDR_ORDER_DEST) instead. See my 
proposal below.

> +		return ext_hdrs;
> +
> +	/* We have Destination Options before the Routing Header. Set
> +	 * the mask of recived extension headers to reflect that. We promote

Reported by AI:
s/recived/received

(in case you keep this part)

> +	 * 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;
> +		}
> +

Didn't test your version, neither did I with this attempt, feedback welcome:

#define IPV6_EXT_HDR_ORDER_HOP			BIT(0)
#define IPV6_EXT_HDR_ORDER_DEST			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_DEST2		BIT(6)

[...]

static const struct inet6_protocol destopt_protocol = {
  	.handler	=	ipv6_destopt_rcv,
  	.flags		=	INET6_PROTO_NOPOLICY,
	.ext_hdr_order	=	IPV6_EXT_HDR_ORDER_DEST,
};

[...]

u32 ext_hdrs;
if (ipprot->ext_hdr_order) {
	if (ipprot->ext_hdr_order <= ext_hdrs) {
		if ((ext_hdrs & IPV6_EXT_HDR_ORDER_DEST2) ||
		    ipprot->ext_hdr_order != IPV6_EXT_HDR_ORDER_DEST ||
		    !(ext_hdrs & IPV6_EXT_HDR_ORDER_ROUTING)) {
			goto discard;
		}

		if  (ipprot->ext_hdr_order == IPV6_EXT_HDR_ORDER_DEST)
			ext_hdrs |= IPV6_EXT_HDR_ORDER_DEST2;
	}

	ext_hdrs |= ipprot->ext_hdr_order;
}

... which looks quite similar to yours without the error mentioned above.

>   		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 = {