Re: [PATCH nft 2/3] netlink_linearize: incorrect netlink bytecode with binary operation and flags

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

 



On Wed, 28 Jul 2021 at 05:05, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>
> On Wed, Jul 28, 2021 at 02:36:18AM +0800, Tom Yan wrote:
> > Hmm, that means `tcp flags & (fin | syn | rst | ack) syn` is now
> > equivalent to 'tcp flags & (fin | syn | rst | ack) == syn'.
>
> Yes, those two are equivalent.
>
> > Does that mean `tcp flags syn` (was supposed to be and) is now
> > equivalent to `tcp flags == syn`
>
> tcp flag syn
>
> is a shortcut to match on the syn bit regarless other bit values, it's
> a property of the bitmask datatypes.
>
> tcp flags == syn
>
> is an exact match, it checks that the syn bit is set on.
>
> > instead of `tcp flags & syn == syn` / `tcp flags & syn != 0`?
>
> these two above are equivalent, I just sent a patch to fix the
> tcp flags & syn == syn case.
>
> > Suppose `tcp flags & syn != 0` should then be translated to `tcp flags
> > syn / syn` instead, please note that while nft translates `tcp flags &
> > syn == syn` to `tcp flags syn / syn`, it does not accept the
> > translation as input (when the mask is not a comma-separated list):
> >
> > # nft --debug=netlink add rule meh tcp_flags 'tcp flags syn / syn'
> > Error: syntax error, unexpected newline, expecting comma
> > add rule meh tcp_flags tcp flags syn / syn
> >                                           ^
>
> The most simple way to express this is: tcp flags == syn.
>
> > Also, does that mean `tcp flags & (fin | syn | rst | ack) fin,syn,ack`
> > will now be equivalent to `tcp flags & (fin | syn | rst | ack) = (fin
> > | syn | ack)`
>
> Yes, those two are equivalent. This is the same example as the one you
> have used at the beginning of this email.
>
> > instead of (ultimately) `tcp flags & (fin | syn | ack)  != 0`?
>
> That's equivalent to:
>
> tcp flags fin,syn,ack
>
> A quick summary:
>
> - If you want an exact match:
>
> tcp flags == fin,syn,ack
>
> - If you want to check that those three bits are set on (regardless
>   the remaining bits):
>
> tcp flags fin,syn,ack / fin,syn,ack
>
> - If you want to check that any of these three bits is set on:
>
> tcp flags fin,syn,ack

This is exactly what I find absurd btw. IMHO it's much better if the
latter just means `tcp flags == (fin | syn | ack)`. I'd rather we keep
`tcp flags & (fin | syn | ack) != 0` and so "unsimplified" or accept
something like `tcp flags { fin / fin, syn / syn, ack / ack }`, if
necessary at all. I think being "obvious" / "unambiguous" /
"straight-forward" enough is much more important than being (too)
"neat".

>
> > Which means `tcp flags & (fin | syn | ack) != 0` should not be
> > translated to `tcp flags fin,syn,ack`?
>
> tcp flags & (fin | syn | ack) != 0 is checking for any of these three
> bits to be set on, this translation is correct.



[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux