diff mbox series

[ipsec-next,v5,05/17] xfrm: netlink: add config (netlink) options

Message ID 20240714202246.1573817-6-chopps@chopps.org (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series Add IP-TFS mode to xfrm | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Christian Hopps July 14, 2024, 8:22 p.m. UTC 
From: Christian Hopps <chopps@labn.net>

Add netlink options for configuring IP-TFS SAs.

Signed-off-by: Christian Hopps <chopps@labn.net>
---
 include/uapi/linux/xfrm.h |  9 ++++++-
 net/xfrm/xfrm_compat.c    | 10 ++++++--
 net/xfrm/xfrm_user.c      | 52 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 3 deletions(-)

Comments

Sabrina Dubroca July 18, 2024, 1:45 p.m. UTC | #1 
2024-07-14, 16:22:33 -0400, Christian Hopps wrote:
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index a552cfa623ea..d42805314a2a 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -297,6 +297,16 @@ static int verify_newsa_info(struct xfrm_usersa_info *p,
>  			NL_SET_ERR_MSG(extack, "TFC padding can only be used in tunnel mode");
>  			goto out;
>  		}
> +		if ((attrs[XFRMA_IPTFS_DROP_TIME] ||
> +		     attrs[XFRMA_IPTFS_REORDER_WINDOW] ||
> +		     attrs[XFRMA_IPTFS_DONT_FRAG] ||
> +		     attrs[XFRMA_IPTFS_INIT_DELAY] ||
> +		     attrs[XFRMA_IPTFS_MAX_QSIZE] ||
> +		     attrs[XFRMA_IPTFS_PKT_SIZE]) &&
> +		    p->mode != XFRM_MODE_IPTFS) {
> +			NL_SET_ERR_MSG(extack, "IP-TFS options can only be used in IP-TFS mode");

AFAICT this only excludes the IPTFS options from ESP with a non-IPTFS
mode, but not from AH, IPcomp, etc.

> +			goto out;
> +		}
>  		break;
>  
>  	case IPPROTO_COMP:
> @@ -417,6 +427,18 @@ static int verify_newsa_info(struct xfrm_usersa_info *p,
>  			goto out;
>  		}
>  
> +		if (attrs[XFRMA_IPTFS_DROP_TIME]) {
> +			NL_SET_ERR_MSG(extack, "Drop time should not be set for output SA");

Maybe add "IPTFS" to all those error messages, to help narrow down the
bogus attribute.

> +			err = -EINVAL;
> +			goto out;
> +		}
> +
> +		if (attrs[XFRMA_IPTFS_REORDER_WINDOW]) {
> +			NL_SET_ERR_MSG(extack, "Reorder window should not be set for output SA");
> +			err = -EINVAL;
> +			goto out;
> +		}
> +
>  		if (attrs[XFRMA_REPLAY_VAL]) {
>  			struct xfrm_replay_state *replay;
>  
> @@ -454,6 +476,30 @@ static int verify_newsa_info(struct xfrm_usersa_info *p,
>  			}
>  
>  		}
> +
> +		if (attrs[XFRMA_IPTFS_DONT_FRAG]) {
> +			NL_SET_ERR_MSG(extack, "Don't fragment should not be set for input SA");
> +			err = -EINVAL;
> +			goto out;
> +		}
> +
> +		if (attrs[XFRMA_IPTFS_INIT_DELAY]) {
> +			NL_SET_ERR_MSG(extack, "Initial delay should not be set for input SA");
> +			err = -EINVAL;
> +			goto out;
> +		}
> +
> +		if (attrs[XFRMA_IPTFS_MAX_QSIZE]) {
> +			NL_SET_ERR_MSG(extack, "Max queue size should not be set for input SA");
> +			err = -EINVAL;
> +			goto out;
> +		}
> +
> +		if (attrs[XFRMA_IPTFS_PKT_SIZE]) {
> +			NL_SET_ERR_MSG(extack, "Packet size should not be set for input SA");
> +			err = -EINVAL;
> +			goto out;
> +		}
>  	}
>  
>  out:
> @@ -3177,6 +3223,12 @@ const struct nla_policy xfrma_policy[XFRMA_MAX+1] = {
>  	[XFRMA_MTIMER_THRESH]   = { .type = NLA_U32 },
>  	[XFRMA_SA_DIR]          = NLA_POLICY_RANGE(NLA_U8, XFRM_SA_DIR_IN, XFRM_SA_DIR_OUT),
>  	[XFRMA_NAT_KEEPALIVE_INTERVAL] = { .type = NLA_U32 },
> +	[XFRMA_IPTFS_DROP_TIME]		= { .type = NLA_U32 },
> +	[XFRMA_IPTFS_REORDER_WINDOW]	= { .type = NLA_U16 },

The corresponding sysctl is a u32, should this be NLA_U32?

> +	[XFRMA_IPTFS_DONT_FRAG]		= { .type = NLA_FLAG },
> +	[XFRMA_IPTFS_INIT_DELAY]	= { .type = NLA_U32 },
> +	[XFRMA_IPTFS_MAX_QSIZE]		= { .type = NLA_U32 },
> +	[XFRMA_IPTFS_PKT_SIZE]	= { .type = NLA_U32 },
>  };
>  EXPORT_SYMBOL_GPL(xfrma_policy);
Christian Hopps July 18, 2024, 2:08 p.m. UTC | #2 
Sabrina Dubroca via Devel <devel@linux-ipsec.org> writes:

