pf: Don't pfsync states with unrecoverable routing information

[vegeta_tuxpowered.net]

Summary

States created by route-to rules can't be trusted when received with
pfsync version 1301 as they lack the rt and rt_kif information. They
are imported, though, and pf_route() function attempts to recover
the missing information for every forwarded packet.

Move the recovery operation to pfsync_state_import() so that it's
performed only once and if it's impossible don't import the state.
Add an additional check for cases when recovery might produce wrong
results.

[vegeta_tuxpowered.net]

sys/netpfil/pf/pf.c

7729

Can ifp even be NULL? Can we instead KASSERT it?

[vegeta_tuxpowered.net]

Improve some comments.

[kp]

LGTM. Approved regardless of how you choose to deal with the remarks I've raised.

And I'm delighted to see the extra test cases.

sys/netpfil/pf/if_pfsync.c

113

We have a define like that one in a number of files (pf.c, pf_lb.c, pf_ruleset.c). We should probably move a single definition into pfvar.h.

That's independent of this patch though. We can either do that first, or after this one.

sys/netpfil/pf/pf.c

7705–7706

That's not going to be true for nat64 pf_route/pf_route6 calls, so I'm not sure it's worth adding an invariant that is pretty much immediately going to be removed again.
On the other hand, it is correct within this patch.

I really need to just land those patches, because I keep having to rebase them on top of your work :)

7729

It can be, yes.

It's possible for a rule to reference an interface that doesn't exist. In that case we'd have a struct pfi_kkif (so pd->act.rt_kif can be non-null), but not a struct ifnet pointer from there (so pfik_ifp == NULL).