Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
On 12.09.23 12:27, Florian Westphal wrote: > Linux regression tracking (Thorsten Leemhuis) > wrote: >> On 12.09.23 00:57, Pablo Neira Ayuso wrote: >>> Userspace nftables v1.0.6 generates incorrect bytecode that hits a new >>> kernel check that rejects adding rules to bound chains. The incorrect >>> bytecode adds the chain binding, attach it to the rule and it adds the >>> rules to the chain binding. I have cherry-picked these three patches >>> for nftables v1.0.6 userspace and your ruleset restores fine. >>> [...] >> >> H. Well, this sounds like a kernel regression to me that normally >> should be dealt with on the kernel level, as users after updating the >> kernel should never have to update any userspace stuff to continue what >> they have been doing before the kernel update. > > This is a combo of a userspace bug and this new sanity check that > rejects the incorrect ordering (adding rules to the already-bound > anonymous chain). > > nf_tables uses a transaction allor-nothing model, this means that any > error that occurs during a transaction has to be reverse/undo all the > pending changes. This has caused a myriad of bugs already. > > So while this can be theoretically fixed in the kernel I don't see > a sane way to do it. Error unwinding / recovery from deeply nested > errors is already too complex for my taste. > >> Can't the kernel somehow detect the incorrect bytecode and do the right >> thing(tm) somehow? > > Theoretically yes, but I don't feel competent enough to do it, just look > at all the UaF bugs of the past month. Thx for the answer. FWIW, as this was a judgement call I mentioned this in my last regression report to Linus; he didn't reply, so I guess it is -- and will remove this issue from my tracking: #regzbot resolve: can be solved by a nftables userspace update; not nice, but likely best solution in this case #regzbot ignore-activity Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page.
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
On 2023-09-25, at 10:31:57 +0200, Arturo Borrero Gonzalez wrote: > On 9/24/23 13:48, Salvatore Bonaccorso wrote: > > The work for bookworm has been done, but for bullseye: would you be > > able to help here and prepare the fixes? Unfortunatlly the fixes will > > not apply cleanly. If we fear to much breakage, maybe upstream can be > > convinced to help? > > > > Hi Salvatore, > > I don't think I will have a lot of time to work on this in the next few weeks. > > I'm sorry :-( I can pick it up. I took a look at the patches yesterday evening and it wasn't much work to fix them up for Bullseye. I'll run the test-suites and check for regressions. J. signature.asc Description: PGP signature
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
On 9/24/23 13:48, Salvatore Bonaccorso wrote: The work for bookworm has been done, but for bullseye: would you be able to help here and prepare the fixes? Unfortunatlly the fixes will not apply cleanly. If we fear to much breakage, maybe upstream can be convinced to help? Hi Salvatore, I don't think I will have a lot of time to work on this in the next few weeks. I'm sorry :-(
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Hi Arturo, On Sat, Sep 16, 2023 at 09:02:34AM +0200, Arturo Borrero Gonzalez wrote: > On Sat, Sep 16, 2023, 08:37 Salvatore Bonaccorso wrote: > > > Hi > > > > Dropping some recipients for the Debian specific handling of this > > issue. So AFAIU upstream will not consider this on src:linux side to > > be further handled and needs to be addressed in nftables. > > > > Arturo: With the patches provided I prepared (as Timo) an update > > targetting bookworm for the next point release (bug for release.d.o to > > be submitted soon). > > > > Attached is the proposed debdiff, ans as well as MR on salsa. > > > > > > https://salsa.debian.org/pkg-netfilter-team/pkg-nftables/-/merge_requests/11 > > (note not touching thte salsa-ci part was deliberate, but to make the > > piuparts test one would need adjustment of the target release. But as > > itwas not done for the +deb12u1 itself, I have not touched this) > > > > The same would be needed OTOH for bullseye as well. > > > Hi Salvatore, > > thanks for working on this. I just approved the salsa MR > > Please go ahead an upload to the archive via NMU as required ASAP. I won't > be near the keyboard today. The work for bookworm has been done, but for bullseye: would you be able to help here and prepare the fixes? Unfortunatlly the fixes will not apply cleanly. If we fear to much breakage, maybe upstream can be convinced to help? Regards, Salvatore
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Hi ARturo, On Sat, Sep 16, 2023 at 09:02:34AM +0200, Arturo Borrero Gonzalez wrote: > On Sat, Sep 16, 2023, 08:37 Salvatore Bonaccorso wrote: > > > Hi > > > > Dropping some recipients for the Debian specific handling of this > > issue. So AFAIU upstream will not consider this on src:linux side to > > be further handled and needs to be addressed in nftables. > > > > Arturo: With the patches provided I prepared (as Timo) an update > > targetting bookworm for the next point release (bug for release.d.o to > > be submitted soon). > > > > Attached is the proposed debdiff, ans as well as MR on salsa. > > > > > > https://salsa.debian.org/pkg-netfilter-team/pkg-nftables/-/merge_requests/11 > > (note not touching thte salsa-ci part was deliberate, but to make the > > piuparts test one would need adjustment of the target release. But as > > itwas not done for the +deb12u1 itself, I have not touched this) > > > > The same would be needed OTOH for bullseye as well. > > > Hi Salvatore, > > thanks for working on this. I just approved the salsa MR > > Please go ahead an upload to the archive via NMU as required ASAP. I won't > be near the keyboard today. > > really appreciated, Thank to you! I just filled the request for the release team, and uploaded the version (following the procedure where one is almost sure that the upload can be accepted as is). I have as well pushed a signed tag to my repo in case you want to fetch that as well. Next we need to look as well to bullseye, feat that it will be more complex there. Regards, Salvatore
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
On Sat, Sep 16, 2023, 08:37 Salvatore Bonaccorso wrote: > Hi > > Dropping some recipients for the Debian specific handling of this > issue. So AFAIU upstream will not consider this on src:linux side to > be further handled and needs to be addressed in nftables. > > Arturo: With the patches provided I prepared (as Timo) an update > targetting bookworm for the next point release (bug for release.d.o to > be submitted soon). > > Attached is the proposed debdiff, ans as well as MR on salsa. > > > https://salsa.debian.org/pkg-netfilter-team/pkg-nftables/-/merge_requests/11 > (note not touching thte salsa-ci part was deliberate, but to make the > piuparts test one would need adjustment of the target release. But as > itwas not done for the +deb12u1 itself, I have not touched this) > > The same would be needed OTOH for bullseye as well. Hi Salvatore, thanks for working on this. I just approved the salsa MR Please go ahead an upload to the archive via NMU as required ASAP. I won't be near the keyboard today. really appreciated, thanks, regards.
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Hi
Dropping some recipients for the Debian specific handling of this
issue. So AFAIU upstream will not consider this on src:linux side to
be further handled and needs to be addressed in nftables.
Arturo: With the patches provided I prepared (as Timo) an update
targetting bookworm for the next point release (bug for release.d.o to
be submitted soon).
Attached is the proposed debdiff, ans as well as MR on salsa.
https://salsa.debian.org/pkg-netfilter-team/pkg-nftables/-/merge_requests/11
(note not touching thte salsa-ci part was deliberate, but to make the
piuparts test one would need adjustment of the target release. But as
itwas not done for the +deb12u1 itself, I have not touched this)
The same would be needed OTOH for bullseye as well.
Regards,
Salvatore
diff -Nru nftables-1.0.6/debian/changelog nftables-1.0.6/debian/changelog
--- nftables-1.0.6/debian/changelog 2023-06-20 16:55:52.0 +0200
+++ nftables-1.0.6/debian/changelog 2023-09-16 07:47:15.0 +0200
@@ -1,3 +1,13 @@
+nftables (1.0.6-2+deb12u2) bookworm; urgency=medium
+
+ * [136245a] Fix incorrect bytecode generation hit with new kernel check that
+rejects adding rules to bound chains (Closes: #1051592)
+- rule: add helper function to expand chain rules intoi commands
+- rule: expand standalone chain that contains rules
+- src: expand table command before evaluation
+
+ -- Salvatore Bonaccorso Sat, 16 Sep 2023 07:47:15 +0200
+
nftables (1.0.6-2+deb12u1) bookworm; urgency=medium
* [7edf72e] d/patches: add 0001-debian-bug-1038724.patch (Closes: #1038724)
diff -Nru
nftables-1.0.6/debian/patches/rule-add-helper-function-to-expand-chain-rules-into-.patch
nftables-1.0.6/debian/patches/rule-add-helper-function-to-expand-chain-rules-into-.patch
---
nftables-1.0.6/debian/patches/rule-add-helper-function-to-expand-chain-rules-into-.patch
1970-01-01 01:00:00.0 +0100
+++
nftables-1.0.6/debian/patches/rule-add-helper-function-to-expand-chain-rules-into-.patch
2023-09-16 07:47:15.0 +0200
@@ -0,0 +1,82 @@
+From 4e5b0a64227dde250f94bec45b3fb127d78b7fd2 Mon Sep 17 00:00:00 2001
+From: Pablo Neira Ayuso
+Date: Mon, 6 Feb 2023 15:28:40 +0100
+Subject: [PATCH 1/3,nft] rule: add helper function to expand chain rules intoi
+ commands
+
+[ upstream commit 784597a4ed63b9decb10d74fdb49a1b021e22728 ]
+
+This patch adds a helper function to expand chain rules into commands.
+This comes in preparation for the follow up patch.
+
+Signed-off-by: Pablo Neira Ayuso
+---
+ src/rule.c | 39 ++-
+ 1 file changed, 22 insertions(+), 17 deletions(-)
+
+diff --git a/src/rule.c b/src/rule.c
+index 1402210acd8d..43c6520517ce 100644
+--- a/src/rule.c
b/src/rule.c
+@@ -1310,13 +1310,31 @@ void cmd_add_loc(struct cmd *cmd, uint16_t offset,
const struct location *loc)
+ cmd->num_attrs++;
+ }
+
++static void nft_cmd_expand_chain(struct chain *chain, struct list_head
*new_cmds)
++{
++ struct rule *rule;
++ struct handle h;
++ struct cmd *new;
++
++ list_for_each_entry(rule, >rules, list) {
++ memset(, 0, sizeof(h));
++ handle_merge(, >handle);
++ if (chain->flags & CHAIN_F_BINDING) {
++ rule->handle.chain_id = chain->handle.chain_id;
++ rule->handle.chain.location = chain->location;
++ }
++ new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, ,
++ >location, rule_get(rule));
++ list_add_tail(>list, new_cmds);
++ }
++}
++
+ void nft_cmd_expand(struct cmd *cmd)
+ {
+ struct list_head new_cmds;
+ struct flowtable *ft;
+ struct table *table;
+ struct chain *chain;
+- struct rule *rule;
+ struct set *set;
+ struct obj *obj;
+ struct cmd *new;
+@@ -1362,22 +1380,9 @@ void nft_cmd_expand(struct cmd *cmd)
+ >location, flowtable_get(ft));
+ list_add_tail(>list, _cmds);
+ }
+- list_for_each_entry(chain, >chains, list) {
+- list_for_each_entry(rule, >rules, list) {
+- memset(, 0, sizeof(h));
+- handle_merge(, >handle);
+- if (chain->flags & CHAIN_F_BINDING) {
+- rule->handle.chain_id =
+- chain->handle.chain_id;
+- rule->handle.chain.location =
+- chain->location;
+- }
+- new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, ,
+- >location,
+- rule_get(rule));
+- list_add_tail(>list, _cmds);
+- }
+- }
++
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Hi, Salvatore Bonaccorso schrieb am 12.09.2023 21:13 (GMT +02:00): > Hi Timo, > > On Tue, Sep 12, 2023 at 01:39:59PM +0200, Timo Sigurdsson wrote: >> Hi Pablo, >> >> Pablo Neira Ayuso schrieb am 12.09.2023 00:57 (GMT +02:00): >> >> > Hi Timo, >> > >> > On Mon, Sep 11, 2023 at 11:37:50PM +0200, Timo Sigurdsson wrote: >> >> Hi, >> >> >> >> recently, Debian updated their stable kernel from 6.1.38 to 6.1.52 >> >> which broke nftables ruleset loading on one of my machines with lots >> >> of "Operation not supported" errors. I've reported this to the >> >> Debian project (see link below) and Salvatore Bonaccorso and I >> >> identified "netfilter: nf_tables: disallow rule addition to bound >> >> chain via NFTA_RULE_CHAIN_ID" (0ebc1064e487) as the offending commit >> >> that introduced the regression. Salvatore also found that this issue >> >> affects the 5.10 stable tree as well (observed in 5.10.191), but he >> >> cannot reproduce it on 6.4.13 and 6.5.2. >> >> >> >> The issue only occurs with some rulesets. While I can't trigger it >> >> with simple/minimal rulesets that I use on some machines, it does >> >> occur with a more complex ruleset that has been in use for months >> >> (if not years, for large parts of it). I'm attaching a somewhat >> >> stripped down version of the ruleset from the machine I originally >> >> observed this issue on. It's still not a small or simple ruleset, >> >> but I'll try to reduce it further when I have more time. >> >> >> >> The error messages shown when trying to load the ruleset don't seem >> >> to be helpful. Just two simple examples: Just to give two simple >> >> examples from the log when nftables fails to start: >> >> /etc/nftables.conf:99:4-44: Error: Could not process rule: Operation not >> >> supported >> >> tcp option maxseg size 1-500 counter drop >> >> ^ >> >> /etc/nftables.conf:308:4-27: Error: Could not process rule: Operation not >> >> supported >> >> tcp dport sip-tls accept >> >> >> > >> > I can reproduce this issue with 5.10.191 and 6.1.52 and nftables v1.0.6, >> > this is not reproducible with v1.0.7 and v1.0.8. >> > >> >> Since the issue only affects some stable trees, Salvatore thought it >> >> might be an incomplete backport that causes this. >> >> >> >> If you need further information, please let me know. >> > >> > Userspace nftables v1.0.6 generates incorrect bytecode that hits a new >> > kernel check that rejects adding rules to bound chains. The incorrect >> > bytecode adds the chain binding, attach it to the rule and it adds the >> > rules to the chain binding. I have cherry-picked these three patches >> > for nftables v1.0.6 userspace and your ruleset restores fine. >> >> hmm, that doesn't explain why Salvatore didn't observe this with >> more recent kernels. >> >> Salvatore, did you use newer userspace components when you tested >> your 6.4.13 and 6.5.2 builds? > > It does explain now because understanding the issue better. While one > while experinting should only change each one constraint for the > 6.4.13 and 6.5.2 testing I indeed switched to a Debian unstable > system, which has newer userpace nftables and so not triggering the > issue. This was missleading for the report. > >> As for the regression and how it be dealt with: Personally, I don't >> really care whether the regression is solved in the kernel or >> userspace. If everybody agrees that this is the best or only viable >> option and Debian decides to push a nftables update to fix this, >> that works for me. But I do feel the burden to justify this should >> be high. A kernel change that leaves users without a working packet >> filter after upgrading their machines is serious, if you ask me. And >> since it affects several stable/longterm trees, I would assume this >> will hit other stable (non-rolling) distributions as well, since >> they will also use older userspace components (unless this is >> behavior specific to nftables 1.0.6 but not older versions). They >> probably should get a heads up then. > > So if it is generally believed on kernel side there should not happen > any further changes to work with older userland, I guess in Debian we > will need to patch nftables. I'm CC'ing Arturo Borrero Gonzalez > , maintainer for the package. The update should go > ideally in the next point releases from October (and maybe released > earlier as well trough the stable-updates mechanism). So, I built nftables 1.0.6-2+deb12u1 with the three cherry-picked patches from Pablo and can confirm that they resolve the issue for me on bookworm. I can now run linux 6.1.52-1 and load my original nftables ruleset again. > FWIW: In Debian bullseye we have 0.9.8 based nftables, in bookworm > 1.0.6, so both will need those fixes. > > As 0ebc1064e487 is to address CVE-2023-4147 other distros picking the > fix will likely
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Hi Timo, On Tue, Sep 12, 2023 at 01:39:59PM +0200, Timo Sigurdsson wrote: > Hi Pablo, > > Pablo Neira Ayuso schrieb am 12.09.2023 00:57 (GMT +02:00): > > > Hi Timo, > > > > On Mon, Sep 11, 2023 at 11:37:50PM +0200, Timo Sigurdsson wrote: > >> Hi, > >> > >> recently, Debian updated their stable kernel from 6.1.38 to 6.1.52 > >> which broke nftables ruleset loading on one of my machines with lots > >> of "Operation not supported" errors. I've reported this to the > >> Debian project (see link below) and Salvatore Bonaccorso and I > >> identified "netfilter: nf_tables: disallow rule addition to bound > >> chain via NFTA_RULE_CHAIN_ID" (0ebc1064e487) as the offending commit > >> that introduced the regression. Salvatore also found that this issue > >> affects the 5.10 stable tree as well (observed in 5.10.191), but he > >> cannot reproduce it on 6.4.13 and 6.5.2. > >> > >> The issue only occurs with some rulesets. While I can't trigger it > >> with simple/minimal rulesets that I use on some machines, it does > >> occur with a more complex ruleset that has been in use for months > >> (if not years, for large parts of it). I'm attaching a somewhat > >> stripped down version of the ruleset from the machine I originally > >> observed this issue on. It's still not a small or simple ruleset, > >> but I'll try to reduce it further when I have more time. > >> > >> The error messages shown when trying to load the ruleset don't seem > >> to be helpful. Just two simple examples: Just to give two simple > >> examples from the log when nftables fails to start: > >> /etc/nftables.conf:99:4-44: Error: Could not process rule: Operation not > >> supported > >> tcp option maxseg size 1-500 counter drop > >> ^ > >> /etc/nftables.conf:308:4-27: Error: Could not process rule: Operation not > >> supported > >> tcp dport sip-tls accept > >> > > > > I can reproduce this issue with 5.10.191 and 6.1.52 and nftables v1.0.6, > > this is not reproducible with v1.0.7 and v1.0.8. > > > >> Since the issue only affects some stable trees, Salvatore thought it > >> might be an incomplete backport that causes this. > >> > >> If you need further information, please let me know. > > > > Userspace nftables v1.0.6 generates incorrect bytecode that hits a new > > kernel check that rejects adding rules to bound chains. The incorrect > > bytecode adds the chain binding, attach it to the rule and it adds the > > rules to the chain binding. I have cherry-picked these three patches > > for nftables v1.0.6 userspace and your ruleset restores fine. > > hmm, that doesn't explain why Salvatore didn't observe this with > more recent kernels. > > Salvatore, did you use newer userspace components when you tested > your 6.4.13 and 6.5.2 builds? It does explain now because understanding the issue better. While one while experinting should only change each one constraint for the 6.4.13 and 6.5.2 testing I indeed switched to a Debian unstable system, which has newer userpace nftables and so not triggering the issue. This was missleading for the report. > As for the regression and how it be dealt with: Personally, I don't > really care whether the regression is solved in the kernel or > userspace. If everybody agrees that this is the best or only viable > option and Debian decides to push a nftables update to fix this, > that works for me. But I do feel the burden to justify this should > be high. A kernel change that leaves users without a working packet > filter after upgrading their machines is serious, if you ask me. And > since it affects several stable/longterm trees, I would assume this > will hit other stable (non-rolling) distributions as well, since > they will also use older userspace components (unless this is > behavior specific to nftables 1.0.6 but not older versions). They > probably should get a heads up then. So if it is generally believed on kernel side there should not happen any further changes to work with older userland, I guess in Debian we will need to patch nftables. I'm CC'ing Arturo Borrero Gonzalez , maintainer for the package. The update should go ideally in the next point releases from October (and maybe released earlier as well trough the stable-updates mechanism). FWIW: In Debian bullseye we have 0.9.8 based nftables, in bookworm 1.0.6, so both will need those fixes. As 0ebc1064e487 is to address CVE-2023-4147 other distros picking the fix will likely encounter the problem at some point. It looks Red Hat has taken it (some RHSA's were released), I assume Ubuntu will shortly as well release USN's containing a fix. Regards, Salvatore
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
On Tue, Sep 12, 2023 at 01:39:59PM +0200, Timo Sigurdsson wrote: > Hi Pablo, > > Pablo Neira Ayuso schrieb am 12.09.2023 00:57 (GMT +02:00): > > > Hi Timo, > > > > On Mon, Sep 11, 2023 at 11:37:50PM +0200, Timo Sigurdsson wrote: > >> Hi, > >> > >> recently, Debian updated their stable kernel from 6.1.38 to 6.1.52 > >> which broke nftables ruleset loading on one of my machines with lots > >> of "Operation not supported" errors. I've reported this to the > >> Debian project (see link below) and Salvatore Bonaccorso and I > >> identified "netfilter: nf_tables: disallow rule addition to bound > >> chain via NFTA_RULE_CHAIN_ID" (0ebc1064e487) as the offending commit > >> that introduced the regression. Salvatore also found that this issue > >> affects the 5.10 stable tree as well (observed in 5.10.191), but he > >> cannot reproduce it on 6.4.13 and 6.5.2. > >> > >> The issue only occurs with some rulesets. While I can't trigger it > >> with simple/minimal rulesets that I use on some machines, it does > >> occur with a more complex ruleset that has been in use for months > >> (if not years, for large parts of it). I'm attaching a somewhat > >> stripped down version of the ruleset from the machine I originally > >> observed this issue on. It's still not a small or simple ruleset, > >> but I'll try to reduce it further when I have more time. > >> > >> The error messages shown when trying to load the ruleset don't seem > >> to be helpful. Just two simple examples: Just to give two simple > >> examples from the log when nftables fails to start: > >> /etc/nftables.conf:99:4-44: Error: Could not process rule: Operation not > >> supported > >> tcp option maxseg size 1-500 counter drop > >> ^ > >> /etc/nftables.conf:308:4-27: Error: Could not process rule: Operation not > >> supported > >> tcp dport sip-tls accept > >> > > > > I can reproduce this issue with 5.10.191 and 6.1.52 and nftables v1.0.6, > > this is not reproducible with v1.0.7 and v1.0.8. > > > >> Since the issue only affects some stable trees, Salvatore thought it > >> might be an incomplete backport that causes this. > >> > >> If you need further information, please let me know. > > > > Userspace nftables v1.0.6 generates incorrect bytecode that hits a new > > kernel check that rejects adding rules to bound chains. The incorrect > > bytecode adds the chain binding, attach it to the rule and it adds the > > rules to the chain binding. I have cherry-picked these three patches > > for nftables v1.0.6 userspace and your ruleset restores fine. > > hmm, that doesn't explain why Salvatore didn't observe this with > more recent kernels. > > Salvatore, did you use newer userspace components when you tested > your 6.4.13 and 6.5.2 builds? > > As for the regression and how it be dealt with: Personally, I don't > really care whether the regression is solved in the kernel or > userspace. If everybody agrees that this is the best or only viable > option and Debian decides to push a nftables update to fix this, > that works for me. But I do feel the burden to justify this should > be high. A kernel change that leaves users without a working packet > filter after upgrading their machines is serious, if you ask me. And > since it affects several stable/longterm trees, I would assume this > will hit other stable (non-rolling) distributions as well, since > they will also use older userspace components (unless this is > behavior specific to nftables 1.0.6 but not older versions). They > probably should get a heads up then. There is coverage for the chain binding feature in our tests infrastructure, but unfortunately the bug did not trigger with newer nftables versions. In the future, I will keep an eye with running our tests on older userspace nftables versions when working on stable trees.
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Timo Sigurdsson wrote: > > Linux regression tracking (Thorsten Leemhuis) > > wrote: > >> On 12.09.23 00:57, Pablo Neira Ayuso wrote: > >> > Userspace nftables v1.0.6 generates incorrect bytecode that hits a new > >> > kernel check that rejects adding rules to bound chains. The incorrect > >> > bytecode adds the chain binding, attach it to the rule and it adds the > >> > rules to the chain binding. I have cherry-picked these three patches > >> > for nftables v1.0.6 userspace and your ruleset restores fine. > >> > [...] > >> > >> H. Well, this sounds like a kernel regression to me that normally > >> should be dealt with on the kernel level, as users after updating the > >> kernel should never have to update any userspace stuff to continue what > >> they have been doing before the kernel update. > > > > This is a combo of a userspace bug and this new sanity check that > > rejects the incorrect ordering (adding rules to the already-bound > > anonymous chain). > > > > Out of curiosity, did the incorrect ordering or bytecode from the older > userspace components actually lead to a wrong representation of the rules in > the kernel or did the rules still work despite all that? It works, but without the stricter behaviour userspace can trigger memory corruption in the kernel. nftables userland will not trigger this.
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Hi, Florian Westphal schrieb am 12.09.2023 12:27 (GMT +02:00): > Linux regression tracking (Thorsten Leemhuis) > wrote: >> On 12.09.23 00:57, Pablo Neira Ayuso wrote: >> > Userspace nftables v1.0.6 generates incorrect bytecode that hits a new >> > kernel check that rejects adding rules to bound chains. The incorrect >> > bytecode adds the chain binding, attach it to the rule and it adds the >> > rules to the chain binding. I have cherry-picked these three patches >> > for nftables v1.0.6 userspace and your ruleset restores fine. >> > [...] >> >> H. Well, this sounds like a kernel regression to me that normally >> should be dealt with on the kernel level, as users after updating the >> kernel should never have to update any userspace stuff to continue what >> they have been doing before the kernel update. > > This is a combo of a userspace bug and this new sanity check that > rejects the incorrect ordering (adding rules to the already-bound > anonymous chain). > Out of curiosity, did the incorrect ordering or bytecode from the older userspace components actually lead to a wrong representation of the rules in the kernel or did the rules still work despite all that? Thanks, Timo
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Hi Pablo, Pablo Neira Ayuso schrieb am 12.09.2023 00:57 (GMT +02:00): > Hi Timo, > > On Mon, Sep 11, 2023 at 11:37:50PM +0200, Timo Sigurdsson wrote: >> Hi, >> >> recently, Debian updated their stable kernel from 6.1.38 to 6.1.52 >> which broke nftables ruleset loading on one of my machines with lots >> of "Operation not supported" errors. I've reported this to the >> Debian project (see link below) and Salvatore Bonaccorso and I >> identified "netfilter: nf_tables: disallow rule addition to bound >> chain via NFTA_RULE_CHAIN_ID" (0ebc1064e487) as the offending commit >> that introduced the regression. Salvatore also found that this issue >> affects the 5.10 stable tree as well (observed in 5.10.191), but he >> cannot reproduce it on 6.4.13 and 6.5.2. >> >> The issue only occurs with some rulesets. While I can't trigger it >> with simple/minimal rulesets that I use on some machines, it does >> occur with a more complex ruleset that has been in use for months >> (if not years, for large parts of it). I'm attaching a somewhat >> stripped down version of the ruleset from the machine I originally >> observed this issue on. It's still not a small or simple ruleset, >> but I'll try to reduce it further when I have more time. >> >> The error messages shown when trying to load the ruleset don't seem >> to be helpful. Just two simple examples: Just to give two simple >> examples from the log when nftables fails to start: >> /etc/nftables.conf:99:4-44: Error: Could not process rule: Operation not >> supported >> tcp option maxseg size 1-500 counter drop >> ^ >> /etc/nftables.conf:308:4-27: Error: Could not process rule: Operation not >> supported >> tcp dport sip-tls accept >> > > I can reproduce this issue with 5.10.191 and 6.1.52 and nftables v1.0.6, > this is not reproducible with v1.0.7 and v1.0.8. > >> Since the issue only affects some stable trees, Salvatore thought it >> might be an incomplete backport that causes this. >> >> If you need further information, please let me know. > > Userspace nftables v1.0.6 generates incorrect bytecode that hits a new > kernel check that rejects adding rules to bound chains. The incorrect > bytecode adds the chain binding, attach it to the rule and it adds the > rules to the chain binding. I have cherry-picked these three patches > for nftables v1.0.6 userspace and your ruleset restores fine. hmm, that doesn't explain why Salvatore didn't observe this with more recent kernels. Salvatore, did you use newer userspace components when you tested your 6.4.13 and 6.5.2 builds? As for the regression and how it be dealt with: Personally, I don't really care whether the regression is solved in the kernel or userspace. If everybody agrees that this is the best or only viable option and Debian decides to push a nftables update to fix this, that works for me. But I do feel the burden to justify this should be high. A kernel change that leaves users without a working packet filter after upgrading their machines is serious, if you ask me. And since it affects several stable/longterm trees, I would assume this will hit other stable (non-rolling) distributions as well, since they will also use older userspace components (unless this is behavior specific to nftables 1.0.6 but not older versions). They probably should get a heads up then. Regards, Timo
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Linux regression tracking (Thorsten Leemhuis) wrote: > On 12.09.23 00:57, Pablo Neira Ayuso wrote: > > Userspace nftables v1.0.6 generates incorrect bytecode that hits a new > > kernel check that rejects adding rules to bound chains. The incorrect > > bytecode adds the chain binding, attach it to the rule and it adds the > > rules to the chain binding. I have cherry-picked these three patches > > for nftables v1.0.6 userspace and your ruleset restores fine. > > [...] > > H. Well, this sounds like a kernel regression to me that normally > should be dealt with on the kernel level, as users after updating the > kernel should never have to update any userspace stuff to continue what > they have been doing before the kernel update. This is a combo of a userspace bug and this new sanity check that rejects the incorrect ordering (adding rules to the already-bound anonymous chain). nf_tables uses a transaction allor-nothing model, this means that any error that occurs during a transaction has to be reverse/undo all the pending changes. This has caused a myriad of bugs already. So while this can be theoretically fixed in the kernel I don't see a sane way to do it. Error unwinding / recovery from deeply nested errors is already too complex for my taste. > Can't the kernel somehow detect the incorrect bytecode and do the right > thing(tm) somehow? Theoretically yes, but I don't feel competent enough to do it, just look at all the UaF bugs of the past month.
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
On 12.09.23 00:57, Pablo Neira Ayuso wrote: > On Mon, Sep 11, 2023 at 11:37:50PM +0200, Timo Sigurdsson wrote: >> >> recently, Debian updated their stable kernel from 6.1.38 to 6.1.52 >> which broke nftables ruleset loading on one of my machines with lots >> of "Operation not supported" errors. I've reported this to the >> Debian project (see link below) and Salvatore Bonaccorso and I >> identified "netfilter: nf_tables: disallow rule addition to bound >> chain via NFTA_RULE_CHAIN_ID" (0ebc1064e487) as the offending commit >> that introduced the regression. Salvatore also found that this issue >> affects the 5.10 stable tree as well (observed in 5.10.191), but he >> cannot reproduce it on 6.4.13 and 6.5.2. >> >> The issue only occurs with some rulesets. While I can't trigger it >> with simple/minimal rulesets that I use on some machines, it does >> occur with a more complex ruleset that has been in use for months >> (if not years, for large parts of it). I'm attaching a somewhat >> stripped down version of the ruleset from the machine I originally >> observed this issue on. It's still not a small or simple ruleset, >> but I'll try to reduce it further when I have more time. >> >> The error messages shown when trying to load the ruleset don't seem >> to be helpful. Just two simple examples: Just to give two simple >> examples from the log when nftables fails to start: >> /etc/nftables.conf:99:4-44: Error: Could not process rule: Operation not >> supported >> tcp option maxseg size 1-500 counter drop >> ^ >> /etc/nftables.conf:308:4-27: Error: Could not process rule: Operation not >> supported >> tcp dport sip-tls accept >> > > I can reproduce this issue with 5.10.191 and 6.1.52 and nftables v1.0.6, > this is not reproducible with v1.0.7 and v1.0.8. > >> Since the issue only affects some stable trees, Salvatore thought it >> might be an incomplete backport that causes this. >> >> If you need further information, please let me know. > > Userspace nftables v1.0.6 generates incorrect bytecode that hits a new > kernel check that rejects adding rules to bound chains. The incorrect > bytecode adds the chain binding, attach it to the rule and it adds the > rules to the chain binding. I have cherry-picked these three patches > for nftables v1.0.6 userspace and your ruleset restores fine. > [...] H. Well, this sounds like a kernel regression to me that normally should be dealt with on the kernel level, as users after updating the kernel should never have to update any userspace stuff to continue what they have been doing before the kernel update. Can't the kernel somehow detect the incorrect bytecode and do the right thing(tm) somehow? But yes, don't worry, I know that reality is not black and white and that it's crucial that things like package filtering do exactly what the user expect it to do; that's why this might be one of those rare situations where "user has to update userspace components to support newer kernels" might be the better of two bad choices. But I had to ask to ensure it's something like that. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page.
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Hi Timo,
On Mon, Sep 11, 2023 at 11:37:50PM +0200, Timo Sigurdsson wrote:
> Hi,
>
> recently, Debian updated their stable kernel from 6.1.38 to 6.1.52
> which broke nftables ruleset loading on one of my machines with lots
> of "Operation not supported" errors. I've reported this to the
> Debian project (see link below) and Salvatore Bonaccorso and I
> identified "netfilter: nf_tables: disallow rule addition to bound
> chain via NFTA_RULE_CHAIN_ID" (0ebc1064e487) as the offending commit
> that introduced the regression. Salvatore also found that this issue
> affects the 5.10 stable tree as well (observed in 5.10.191), but he
> cannot reproduce it on 6.4.13 and 6.5.2.
>
> The issue only occurs with some rulesets. While I can't trigger it
> with simple/minimal rulesets that I use on some machines, it does
> occur with a more complex ruleset that has been in use for months
> (if not years, for large parts of it). I'm attaching a somewhat
> stripped down version of the ruleset from the machine I originally
> observed this issue on. It's still not a small or simple ruleset,
> but I'll try to reduce it further when I have more time.
>
> The error messages shown when trying to load the ruleset don't seem
> to be helpful. Just two simple examples: Just to give two simple
> examples from the log when nftables fails to start:
> /etc/nftables.conf:99:4-44: Error: Could not process rule: Operation not
> supported
> tcp option maxseg size 1-500 counter drop
> ^
> /etc/nftables.conf:308:4-27: Error: Could not process rule: Operation not
> supported
> tcp dport sip-tls accept
>
I can reproduce this issue with 5.10.191 and 6.1.52 and nftables v1.0.6,
this is not reproducible with v1.0.7 and v1.0.8.
> Since the issue only affects some stable trees, Salvatore thought it
> might be an incomplete backport that causes this.
>
> If you need further information, please let me know.
Userspace nftables v1.0.6 generates incorrect bytecode that hits a new
kernel check that rejects adding rules to bound chains. The incorrect
bytecode adds the chain binding, attach it to the rule and it adds the
rules to the chain binding. I have cherry-picked these three patches
for nftables v1.0.6 userspace and your ruleset restores fine.
See patches enclosed to this email.
>From 4e5b0a64227dde250f94bec45b3fb127d78b7fd2 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso
Date: Mon, 6 Feb 2023 15:28:40 +0100
Subject: [PATCH 1/3,nft] rule: add helper function to expand chain rules intoi
commands
[ upstream commit 784597a4ed63b9decb10d74fdb49a1b021e22728 ]
This patch adds a helper function to expand chain rules into commands.
This comes in preparation for the follow up patch.
Signed-off-by: Pablo Neira Ayuso
---
src/rule.c | 39 ++-
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/src/rule.c b/src/rule.c
index 1402210acd8d..43c6520517ce 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1310,13 +1310,31 @@ void cmd_add_loc(struct cmd *cmd, uint16_t offset, const struct location *loc)
cmd->num_attrs++;
}
+static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds)
+{
+ struct rule *rule;
+ struct handle h;
+ struct cmd *new;
+
+ list_for_each_entry(rule, >rules, list) {
+ memset(, 0, sizeof(h));
+ handle_merge(, >handle);
+ if (chain->flags & CHAIN_F_BINDING) {
+ rule->handle.chain_id = chain->handle.chain_id;
+ rule->handle.chain.location = chain->location;
+ }
+ new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, ,
+>location, rule_get(rule));
+ list_add_tail(>list, new_cmds);
+ }
+}
+
void nft_cmd_expand(struct cmd *cmd)
{
struct list_head new_cmds;
struct flowtable *ft;
struct table *table;
struct chain *chain;
- struct rule *rule;
struct set *set;
struct obj *obj;
struct cmd *new;
@@ -1362,22 +1380,9 @@ void nft_cmd_expand(struct cmd *cmd)
>location, flowtable_get(ft));
list_add_tail(>list, _cmds);
}
- list_for_each_entry(chain, >chains, list) {
- list_for_each_entry(rule, >rules, list) {
-memset(, 0, sizeof(h));
-handle_merge(, >handle);
-if (chain->flags & CHAIN_F_BINDING) {
- rule->handle.chain_id =
- chain->handle.chain_id;
- rule->handle.chain.location =
- chain->location;
-}
-new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, ,
- >location,
- rule_get(rule));
-list_add_tail(>list, _cmds);
- }
- }
+ list_for_each_entry(chain, >chains, list)
+ nft_cmd_expand_chain(chain, _cmds);
+
list_splice(_cmds, >list);
break;
case CMD_OBJ_SET:
--
2.30.2
>From 70c03d81df0e87fb416bd1f38409367e9d08ed7f Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso
Date: Mon, 6 Feb 2023 15:28:41 +0100
Subject: [PATCH 2/3,nft] rule: expand standalone chain that contains rules
[ upstream 27c753e4a8d4744f479345e3f5e34cafef751602 commit ]
Bug#1051592: Regression: Commit "netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID" breaks ruleset loading in linux-stable
Hi,
recently, Debian updated their stable kernel from 6.1.38 to 6.1.52 which broke
nftables ruleset loading on one of my machines with lots of "Operation not
supported" errors. I've reported this to the Debian project (see link below)
and Salvatore Bonaccorso and I identified "netfilter: nf_tables: disallow rule
addition to bound chain via NFTA_RULE_CHAIN_ID" (0ebc1064e487) as the offending
commit that introduced the regression. Salvatore also found that this issue
affects the 5.10 stable tree as well (observed in 5.10.191), but he cannot
reproduce it on 6.4.13 and 6.5.2.
The issue only occurs with some rulesets. While I can't trigger it with
simple/minimal rulesets that I use on some machines, it does occur with a more
complex ruleset that has been in use for months (if not years, for large parts
of it). I'm attaching a somewhat stripped down version of the ruleset from the
machine I originally observed this issue on. It's still not a small or simple
ruleset, but I'll try to reduce it further when I have more time.
The error messages shown when trying to load the ruleset don't seem to be
helpful. Just two simple examples:
Just to give two simple examples from the log when nftables fails to start:
/etc/nftables.conf:99:4-44: Error: Could not process rule: Operation not
supported
tcp option maxseg size 1-500 counter drop
^
/etc/nftables.conf:308:4-27: Error: Could not process rule: Operation not
supported
tcp dport sip-tls accept
Since the issue only affects some stable trees, Salvatore thought it might be
an incomplete backport that causes this.
If you need further information, please let me know.
Thanks and kind regards,
Timo
#regzbot introduced: 0ebc1064e487
#regzbot monitor: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051592#!/usr/sbin/nft -f
flush ruleset
define public_if = eth0
define trusted_if = eth1
define voip_if = eth2.10
define guest_if = eth2.20
define home_if = { $trusted_if, $voip_if, $guest_if }
define home_ipv6_if = { $trusted_if, $voip_if, $guest_if }
define masq_ip = { 192.168.1.0/24, 192.168.2.0/24, 192.168.3.0/24,
192.168.4.0/24 }
define masq_if = $public_if
define host1_ip = 192.168.1.10
define host2_ip = 192.168.2.20
define host3_ip = 192.168.3.30
define host4_ip = 192.168.4.40
define proxy_port = 8443
define private_ip = { 192.168.0.0/16, 172.16.0.0/12, 10.0.0.0/8 }
define private_ip6 = { fe80::/64, fd00::/8 }
define bogons_ip = { 0.0.0.0/8, 10.0.0.0/8, 100.64.0.0/10, 127.0.0.0/8,
169.254.0.0/16, 172.16.0.0/12, 192.0.0.0/24, 192.0.2.0/24, 192.168.0.0/16,
198.18.0.0/15, 198.51.100.0/24, 203.0.113.0/24, 224.0.0.0/3 }
define bogons_ip6 = { ::/3, 2001:0002::/48, 2001:0003::/32, 2001:10::/28,
2001:20::/28, 2001::/32, 2001:db8::/32, 2002::/16, 3000::/4, 4000::/2, 8000::/1
}
define sip_whitelist_ip6 = { 2001:db8::1/128, 2001:db8::2/128 }
define smtps_whitelist_ip = 10.0.0.1
define protocol_whitelist = { tcp, udp, icmp, ipv6-icmp }
table inet filter {
map if_input {
type ifname : verdict;
elements = { $public_if : jump public_input, $trusted_if : jump
home_input, $voip_if : jump home_input, $guest_if : jump home_input }
}
map if_forward {
type ifname : verdict;
elements = { $public_if : jump public_forward, $trusted_if :
jump trusted_forward, $voip_if : jump voip_forward, $guest_if : jump
guest_forward }
}
map if_output {
type ifname : verdict;
elements = { $public_if : jump public_output, $trusted_if :
jump home_output, $voip_if : jump home_output, $guest_if : jump home_output }
}
set ipv4_blacklist { type ipv4_addr; flags interval; auto-merge; }
set ipv6_blacklist { type ipv6_addr; flags interval; auto-merge; }
set limit_src_ip { type ipv4_addr; flags dynamic, timeout; size 1024; }
set limit_src_ip6 { type ipv6_addr; flags dynamic, timeout; size 1024; }
chain PREROUTING_RAW {
type filter hook prerouting priority raw;
meta l4proto != $protocol_whitelist counter drop
tcp flags syn jump {
tcp option maxseg size 1-500 counter drop
tcp sport 0 counter drop
}
rt type 0 counter drop
}
chain PREROUTING_MANGLE {
type filter hook prerouting priority mangle;
ct state vmap { invalid : jump ct_invalid_pre, untracked : jump
ct_untracked_pre, new : jump ct_new_pre, related : jump rpfilter }
}
chain ct_invalid_pre {
counter drop
}
chain ct_untracked_pre {
icmpv6 type { nd-router-solicit, nd-router-advert,
nd-neighbor-solicit, nd-neighbor-advert, mld-listener-query,
