rpms/iptables
7
0
Fork 0
Code Issues Pull Requests Releases Activity

iptables-1.8.7-29.el9

Browse Source 
- libxtables: Boost rule target checks by announcing chain names
- libxtables: Implement notargets hash table
- nft: Reject standard targets as chain names when restoring
- xshared: Merge and share parse_chain()
- xshared: Prefer xtables_chain_protos lookup over getprotoent
- nft: Speed up immediate parsing
- nft: Simplify immediate parsing

Resolves: rhbz#2065330
This commit is contained in:
Phil Sutter 2022-03-18 11:37:11 +01:00
parent 63c9e2ff10
commit a556128cb3
8 changed files with 942 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

199
0027-nft-Simplify-immediate-parsing.patch Normal file
View File

@ -0,0 +1,199 @@
From c4bc02802cb95af82d30cb0ad605060694640e07 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Tue, 1 Mar 2022 18:59:31 +0100
Subject: [PATCH] nft: Simplify immediate parsing
Implementations of parse_immediate callback are mostly trivial, the only
relevant part is access to family-specific parts of struct
iptables_command_state when setting goto flag for iptables and
ip6tables. Refactor them into simple set_goto_flag callbacks.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
(cherry picked from commit b5f2faea325a315bfb932ebc634f3298d4824cae)
---
iptables/nft-arp.c | 9 ---------
iptables/nft-bridge.c | 9 ---------
iptables/nft-ipv4.c | 12 +++---------
iptables/nft-ipv6.c | 12 +++---------
iptables/nft-shared.c | 17 +++++++----------
iptables/nft-shared.h | 2 +-
6 files changed, 14 insertions(+), 47 deletions(-)
diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index 2a9387a18dffe..d55e06572b283 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -182,14 +182,6 @@ static void nft_arp_parse_meta(struct nft_xt_ctx *ctx, struct nftnl_expr *e,
fw->arp.invflags |= flags;
}
-static void nft_arp_parse_immediate(const char *jumpto, bool nft_goto,
- void *data)
-{
- struct iptables_command_state *cs = data;
-
- cs->jumpto = jumpto;
-}
-
static void parse_mask_ipv4(struct nft_xt_ctx *ctx, struct in_addr *mask)
{
mask->s_addr = ctx->bitwise.mask[0];
@@ -552,7 +544,6 @@ struct nft_family_ops nft_family_ops_arp = {
.print_payload = NULL,
.parse_meta = nft_arp_parse_meta,
.parse_payload = nft_arp_parse_payload,
- .parse_immediate = nft_arp_parse_immediate,
.print_header = nft_arp_print_header,
.print_rule = nft_arp_print_rule,
.save_rule = nft_arp_save_rule,
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index d98fd527d9549..5807c0d32a97c 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -251,14 +251,6 @@ static void nft_bridge_parse_payload(struct nft_xt_ctx *ctx,
}
}
-static void nft_bridge_parse_immediate(const char *jumpto, bool nft_goto,
- void *data)
-{
- struct iptables_command_state *cs = data;
-
- cs->jumpto = jumpto;
-}
-
/* return 0 if saddr, 1 if daddr, -1 on error */
static int
lookup_check_ether_payload(uint32_t base, uint32_t offset, uint32_t len)
@@ -889,7 +881,6 @@ struct nft_family_ops nft_family_ops_bridge = {
.print_payload = NULL,
.parse_meta = nft_bridge_parse_meta,
.parse_payload = nft_bridge_parse_payload,
- .parse_immediate = nft_bridge_parse_immediate,
.parse_lookup = nft_bridge_parse_lookup,
.parse_match = nft_bridge_parse_match,
.parse_target = nft_bridge_parse_target,
diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index 34f94bd8cc24a..80b8954f4a39d 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -241,15 +241,9 @@ static void nft_ipv4_parse_payload(struct nft_xt_ctx *ctx,
}
}
-static void nft_ipv4_parse_immediate(const char *jumpto, bool nft_goto,
- void *data)
+static void nft_ipv4_set_goto_flag(struct iptables_command_state *cs)
{
- struct iptables_command_state *cs = data;
-
- cs->jumpto = jumpto;
-
- if (nft_goto)
- cs->fw.ip.flags |= IPT_F_GOTO;
+ cs->fw.ip.flags |= IPT_F_GOTO;
}
static void print_fragment(unsigned int flags, unsigned int invflags,
@@ -473,7 +467,7 @@ struct nft_family_ops nft_family_ops_ipv4 = {
.is_same = nft_ipv4_is_same,
.parse_meta = nft_ipv4_parse_meta,
.parse_payload = nft_ipv4_parse_payload,
- .parse_immediate = nft_ipv4_parse_immediate,
+ .set_goto_flag = nft_ipv4_set_goto_flag,
.print_header = print_header,
.print_rule = nft_ipv4_print_rule,
.save_rule = nft_ipv4_save_rule,
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index d9c9400ad7dc3..663401b49f94d 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -180,15 +180,9 @@ static void nft_ipv6_parse_payload(struct nft_xt_ctx *ctx,
}
}
-static void nft_ipv6_parse_immediate(const char *jumpto, bool nft_goto,
- void *data)
+static void nft_ipv6_set_goto_flag(struct iptables_command_state *cs)
{
- struct iptables_command_state *cs = data;
-
- cs->jumpto = jumpto;
-
- if (nft_goto)
- cs->fw6.ipv6.flags |= IP6T_F_GOTO;
+ cs->fw6.ipv6.flags |= IP6T_F_GOTO;
}
static void nft_ipv6_print_rule(struct nft_handle *h, struct nftnl_rule *r,
@@ -415,7 +409,7 @@ struct nft_family_ops nft_family_ops_ipv6 = {
.is_same = nft_ipv6_is_same,
.parse_meta = nft_ipv6_parse_meta,
.parse_payload = nft_ipv6_parse_payload,
- .parse_immediate = nft_ipv6_parse_immediate,
+ .set_goto_flag = nft_ipv6_set_goto_flag,
.print_header = print_header,
.print_rule = nft_ipv6_print_rule,
.save_rule = nft_ipv6_save_rule,
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index c1664b50f9383..bd0c8895d48bb 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -510,9 +510,7 @@ static void nft_parse_counter(struct nftnl_expr *e, struct xt_counters *counters
static void nft_parse_immediate(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
{
const char *chain = nftnl_expr_get_str(e, NFTNL_EXPR_IMM_CHAIN);
- const char *jumpto = NULL;
- bool nft_goto = false;
- void *data = ctx->cs;
+ struct iptables_command_state *cs = ctx->cs;
int verdict;
if (nftnl_expr_is_set(e, NFTNL_EXPR_IMM_DATA)) {
@@ -535,23 +533,22 @@ static void nft_parse_immediate(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
/* Standard target? */
switch(verdict) {
case NF_ACCEPT:
- jumpto = "ACCEPT";
+ cs->jumpto = "ACCEPT";
break;
case NF_DROP:
- jumpto = "DROP";
+ cs->jumpto = "DROP";
break;
case NFT_RETURN:
- jumpto = "RETURN";
+ cs->jumpto = "RETURN";
break;;
case NFT_GOTO:
- nft_goto = true;
+ if (ctx->h->ops->set_goto_flag)
+ ctx->h->ops->set_goto_flag(cs);
/* fall through */
case NFT_JUMP:
- jumpto = chain;
+ cs->jumpto = chain;
break;
}
-
- ctx->h->ops->parse_immediate(jumpto, nft_goto, data);
}
static void nft_parse_limit(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index da4ba9d2ba8de..e4ef16cc24f12 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -89,7 +89,7 @@ struct nft_family_ops {
void *data);
void (*parse_lookup)(struct nft_xt_ctx *ctx, struct nftnl_expr *e,
void *data);
- void (*parse_immediate)(const char *jumpto, bool nft_goto, void *data);
+ void (*set_goto_flag)(struct iptables_command_state *cs);
void (*print_table_header)(const char *tablename);
void (*print_header)(unsigned int format, const char *chain,
--
2.34.1

119
0028-nft-Speed-up-immediate-parsing.patch Normal file
View File

@ -0,0 +1,119 @@
From 88dbd59dcf172b54e650f267c490d70ee55235d5 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Tue, 1 Mar 2022 19:46:21 +0100
Subject: [PATCH] nft: Speed up immediate parsing
Parsing of rules which jump to a chain pointlessly causes a call to
xtables_find_target() despite the code already knowing the outcome.
Avoid the significant delay for rulesets with many chain jumps by
performing the (standard) target lookup only for accept/drop/return
verdicts.
From a biased test-case on my VM:
| # iptables-nft-save | grep -c -- '-j'
| 133943
| # time ./old/iptables-nft-save >/dev/null
| real 0m45.566s
| user 0m1.308s
| sys 0m8.430s
| # time ./new/iptables-nft-save >/dev/null
| real 0m3.547s
| user 0m0.762s
| sys 0m2.476s
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
(cherry picked from commit 07ee529f5a62838d68be59683be99bf6a7cda0f2)
---
iptables/nft-bridge.c | 1 +
iptables/nft-shared.c | 37 ++++++++++++++++++-------------------
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 5807c0d32a97c..4df71f0c775e3 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -506,6 +506,7 @@ static void nft_bridge_parse_target(struct xtables_target *t, void *data)
}
cs->target = t;
+ cs->jumpto = t->name;
}
static void nft_rule_to_ebtables_command_state(struct nft_handle *h,
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index bd0c8895d48bb..1aaaa8159ff61 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -511,6 +511,8 @@ static void nft_parse_immediate(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
{
const char *chain = nftnl_expr_get_str(e, NFTNL_EXPR_IMM_CHAIN);
struct iptables_command_state *cs = ctx->cs;
+ struct xt_entry_target *t;
+ uint32_t size;
int verdict;
if (nftnl_expr_is_set(e, NFTNL_EXPR_IMM_DATA)) {
@@ -547,8 +549,21 @@ static void nft_parse_immediate(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
/* fall through */
case NFT_JUMP:
cs->jumpto = chain;
- break;
+ /* fall through */
+ default:
+ return;
}
+
+ cs->target = xtables_find_target(cs->jumpto, XTF_TRY_LOAD);
+ if (!cs->target)
+ return;
+
+ size = XT_ALIGN(sizeof(struct xt_entry_target)) + cs->target->size;
+ t = xtables_calloc(1, size);
+ t->u.target_size = size;
+ t->u.user.revision = cs->target->revision;
+ strcpy(t->u.user.name, cs->jumpto);
+ cs->target->t = t;
}
static void nft_parse_limit(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
@@ -676,25 +691,8 @@ void nft_rule_to_iptables_command_state(struct nft_handle *h,
}
}
- if (cs->target != NULL) {
- cs->jumpto = cs->target->name;
- } else if (cs->jumpto != NULL) {
- struct xt_entry_target *t;
- uint32_t size;
-
- cs->target = xtables_find_target(cs->jumpto, XTF_TRY_LOAD);
- if (!cs->target)
- return;
-
- size = XT_ALIGN(sizeof(struct xt_entry_target)) + cs->target->size;
- t = xtables_calloc(1, size);
- t->u.target_size = size;
- t->u.user.revision = cs->target->revision;
- strcpy(t->u.user.name, cs->jumpto);
- cs->target->t = t;
- } else {
+ if (!cs->jumpto)
cs->jumpto = "";
- }
}
void nft_clear_iptables_command_state(struct iptables_command_state *cs)
@@ -983,6 +981,7 @@ void nft_ipv46_parse_target(struct xtables_target *t, void *data)
struct iptables_command_state *cs = data;
cs->target = t;
+ cs->jumpto = t->name;
}
void nft_check_xt_legacy(int family, bool is_ipt_save)
--
2.34.1

104
0029-xshared-Prefer-xtables_chain_protos-lookup-over-getp.patch Normal file
View File

@ -0,0 +1,104 @@
From 970ab34aa07f2fdb6ad8a79bc5b7a1d9a5bcd482 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Tue, 1 Mar 2022 23:05:29 +0100
Subject: [PATCH] xshared: Prefer xtables_chain_protos lookup over getprotoent
When dumping a large ruleset, common protocol matches such as for TCP
port number significantly slow down rule printing due to repeated calls
for getprotobynumber(). The latter does not involve any caching, so
/etc/protocols is consulted over and over again.
As a simple countermeasure, make functions converting between proto
number and name prefer the built-in list of "well-known" protocols. This
is not a perfect solution, repeated rules for protocol names libxtables
does not cache (e.g. igmp or dccp) will still be slow. Implementing
getprotoent() result caching could solve this.
As a side-effect, explicit check for pseudo-protocol "all" may be
dropped as it is contained in the built-in list and therefore immutable.
Also update xtables_chain_protos entries a bit to align with typical
/etc/protocols contents. The testsuite assumes those names, so the
preferred ones prior to this patch are indeed uncommon nowadays.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
(cherry picked from commit b6196c7504d4d41827cea86c167926125cdbf1f3)
---
iptables/xshared.c | 8 ++++----
libxtables/xtables.c | 19 ++++++-------------
2 files changed, 10 insertions(+), 17 deletions(-)
diff --git a/iptables/xshared.c b/iptables/xshared.c
index 4027d9240215e..68aea42146c36 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -52,16 +52,16 @@ proto_to_name(uint8_t proto, int nolookup)
{
unsigned int i;
+ for (i = 0; xtables_chain_protos[i].name != NULL; ++i)
+ if (xtables_chain_protos[i].num == proto)
+ return xtables_chain_protos[i].name;
+
if (proto && !nolookup) {
struct protoent *pent = getprotobynumber(proto);
if (pent)
return pent->p_name;
}
- for (i = 0; xtables_chain_protos[i].name != NULL; ++i)
- if (xtables_chain_protos[i].num == proto)
- return xtables_chain_protos[i].name;
-
return NULL;
}
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index 1931e3896262a..a991e4fcc9b92 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -2078,10 +2078,11 @@ const struct xtables_pprot xtables_chain_protos[] = {
{"udp", IPPROTO_UDP},
{"udplite", IPPROTO_UDPLITE},
{"icmp", IPPROTO_ICMP},
- {"icmpv6", IPPROTO_ICMPV6},
{"ipv6-icmp", IPPROTO_ICMPV6},
+ {"icmpv6", IPPROTO_ICMPV6},
{"esp", IPPROTO_ESP},
{"ah", IPPROTO_AH},
+ {"mobility-header", IPPROTO_MH},
{"ipv6-mh", IPPROTO_MH},
{"mh", IPPROTO_MH},
{"all", 0},
@@ -2097,23 +2098,15 @@ xtables_parse_protocol(const char *s)
if (xtables_strtoui(s, NULL, &proto, 0, UINT8_MAX))
return proto;
- /* first deal with the special case of 'all' to prevent
- * people from being able to redefine 'all' in nsswitch
- * and/or provoke expensive [not working] ldap/nis/...
- * lookups */
- if (strcmp(s, "all") == 0)
- return 0;
+ for (i = 0; xtables_chain_protos[i].name != NULL; ++i) {
+ if (strcmp(s, xtables_chain_protos[i].name) == 0)
+ return xtables_chain_protos[i].num;
+ }
pent = getprotobyname(s);
if (pent != NULL)
return pent->p_proto;
- for (i = 0; i < ARRAY_SIZE(xtables_chain_protos); ++i) {
- if (xtables_chain_protos[i].name == NULL)
- continue;
- if (strcmp(s, xtables_chain_protos[i].name) == 0)
- return xtables_chain_protos[i].num;
- }
xt_params->exit_err(PARAMETER_PROBLEM,
"unknown protocol \"%s\" specified", s);
return -1;
--
2.34.1

164
0030-xshared-Merge-and-share-parse_chain.patch Normal file
View File

@ -0,0 +1,164 @@
From b65a9a258a0fc3c19a64a8426b28f7bcbe438cf5 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Fri, 5 Apr 2019 13:21:19 +0200
Subject: [PATCH] xshared: Merge and share parse_chain()
Have a common routine to perform chain name checks, combining all
variants' requirements.
Signed-off-by: Phil Sutter <phil@nwl.cc>
(cherry picked from commit 1189d830ea4fd269da87761d400ebabca02e1ef3)
Conflicts:
iptables/ip6tables.c
iptables/xshared.c
-> Context changes due to missing commit 9dc50b5b8e441
("xshared: Merge invflags handling code")
---
iptables/ip6tables.c | 26 --------------------------
iptables/iptables.c | 25 -------------------------
iptables/xshared.c | 24 ++++++++++++++++++++++++
iptables/xshared.h | 1 +
iptables/xtables.c | 9 +--------
5 files changed, 26 insertions(+), 59 deletions(-)
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index c95355b091568..f53d0a8d0f169 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -248,32 +248,6 @@ static int is_exthdr(uint16_t proto)
proto == IPPROTO_DSTOPTS);
}
-static void
-parse_chain(const char *chainname)
-{
- const char *ptr;
-
- if (strlen(chainname) >= XT_EXTENSION_MAXNAMELEN)
- xtables_error(PARAMETER_PROBLEM,
- "chain name `%s' too long (must be under %u chars)",
- chainname, XT_EXTENSION_MAXNAMELEN);
-
- if (*chainname == '-' || *chainname == '!')
- xtables_error(PARAMETER_PROBLEM,
- "chain name not allowed to start "
- "with `%c'\n", *chainname);
-
- if (xtables_find_target(chainname, XTF_TRY_LOAD))
- xtables_error(PARAMETER_PROBLEM,
- "chain name may not clash "
- "with target name\n");
-
- for (ptr = chainname; *ptr; ptr++)
- if (isspace(*ptr))
- xtables_error(PARAMETER_PROBLEM,
- "Invalid chain name `%s'", chainname);
-}
-
static void
set_option(unsigned int *options, unsigned int option, uint8_t *invflg,
int invert)
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 7d6183116d265..18cc4f7690a24 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -239,31 +239,6 @@ iptables_exit_error(enum xtables_exittype status, const char *msg, ...)
/* Christophe Burki wants `-p 6' to imply `-m tcp'. */
-static void
-parse_chain(const char *chainname)
-{
- const char *ptr;
-
- if (strlen(chainname) >= XT_EXTENSION_MAXNAMELEN)
- xtables_error(PARAMETER_PROBLEM,
- "chain name `%s' too long (must be under %u chars)",
- chainname, XT_EXTENSION_MAXNAMELEN);
-
- if (*chainname == '-' || *chainname == '!')
- xtables_error(PARAMETER_PROBLEM,
- "chain name not allowed to start "
- "with `%c'\n", *chainname);
-
- if (xtables_find_target(chainname, XTF_TRY_LOAD))
- xtables_error(PARAMETER_PROBLEM,
- "chain name may not clash "
- "with target name\n");
-
- for (ptr = chainname; *ptr; ptr++)
- if (isspace(*ptr))
- xtables_error(PARAMETER_PROBLEM,
- "Invalid chain name `%s'", chainname);
-}
static void
set_option(unsigned int *options, unsigned int option, uint8_t *invflg,
diff --git a/iptables/xshared.c b/iptables/xshared.c
index 68aea42146c36..d71724a2591dc 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -853,3 +853,27 @@ char opt2char(int option)
return *ptr;
}
+
+void parse_chain(const char *chainname)
+{
+ const char *ptr;
+
+ if (strlen(chainname) >= XT_EXTENSION_MAXNAMELEN)
+ xtables_error(PARAMETER_PROBLEM,
+ "chain name `%s' too long (must be under %u chars)",
+ chainname, XT_EXTENSION_MAXNAMELEN);
+
+ if (*chainname == '-' || *chainname == '!')
+ xtables_error(PARAMETER_PROBLEM,
+ "chain name not allowed to start with `%c'\n",
+ *chainname);
+
+ if (xtables_find_target(chainname, XTF_TRY_LOAD))
+ xtables_error(PARAMETER_PROBLEM,
+ "chain name may not clash with target name\n");
+
+ for (ptr = chainname; *ptr; ptr++)
+ if (isspace(*ptr))
+ xtables_error(PARAMETER_PROBLEM,
+ "Invalid chain name `%s'", chainname);
+}
diff --git a/iptables/xshared.h b/iptables/xshared.h
index 1e86aba8b2375..4f52117d5228d 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -223,6 +223,7 @@ char cmd2char(int option);
void add_command(unsigned int *cmd, const int newcmd,
const int othercmds, int invert);
int parse_rulenumber(const char *rule);
+void parse_chain(const char *chainname);
void generic_opt_check(int command, int options);
char opt2char(int option);
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 9779bd83d53b3..54f887f80497e 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -587,14 +587,7 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
break;
case 'N':
- if (optarg && (*optarg == '-' || *optarg == '!'))
- xtables_error(PARAMETER_PROBLEM,
- "chain name not allowed to start "
- "with `%c'\n", *optarg);
- if (xtables_find_target(optarg, XTF_TRY_LOAD))
- xtables_error(PARAMETER_PROBLEM,
- "chain name may not clash "
- "with target name\n");
+ parse_chain(optarg);
add_command(&p->command, CMD_NEW_CHAIN, CMD_NONE,
cs->invert);
p->chain = optarg;
--
2.34.1

113
0031-nft-Reject-standard-targets-as-chain-names-when-rest.patch Normal file
View File

@ -0,0 +1,113 @@
From dfd4dc4b930101659aed2d46b3ec70a6df3a520d Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 16 Mar 2022 17:14:07 +0100
Subject: [PATCH] nft: Reject standard targets as chain names when restoring
Reuse parse_chain() called from do_parse() for '-N' and rename it for a
better description of what it does.
Note that by itself, this patch will likely kill iptables-restore
performance for big rulesets due to the extra extension lookup for chain
lines. A following patch announcing those chains to libxtables will
alleviate that.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Florian Westphal <fw@strlen.de>
(cherry picked from commit b1aee6b2238794446feba41778f88703784560f7)
Conflicts:
iptables/xshared.c
-> Parts manually applied due to unmerged do_parse() function.
---
iptables/ip6tables.c | 2 +-
iptables/iptables.c | 2 +-
iptables/xshared.c | 2 +-
iptables/xshared.h | 2 +-
iptables/xtables-restore.c | 5 +----
iptables/xtables.c | 2 +-
6 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index f53d0a8d0f169..4422e28276ab5 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -1168,7 +1168,7 @@ int do_command6(int argc, char *argv[], char **table,
break;
case 'N':
- parse_chain(optarg);
+ assert_valid_chain_name(optarg);
add_command(&command, CMD_NEW_CHAIN, CMD_NONE,
cs.invert);
chain = optarg;
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 18cc4f7690a24..bbb87f16f8d1d 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -1163,7 +1163,7 @@ int do_command4(int argc, char *argv[], char **table,
break;
case 'N':
- parse_chain(optarg);
+ assert_valid_chain_name(optarg);
add_command(&command, CMD_NEW_CHAIN, CMD_NONE,
cs.invert);
chain = optarg;
diff --git a/iptables/xshared.c b/iptables/xshared.c
index d71724a2591dc..171b0bdb7ca27 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -854,7 +854,7 @@ char opt2char(int option)
return *ptr;
}
-void parse_chain(const char *chainname)
+void assert_valid_chain_name(const char *chainname)
{
const char *ptr;
diff --git a/iptables/xshared.h b/iptables/xshared.h
index 4f52117d5228d..84f1a559aafb2 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -223,7 +223,7 @@ char cmd2char(int option);
void add_command(unsigned int *cmd, const int newcmd,
const int othercmds, int invert);
int parse_rulenumber(const char *rule);
-void parse_chain(const char *chainname);
+void assert_valid_chain_name(const char *chainname);
void generic_opt_check(int command, int options);
char opt2char(int option);
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index d27394972d90c..074efeab0b4e7 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -155,10 +155,7 @@ static void xtables_restore_parse_line(struct nft_handle *h,
"%s: line %u chain name invalid\n",
xt_params->program_name, line);
- if (strlen(chain) >= XT_EXTENSION_MAXNAMELEN)
- xtables_error(PARAMETER_PROBLEM,
- "Invalid chain name `%s' (%u chars max)",
- chain, XT_EXTENSION_MAXNAMELEN - 1);
+ assert_valid_chain_name(chain);
policy = strtok(NULL, " \t\n");
DEBUGP("line %u, policy '%s'\n", line, policy);
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 54f887f80497e..a9b9e12e55c2f 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -587,7 +587,7 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
break;
case 'N':
- parse_chain(optarg);
+ assert_valid_chain_name(optarg);
add_command(&p->command, CMD_NEW_CHAIN, CMD_NONE,
cs->invert);
p->chain = optarg;
--
2.34.1

140
0032-libxtables-Implement-notargets-hash-table.patch Normal file
View File

@ -0,0 +1,140 @@
From 7241ebe5cff638bb1422448679d672aa52b3c367 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Tue, 15 Dec 2020 15:40:56 +0100
Subject: [PATCH] libxtables: Implement notargets hash table
Target lookup is relatively costly due to the filesystem access. Avoid
this overhead in huge rulesets which contain many chain jumps by caching
the failed lookups into a hashtable for later.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
(cherry picked from commit f58b0d7406451afbb4b9b6c7888990c964fa7c79)
---
libxtables/xtables.c | 75 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index a991e4fcc9b92..e8e900e869e1e 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -48,6 +48,7 @@
#include <linux/netfilter_ipv4/ip_tables.h>
#include <linux/netfilter_ipv6/ip6_tables.h>
#include <libiptc/libxtc.h>
+#include <libiptc/linux_list.h>
#ifndef NO_SHARED_LIBS
#include <dlfcn.h>
@@ -242,6 +243,71 @@ static void dlreg_free(void)
}
#endif
+struct notarget {
+ struct hlist_node node;
+ char name[];
+};
+
+#define NOTARGET_HSIZE 512
+static struct hlist_head notargets[NOTARGET_HSIZE];
+
+static void notargets_hlist_init(void)
+{
+ int i;
+
+ for (i = 0; i < NOTARGET_HSIZE; i++)
+ INIT_HLIST_HEAD(&notargets[i]);
+}
+
+static void notargets_hlist_free(void)
+{
+ struct hlist_node *pos, *n;
+ struct notarget *cur;
+ int i;
+
+ for (i = 0; i < NOTARGET_HSIZE; i++) {
+ hlist_for_each_entry_safe(cur, pos, n, &notargets[i], node) {
+ hlist_del(&cur->node);
+ free(cur);
+ }
+ }
+}
+
+static uint32_t djb_hash(const char *key)
+{
+ uint32_t i, hash = 5381;
+
+ for (i = 0; i < strlen(key); i++)
+ hash = ((hash << 5) + hash) + key[i];
+
+ return hash;
+}
+
+static struct notarget *notargets_hlist_lookup(const char *name)
+{
+ uint32_t key = djb_hash(name) % NOTARGET_HSIZE;
+ struct hlist_node *node;
+ struct notarget *cur;
+
+ hlist_for_each_entry(cur, node, &notargets[key], node) {
+ if (!strcmp(name, cur->name))
+ return cur;
+ }
+ return NULL;
+}
+
+static void notargets_hlist_insert(const char *name)
+{
+ struct notarget *cur;
+
+ if (!name)
+ return;
+
+ cur = xtables_malloc(sizeof(*cur) + strlen(name) + 1);
+ strcpy(cur->name, name);
+ hlist_add_head(&cur->node, &notargets[djb_hash(name) % NOTARGET_HSIZE]);
+}
+
void xtables_init(void)
{
xtables_libdir = getenv("XTABLES_LIBDIR");
@@ -267,6 +333,8 @@ void xtables_init(void)
return;
}
xtables_libdir = XTABLES_LIBDIR;
+
+ notargets_hlist_init();
}
void xtables_fini(void)
@@ -274,6 +342,7 @@ void xtables_fini(void)
#ifndef NO_SHARED_LIBS
dlreg_free();
#endif
+ notargets_hlist_free();
}
void xtables_set_nfproto(uint8_t nfproto)
@@ -808,6 +877,10 @@ xtables_find_target(const char *name, enum xtables_tryload tryload)
|| strcmp(name, XTC_LABEL_QUEUE) == 0
|| strcmp(name, XTC_LABEL_RETURN) == 0)
name = "standard";
+ /* known non-target? */
+ else if (notargets_hlist_lookup(name) &&
+ tryload != XTF_LOAD_MUST_SUCCEED)
+ return NULL;
/* Trigger delayed initialization */
for (dptr = &xtables_pending_targets; *dptr; ) {
@@ -871,6 +944,8 @@ xtables_find_target(const char *name, enum xtables_tryload tryload)
if (ptr)
ptr->used = 1;
+ else
+ notargets_hlist_insert(name);
return ptr;
}
--
2.34.1

86
0033-libxtables-Boost-rule-target-checks-by-announcing-ch.patch Normal file
View File

@ -0,0 +1,86 @@
From 2b17d7f73969459ee0d6503f10ac4122452431d1 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Fri, 4 Mar 2022 12:50:01 +0100
Subject: [PATCH] libxtables: Boost rule target checks by announcing chain
names
When restoring a ruleset, feed libxtables with chain names from
respective lines to avoid an extension search.
While the user's intention is clear, this effectively disables the
sanity check for clashes with target extensions. But:
* The check yielded only a warning and the clashing chain was finally
accepted.
* Users crafting iptables dumps for feeding into iptables-restore likely
know what they're doing.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
(cherry picked from commit ac4c84cc63d3cc021ca532692885a644fcde4518)
---
include/xtables.h | 3 +++
iptables/iptables-restore.c | 1 +
iptables/xtables-restore.c | 1 +
libxtables/xtables.c | 6 ++++++
4 files changed, 11 insertions(+)
diff --git a/include/xtables.h b/include/xtables.h
index 107ad7d65e6fc..b5a6764abfa4e 100644
--- a/include/xtables.h
+++ b/include/xtables.h
@@ -637,6 +637,9 @@ void xt_xlate_add_comment(struct xt_xlate *xl, const char *comment);
const char *xt_xlate_get_comment(struct xt_xlate *xl);
const char *xt_xlate_get(struct xt_xlate *xl);
+/* informed target lookups */
+void xtables_announce_chain(const char *name);
+
#ifdef XTABLES_INTERNAL
/* Shipped modules rely on this... */
diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c
index cc2c2b8b10086..a34d95015c93c 100644
--- a/iptables/iptables-restore.c
+++ b/iptables/iptables-restore.c
@@ -311,6 +311,7 @@ ip46tables_restore_main(const struct iptables_restore_cb *cb,
cb->ops->strerror(errno));
}
+ xtables_announce_chain(chain);
ret = 1;
} else if (in_table) {
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index 074efeab0b4e7..7e347e49aaaaa 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -155,6 +155,7 @@ static void xtables_restore_parse_line(struct nft_handle *h,
"%s: line %u chain name invalid\n",
xt_params->program_name, line);
+ xtables_announce_chain(chain);
assert_valid_chain_name(chain);
policy = strtok(NULL, " \t\n");
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index e8e900e869e1e..64d347e6ed434 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -308,6 +308,12 @@ static void notargets_hlist_insert(const char *name)
hlist_add_head(&cur->node, &notargets[djb_hash(name) % NOTARGET_HSIZE]);
}
+void xtables_announce_chain(const char *name)
+{
+ if (!notargets_hlist_lookup(name))
+ notargets_hlist_insert(name);
+}
+
void xtables_init(void)
{
xtables_libdir = getenv("XTABLES_LIBDIR");
--
2.34.1

18
iptables.spec
View File

@ -16,7 +16,7 @@ Name: iptables
Summary: Tools for managing Linux kernel packet filtering capabilities
URL: https://www.netfilter.org/projects/iptables
Version: 1.8.7
Release: 28%{?dist}
Release: 29%{?dist}
Source: %{url}/files/%{name}-%{version}.tar.bz2
Source1: iptables.init
Source2: iptables-config
@ -55,6 +55,13 @@ Patch23: 0023-nft-Fix-for-non-verbose-check-command.patch
Patch24: 0024-tests-shell-Assert-non-verbose-mode-is-silent.patch
Patch25: 0025-extensions-SECMARK-Implement-revision-1.patch
Patch26: 0026-extensions-SECMARK-Use-a-better-context-in-test-case.patch
Patch27: 0027-nft-Simplify-immediate-parsing.patch
Patch28: 0028-nft-Speed-up-immediate-parsing.patch
Patch29: 0029-xshared-Prefer-xtables_chain_protos-lookup-over-getp.patch
Patch30: 0030-xshared-Merge-and-share-parse_chain.patch
Patch31: 0031-nft-Reject-standard-targets-as-chain-names-when-rest.patch
Patch32: 0032-libxtables-Implement-notargets-hash-table.patch
Patch33: 0033-libxtables-Boost-rule-target-checks-by-announcing-ch.patch
# pf.os: ISC license
# iptables-apply: Artistic 2.0
@ -470,6 +477,15 @@ fi
%ghost %{_mandir}/man8/ebtables.8.gz
%changelog
* Fri Mar 18 2022 Phil Sutter <psutter@redhat.com> - 1.8.7-29
- libxtables: Boost rule target checks by announcing chain names
- libxtables: Implement notargets hash table
- nft: Reject standard targets as chain names when restoring
- xshared: Merge and share parse_chain()
- xshared: Prefer xtables_chain_protos lookup over getprotoent
- nft: Speed up immediate parsing
- nft: Simplify immediate parsing
* Wed Feb 16 2022 Phil Sutter <psutter@redhat.com> - 1.8.7-28
- extensions: SECMARK: Use a better context in test case