[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CALx6S37u+8b=QRGVfBFtbq7m-Ei3CvWaSmz_seRHZL5izd4ong@mail.gmail.com>
Date: Mon, 26 Jan 2026 11:38:11 -0800
From: Tom Herbert <tom@...bertland.com>
To: Justin Iurman <justin.iurman@...il.com>
Cc: davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v4 6/7] ipv6: Enforce Extension Header ordering
On Mon, Jan 26, 2026 at 11:23 AM Justin Iurman <justin.iurman@...il.com> wrote:
>
> On 1/26/26 17:56, Tom Herbert wrote:
> > On Fri, Jan 23, 2026 at 1:35 PM Justin Iurman <justin.iurman@...il.com> wrote:
> >>
> >> On 1/21/26 22:49, 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 expection of Destination Options that may
> >>> occur twice. This patch enforces the ordering be folowed in received
> >>> packets. It also disregrards Destination Options before the Routing
> >>> Header as those are unused in deployment asnd there is a proposal
> >>> to deprecate them in draft-herbert-deprecate-destops-before-rh.
> >>
> >> I'll have to pushback on this specifically, at least as long as
> >> draft-herbert-deprecate-destops-before-rh is not adopted and published
> >> (to be clear, I'm not against it, just that it's still an individual
> >> draft right now and there is no consensus). We're already at the edge of
> >> remaining compliant with RFC8200 with this series (which is fine
> >> considering its security gaps), so let's not rush by removing the
> >> DestOps before the Routing header. It can be done later, this series
> >> already mitigates DoS attacks quite well.
> >
> > Justin,
> >
> > If we need to wait for permission from IETF then pretty much most of
> > this patch set isn't viable. For instance
> > draft-iurman-6man-eh-occurrences is also not adopted and published
> > either, and if we enforce the number of occurrences in implementation
> > then we are non-conformant with RFC8200. But I think it is justified
> > to protect users from an obvious DoS attack.
>
> Hi Tom,
>
> This is why I mentioned that, with this series, we're on the edge
> regarding RFC8200 compliance. However, the text in Sec. 4.6 is somewhat
> ambiguous and does not use normative language. Therefore, this series
> does not actually violate RFC8200 compliance.
>
> On the other hand, removing the DestOpt before RH would completely
> violate RFC8200 and, IMO, is not necessary at the moment (because this
> series already mitigates quite well DoS attacks).
>
> > As for Destination Options before the Routing header, _no_one_ is
> > using this. There is no deployment of that. The only routing header
> > being deployed is SRv6 and the header has its own options, no routers
> > support Destination Options before the Routing Header. But, if by some
> > miracle someone is using them in their network, then they can simply
> > set the sysctl to not enforce EH ordering. IMO, this approach is
> > better than adding additional complexity to the stack just to detect a
> > case that nobody cares about and a code path that is never exercised.
>
> What if I want to enforce EH ordering in my network *AND* use the
> DestOpt before RH? I don't think we should make it an exclusive
> condition. And, as discussed offline, draft-bonica-6man-crh-helper-opt
> defines a DestOpt before RH. Therefore, we have at least one use case
> (still a draft, though) in limited domains. Let's wait and see if this
> draft progresses, or if draft-herbert-deprecate-destops-before-rh gets
> consensus.
Okay, I'll submit a new patch set shortly.
Tom
>
> Justin
>
> > Tom
> >
> >>
> >> FWIW, the rest of this patch makes sense.
> >>
> >>> The allowed order of Extension Headers is:
> >>>
> >>> IPv6 header
> >>> Hop-by-Hop Options 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@...bertland.com>
> >>> ---
> >>> include/net/netns/ipv6.h | 1 +
> >>> include/net/protocol.h | 16 ++++++++++++++++
> >>> net/ipv6/af_inet6.c | 1 +
> >>> net/ipv6/exthdrs.c | 2 ++
> >>> net/ipv6/ip6_input.c | 14 ++++++++++++++
> >>> net/ipv6/reassembly.c | 1 +
> >>> net/ipv6/sysctl_net_ipv6.c | 7 +++++++
> >>> net/ipv6/xfrm6_protocol.c | 2 ++
> >>> 8 files changed, 44 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..70cc2d0fdc0c 100644
> >>> --- a/include/net/protocol.h
> >>> +++ b/include/net/protocol.h
> >>> @@ -50,6 +50,21 @@ 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. Destination Options before the Routing Header is not included
> >>> + * in the list as they are unused in deployment and it has been proposed that
> >>> + * they be deprecated. See:
> >>> + * www.ietf.org/archive/id/draft-herbert-deprecate-destops-before-rh-01.txt
> >>> + */
> >>> +#define IPV6_EXT_HDR_ORDER_HOP BIT(0)
> >>> +#define IPV6_EXT_HDR_ORDER_ROUTING BIT(1)
> >>> +#define IPV6_EXT_HDR_ORDER_FRAGMENT BIT(2)
> >>> +#define IPV6_EXT_HDR_ORDER_AUTH BIT(3)
> >>> +#define IPV6_EXT_HDR_ORDER_ESP BIT(4)
> >>> +#define IPV6_EXT_HDR_ORDER_DEST BIT(5)
> >>> +
> >>> struct inet6_protocol {
> >>> int (*handler)(struct sk_buff *skb);
> >>>
> >>> @@ -61,6 +76,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 394e3397e4d4..2af16694db46 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..5254ead5c226 100644
> >>> --- a/net/ipv6/ip6_input.c
> >>> +++ b/net/ipv6/ip6_input.c
> >>> @@ -366,6 +366,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 +428,19 @@ 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)
> >>> + 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 = {
> >>
>
Powered by blists - more mailing lists