| Message ID | f7492e95b2a838f78032424a18c3509e0faacba5.1713874887.git.antony.antony@secunet.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | xfrm: Introduce direction attribute for SA | expand |
Le 23/04/2024 à 14:50, Antony Antony a écrit : > Introduces validation for the x->dir attribute within the XFRM input > data lookup path. If the configured direction does not match the > expected direction, input, increment the XfrmInStateDirError counter > and drop the packet to ensure data integrity and correct flow handling. > > grep -vw 0 /proc/net/xfrm_stat > XfrmInStateDirError 1 > > Signed-off-by: Antony Antony <antony.antony@secunet.com> > --- > v11 -> 12 > - add documentation to xfrm_proc.rst > > v10->v11 > - rename error s/XfrmInDirError/XfrmInStateDirError/ > --- > Documentation/networking/xfrm_proc.rst | 3 +++ > include/uapi/linux/snmp.h | 1 + > net/ipv6/xfrm6_input.c | 7 +++++++ > net/xfrm/xfrm_input.c | 11 +++++++++++ > net/xfrm/xfrm_proc.c | 1 + > 5 files changed, 23 insertions(+) > > diff --git a/Documentation/networking/xfrm_proc.rst b/Documentation/networking/xfrm_proc.rst > index c237bef03fb6..b4f4d9552dea 100644 > --- a/Documentation/networking/xfrm_proc.rst > +++ b/Documentation/networking/xfrm_proc.rst > @@ -73,6 +73,9 @@ XfrmAcquireError: > XfrmFwdHdrError: > Forward routing of a packet is not allowed > > +XfrmInStateDirError: > + State direction input mismatched with lookup path direction It's a bit confusing because when this error occurs, the state direction is not 'input'. This statistic is under 'Inbound errors', so may something like this is enough: 'State direction is output.' Regards, Nicolas
2024-04-23, 17:27:37 +0200, Nicolas Dichtel wrote: > Le 23/04/2024 à 14:50, Antony Antony a écrit : > > Introduces validation for the x->dir attribute within the XFRM input > > data lookup path. If the configured direction does not match the > > expected direction, input, increment the XfrmInStateDirError counter > > and drop the packet to ensure data integrity and correct flow handling. > > > > grep -vw 0 /proc/net/xfrm_stat > > XfrmInStateDirError 1 > > > > Signed-off-by: Antony Antony <antony.antony@secunet.com> > > --- > > v11 -> 12 > > - add documentation to xfrm_proc.rst > > > > v10->v11 > > - rename error s/XfrmInDirError/XfrmInStateDirError/ > > --- > > Documentation/networking/xfrm_proc.rst | 3 +++ > > include/uapi/linux/snmp.h | 1 + > > net/ipv6/xfrm6_input.c | 7 +++++++ > > net/xfrm/xfrm_input.c | 11 +++++++++++ > > net/xfrm/xfrm_proc.c | 1 + > > 5 files changed, 23 insertions(+) > > > > diff --git a/Documentation/networking/xfrm_proc.rst b/Documentation/networking/xfrm_proc.rst > > index c237bef03fb6..b4f4d9552dea 100644 > > --- a/Documentation/networking/xfrm_proc.rst > > +++ b/Documentation/networking/xfrm_proc.rst > > @@ -73,6 +73,9 @@ XfrmAcquireError: > > XfrmFwdHdrError: > > Forward routing of a packet is not allowed > > > > +XfrmInStateDirError: > > + State direction input mismatched with lookup path direction > It's a bit confusing because when this error occurs, the state direction is not > 'input'. Agree. > This statistic is under 'Inbound errors', so may something like this is enough: > 'State direction is output.' Maybe something like: State direction mismatch (lookup found an output state on the input path, expected input or no direction) It's a bit verbose, but I think those extra details would help users understand what went wrong.
Le 24/04/2024 à 10:40, Sabrina Dubroca a écrit : [snip] >>> diff --git a/Documentation/networking/xfrm_proc.rst b/Documentation/networking/xfrm_proc.rst >>> index c237bef03fb6..b4f4d9552dea 100644 >>> --- a/Documentation/networking/xfrm_proc.rst >>> +++ b/Documentation/networking/xfrm_proc.rst >>> @@ -73,6 +73,9 @@ XfrmAcquireError: >>> XfrmFwdHdrError: >>> Forward routing of a packet is not allowed >>> >>> +XfrmInStateDirError: >>> + State direction input mismatched with lookup path direction >> It's a bit confusing because when this error occurs, the state direction is not >> 'input'. > > Agree. > >> This statistic is under 'Inbound errors', so may something like this is enough: >> 'State direction is output.' > > Maybe something like: > > State direction mismatch (lookup found an output state on the input path, expected input or no direction) > > It's a bit verbose, but I think those extra details would help users > understand what went wrong. > Sure, it's ok for me.
On Wed, Apr 24, 2024 at 12:04:12PM +0200, Nicolas Dichtel via Devel wrote: > Le 24/04/2024 à 10:40, Sabrina Dubroca a écrit : > [snip] > >>> diff --git a/Documentation/networking/xfrm_proc.rst b/Documentation/networking/xfrm_proc.rst > >>> index c237bef03fb6..b4f4d9552dea 100644 > >>> --- a/Documentation/networking/xfrm_proc.rst > >>> +++ b/Documentation/networking/xfrm_proc.rst > >>> @@ -73,6 +73,9 @@ XfrmAcquireError: > >>> XfrmFwdHdrError: > >>> Forward routing of a packet is not allowed > >>> > >>> +XfrmInStateDirError: > >>> + State direction input mismatched with lookup path direction > >> It's a bit confusing because when this error occurs, the state direction is not > >> 'input'. > > > > Agree. > > > >> This statistic is under 'Inbound errors', so may something like this is enough: > >> 'State direction is output.' > > > > Maybe something like: > > > > State direction mismatch (lookup found an output state on the input path, expected input or no direction) This is better. Fixed in v13. > > > > It's a bit verbose, but I think those extra details would help users > > understand what went wrong. > > > Sure, it's ok for me. thanks, -antony
diff --git a/Documentation/networking/xfrm_proc.rst b/Documentation/networking/xfrm_proc.rst index c237bef03fb6..b4f4d9552dea 100644 --- a/Documentation/networking/xfrm_proc.rst +++ b/Documentation/networking/xfrm_proc.rst @@ -73,6 +73,9 @@ XfrmAcquireError: XfrmFwdHdrError: Forward routing of a packet is not allowed +XfrmInStateDirError: + State direction input mismatched with lookup path direction + Outbound errors ~~~~~~~~~~~~~~~ XfrmOutError: diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h index 23792b8412bd..adf5fd78dd50 100644 --- a/include/uapi/linux/snmp.h +++ b/include/uapi/linux/snmp.h @@ -338,6 +338,7 @@ enum LINUX_MIB_XFRMOUTSTATEINVALID, /* XfrmOutStateInvalid */ LINUX_MIB_XFRMACQUIREERROR, /* XfrmAcquireError */ LINUX_MIB_XFRMOUTSTATEDIRERROR, /* XfrmOutStateDirError */ + LINUX_MIB_XFRMINSTATEDIRERROR, /* XfrmInStateDirError */ __LINUX_MIB_XFRMMAX }; diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c index 2c6aeb090b7a..d5bac0d76b6e 100644 --- a/net/ipv6/xfrm6_input.c +++ b/net/ipv6/xfrm6_input.c @@ -266,6 +266,13 @@ int xfrm6_input_addr(struct sk_buff *skb, xfrm_address_t *daddr, if (!x) continue; + if (unlikely(x->dir && x->dir != XFRM_SA_DIR_IN)) { + XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEDIRERROR); + xfrm_state_put(x); + x = NULL; + continue; + } + spin_lock(&x->lock); if ((!i || (x->props.flags & XFRM_STATE_WILDRECV)) && diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c index 161f535c8b94..71b42de6e3c9 100644 --- a/net/xfrm/xfrm_input.c +++ b/net/xfrm/xfrm_input.c @@ -466,6 +466,11 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type) if (encap_type < 0 || (xo && xo->flags & XFRM_GRO)) { x = xfrm_input_state(skb); + if (unlikely(x->dir && x->dir != XFRM_SA_DIR_IN)) { + XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEDIRERROR); + goto drop; + } + if (unlikely(x->km.state != XFRM_STATE_VALID)) { if (x->km.state == XFRM_STATE_ACQ) XFRM_INC_STATS(net, LINUX_MIB_XFRMACQUIREERROR); @@ -571,6 +576,12 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type) goto drop; } + if (unlikely(x->dir && x->dir != XFRM_SA_DIR_IN)) { + XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEDIRERROR); + xfrm_state_put(x); + goto drop; + } + skb->mark = xfrm_smark_get(skb->mark, x); sp->xvec[sp->len++] = x; diff --git a/net/xfrm/xfrm_proc.c b/net/xfrm/xfrm_proc.c index 98606f1078f7..eeb984be03a7 100644 --- a/net/xfrm/xfrm_proc.c +++ b/net/xfrm/xfrm_proc.c @@ -42,6 +42,7 @@ static const struct snmp_mib xfrm_mib_list[] = { SNMP_MIB_ITEM("XfrmOutStateInvalid", LINUX_MIB_XFRMOUTSTATEINVALID), SNMP_MIB_ITEM("XfrmAcquireError", LINUX_MIB_XFRMACQUIREERROR), SNMP_MIB_ITEM("XfrmOutStateDirError", LINUX_MIB_XFRMOUTSTATEDIRERROR), + SNMP_MIB_ITEM("XfrmInStateDirError", LINUX_MIB_XFRMINSTATEDIRERROR), SNMP_MIB_SENTINEL };
Introduces validation for the x->dir attribute within the XFRM input data lookup path. If the configured direction does not match the expected direction, input, increment the XfrmInStateDirError counter and drop the packet to ensure data integrity and correct flow handling. grep -vw 0 /proc/net/xfrm_stat XfrmInStateDirError 1 Signed-off-by: Antony Antony <antony.antony@secunet.com> --- v11 -> 12 - add documentation to xfrm_proc.rst v10->v11 - rename error s/XfrmInDirError/XfrmInStateDirError/ --- Documentation/networking/xfrm_proc.rst | 3 +++ include/uapi/linux/snmp.h | 1 + net/ipv6/xfrm6_input.c | 7 +++++++ net/xfrm/xfrm_input.c | 11 +++++++++++ net/xfrm/xfrm_proc.c | 1 + 5 files changed, 23 insertions(+) -- 2.30.2