Re: [PATCH 1/7] udp: Don't attempt to translate a 0.0.0.0 source address

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2978 bytes --]

On Thu, Feb 22, 2024 at 06:46:02PM +0100, Stefano Brivio wrote:
> On Thu, 22 Feb 2024 10:21:09 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > If an incoming packet has a source address of 0.0.0.0 we translate that to
> > the gateway address.  This doesn't really make sense, because we have no
> > way to do a reverse translation for reply packets.
> 
> Well, we would translate that back to a loopback address, which is fine
> if we take 0.0.0.0 as "This host on this network".

Not really, because "this host" has a different meaning to the sender
than it does to us.  For example, returning replies to DHCP broadcasts
to localhost would absolutely not be correct.  Of course, attempting
to run a DHCP server within passt/pasta sounds problematic, but my
point is that localhost is not a reasonable translation.

> Actually, after my
> previous note based on RFC 6890, I went and had a look at RFC 1122,
> section 3.2.1.3, which also states that 0.0.0.0:
> 
>   MUST NOT be sent, except as a source address as part of an
>   initialization procedure by which the host learns its own IP address.
> 
> ...so I guess dropping it here is fine.

I'm not dropping it: I'm leaving it untranslated.

> By the way, I added this originally as part of commit 6488c3e8489d
> ("tcp, udp: Replace loopback source address by gateway address") on the
> basis that 0.0.0.0 could be used in lieu of a loopback address, but
> sure, we shouldn't even get it from the kernel to start with.

Again, that's true for certain API calls, but AFAICT it's not true on
the wire.  At least it seems to be that way in practice, although I
haven't located an RFC to say that explicitly.

> > Certain UDP protocols do use an unspecified source address in some
> > circumstances (e.g. DHCP).  These generally either require no reply, a
> > multicast reply, or provide a suitable reply address by other means.
> > 
> > In none of those cases does translating it in passt/pasta make sense.  The
> > best we can really do here is just leave it as is.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  udp.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/udp.c b/udp.c
> > index a3961bfd..d2f8027c 100644
> > --- a/udp.c
> > +++ b/udp.c
> > @@ -599,7 +599,6 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
> >  	    src_port == 53) {
> >  		b->iph.saddr = c->ip4.dns_match.s_addr;
> >  	} else if (IN4_IS_ADDR_LOOPBACK(&b->s_in.sin_addr) ||
> > -		   IN4_IS_ADDR_UNSPECIFIED(&b->s_in.sin_addr)||
> >  		   IN4_ARE_ADDR_EQUAL(&b->s_in.sin_addr, &c->ip4.addr_seen)) {
> >  		b->iph.saddr = c->ip4.gw.s_addr;
> >  		udp_tap_map[V4][src_port].ts = now->tv_sec;
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]