On Mon, Jan 25, 2021 at 01:11:35PM +0100, Alexandr Nedvedicky wrote:
> Hello,
>
> </snip>
> > >
> > > I understand that simple is better here, so I won't object
> > > if we will lean towards simplified model above. However I still
> > > would like to share my view on current PF.
> > >
> > > the way I understand how things (should) work currently is fairly
> > > simple:
> > >
> > > we always run pf_test() as packet crosses interface.
> > > packet can cross interface either in outbound or
> > > inbound direction.
> >
> > That's how I understand the current code. I'm proposing that we change
> > the semantics so they are:
> >
> > - we always run pf_test as a packet enters or leaves the network stack.
> > - pf is able to filter or apply policy based on various attributes
> > of the packet such as addresses and ports, but also metadata about
> > the packet such as the current prio, or the interface it came
> > from or is going to.
> > - changing a packet or it's metadata does not cause a rerun of pf_test.
> > - route-to on an incoming packet basically bypasses the default
> > stack processing with a "fast route" out of the stack.
> >
> > > this way we can always create a complex route-to loops,
> > > however it can also solve some route-to vs. NAT issues.
> > > consider those fairly innocent rules:
> > >
> > > --------8<---------------8<---------------8<------------------8<--------
> > > table <hops> { 10.10.10.10, 172.16.1.1 }
> > >
> > > pass out on em0 from 192.168.1.0/24 to any route-to <hops>
> > > pass out on em1 from 192.168.1.0 to any nat-to (em1)
> > > pass out on em2 all
> > > --------8<---------------8<---------------8<------------------8<--------
> > >
> > > Rules above should currently work, but will stop if we will
> > > go with simplified model.
> >
> > The entries in <hops> make the packet go out em1 and em2?
>
> yes they do. let's say 10.10.10.10 is reached over em1, 172.16.1.1 is
> reached over em2. sorry I have not specified that in my earlier email.
npz.
>
> >
> > I'm ok with breaking configs like that. We don't run pf_test again for
> > other changes to the packet, so if we do want to support something like
> > that I think we should make the following work:
> >
> > # pf_pdesc kif is em0
> > match out on em0 from 192.168.1.0/24 to any route-to <hops>
> > # pf_pdesc kif is now em1
> > pass out on em1 from 192.168.1.0 to any nat-to (em1)
> > pass out on em2 all
> >
> > This is more in line with how NAT rules operate.
>
> If I understand the idea right, then basically 'match out on em0....'
> figures out the new 'outbound interface' so either 'pass out on em1...' or
> 'pass out on em2...' will kick in. In other words:
>
> depending on the destination picked up from <hops> table,
> the route-to action will override the em0 interface to
> either em1 or em2.
yes.
i dont understand the kif lifetimes though. can we just point a
pdesc at an arbitrary kif or do we need ot reference count?
> I think this might be way to go.
>
> My only concern now is that such change is subtle. I mean the
>
> pass out ... route-to <hops>
>
> will change behavior in sense that current code will dispatch
> packet to new interface and run pf_test() again. Once your diff
> will be in the same rule will be accepted, but will bring entirely
> different behaviour: just dispatch packet to new interface.
yeah.
the counter example is that i was extremely surprised when i
discovered that pf_test gets run again when the outgoing interface
is changed with a route-to rule.
there's subtlety either way, we're just figuring out which one we're
going for.
> > > I'll be OK with your simplified model if it will make things
> > > more explicit:
> > >
> > > route-to option should be applied on inbound rules
> > > only
> >
> > This would restrict how we currently write rules. See below about how we
> > would be using it.
> >
> > > reply-to option should be applied on outbound rule
> > > only
> >
> > I'm using reply-to on inbound rules. On these boxes I have a service
> > (it's a dns resolver running unbound) that is accessible only via
> > gre(4) tunnels, and I need the replies to those connections to go
> > out the same interface they came in on. I'm running an older version of
> > my diff, so I can have rules like this to make it work:
> >
> > pass in quick on gre0 reply-to gre0:peer
> > pass in quick on gre1 reply-to gre1:peer
> >
> > The DNS traffic isn't going through this box, the replies that
> > unbound is generating match the state created by the inbound rule.
> >
> > If I'm remembering correctly, sthen@ had a similar use case.
>
> you are right, I did not think much of a local bound traffic.
> in this case reply-to needs to be kept as-is.
>
> >
> > > dup-to option can go either way (in/out)
> >
> > Yep.
> >
> > > does it make sense? IMO yes, because doing route-to
> > > on outbound path feels unnatural to me.
> >
> > I agree that it feels a bit unnatural, but so far all the route-to rules
> > I've been writing have been on pass out rules. That could be peculiar to
> > my setup, but we generally allow packets in on our external links, and
> > apply policy on the outbound interface heading towards the relevant
> > service. eg:
> >
> > block
> > pass in on $if_external
> > pass out on $if_webservers proto tcp to port { http https }
> > pass out on $if_relays proto { tcp udp } to port domain
> >
> > We'd be sprinkling route-to on these pass out rules to tie connections
> > to specific backends.
> >
> > >
> > > </snip>
> > >
> > > >
> > > > this also breaks the ability to do route-to without states. is there a
> > > > reason to do that apart from the DSR type things? did we agree that
> > > > those use cases could be handled by sloppy states instead?
> > >
> > > If I remember correct we need to make 'keep state' mandatory
> > > for route-to so it can work well with pfsync(4), right?
> >
> > That's correct.
>
> I think this is acceptable. If this will cause a friction we can always
> adjust the code in follow up commit to allow state-less route-to/reply-to
> with no support from pfsync(4).
if we're going to support route-to on match rules i think this will be
easy to implement.
> >
> > > >
> > > > lastly, the "argument" or address specified with route-to (and
> > > > reply-to and dup-to) is a destination address, not a next-hop. this
> > > > has been discussed on the lists a couple of times before, so i won't
> > > > go over it again, except to reiterate that it allows pf to force
> > > > "sticky" path selection while opening up the possibility for ecmp
> > > > and failover for where that path traverses.
> > >
> > > I keep forgetting about it as I still stick to current interpretation.
> > >
> > >
> > > I've seen changes to pfctl. Diff below still allows rule:
> > >
> > > pass in on net0 from 192.168.1.0/24 to any route-to 10.10.10.10@em0
> >
> > Is there use case for the @interface syntax apart from the current
> > route-to rules? If not, we can just delete it.
>
> perhaps I'm still not quite on the same page as you then. I also
> had no time to entirely test you diff. The way I understand your
> effort is to change route-to behavior such it will be using
> a destination instead of next-hop@interface. Or are you planning
> to keep current form ('route-to next-hop@interface') working?
if we ignore route-to, what's the use case for the interface part of
address@interface? it doesnt seem to be accepted as part of an address
in other parts of the grammar:
dlg@kbuild ~$ echo pass in from 192.168.0.0@vmx0 | sudo pfctl -nf -
stdin:1: @if syntax not permitted in from or to
stdin:1: skipping rule due to errors
stdin:1: rule expands to no valid combination
dlg@kbuild ~$ echo pass from 192.168.0.0@vmx0 | sudo pfctl -nf -
stdin:1: @if syntax not permitted in from or to
stdin:1: skipping rule due to errors
stdin:1: rule expands to no valid combination
dlg@kbuild ~$ echo pass nat-to 192.168.0.0@vmx0 | sudo pfctl -nf -
stdin:1: @if not permitted
stdin:1: nat-to and rdr-to require a direction
stdin:1: skipping rule due to errors
stdin:1: rule expands to no valid combination
dlg@kbuild ~$ echo pass nat-to 192.168.0.0@vmx0 | sudo pfctl -nf -
if route-to isn't going to use it, can we cut the @interface part out of
fthe address parser?
> > > it also allows rule:
> > >
> > > pass in on net0 from 192.168.1.0/24 to any route-to em0
> > >
> > > I think we don't want support those two anymore, is that correct?
> >
> > em0 gets resolved to the addresses on the interface. It's a silly
> > config, but it's not wrong.
> >
> > $ echo pass in on vmx0 from 192.168.1.0/24 to any route-to vmx0 | pfctl
> > -vnf -
> > pass in on vmx0 inet from 192.168.1.0/24 to any flags S/SA route-to
> > 192.0.2.34
> >
> > It does raise the question of what pf_route should do if it resolves
> > something with RTF_LOCAL set. Or RTF_BLACKHOLE and RTF_REJECT for that
> > matter.
> >
>
> unless we don't want to support yet another way of doing block/block
> return
> we should fail with error, I think.
ok. i'll update my diff to have pf_route{,6} check these flags tomorrow.
cheers,
dlg