From: Michał Markowski <markowski1@gmail.com>
Subject: Re: do not add default routes with blackhole or reject to the egress group
To: Peter Hessler <phessler@theapt.org>
Cc: tech@openbsd.org
Date: Thu, 1 May 2025 20:52:37 +0200

czw., 1 maj 2025 o 12:18 Peter Hessler <phessler@theapt.org> napisał(a):
>
> On 2025 May 01 (Thu) at 12:04:00 +0200 (+0200), Claudio Jeker wrote:
> :On Thu, May 01, 2025 at 11:52:48AM +0200, Peter Hessler wrote:
> :> The egress group is automatically added to any interface that has a
> :> default route pointing to it.  However, not all default routes are
> :> usable.  Both blackhole and reject routes are unreachable, so do not add
> :> those to the egress group.
> :>
> :> Found by tb@, debugged by myself and claudio@
> :>
> :> OK?
> :
> :It would probably be nicer to write those while loops as for () loops
> :and then use a simple continue. Like:
> :
> :       rt = rtable_lookup(0, sintosa(&sa_in), sintosa(&sa_in), NULL, RTP_ANY);
> :       for (; rt != NULL; rt = rtable_iterate(rt)) {
> :               if (ISSET(rt->rt_flags, RTF_REJECT | RTF_BLACKHOLE))
> :                       continue;
> :               ...
> :       }
> :
> :I did not fold the rtable_lookup() line into the for loop since it is tool
> :long.
> :
>
> Sure, why not.  as with the previous diff, tested both IPv4 and IPv6.
>
>
> Index: net/if.c
> ===================================================================
> RCS file: /cvs/openbsd/src/sys/net/if.c,v
> diff -u -p -u -p -r1.730 if.c
> --- net/if.c    17 Apr 2025 17:23:17 -0000      1.730
> +++ net/if.c    1 May 2025 10:07:56 -0000
> @@ -3286,26 +3286,28 @@ if_group_egress_build(void)
>         sa_in.sin_len = sizeof(sa_in);
>         sa_in.sin_family = AF_INET;
>         rt = rtable_lookup(0, sintosa(&sa_in), sintosa(&sa_in), NULL, RTP_ANY);
> -       while (rt != NULL) {
> +       for (; rt != NULL; rt = rtable_iterate(rt)) {
> +               if (ISSET(rt->rt_flags, RTF_REJECT | RTF_BLACKHOLE))
> +                       continue;
>                 ifp = if_get(rt->rt_ifidx);
>                 if (ifp != NULL) {
>                         if_addgroup(ifp, IFG_EGRESS);
>                         if_put(ifp);
>                 }
> -               rt = rtable_iterate(rt);
>         }
>
>  #ifdef INET6
>         bcopy(&sa6_any, &sa_in6, sizeof(sa_in6));
>         rt = rtable_lookup(0, sin6tosa(&sa_in6), sin6tosa(&sa_in6), NULL,
>             RTP_ANY);
> -       while (rt != NULL) {
> +       for (; rt != NULL; rt = rtable_iterate(rt)) {
> +               if (ISSET(rt->rt_flags, RTF_REJECT | RTF_BLACKHOLE))
> +                       continue;
>                 ifp = if_get(rt->rt_ifidx);
>                 if (ifp != NULL) {
>                         if_addgroup(ifp, IFG_EGRESS);
>                         if_put(ifp);
>                 }
> -               rt = rtable_iterate(rt);
>         }
>  #endif /* INET6 */
>
>
>
>
>
> --
> I went into a general store, and they wouldn't sell me anything
> specific.
>                 -- Steven Wright
>

Maybe this should be mentioned explicitly in ifconfig(8).

--- sbin/ifconfig/ifconfig.8
+++ sbin/ifconfig/ifconfig.8
@@ -247,7 +247,11 @@ interface group.
 .It
 The interfaces the default routes point to are members of the
 .Dq egress
-interface group.
+interface group, except for the ones marked with
+.Fl blackhole
+or
+.Fl reject
+flag.
 .It
 IEEE 802.11 wireless interfaces are members of the
 .Dq wlan