Phil Sutter <phil@xxxxxx> wrote:
> Legacy ebtables supports policies for user-defined chains - and what's
> worse, they default to ACCEPT unlike anywhere else. So lack of support
> for this braindead feature in ebtables-nft is actually a change of
> behaviour which very likely affects all ebtables users out there.
>
> The solution implemented here uses an implicit (and transparent) last
> rule in all user-defined ebtables-nft chains with policy other than
> RETURN. This rule is identified by an nft comment
> "XTABLES_EB_INTERNAL_POLICY_RULE" (since commit ccf154d7420c0 ("xtables:
> Don't use native nftables comments") nft comments are not used
> otherwise).
>
> To minimize interference with existing code, this policy rule is removed
> from chains during cache population and the policy is saved in
> NFTNL_CHAIN_POLICY attribute. When committing changes to the kernel,
> nft_commit() traverses through the list of chains and (re-)creates
> policy rules if required.
>
> In ebtables-nft-restore, table flushes are problematic. To avoid weird
> kernel error responses, introduce a custom 'table_flush' callback which
> removes any pending policy rule add/remove jobs prior to creating the
> NFT_COMPAT_TABLE_FLUSH one.
>
> I've hidden all this mess behind checks for h->family, so hopefully
> impact on {ip,ip6,arp}tables-nft should be negligible.
>
> Signed-off-by: Phil Sutter <phil@xxxxxx>
> ---
> iptables/nft-bridge.c | 2 +-
> iptables/nft.c | 215 +++++++++++++++++-
> iptables/nft.h | 4 +
> .../ebtables/0002-ebtables-save-restore_0 | 7 +
> iptables/xtables-eb.c | 20 +-
> iptables/xtables-restore.c | 23 +-
> 6 files changed, 249 insertions(+), 22 deletions(-)
>
> diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
> index 7c390dfa2a898..72c89987a07f2 100644
> --- a/iptables/nft-bridge.c
> +++ b/iptables/nft-bridge.c
> @@ -358,7 +358,7 @@ static void nft_bridge_print_header(unsigned int format, const char *chain,
> bool basechain, uint32_t refs, uint32_t entries)
> {
> printf("Bridge chain: %s, entries: %u, policy: %s\n",
> - chain, entries, basechain ? pol : "RETURN");
> + chain, entries, pol);
This causes
Bridge chain: FOOBAR, entries: 0, policy: (null)
after "ebtables -P FOOBAR RETURN".
Reverting this hunk shows "RETURN" as expected.
Doing "ebtables -P FOOBAR DROP" sets a hidden rule, but its still shown
as ACCEPT. I'll debiug this further.
plain nft shows:
ether type 0x0000 counter packets 0 bytes 0 drop comment "XTABLES_EB_INTERNAL_POLICY_RULE"
This "ether type" is bonkers as well, it should not be there.
Looks like ebt_add_policy_rule needs to set "cs.eb.bitmask = EBT_NOPROTO".
> + while (expr != NULL) {
> + if (strcmp("immediate",
> + nftnl_expr_get_str(expr, NFTNL_EXPR_NAME))) {
> + expr = nftnl_expr_iter_next(iter);
> + continue;
> + }
The imm should come first, in case there is a different expr this isn't
an "implicit policy" (because its not unconditional verdict).
Should perhaps also check nftnl_expr_is_set(expr, NFTNL_EXPR_IMM_VERDICT)
so we really don't fall for random immediates.
Still reviewing the rest.
