[PATCH ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[PATCH ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[Antony Antony]

From: Steffen Klassert <steffen.klassert@secunet.com>

As the default we assume the traffic to pass, if we have no
matching IPsec policy. With this patch, we have a possibility to
change this default from allow to block. It can be configured
via netlink. Each direction (input/output/forward) can be
configured separately. With the default to block configuered,
we need allow policies for all packet flows we accept.
We do not use default policy lookup for the loopback device.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Co-developed-by: Christian Langrock <christian.langrock@secunet.com>
Signed-off-by: Christian Langrock <christian.langrock@secunet.com>
Co-developed-by: Antony Antony <antony.antony@secunet.com>
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 include/net/netns/xfrm.h  |  7 ++++++
 include/net/xfrm.h        | 36 ++++++++++++++++++++++-----
 include/uapi/linux/xfrm.h | 10 ++++++++
 net/xfrm/xfrm_policy.c    | 16 ++++++++++++
 net/xfrm/xfrm_user.c      | 52 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 115 insertions(+), 6 deletions(-)

diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index e946366e8ba5..88c647302977 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -65,6 +65,13 @@ struct netns_xfrm {
 	u32			sysctl_aevent_rseqth;
 	int			sysctl_larval_drop;
 	u32			sysctl_acq_expires;
+
+	u8			policy_default;
+#define XFRM_POL_DEFAULT_IN	1
+#define XFRM_POL_DEFAULT_OUT	2
+#define XFRM_POL_DEFAULT_FWD	4
+#define XFRM_POL_DEFAULT_MASK	7
+
 #ifdef CONFIG_SYSCTL
 	struct ctl_table_header	*sysctl_hdr;
 #endif
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index cbff7c2a9724..d9b470d47af1 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1074,6 +1074,22 @@ xfrm_state_addr_cmp(const struct xfrm_tmpl *tmpl, const struct xfrm_state *x, un
 	return !0;
 }
 
+static inline bool
+xfrm_default_allow(struct net *net, int dir)
+{
+	u8 def = net->xfrm.policy_default;
+
+	switch (dir) {
+	case XFRM_POLICY_IN:
+		return def & XFRM_POL_DEFAULT_IN ? false : true;
+	case XFRM_POLICY_OUT:
+		return def & XFRM_POL_DEFAULT_OUT ? false : true;
+	case XFRM_POLICY_FWD:
+		return def & XFRM_POL_DEFAULT_FWD ? false : true;
+	}
+	return false;
+}
+
 #ifdef CONFIG_XFRM
 int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb,
 			unsigned short family);
@@ -1088,9 +1104,13 @@ static inline int __xfrm_policy_check2(struct sock *sk, int dir,
 	if (sk && sk->sk_policy[XFRM_POLICY_IN])
 		return __xfrm_policy_check(sk, ndir, skb, family);
 
-	return	(!net->xfrm.policy_count[dir] && !secpath_exists(skb)) ||
-		(skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) ||
-		__xfrm_policy_check(sk, ndir, skb, family);
+	if (xfrm_default_allow(net, dir))
+		return (!net->xfrm.policy_count[dir] && !secpath_exists(skb)) ||
+		       (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) ||
+		       __xfrm_policy_check(sk, ndir, skb, family);
+	else
+		return (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) ||
+		       __xfrm_policy_check(sk, ndir, skb, family);
 }
 
 static inline int xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, unsigned short family)
@@ -1142,9 +1162,13 @@ static inline int xfrm_route_forward(struct sk_buff *skb, unsigned short family)
 {
 	struct net *net = dev_net(skb->dev);
 
-	return	!net->xfrm.policy_count[XFRM_POLICY_OUT] ||
-		(skb_dst(skb)->flags & DST_NOXFRM) ||
-		__xfrm_route_forward(skb, family);
+	if (xfrm_default_allow(net, XFRM_POLICY_FWD))
+		return !net->xfrm.policy_count[XFRM_POLICY_OUT] ||
+			(skb_dst(skb)->flags & DST_NOXFRM) ||
+			__xfrm_route_forward(skb, family);
+	else
+		return (skb_dst(skb)->flags & DST_NOXFRM) ||
+			__xfrm_route_forward(skb, family);
 }
 
 static inline int xfrm4_route_forward(struct sk_buff *skb)
diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index ffc6a5391bb7..6e8095106192 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -213,6 +213,11 @@ enum {
 	XFRM_MSG_GETSPDINFO,
 #define XFRM_MSG_GETSPDINFO XFRM_MSG_GETSPDINFO
 
+	XFRM_MSG_SETDEFAULT,
+#define XFRM_MSG_SETDEFAULT XFRM_MSG_SETDEFAULT
+	XFRM_MSG_GETDEFAULT,
+#define XFRM_MSG_GETDEFAULT XFRM_MSG_GETDEFAULT
+
 	XFRM_MSG_MAPPING,
 #define XFRM_MSG_MAPPING XFRM_MSG_MAPPING
 	__XFRM_MSG_MAX
@@ -508,6 +513,11 @@ struct xfrm_user_offload {
 #define XFRM_OFFLOAD_IPV6	1
 #define XFRM_OFFLOAD_INBOUND	2
 
+struct xfrm_userpolicy_default {
+	__u8				dirmask;
+	__u8				action;
+};
+
 #ifndef __KERNEL__
 /* backwards compatibility for userspace */
 #define XFRMGRP_ACQUIRE		1
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 827d84255021..d5cb082e11fc 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -3165,6 +3165,11 @@ struct dst_entry *xfrm_lookup_with_ifid(struct net *net,
 	return dst;
 
 nopol:
+	if (!(dst_orig->dev->flags & IFF_LOOPBACK) &&
+	    !xfrm_default_allow(net, dir)) {
+		err = -EPERM;
+		goto error;
+	}
 	if (!(flags & XFRM_LOOKUP_ICMP)) {
 		dst = dst_orig;
 		goto ok;
@@ -3553,6 +3558,11 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
 	}
 
 	if (!pol) {
+		if (!xfrm_default_allow(net, dir)) {
+			XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOPOLS);
+			return 0;
+		}
+
 		if (sp && secpath_has_nontransport(sp, 0, &xerr_idx)) {
 			xfrm_secpath_reject(xerr_idx, skb, &fl);
 			XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOPOLS);
@@ -3607,6 +3617,12 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
 				tpp[ti++] = &pols[pi]->xfrm_vec[i];
 		}
 		xfrm_nr = ti;
+
+		if (!xfrm_default_allow(net, dir) && !xfrm_nr) {
+			XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOSTATES);
+			goto reject;
+		}
+
 		if (npols > 1) {
 			xfrm_tmpl_sort(stp, tpp, xfrm_nr, family);
 			tpp = stp;
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index b47d613409b7..4eafd1130c3e 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1961,6 +1961,54 @@ static struct sk_buff *xfrm_policy_netlink(struct sk_buff *in_skb,
 	return skb;
 }
 
+static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
+			    struct nlattr **attrs)
+{
+	struct net *net = sock_net(skb->sk);
+	struct xfrm_userpolicy_default *up = nlmsg_data(nlh);
+	u8 dirmask = (1 << up->dirmask) & XFRM_POL_DEFAULT_MASK;
+	u8 old_default = net->xfrm.policy_default;
+
+	net->xfrm.policy_default = (old_default & (0xff ^ dirmask))
+				    | (up->action << up->dirmask);
+
+	rt_genid_bump_all(net);
+
+	return 0;
+}
+
+static int xfrm_get_default(struct sk_buff *skb, struct nlmsghdr *nlh,
+			    struct nlattr **attrs)
+{
+	struct sk_buff *r_skb;
+	struct nlmsghdr *r_nlh;
+	struct net *net = sock_net(skb->sk);
+	struct xfrm_userpolicy_default *r_up, *up;
+	int len = NLMSG_ALIGN(sizeof(struct xfrm_userpolicy_default));
+	u32 portid = NETLINK_CB(skb).portid;
+	u32 seq = nlh->nlmsg_seq;
+
+	up = nlmsg_data(nlh);
+
+	r_skb = nlmsg_new(len, GFP_ATOMIC);
+	if (!r_skb)
+		return -ENOMEM;
+
+	r_nlh = nlmsg_put(r_skb, portid, seq, XFRM_MSG_GETDEFAULT, sizeof(*r_up), 0);
+	if (!r_nlh) {
+		kfree_skb(r_skb);
+		return -EMSGSIZE;
+	}
+
+	r_up = nlmsg_data(r_nlh);
+
+	r_up->action = ((net->xfrm.policy_default & (1 << up->dirmask)) >> up->dirmask);
+	r_up->dirmask = up->dirmask;
+	nlmsg_end(r_skb, r_nlh);
+
+	return nlmsg_unicast(net->xfrm.nlsk, r_skb, portid);
+}
+
 static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 		struct nlattr **attrs)
 {
@@ -2664,6 +2712,8 @@ const int xfrm_msg_min[XFRM_NR_MSGTYPES] = {
 	[XFRM_MSG_GETSADINFO  - XFRM_MSG_BASE] = sizeof(u32),
 	[XFRM_MSG_NEWSPDINFO  - XFRM_MSG_BASE] = sizeof(u32),
 	[XFRM_MSG_GETSPDINFO  - XFRM_MSG_BASE] = sizeof(u32),
+	[XFRM_MSG_SETDEFAULT  - XFRM_MSG_BASE] = XMSGSIZE(xfrm_userpolicy_default),
+	[XFRM_MSG_GETDEFAULT  - XFRM_MSG_BASE] = XMSGSIZE(xfrm_userpolicy_default),
 };
 EXPORT_SYMBOL_GPL(xfrm_msg_min);
 
@@ -2743,6 +2793,8 @@ static const struct xfrm_link {
 						   .nla_pol = xfrma_spd_policy,
 						   .nla_max = XFRMA_SPD_MAX },
 	[XFRM_MSG_GETSPDINFO  - XFRM_MSG_BASE] = { .doit = xfrm_get_spdinfo   },
+	[XFRM_MSG_SETDEFAULT  - XFRM_MSG_BASE] = { .doit = xfrm_set_default   },
+	[XFRM_MSG_GETDEFAULT  - XFRM_MSG_BASE] = { .doit = xfrm_get_default   },
 };
 
 static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
-- 
2.20.1

Re: [PATCH ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2208 bytes --]

Hi Antony,

I love your patch! Yet something to improve:

[auto build test ERROR on ipsec-next/master]

url:    https://github.com/0day-ci/linux/commits/Antony-Antony/xfrm-Add-possibility-to-set-the-default-to-block-if-we-have-no-policy/20210718-093119
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
config: s390-randconfig-m031-20210718 (attached as .config)
compiler: s390-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/5aac4e97776280ef1fbe4c3d16c28833018d0985
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Antony-Antony/xfrm-Add-possibility-to-set-the-default-to-block-if-we-have-no-policy/20210718-093119
        git checkout 5aac4e97776280ef1fbe4c3d16c28833018d0985
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=s390 SHELL=/bin/bash net/core/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from net/core/sock.c:127:
   include/net/xfrm.h: In function 'xfrm_default_allow':
>> include/net/xfrm.h:1080:14: error: 'struct net' has no member named 'xfrm'
    1080 |  u8 def = net->xfrm.policy_default;
         |              ^~


vim +1080 include/net/xfrm.h

  1076	
  1077	static inline bool
  1078	xfrm_default_allow(struct net *net, int dir)
  1079	{
> 1080		u8 def = net->xfrm.policy_default;
  1081	
  1082		switch (dir) {
  1083		case XFRM_POLICY_IN:
  1084			return def & XFRM_POL_DEFAULT_IN ? false : true;
  1085		case XFRM_POLICY_OUT:
  1086			return def & XFRM_POL_DEFAULT_OUT ? false : true;
  1087		case XFRM_POLICY_FWD:
  1088			return def & XFRM_POL_DEFAULT_FWD ? false : true;
  1089		}
  1090		return false;
  1091	}
  1092	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 17048 bytes --]

