Hello,

the issue has been brought up by claudio@ off-list:

> I have this rule in my pf.conf:
> 
>         match out on egress received-on tap nat-to (egress)
>
> Now if I ping6 www.google.com from the VM it should rewrite the IPv6
> address to the one on my egress and all is good but it seems pf(4) likes
> to pick deprecated addresses for that and therefor the NAT is not working.
>

</snip>
> # ifconfig egress | grep inet6 
>        inet6 fe80::3b00:8db4:3239:ffa8%iwm0 prefixlen 64 scopeid 0x1
>        inet6 2a02:aa13:7100:6c80:3ec4:738:9487:2ce prefixlen 64 autoconf 
> pltime 410625 vltime 895936
>        inet6 2a02:aa13:7100:6c80:31fc:27f6:5215:a467 prefixlen 64 deprecated 
> autoconf autoconfprivacy pltime 0 vltime 592919
>        inet6 2001:4bf8:30:8342:2ff2:f8ed:64b7:9fed prefixlen 64 deprecated 
> autoconf autoconfprivacy pltime 0 vltime 592919
>        inet6 2001:4bf8:30:8342:3710:4c:49a3:3f17 prefixlen 64 deprecated 
> autoconf pltime 0 vltime 2580119
>        inet6 2001:4bf8:30:8342:e632:c7a0:7248:1b2 prefixlen 64 deprecated 
> autoconf autoconfprivacy pltime 0 vltime 592919
</snip

>
> Only 2a02:aa13:7100:6c80:3ec4:738:9487:2ce is currently "valid" and all
> other addresses should not be used for outgoing connections (which
> includes NAT).

The idea to fix it is straightforward: teach PF not to use deprecated
addresses for NAT. Unfortunately there is some pitfall, which we should keep
in mind as claudio@ correctly points out:

> What I'm unsure about is if
>    pass in to (em0)
> should match on deprecated addresses? I know that for nat-to and rdr-to
> they should no longer be used.

To understand the pitfall one has to look at how PF treats dynamic
addresses:
        pass ... nat-to (egress)
        pass in to (em0)

The (egress) is group of interfaces, which serve default routes. PF treats
group as radix table of IP addresses assigned to interfaces in group. The
parentheses tell PF to update the group at runtime as interfaces and addresses
come up and down. The (em0) is similar, the table contains all IP addresses
bound to em0 interface. The table is also updated at runtime as addresses
come up and down.

As florian@ points out, we want let PF to match deprecated addresses
for inbound packets:

>> What I'm unsure about is if
>>     pass in to (em0)
>
> I'm positive this should match deprecated addresses. I'd argue it
> should match all addresses, including ones marked tentative,
> duplicated and invalid (vltime == 0). The invalid ones will quickly
> disapear from the interface.
>
>> should match on deprecated addresses? I know that for nat-to and rdr-to
>> they should no longer be used.
>
> For nat-to and rdr-to we should only use preferred addresses and
> additionally avoid privacy addresses.

The correct solution requires us to introduce yet another radix
table, which will keep dynamic addresses for NAT. This looks
like too much effort to fix rather some rare corner case.

So I wonder how far we can get with simple idea to remove deprecated
addresses from dynamic tables. The solution is not correct, but seems
to be a sufficient approximation.  It should work reasonably well for
stateful rules. The inbound packets with now deprecated destination
address will be allowed iff they will match existing state.

I have no IPv6 set up by hand to give my change a try to see/evaluate
an impact. I'd like to ask for testing and eventual OK.

thanks and
regards
sasha

--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf_if.c b/sys/net/pf_if.c
index 827ecb097d7..a7075722908 100644
--- a/sys/net/pf_if.c
+++ b/sys/net/pf_if.c
@@ -50,6 +50,7 @@
 #include <netinet/in.h>
 #include <netinet/ip.h>
 #include <netinet/ip_var.h>
+#include <netinet6/in6_var.h>
 
 #include <net/pfvar.h>
 
@@ -523,8 +524,18 @@ pfi_instance_add(struct ifnet *ifp, u_int8_t net, int 
flags)
                        pfi_address_add(ifa->ifa_broadaddr, af, net2);
                else if (flags & PFI_AFLAG_PEER)
                        pfi_address_add(ifa->ifa_dstaddr, af, net2);
-               else
+               else {
+                       /*
+                        * Do not insert deprecated IPv6 addresses
+                        */
+                       if (af == AF_INET6) {
+                               struct in6_ifaddr       *ia6 = ifatoia6(ifa);
+
+                               if (ia6->ia6_flags & IN6_IFF_DEPRECATED)
+                                       continue;
+                       }
                        pfi_address_add(ifa->ifa_addr, af, net2);
+               }
        }
 }
 
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index f11c067fbbd..c20af6ec6b1 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -484,6 +484,7 @@ void
 nd6_expire(void *unused)
 {
        struct ifnet *ifp;
+       int     do_update = 0;
 
        KERNEL_LOCK();
        NET_LOCK();
@@ -500,13 +501,18 @@ nd6_expire(void *unused)
                        if (IFA6_IS_INVALID(ia6)) {
                                in6_purgeaddr(&ia6->ia_ifa);
                        } else {
-                               if (IFA6_IS_DEPRECATED(ia6))
+                               if (IFA6_IS_DEPRECATED(ia6)) {
                                        ia6->ia6_flags |= IN6_IFF_DEPRECATED;
+                                       do_update = 1;
+                               }
                                nd6_expire_timer_update(ia6);
                        }
                }
        }
 
+       if (do_update == 1)
+               dohooks(ifp->if_addrhooks, 0);
+
        NET_UNLOCK();
        KERNEL_UNLOCK();
 }

Reply via email to