netlink/route: make route deletion behavior match route(4) socket

[glebius]

Summary

Deleting a route with help of route(8) command in pre-netlink times was a
two step action: first the route(8) would RTM_GET the route from the
kernel and then copy its parameters into the RTM_DEL request. This
allowed deletion of pinned (RTF_PINNED) routes, as the flag was carried
over. The flag enforced call into rt_delete_conditional() with prio=2
parameter.

With netlink(4) enabled route(8), we construct the NL_RTM_DELROUTE request
from scratch and this ends in rt_delete_conditional() being called with
prio=1, which effectively blocks deleting a pinned route.

Make the netlink(4) code provide RTM_F_FORCE flag for rib_del_route_px(),
which would end up with prio=2 for rt_delete_conditional(). The rationale
is here that netlink(4) path isn't something automatic, it is either an
explicit request from an operator with route(8) or some other routing
daemon, and the RTF_PINNED protection shall be bypassed for any call via
netlink(4).

[vvd]

This patch fixed https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=279988 on 14.1.

[vvd]

I would really like to do EN for this to 14.1.

[zlei]

Looks good to me.

[vvd]

What else is needed to commit this patch?

[markj]

@glebius do you plan to commit this?

[melifaro]

I'm going to come up with a different version of this patch (likely using a new flag rtmsg->rtm_flags to signal RTM_F_FORCE) in a day or two. The current version allows all netlink customers to fully bypass PINNED route protection, which defeats its purpose.

[franco_opnsense.org]

In D46301#1079978, @melifaro wrote:

I'm going to come up with a different version of this patch (likely using a new flag rtmsg->rtm_flags to signal RTM_F_FORCE) in a day or two. The current version allows all netlink customers to fully bypass PINNED route protection, which defeats its purpose.

Did this ever happen? Chased the bug with a user and D47534 did not fix it but that was likely intended. Unable to locate another commit or review.

[ae]

I don't like the idea that you can easily remove PINNED route, but it seems it always worked before.
However as I see, route(8) should pass RTF_PINNED flag to netlink via attrs.rta_rtflags. At least we should reduce use of RTM_F_FORCE only for case when RTF_PINNED was sent from userland.

[franco_opnsense.org]

Like this?

(attrs.rta_rtflags & RTF_PINNED) ? RTM_F_FORCE : 0

It seemed strange to me the user said a binary from a 13.2 system was able to drop the route in a 14.1 system while the native binary was not able to. Could hint at netlink getting in the way? Nevermind, this is the netlink code after all so here's the reason why it differs.

[ae]

In D46301#1091706, @franco_opnsense.org wrote:

(attrs.rta_rtflags & RTF_PINNED) ? RTM_F_FORCE : 0

Yes, like this.

It seemed strange to me the user said a binary from a 13.2 system was able to drop the route in a 14.1 system while the native binary was not able to. Could hint at netlink getting in the way? Nevermind, this is the netlink code after all so here's the reason why it differs.

The binary from 13.2 probably uses old rtsock interface, new binary by default is build with netlink support.

[ae]

I just tested these commands from PR:

ifconfig tun0 create
ifconfig tun0 10.10.10.10 20.20.20.20
route -n delete -host 20.20.20.20 -interface tun0

with this patch:

--- a/sys/netlink/route/rt.c
+++ b/sys/netlink/route/rt.c
@@ -1010,8 +1010,9 @@ rtnl_handle_delroute(struct nlmsghdr *hdr, struct nlpcb *nlp,
                return (EINVAL);
        }

+       int op_flags = (attrs.rta_rtflags & RTF_PINNED) ? RTM_F_FORCE : 0;
        error = rib_del_route_px(attrs.rta_table, attrs.rta_dst,
-           attrs.rtm_dst_len, path_match_func, &attrs, 0, &rc);
+           attrs.rtm_dst_len, path_match_func, &attrs, op_flags, &rc);
        if (error == 0)
                report_operation(attrs.rta_table, &rc, nlp, hdr);
        return (error);

It seems works as expected.

[franco_opnsense.org]

Offered a test kernel to the user in the meantime. @glebius is probably busy (last I spoke with him) and doesn't mind if you commit?