[PATCH v2 ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[Antony Antony]

From: Steffen Klassert <steffen.klassert@secunet.com>

As the default we assume the traffic to pass, if we have no
matching IPsec policy. With this patch, we have a possibility to
change this default from allow to block. It can be configured
via netlink. Each direction (input/output/forward) can be
configured separately. With the default to block configuered,
we need allow policies for all packet flows we accept.
We do not use default policy lookup for the loopback device.

v1->v2
 - fix compiling when XFRM is disabled
 - Reported-by: kernel test robot <lkp@intel.com>

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Co-developed-by: Christian Langrock <christian.langrock@secunet.com>
Signed-off-by: Christian Langrock <christian.langrock@secunet.com>
Co-developed-by: Antony Antony <antony.antony@secunet.com>
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 include/net/netns/xfrm.h  |  7 ++++++
 include/net/xfrm.h        | 36 ++++++++++++++++++++++-----
 include/uapi/linux/xfrm.h | 10 ++++++++
 net/xfrm/xfrm_policy.c    | 16 ++++++++++++
 net/xfrm/xfrm_user.c      | 52 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 115 insertions(+), 6 deletions(-)

diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index e946366e8ba5..88c647302977 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -65,6 +65,13 @@ struct netns_xfrm {
 	u32			sysctl_aevent_rseqth;
 	int			sysctl_larval_drop;
 	u32			sysctl_acq_expires;
+
+	u8			policy_default;
+#define XFRM_POL_DEFAULT_IN	1
+#define XFRM_POL_DEFAULT_OUT	2
+#define XFRM_POL_DEFAULT_FWD	4
+#define XFRM_POL_DEFAULT_MASK	7
+
 #ifdef CONFIG_SYSCTL
 	struct ctl_table_header	*sysctl_hdr;
 #endif
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index cbff7c2a9724..2308210793a0 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1075,6 +1075,22 @@ xfrm_state_addr_cmp(const struct xfrm_tmpl *tmpl, const struct xfrm_state *x, un
 }

 #ifdef CONFIG_XFRM
+static inline bool
+xfrm_default_allow(struct net *net, int dir)
+{
+	u8 def = net->xfrm.policy_default;
+
+	switch (dir) {
+	case XFRM_POLICY_IN:
+		return def & XFRM_POL_DEFAULT_IN ? false : true;
+	case XFRM_POLICY_OUT:
+		return def & XFRM_POL_DEFAULT_OUT ? false : true;
+	case XFRM_POLICY_FWD:
+		return def & XFRM_POL_DEFAULT_FWD ? false : true;
+	}
+	return false;
+}
+
 int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb,
 			unsigned short family);

@@ -1088,9 +1104,13 @@ static inline int __xfrm_policy_check2(struct sock *sk, int dir,
 	if (sk && sk->sk_policy[XFRM_POLICY_IN])
 		return __xfrm_policy_check(sk, ndir, skb, family);

-	return	(!net->xfrm.policy_count[dir] && !secpath_exists(skb)) ||
-		(skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) ||
-		__xfrm_policy_check(sk, ndir, skb, family);
+	if (xfrm_default_allow(net, dir))
+		return (!net->xfrm.policy_count[dir] && !secpath_exists(skb)) ||
+		       (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) ||
+		       __xfrm_policy_check(sk, ndir, skb, family);
+	else
+		return (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) ||
+		       __xfrm_policy_check(sk, ndir, skb, family);
 }

 static inline int xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, unsigned short family)
@@ -1142,9 +1162,13 @@ static inline int xfrm_route_forward(struct sk_buff *skb, unsigned short family)
 {
 	struct net *net = dev_net(skb->dev);

-	return	!net->xfrm.policy_count[XFRM_POLICY_OUT] ||
-		(skb_dst(skb)->flags & DST_NOXFRM) ||
-		__xfrm_route_forward(skb, family);
+	if (xfrm_default_allow(net, XFRM_POLICY_FWD))
+		return !net->xfrm.policy_count[XFRM_POLICY_OUT] ||
+			(skb_dst(skb)->flags & DST_NOXFRM) ||
+			__xfrm_route_forward(skb, family);
+	else
+		return (skb_dst(skb)->flags & DST_NOXFRM) ||
+			__xfrm_route_forward(skb, family);
 }

 static inline int xfrm4_route_forward(struct sk_buff *skb)
diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index ffc6a5391bb7..6e8095106192 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -213,6 +213,11 @@ enum {
 	XFRM_MSG_GETSPDINFO,
 #define XFRM_MSG_GETSPDINFO XFRM_MSG_GETSPDINFO

+	XFRM_MSG_SETDEFAULT,
+#define XFRM_MSG_SETDEFAULT XFRM_MSG_SETDEFAULT
+	XFRM_MSG_GETDEFAULT,
+#define XFRM_MSG_GETDEFAULT XFRM_MSG_GETDEFAULT
+
 	XFRM_MSG_MAPPING,
 #define XFRM_MSG_MAPPING XFRM_MSG_MAPPING
 	__XFRM_MSG_MAX
@@ -508,6 +513,11 @@ struct xfrm_user_offload {
 #define XFRM_OFFLOAD_IPV6	1
 #define XFRM_OFFLOAD_INBOUND	2

+struct xfrm_userpolicy_default {
+	__u8				dirmask;
+	__u8				action;
+};
+
 #ifndef __KERNEL__
 /* backwards compatibility for userspace */
 #define XFRMGRP_ACQUIRE		1
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 827d84255021..d5cb082e11fc 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -3165,6 +3165,11 @@ struct dst_entry *xfrm_lookup_with_ifid(struct net *net,
 	return dst;

 nopol:
+	if (!(dst_orig->dev->flags & IFF_LOOPBACK) &&
+	    !xfrm_default_allow(net, dir)) {
+		err = -EPERM;
+		goto error;
+	}
 	if (!(flags & XFRM_LOOKUP_ICMP)) {
 		dst = dst_orig;
 		goto ok;
@@ -3553,6 +3558,11 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
 	}

 	if (!pol) {
+		if (!xfrm_default_allow(net, dir)) {
+			XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOPOLS);
+			return 0;
+		}
+
 		if (sp && secpath_has_nontransport(sp, 0, &xerr_idx)) {
 			xfrm_secpath_reject(xerr_idx, skb, &fl);
 			XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOPOLS);
@@ -3607,6 +3617,12 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
 				tpp[ti++] = &pols[pi]->xfrm_vec[i];
 		}
 		xfrm_nr = ti;
+
+		if (!xfrm_default_allow(net, dir) && !xfrm_nr) {
+			XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOSTATES);
+			goto reject;
+		}
+
 		if (npols > 1) {
 			xfrm_tmpl_sort(stp, tpp, xfrm_nr, family);
 			tpp = stp;
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index b47d613409b7..4eafd1130c3e 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1961,6 +1961,54 @@ static struct sk_buff *xfrm_policy_netlink(struct sk_buff *in_skb,
 	return skb;
 }

+static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
+			    struct nlattr **attrs)
+{
+	struct net *net = sock_net(skb->sk);
+	struct xfrm_userpolicy_default *up = nlmsg_data(nlh);
+	u8 dirmask = (1 << up->dirmask) & XFRM_POL_DEFAULT_MASK;
+	u8 old_default = net->xfrm.policy_default;
+
+	net->xfrm.policy_default = (old_default & (0xff ^ dirmask))
+				    | (up->action << up->dirmask);
+
+	rt_genid_bump_all(net);
+
+	return 0;
+}
+
+static int xfrm_get_default(struct sk_buff *skb, struct nlmsghdr *nlh,
+			    struct nlattr **attrs)
+{
+	struct sk_buff *r_skb;
+	struct nlmsghdr *r_nlh;
+	struct net *net = sock_net(skb->sk);
+	struct xfrm_userpolicy_default *r_up, *up;
+	int len = NLMSG_ALIGN(sizeof(struct xfrm_userpolicy_default));
+	u32 portid = NETLINK_CB(skb).portid;
+	u32 seq = nlh->nlmsg_seq;
+
+	up = nlmsg_data(nlh);
+
+	r_skb = nlmsg_new(len, GFP_ATOMIC);
+	if (!r_skb)
+		return -ENOMEM;
+
+	r_nlh = nlmsg_put(r_skb, portid, seq, XFRM_MSG_GETDEFAULT, sizeof(*r_up), 0);
+	if (!r_nlh) {
+		kfree_skb(r_skb);
+		return -EMSGSIZE;
+	}
+
+	r_up = nlmsg_data(r_nlh);
+
+	r_up->action = ((net->xfrm.policy_default & (1 << up->dirmask)) >> up->dirmask);
+	r_up->dirmask = up->dirmask;
+	nlmsg_end(r_skb, r_nlh);
+
+	return nlmsg_unicast(net->xfrm.nlsk, r_skb, portid);
+}
+
 static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 		struct nlattr **attrs)
 {
@@ -2664,6 +2712,8 @@ const int xfrm_msg_min[XFRM_NR_MSGTYPES] = {
 	[XFRM_MSG_GETSADINFO  - XFRM_MSG_BASE] = sizeof(u32),
 	[XFRM_MSG_NEWSPDINFO  - XFRM_MSG_BASE] = sizeof(u32),
 	[XFRM_MSG_GETSPDINFO  - XFRM_MSG_BASE] = sizeof(u32),
+	[XFRM_MSG_SETDEFAULT  - XFRM_MSG_BASE] = XMSGSIZE(xfrm_userpolicy_default),
+	[XFRM_MSG_GETDEFAULT  - XFRM_MSG_BASE] = XMSGSIZE(xfrm_userpolicy_default),
 };
 EXPORT_SYMBOL_GPL(xfrm_msg_min);

@@ -2743,6 +2793,8 @@ static const struct xfrm_link {
 						   .nla_pol = xfrma_spd_policy,
 						   .nla_max = XFRMA_SPD_MAX },
 	[XFRM_MSG_GETSPDINFO  - XFRM_MSG_BASE] = { .doit = xfrm_get_spdinfo   },
+	[XFRM_MSG_SETDEFAULT  - XFRM_MSG_BASE] = { .doit = xfrm_set_default   },
+	[XFRM_MSG_GETDEFAULT  - XFRM_MSG_BASE] = { .doit = xfrm_get_default   },
 };

 static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
--
2.20.1

