netdev - Re: [PATCH net] ip: fix triggering of 'icmp redirect'

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 29 Sep 2022 21:43:38 +0300 (EEST)
From:   Julian Anastasov <ja@....bg>
To:     Nicolas Dichtel <nicolas.dichtel@...nd.com>
cc:     David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH net] ip: fix triggering of 'icmp redirect'


	Hello,

On Thu, 29 Sep 2022, Nicolas Dichtel wrote:

> Le 27/09/2022 à 14:56, Julian Anastasov a écrit :
> > 
> > nhc_gw	nhc_scope		rt_gw4		fib_scope (route)
> > ---------------------------------------------------------------------------
> > 0	RT_SCOPE_NOWHERE	Host		RT_SCOPE_HOST (local)
> > 0	RT_SCOPE_HOST		LAN_TARGET	RT_SCOPE_LINK (link)
> > LOCAL1	RT_SCOPE_HOST		LAN_TARGET	RT_SCOPE_LINK (link)
> > REM_GW1	RT_SCOPE_LINK		Universe	RT_SCOPE_UNIVERSE (indirect)
> > 
> > 	For the code above: we do not check res->scope,
> > we are interested what is the nhc_gw's scope (LINK/HOST/NOWHERE).
> > It means, reverse route points back to same device and sender is not
> > reached via gateway REM_GW1.

	In short, to send redirects, sender should be
reachable via link route (with nhc_scope = RT_SCOPE_HOST).

> iproute2 reject a gw which is not directly connected, am I wrong?

	ip tool can not do it. It only provides route's scope
specified by user and the GW's scope is determined by 
fib_check_nh_v4_gw() as route's scope + 1 but at least RT_SCOPE_LINK:

                /* It is not necessary, but requires a bit of thinking */
                if (fl4.flowi4_scope < RT_SCOPE_LINK)
                        fl4.flowi4_scope = RT_SCOPE_LINK;

	The other allowed value for nhc_scope when rt_gw4 is not 0 is
RT_SCOPE_HOST (GW is a local IP, useful when autoselecting source
address from same subnet). It is set by fib_check_nh_v4_gw() when
res.type = RTN_LOCAL.

> > 	By changing it to nhc_scope >= RT_SCOPE_LINK, ret always
> > will be 1 because nhc_scope is not set below RT_SCOPE_LINK (253).
> > Note that RT_SCOPE_HOST is 254.
> Do you have a setup which shows the problem?

	No, just by analyze. RT_SCOPE_LINK indicates sender
is reached via GW.

> After reverting the two commits (747c14307214 and eb55dc09b5dd) and putting the
> below patch, the initial problem is fixed. But it's not clear what is broken
> with the current code. Before sending these patches formally, it would be nice
> to be able to add a selftest to demonstrate what is wrong.

	What is broken? I guess, __fib_validate_source always
returns 1 causing redirects.

	As for nh_create_ipv4(), may be using scope=0 as
arg to fib_check_nh() should work. Now I can not find example
for corner case where this can fail.

> @@ -2534,7 +2534,7 @@ static int nh_create_ipv4(struct net *net, struct nexthop *nh,
>  	if (!err) {
>  		nh->nh_flags = fib_nh->fib_nh_flags;
>  		fib_info_update_nhc_saddr(net, &fib_nh->nh_common,
> -					  fib_nh->fib_nh_scope);
> +					  !fib_nh->fib_nh_scope ? 0 : fib_nh->fib_nh_scope - 1);

	And this fix is needed to not expose scope host
saddr (127.0.0.1) when nexthop is without GW and to
not expose scope link saddr when nexthop is with GW (for
traffic via scope global routes).

>  	} else {
>  		fib_nh_release(net, fib_nh);
>  	}

Regards

--
Julian Anastasov <ja@....bg>