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 [off-list ref]
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Co-developed-by: Christian Langrock <redacted>
Signed-off-by: Christian Langrock <redacted>
Co-developed-by: Antony Antony <redacted>
Signed-off-by: Antony Antony <redacted>
---
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(-)
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 [off-list ref]
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Co-developed-by: Christian Langrock <redacted>
Signed-off-by: Christian Langrock <redacted>
Co-developed-by: Antony Antony <redacted>
Signed-off-by: Antony Antony <redacted>
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.
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
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.
From: Dmitry V. Levin <hidden> Date: 2021-09-01 15:14:06
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 [off-list ref]
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Co-developed-by: Christian Langrock <redacted>
Signed-off-by: Christian Langrock <redacted>
Co-developed-by: Antony Antony <redacted>
Signed-off-by: Antony Antony <redacted>
[...]
The following part of this patch is ABI break:
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
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");
@@ -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)return0;}+staticintxfrm_spd_setdefault(intargc,char**argv)+{+structrtnl_handlerth;+struct{+structnlmsghdrn;+structxfrm_userpolicy_defaultup;+}req={+.n.nlmsg_len=NLMSG_LENGTH(sizeof(structxfrm_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;+elseif(strcmp(*argv,"accept")==0)+req.up.in=XFRM_USERPOLICY_ACCEPT;+else+invarg("in policy value is invalid",*argv);+}elseif(strcmp(*argv,"fwd")==0){+if(req.up.fwd)+duparg("fwd",*argv);++NEXT_ARG();+if(strcmp(*argv,"block")==0)+req.up.fwd=XFRM_USERPOLICY_BLOCK;+elseif(strcmp(*argv,"accept")==0)+req.up.fwd=XFRM_USERPOLICY_ACCEPT;+else+invarg("fwd policy value is invalid",*argv);+}elseif(strcmp(*argv,"out")==0){+if(req.up.out)+duparg("out",*argv);++NEXT_ARG();+if(strcmp(*argv,"block")==0)+req.up.out=XFRM_USERPOLICY_BLOCK;+elseif(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);++return0;+}++intxfrm_policy_default_print(structnlmsghdr*n,FILE*fp)+{+structxfrm_userpolicy_default*up=NLMSG_DATA(n);+intlen=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);++return0;+}++staticintxfrm_spd_getdefault(intargc,char**argv)+{+structrtnl_handlerth;+struct{+structnlmsghdrn;+structxfrm_userpolicy_defaultup;+}req={+.n.nlmsg_len=NLMSG_LENGTH(sizeof(structxfrm_userpolicy_default)),+.n.nlmsg_flags=NLM_F_REQUEST,+.n.nlmsg_type=XFRM_MSG_GETDEFAULT,+};+structnlmsghdr*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);++return0;+}+staticintxfrm_policy_flush(intargc,char**argv){structrtnl_handlerth;
@@ -1197,6 +1314,10 @@ int do_xfrm_policy(int argc, char **argv)returnxfrm_spd_getinfo(argc,argv);if(matches(*argv,"set")==0)returnxfrm_spd_setinfo(argc-1,argv+1);+if(matches(*argv,"setdefault")==0)+returnxfrm_spd_setdefault(argc-1,argv+1);+if(matches(*argv,"getdefault")==0)+returnxfrm_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);
From: Nicolas Dichtel <hidden> Date: 2021-09-07 19:42:31
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 <redacted>
---
net/xfrm/xfrm_user.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
From: Nicolas Dichtel <hidden> Date: 2021-09-07 19:42:32
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 <redacted>
---
include/uapi/linux/xfrm.h | 9 ++++++---
net/xfrm/xfrm_user.c | 27 ++++++++++++++++++---------
2 files changed, 24 insertions(+), 12 deletions(-)
From: Nicolas Dichtel <hidden> Date: 2021-09-07 19:42:34
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
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
From: Nicolas Dichtel <hidden> Date: 2021-09-08 07:24:08
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
@@ -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)return0;}+staticintxfrm_spd_setdefault(intargc,char**argv)+{+structrtnl_handlerth;+struct{+structnlmsghdrn;+structxfrm_userpolicy_defaultup;+}req={+.n.nlmsg_len=NLMSG_LENGTH(sizeof(structxfrm_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;+elseif(strcmp(*argv,"accept")==0)+req.up.in=XFRM_USERPOLICY_ACCEPT;+else+invarg("in policy value is invalid",*argv);+}elseif(strcmp(*argv,"fwd")==0){+if(req.up.fwd)+duparg("fwd",*argv);++NEXT_ARG();+if(strcmp(*argv,"block")==0)+req.up.fwd=XFRM_USERPOLICY_BLOCK;+elseif(strcmp(*argv,"accept")==0)+req.up.fwd=XFRM_USERPOLICY_ACCEPT;+else+invarg("fwd policy value is invalid",*argv);+}elseif(strcmp(*argv,"out")==0){+if(req.up.out)+duparg("out",*argv);++NEXT_ARG();+if(strcmp(*argv,"block")==0)+req.up.out=XFRM_USERPOLICY_BLOCK;+elseif(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);++return0;+}++intxfrm_policy_default_print(structnlmsghdr*n,FILE*fp)+{+structxfrm_userpolicy_default*up=NLMSG_DATA(n);+intlen=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);++return0;+}++staticintxfrm_spd_getdefault(intargc,char**argv)+{+structrtnl_handlerth;+struct{+structnlmsghdrn;+structxfrm_userpolicy_defaultup;+}req={+.n.nlmsg_len=NLMSG_LENGTH(sizeof(structxfrm_userpolicy_default)),+.n.nlmsg_flags=NLM_F_REQUEST,+.n.nlmsg_type=XFRM_MSG_GETDEFAULT,+};+structnlmsghdr*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);++return0;+}+staticintxfrm_policy_flush(intargc,char**argv){structrtnl_handlerth;
@@ -1197,6 +1314,10 @@ int do_xfrm_policy(int argc, char **argv)returnxfrm_spd_getinfo(argc,argv);if(matches(*argv,"set")==0)returnxfrm_spd_setinfo(argc-1,argv+1);+if(matches(*argv,"setdefault")==0)+returnxfrm_spd_setdefault(argc-1,argv+1);+if(matches(*argv,"getdefault")==0)+returnxfrm_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);
From: Nicolas Dichtel <hidden> Date: 2021-09-08 07:24:10
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 <redacted>
---
net/xfrm/xfrm_user.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
From: Nicolas Dichtel <hidden> Date: 2021-09-08 07:24:11
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 <redacted>
---
include/uapi/linux/xfrm.h | 9 ++++++---
net/xfrm/xfrm_user.c | 31 +++++++++++++++++++------------
2 files changed, 25 insertions(+), 15 deletions(-)
From: Nicolas Dichtel <hidden> Date: 2021-09-14 14:47:24
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 <redacted>
---
include/uapi/linux/xfrm.h | 9 ++++++---
net/xfrm/xfrm_user.c | 36 +++++++++++++++++++-----------------
2 files changed, 25 insertions(+), 20 deletions(-)
From: Nicolas Dichtel <hidden> Date: 2021-09-14 14:47:27
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 <redacted>
---
net/xfrm/xfrm_user.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
From: Nicolas Dichtel <hidden> Date: 2021-09-14 14:47:30
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
@@ -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)return0;}+staticintxfrm_spd_setdefault(intargc,char**argv)+{+structrtnl_handlerth;+struct{+structnlmsghdrn;+structxfrm_userpolicy_defaultup;+}req={+.n.nlmsg_len=NLMSG_LENGTH(sizeof(structxfrm_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;+elseif(strcmp(*argv,"accept")==0)+req.up.in=XFRM_USERPOLICY_ACCEPT;+else+invarg("in policy value is invalid",*argv);+}elseif(strcmp(*argv,"fwd")==0){+if(req.up.fwd)+duparg("fwd",*argv);++NEXT_ARG();+if(strcmp(*argv,"block")==0)+req.up.fwd=XFRM_USERPOLICY_BLOCK;+elseif(strcmp(*argv,"accept")==0)+req.up.fwd=XFRM_USERPOLICY_ACCEPT;+else+invarg("fwd policy value is invalid",*argv);+}elseif(strcmp(*argv,"out")==0){+if(req.up.out)+duparg("out",*argv);++NEXT_ARG();+if(strcmp(*argv,"block")==0)+req.up.out=XFRM_USERPOLICY_BLOCK;+elseif(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);++return0;+}++intxfrm_policy_default_print(structnlmsghdr*n,FILE*fp)+{+structxfrm_userpolicy_default*up=NLMSG_DATA(n);+intlen=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);++return0;+}++staticintxfrm_spd_getdefault(intargc,char**argv)+{+structrtnl_handlerth;+struct{+structnlmsghdrn;+structxfrm_userpolicy_defaultup;+}req={+.n.nlmsg_len=NLMSG_LENGTH(sizeof(structxfrm_userpolicy_default)),+.n.nlmsg_flags=NLM_F_REQUEST,+.n.nlmsg_type=XFRM_MSG_GETDEFAULT,+};+structnlmsghdr*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);++return0;+}+staticintxfrm_policy_flush(intargc,char**argv){structrtnl_handlerth;
@@ -1197,6 +1314,10 @@ int do_xfrm_policy(int argc, char **argv)returnxfrm_spd_getinfo(argc,argv);if(matches(*argv,"set")==0)returnxfrm_spd_setinfo(argc-1,argv+1);+if(matches(*argv,"setdefault")==0)+returnxfrm_spd_setdefault(argc-1,argv+1);+if(matches(*argv,"getdefault")==0)+returnxfrm_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);
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
From: Nicolas Dichtel <hidden> Date: 2021-09-15 09:55:59
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:
quoted
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
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
From: Nicolas Dichtel <hidden> Date: 2021-09-17 07:54:47
Le 17/09/2021 à 09:06, Steffen Klassert a écrit :
On Tue, Sep 14, 2021 at 04:46:32PM +0200, Nicolas Dichtel wrote:
quoted
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
From: Paul Cercueil <paul@crapouillou.net> Date: 2021-09-19 22:41:03
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
[off-list ref] a écrit :
quoted hunk
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 [off-list ref]
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Co-developed-by: Christian Langrock <redacted>
Signed-off-by: Christian Langrock <redacted>
Co-developed-by: Antony Antony <redacted>
Signed-off-by: Antony Antony <redacted>
---
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(-)
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 [off-list ref]
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 [off-list ref]
Acked-by: Antony Antony [off-list ref]
Acked-by: Nicolas Dichtel [off-list ref]
Signed-off-by: Steffen Klassert [off-list ref]
It will likely go upstream this week.
Thanks!