NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/52074: -current npf map directive broken

Hi Roy !

I had a quick look at IPv6. There the packet filter seems to be after local address checking and scope related checks. I haven't read up on the scope rules - so somebody should look at the different pfil run location and whether it is appropriate to handle in case the packet filter changes to source address to be a non local source address.


On 05/07/17 21:50, Roy Marples wrote:
On 07/05/2017 17:28, Mindaugas Rasiukevicius wrote:
 The condition in 620 currently is (see if()):
      ia = in_get_ia_psref(ip->ip_src, &psref_ia);

      /* Ensure we only send from a valid address. */==>
      if ((ia != NULL || (flags & IP_FORWARDING) == 0) && <<<<!
          (error = ip_ifaddrvalid(ia)) != 0)
              "refusing to send from invalid address %s (pid %d)\n",
              ARPLOGADDR(ip->ip_src), curproc->p_pid);
          if (error == 1)
               * Address exists, but is tentative or detached.
               * We can't send from it because it's invalid,
               * so we drop the packet.
              error = 0;
              error = EADDRNOTAVAIL;
          goto bad;

Proposed fix is to replace the || inf the if with && as this also seems
 to have been the original intention by the author.

Good catch.

Looks like the regression was introduced 7 months ago, as part of the
ip_output.c rev 1.261.  Roy, would you like to have a look into this?

I  think xtos already comitted a fix, but I'm unsure his fix is correct.
I think the workflow should be this:

if (ia != NULL)
    error = ip_ifaddrvalid(ia);
    error = flags & IP_FORWARDING ? 0 : -1;
if (error != 0) { ...

The idea is that if we claim to send from an address it has to be valid, but allow the NULL address if forwarded from the filter.

Does this make sense?
The same path probably needs adjustment in inet6.


Home | Main Index | Thread Index | Old Index