Re: [PATCH V2 iptables] extensions: libxt_connlabel: Add translation to nft

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > How so?
> 
> So this is there just to cover the fail the ENOSPC when setting label?

No label extension present or skb->nfct is untracked.
-m label --label bit40 will never match if the packet has no conntrack
attached.

"-m label --label bit40 --set" will behave the same in that case.

I would really prefer to expose this 1:1 in the translation
because it matches the behaviour.

Users that don't care about success can always just
"ct label set foo".

> This internal behaviour in xt connlabel seems confusing to me, this
> rule:
> 
>         iptables -A INPUT -m connlabel ! --label bit40 --set
> 
> following the reading from left to right convention tells me:
> 
>         if not bit40 set, then set it.

If not set, then *try* to set it:

if (ct == NULL || nf_ct_is_untracked(ct))
 return invert;

if (info->options & XT_CONNLABEL_OP_SET)
  return (nf_connlabel_set(ct, info->bit) == 0) ^ invert;

return connlabel_match(ct, info->bit) ^ invert;

> But this is actually setting in first place inconditionally, then
> checking this is not set, what is the use case for this?

The xt module doesn't have to recheck, if nf_connlabel_set returns 0
then the bit will be set.

> Actually the kernel code first sets the bit, then checks if this is
> unset for this. Note iptables-save displays this in that way as
> output.
> 
> You can probably introduce in iptables something like:
> 
>         iptables -A INPUT -m connlabel --set-label bit40

This is identical to

iptables -A INPUT -m connlabel --label bit40 --set

... unless you meant that this "--set-label bit40" should always return
true even if skb->nfct is NULL, but that seems wrong to me.

It would be more xtables-style to add
-j CONNLABEL --set-bit40

[ i.e. XT_CONTINUE regardless if we could set anything ]

But that doesn't make xtables any better and provides no benefit to
end users.