@@ -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 ?: "RETURN");
}
static void print_matches_and_watchers(const struct iptables_command_state *cs,
@@ -55,6 +55,7 @@
#include "nft.h"
#include "xshared.h" /* proto_to_name */
#include "nft-shared.h"
+#include "nft-bridge.h" /* EBT_NOPROTO */
#include "xtables-config-parser.h"
static void *nft_fn;
@@ -1322,6 +1323,86 @@ retry:
return ret;
}
+#define EBT_POLICY_COMMENT "XTABLES_EB_INTERNAL_POLICY_RULE"
+
+static bool nft_rule_is_policy_rule(struct nftnl_rule *r)
+{
+ const void *data;
+ char *comment;
+ uint32_t len;
+
+ if (!nftnl_rule_is_set(r, NFTNL_RULE_USERDATA))
+ return false;
+
+ data = nftnl_rule_get_data(r, NFTNL_RULE_USERDATA, &len);
+ comment = get_comment(data, len);
+ if (!comment || strcmp(comment, EBT_POLICY_COMMENT))
+ return false;
+
+ return true;
+}
+
+static struct nftnl_rule *nft_chain_last_rule(struct nftnl_chain *c)
+{
+ struct nftnl_rule *r = NULL, *last;
+ struct nftnl_rule_iter *iter;
+
+ iter = nftnl_rule_iter_create(c);
+ if (!iter)
+ return NULL;
+
+ do {
+ last = r;
+ r = nftnl_rule_iter_next(iter);
+ } while (r);
+ nftnl_rule_iter_destroy(iter);
+
+ return last;
+}
+
+static void nft_bridge_chain_postprocess(struct nft_handle *h,
+ struct nftnl_chain *c)
+{
+ struct nftnl_rule *last = nft_chain_last_rule(c);
+ struct nftnl_expr_iter *iter;
+ struct nftnl_expr *expr;
+ int verdict;
+
+ if (!last || !nft_rule_is_policy_rule(last))
+ return;
+
+ iter = nftnl_expr_iter_create(last);
+ if (!iter)
+ return;
+
+ expr = nftnl_expr_iter_next(iter);
+ if (!expr ||
+ strcmp("counter", nftnl_expr_get_str(expr, NFTNL_EXPR_NAME)))
+ goto out_iter;
+
+ expr = nftnl_expr_iter_next(iter);
+ if (!expr ||
+ strcmp("immediate", nftnl_expr_get_str(expr, NFTNL_EXPR_NAME)) ||
+ !nftnl_expr_is_set(expr, NFTNL_EXPR_IMM_VERDICT))
+ goto out_iter;
+
+ verdict = nftnl_expr_get_u32(expr, NFTNL_EXPR_IMM_VERDICT);
+ switch (verdict) {
+ case NF_ACCEPT:
+ case NF_DROP:
+ break;
+ default:
+ goto out_iter;
+ }
+
+ nftnl_chain_set_u32(c, NFTNL_CHAIN_POLICY, verdict);
+ if (batch_rule_add(h, NFT_COMPAT_RULE_DELETE, last) < 0)
+ fprintf(stderr, "Failed to delete old policy rule\n");
+ nftnl_chain_rule_del(last);
+out_iter:
+ nftnl_expr_iter_destroy(iter);
+}
+
static int nftnl_rule_list_cb(const struct nlmsghdr *nlh, void *data)
{
struct nftnl_chain *c = data;
@@ -1375,6 +1456,10 @@ retry:
}
nftnl_rule_free(rule);
+
+ if (h->family == NFPROTO_BRIDGE)
+ nft_bridge_chain_postprocess(h, c);
+
return 0;
}
@@ -1440,6 +1525,15 @@ int nft_chain_save(struct nft_handle *h, struct nftnl_chain_list *list)
if (nftnl_chain_get(c, NFTNL_CHAIN_POLICY))
pol = nftnl_chain_get_u32(c, NFTNL_CHAIN_POLICY);
policy = policy_name[pol];
+ } else if (h->family == NFPROTO_BRIDGE) {
+ if (nftnl_chain_is_set(c, NFTNL_CHAIN_POLICY)) {
+ uint32_t pol;
+
+ pol = nftnl_chain_get_u32(c, NFTNL_CHAIN_POLICY);
+ policy = policy_name[pol];
+ } else {
+ policy = "RETURN";
+ }
}
if (ops->save_chain)
@@ -1634,6 +1728,8 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl
nftnl_chain_set(c, NFTNL_CHAIN_TABLE, (char *)table);
nftnl_chain_set(c, NFTNL_CHAIN_NAME, (char *)chain);
+ if (h->family == NFPROTO_BRIDGE)
+ nftnl_chain_set_u32(c, NFTNL_CHAIN_POLICY, NF_ACCEPT);
ret = batch_chain_add(h, NFT_COMPAT_CHAIN_USER_ADD, c);
@@ -2275,7 +2371,6 @@ static void __nft_print_header(struct nft_handle *h,
struct nftnl_chain *c, unsigned int format)
{
const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
- uint32_t policy = nftnl_chain_get_u32(c, NFTNL_CHAIN_POLICY);
bool basechain = !!nftnl_chain_get(c, NFTNL_CHAIN_HOOKNUM);
uint32_t refs = nftnl_chain_get_u32(c, NFTNL_CHAIN_USE);
uint32_t entries = nft_rule_count(h, c);
@@ -2283,8 +2378,12 @@ static void __nft_print_header(struct nft_handle *h,
.pcnt = nftnl_chain_get_u64(c, NFTNL_CHAIN_PACKETS),
.bcnt = nftnl_chain_get_u64(c, NFTNL_CHAIN_BYTES),
};
+ const char *pname = NULL;
+
+ if (nftnl_chain_is_set(c, NFTNL_CHAIN_POLICY))
+ pname = policy_name[nftnl_chain_get_u32(c, NFTNL_CHAIN_POLICY)];
- ops->print_header(format, chain_name, policy_name[policy],
+ ops->print_header(format, chain_name, pname,
&ctrs, basechain, refs - entries, entries);
}
@@ -2668,8 +2767,111 @@ static int nft_action(struct nft_handle *h, int action)
return ret == 0 ? 1 : 0;
}
+static int ebt_add_policy_rule(struct nftnl_chain *c, void *data)
+{
+ uint32_t policy = nftnl_chain_get_u32(c, NFTNL_CHAIN_POLICY);
+ struct iptables_command_state cs = {
+ .eb.bitmask = EBT_NOPROTO,
+ };
+ struct nftnl_udata_buf *udata;
+ struct nft_handle *h = data;
+ struct nftnl_rule *r;
+ const char *pname;
+
+ if (nftnl_chain_get(c, NFTNL_CHAIN_HOOKNUM))
+ return 0; /* ignore base chains */
+
+ if (!nftnl_chain_is_set(c, NFTNL_CHAIN_POLICY))
+ return 0;
+
+ nftnl_chain_unset(c, NFTNL_CHAIN_POLICY);
+
+ switch (policy) {
+ case NFT_RETURN:
+ return 0; /* return policy is default for nft chains */
+ case NF_ACCEPT:
+ pname = "ACCEPT";
+ break;
+ case NF_DROP:
+ pname = "DROP";
+ break;
+ default:
+ return -1;
+ }
+
+ command_jump(&cs, pname);
+
+ r = nft_rule_new(h, nftnl_chain_get_str(c, NFTNL_CHAIN_NAME),
+ nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE), &cs);
+ if (!r)
+ return -1;
+
+ udata = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
+ if (!udata)
+ return -1;
+
+ if (!nftnl_udata_put_strz(udata, UDATA_TYPE_COMMENT, EBT_POLICY_COMMENT))
+ return -1;
+
+ nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
+ nftnl_udata_buf_data(udata),
+ nftnl_udata_buf_len(udata));
+ nftnl_udata_buf_free(udata);
+
+ if (batch_rule_add(h, NFT_COMPAT_RULE_APPEND, r) < 0) {
+ nftnl_rule_free(r);
+ return -1;
+ }
+
+ return 0;
+}
+
+int ebt_set_user_chain_policy(struct nft_handle *h, const char *table,
+ const char *chain, const char *policy)
+{
+ struct nftnl_chain *c = nft_chain_find(h, table, chain);
+ int pval;
+
+ if (!c)
+ return 0;
+
+ if (!strcmp(policy, "DROP"))
+ pval = NF_DROP;
+ else if (!strcmp(policy, "ACCEPT"))
+ pval = NF_ACCEPT;
+ else if (!strcmp(policy, "RETURN"))
+ pval = NFT_RETURN;
+ else
+ return 0;
+
+ nftnl_chain_set_u32(c, NFTNL_CHAIN_POLICY, pval);
+ return 1;
+}
+
+static void nft_bridge_commit_prepare(struct nft_handle *h)
+{
+ const struct builtin_table *t;
+ struct nftnl_chain_list *list;
+ int i;
+
+ for (i = 0; i < NFT_TABLE_MAX; i++) {
+ t = &h->tables[i];
+
+ if (!t->name)
+ continue;
+
+ list = h->table[t->type].chain_cache;
+ if (!list)
+ continue;
+
+ nftnl_chain_list_foreach(list, ebt_add_policy_rule, h);
+ }
+}
+
int nft_commit(struct nft_handle *h)
{
+ if (h->family == NFPROTO_BRIDGE)
+ nft_bridge_commit_prepare(h);
return nft_action(h, NFT_COMPAT_COMMIT);
}
@@ -2678,6 +2880,27 @@ int nft_abort(struct nft_handle *h)
return nft_action(h, NFT_COMPAT_ABORT);
}
+int nft_abort_policy_rule(struct nft_handle *h, const char *table)
+{
+ struct obj_update *n, *tmp;
+
+ list_for_each_entry_safe(n, tmp, &h->obj_list, head) {
+ if (n->type != NFT_COMPAT_RULE_APPEND &&
+ n->type != NFT_COMPAT_RULE_DELETE)
+ continue;
+
+ if (strcmp(table,
+ nftnl_rule_get_str(n->rule, NFTNL_RULE_TABLE)))
+ continue;
+
+ if (!nft_rule_is_policy_rule(n->rule))
+ continue;
+
+ batch_obj_del(h, n);
+ }
+ return 0;
+}
+
int nft_compatible_revision(const char *name, uint8_t rev, int opt)
{
struct mnl_socket *nl;
@@ -137,6 +137,7 @@ uint32_t nft_invflags2cmp(uint32_t invflags, uint32_t flag);
*/
int nft_commit(struct nft_handle *h);
int nft_abort(struct nft_handle *h);
+int nft_abort_policy_rule(struct nft_handle *h, const char *table);
/*
* revision compatibility.
@@ -203,4 +204,7 @@ void nft_rule_to_arpt_entry(struct nftnl_rule *r, struct arpt_entry *fw);
bool nft_is_table_compatible(struct nft_handle *h, const char *name);
+int ebt_set_user_chain_policy(struct nft_handle *h, const char *table,
+ const char *chain, const char *policy);
+
#endif
@@ -50,6 +50,9 @@ $XT_MULTI ebtables -A foo --pkttype-type multicast --limit 100 -j ACCEPT
$XT_MULTI ebtables -A FORWARD -j foo
+$XT_MULTI ebtables -N bar
+$XT_MULTI ebtables -P bar RETURN
+
$XT_MULTI ebtables -t nat -A PREROUTING --redirect-target ACCEPT
#$XT_MULTI ebtables -t nat -A PREROUTING --to-src fe:ed:ba:be:00:01
@@ -59,6 +62,8 @@ $XT_MULTI ebtables -t nat -P OUTPUT DROP
$XT_MULTI ebtables -t nat -A POSTROUTING -j ACCEPT
#$XT_MULTI ebtables -t nat -A POSTROUTING --to-dst fe:ed:ba:be:00:01 --dnat-target ACCEPT
+$XT_MULTI ebtables -t nat -N nat_foo -P DROP
+
# compare against stored ebtables dump
DUMP='*filter
@@ -66,6 +71,7 @@ DUMP='*filter
:FORWARD DROP
:OUTPUT ACCEPT
:foo ACCEPT
+:bar RETURN
-A INPUT -p IPv4 -i lo -j ACCEPT
-A FORWARD -j foo
-A OUTPUT -s Broadcast -j DROP
@@ -98,6 +104,7 @@ DUMP='*filter
:PREROUTING ACCEPT
:OUTPUT DROP
:POSTROUTING ACCEPT
+:nat_foo DROP
-A PREROUTING -j redirect
-A OUTPUT -j ACCEPT
-A POSTROUTING -j ACCEPT
@@ -800,7 +800,6 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table,
case 'E': /* Rename chain */
case 'X': /* Delete chain */
/* We allow -N chainname -P policy */
- /* XXX: Not in ebtables-compat */
if (command == 'N' && c == 'P') {
command = c;
optind--; /* No table specified */
@@ -1225,17 +1224,16 @@ print_zero:
if (command == 'P') {
if (selected_chain < 0) {
- xtables_error(PARAMETER_PROBLEM,
- "Policy %s not allowed for user defined chains",
- policy);
- }
- if (strcmp(policy, "RETURN") == 0) {
- xtables_error(PARAMETER_PROBLEM,
- "Policy RETURN only allowed for user defined chains");
+ ret = ebt_set_user_chain_policy(h, *table, chain, policy);
+ } else {
+ if (strcmp(policy, "RETURN") == 0) {
+ xtables_error(PARAMETER_PROBLEM,
+ "Policy RETURN only allowed for user defined chains");
+ }
+ ret = nft_chain_set(h, *table, chain, policy, NULL);
+ if (ret < 0)
+ xtables_error(PARAMETER_PROBLEM, "Wrong policy");
}
- ret = nft_chain_set(h, *table, chain, policy, NULL);
- if (ret < 0)
- xtables_error(PARAMETER_PROBLEM, "Wrong policy");
} else if (command == 'L') {
ret = list_rules(h, chain, *table, rule_nr,
0,
@@ -226,14 +226,20 @@ void xtables_restore_parse(struct nft_handle *h,
curtable->name, chain);
} else if (cb->chain_user_add &&
cb->chain_user_add(h, chain,
- curtable->name) < 0) {
- if (errno == EEXIST)
- continue;
-
+ curtable->name) < 0 &&
+ errno != EEXIST) {
xtables_error(PARAMETER_PROBLEM,
"cannot create chain "
"'%s' (%s)\n", chain,
strerror(errno));
+ } else if (h->family == NFPROTO_BRIDGE &&
+ !ebt_set_user_chain_policy(h, curtable->name,
+ chain, policy)) {
+ xtables_error(OTHER_PROBLEM,
+ "Can't set policy `%s'"
+ " on `%s' line %u: %s\n",
+ policy, chain, line,
+ ops->strerror(errno));
}
ret = 1;
} else if (in_table) {
@@ -462,11 +468,18 @@ int xtables_ip6_restore_main(int argc, char *argv[])
argc, argv);
}
+static int ebt_table_flush(struct nft_handle *h, const char *table)
+{
+ /* drop any pending policy rule add/removal jobs */
+ nft_abort_policy_rule(h, table);
+ return nft_table_flush(h, table);
+}
+
struct nft_xt_restore_cb ebt_restore_cb = {
.chain_list = get_chain_list,
.commit = nft_commit,
.table_new = nft_table_new,
- .table_flush = nft_table_flush,
+ .table_flush = ebt_table_flush,
.chain_user_flush = nft_chain_user_flush,
.do_command = do_commandeb,
.chain_set = nft_chain_set,
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@nwl.cc> --- Changes since v1: - Set EBT_NOPROTO in policy rules to avoid generating spurious ether type match. - Fix RETURN policy printing for user-defined ebtables chains. - Switch from a per-rule approach to chain postprocessing after rules have been fetched. This way only the last rule is considered as policy rule, so if a user appended a rule to that chain using 'nft' command, chain will fall back to RETURN policy and the former policy rule stays in place. - Introduce helper nft_chain_last_rule() to aid in the above. - Improve policy rule parsing as suggested by Florian. - Introduce nft_bridge_commit_prepare() to outsource bridge-specific actions at nft_commit() time. Note that I didn't drop the policy comment. It is required by nft_abort_policy_rule() to identify suitable rule append/delete jobs. This is strictly not necessary, the function runs only prior to a table flush - so in theory one could just drop any rule append/delete jobs from the batch for that particular table. After that, policy rules would look just like regular ones also in 'nft' output. The downside is that a user could accidentally set the chain's policy this way since | ebtables-nft -A foo -j ACCEPT would yield identical results as: | ebtables-nft -P foo ACCEPT This seems unproblematic, but consider a stupid example: | ebtables-nft -N foo -P RETURN | ebtables-nft -A foo -j ACCEPT | ebtables-nft -A foo -j DROP So after the second command, chain foo has one rule which accepts everything. During the third command, that accept rule is turned into the policy and after it we have a chain with accept policy but a single rule which drops everything. That is at least a functional deviation from legacy ebtables, and a confusing one as well. :) --- iptables/nft-bridge.c | 2 +- iptables/nft.c | 227 +++++++++++++++++- iptables/nft.h | 4 + .../ebtables/0002-ebtables-save-restore_0 | 7 + iptables/xtables-eb.c | 20 +- iptables/xtables-restore.c | 23 +- 6 files changed, 264 insertions(+), 19 deletions(-)