Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: fix network unlock with iptables-nft #2323

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

rst0git
Copy link
Member

@rst0git rst0git commented Jan 5, 2024

When iptables-nft is used as backend for iptables, the rules for network locking are translated into the following nft rules:

$ iptables-restore-translate -f lock.txt
add table ip filter
add chain ip filter CRIU
insert rule ip filter INPUT counter jump CRIU
insert rule ip filter OUTPUT counter jump CRIU
add rule ip filter CRIU mark 0xc114 counter accept
add rule ip filter CRIU counter drop

These rules create the following chains:

table ip filter { # handle 1
	chain CRIU { # handle 1
		meta mark 0x0000c114 counter packets 16 bytes 890 accept # handle 6
		counter packets 1 bytes 60 drop # handle 7
		meta mark 0x0000c114 counter packets 0 bytes 0 accept # handle 8
		counter packets 0 bytes 0 drop # handle 9
	}

	chain INPUT { # handle 2
		type filter hook input priority filter; policy accept;
		counter packets 8 bytes 445 jump CRIU # handle 3
		counter packets 0 bytes 0 jump CRIU # handle 10
	}

	chain OUTPUT { # handle 4
		type filter hook output priority filter; policy accept;
		counter packets 9 bytes 505 jump CRIU # handle 5
		counter packets 0 bytes 0 jump CRIU # handle 11
	}
}

In order to delete the CRIU chain, we need to first delete all four jump targets. Otherwise, -X CRIU would fail with the following error:

iptables-restore v1.8.10 (nf_tables):
line 5: CHAIN_DEL failed (Resource busy): chain CRIU

Fixes: #2313

@codecov-commenter
Copy link

codecov-commenter commented Jan 5, 2024

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (50aa6da) 70.51% compared to head (b7482ae) 70.62%.

❗ Current head b7482ae differs from pull request most recent head e35df4d. Consider uploading reports for the commit e35df4d to get more accurate results

Files Patch % Lines
criu/net.c 76.19% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           criu-dev    #2323      +/-   ##
============================================
+ Coverage     70.51%   70.62%   +0.10%     
============================================
  Files           133      133              
  Lines         33534    33556      +22     
============================================
+ Hits          23646    23698      +52     
+ Misses         9888     9858      -30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rst0git rst0git force-pushed the fix-iptables-unlock branch 4 times, most recently from 9eacd6e to c3d604b Compare January 5, 2024 18:53
When iptables-nft is used as backend for iptables, the rules for
network locking are translated into the following nft rules:

```
$ iptables-restore-translate -f lock.txt
add table ip filter
add chain ip filter CRIU
insert rule ip filter INPUT counter jump CRIU
insert rule ip filter OUTPUT counter jump CRIU
add rule ip filter CRIU mark 0xc114 counter accept
add rule ip filter CRIU counter drop
```

These rules create the following chains:

```
table ip filter { # handle 1
	chain CRIU { # handle 1
		meta mark 0x0000c114 counter packets 16 bytes 890 accept # handle 6
		counter packets 1 bytes 60 drop # handle 7
		meta mark 0x0000c114 counter packets 0 bytes 0 accept # handle 8
		counter packets 0 bytes 0 drop # handle 9
	}

	chain INPUT { # handle 2
		type filter hook input priority filter; policy accept;
		counter packets 8 bytes 445 jump CRIU # handle 3
		counter packets 0 bytes 0 jump CRIU # handle 10
	}

	chain OUTPUT { # handle 4
		type filter hook output priority filter; policy accept;
		counter packets 9 bytes 505 jump CRIU # handle 5
		counter packets 0 bytes 0 jump CRIU # handle 11
	}
}
```

In order to delete the CRIU chain, we need to first delete all four
jump targets. Otherwise, `-X CRIU` would fail with the following error:

iptables-restore v1.8.10 (nf_tables):
line 5: CHAIN_DEL failed (Resource busy): chain CRIU

Reported-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
@rst0git rst0git force-pushed the fix-iptables-unlock branch 3 times, most recently from 0ac0e13 to cbf0e64 Compare January 7, 2024 18:27
@rst0git rst0git requested a review from mihalicyn January 7, 2024 18:31
@rst0git rst0git force-pushed the fix-iptables-unlock branch 2 times, most recently from fcfdc90 to 9069e76 Compare January 8, 2024 00:02
nft does not support xtables compat expressions
https://git.netfilter.org/nftables/commit/?id=79195a8cc9e9d9cf2d17165bf07ac4cc9d55539f

Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Show appropriate error messages when restore of nftables fails.

Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
@mihalicyn
Copy link
Member

Great job, Radostin!

LGTM.

to discuss: probably at some point it makes sense to change NETWORK_LOCK_DEFAULT value to NETWORK_LOCK_NFTABLES.

@avagin avagin merged commit 8f4430d into checkpoint-restore:criu-dev Jan 17, 2024
37 of 39 checks passed
@rst0git rst0git deleted the fix-iptables-unlock branch January 17, 2024 10:28
criu/net.c Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alpine test/build fails
5 participants