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.