[PATCH net] net:ipv4:ip_route_input_slow: Change behaviour of routing decision when IP router alert option is present

[PATCH net] net:ipv4:ip_route_input_slow: Change behaviour of routing decision when IP router alert option is present

[Guy Avraham]

When an IP packet with the IP router alert (RFC 2113) field arrives
to some host who is not the destination of that packet (i.e - non of
its interfaces is the address in the destination IP address field of that
packet) and, for whatever reason, it does not have a route to this
destination address, it drops this packet during the "routing decision"
flow even though it should potentially pass it to the relevant
application(s) that are interested in this packet's content - which happens
in the "forwarding decision" flow. The suggested fix changes this behaviour
by setting the ip_forward as the next "step" in the flow of the packet,
just before it (previously was) is dropped, so that later the ip_forward,
as usual, will pass it on to its relevant recipient (socket), by
invoking the ip_call_ra_chain.

Signed-off-by: Guy Avraham <guyavrah1986@gmail.com>
---
The fix was tested and verified on Linux hosts that act as routers in which
there are kerenls 3.10 and 5.2. The verification was done by simulating
a scenario in which an RSVP (RFC 2205) Path message (that has the IP
router alert option set) arrives to a transit RSVP node, and this host
passes on the RSVP Path message to the relevant socket (of the RSVP
deamon) even though upon arrival of this packet it does NOT have route
to the destination IP address of the IP packet (that encapsulates the
RSVP Path message).

 net/ipv4/route.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 13c0f1d455f3..7c416eca84f8 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2360,8 +2360,12 @@ out:	return err;
 
 	RT_CACHE_STAT_INC(in_slow_tot);
 	if (res->type == RTN_UNREACHABLE) {
-		rth->dst.input= ip_error;
-		rth->dst.error= -err;
+		if (IPCB(skb)->opt.router_alert)
+			rth->dst.input = ip_forward;
+		else
+			rth->dst.input = ip_error;
+
+		rth->dst.error = -err;
 		rth->rt_flags	&= ~RTCF_LOCAL;
 	}
 
-- 
2.25.1

Re: [PATCH net] net:ipv4:ip_route_input_slow: Change behaviour of routing decision when IP router alert option is present

[Paolo Abeni]

Hi,

On 9/12/24 16:14, Guy Avraham wrote:
> When an IP packet with the IP router alert (RFC 2113) field arrives
> to some host who is not the destination of that packet (i.e - non of
> its interfaces is the address in the destination IP address field of that
> packet) and, for whatever reason, it does not have a route to this
> destination address, it drops this packet during the "routing decision"
> flow even though it should potentially pass it to the relevant
> application(s) that are interested in this packet's content - which happens
> in the "forwarding decision" flow. The suggested fix changes this behaviour
> by setting the ip_forward as the next "step" in the flow of the packet,
> just before it (previously was) is dropped, so that later the ip_forward,
> as usual, will pass it on to its relevant recipient (socket), by
> invoking the ip_call_ra_chain.
> 
> Signed-off-by: Guy Avraham <guyavrah1986@gmail.com>
> ---
> The fix was tested and verified on Linux hosts that act as routers in which
> there are kerenls 3.10 and 5.2. The verification was done by simulating
> a scenario in which an RSVP (RFC 2205) Path message (that has the IP
> router alert option set) arrives to a transit RSVP node, and this host
> passes on the RSVP Path message to the relevant socket (of the RSVP
> deamon) even though upon arrival of this packet it does NOT have route
> to the destination IP address of the IP packet (that encapsulates the
> RSVP Path message).
> 
>   net/ipv4/route.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 13c0f1d455f3..7c416eca84f8 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2360,8 +2360,12 @@ out:	return err;
>   
>   	RT_CACHE_STAT_INC(in_slow_tot);
>   	if (res->type == RTN_UNREACHABLE) {
> -		rth->dst.input= ip_error;
> -		rth->dst.error= -err;
> +		if (IPCB(skb)->opt.router_alert)
> +			rth->dst.input = ip_forward;
> +		else
> +			rth->dst.input = ip_error;
> +
> +		rth->dst.error = -err;
>   		rth->rt_flags	&= ~RTCF_LOCAL;
>   	}
>   

I think this is not the correct solution. At very least you should check 
the host is actually a router (forwarding is enabled) and someone has 
registered to receive router alerts. At that point you will be better 
off processing the router alert in place directly calling 
ip_call_ra_chain().

However I'm unsure all the above is actually required. It can be argued 
your host has a bad configuration.

If it's a AS border router, and there is no route for the destination, 
the packet not matching any route is invalid and should be indeed 
dropped/not processed.

Otherwise you should have/add a catch-up default route - at very least 
to handle this cases. If you really want to forward packets only to 
known destination, you could make such route as blackhole one.

Cheers,

Paolo

Re: [PATCH net] net:ipv4:ip_route_input_slow: Change behaviour of routing decision when IP router alert option is present

