| Message ID | 055d987a-e530-c65c-82de-de761cc31878@oracle.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | ipsec: ipcomp alg problem on vti interface | expand |
On Wed, Nov 22, 2017 at 07:06:13PM +0300, Alexey Kodanev wrote: > Hi Steffen, > > LTP has vti test-cases which fail on ipcomp alg, e.g. > "tcp_ipsec_vti.sh -p comp -m tunnel -s 100" > > Basically, the setupconsists of the following commands: > > ip li add ltp_vti0 type vti local 10.0.0.2 remote 10.0.0.1 key 10 dev ltp_ns_veth2 > ip li set ltp_vti0 up > ip -4 xf st add src 10.0.0.1 dst 10.0.0.2 proto comp spi 0x1001 comp deflate mode tunnel > ip -4 xf po add dir out tmpl src 10.0.0.2 dst 10.0.0.1 proto comp mode tunnel mark 10 > ip -4 xf po add dir in tmpl src 10.0.0.1 dst 10.0.0.2 proto comp mode tunnel mark 10 > ip route add 10.23.1.0/30 dev ltp_vti0 > ip a add 10.23.1.1/30 dev ltp_vti0 > > ...omitted corresponded setup in netns for the other end. > > The problem appears with the small packets like SYN which are not > compressed and sent as is through vti tunnel and appear on ltp_ns_veth2 > as IPIP packets. On the other end, vti doesn't handle them and theyare > rejected (InNoPol stats increased). > > As a workaround, setting: > # sysctl net.ipv4.conf.ltp_ns_veth2.disable_policy=1 > # sysctl net.ipv4.conf.ltp_ns_veth1.disable_policy=1 > > works, but compressed packets seen on vti device, the other on ltp_ns_veth2. > > Is there some flaw in setup or vti not designed to handle ipcomp alg that > can send packets with/without compression (or without further encryption)? VTI is not designed to handle packets without IPsec header, so yes this does not work well with ipcomp that might omit the compression header. > > May be we should handle such packets by registering additional tunnel > handler onvti, like in the diff below? > > diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c > index 89453cf..99ad70b 100644 > --- a/net/ipv4/ip_vti.c > +++ b/net/ipv4/ip_vti.c > @@ -44,11 +44,20 @@ > #include <net/net_namespace.h> > #include <net/netns/generic.h> > > +static bool log_ecn_error = true; > +module_param(log_ecn_error, bool, 0644); > +MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN"); > + > static struct rtnl_link_ops vti_link_ops __read_mostly; > > static unsigned int vti_net_id __read_mostly; > static int vti_tunnel_init(struct net_device *dev); > > +static const struct tnl_ptk_info tpi = { > + /* no tunnel info required for ipip. */ > + .proto = htons(ETH_P_IP), > +}; > + > static int vti_input(struct sk_buff *skb, int nexthdr, __be32 spi, > int encap_type) > { > @@ -65,6 +74,13 @@ static int vti_input(struct sk_buff *skb, int nexthdr, __be32 spi, > > XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = tunnel; > > + if (ip_hdr(skb)->protocol == IPPROTO_IPIP) { > + if (iptunnel_pull_header(skb, 0, tpi.proto, false)) > + goto drop; > + return ip_tunnel_rcv(tunnel, skb, &tpi, NULL, > + log_ecn_error); > + } > + As it is, we can rely that packets received by a vti interface came through a secure channel. This would not be the case anymore with your change and I'd not like to weaken this for a corener case like ipcomp in tunnel mode with vti.
On 11/27/2017 03:07 PM, Steffen Klassert wrote: > On Wed, Nov 22, 2017 at 07:06:13PM +0300, Alexey Kodanev wrote: >> Hi Steffen, >> >> LTP has vti test-cases which fail on ipcomp alg, e.g. >> "tcp_ipsec_vti.sh -p comp -m tunnel -s 100" >> >> Basically, the setupconsists of the following commands: >> >> ip li add ltp_vti0 type vti local 10.0.0.2 remote 10.0.0.1 key 10 dev ltp_ns_veth2 >> ip li set ltp_vti0 up >> ip -4 xf st add src 10.0.0.1 dst 10.0.0.2 proto comp spi 0x1001 comp deflate mode tunnel >> ip -4 xf po add dir out tmpl src 10.0.0.2 dst 10.0.0.1 proto comp mode tunnel mark 10 >> ip -4 xf po add dir in tmpl src 10.0.0.1 dst 10.0.0.2 proto comp mode tunnel mark 10 >> ip route add 10.23.1.0/30 dev ltp_vti0 >> ip a add 10.23.1.1/30 dev ltp_vti0 >> >> ...omitted corresponded setup in netns for the other end. >> >> The problem appears with the small packets like SYN which are not >> compressed and sent as is through vti tunnel and appear on ltp_ns_veth2 >> as IPIP packets. On the other end, vti doesn't handle them and theyare >> rejected (InNoPol stats increased). >> >> As a workaround, setting: >> # sysctl net.ipv4.conf.ltp_ns_veth2.disable_policy=1 >> # sysctl net.ipv4.conf.ltp_ns_veth1.disable_policy=1 >> >> works, but compressed packets seen on vti device, the other on ltp_ns_veth2. >> >> Is there some flaw in setup or vti not designed to handle ipcomp alg that >> can send packets with/without compression (or without further encryption)? > VTI is not designed to handle packets without IPsec header, so yes > this does not work well with ipcomp that might omit the compression > header. if so, is it reasonable to keep ipcomp handling in VTI? Thanks, Alexey > >> May be we should handle such packets by registering additional tunnel >> handler onvti, like in the diff below? >> >> diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c >> index 89453cf..99ad70b 100644 >> --- a/net/ipv4/ip_vti.c >> +++ b/net/ipv4/ip_vti.c >> @@ -44,11 +44,20 @@ >> #include <net/net_namespace.h> >> #include <net/netns/generic.h> >> >> +static bool log_ecn_error = true; >> +module_param(log_ecn_error, bool, 0644); >> +MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN"); >> + >> static struct rtnl_link_ops vti_link_ops __read_mostly; >> >> static unsigned int vti_net_id __read_mostly; >> static int vti_tunnel_init(struct net_device *dev); >> >> +static const struct tnl_ptk_info tpi = { >> + /* no tunnel info required for ipip. */ >> + .proto = htons(ETH_P_IP), >> +}; >> + >> static int vti_input(struct sk_buff *skb, int nexthdr, __be32 spi, >> int encap_type) >> { >> @@ -65,6 +74,13 @@ static int vti_input(struct sk_buff *skb, int nexthdr, __be32 spi, >> >> XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = tunnel; >> >> + if (ip_hdr(skb)->protocol == IPPROTO_IPIP) { >> + if (iptunnel_pull_header(skb, 0, tpi.proto, false)) >> + goto drop; >> + return ip_tunnel_rcv(tunnel, skb, &tpi, NULL, >> + log_ecn_error); >> + } >> + > As it is, we can rely that packets received by a vti interface came > through a secure channel. This would not be the case anymore with > your change and I'd not like to weaken this for a corener case > like ipcomp in tunnel mode with vti.
On Mon, Nov 27, 2017 at 04:00:38PM +0300, Alexey Kodanev wrote: > On 11/27/2017 03:07 PM, Steffen Klassert wrote: > > On Wed, Nov 22, 2017 at 07:06:13PM +0300, Alexey Kodanev wrote: > >> > >> Is there some flaw in setup or vti not designed to handle ipcomp alg that > >> can send packets with/without compression (or without further encryption)? > > VTI is not designed to handle packets without IPsec header, so yes > > this does not work well with ipcomp that might omit the compression > > header. > > if so, is it reasonable to keep ipcomp handling in VTI? I'd say it does not make sense to use vti with ipcomp, but it might be that somebody created a usecase from that. Unfortunately it is always critical to remove functionality that is exposed to userspace, even if it is broken.
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c index 89453cf..99ad70b 100644 --- a/net/ipv4/ip_vti.c +++ b/net/ipv4/ip_vti.c @@ -44,11 +44,20 @@ #include <net/net_namespace.h> #include <net/netns/generic.h> +static bool log_ecn_error = true; +module_param(log_ecn_error, bool, 0644); +MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN"); + static struct rtnl_link_ops vti_link_ops __read_mostly; static unsigned int vti_net_id __read_mostly; static int vti_tunnel_init(struct net_device *dev); +static const struct tnl_ptk_info tpi = { + /* no tunnel info required for ipip. */ + .proto = htons(ETH_P_IP), +}; + static int vti_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type) { @@ -65,6 +74,13 @@ static int vti_input(struct sk_buff *skb, int nexthdr, __be32 spi, XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = tunnel; + if (ip_hdr(skb)->protocol == IPPROTO_IPIP) { + if (iptunnel_pull_header(skb, 0, tpi.proto, false)) + goto drop; + return ip_tunnel_rcv(tunnel, skb, &tpi, NULL, + log_ecn_error); + } + return xfrm_input(skb, nexthdr, spi, encap_type); } @@ -335,6 +351,11 @@ static int vti4_err(struct sk_buff *skb, u32 info) return 0; } +static int vti_ip_err(struct sk_buff *skb, u32 info) +{ + return -ENOENT; +} + static int vti_tunnel_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) { @@ -440,6 +461,12 @@ static void __net_init vti_fb_tunnel_init(struct net_device *dev) .priority = 100, }; +static struct xfrm_tunnel vti_ip4_tunnnel __read_mostly = { + .handler = vti_rcv, + .err_handler = vti_ip_err, + .priority = 1, +}; + static int __net_init vti_init_net(struct net *net) { int err; @@ -607,6 +634,9 @@ static int __init vti_init(void) err = xfrm4_protocol_register(&vti_ipcomp4_protocol, IPPROTO_COMP); if (err < 0) goto xfrm_proto_comp_failed; + err = xfrm4_tunnel_register(&vti_ip4_tunnnel, AF_INET); + if (err < 0) + goto xfrm_tunnel_failed; msg = "netlink interface"; err = rtnl_link_register(&vti_link_ops); @@ -616,6 +646,8 @@ static int __init vti_init(void) return err; rtnl_link_failed: + xfrm4_tunnel_deregister(&vti_ip4_tunnnel, AF_INET); +xfrm_tunnel_failed: xfrm4_protocol_deregister(&vti_ipcomp4_protocol, IPPROTO_COMP); xfrm_proto_comp_failed: xfrm4_protocol_deregister(&vti_ah4_protocol, IPPROTO_AH); @@ -631,6 +663,7 @@ static int __init vti_init(void) static void __exit vti_fini(void) { rtnl_link_unregister(&vti_link_ops); + xfrm4_tunnel_deregister(&vti_ip4_tunnnel, AF_INET); xfrm4_protocol_deregister(&vti_ipcomp4_protocol, IPPROTO_COMP); xfrm4_protocol_deregister(&vti_ah4_protocol, IPPROTO_AH); xfrm4_protocol_deregister(&vti_esp4_protocol, IPPROTO_ESP);