Re: [PATCH v2 ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[Steffen Klassert]

On Sun, Jul 18, 2021 at 09:11:06AM +0200, Antony Antony wrote:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> 
> As the default we assume the traffic to pass, if we have no
> matching IPsec policy. With this patch, we have a possibility to
> change this default from allow to block. It can be configured
> via netlink. Each direction (input/output/forward) can be
> configured separately. With the default to block configuered,
> we need allow policies for all packet flows we accept.
> We do not use default policy lookup for the loopback device.
> 
> v1->v2
>  - fix compiling when XFRM is disabled
>  - Reported-by: kernel test robot <lkp@intel.com>
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> Co-developed-by: Christian Langrock <christian.langrock@secunet.com>
> Signed-off-by: Christian Langrock <christian.langrock@secunet.com>
> Co-developed-by: Antony Antony <antony.antony@secunet.com>
> Signed-off-by: Antony Antony <antony.antony@secunet.com>

Applied, thanks for pushing this upstream Antony!

Re: [PATCH v2 ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[Nicolas Dichtel]

Le 18/07/2021 à 09:11, Antony Antony a écrit :
> From: Steffen Klassert <steffen.klassert@secunet.com>
Sorry for my late reply, I was off.

> 
> As the default we assume the traffic to pass, if we have no
> matching IPsec policy. With this patch, we have a possibility to
> change this default from allow to block. It can be configured
> via netlink. Each direction (input/output/forward) can be
> configured separately. With the default to block configuered,
> we need allow policies for all packet flows we accept.
> We do not use default policy lookup for the loopback device.
> 

[snip]

> diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
> index e946366e8ba5..88c647302977 100644
> --- a/include/net/netns/xfrm.h
> +++ b/include/net/netns/xfrm.h
> @@ -65,6 +65,13 @@ struct netns_xfrm {
>  	u32			sysctl_aevent_rseqth;
>  	int			sysctl_larval_drop;
>  	u32			sysctl_acq_expires;
> +
> +	u8			policy_default;
> +#define XFRM_POL_DEFAULT_IN	1
> +#define XFRM_POL_DEFAULT_OUT	2
> +#define XFRM_POL_DEFAULT_FWD	4
> +#define XFRM_POL_DEFAULT_MASK	7
> +

[snip]

> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
> index ffc6a5391bb7..6e8095106192 100644
> --- a/include/uapi/linux/xfrm.h
> +++ b/include/uapi/linux/xfrm.h
> @@ -213,6 +213,11 @@ enum {
>  	XFRM_MSG_GETSPDINFO,
>  #define XFRM_MSG_GETSPDINFO XFRM_MSG_GETSPDINFO
> 
> +	XFRM_MSG_SETDEFAULT,
> +#define XFRM_MSG_SETDEFAULT XFRM_MSG_SETDEFAULT
> +	XFRM_MSG_GETDEFAULT,
> +#define XFRM_MSG_GETDEFAULT XFRM_MSG_GETDEFAULT
> +
>  	XFRM_MSG_MAPPING,
>  #define XFRM_MSG_MAPPING XFRM_MSG_MAPPING
>  	__XFRM_MSG_MAX
> @@ -508,6 +513,11 @@ struct xfrm_user_offload {
>  #define XFRM_OFFLOAD_IPV6	1
>  #define XFRM_OFFLOAD_INBOUND	2
> 
> +struct xfrm_userpolicy_default {
> +	__u8				dirmask;
> +	__u8				action;
> +};
> +
Should XFRM_POL_DEFAULT_* be moved in the uapi?
How can a user knows what value is expected in dirmask?

Same question for action. We should avoid magic values. 0 means drop or accept?
Maybe renaming this field to 'drop' is enough.


Regards,
Nicolas

Re: [PATCH v2 ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[Antony Antony]

On Wed, Aug 11, 2021 at 18:14:08 +0200, Nicolas Dichtel wrote:
> Le 18/07/2021 à 09:11, Antony Antony a écrit :
> > From: Steffen Klassert <steffen.klassert@secunet.com>
> Sorry for my late reply, I was off.
> 
> > 
> > As the default we assume the traffic to pass, if we have no
> > matching IPsec policy. With this patch, we have a possibility to
> > change this default from allow to block. It can be configured
> > via netlink. Each direction (input/output/forward) can be
> > configured separately. With the default to block configuered,
> > we need allow policies for all packet flows we accept.
> > We do not use default policy lookup for the loopback device.
> > 
> 
> [snip]
> 
> > diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
> > index e946366e8ba5..88c647302977 100644
> > --- a/include/net/netns/xfrm.h
> > +++ b/include/net/netns/xfrm.h
> > @@ -65,6 +65,13 @@ struct netns_xfrm {
> >  	u32			sysctl_aevent_rseqth;
> >  	int			sysctl_larval_drop;
> >  	u32			sysctl_acq_expires;
> > +
> > +	u8			policy_default;
> > +#define XFRM_POL_DEFAULT_IN	1
> > +#define XFRM_POL_DEFAULT_OUT	2
> > +#define XFRM_POL_DEFAULT_FWD	4
> > +#define XFRM_POL_DEFAULT_MASK	7
> > +
> 
> [snip]
> 
> > diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
> > index ffc6a5391bb7..6e8095106192 100644
> > --- a/include/uapi/linux/xfrm.h
> > +++ b/include/uapi/linux/xfrm.h
> > @@ -213,6 +213,11 @@ enum {
> >  	XFRM_MSG_GETSPDINFO,
> >  #define XFRM_MSG_GETSPDINFO XFRM_MSG_GETSPDINFO
> > 
> > +	XFRM_MSG_SETDEFAULT,
> > +#define XFRM_MSG_SETDEFAULT XFRM_MSG_SETDEFAULT
> > +	XFRM_MSG_GETDEFAULT,
> > +#define XFRM_MSG_GETDEFAULT XFRM_MSG_GETDEFAULT
> > +
> >  	XFRM_MSG_MAPPING,
> >  #define XFRM_MSG_MAPPING XFRM_MSG_MAPPING
> >  	__XFRM_MSG_MAX
> > @@ -508,6 +513,11 @@ struct xfrm_user_offload {
> >  #define XFRM_OFFLOAD_IPV6	1
> >  #define XFRM_OFFLOAD_INBOUND	2
> > 
> > +struct xfrm_userpolicy_default {
> > +	__u8				dirmask;
> > +	__u8				action;
> > +};
> > +
> Should XFRM_POL_DEFAULT_* be moved in the uapi?

It is good point. Thanks for the feedback.

> How can a user knows what value is expected in dirmask?
> 
> Same question for action. We should avoid magic values. 0 means drop or accept?

I have an iproute2 patch I want to sent out, moving to uapi would avoid using
hardcoded magic values there.


> Maybe renaming this field to 'drop' is enough.

action is a bitwise flag, one direction it may drop and ther other might
be allow.

Re: [PATCH v2 ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[Nicolas Dichtel]

Le 17/08/2021 à 13:19, Antony Antony a écrit :
> On Wed, Aug 11, 2021 at 18:14:08 +0200, Nicolas Dichtel wrote:
[snip]
>> Maybe renaming this field to 'drop' is enough.
> 
> action is a bitwise flag, one direction it may drop and ther other might
> be allow.
> 
Sure, but I still think drop or accept would be better.

[PATCH ipsec 0/2] xfrm: fix uapi for the default policy

[Nicolas Dichtel]

This feature has just been merged after the last release, thus it's still
time to fix the uapi.
As stated in the thread, the uapi is based on some magic values (from the
userland POV).
Here is a proposal to simplify this uapi and make it clear how to use it.
The other problem was the notification: changing the default policy may
radically change the packets flows.

Nicolas Dichtel (2):
  xfrm: make user policy API complete
  xfrm: notify default policy on update

 include/uapi/linux/xfrm.h |  9 ++++--
 net/xfrm/xfrm_user.c      | 58 +++++++++++++++++++++++++++++++++------
 2 files changed, 55 insertions(+), 12 deletions(-)

Comments are welcome,
Nicolas
-- 
2.33.0

[PATCH ipsec 1/2] xfrm: make user policy API complete

[Nicolas Dichtel]

From a userland POV, this API was based on some magic values:
 - dirmask and action were bitfields but meaning of bits
   (XFRM_POL_DEFAULT_*) are not exported;
 - action is confusing, if a bit is set, does it mean drop or accept?

Let's try to simplify this uapi by using explicit field and macros.

Fixes: 2d151d39073a ("xfrm: Add possibility to set the default to block if we have no policy")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/uapi/linux/xfrm.h |  9 ++++++---
 net/xfrm/xfrm_user.c      | 27 ++++++++++++++++++---------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index b96c1ea7166d..3e605b09eb6f 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -514,9 +514,12 @@ struct xfrm_user_offload {
 #define XFRM_OFFLOAD_INBOUND	2
 
 struct xfrm_userpolicy_default {
-#define XFRM_USERPOLICY_DIRMASK_MAX	(sizeof(__u8) * 8)
-	__u8				dirmask;
-	__u8				action;
+#define XFRM_USERPOLICY_UNSPEC	0
+#define XFRM_USERPOLICY_BLOCK	1
+#define XFRM_USERPOLICY_ACCEPT	2
+	__u8				in;
+	__u8				fwd;
+	__u8				out;
 };
 
 #ifndef __KERNEL__
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 03b66d154b2b..4e1c4dd53fe2 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1966,16 +1966,21 @@ static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 {
 	struct net *net = sock_net(skb->sk);
 	struct xfrm_userpolicy_default *up = nlmsg_data(nlh);
-	u8 dirmask;
-	u8 old_default = net->xfrm.policy_default;
 
-	if (up->dirmask >= XFRM_USERPOLICY_DIRMASK_MAX)
-		return -EINVAL;
+	if (up->in == XFRM_USERPOLICY_BLOCK)
+		net->xfrm.policy_default |= XFRM_POL_DEFAULT_IN;
+	else if (up->in == XFRM_USERPOLICY_ACCEPT)
+		net->xfrm.policy_default &= ~XFRM_POL_DEFAULT_IN;
 
-	dirmask = (1 << up->dirmask) & XFRM_POL_DEFAULT_MASK;
+	if (up->fwd == XFRM_USERPOLICY_BLOCK)
+		net->xfrm.policy_default |= XFRM_POL_DEFAULT_FWD;
+	else if (up->fwd == XFRM_USERPOLICY_ACCEPT)
+		net->xfrm.policy_default &= ~XFRM_POL_DEFAULT_FWD;
 
-	net->xfrm.policy_default = (old_default & (0xff ^ dirmask))
-				    | (up->action << up->dirmask);
+	if (up->out == XFRM_USERPOLICY_BLOCK)
+		net->xfrm.policy_default |= XFRM_POL_DEFAULT_OUT;
+	else if (up->out == XFRM_USERPOLICY_ACCEPT)
+		net->xfrm.policy_default &= ~XFRM_POL_DEFAULT_OUT;
 
 	rt_genid_bump_all(net);
 
@@ -2007,8 +2012,12 @@ static int xfrm_get_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	r_up = nlmsg_data(r_nlh);
 
-	r_up->action = ((net->xfrm.policy_default & (1 << up->dirmask)) >> up->dirmask);
-	r_up->dirmask = up->dirmask;
+	r_up->in = net->xfrm.policy_default & XFRM_POL_DEFAULT_IN ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	r_up->fwd = net->xfrm.policy_default & XFRM_POL_DEFAULT_FWD ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	r_up->out = net->xfrm.policy_default & XFRM_POL_DEFAULT_OUT ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
 	nlmsg_end(r_skb, r_nlh);
 
 	return nlmsg_unicast(net->xfrm.nlsk, r_skb, portid);
-- 
2.33.0

[PATCH ipsec 2/2] xfrm: notify default policy on update

[Nicolas Dichtel]

This configuration knob is very sensible, it should be notified when
changing.

Fixes: 2d151d39073a ("xfrm: Add possibility to set the default to block if we have no policy")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/xfrm/xfrm_user.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 4e1c4dd53fe2..af9803f18ff7 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1961,6 +1961,36 @@ static struct sk_buff *xfrm_policy_netlink(struct sk_buff *in_skb,
 	return skb;
 }
 
+static int xfrm_notify_userpolicy(const struct net *net)
+{
+	struct xfrm_userpolicy_default *up;
+	int len = NLMSG_ALIGN(sizeof(*up));
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+
+	skb = nlmsg_new(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+
+	nlh = nlmsg_put(skb, 0, 0, XFRM_MSG_GETDEFAULT, sizeof(*up), 0);
+	if (nlh == NULL) {
+		kfree_skb(skb);
+		return -EMSGSIZE;
+	}
+
+	up = nlmsg_data(nlh);
+	up->in = net->xfrm.policy_default & XFRM_POL_DEFAULT_IN ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	up->fwd = net->xfrm.policy_default & XFRM_POL_DEFAULT_FWD ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	up->out = net->xfrm.policy_default & XFRM_POL_DEFAULT_OUT ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+
+	nlmsg_end(skb, nlh);
+
+	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_POLICY);
+}
+
 static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 			    struct nlattr **attrs)
 {
@@ -1984,6 +2014,7 @@ static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	rt_genid_bump_all(net);
 
+	xfrm_notify_userpolicy(net);
 	return 0;
 }
 
-- 
2.33.0

Re: [PATCH ipsec 2/2] xfrm: notify default policy on update

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3514 bytes --]

Hi Nicolas,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on ipsec-next/master]
[also build test ERROR on net-next/master net/master next-20210907]
[cannot apply to ipsec/master v5.14]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nicolas-Dichtel/xfrm-make-user-policy-API-complete/20210908-043604
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
config: x86_64-randconfig-a011-20210906 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 9c476172b93367d2cb88d7d3f4b1b5b456fa6020)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/6f3af39482e7fa1c873b3e6ee460a03feb7b796a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nicolas-Dichtel/xfrm-make-user-policy-API-complete/20210908-043604
        git checkout 6f3af39482e7fa1c873b3e6ee460a03feb7b796a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net/xfrm/xfrm_user.c:1991:30: error: passing 'const struct net *' to parameter of type 'struct net *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
           return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_POLICY);
                                       ^~~
   net/xfrm/xfrm_user.c:1154:52: note: passing argument to parameter 'net' here
   static inline int xfrm_nlmsg_multicast(struct net *net, struct sk_buff *skb,
                                                      ^
   net/xfrm/xfrm_user.c:2027:41: warning: variable 'up' set but not used [-Wunused-but-set-variable]
           struct xfrm_userpolicy_default *r_up, *up;
                                                  ^
   1 warning and 1 error generated.


vim +1991 net/xfrm/xfrm_user.c

  1963	
  1964	static int xfrm_notify_userpolicy(const struct net *net)
  1965	{
  1966		struct xfrm_userpolicy_default *up;
  1967		int len = NLMSG_ALIGN(sizeof(*up));
  1968		struct nlmsghdr *nlh;
  1969		struct sk_buff *skb;
  1970	
  1971		skb = nlmsg_new(len, GFP_ATOMIC);
  1972		if (skb == NULL)
  1973			return -ENOMEM;
  1974	
  1975		nlh = nlmsg_put(skb, 0, 0, XFRM_MSG_GETDEFAULT, sizeof(*up), 0);
  1976		if (nlh == NULL) {
  1977			kfree_skb(skb);
  1978			return -EMSGSIZE;
  1979		}
  1980	
  1981		up = nlmsg_data(nlh);
  1982		up->in = net->xfrm.policy_default & XFRM_POL_DEFAULT_IN ?
  1983				XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
  1984		up->fwd = net->xfrm.policy_default & XFRM_POL_DEFAULT_FWD ?
  1985				XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
  1986		up->out = net->xfrm.policy_default & XFRM_POL_DEFAULT_OUT ?
  1987				XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
  1988	
  1989		nlmsg_end(skb, nlh);
  1990	
> 1991		return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_POLICY);
  1992	}
  1993	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37256 bytes --]

[PATCH ipsec v2 0/2] xfrm: fix uapi for the default policy

[Nicolas Dichtel]

This feature has just been merged after the last release, thus it's still
time to fix the uapi.
As stated in the thread, the uapi is based on some magic values (from the
userland POV).
Here is a proposal to simplify this uapi and make it clear how to use it.
The other problem was the notification: changing the default policy may
radically change the packets flows.

v1 -> v2: fix warnings reported by the kernel test robot

Nicolas Dichtel (2):
  xfrm: make user policy API complete
  xfrm: notify default policy on update

 include/uapi/linux/xfrm.h |  9 ++++--
 net/xfrm/xfrm_user.c      | 62 +++++++++++++++++++++++++++++++--------
 2 files changed, 56 insertions(+), 15 deletions(-)

Comments are welcome,
Nicolas
-- 
2.33.0

[PATCH ipsec v2 1/2] xfrm: make user policy API complete

[Nicolas Dichtel]

From a userland POV, this API was based on some magic values:
 - dirmask and action were bitfields but meaning of bits
   (XFRM_POL_DEFAULT_*) are not exported;
 - action is confusing, if a bit is set, does it mean drop or accept?

Let's try to simplify this uapi by using explicit field and macros.

Fixes: 2d151d39073a ("xfrm: Add possibility to set the default to block if we have no policy")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/uapi/linux/xfrm.h |  9 ++++++---
 net/xfrm/xfrm_user.c      | 31 +++++++++++++++++++------------
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index b96c1ea7166d..3e605b09eb6f 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -514,9 +514,12 @@ struct xfrm_user_offload {
 #define XFRM_OFFLOAD_INBOUND	2
 
 struct xfrm_userpolicy_default {
-#define XFRM_USERPOLICY_DIRMASK_MAX	(sizeof(__u8) * 8)
-	__u8				dirmask;
-	__u8				action;
+#define XFRM_USERPOLICY_UNSPEC	0
+#define XFRM_USERPOLICY_BLOCK	1
+#define XFRM_USERPOLICY_ACCEPT	2
+	__u8				in;
+	__u8				fwd;
+	__u8				out;
 };
 
 #ifndef __KERNEL__
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 03b66d154b2b..90c88390f1fe 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1966,16 +1966,21 @@ static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 {
 	struct net *net = sock_net(skb->sk);
 	struct xfrm_userpolicy_default *up = nlmsg_data(nlh);
-	u8 dirmask;
-	u8 old_default = net->xfrm.policy_default;
 
-	if (up->dirmask >= XFRM_USERPOLICY_DIRMASK_MAX)
-		return -EINVAL;
+	if (up->in == XFRM_USERPOLICY_BLOCK)
+		net->xfrm.policy_default |= XFRM_POL_DEFAULT_IN;
+	else if (up->in == XFRM_USERPOLICY_ACCEPT)
+		net->xfrm.policy_default &= ~XFRM_POL_DEFAULT_IN;
 
-	dirmask = (1 << up->dirmask) & XFRM_POL_DEFAULT_MASK;
+	if (up->fwd == XFRM_USERPOLICY_BLOCK)
+		net->xfrm.policy_default |= XFRM_POL_DEFAULT_FWD;
+	else if (up->fwd == XFRM_USERPOLICY_ACCEPT)
+		net->xfrm.policy_default &= ~XFRM_POL_DEFAULT_FWD;
 
-	net->xfrm.policy_default = (old_default & (0xff ^ dirmask))
-				    | (up->action << up->dirmask);
+	if (up->out == XFRM_USERPOLICY_BLOCK)
+		net->xfrm.policy_default |= XFRM_POL_DEFAULT_OUT;
+	else if (up->out == XFRM_USERPOLICY_ACCEPT)
+		net->xfrm.policy_default &= ~XFRM_POL_DEFAULT_OUT;
 
 	rt_genid_bump_all(net);
 
@@ -1988,13 +1993,11 @@ static int xfrm_get_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct sk_buff *r_skb;
 	struct nlmsghdr *r_nlh;
 	struct net *net = sock_net(skb->sk);
-	struct xfrm_userpolicy_default *r_up, *up;
+	struct xfrm_userpolicy_default *r_up;
 	int len = NLMSG_ALIGN(sizeof(struct xfrm_userpolicy_default));
 	u32 portid = NETLINK_CB(skb).portid;
 	u32 seq = nlh->nlmsg_seq;
 
-	up = nlmsg_data(nlh);
-
 	r_skb = nlmsg_new(len, GFP_ATOMIC);
 	if (!r_skb)
 		return -ENOMEM;
@@ -2007,8 +2010,12 @@ static int xfrm_get_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	r_up = nlmsg_data(r_nlh);
 
-	r_up->action = ((net->xfrm.policy_default & (1 << up->dirmask)) >> up->dirmask);
-	r_up->dirmask = up->dirmask;
+	r_up->in = net->xfrm.policy_default & XFRM_POL_DEFAULT_IN ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	r_up->fwd = net->xfrm.policy_default & XFRM_POL_DEFAULT_FWD ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	r_up->out = net->xfrm.policy_default & XFRM_POL_DEFAULT_OUT ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
 	nlmsg_end(r_skb, r_nlh);
 
 	return nlmsg_unicast(net->xfrm.nlsk, r_skb, portid);
-- 
2.33.0

[PATCH ipsec v2 2/2] xfrm: notify default policy on update

[Nicolas Dichtel]

This configuration knob is very sensible, it should be notified when
changing.

Fixes: 2d151d39073a ("xfrm: Add possibility to set the default to block if we have no policy")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/xfrm/xfrm_user.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 90c88390f1fe..0eba0c27c665 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1961,6 +1961,36 @@ static struct sk_buff *xfrm_policy_netlink(struct sk_buff *in_skb,
 	return skb;
 }
 
+static int xfrm_notify_userpolicy(struct net *net)
+{
+	struct xfrm_userpolicy_default *up;
+	int len = NLMSG_ALIGN(sizeof(*up));
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+
+	skb = nlmsg_new(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+
+	nlh = nlmsg_put(skb, 0, 0, XFRM_MSG_GETDEFAULT, sizeof(*up), 0);
+	if (nlh == NULL) {
+		kfree_skb(skb);
+		return -EMSGSIZE;
+	}
+
+	up = nlmsg_data(nlh);
+	up->in = net->xfrm.policy_default & XFRM_POL_DEFAULT_IN ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	up->fwd = net->xfrm.policy_default & XFRM_POL_DEFAULT_FWD ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	up->out = net->xfrm.policy_default & XFRM_POL_DEFAULT_OUT ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+
+	nlmsg_end(skb, nlh);
+
+	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_POLICY);
+}
+
 static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 			    struct nlattr **attrs)
 {
@@ -1984,6 +2014,7 @@ static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	rt_genid_bump_all(net);
 
+	xfrm_notify_userpolicy(net);
 	return 0;
 }
 
-- 
2.33.0

[RFC PATCH iproute2 v2] xfrm: enable to manage default policies

[Nicolas Dichtel]

Two new commands to manage default policies:
 - ip xfrm policy setdefault
 - ip xfrm policy getdefault

And the corresponding part in 'ip xfrm monitor'.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/uapi/linux/xfrm.h |   9 ++-
 ip/xfrm.h                 |   1 +
 ip/xfrm_monitor.c         |   3 +
 ip/xfrm_policy.c          | 121 ++++++++++++++++++++++++++++++++++++++
 man/man8/ip-xfrm.8        |  12 ++++
 5 files changed, 143 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index ecd06396eb16..2a8135e31f9d 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -514,9 +514,12 @@ struct xfrm_user_offload {
 #define XFRM_OFFLOAD_INBOUND	2
 
 struct xfrm_userpolicy_default {
-#define XFRM_USERPOLICY_DIRMASK_MAX	(sizeof(__u8) * 8)
-	__u8				dirmask;
-	__u8				action;
+#define XFRM_USERPOLICY_UNSPEC	0
+#define XFRM_USERPOLICY_BLOCK	1
+#define XFRM_USERPOLICY_ACCEPT	2
+	__u8				in;
+	__u8				fwd;
+	__u8				out;
 };
 
 /* backwards compatibility for userspace */
diff --git a/ip/xfrm.h b/ip/xfrm.h
index 9ba5ca61d5e4..17dcf3fea83f 100644
--- a/ip/xfrm.h
+++ b/ip/xfrm.h
@@ -132,6 +132,7 @@ void xfrm_state_info_print(struct xfrm_usersa_info *xsinfo,
 void xfrm_policy_info_print(struct xfrm_userpolicy_info *xpinfo,
 			    struct rtattr *tb[], FILE *fp, const char *prefix,
 			    const char *title);
+int xfrm_policy_default_print(struct nlmsghdr *n, FILE *fp);
 int xfrm_id_parse(xfrm_address_t *saddr, struct xfrm_id *id, __u16 *family,
 		  int loose, int *argcp, char ***argvp);
 int xfrm_mode_parse(__u8 *mode, int *argcp, char ***argvp);
diff --git a/ip/xfrm_monitor.c b/ip/xfrm_monitor.c
index e34b5fbda130..f67424c5be06 100644
--- a/ip/xfrm_monitor.c
+++ b/ip/xfrm_monitor.c
@@ -323,6 +323,9 @@ static int xfrm_accept_msg(struct rtnl_ctrl_data *ctrl,
 	case XFRM_MSG_MAPPING:
 		xfrm_mapping_print(n, arg);
 		return 0;
+	case XFRM_MSG_GETDEFAULT:
+		xfrm_policy_default_print(n, arg);
+		return 0;
 	default:
 		break;
 	}
diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index 7cc00e7c2f5b..744f331ff564 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -66,6 +66,8 @@ static void usage(void)
 		"Usage: ip xfrm policy flush [ ptype PTYPE ]\n"
 		"Usage: ip xfrm policy count\n"
 		"Usage: ip xfrm policy set [ hthresh4 LBITS RBITS ] [ hthresh6 LBITS RBITS ]\n"
+		"Usage: ip xfrm policy setdefault DIR ACTION [ DIR ACTION ] [ DIR ACTION ]\n"
+		"Usage: ip xfrm policy getdefault\n"
 		"SELECTOR := [ src ADDR[/PLEN] ] [ dst ADDR[/PLEN] ] [ dev DEV ] [ UPSPEC ]\n"
 		"UPSPEC := proto { { tcp | udp | sctp | dccp } [ sport PORT ] [ dport PORT ] |\n"
 		"                  { icmp | ipv6-icmp | mobility-header } [ type NUMBER ] [ code NUMBER ] |\n"
@@ -1124,6 +1126,121 @@ static int xfrm_spd_getinfo(int argc, char **argv)
 	return 0;
 }
 
+static int xfrm_spd_setdefault(int argc, char **argv)
+{
+	struct rtnl_handle rth;
+	struct {
+		struct nlmsghdr			n;
+		struct xfrm_userpolicy_default  up;
+	} req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_default)),
+		.n.nlmsg_flags = NLM_F_REQUEST,
+		.n.nlmsg_type = XFRM_MSG_SETDEFAULT,
+	};
+
+	while (argc > 0) {
+		if (strcmp(*argv, "in") == 0) {
+			if (req.up.in)
+				duparg("in", *argv);
+
+			NEXT_ARG();
+			if (strcmp(*argv, "block") == 0)
+				req.up.in = XFRM_USERPOLICY_BLOCK;
+			else if (strcmp(*argv, "accept") == 0)
+				req.up.in = XFRM_USERPOLICY_ACCEPT;
+			else
+				invarg("in policy value is invalid", *argv);
+		} else if (strcmp(*argv, "fwd") == 0) {
+			if (req.up.fwd)
+				duparg("fwd", *argv);
+
+			NEXT_ARG();
+			if (strcmp(*argv, "block") == 0)
+				req.up.fwd = XFRM_USERPOLICY_BLOCK;
+			else if (strcmp(*argv, "accept") == 0)
+				req.up.fwd = XFRM_USERPOLICY_ACCEPT;
+			else
+				invarg("fwd policy value is invalid", *argv);
+		} else if (strcmp(*argv, "out") == 0) {
+			if (req.up.out)
+				duparg("out", *argv);
+
+			NEXT_ARG();
+			if (strcmp(*argv, "block") == 0)
+				req.up.out = XFRM_USERPOLICY_BLOCK;
+			else if (strcmp(*argv, "accept") == 0)
+				req.up.out = XFRM_USERPOLICY_ACCEPT;
+			else
+				invarg("out policy value is invalid", *argv);
+		} else {
+			invarg("unknown direction", *argv);
+		}
+
+		argc--; argv++;
+	}
+
+	if (rtnl_open_byproto(&rth, 0, NETLINK_XFRM) < 0)
+		exit(1);
+
+	if (rtnl_talk(&rth, &req.n, NULL) < 0)
+		exit(2);
+
+	rtnl_close(&rth);
+
+	return 0;
+}
+
+int xfrm_policy_default_print(struct nlmsghdr *n, FILE *fp)
+{
+	struct xfrm_userpolicy_default *up = NLMSG_DATA(n);
+	int len = n->nlmsg_len - NLMSG_SPACE(sizeof(*up));
+
+	if (len < 0) {
+		fprintf(stderr,
+			"BUG: short nlmsg len %u (expect %lu) for XFRM_MSG_GETDEFAULT\n",
+			n->nlmsg_len, NLMSG_SPACE(sizeof(*up)));
+		return -1;
+	}
+
+	fprintf(fp, "Default policies:\n");
+	fprintf(fp, " in:  %s\n",
+		up->in == XFRM_USERPOLICY_BLOCK ? "block" : "accept");
+	fprintf(fp, " fwd: %s\n",
+		up->fwd == XFRM_USERPOLICY_BLOCK ? "block" : "accept");
+	fprintf(fp, " out: %s\n",
+		up->out == XFRM_USERPOLICY_BLOCK ? "block" : "accept");
+	fflush(fp);
+
+	return 0;
+}
+
+static int xfrm_spd_getdefault(int argc, char **argv)
+{
+	struct rtnl_handle rth;
+	struct {
+		struct nlmsghdr			n;
+		struct xfrm_userpolicy_default  up;
+	} req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_default)),
+		.n.nlmsg_flags = NLM_F_REQUEST,
+		.n.nlmsg_type = XFRM_MSG_GETDEFAULT,
+	};
+	struct nlmsghdr *answer;
+
+	if (rtnl_open_byproto(&rth, 0, NETLINK_XFRM) < 0)
+		exit(1);
+
+	if (rtnl_talk(&rth, &req.n, &answer) < 0)
+		exit(2);
+
+	xfrm_policy_default_print(answer, (FILE *)stdout);
+
+	free(answer);
+	rtnl_close(&rth);
+
+	return 0;
+}
+
 static int xfrm_policy_flush(int argc, char **argv)
 {
 	struct rtnl_handle rth;
@@ -1197,6 +1314,10 @@ int do_xfrm_policy(int argc, char **argv)
 		return xfrm_spd_getinfo(argc, argv);
 	if (matches(*argv, "set") == 0)
 		return xfrm_spd_setinfo(argc-1, argv+1);
+	if (matches(*argv, "setdefault") == 0)
+		return xfrm_spd_setdefault(argc-1, argv+1);
+	if (matches(*argv, "getdefault") == 0)
+		return xfrm_spd_getdefault(argc-1, argv+1);
 	if (matches(*argv, "help") == 0)
 		usage();
 	fprintf(stderr, "Command \"%s\" is unknown, try \"ip xfrm policy help\".\n", *argv);
diff --git a/man/man8/ip-xfrm.8 b/man/man8/ip-xfrm.8
index 003f6c3d1c28..bf725cabb82d 100644
--- a/man/man8/ip-xfrm.8
+++ b/man/man8/ip-xfrm.8
@@ -298,6 +298,18 @@ ip-xfrm \- transform configuration
 .RB "[ " hthresh6
 .IR LBITS " " RBITS " ]"
 
+.ti -8
+.B "ip xfrm policy setdefault"
+.IR DIR
+.IR ACTION " [ "
+.IR DIR
+.IR ACTION " ] [ "
+.IR DIR
+.IR ACTION " ]"
+
+.ti -8
+.B "ip xfrm policy getdefault"
+
 .ti -8
 .IR SELECTOR " :="
 .RB "[ " src
-- 
2.33.0

[PATCH ipsec v3 0/2] xfrm: fix uapi for the default policy

[Nicolas Dichtel]

This feature has just been merged after the last release, thus it's still
time to fix the uapi.
As stated in the thread, the uapi is based on some magic values (from the
userland POV).
Here is a proposal to simplify this uapi and make it clear how to use it.
The other problem was the notification: changing the default policy may
radically change the packets flows.

v2 -> v3: rebase on top of ipsec tree

v1 -> v2: fix warnings reported by the kernel test robot

Nicolas Dichtel (2):
  xfrm: make user policy API complete
  xfrm: notify default policy on update

 include/uapi/linux/xfrm.h |  9 ++++--
 net/xfrm/xfrm_user.c      | 67 +++++++++++++++++++++++++++++----------
 2 files changed, 56 insertions(+), 20 deletions(-)

Comments are welcome,
Nicolas
-- 
2.33.0

[PATCH ipsec v3 1/2] xfrm: make user policy API complete

[Nicolas Dichtel]

From a userland POV, this API was based on some magic values:
 - dirmask and action were bitfields but meaning of bits
   (XFRM_POL_DEFAULT_*) are not exported;
 - action is confusing, if a bit is set, does it mean drop or accept?

Let's try to simplify this uapi by using explicit field and macros.

Fixes: 2d151d39073a ("xfrm: Add possibility to set the default to block if we have no policy")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/uapi/linux/xfrm.h |  9 ++++++---
 net/xfrm/xfrm_user.c      | 36 +++++++++++++++++++-----------------
 2 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index b96c1ea7166d..3e605b09eb6f 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -514,9 +514,12 @@ struct xfrm_user_offload {
 #define XFRM_OFFLOAD_INBOUND	2
 
 struct xfrm_userpolicy_default {
-#define XFRM_USERPOLICY_DIRMASK_MAX	(sizeof(__u8) * 8)
-	__u8				dirmask;
-	__u8				action;
+#define XFRM_USERPOLICY_UNSPEC	0
+#define XFRM_USERPOLICY_BLOCK	1
+#define XFRM_USERPOLICY_ACCEPT	2
+	__u8				in;
+	__u8				fwd;
+	__u8				out;
 };
 
 #ifndef __KERNEL__
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 4719a6d54aa6..90c88390f1fe 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1966,16 +1966,21 @@ static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 {
 	struct net *net = sock_net(skb->sk);
 	struct xfrm_userpolicy_default *up = nlmsg_data(nlh);
-	u8 dirmask;
-	u8 old_default = net->xfrm.policy_default;
 
-	if (up->dirmask >= XFRM_USERPOLICY_DIRMASK_MAX)
-		return -EINVAL;
+	if (up->in == XFRM_USERPOLICY_BLOCK)
+		net->xfrm.policy_default |= XFRM_POL_DEFAULT_IN;
+	else if (up->in == XFRM_USERPOLICY_ACCEPT)
+		net->xfrm.policy_default &= ~XFRM_POL_DEFAULT_IN;
 
-	dirmask = (1 << up->dirmask) & XFRM_POL_DEFAULT_MASK;
+	if (up->fwd == XFRM_USERPOLICY_BLOCK)
+		net->xfrm.policy_default |= XFRM_POL_DEFAULT_FWD;
+	else if (up->fwd == XFRM_USERPOLICY_ACCEPT)
+		net->xfrm.policy_default &= ~XFRM_POL_DEFAULT_FWD;
 
-	net->xfrm.policy_default = (old_default & (0xff ^ dirmask))
-				    | (up->action << up->dirmask);
+	if (up->out == XFRM_USERPOLICY_BLOCK)
+		net->xfrm.policy_default |= XFRM_POL_DEFAULT_OUT;
+	else if (up->out == XFRM_USERPOLICY_ACCEPT)
+		net->xfrm.policy_default &= ~XFRM_POL_DEFAULT_OUT;
 
 	rt_genid_bump_all(net);
 
@@ -1988,13 +1993,11 @@ static int xfrm_get_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct sk_buff *r_skb;
 	struct nlmsghdr *r_nlh;
 	struct net *net = sock_net(skb->sk);
-	struct xfrm_userpolicy_default *r_up, *up;
+	struct xfrm_userpolicy_default *r_up;
 	int len = NLMSG_ALIGN(sizeof(struct xfrm_userpolicy_default));
 	u32 portid = NETLINK_CB(skb).portid;
 	u32 seq = nlh->nlmsg_seq;
 
-	up = nlmsg_data(nlh);
-
 	r_skb = nlmsg_new(len, GFP_ATOMIC);
 	if (!r_skb)
 		return -ENOMEM;
@@ -2005,15 +2008,14 @@ static int xfrm_get_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 		return -EMSGSIZE;
 	}
 
-	if (up->dirmask >= XFRM_USERPOLICY_DIRMASK_MAX) {
-		kfree_skb(r_skb);
-		return -EINVAL;
-	}
-
 	r_up = nlmsg_data(r_nlh);
 
-	r_up->action = ((net->xfrm.policy_default & (1 << up->dirmask)) >> up->dirmask);
-	r_up->dirmask = up->dirmask;
+	r_up->in = net->xfrm.policy_default & XFRM_POL_DEFAULT_IN ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	r_up->fwd = net->xfrm.policy_default & XFRM_POL_DEFAULT_FWD ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	r_up->out = net->xfrm.policy_default & XFRM_POL_DEFAULT_OUT ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
 	nlmsg_end(r_skb, r_nlh);
 
 	return nlmsg_unicast(net->xfrm.nlsk, r_skb, portid);
-- 
2.33.0

[PATCH ipsec v3 2/2] xfrm: notify default policy on update

[Nicolas Dichtel]

This configuration knob is very sensible, it should be notified when
changing.

Fixes: 2d151d39073a ("xfrm: Add possibility to set the default to block if we have no policy")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/xfrm/xfrm_user.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 90c88390f1fe..0eba0c27c665 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1961,6 +1961,36 @@ static struct sk_buff *xfrm_policy_netlink(struct sk_buff *in_skb,
 	return skb;
 }
 
+static int xfrm_notify_userpolicy(struct net *net)
+{
+	struct xfrm_userpolicy_default *up;
+	int len = NLMSG_ALIGN(sizeof(*up));
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+
+	skb = nlmsg_new(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+
+	nlh = nlmsg_put(skb, 0, 0, XFRM_MSG_GETDEFAULT, sizeof(*up), 0);
+	if (nlh == NULL) {
+		kfree_skb(skb);
+		return -EMSGSIZE;
+	}
+
+	up = nlmsg_data(nlh);
+	up->in = net->xfrm.policy_default & XFRM_POL_DEFAULT_IN ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	up->fwd = net->xfrm.policy_default & XFRM_POL_DEFAULT_FWD ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+	up->out = net->xfrm.policy_default & XFRM_POL_DEFAULT_OUT ?
+			XFRM_USERPOLICY_BLOCK : XFRM_USERPOLICY_ACCEPT;
+
+	nlmsg_end(skb, nlh);
+
+	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_POLICY);
+}
+
 static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 			    struct nlattr **attrs)
 {
@@ -1984,6 +2014,7 @@ static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	rt_genid_bump_all(net);
 
+	xfrm_notify_userpolicy(net);
 	return 0;
 }
 
-- 
2.33.0

[RFC PATCH iproute2 v2] xfrm: enable to manage default policies

[Nicolas Dichtel]

Two new commands to manage default policies:
 - ip xfrm policy setdefault
 - ip xfrm policy getdefault

And the corresponding part in 'ip xfrm monitor'.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/uapi/linux/xfrm.h |   9 ++-
 ip/xfrm.h                 |   1 +
 ip/xfrm_monitor.c         |   3 +
 ip/xfrm_policy.c          | 121 ++++++++++++++++++++++++++++++++++++++
 man/man8/ip-xfrm.8        |  12 ++++
 5 files changed, 143 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index ecd06396eb16..2a8135e31f9d 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -514,9 +514,12 @@ struct xfrm_user_offload {
 #define XFRM_OFFLOAD_INBOUND	2
 
 struct xfrm_userpolicy_default {
-#define XFRM_USERPOLICY_DIRMASK_MAX	(sizeof(__u8) * 8)
-	__u8				dirmask;
-	__u8				action;
+#define XFRM_USERPOLICY_UNSPEC	0
+#define XFRM_USERPOLICY_BLOCK	1
+#define XFRM_USERPOLICY_ACCEPT	2
+	__u8				in;
+	__u8				fwd;
+	__u8				out;
 };
 
 /* backwards compatibility for userspace */
diff --git a/ip/xfrm.h b/ip/xfrm.h
index 9ba5ca61d5e4..17dcf3fea83f 100644
--- a/ip/xfrm.h
+++ b/ip/xfrm.h
@@ -132,6 +132,7 @@ void xfrm_state_info_print(struct xfrm_usersa_info *xsinfo,
 void xfrm_policy_info_print(struct xfrm_userpolicy_info *xpinfo,
 			    struct rtattr *tb[], FILE *fp, const char *prefix,
 			    const char *title);
+int xfrm_policy_default_print(struct nlmsghdr *n, FILE *fp);
 int xfrm_id_parse(xfrm_address_t *saddr, struct xfrm_id *id, __u16 *family,
 		  int loose, int *argcp, char ***argvp);
 int xfrm_mode_parse(__u8 *mode, int *argcp, char ***argvp);
diff --git a/ip/xfrm_monitor.c b/ip/xfrm_monitor.c
index e34b5fbda130..f67424c5be06 100644
--- a/ip/xfrm_monitor.c
+++ b/ip/xfrm_monitor.c
@@ -323,6 +323,9 @@ static int xfrm_accept_msg(struct rtnl_ctrl_data *ctrl,
 	case XFRM_MSG_MAPPING:
 		xfrm_mapping_print(n, arg);
 		return 0;
+	case XFRM_MSG_GETDEFAULT:
+		xfrm_policy_default_print(n, arg);
+		return 0;
 	default:
 		break;
 	}
diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index 7cc00e7c2f5b..744f331ff564 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -66,6 +66,8 @@ static void usage(void)
 		"Usage: ip xfrm policy flush [ ptype PTYPE ]\n"
 		"Usage: ip xfrm policy count\n"
 		"Usage: ip xfrm policy set [ hthresh4 LBITS RBITS ] [ hthresh6 LBITS RBITS ]\n"
+		"Usage: ip xfrm policy setdefault DIR ACTION [ DIR ACTION ] [ DIR ACTION ]\n"
+		"Usage: ip xfrm policy getdefault\n"
 		"SELECTOR := [ src ADDR[/PLEN] ] [ dst ADDR[/PLEN] ] [ dev DEV ] [ UPSPEC ]\n"
 		"UPSPEC := proto { { tcp | udp | sctp | dccp } [ sport PORT ] [ dport PORT ] |\n"
 		"                  { icmp | ipv6-icmp | mobility-header } [ type NUMBER ] [ code NUMBER ] |\n"
@@ -1124,6 +1126,121 @@ static int xfrm_spd_getinfo(int argc, char **argv)
 	return 0;
 }
 
+static int xfrm_spd_setdefault(int argc, char **argv)
+{
+	struct rtnl_handle rth;
+	struct {
+		struct nlmsghdr			n;
+		struct xfrm_userpolicy_default  up;
+	} req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_default)),
+		.n.nlmsg_flags = NLM_F_REQUEST,
+		.n.nlmsg_type = XFRM_MSG_SETDEFAULT,
+	};
+
+	while (argc > 0) {
+		if (strcmp(*argv, "in") == 0) {
+			if (req.up.in)
+				duparg("in", *argv);
+
+			NEXT_ARG();
+			if (strcmp(*argv, "block") == 0)
+				req.up.in = XFRM_USERPOLICY_BLOCK;
+			else if (strcmp(*argv, "accept") == 0)
+				req.up.in = XFRM_USERPOLICY_ACCEPT;
+			else
+				invarg("in policy value is invalid", *argv);
+		} else if (strcmp(*argv, "fwd") == 0) {
+			if (req.up.fwd)
+				duparg("fwd", *argv);
+
+			NEXT_ARG();
+			if (strcmp(*argv, "block") == 0)
+				req.up.fwd = XFRM_USERPOLICY_BLOCK;
+			else if (strcmp(*argv, "accept") == 0)
+				req.up.fwd = XFRM_USERPOLICY_ACCEPT;
+			else
+				invarg("fwd policy value is invalid", *argv);
+		} else if (strcmp(*argv, "out") == 0) {
+			if (req.up.out)
+				duparg("out", *argv);
+
+			NEXT_ARG();
+			if (strcmp(*argv, "block") == 0)
+				req.up.out = XFRM_USERPOLICY_BLOCK;
+			else if (strcmp(*argv, "accept") == 0)
+				req.up.out = XFRM_USERPOLICY_ACCEPT;
+			else
+				invarg("out policy value is invalid", *argv);
+		} else {
+			invarg("unknown direction", *argv);
+		}
+
+		argc--; argv++;
+	}
+
+	if (rtnl_open_byproto(&rth, 0, NETLINK_XFRM) < 0)
+		exit(1);
+
+	if (rtnl_talk(&rth, &req.n, NULL) < 0)
+		exit(2);
+
+	rtnl_close(&rth);
+
+	return 0;
+}
+
+int xfrm_policy_default_print(struct nlmsghdr *n, FILE *fp)
+{
+	struct xfrm_userpolicy_default *up = NLMSG_DATA(n);
+	int len = n->nlmsg_len - NLMSG_SPACE(sizeof(*up));
+
+	if (len < 0) {
+		fprintf(stderr,
+			"BUG: short nlmsg len %u (expect %lu) for XFRM_MSG_GETDEFAULT\n",
+			n->nlmsg_len, NLMSG_SPACE(sizeof(*up)));
+		return -1;
+	}
+
+	fprintf(fp, "Default policies:\n");
+	fprintf(fp, " in:  %s\n",
+		up->in == XFRM_USERPOLICY_BLOCK ? "block" : "accept");
+	fprintf(fp, " fwd: %s\n",
+		up->fwd == XFRM_USERPOLICY_BLOCK ? "block" : "accept");
+	fprintf(fp, " out: %s\n",
+		up->out == XFRM_USERPOLICY_BLOCK ? "block" : "accept");
+	fflush(fp);
+
+	return 0;
+}
+
+static int xfrm_spd_getdefault(int argc, char **argv)
+{
+	struct rtnl_handle rth;
+	struct {
+		struct nlmsghdr			n;
+		struct xfrm_userpolicy_default  up;
+	} req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_default)),
+		.n.nlmsg_flags = NLM_F_REQUEST,
+		.n.nlmsg_type = XFRM_MSG_GETDEFAULT,
+	};
+	struct nlmsghdr *answer;
+
+	if (rtnl_open_byproto(&rth, 0, NETLINK_XFRM) < 0)
+		exit(1);
+
+	if (rtnl_talk(&rth, &req.n, &answer) < 0)
+		exit(2);
+
+	xfrm_policy_default_print(answer, (FILE *)stdout);
+
+	free(answer);
+	rtnl_close(&rth);
+
+	return 0;
+}
+
 static int xfrm_policy_flush(int argc, char **argv)
 {
 	struct rtnl_handle rth;
@@ -1197,6 +1314,10 @@ int do_xfrm_policy(int argc, char **argv)
 		return xfrm_spd_getinfo(argc, argv);
 	if (matches(*argv, "set") == 0)
 		return xfrm_spd_setinfo(argc-1, argv+1);
+	if (matches(*argv, "setdefault") == 0)
+		return xfrm_spd_setdefault(argc-1, argv+1);
+	if (matches(*argv, "getdefault") == 0)
+		return xfrm_spd_getdefault(argc-1, argv+1);
 	if (matches(*argv, "help") == 0)
 		usage();
 	fprintf(stderr, "Command \"%s\" is unknown, try \"ip xfrm policy help\".\n", *argv);
diff --git a/man/man8/ip-xfrm.8 b/man/man8/ip-xfrm.8
index 003f6c3d1c28..bf725cabb82d 100644
--- a/man/man8/ip-xfrm.8
+++ b/man/man8/ip-xfrm.8
@@ -298,6 +298,18 @@ ip-xfrm \- transform configuration
 .RB "[ " hthresh6
 .IR LBITS " " RBITS " ]"
 
+.ti -8
+.B "ip xfrm policy setdefault"
+.IR DIR
+.IR ACTION " [ "
+.IR DIR
+.IR ACTION " ] [ "
+.IR DIR
+.IR ACTION " ]"
+
+.ti -8
+.B "ip xfrm policy getdefault"
+
 .ti -8
 .IR SELECTOR " :="
 .RB "[ " src
-- 
2.33.0

Re: [PATCH ipsec v3 0/2] xfrm: fix uapi for the default policy

[Antony Antony]

Hi Nicolas,

On Tue, Sep 14, 2021 at 16:46:32 +0200, Nicolas Dichtel wrote:
> This feature has just been merged after the last release, thus it's still
> time to fix the uapi.
> As stated in the thread, the uapi is based on some magic values (from the
> userland POV).

I like your proposal to make uapi 3 different variables, instead of flags.

This fix leave kernel internal representation as a flags in
struct netns_xfrm
	u8 policy_default;

I have a concern. If your patch is applied, the uapi and xfrm internal representations would be inconsistant. I think they should be the same in this case.
It would easier to follow the code path.
On the other hand we should apply this uapi change ASAP, in 5.15 release cycle, to avoid ABI change.

Could you also change xfrm policy_default to three variables?

> Here is a proposal to simplify this uapi and make it clear how to use it.
> The other problem was the notification: changing the default policy may
> radically change the packets flows.
> 
> v2 -> v3: rebase on top of ipsec tree
> 
> v1 -> v2: fix warnings reported by the kernel test robot
> 

Re: [PATCH ipsec v3 0/2] xfrm: fix uapi for the default policy

[Nicolas Dichtel]

Le 15/09/2021 à 11:19, Antony Antony a écrit :
> Hi Nicolas,
> 
> On Tue, Sep 14, 2021 at 16:46:32 +0200, Nicolas Dichtel wrote:
>> This feature has just been merged after the last release, thus it's still
>> time to fix the uapi.
>> As stated in the thread, the uapi is based on some magic values (from the
>> userland POV).
> 
> I like your proposal to make uapi 3 different variables, instead of flags.
> 
> This fix leave kernel internal representation as a flags in
> struct netns_xfrm
> 	u8 policy_default;
> 
> I have a concern. If your patch is applied, the uapi and xfrm internal representations would be inconsistant. I think they should be the same in this case.
I agree.

> It would easier to follow the code path.
> On the other hand we should apply this uapi change ASAP, in 5.15 release cycle, to avoid ABI change.
I also agree.

> 
> Could you also change xfrm policy_default to three variables?
Yes, I propose to send a follow up on ipsec-next once this series is applied.
The internal representation could be changed later, I prefer to keep this change
minimal for the ipsec tree.


Thank you,
Nicolas

Re: [PATCH ipsec v3 0/2] xfrm: fix uapi for the default policy

[Steffen Klassert]

On Tue, Sep 14, 2021 at 04:46:32PM +0200, Nicolas Dichtel wrote:
> This feature has just been merged after the last release, thus it's still
> time to fix the uapi.
> As stated in the thread, the uapi is based on some magic values (from the
> userland POV).
> Here is a proposal to simplify this uapi and make it clear how to use it.
> The other problem was the notification: changing the default policy may
> radically change the packets flows.
> 
> v2 -> v3: rebase on top of ipsec tree
> 
> v1 -> v2: fix warnings reported by the kernel test robot
> 
> Nicolas Dichtel (2):
>   xfrm: make user policy API complete
>   xfrm: notify default policy on update

Applied, thanks a lot Nicolas!

Re: [PATCH ipsec v3 0/2] xfrm: fix uapi for the default policy

[Nicolas Dichtel]

Le 17/09/2021 à 09:06, Steffen Klassert a écrit :
> On Tue, Sep 14, 2021 at 04:46:32PM +0200, Nicolas Dichtel wrote:
>> This feature has just been merged after the last release, thus it's still
>> time to fix the uapi.
>> As stated in the thread, the uapi is based on some magic values (from the
>> userland POV).
>> Here is a proposal to simplify this uapi and make it clear how to use it.
>> The other problem was the notification: changing the default policy may
>> radically change the packets flows.
>>
>> v2 -> v3: rebase on top of ipsec tree
>>
>> v1 -> v2: fix warnings reported by the kernel test robot
>>
>> Nicolas Dichtel (2):
>>   xfrm: make user policy API complete
>>   xfrm: notify default policy on update
> 
> Applied, thanks a lot Nicolas!
> 
Thanks Steffen. I will write the follow up patch once the ipsec tree is merged
into ipsec-next.


Regards,
Nicolas

[RFC PATCH iproute2] xfrm: enable to manage default policies

[Nicolas Dichtel]

Two new commands to manage default policies:
 - ip xfrm policy setdefault
 - ip xfrm policy getdefault

And the corresponding part in 'ip xfrm monitor'.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/uapi/linux/xfrm.h |   9 ++-
 ip/xfrm.h                 |   1 +
 ip/xfrm_monitor.c         |   3 +
 ip/xfrm_policy.c          | 121 ++++++++++++++++++++++++++++++++++++++
 man/man8/ip-xfrm.8        |  12 ++++
 5 files changed, 143 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index ecd06396eb16..2a8135e31f9d 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -514,9 +514,12 @@ struct xfrm_user_offload {
 #define XFRM_OFFLOAD_INBOUND	2
 
 struct xfrm_userpolicy_default {
-#define XFRM_USERPOLICY_DIRMASK_MAX	(sizeof(__u8) * 8)
-	__u8				dirmask;
-	__u8				action;
+#define XFRM_USERPOLICY_UNSPEC	0
+#define XFRM_USERPOLICY_BLOCK	1
+#define XFRM_USERPOLICY_ACCEPT	2
+	__u8				in;
+	__u8				fwd;
+	__u8				out;
 };
 
 /* backwards compatibility for userspace */
diff --git a/ip/xfrm.h b/ip/xfrm.h
index 9ba5ca61d5e4..17dcf3fea83f 100644
--- a/ip/xfrm.h
+++ b/ip/xfrm.h
@@ -132,6 +132,7 @@ void xfrm_state_info_print(struct xfrm_usersa_info *xsinfo,
 void xfrm_policy_info_print(struct xfrm_userpolicy_info *xpinfo,
 			    struct rtattr *tb[], FILE *fp, const char *prefix,
 			    const char *title);
+int xfrm_policy_default_print(struct nlmsghdr *n, FILE *fp);
 int xfrm_id_parse(xfrm_address_t *saddr, struct xfrm_id *id, __u16 *family,
 		  int loose, int *argcp, char ***argvp);
 int xfrm_mode_parse(__u8 *mode, int *argcp, char ***argvp);
diff --git a/ip/xfrm_monitor.c b/ip/xfrm_monitor.c
index e34b5fbda130..f67424c5be06 100644
--- a/ip/xfrm_monitor.c
+++ b/ip/xfrm_monitor.c
@@ -323,6 +323,9 @@ static int xfrm_accept_msg(struct rtnl_ctrl_data *ctrl,
 	case XFRM_MSG_MAPPING:
 		xfrm_mapping_print(n, arg);
 		return 0;
+	case XFRM_MSG_GETDEFAULT:
+		xfrm_policy_default_print(n, arg);
+		return 0;
 	default:
 		break;
 	}
diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index 7cc00e7c2f5b..744f331ff564 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -66,6 +66,8 @@ static void usage(void)
 		"Usage: ip xfrm policy flush [ ptype PTYPE ]\n"
 		"Usage: ip xfrm policy count\n"
 		"Usage: ip xfrm policy set [ hthresh4 LBITS RBITS ] [ hthresh6 LBITS RBITS ]\n"
+		"Usage: ip xfrm policy setdefault DIR ACTION [ DIR ACTION ] [ DIR ACTION ]\n"
+		"Usage: ip xfrm policy getdefault\n"
 		"SELECTOR := [ src ADDR[/PLEN] ] [ dst ADDR[/PLEN] ] [ dev DEV ] [ UPSPEC ]\n"
 		"UPSPEC := proto { { tcp | udp | sctp | dccp } [ sport PORT ] [ dport PORT ] |\n"
 		"                  { icmp | ipv6-icmp | mobility-header } [ type NUMBER ] [ code NUMBER ] |\n"
@@ -1124,6 +1126,121 @@ static int xfrm_spd_getinfo(int argc, char **argv)
 	return 0;
 }
 
+static int xfrm_spd_setdefault(int argc, char **argv)
+{
+	struct rtnl_handle rth;
+	struct {
+		struct nlmsghdr			n;
+		struct xfrm_userpolicy_default  up;
+	} req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_default)),
+		.n.nlmsg_flags = NLM_F_REQUEST,
+		.n.nlmsg_type = XFRM_MSG_SETDEFAULT,
+	};
+
+	while (argc > 0) {
+		if (strcmp(*argv, "in") == 0) {
+			if (req.up.in)
+				duparg("in", *argv);
+
+			NEXT_ARG();
+			if (strcmp(*argv, "block") == 0)
+				req.up.in = XFRM_USERPOLICY_BLOCK;
+			else if (strcmp(*argv, "accept") == 0)
+				req.up.in = XFRM_USERPOLICY_ACCEPT;
+			else
+				invarg("in policy value is invalid", *argv);
+		} else if (strcmp(*argv, "fwd") == 0) {
+			if (req.up.fwd)
+				duparg("fwd", *argv);
+
+			NEXT_ARG();
+			if (strcmp(*argv, "block") == 0)
+				req.up.fwd = XFRM_USERPOLICY_BLOCK;
+			else if (strcmp(*argv, "accept") == 0)
+				req.up.fwd = XFRM_USERPOLICY_ACCEPT;
+			else
+				invarg("fwd policy value is invalid", *argv);
+		} else if (strcmp(*argv, "out") == 0) {
+			if (req.up.out)
+				duparg("out", *argv);
+
+			NEXT_ARG();
+			if (strcmp(*argv, "block") == 0)
+				req.up.out = XFRM_USERPOLICY_BLOCK;
+			else if (strcmp(*argv, "accept") == 0)
+				req.up.out = XFRM_USERPOLICY_ACCEPT;
+			else
+				invarg("out policy value is invalid", *argv);
+		} else {
+			invarg("unknown direction", *argv);
+		}
+
+		argc--; argv++;
+	}
+
+	if (rtnl_open_byproto(&rth, 0, NETLINK_XFRM) < 0)
+		exit(1);
+
+	if (rtnl_talk(&rth, &req.n, NULL) < 0)
+		exit(2);
+
+	rtnl_close(&rth);
+
+	return 0;
+}
+
+int xfrm_policy_default_print(struct nlmsghdr *n, FILE *fp)
+{
+	struct xfrm_userpolicy_default *up = NLMSG_DATA(n);
+	int len = n->nlmsg_len - NLMSG_SPACE(sizeof(*up));
+
+	if (len < 0) {
+		fprintf(stderr,
+			"BUG: short nlmsg len %u (expect %lu) for XFRM_MSG_GETDEFAULT\n",
+			n->nlmsg_len, NLMSG_SPACE(sizeof(*up)));
+		return -1;
+	}
+
+	fprintf(fp, "Default policies:\n");
+	fprintf(fp, " in:  %s\n",
+		up->in == XFRM_USERPOLICY_BLOCK ? "block" : "accept");
+	fprintf(fp, " fwd: %s\n",
+		up->fwd == XFRM_USERPOLICY_BLOCK ? "block" : "accept");
+	fprintf(fp, " out: %s\n",
+		up->out == XFRM_USERPOLICY_BLOCK ? "block" : "accept");
+	fflush(fp);
+
+	return 0;
+}
+
+static int xfrm_spd_getdefault(int argc, char **argv)
+{
+	struct rtnl_handle rth;
+	struct {
+		struct nlmsghdr			n;
+		struct xfrm_userpolicy_default  up;
+	} req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_default)),
+		.n.nlmsg_flags = NLM_F_REQUEST,
+		.n.nlmsg_type = XFRM_MSG_GETDEFAULT,
+	};
+	struct nlmsghdr *answer;
+
+	if (rtnl_open_byproto(&rth, 0, NETLINK_XFRM) < 0)
+		exit(1);
+
+	if (rtnl_talk(&rth, &req.n, &answer) < 0)
+		exit(2);
+
+	xfrm_policy_default_print(answer, (FILE *)stdout);
+
+	free(answer);
+	rtnl_close(&rth);
+
+	return 0;
+}
+
 static int xfrm_policy_flush(int argc, char **argv)
 {
 	struct rtnl_handle rth;
@@ -1197,6 +1314,10 @@ int do_xfrm_policy(int argc, char **argv)
 		return xfrm_spd_getinfo(argc, argv);
 	if (matches(*argv, "set") == 0)
 		return xfrm_spd_setinfo(argc-1, argv+1);
+	if (matches(*argv, "setdefault") == 0)
+		return xfrm_spd_setdefault(argc-1, argv+1);
+	if (matches(*argv, "getdefault") == 0)
+		return xfrm_spd_getdefault(argc-1, argv+1);
 	if (matches(*argv, "help") == 0)
 		usage();
 	fprintf(stderr, "Command \"%s\" is unknown, try \"ip xfrm policy help\".\n", *argv);
diff --git a/man/man8/ip-xfrm.8 b/man/man8/ip-xfrm.8
index 003f6c3d1c28..bf725cabb82d 100644
--- a/man/man8/ip-xfrm.8
+++ b/man/man8/ip-xfrm.8
@@ -298,6 +298,18 @@ ip-xfrm \- transform configuration
 .RB "[ " hthresh6
 .IR LBITS " " RBITS " ]"
 
+.ti -8
+.B "ip xfrm policy setdefault"
+.IR DIR
+.IR ACTION " [ "
+.IR DIR
+.IR ACTION " ] [ "
+.IR DIR
+.IR ACTION " ]"
+
+.ti -8
+.B "ip xfrm policy getdefault"
+
 .ti -8
 .IR SELECTOR " :="
 .RB "[ " src
-- 
2.33.0

Re: [PATCH v2 ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[Dmitry V. Levin]

Hi,

On Sun, Jul 18, 2021 at 09:11:06AM +0200, Antony Antony wrote:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> 
> As the default we assume the traffic to pass, if we have no
> matching IPsec policy. With this patch, we have a possibility to
> change this default from allow to block. It can be configured
> via netlink. Each direction (input/output/forward) can be
> configured separately. With the default to block configuered,
> we need allow policies for all packet flows we accept.
> We do not use default policy lookup for the loopback device.
> 
> v1->v2
>  - fix compiling when XFRM is disabled
>  - Reported-by: kernel test robot <lkp@intel.com>
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> Co-developed-by: Christian Langrock <christian.langrock@secunet.com>
> Signed-off-by: Christian Langrock <christian.langrock@secunet.com>
> Co-developed-by: Antony Antony <antony.antony@secunet.com>
> Signed-off-by: Antony Antony <antony.antony@secunet.com>
[...]

The following part of this patch is ABI break:

> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
> index ffc6a5391bb7..6e8095106192 100644
> --- a/include/uapi/linux/xfrm.h
> +++ b/include/uapi/linux/xfrm.h
> @@ -213,6 +213,11 @@ enum {
>  	XFRM_MSG_GETSPDINFO,
>  #define XFRM_MSG_GETSPDINFO XFRM_MSG_GETSPDINFO
> 
> +	XFRM_MSG_SETDEFAULT,
> +#define XFRM_MSG_SETDEFAULT XFRM_MSG_SETDEFAULT
> +	XFRM_MSG_GETDEFAULT,
> +#define XFRM_MSG_GETDEFAULT XFRM_MSG_GETDEFAULT
> +
>  	XFRM_MSG_MAPPING,
>  #define XFRM_MSG_MAPPING XFRM_MSG_MAPPING
>  	__XFRM_MSG_MAX

After this change, strace no longer builds with the following diagnostics:

../../../src/xlat/nl_xfrm_types.h:162:1: error: static assertion failed: "XFRM_MSG_MAPPING != 0x26"
  162 | static_assert((XFRM_MSG_MAPPING) == (0x26), "XFRM_MSG_MAPPING != 0x26");


-- 
ldv

Re: [PATCH v2 ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[Steffen Klassert]

On Wed, Sep 01, 2021 at 06:14:02PM +0300, Dmitry V. Levin wrote:
> 
> The following part of this patch is ABI break:
> 
> > diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
> > index ffc6a5391bb7..6e8095106192 100644
> > --- a/include/uapi/linux/xfrm.h
> > +++ b/include/uapi/linux/xfrm.h
> > @@ -213,6 +213,11 @@ enum {
> >  	XFRM_MSG_GETSPDINFO,
> >  #define XFRM_MSG_GETSPDINFO XFRM_MSG_GETSPDINFO
> > 
> > +	XFRM_MSG_SETDEFAULT,
> > +#define XFRM_MSG_SETDEFAULT XFRM_MSG_SETDEFAULT
> > +	XFRM_MSG_GETDEFAULT,
> > +#define XFRM_MSG_GETDEFAULT XFRM_MSG_GETDEFAULT
> > +
> >  	XFRM_MSG_MAPPING,
> >  #define XFRM_MSG_MAPPING XFRM_MSG_MAPPING
> >  	__XFRM_MSG_MAX
> 
> After this change, strace no longer builds with the following diagnostics:
> 
> ../../../src/xlat/nl_xfrm_types.h:162:1: error: static assertion failed: "XFRM_MSG_MAPPING != 0x26"
>   162 | static_assert((XFRM_MSG_MAPPING) == (0x26), "XFRM_MSG_MAPPING != 0x26");

Thanks for the report! In the meantime there is a fix proposed:

https://www.spinics.net/lists/netdev/msg764744.html

Re: [PATCH v2 ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[Paul Cercueil]

Hi,

I think this patch was merged in v5.15-rc1, right?

"strace" fails to build because of this:

In file included from print_fields.h:12,
                 from defs.h:1869,
                 from netlink.c:10:
static_assert.h:20:25: error: static assertion failed: 
"XFRM_MSG_MAPPING != 0x26"
   20 | # define static_assert _Static_assert
      | ^~~~~~~~~~~~~~
xlat/nl_xfrm_types.h:162:1: note: in expansion of macro 'static_assert'
  162 | static_assert((XFRM_MSG_MAPPING) == (0x26), "XFRM_MSG_MAPPING 
!= 0x26");
      | ^~~~~~~~~~~~~
make[5]: *** [Makefile:5834: libstrace_a-netlink.o] Error 1

Cheers,
-Paul


Le dim., juil. 18 2021 at 09:11:06 +0200, Antony Antony 
<antony.antony@secunet.com> a écrit :
> From: Steffen Klassert <steffen.klassert@secunet.com>
> 
> As the default we assume the traffic to pass, if we have no
> matching IPsec policy. With this patch, we have a possibility to
> change this default from allow to block. It can be configured
> via netlink. Each direction (input/output/forward) can be
> configured separately. With the default to block configuered,
> we need allow policies for all packet flows we accept.
> We do not use default policy lookup for the loopback device.
> 
> v1->v2
>  - fix compiling when XFRM is disabled
>  - Reported-by: kernel test robot <lkp@intel.com>
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> Co-developed-by: Christian Langrock <christian.langrock@secunet.com>
> Signed-off-by: Christian Langrock <christian.langrock@secunet.com>
> Co-developed-by: Antony Antony <antony.antony@secunet.com>
> Signed-off-by: Antony Antony <antony.antony@secunet.com>
> ---
>  include/net/netns/xfrm.h  |  7 ++++++
>  include/net/xfrm.h        | 36 ++++++++++++++++++++++-----
>  include/uapi/linux/xfrm.h | 10 ++++++++
>  net/xfrm/xfrm_policy.c    | 16 ++++++++++++
>  net/xfrm/xfrm_user.c      | 52 
> +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 115 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
> index e946366e8ba5..88c647302977 100644
> --- a/include/net/netns/xfrm.h
> +++ b/include/net/netns/xfrm.h
> @@ -65,6 +65,13 @@ struct netns_xfrm {
>  	u32			sysctl_aevent_rseqth;
>  	int			sysctl_larval_drop;
>  	u32			sysctl_acq_expires;
> +
> +	u8			policy_default;
> +#define XFRM_POL_DEFAULT_IN	1
> +#define XFRM_POL_DEFAULT_OUT	2
> +#define XFRM_POL_DEFAULT_FWD	4
> +#define XFRM_POL_DEFAULT_MASK	7
> +
>  #ifdef CONFIG_SYSCTL
>  	struct ctl_table_header	*sysctl_hdr;
>  #endif
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index cbff7c2a9724..2308210793a0 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1075,6 +1075,22 @@ xfrm_state_addr_cmp(const struct xfrm_tmpl 
> *tmpl, const struct xfrm_state *x, un
>  }
> 
>  #ifdef CONFIG_XFRM
> +static inline bool
> +xfrm_default_allow(struct net *net, int dir)
> +{
> +	u8 def = net->xfrm.policy_default;
> +
> +	switch (dir) {
> +	case XFRM_POLICY_IN:
> +		return def & XFRM_POL_DEFAULT_IN ? false : true;
> +	case XFRM_POLICY_OUT:
> +		return def & XFRM_POL_DEFAULT_OUT ? false : true;
> +	case XFRM_POLICY_FWD:
> +		return def & XFRM_POL_DEFAULT_FWD ? false : true;
> +	}
> +	return false;
> +}
> +
>  int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb,
>  			unsigned short family);
> 
> @@ -1088,9 +1104,13 @@ static inline int __xfrm_policy_check2(struct 
> sock *sk, int dir,
>  	if (sk && sk->sk_policy[XFRM_POLICY_IN])
>  		return __xfrm_policy_check(sk, ndir, skb, family);
> 
> -	return	(!net->xfrm.policy_count[dir] && !secpath_exists(skb)) ||
> -		(skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) ||
> -		__xfrm_policy_check(sk, ndir, skb, family);
> +	if (xfrm_default_allow(net, dir))
> +		return (!net->xfrm.policy_count[dir] && !secpath_exists(skb)) ||
> +		       (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) ||
> +		       __xfrm_policy_check(sk, ndir, skb, family);
> +	else
> +		return (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) ||
> +		       __xfrm_policy_check(sk, ndir, skb, family);
>  }
> 
>  static inline int xfrm_policy_check(struct sock *sk, int dir, struct 
> sk_buff *skb, unsigned short family)
> @@ -1142,9 +1162,13 @@ static inline int xfrm_route_forward(struct 
> sk_buff *skb, unsigned short family)
>  {
>  	struct net *net = dev_net(skb->dev);
> 
> -	return	!net->xfrm.policy_count[XFRM_POLICY_OUT] ||
> -		(skb_dst(skb)->flags & DST_NOXFRM) ||
> -		__xfrm_route_forward(skb, family);
> +	if (xfrm_default_allow(net, XFRM_POLICY_FWD))
> +		return !net->xfrm.policy_count[XFRM_POLICY_OUT] ||
> +			(skb_dst(skb)->flags & DST_NOXFRM) ||
> +			__xfrm_route_forward(skb, family);
> +	else
> +		return (skb_dst(skb)->flags & DST_NOXFRM) ||
> +			__xfrm_route_forward(skb, family);
>  }
> 
>  static inline int xfrm4_route_forward(struct sk_buff *skb)
> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
> index ffc6a5391bb7..6e8095106192 100644
> --- a/include/uapi/linux/xfrm.h
> +++ b/include/uapi/linux/xfrm.h
> @@ -213,6 +213,11 @@ enum {
>  	XFRM_MSG_GETSPDINFO,
>  #define XFRM_MSG_GETSPDINFO XFRM_MSG_GETSPDINFO
> 
> +	XFRM_MSG_SETDEFAULT,
> +#define XFRM_MSG_SETDEFAULT XFRM_MSG_SETDEFAULT
> +	XFRM_MSG_GETDEFAULT,
> +#define XFRM_MSG_GETDEFAULT XFRM_MSG_GETDEFAULT
> +
>  	XFRM_MSG_MAPPING,
>  #define XFRM_MSG_MAPPING XFRM_MSG_MAPPING
>  	__XFRM_MSG_MAX
> @@ -508,6 +513,11 @@ struct xfrm_user_offload {
>  #define XFRM_OFFLOAD_IPV6	1
>  #define XFRM_OFFLOAD_INBOUND	2
> 
> +struct xfrm_userpolicy_default {
> +	__u8				dirmask;
> +	__u8				action;
> +};
> +
>  #ifndef __KERNEL__
>  /* backwards compatibility for userspace */
>  #define XFRMGRP_ACQUIRE		1
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 827d84255021..d5cb082e11fc 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -3165,6 +3165,11 @@ struct dst_entry *xfrm_lookup_with_ifid(struct 
> net *net,
>  	return dst;
> 
>  nopol:
> +	if (!(dst_orig->dev->flags & IFF_LOOPBACK) &&
> +	    !xfrm_default_allow(net, dir)) {
> +		err = -EPERM;
> +		goto error;
> +	}
>  	if (!(flags & XFRM_LOOKUP_ICMP)) {
>  		dst = dst_orig;
>  		goto ok;
> @@ -3553,6 +3558,11 @@ int __xfrm_policy_check(struct sock *sk, int 
> dir, struct sk_buff *skb,
>  	}
> 
>  	if (!pol) {
> +		if (!xfrm_default_allow(net, dir)) {
> +			XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOPOLS);
> +			return 0;
> +		}
> +
>  		if (sp && secpath_has_nontransport(sp, 0, &xerr_idx)) {
>  			xfrm_secpath_reject(xerr_idx, skb, &fl);
>  			XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOPOLS);
> @@ -3607,6 +3617,12 @@ int __xfrm_policy_check(struct sock *sk, int 
> dir, struct sk_buff *skb,
>  				tpp[ti++] = &pols[pi]->xfrm_vec[i];
>  		}
>  		xfrm_nr = ti;
> +
> +		if (!xfrm_default_allow(net, dir) && !xfrm_nr) {
> +			XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOSTATES);
> +			goto reject;
> +		}
> +
>  		if (npols > 1) {
>  			xfrm_tmpl_sort(stp, tpp, xfrm_nr, family);
>  			tpp = stp;
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index b47d613409b7..4eafd1130c3e 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -1961,6 +1961,54 @@ static struct sk_buff 
> *xfrm_policy_netlink(struct sk_buff *in_skb,
>  	return skb;
>  }
> 
> +static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr 
> *nlh,
> +			    struct nlattr **attrs)
> +{
> +	struct net *net = sock_net(skb->sk);
> +	struct xfrm_userpolicy_default *up = nlmsg_data(nlh);
> +	u8 dirmask = (1 << up->dirmask) & XFRM_POL_DEFAULT_MASK;
> +	u8 old_default = net->xfrm.policy_default;
> +
> +	net->xfrm.policy_default = (old_default & (0xff ^ dirmask))
> +				    | (up->action << up->dirmask);
> +
> +	rt_genid_bump_all(net);
> +
> +	return 0;
> +}
> +
> +static int xfrm_get_default(struct sk_buff *skb, struct nlmsghdr 
> *nlh,
> +			    struct nlattr **attrs)
> +{
> +	struct sk_buff *r_skb;
> +	struct nlmsghdr *r_nlh;
> +	struct net *net = sock_net(skb->sk);
> +	struct xfrm_userpolicy_default *r_up, *up;
> +	int len = NLMSG_ALIGN(sizeof(struct xfrm_userpolicy_default));
> +	u32 portid = NETLINK_CB(skb).portid;
> +	u32 seq = nlh->nlmsg_seq;
> +
> +	up = nlmsg_data(nlh);
> +
> +	r_skb = nlmsg_new(len, GFP_ATOMIC);
> +	if (!r_skb)
> +		return -ENOMEM;
> +
> +	r_nlh = nlmsg_put(r_skb, portid, seq, XFRM_MSG_GETDEFAULT, 
> sizeof(*r_up), 0);
> +	if (!r_nlh) {
> +		kfree_skb(r_skb);
> +		return -EMSGSIZE;
> +	}
> +
> +	r_up = nlmsg_data(r_nlh);
> +
> +	r_up->action = ((net->xfrm.policy_default & (1 << up->dirmask)) >> 
> up->dirmask);
> +	r_up->dirmask = up->dirmask;
> +	nlmsg_end(r_skb, r_nlh);
> +
> +	return nlmsg_unicast(net->xfrm.nlsk, r_skb, portid);
> +}
> +
>  static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
>  		struct nlattr **attrs)
>  {
> @@ -2664,6 +2712,8 @@ const int xfrm_msg_min[XFRM_NR_MSGTYPES] = {
>  	[XFRM_MSG_GETSADINFO  - XFRM_MSG_BASE] = sizeof(u32),
>  	[XFRM_MSG_NEWSPDINFO  - XFRM_MSG_BASE] = sizeof(u32),
>  	[XFRM_MSG_GETSPDINFO  - XFRM_MSG_BASE] = sizeof(u32),
> +	[XFRM_MSG_SETDEFAULT  - XFRM_MSG_BASE] = 
> XMSGSIZE(xfrm_userpolicy_default),
> +	[XFRM_MSG_GETDEFAULT  - XFRM_MSG_BASE] = 
> XMSGSIZE(xfrm_userpolicy_default),
>  };
>  EXPORT_SYMBOL_GPL(xfrm_msg_min);
> 
> @@ -2743,6 +2793,8 @@ static const struct xfrm_link {
>  						   .nla_pol = xfrma_spd_policy,
>  						   .nla_max = XFRMA_SPD_MAX },
>  	[XFRM_MSG_GETSPDINFO  - XFRM_MSG_BASE] = { .doit = xfrm_get_spdinfo 
>   },
> +	[XFRM_MSG_SETDEFAULT  - XFRM_MSG_BASE] = { .doit = xfrm_set_default 
>   },
> +	[XFRM_MSG_GETDEFAULT  - XFRM_MSG_BASE] = { .doit = xfrm_get_default 
>   },
>  };
> 
>  static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr 
> *nlh,
> --
> 2.20.1

Re: [PATCH v2 ipsec-next] xfrm: Add possibility to set the default to block if we have no policy

[Steffen Klassert]

On Sun, Sep 19, 2021 at 11:40:37PM +0100, Paul Cercueil wrote:
> Hi,
> 
> I think this patch was merged in v5.15-rc1, right?
> 
> "strace" fails to build because of this:
> 
> In file included from print_fields.h:12,
>                 from defs.h:1869,
>                 from netlink.c:10:
> static_assert.h:20:25: error: static assertion failed: "XFRM_MSG_MAPPING !=
> 0x26"
>   20 | # define static_assert _Static_assert
>      | ^~~~~~~~~~~~~~
> xlat/nl_xfrm_types.h:162:1: note: in expansion of macro 'static_assert'
>  162 | static_assert((XFRM_MSG_MAPPING) == (0x26), "XFRM_MSG_MAPPING !=
> 0x26");
>      | ^~~~~~~~~~~~~
> make[5]: *** [Makefile:5834: libstrace_a-netlink.o] Error 1

Thanks for the report!

This is already fixed in the ipsec tree with:

commit 844f7eaaed9267ae17d33778efe65548cc940205
Author: Eugene Syromiatnikov <esyr@redhat.com>
Date:   Sun Sep 12 14:22:34 2021 +0200

    include/uapi/linux/xfrm.h: Fix XFRM_MSG_MAPPING ABI breakage

    Commit 2d151d39073a ("xfrm: Add possibility to set the default to block
    if we have no policy") broke ABI by changing the value of the XFRM_MSG_MAPPING
    enum item, thus also evading the build-time check
    in security/selinux/nlmsgtab.c:selinux_nlmsg_lookup for presence of proper
    security permission checks in nlmsg_xfrm_perms.  Fix it by placing
    XFRM_MSG_SETDEFAULT/XFRM_MSG_GETDEFAULT to the end of the enum, right before
    __XFRM_MSG_MAX, and updating the nlmsg_xfrm_perms accordingly.

    Fixes: 2d151d39073a ("xfrm: Add possibility to set the default to block if we have no policy")
    References: https://lore.kernel.org/netdev/20210901151402.GA2557@altlinux.org/
    Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
    Acked-by: Antony Antony <antony.antony@secunet.com>
    Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
    Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

It will likely go upstream this week.

Thanks!