nconsistencies with IP_ONESBCAST and/or IP_SENDSRCADDR

[Bruce M. Simpson]

Bruce M Simpson wrote:
> Hello,
>
> In preparation for tightening up our handling of INADDR_BROADCAST 
> sends, I ran some brief tests today on the network stack with the 
> attached test code.
>
> I found some inconsistencies when run against 6.2-RELEASE;
>
> 1. IP_ONESBCAST breaks if SO_DONTROUTE is specified.
>
> One thing appears to be consistent about the failure mode: bad UDP 
> checksums.
> dc(4) is being used on the destination end of the test network, so 
> checksum offloading should not be an issue.
> I am also seeing the wrong destination address being used in most 
> cases. This is intermittent regardless of whether the socket is bound 
> or unbound.
This is consistent with ip_output() treating its internal flag 
IP_SENDONES as separate from IP_ROUTETOIF. I was skimming an old patch 
of mine which attempts to implement part of SO_BINDTODEVICE which 
contains a fix related to this condition.

The fix isn't the right fix so I will revisit this now and hopefully 
commit a fix shortly.
>
> 2. IP_SENDSRCADDR has some other inconsistencies.
> a. The option is always rejected if the socket is not bound.
> I find this behaviour suspect; the whole point of the option is to 
> specify, for SOCK_DGRAM and SOCK_RAW, the source address of a packet.
> b. 0.0.0.0 is always accepted.
> A regular interface lookup is used based on destination if this is 
> specified. This appears suspect to me because such an option is 
> redundant.
This is of course a separate issue. Because it's more involved (it 
concerns the general concept of 'ip unnumbered' in the stack) it needs 
further consideration before any fix is attempted.

udp_output() will only call in_pcbbind_setup() if a non-INADDR_ANY 
source address was specified; this is usually obtained from the socket 
being bound previously. This explains why the IP_SENDSRCADDR option is 
rejected in udp_output() for an unbound socket. It *will* be accepted if 
the option contains INADDR_ANY. In this case, normal source address 
selection takes place.

This is a good use case demonstrating the need for source address 
selection logic such as is now found in NetBSD.

There is no sanity checking on the IP_SENDSRCADDR option data containing 
INADDR_ANY; such an option is redundant and is nonsensical for an 
unbound socket. We should reject the option if it contains INADDR_ANY if 
and only if the socket is not bound. Implementing such a check is fairly 
easy and makes sense for this use case. Returning EINVAL in this case 
seems acceptable according to ip(4).

The option *should* be accepted if the application has bound the socket 
to a device somehow (oh dear, SO_BINDTODEVICE rears its head again) as 
DHCP for example needs to override any IPv4 address which may be 
assigned on an ifnet with 0.0.0.0.

Regards,
BMS