On Tue, Jan 26, 2021 at 12:33:25PM +0100, Alexander Bluhm wrote:
> On Tue, Jan 26, 2021 at 10:39:30AM +1000, David Gwynne wrote:
> > > But what about dup-to? The packet is duplicated for both directions.
> > > I guess the main use case for dup-to is implementing a monitor port.
> > > There you have to pass packets stateless, otherwise it would not
> > > work anyway. The strange semantics is not related to this diff.
> >
> > are you saying i should skip pf_test for all dup-to generated packets?
>
> I am not sure.
>
> When we have an in dup-to rule, the incoming packets in request
> direction are dupped and tested with the out ruleset. The reply
> packets for this state are also dupped, but not tested when they
> leave the dup interface.
>
> This is inconsistent and cannot work statefully. Stateful filtering
> with dupped packets does not make sense anyway. The only working
> config is "pass out on dup-interface no state".
>
> Do we think this rule should be required?
dup-to is tricky. In general you should run the collector on its own
interface and then 'set skip on $dupif' could work. I would assume that
the copy of the packet is bypassing pf and is just sent directly without
hitting pf_test again. At least for route-to & reply-to I would expect
this behaviour (like I do not expect that I need an extra pass rule to
allow a rdr-to through).
> 1. No packet should leave an interface without a rule.
>
> if (pd->dir == PF_IN || s->rt == PF_DUPTO) {
> if (pf_test(AF_INET, PF_OUT, ifp, &m0) != PF_PASS)
>
> 2. The config says we want a monitor port. We risk that the
> original packet and the dupped packet match the same rule.
> Stateful filtering cannot work, we do not expect reply packets
> for the dups.
>
> if (pd->dir == PF_IN && s->rt != PF_DUPTO) {
> if (pf_test(AF_INET, PF_OUT, ifp, &m0) != PF_PASS)
I guess this is for the case where route-to is used in PF_IN. I agree this
should be done so that the state table is properly set. Skipping this for
the copy of dup-to packets makes sense.
Running pf_test() for the same mbuf and direction but with different ifp
is causing more harm the good.
> 3. Some sort of problem was there before, but different. Don't
> address it now.
>
> Maybe 2 has less impact for the users and is easy to understand.
> We should document that in the man page.
>
> > > We are reaching a state where this diff can go in. I just startet
> > > a regress run with it. OK bluhm@
> >
> > hopefully i fixed the pfctl error messages up so the regress tests arent
> > too unhappy.
>
> pf forward and pf fragment tests pass. They include route-to and
> reply-to rules. I have no test for dup-to. Regress pfctl fails,
> but I think dlg@ has a diff for that.
>
> bluhm
>
--
:wq Claudio
