* [nft PATCH] More extensive error checking on base chain
@ 2014-08-12 9:37 Yanchuan Nian
2014-08-14 18:59 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Yanchuan Nian @ 2014-08-12 9:37 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, Yanchuan Nian
The evaluation of base chain isn't extensive enough. It cannot
detect whether a chain type is supported or not in certain families.
It also cannot deletect whether a hook is supported or not in certain
chains. The evaluation is done by kernel, and the information is unclear.
nft> add chain arp arptable chain1 {type nat hook input priority 0 ;}
<cli>:1:1-64: Error: Could not add chain: No such file or directory
add chain arp arptable chain1 {type nat hook input priority 0 ;}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
nft> add chain ip iptable chain1 {type nat hook forward priority 0 ;}
<cli>:1:1-64: Error: Could not add chain: Operation not supported
add chain ip iptable chain1 {type nat hook forward priority 0 ;}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This patch performs more extensive error checking, so the information
is more clear.
nft> add chain arp arptable chain1 {type nat hook input priority 0 ;}
<cli>:1:31-63: Error: invalid type name nat
add chain arp arptable chain1 {type nat hook input priority 0 ;}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
nft> add chain ip iptable chain1 {type nat hook forward priority 0 ;}
<cli>:1:29-63: Error: invalid hook name forward
add chain ip iptable chain1 {type nat hook forward priority 0 ;}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Signed-off-by: Yanchuan Nian <ycnian@gmail.com>
---
include/rule.h | 2 -
src/evaluate.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++----------
src/parser.y | 28 ++-------
src/rule.c | 40 ------------
4 files changed, 168 insertions(+), 98 deletions(-)
diff --git a/include/rule.h b/include/rule.h
index db91406..8caf088 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -121,8 +121,6 @@ struct chain {
struct list_head rules;
};
-extern const char *chain_type_name_lookup(const char *name);
-extern const char *chain_hookname_lookup(const char *name);
extern struct chain *chain_alloc(const char *name);
extern void chain_free(struct chain *chain);
extern void chain_add_hash(struct chain *chain, struct table *table);
diff --git a/src/evaluate.c b/src/evaluate.c
index f66a8ea..d6129c7 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -14,8 +14,10 @@
#include <stdint.h>
#include <string.h>
#include <arpa/inet.h>
+#include <net/if.h>
#include <linux/netfilter.h>
#include <linux/netfilter_arp.h>
+#include <linux/netfilter_bridge.h>
#include <linux/netfilter/nf_tables.h>
#include <expression.h>
@@ -1309,36 +1311,156 @@ static int rule_evaluate(struct eval_ctx *ctx, struct rule *rule)
return 0;
}
-static uint32_t str2hooknum(uint32_t family, const char *hook)
+static const char *inet_hooks_str_array[NF_INET_NUMHOOKS] = {
+ [NF_INET_PRE_ROUTING] = "prerouting",
+ [NF_INET_LOCAL_IN] = "input",
+ [NF_INET_FORWARD] = "forward",
+ [NF_INET_LOCAL_OUT] = "output",
+ [NF_INET_POST_ROUTING] = "postrouting",
+};
+
+static const char *bridge_hooks_str_array[NF_BR_NUMHOOKS] = {
+ [NF_BR_PRE_ROUTING] = "prerouting",
+ [NF_BR_LOCAL_IN] = "input",
+ [NF_BR_FORWARD] = "forward",
+ [NF_BR_LOCAL_OUT] = "output",
+ [NF_BR_POST_ROUTING] = "postrouting",
+ [NF_BR_BROUTING] = NULL,
+};
+
+static const char *arp_hooks_str_array[NF_ARP_NUMHOOKS] = {
+ [NF_ARP_IN] = "input",
+ [NF_ARP_OUT] = "output",
+ [NF_ARP_FORWARD] = "forward",
+};
+
+struct chain_type {
+ struct list_head list;
+ uint32_t family;
+ const char *type;
+ unsigned int num;
+ const char *hooks[0];
+};
+
+static struct list_head chain_types;
+
+static struct chain_type *chain_type_name_lookup(uint32_t family,
+ const char *name)
{
- switch (family) {
- case NFPROTO_IPV4:
- case NFPROTO_BRIDGE:
- case NFPROTO_IPV6:
- case NFPROTO_INET:
- /* These families have overlapping values for each hook */
- if (!strcmp(hook, "prerouting"))
- return NF_INET_PRE_ROUTING;
- else if (!strcmp(hook, "input"))
- return NF_INET_LOCAL_IN;
- else if (!strcmp(hook, "forward"))
- return NF_INET_FORWARD;
- else if (!strcmp(hook, "postrouting"))
- return NF_INET_POST_ROUTING;
- else if (!strcmp(hook, "output"))
- return NF_INET_LOCAL_OUT;
- case NFPROTO_ARP:
- if (!strcmp(hook, "input"))
- return NF_ARP_IN;
- else if (!strcmp(hook, "forward"))
- return NF_ARP_FORWARD;
- else if (!strcmp(hook, "output"))
- return NF_ARP_OUT;
- default:
- break;
+ struct chain_type *type;
+
+ list_for_each_entry(type, &chain_types, list) {
+ if (type->family == family && !strcmp(type->type, name))
+ return type;
+ }
+ return NULL;
+}
+
+static int chain_hook_name_lookup(struct chain_type *type, const char *name)
+{
+ unsigned int num;
+ const char **hookstr;
+
+ if (!type)
+ return -1;
+ hookstr = type->hooks;
+ for (num = 0; num < type->num; num++) {
+ if (hookstr[num] && !(strcmp(hookstr[num], name)))
+ return num;
+ }
+ return -1;
+}
+
+static void chain_type_register(uint32_t family, const char *type,
+ unsigned int max, unsigned int hook_mask)
+{
+ unsigned int num;
+ const char **hookstr;
+ struct chain_type *chain_type;
+
+ chain_type = xmalloc(sizeof(struct chain_type) +
+ max * sizeof(char *));
+ init_list_head(&chain_type->list);
+ chain_type->family = family;
+ chain_type->type = type;
+ chain_type->num = max;
+
+ hookstr = chain_type->hooks;
+ for (num = 0; num < max; num++) {
+ if (hook_mask & (1 << num)) {
+ switch (family) {
+ case NFPROTO_IPV4:
+ case NFPROTO_IPV6:
+ case NFPROTO_INET:
+ hookstr[num] = inet_hooks_str_array[num];
+ break;
+ case NFPROTO_ARP:
+ hookstr[num] = arp_hooks_str_array[num];
+ break;
+ case NFPROTO_BRIDGE:
+ hookstr[num] = bridge_hooks_str_array[num];
+ break;
+ default:
+ BUG("invalid family %u\n", family);
+ }
+ } else
+ hookstr[num] = NULL;
}
+ list_add_tail(&chain_type->list, &chain_types);
+}
- return NF_INET_NUMHOOKS;
+static void __init chain_type_init(void)
+{
+ init_list_head(&chain_types);
+
+ chain_type_register(NFPROTO_IPV4, "filter", NF_INET_NUMHOOKS,
+ (1 << NF_INET_PRE_ROUTING) |
+ (1 << NF_INET_LOCAL_IN) |
+ (1 << NF_INET_FORWARD) |
+ (1 << NF_INET_LOCAL_OUT) |
+ (1 << NF_INET_POST_ROUTING));
+
+ chain_type_register(NFPROTO_IPV4, "nat", NF_INET_NUMHOOKS,
+ (1 << NF_INET_PRE_ROUTING) |
+ (1 << NF_INET_LOCAL_IN) |
+ (1 << NF_INET_LOCAL_OUT) |
+ (1 << NF_INET_POST_ROUTING));
+
+ chain_type_register(NFPROTO_IPV4, "route", NF_INET_NUMHOOKS,
+ (1 << NF_INET_LOCAL_OUT));
+
+ chain_type_register(NFPROTO_IPV6, "filter", NF_INET_NUMHOOKS,
+ (1 << NF_INET_PRE_ROUTING) |
+ (1 << NF_INET_LOCAL_IN) |
+ (1 << NF_INET_FORWARD) |
+ (1 << NF_INET_LOCAL_OUT) |
+ (1 << NF_INET_POST_ROUTING));
+
+ chain_type_register(NFPROTO_IPV6, "nat", NF_INET_NUMHOOKS,
+ (1 << NF_INET_PRE_ROUTING) |
+ (1 << NF_INET_LOCAL_IN) |
+ (1 << NF_INET_LOCAL_OUT) |
+ (1 << NF_INET_POST_ROUTING));
+
+ chain_type_register(NFPROTO_IPV6, "route", NF_INET_NUMHOOKS,
+ (1 << NF_INET_LOCAL_OUT));
+
+ chain_type_register(NFPROTO_INET, "filter", NF_INET_NUMHOOKS,
+ (1 << NF_INET_PRE_ROUTING) |
+ (1 << NF_INET_LOCAL_IN) |
+ (1 << NF_INET_FORWARD) |
+ (1 << NF_INET_LOCAL_OUT) |
+ (1 << NF_INET_POST_ROUTING));
+
+ chain_type_register(NFPROTO_ARP, "filter", NF_ARP_NUMHOOKS,
+ (1 << NF_ARP_IN) |
+ (1 << NF_ARP_OUT) |
+ (1 << NF_ARP_FORWARD));
+
+ chain_type_register(NFPROTO_BRIDGE, "filter", NF_BR_NUMHOOKS,
+ (1 << NF_BR_LOCAL_IN) |
+ (1 << NF_BR_FORWARD) |
+ (1 << NF_BR_LOCAL_OUT));
}
static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
@@ -1346,11 +1468,21 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
struct rule *rule;
if (chain->flags & CHAIN_F_BASECHAIN) {
- chain->hooknum = str2hooknum(chain->handle.family,
- chain->hookstr);
- if (chain->hooknum == NF_INET_NUMHOOKS)
- return chain_error(ctx, chain, "invalid hook %s",
- chain->hookstr);
+ int hooknum;
+ struct chain_type *chain_type;
+
+ chain_type = chain_type_name_lookup(chain->handle.family,
+ chain->type);
+ if (!chain_type)
+ return chain_error(ctx, chain, "invalid type name %s",
+ chain->type);
+ hooknum = chain_hook_name_lookup(chain_type, chain->hookstr);
+ if (hooknum == -1)
+ return chain_error(ctx, chain, "invalid hook name %s",
+ chain->hookstr);
+ xfree(chain->type);
+ chain->type = chain_type->type;
+ chain->hooknum = (unsigned int)hooknum;
}
list_for_each_entry(rule, &chain->rules, list) {
diff --git a/src/parser.y b/src/parser.y
index 26d2879..2a816be 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -1119,35 +1119,15 @@ map_block : /* empty */ { $$ = $<set>-1; }
hook_spec : TYPE STRING HOOK STRING PRIORITY NUM
{
- $<chain>0->type = chain_type_name_lookup($2);
- if ($<chain>0->type == NULL) {
- erec_queue(error(&@2, "unknown chain type %s", $2),
- state->msgs);
- YYERROR;
- }
- $<chain>0->hookstr = chain_hookname_lookup($4);
- if ($<chain>0->hookstr == NULL) {
- erec_queue(error(&@4, "unknown chain type %s", $4),
- state->msgs);
- YYERROR;
- }
+ $<chain>0->type = $2;
+ $<chain>0->hookstr = $4;
$<chain>0->priority = $6;
$<chain>0->flags |= CHAIN_F_BASECHAIN;
}
| TYPE STRING HOOK STRING PRIORITY DASH NUM
{
- $<chain>0->type = chain_type_name_lookup($2);
- if ($<chain>0->type == NULL) {
- erec_queue(error(&@2, "unknown type name %s", $2),
- state->msgs);
- YYERROR;
- }
- $<chain>0->hookstr = chain_hookname_lookup($4);
- if ($<chain>0->hookstr == NULL) {
- erec_queue(error(&@4, "unknown hook name %s", $4),
- state->msgs);
- YYERROR;
- }
+ $<chain>0->type = $2;
+ $<chain>0->hookstr = $4;
$<chain>0->priority = -$7;
$<chain>0->flags |= CHAIN_F_BASECHAIN;
}
diff --git a/src/rule.c b/src/rule.c
index 1e54526..c360bc7 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -275,46 +275,6 @@ struct symbol *symbol_lookup(const struct scope *scope, const char *identifier)
return NULL;
}
-static const char *chain_type_str_array[] = {
- "filter",
- "nat",
- "route",
- NULL,
-};
-
-const char *chain_type_name_lookup(const char *name)
-{
- int i;
-
- for (i = 0; chain_type_str_array[i]; i++) {
- if (!strcmp(name, chain_type_str_array[i]))
- return chain_type_str_array[i];
- }
-
- return NULL;
-}
-
-static const char *chain_hookname_str_array[] = {
- "prerouting",
- "input",
- "forward",
- "postrouting",
- "output",
- NULL,
-};
-
-const char *chain_hookname_lookup(const char *name)
-{
- int i;
-
- for (i = 0; chain_hookname_str_array[i]; i++) {
- if (!strcmp(name, chain_hookname_str_array[i]))
- return chain_hookname_str_array[i];
- }
-
- return NULL;
-}
-
struct chain *chain_alloc(const char *name)
{
struct chain *chain;
--
1.9.3
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [nft PATCH] More extensive error checking on base chain
2014-08-12 9:37 [nft PATCH] More extensive error checking on base chain Yanchuan Nian
@ 2014-08-14 18:59 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2014-08-14 18:59 UTC (permalink / raw)
To: Yanchuan Nian; +Cc: netfilter-devel
On Tue, Aug 12, 2014 at 05:37:24PM +0800, Yanchuan Nian wrote:
> The evaluation of base chain isn't extensive enough. It cannot
> detect whether a chain type is supported or not in certain families.
> It also cannot deletect whether a hook is supported or not in certain
> chains. The evaluation is done by kernel, and the information is unclear.
>
> nft> add chain arp arptable chain1 {type nat hook input priority 0 ;}
> <cli>:1:1-64: Error: Could not add chain: No such file or directory
> add chain arp arptable chain1 {type nat hook input priority 0 ;}
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> nft> add chain ip iptable chain1 {type nat hook forward priority 0 ;}
> <cli>:1:1-64: Error: Could not add chain: Operation not supported
> add chain ip iptable chain1 {type nat hook forward priority 0 ;}
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> This patch performs more extensive error checking, so the information
> is more clear.
>
> nft> add chain arp arptable chain1 {type nat hook input priority 0 ;}
> <cli>:1:31-63: Error: invalid type name nat
> add chain arp arptable chain1 {type nat hook input priority 0 ;}
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> nft> add chain ip iptable chain1 {type nat hook forward priority 0 ;}
> <cli>:1:29-63: Error: invalid hook name forward
> add chain ip iptable chain1 {type nat hook forward priority 0 ;}
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I don't think this belongs to userspace. Patrick and I have been
discussing some extension for netlink to provide better error
reporting to userspace, such as the offset to the attribute that has
caused the problem. This should provide enough context to userspace to
display more accurate error reporting.
On top of that, this is not going to help to catch the chain type
module is missing case.
Meanwhile, I have documented this here:
http://wiki.nftables.org/wiki-nftables/index.php/Troubleshooting
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-08-14 18:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-12 9:37 [nft PATCH] More extensive error checking on base chain Yanchuan Nian
2014-08-14 18:59 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).