iptables: xtables-eb: adjust policy in user-defined chains
PATCH 1/1 iptables: xtables-eb: adjust policy in user-defined chains
The ebtables-compat tool doesn't support default policy in custom chains. RETURN is the default policy in this case, and is mandatory (this is the behaviour of nf_tables).
While at it, fix the error message when trying to change the default policy to RETURN in builtin chains to match the original ebtables message.
--- iptables/nft-bridge.c | 7 ++++++- iptables/xtables-eb.c | 19 ++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-)
iptables/nft-bridge.c +6 -1
@@ -356,7 +356,12 @@ static void nft_bridge_print_header(unsigned int format, const char *chain, const struct xt_counters *counters, bool basechain, uint32_t refs) { - printf("Bridge chain: %s, entries: %u, policy: %s\n", chain, refs, pol); + if (basechain) + printf("Bridge chain: %s, entries: %u, policy: %s\n", + chain, refs, pol); + else + printf("Bridge chain: %s, entries: %u, policy: RETURN\n", + chain, refs); } static void nft_bridge_print_firewall(struct nft_rule *r, unsigned int num,
iptables/xtables-eb.c +12 -7
@@ -615,11 +615,10 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table)case 'N' :/* Make a user defined chain */ case 'E' :/* Rename chain */ case 'X' :/* Delete chain */ -/* We allow -N chainname -P policy */ if (command =='N' && c =='P' ) { - command = c; - optind--;/* No table specified */ - goto handle_P; +xtables_error (PARAMETER_PROBLEM , +"The default policy in user-defined" +" chains is RETURN (mandatory)" ); }if (OPT_COMMANDS ) xtables_error(PARAMETER_PROBLEM,
@@ -663,7 +662,6 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table) optind++; } }else if (c =='P' ) { -handle_P:if (optind >= argc)xtables_error (PARAMETER_PROBLEM ,"No policy specified" );
@@ -1146,9 +1144,16 @@ check_extension: */ cs.fw.ethproto =htons (cs.fw.ethproto);if (command =='P' ) { -if (selected_chain <NF_BR_NUMHOOKS &&strcmp (policy,"RETURN" )==0 ) +if (selected_chain <0 ) {xtables_error (PARAMETER_PROBLEM , -"Policy RETURN only allowed for user defined chains" ); +"Default policy in user-defined chains " +"is mandatory RETURN" ); + } +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" );
Pablo Neira Ayuso
maintainerQuoted 19 lines · Arturo Borrero Gonzalez
The ebtables-compat tool doesn't support default policy in custom chains. RETURN is the default policy in this case, and is mandatory (this is the behaviour of
nf_tables).While at it, fix the error message when trying to change the default policy to RETURN in builtin chains to match the original ebtables message.
Signed-off-by Arturo Borrero Gonzalez <[email protected]>--- iptables/nft-bridge.c | 7 ++++++- iptables/xtables-eb.c | 19 ++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-)iptables/nft-bridge.c +0
-0@@ -356,7 +356,12 @@ static void nft_bridge_print_header(unsigned int format, const char *chain, const struct xt_counters *counters,
iptables/nft-bridge.c static void nft_bridge_print_header(unsigned int format, const char *chain, · line 365
Quoted 10 lines
iptables/nft-bridge.c +6
-1bool basechain, uint32_t refs) { -printf ("Bridge chain: %s, entries: %u, policy: %s\n" , chain, refs, pol); + if (basechain) + printf("Bridge chain: %s, entries: %u, policy: %s\n", + chain, refs, pol); + else + printf("Bridge chain: %s, entries: %u, policy: RETURN\n", + chain, refs); }
I'd suggest:
printf("Bridge chain: %s, entries: %u, policy: %s\n",
chain, refs, basechain ? pol : "RETURN");
iptables/xtables-eb.c int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table) · line 618
Quoted 10 lines
iptables/nft-bridge.c +0
-0static void nft_bridge_print_firewall(struct nft_rule *r, unsigned int num,iptables/xtables-eb.c +0
-1@@ -615,11 +615,10 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table)case 'N' :/* Make a user defined chain */ case 'E' :/* Rename chain */ case 'X' :/* Delete chain */ -/* We allow -N chainname -P policy */
Better extend this comment:
/* We allow -N chainname -P policy */
/* XXX: Not in ebtables-compat */iptables/xtables-eb.c int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table) · line 622
Quoted 8 lines
iptables/xtables-eb.c +3
-3if (command =='N' && c =='P' ) { - command = c; - optind--;/* No table specified */ - goto handle_P; +xtables_error (PARAMETER_PROBLEM , +"The default policy in user-defined" +" chains is RETURN (mandatory)" ); }
I'd really prefer to avoid changes in the ebtables parser. Could you perform this checking from later on, when validating the configuration.
if (strcmp(chain, "INPUT") != 0 &&
strcmp(chain, "FORWARD") != 0 &&
... &&
strcmp(policy, "RETURN") != 0)
xtables_error("...);Quoted 16 lines
iptables/xtables-eb.c +1
-2if (OPT_COMMANDS ) xtables_error(PARAMETER_PROBLEM,@@ -663,7 +662,6 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table) optind++; } }else if (c =='P' ) { -handle_P:if (optind >= argc)xtables_error (PARAMETER_PROBLEM ,"No policy specified" );@@ -1146,9 +1144,16 @@ check_extension: */ cs.fw.ethproto =htons (cs.fw.ethproto);if (command =='P' ) { -if (selected_chain <NF_BR_NUMHOOKS &&strcmp (policy,"RETURN" )==0 ) +if (selected_chain <0 ) {
iptables/xtables-eb.c check_extension: */ · line 1156
Quoted 10 lines
iptables/xtables-eb.c +8
-1xtables_error (PARAMETER_PROBLEM , -"Policy RETURN only allowed for user defined chains" ); +"Default policy in user-defined chains " +"is mandatory RETURN" ); + } +if (strcmp (policy,"RETURN" ) ==0 ) { +xtables_error (PARAMETER_PROBLEM , +"Policy RETURN only allowed for user " +"defined chains" ); + }
Not sure why you need this change.
Quoted 13 lines · Pablo Neira Ayuso
iptables/xtables-eb.c int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table) · line 622
Quoted 8 lines
if (command == 'N' && c == 'P') {iptables/xtables-eb.c +3
-3- command = c; - optind--;/* No table specified */ - goto handle_P; +xtables_error (PARAMETER_PROBLEM , +"The default policy in user-defined" +" chains is RETURN (mandatory)" ); }I'd really prefer to avoid changes in the ebtables parser. Could you perform this checking from later on, when validating the configuration.
Quoted 6 lines
if (strcmp(chain, "INPUT") != 0 &&
strcmp(chain, "FORWARD") != 0 &&
... &&
strcmp(policy, "RETURN") != 0)
xtables_error("...);In the user-defined chain creation path, the code jumps to several places (the goto handle_P; thing). I don't see clearly a zone where to put these validations.
Could you please point to where would you like to put this code?
regards.
Signature · 5 lines
Arturo Borrero González -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso
maintainerQuoted 21 lines · Arturo Borrero Gonzalez
Quoted 13 lines · Pablo Neira Ayuso
iptables/xtables-eb.c int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table) · line 622
Quoted 8 lines
if (command == 'N' && c == 'P') {iptables/xtables-eb.c +3
-3- command = c; - optind--;/* No table specified */ - goto handle_P; +xtables_error (PARAMETER_PROBLEM , +"The default policy in user-defined" +" chains is RETURN (mandatory)" ); }I'd really prefer to avoid changes in the ebtables parser. Could you perform this checking from later on, when validating the configuration.
Quoted 6 lines
if (strcmp(chain, "INPUT") != 0 && strcmp(chain, "FORWARD") != 0 && ... && strcmp(policy, "RETURN") != 0) xtables_error("...);
Quoted 5 lines
In the user-defined chain creation path, the code jumps to several places (the goto
handle_P; thing). I don't see clearly a zone where to put these validations.Could you please point to where would you like to put this code?
This will leave the while ((c = getopt_long ...) loop at some point, right? So perform the validation after the parsing is done.