> 2024-07-14, 16:22:33 -0400, Christian Hopps wrote:
>> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
>> index a552cfa623ea..d42805314a2a 100644
>> --- a/net/xfrm/xfrm_user.c
>> +++ b/net/xfrm/xfrm_user.c
>> @@ -297,6 +297,16 @@ static int verify_newsa_info(struct xfrm_usersa_info *p,
>>  			NL_SET_ERR_MSG(extack, "TFC padding can only be used in tunnel mode");
>>  			goto out;
>>  		}
>> +		if ((attrs[XFRMA_IPTFS_DROP_TIME] ||
>> +		     attrs[XFRMA_IPTFS_REORDER_WINDOW] ||
>> +		     attrs[XFRMA_IPTFS_DONT_FRAG] ||
>> +		     attrs[XFRMA_IPTFS_INIT_DELAY] ||
>> +		     attrs[XFRMA_IPTFS_MAX_QSIZE] ||
>> +		     attrs[XFRMA_IPTFS_PKT_SIZE]) &&
>> +		    p->mode != XFRM_MODE_IPTFS) {
>> +			NL_SET_ERR_MSG(extack, "IP-TFS options can only be used in IP-TFS mode");
>
> AFAICT this only excludes the IPTFS options from ESP with a non-IPTFS
> mode, but not from AH, IPcomp, etc.

Ok, the change I'll make here is to only allow IPTFS mode selection for proto == IPPROTO_ESP (as that reflects reality). This handles the above issue as well as adds an additional useful check.

>
>> +			goto out;
>> +		}
>>  		break;
>>
>>  	case IPPROTO_COMP:
>> @@ -417,6 +427,18 @@ static int verify_newsa_info(struct xfrm_usersa_info *p,
>>  			goto out;
>>  		}
>>
>> +		if (attrs[XFRMA_IPTFS_DROP_TIME]) {
>> +			NL_SET_ERR_MSG(extack, "Drop time should not be set for output SA");
>
> Maybe add "IPTFS" to all those error messages, to help narrow down the
> bogus attribute.

Done.

>
>> +			err = -EINVAL;
>> +			goto out;
>> +		}
>> +
>> +		if (attrs[XFRMA_IPTFS_REORDER_WINDOW]) {
>> +			NL_SET_ERR_MSG(extack, "Reorder window should not be set for output SA");
>> +			err = -EINVAL;
>> +			goto out;
>> +		}
>> +
>>  		if (attrs[XFRMA_REPLAY_VAL]) {
>>  			struct xfrm_replay_state *replay;
>>
>> @@ -454,6 +476,30 @@ static int verify_newsa_info(struct xfrm_usersa_info *p,
>>  			}
>>
>>  		}
>> +
>> +		if (attrs[XFRMA_IPTFS_DONT_FRAG]) {
>> +			NL_SET_ERR_MSG(extack, "Don't fragment should not be set for input SA");
>> +			err = -EINVAL;
>> +			goto out;
>> +		}
>> +
>> +		if (attrs[XFRMA_IPTFS_INIT_DELAY]) {
>> +			NL_SET_ERR_MSG(extack, "Initial delay should not be set for input SA");
>> +			err = -EINVAL;
>> +			goto out;
>> +		}
>> +
>> +		if (attrs[XFRMA_IPTFS_MAX_QSIZE]) {
>> +			NL_SET_ERR_MSG(extack, "Max queue size should not be set for input SA");
>> +			err = -EINVAL;
>> +			goto out;
>> +		}
>> +
>> +		if (attrs[XFRMA_IPTFS_PKT_SIZE]) {
>> +			NL_SET_ERR_MSG(extack, "Packet size should not be set for input SA");
>> +			err = -EINVAL;
>> +			goto out;
>> +		}
>>  	}
>>
>>  out:
>> @@ -3177,6 +3223,12 @@ const struct nla_policy xfrma_policy[XFRMA_MAX+1] = {
>>  	[XFRMA_MTIMER_THRESH]   = { .type = NLA_U32 },
>>  	[XFRMA_SA_DIR]          = NLA_POLICY_RANGE(NLA_U8, XFRM_SA_DIR_IN, XFRM_SA_DIR_OUT),
>>  	[XFRMA_NAT_KEEPALIVE_INTERVAL] = { .type = NLA_U32 },
>> +	[XFRMA_IPTFS_DROP_TIME]		= { .type = NLA_U32 },
>> +	[XFRMA_IPTFS_REORDER_WINDOW]	= { .type = NLA_U16 },
>
> The corresponding sysctl is a u32, should this be NLA_U32?

While it's not unbelievable that one might want to handle more than 255 out-of-order packets, it is unbelievable that one would try and handle more than 65535. :)

So the change I'll make is to switch the sysctl handler to proc_douintvec_minmax with a max value to 65535 -- the sysctl functions work with "uint" and "ulong" there doesn't appear to be any support for "ushort"/"u16"s.

Thanks,
Chris.

>
>> +	[XFRMA_IPTFS_DONT_FRAG]		= { .type = NLA_FLAG },
>> +	[XFRMA_IPTFS_INIT_DELAY]	= { .type = NLA_U32 },
>> +	[XFRMA_IPTFS_MAX_QSIZE]		= { .type = NLA_U32 },
>> +	[XFRMA_IPTFS_PKT_SIZE]	= { .type = NLA_U32 },
>>  };
>>  EXPORT_SYMBOL_GPL(xfrma_policy);
>
> --
> Sabrina
diff mbox series