[Guy Avraham]

Hi Paolo,

Thanks for your response and inputs!

Please consider the following:

1. Regarding the  check  of IP forwarding enabled - totally agree, I
forgot to add it.

2. About the validation that someone has registered to receive router
alerts - as
far as I understand, this check is done in the ip_call_ra_chain
function - if indeed
there is no one interested in receiving IP packets with the IP router
alert option, then
this function will return false (and do nothing essentially), so
calling it directly should
be OK.

3. About calling the ip_call_ra_chain function directly in place - I
also agree but what
I am missing is how to set the rth->dst.input in this case?
- ip_forward is not relevant (we eliminated, wisely, the need to call
it once the ip_call_ra_chain
is to be invoked directly).
- ip_local_deliver is not needed (the packet was already
consumed by the relevant recipient - the socket that registered for
the IP router alert).
- ip_error is also not needed because sending an ICMP error packet is
not exactly what is needed in this case (at least not for the use case
I refer to in which the IP packet holds an RSVP message).
It leaves the "option" of (whether or not the ip_call_ra_chain was
successful or not, i.e. -
it returned true/false) returning NET_RX_DROP --> this way when the
flow unfolds all the
way back to the ip_rcv_finish function, the dst.input function pointer
won't be invoked (in the line
ret = dst_input(skb);)
One thing about it, is whether or not it is "fine" for the function
further back in the flow
of the packet reception (netif_rx,etc...) to receive this value
(NET_RX_DROP and not NET_RX_SUCCESS), even though the packet was
consumed eventually.

4. In the specific use case I am talking about the host is a router
which is not an AS border router.
About the blackhole - it is not what I need to achieve. In my case I
do wish that the IP packet will
arrive to the relevant socket (of the RSVP daemon), but because the
host that received the IP packet with the IP router alert does not
have a route to the destination IP, the flow terminates without going
through the ip_call_ra_chain (which is done only in the ip_forward
function later on).
I can elaborate more on this specific use case if needed (it has to do
with the way OSPF and RSVP work).

Appreciate your response,
Guy.


On Thu, Sep 19, 2024 at 1:06 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi,
>
> On 9/12/24 16:14, Guy Avraham wrote:
> > When an IP packet with the IP router alert (RFC 2113) field arrives
> > to some host who is not the destination of that packet (i.e - non of
> > its interfaces is the address in the destination IP address field of that
> > packet) and, for whatever reason, it does not have a route to this
> > destination address, it drops this packet during the "routing decision"
> > flow even though it should potentially pass it to the relevant
> > application(s) that are interested in this packet's content - which happens
> > in the "forwarding decision" flow. The suggested fix changes this behaviour
> > by setting the ip_forward as the next "step" in the flow of the packet,
> > just before it (previously was) is dropped, so that later the ip_forward,
> > as usual, will pass it on to its relevant recipient (socket), by
> > invoking the ip_call_ra_chain.
> >
> > Signed-off-by: Guy Avraham <guyavrah1986@gmail.com>
> > ---
> > The fix was tested and verified on Linux hosts that act as routers in which
> > there are kerenls 3.10 and 5.2. The verification was done by simulating
> > a scenario in which an RSVP (RFC 2205) Path message (that has the IP
> > router alert option set) arrives to a transit RSVP node, and this host
> > passes on the RSVP Path message to the relevant socket (of the RSVP
> > deamon) even though upon arrival of this packet it does NOT have route
> > to the destination IP address of the IP packet (that encapsulates the
> > RSVP Path message).
> >
> >   net/ipv4/route.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index 13c0f1d455f3..7c416eca84f8 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -2360,8 +2360,12 @@ out:   return err;
> >
> >       RT_CACHE_STAT_INC(in_slow_tot);
> >       if (res->type == RTN_UNREACHABLE) {
> > -             rth->dst.input= ip_error;
> > -             rth->dst.error= -err;
> > +             if (IPCB(skb)->opt.router_alert)
> > +                     rth->dst.input = ip_forward;
> > +             else
> > +                     rth->dst.input = ip_error;
> > +
> > +             rth->dst.error = -err;
> >               rth->rt_flags   &= ~RTCF_LOCAL;
> >       }
> >
>
> I think this is not the correct solution. At very least you should check
> the host is actually a router (forwarding is enabled) and someone has
> registered to receive router alerts. At that point you will be better
> off processing the router alert in place directly calling
> ip_call_ra_chain().
>
> However I'm unsure all the above is actually required. It can be argued
> your host has a bad configuration.
>
> If it's a AS border router, and there is no route for the destination,
> the packet not matching any route is invalid and should be indeed
> dropped/not processed.
>
> Otherwise you should have/add a catch-up default route - at very least
> to handle this cases. If you really want to forward packets only to
> known destination, you could make such route as blackhole one.
>
> Cheers,
>
> Paolo
>