[PATCH net] xfrm: Interpret XFRM_INF as 32 bit value for non-ESN states

[PATCH net] xfrm: Interpret XFRM_INF as 32 bit value for non-ESN states

[Thomas Egerer]

Currently, when left unconfigured, hard and soft packet limit are set to
XFRM_INF ((__u64)~0). This can be problematic for non-ESN states, as
their 'natural' packet limit is 2^32 - 1 packets. When reached, instead
of creating an expire event, the states become unusable and increase
their respective 'state expired' counter in the xfrm statistics. The
only way for them to actually expire is based on their lifetime limits.

This patch reduces the packet limit of non-ESN states with XFRM_INF as
their soft/hard packet limit to their maximum achievable sequence
number in order to trigger an expire, which can then be used by an IKE
daemon to reestablish the connection.

Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
---
 net/xfrm/xfrm_user.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index b88ba45..84d4008 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -505,6 +505,13 @@ static void copy_from_user_state(struct xfrm_state *x, struct xfrm_usersa_info *
 
 	if (!x->sel.family && !(p->flags & XFRM_STATE_AF_UNSPEC))
 		x->sel.family = p->family;
+
+	if ((x->props.flags & XFRM_STATE_ESN) == 0 {
+		if (x->lft.soft_packet_limit == XFRM_INF)
+			x->lft.soft_packet_limit == (__u32)~0;
+		if (x->lft.hard_packet_limit == XFRM_INF)
+			x->lft.hard_packet_limit == (__u32)~0;
+	}
 }
 
 /*
-- 
2.6.4

Re: [PATCH net] xfrm: Interpret XFRM_INF as 32 bit value for non-ESN states

[David Miller]

From: Thomas Egerer <thomas.egerer@secunet.com>
Date: Mon, 27 Jan 2020 15:31:14 +0100

> Currently, when left unconfigured, hard and soft packet limit are set to
> XFRM_INF ((__u64)~0). This can be problematic for non-ESN states, as
> their 'natural' packet limit is 2^32 - 1 packets. When reached, instead
> of creating an expire event, the states become unusable and increase
> their respective 'state expired' counter in the xfrm statistics. The
> only way for them to actually expire is based on their lifetime limits.
> 
> This patch reduces the packet limit of non-ESN states with XFRM_INF as
> their soft/hard packet limit to their maximum achievable sequence
> number in order to trigger an expire, which can then be used by an IKE
> daemon to reestablish the connection.
> 
> Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>

Please always CC: the ipsec maintainers for patches to IPSEC.

Steffen, I assume I will get this from you.

Thanks.

> ---
>  net/xfrm/xfrm_user.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index b88ba45..84d4008 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -505,6 +505,13 @@ static void copy_from_user_state(struct xfrm_state *x, struct xfrm_usersa_info *
>  
>  	if (!x->sel.family && !(p->flags & XFRM_STATE_AF_UNSPEC))
>  		x->sel.family = p->family;
> +
> +	if ((x->props.flags & XFRM_STATE_ESN) == 0 {
> +		if (x->lft.soft_packet_limit == XFRM_INF)
> +			x->lft.soft_packet_limit == (__u32)~0;
> +		if (x->lft.hard_packet_limit == XFRM_INF)
> +			x->lft.hard_packet_limit == (__u32)~0;
> +	}
>  }
>  
>  /*
> -- 
> 2.6.4
> 

Re: [PATCH net] xfrm: Interpret XFRM_INF as 32 bit value for non-ESN states

[Steffen Klassert]

On Tue, Jan 28, 2020 at 10:54:23AM +0100, David Miller wrote:
> From: Thomas Egerer <thomas.egerer@secunet.com>
> Date: Mon, 27 Jan 2020 15:31:14 +0100
> 
> > Currently, when left unconfigured, hard and soft packet limit are set to
> > XFRM_INF ((__u64)~0). This can be problematic for non-ESN states, as
> > their 'natural' packet limit is 2^32 - 1 packets. When reached, instead
> > of creating an expire event, the states become unusable and increase
> > their respective 'state expired' counter in the xfrm statistics. The
> > only way for them to actually expire is based on their lifetime limits.
> > 
> > This patch reduces the packet limit of non-ESN states with XFRM_INF as
> > their soft/hard packet limit to their maximum achievable sequence
> > number in order to trigger an expire, which can then be used by an IKE
> > daemon to reestablish the connection.
> > 
> > Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
> 
> Please always CC: the ipsec maintainers for patches to IPSEC.
> 
> Steffen, I assume I will get this from you.

Yes, I have it already in my queue.

Re: [PATCH net] xfrm: Interpret XFRM_INF as 32 bit value for non-ESN states

[Steffen Klassert]

On Mon, Jan 27, 2020 at 03:31:14PM +0100, Thomas Egerer wrote:
> Currently, when left unconfigured, hard and soft packet limit are set to
> XFRM_INF ((__u64)~0). This can be problematic for non-ESN states, as
> their 'natural' packet limit is 2^32 - 1 packets. When reached, instead
> of creating an expire event, the states become unusable and increase
> their respective 'state expired' counter in the xfrm statistics. The
> only way for them to actually expire is based on their lifetime limits.
> 
> This patch reduces the packet limit of non-ESN states with XFRM_INF as
> their soft/hard packet limit to their maximum achievable sequence
> number in order to trigger an expire, which can then be used by an IKE
> daemon to reestablish the connection.
> 
> Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
> ---
>  net/xfrm/xfrm_user.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index b88ba45..84d4008 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -505,6 +505,13 @@ static void copy_from_user_state(struct xfrm_state *x, struct xfrm_usersa_info *
>  
>  	if (!x->sel.family && !(p->flags & XFRM_STATE_AF_UNSPEC))
>  		x->sel.family = p->family;
> +
> +	if ((x->props.flags & XFRM_STATE_ESN) == 0 {

You need one more close bracket here:

/home/klassert/git/ipsec/net/xfrm/xfrm_user.c: In function ‘copy_from_user_state’:
/home/klassert/git/ipsec/net/xfrm/xfrm_user.c:509:45: error: expected ‘)’ before ‘{’ token
  if ((x->props.flags & XFRM_STATE_ESN) == 0 {
                                             ^
/home/klassert/git/ipsec/net/xfrm/xfrm_user.c:515:1: error: expected expression before ‘}’ token
 }
 ^

Please fix and resend.

Thanks.

Re: [PATCH net] xfrm: Interpret XFRM_INF as 32 bit value for non-ESN states

[Thomas Egerer]

Hello Steffen,

On 1/30/20 11:34 AM, Steffen Klassert wrote:
> On Mon, Jan 27, 2020 at 03:31:14PM +0100, Thomas Egerer wrote:
> 
> You need one more close bracket here:
> 
> /home/klassert/git/ipsec/net/xfrm/xfrm_user.c: In function ‘copy_from_user_state’:
> /home/klassert/git/ipsec/net/xfrm/xfrm_user.c:509:45: error: expected ‘)’ before ‘{’ token
>   if ((x->props.flags & XFRM_STATE_ESN) == 0 {
>                                              ^
> /home/klassert/git/ipsec/net/xfrm/xfrm_user.c:515:1: error: expected expression before ‘}’ token
>  }
>  ^
> 
> Please fix and resend.
You're right. I wonder how that could have happened. I will rebuild
and make sure, I tested the kernel version including the patch and
not the one without.


Thomas