Patch

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index f28701500714..042ebf94bb3d 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -158,7 +158,8 @@  enum {
 #define XFRM_MODE_ROUTEOPTIMIZATION 2
 #define XFRM_MODE_IN_TRIGGER 3
 #define XFRM_MODE_BEET 4
-#define XFRM_MODE_MAX 5
+#define XFRM_MODE_IPTFS 5
+#define XFRM_MODE_MAX 6
 
 /* Netlink configuration messages.  */
 enum {
@@ -322,6 +323,12 @@  enum xfrm_attr_type_t {
 	XFRMA_MTIMER_THRESH,	/* __u32 in seconds for input SA */
 	XFRMA_SA_DIR,		/* __u8 */
 	XFRMA_NAT_KEEPALIVE_INTERVAL,	/* __u32 in seconds for NAT keepalive */
+	XFRMA_IPTFS_DROP_TIME,	/* __u32 in: usec to wait for next seq */
+	XFRMA_IPTFS_REORDER_WINDOW, /* __u16 in: reorder window size (pkts) */
+	XFRMA_IPTFS_DONT_FRAG,	/* out: don't use fragmentation */
+	XFRMA_IPTFS_INIT_DELAY,	/* __u32 out: initial packet wait delay (usec) */
+	XFRMA_IPTFS_MAX_QSIZE,	/* __u32 out: max ingress queue size (octets) */
+	XFRMA_IPTFS_PKT_SIZE,	/* __u32 out: size of outer packet, 0 for PMTU */
 	__XFRMA_MAX
 
 #define XFRMA_OUTPUT_MARK XFRMA_SET_MARK	/* Compatibility */
diff --git a/net/xfrm/xfrm_compat.c b/net/xfrm/xfrm_compat.c
index 91357ccaf4af..5c55e07f3d10 100644
--- a/net/xfrm/xfrm_compat.c
+++ b/net/xfrm/xfrm_compat.c
@@ -282,9 +282,15 @@  static int xfrm_xlate64_attr(struct sk_buff *dst, const struct nlattr *src)
 	case XFRMA_MTIMER_THRESH:
 	case XFRMA_SA_DIR:
 	case XFRMA_NAT_KEEPALIVE_INTERVAL:
+	case XFRMA_IPTFS_DROP_TIME:
+	case XFRMA_IPTFS_REORDER_WINDOW:
+	case XFRMA_IPTFS_DONT_FRAG:
+	case XFRMA_IPTFS_INIT_DELAY:
+	case XFRMA_IPTFS_MAX_QSIZE:
+	case XFRMA_IPTFS_PKT_SIZE:
 		return xfrm_nla_cpy(dst, src, nla_len(src));
 	default:
-		BUILD_BUG_ON(XFRMA_MAX != XFRMA_NAT_KEEPALIVE_INTERVAL);
+		BUILD_BUG_ON(XFRMA_MAX != XFRMA_IPTFS_PKT_SIZE);
 		pr_warn_once("unsupported nla_type %d\n", src->nla_type);
 		return -EOPNOTSUPP;
 	}
@@ -439,7 +445,7 @@  static int xfrm_xlate32_attr(void *dst, const struct nlattr *nla,
 	int err;
 
 	if (type > XFRMA_MAX) {
-		BUILD_BUG_ON(XFRMA_MAX != XFRMA_NAT_KEEPALIVE_INTERVAL);
+		BUILD_BUG_ON(XFRMA_MAX != XFRMA_IPTFS_PKT_SIZE);
 		NL_SET_ERR_MSG(extack, "Bad attribute");
 		return -EOPNOTSUPP;
 	}
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index a552cfa623ea..d42805314a2a 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -297,6 +297,16 @@  static int verify_newsa_info(struct xfrm_usersa_info *p,
 			NL_SET_ERR_MSG(extack, "TFC padding can only be used in tunnel mode");
 			goto out;
 		}
+		if ((attrs[XFRMA_IPTFS_DROP_TIME] ||
+		     attrs[XFRMA_IPTFS_REORDER_WINDOW] ||
+		     attrs[XFRMA_IPTFS_DONT_FRAG] ||
+		     attrs[XFRMA_IPTFS_INIT_DELAY] ||
+		     attrs[XFRMA_IPTFS_MAX_QSIZE] ||
+		     attrs[XFRMA_IPTFS_PKT_SIZE]) &&
+		    p->mode != XFRM_MODE_IPTFS) {
+			NL_SET_ERR_MSG(extack, "IP-TFS options can only be used in IP-TFS mode");
+			goto out;
+		}
 		break;
 
 	case IPPROTO_COMP:
@@ -417,6 +427,18 @@  static int verify_newsa_info(struct xfrm_usersa_info *p,
 			goto out;
 		}
 
+		if (attrs[XFRMA_IPTFS_DROP_TIME]) {
+			NL_SET_ERR_MSG(extack, "Drop time should not be set for output SA");
+			err = -EINVAL;
+			goto out;
+		}
+
+		if (attrs[XFRMA_IPTFS_REORDER_WINDOW]) {
+			NL_SET_ERR_MSG(extack, "Reorder window should not be set for output SA");
+			err = -EINVAL;
+			goto out;
+		}
+
 		if (attrs[XFRMA_REPLAY_VAL]) {
 			struct xfrm_replay_state *replay;
 
@@ -454,6 +476,30 @@  static int verify_newsa_info(struct xfrm_usersa_info *p,
 			}
 
 		}
+
+		if (attrs[XFRMA_IPTFS_DONT_FRAG]) {
+			NL_SET_ERR_MSG(extack, "Don't fragment should not be set for input SA");
+			err = -EINVAL;
+			goto out;
+		}
+
+		if (attrs[XFRMA_IPTFS_INIT_DELAY]) {
+			NL_SET_ERR_MSG(extack, "Initial delay should not be set for input SA");
+			err = -EINVAL;
+			goto out;
+		}
+
+		if (attrs[XFRMA_IPTFS_MAX_QSIZE]) {
+			NL_SET_ERR_MSG(extack, "Max queue size should not be set for input SA");
+			err = -EINVAL;
+			goto out;
+		}
+
+		if (attrs[XFRMA_IPTFS_PKT_SIZE]) {
+			NL_SET_ERR_MSG(extack, "Packet size should not be set for input SA");
+			err = -EINVAL;
+			goto out;
+		}
 	}
 
 out:
@@ -3177,6 +3223,12 @@  const struct nla_policy xfrma_policy[XFRMA_MAX+1] = {
 	[XFRMA_MTIMER_THRESH]   = { .type = NLA_U32 },
 	[XFRMA_SA_DIR]          = NLA_POLICY_RANGE(NLA_U8, XFRM_SA_DIR_IN, XFRM_SA_DIR_OUT),
 	[XFRMA_NAT_KEEPALIVE_INTERVAL] = { .type = NLA_U32 },
+	[XFRMA_IPTFS_DROP_TIME]		= { .type = NLA_U32 },
+	[XFRMA_IPTFS_REORDER_WINDOW]	= { .type = NLA_U16 },
+	[XFRMA_IPTFS_DONT_FRAG]		= { .type = NLA_FLAG },
+	[XFRMA_IPTFS_INIT_DELAY]	= { .type = NLA_U32 },
+	[XFRMA_IPTFS_MAX_QSIZE]		= { .type = NLA_U32 },
+	[XFRMA_IPTFS_PKT_SIZE]	= { .type = NLA_U32 },
 };
 EXPORT_SYMBOL_GPL(xfrma_policy);