dco: rpf check improvements

[cron2]

make DCO driver on Linux respect rp_filter flag (sysctl) or make it a per-peer config flag to disable RPF check on individual peers. Or both.

(This becomes relevant if routing topologies are not symmetric, for whatever reasons)

[mkroesen]

The internal RPF check in the ovpn kernel module currently breaks setups that use Policy Based Routing (PBR) for clients with, for example, overlapping subnets.

For egress traffic, we separate these identical subnets by routing each flow into its own dedicated routing table via from PREFIX lookup TABLE_ID to set the correct individual next-hop per client. Since ingress/return packets are legitimate replies to these established connections, Linux Conntrack already handles the connection state and validates the flow. Under normal circumstances, this egress setup would be completely sufficient, and no return-path rules would be required because Conntrack matches the reply.

However, the ovpn driver strictly enforces its own peer validation on ingress. Because there are no global routes for these overlapping subnets in the main table, this hardcoded check drops the packets - completely ignoring rp_filter settings.

Suggested solution: Instead of blindly forcing a fib_lookup on every ingress packet, the driver should check if the packet is already part of an established/related connection via Conntrack. If Conntrack validates the reply, the fib_lookup could be safely bypassed. Alternatively, providing a config option to disable this internal ingress validation entirely would allow users to hand over full control to Conntrack and nftables.

[ordex]

Hi @mkroesen and sorry for the delay.
I fully get your case and it is indeed an interesting one.

Probably respecting the rp_filter knob and allow the user to switch the logic off may be the easiest next step.
However, I also don't want to introduce a user facing behaviour so lightly as we will have to carry it along for quite some time.

Talking to conntrack may be non trivial (and bring its own set of issues), but next week I will deepen this aspect for sure.

Thanks!

[mkroesen]

Hi @ordex, thank you very much for your feedback. I’ve been looking into this in more detail over the last few days, and in my case, disabling the rp_filter wouldn’t be secure enough, since I’m using nftables masquerading on outgoing packets.

To address this, I modified the ovpn_nexthop_from_rt4 function and added ovpn_flowi4_from_skb4 to peer.c. This function attempts to obtain an nf_conn for the SKB, which allows the flowi4 to be constructed more precisely so that ovpn_nexthop_from_rt4 can return a positive result.

In case it’s relevant, here are the two functions for reference:

#if IS_ENABLED(CONFIG_NF_CONNTRACK)
#include <net/netfilter/nf_conntrack_core.h>
#endif

static struct flowi4 ovpn_flowi4_from_skb4(struct ovpn_priv *ovpn, struct sk_buff *skb)
{
	struct flowi4 fl = {
		.daddr = ip_hdr(skb)->saddr
	};

#if IS_ENABLED(CONFIG_NF_CONNTRACK)
	struct nf_conntrack_tuple_hash *h;
	struct nf_conntrack_tuple tuple;

	if (likely(!nf_ct_get_tuplepr(skb, skb_network_offset(skb), PF_INET, dev_net(ovpn->dev), &tuple)))
		return fl;

	h = nf_conntrack_find_get(dev_net(ovpn->dev), &nf_ct_zone_dflt, &tuple);

	if (likely(h)) {
		struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(h);

		fl.saddr = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u3.ip;
		fl.daddr = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.u3.ip;

#if IS_ENABLED(CONFIG_NF_NAT)
		if (ct->status & IPS_SRC_NAT)
			fl.flowi4_flags |= FLOWI_FLAG_ANYSRC;
#endif

		fl.flowi4_proto = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum;

		fl.fl4_sport = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u.all;
		fl.fl4_dport = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.u.all;

#if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
		fl.flowi4_mark = ct->mark;
#endif

#if IS_ENABLED(CONFIG_NF_CONNTRACK_SECMARK)
		fl.flowi4_secid = ct->secmark;
#endif

		nf_ct_put(ct);
	}
#endif

	return fl;
}

static __be32 ovpn_nexthop_from_rt4(struct ovpn_priv *ovpn, struct sk_buff *skb)
{
	__be32 dest;
	struct rtable *rt;
	struct flowi4 fl = ovpn_flowi4_from_skb4(skb);

	dest = fl.daddr;

	rt = ip_route_output_flow(dev_net(ovpn->dev), &fl, NULL);
	if (IS_ERR(rt)) {
		net_dbg_ratelimited("%s: no route to host %pI4\n",
				    netdev_name(ovpn->dev), &dest);
		return dest;
	}

	if (!ovpn_rt4_uses_gateway(rt))
		goto out;

	dest = rt->rt_gw4;
out:
	ip_rt_put(rt);
	return dest;
}

Tested on Debian 12 with kernel version 6.12.90 using your backport package. I also implemented the ovpn_flowi6_from_skb6 function for ovpn_nexthop_from_rt6, but I haven't tested it yet. Unfortunately, my knowledge of this is limited, so I can't tell how good it would be to implement something like this properly.