WIP: firewall: add nftables backend #462

greenpau · 2020-03-15T19:25:56Z

Resolves: #461

greenpau · 2020-03-16T03:09:53Z

@mars1024 , could you please take a look at this PR? 🙏 It is almost ready (pending adding tests), but I did update README.md.

mars1024 · 2020-03-16T04:23:18Z

@mars1024 , could you please take a look at this PR? 🙏 It is almost ready (pending adding tests), but I did update README.md.

Of course, and you should add some tests later.

mars1024 · 2020-03-16T04:23:43Z

BTW, cc firewall plugin author @dcbw

greenpau · 2020-03-17T02:04:52Z

@mars1024 , I tried ./test_linux.sh, but it tests all plugins, not just firewall.

mars1024 · 2020-03-17T05:25:42Z

ginko can also work with go test, please try go test github.com/containernetworking/plugins/plugins/meta/firewall

plugins/meta/firewall/README.md

plugins/meta/firewall/nftables.go

mars1024 · 2020-03-18T11:54:52Z

We need more tests @greenpau

greenpau · 2020-03-18T15:30:45Z

We need more tests @greenpau

@mars1024 , working on ti. Thank you for the review!

greenpau · 2020-03-19T17:17:55Z

How about move these rules into a new chain of filter table? just like iptables backend

@mars1024, after thinking about it a bit, it may require its own chain .. for consistency.

What would the logic behind naming the individual chains? For example, I see IP masquarade chaing names in the following fashion. I guess, what is the logic behind 586108da8523562bc2ca795b ?

table ip nat { # handle 27

        chain POSTROUTING { # handle 3
                ip saddr 192.168.124.145  counter packets 0 bytes 0 jump CNI-586108da8523562bc2ca795b # handle 562

        chain CNI-586108da8523562bc2ca795b { # handle 559
                ip daddr 192.168.124.0/24  counter packets 0 bytes 0 accept # handle 560
                ip daddr != 224.0.0.0/4  counter packets 0 bytes 0 masquerade  # handle 561
        }

greenpau · 2020-03-19T19:51:33Z

@mars1024, after thinking about it a bit, it may require its own chain .. for consistency.

@mars1024 , running into an issue implementing it as a chain. how many chain would you create for the below rules?

nft insert rule filter FORWARD position 51 oifname "cni-podman0" ip daddr 192.168.124.0/24 ct state established,related counter packets 0 bytes 0 accept
nft insert rule filter FORWARD position 51 iifname "cni-podman0" ip saddr 192.168.124.0/24 counter packets 0 bytes 0 accept

Do you create a chain for interface? e.g. CNI-FORWARD-PODMAN0? them sub-chain for each IP address?

I am still think it could be proper to do it with a subnet match.

greenpau · 2020-07-30T15:28:10Z

Back on this PR.

greenpau · 2020-07-31T03:08:26Z

Pivoting from invoking nft via shell to using github.com/google/nftables.

Found decent blog posts on the subject:

dcbw · 2020-08-05T15:22:20Z

@erig0 can you review this one too? thanks!

greenpau · 2020-08-05T15:38:10Z

@erig0 can you review this one too? thanks!

@mars1024 , @dcbw , @erig0, it might actually not work due to the fact the containernetworking/plugins using older version of Go. Please see this comment: google/nftables#110 (comment). Would love your input as to how to resolve this.

dcbw · 2020-08-05T15:50:36Z

@greenpau we can certainly bump the required Go version of the CNI plugins repo. Our general policy is Go versions N and N-1, which means we should support Go 1.13 and Go 1.14. If it's possible to support 1.12 that's great, but not required. Anything 1.11 and lower is not supported.

pushed as a PR in #521

greenpau · 2020-08-05T21:17:02Z

we can certainly bump the required Go version of the CNI plugins repo.

@dcbw , that would be great!

erig0 · 2020-08-06T20:14:06Z

Pivoting from invoking nft via shell to using github.com/google/nftables.

@greenpau, Another option is to use libnftables. This what nft uses. firewalld also uses the JSON support from libnftables.

erig0 · 2020-08-05T20:27:28Z

plugins/meta/firewall/README.md

+```
+
+Prior to the invocation of CNI `firewall` plugin, the `FORWARD` chain in `filter`
+table might be configured be as follows:


This is not guaranteed to exist. It will likely exist on any machine that uses the iptables-nft variant of iptables as it implicitly create this table/chain. I would not assume the table/chain exists.

CNI could use its own table and set of chains and thus avoid many interaction issues with iptables-nft. However, a separate table and chains has its own issue related to netfilter hooks. The tl;dr is nftables allows multiple chains to use the same netfilter hook. A consequence of this is that packets that are accepted are still subject to the rules of other chains hooked into the hook type.

This is not guaranteed to exist. It will likely exist on any machine that uses the iptables-nft variant of iptables as it implicitly create this table/chain. I would not assume the table/chain exists.

@erig0, I can create a chain. that is I do it when testing changes.

@dcbw , @mars1024 , agreed?

erig0 · 2020-08-06T20:22:11Z

plugins/meta/firewall/README.md

+        chain cnins-3-4026550857-dummy0 {
+                oifname "dummy0" ip daddr 192.168.100.100 ct state established,related counter packets 0 bytes 0 accept
+                iifname "dummy0" ip saddr 192.168.100.100 counter packets 0 bytes 0 accept
+                iifname "dummy0" oifname "dummy0" counter packets 0 bytes 0 accept
+        }
+}
+table ip6 filter {
+        chain FORWARD {
+                type filter hook forward priority filter; policy drop;
+                jump cnins-3-4026550857-dummy0
+        }
+
+        chain cnins-3-4026550857-dummy0 {
+                oifname "dummy0" ip6 daddr 2001:db8:100:100::1 ct state established,related counter packets 0 bytes 0 accept
+                iifname "dummy0" ip6 saddr 2001:db8:100:100::1 counter packets 0 bytes 0 accept
+                iifname "dummy0" oifname "dummy0" counter packets 0 bytes 0 accept
+        }


Ideally these would be combined into one chain in a table of family inet. I don't think there is value in splitting them up. It can probably avoid some duplicate rules.

Ideally these would be combined into one chain in a table of family inet. I don't think there is value in splitting them up. It can probably avoid some duplicate rules.

@erig0, please see @mars1024 comment that he wanted a jump.

I don't think there is value in splitting them up. It can probably avoid some duplicate rules.

@erig0, it has a value in a sense that it is easier to delete the rule when you have a jump like this. It would be very hard to parse each rule.

I get the jump to a dedicated chain. That 100% makes sense. I was saying I don't think you need to split the "ip" and "ip6" families. You could use "inet" which covers both IPv4 and IPv6.

I get the jump to a dedicated chain. That 100% makes sense. I was saying I don't think you need to split the "ip" and "ip6" families. You could use "inet" which covers both IPv4 and IPv6.

@erig0, that I could do. 👍 It makes sense because IPv4/6 are all a part of the same construct.

greenpau · 2020-08-06T20:57:55Z

From failed tests:

  Unexpected error:
      <*errors.errorString | 0xc000085fa0>: {
          s: "nftBackend.Add() error: failed parsing netfilter tables: nftables table filter not found",
      }
      nftBackend.Add() error: failed parsing netfilter tables: nftables table filter not found
  occurred

Now most of the tests pass. Noticed the issue with table filter not found. Per comment by @erig0 , need to add the chain creation.

greenpau · 2020-08-07T21:27:17Z

just noticed an issue with chanName. I forgot to add ContainerID in the mix.

greenpau · 2020-08-10T03:45:35Z

another issue is with the interface name ... I take the interface name of the container. It should be the interface name of the bridge ... the very first interface of in the list.

greenpau · 2020-08-10T20:38:38Z

Added working version of nftables plugin in https://github.com/greenpau/cni-plugins#cni-nftables-firewall. Still testing it.

PavelSosin-320 · 2020-09-10T10:23:52Z

Why firewall configuration is needed when Podman runs inside a VM container, like WSL2 ? A firewall is always supplied by the Host node. The communication between the Podman network and interface Eth0 provided dynamically by the Virtual switch doesn't need any filtering. In the overlay network, the entire local network can be protected by the router if it runs OpenWRT or Free Balena.

06kellyjac · 2020-09-10T11:40:37Z

Why firewall configuration is needed when Podman runs inside a VM container, like WSL2 ? A firewall is always supplied by the Host node. The communication between the Podman network and interface Eth0 provided dynamically by the Virtual switch doesn't need any filtering. In the overlay network, the entire local network can be protected by the router if it runs OpenWRT or Free Balena.
@PavelSosin-320

In this context the firewall plugin is how networking is set up for the containers. This is separate to what you refer to as the "overlay network".
You can use a "fully featured" firewall like firewalld or, as most people use containers, it'll actually be using iptables (a packet filter).
This PR is to add nftables as an option (a different packet filter).

You can read up on the CNI firewall plugin here: https://github.com/containernetworking/plugins/blob/master/plugins/meta/firewall/README.md

You can already use podman with iptables in WSL2 without this PR, as long as your linux distro still uses iptables.

If I got anything wrong here let me know

dcbw · 2020-09-10T13:59:21Z

another issue is with the interface name ... I take the interface name of the container. It should be the interface name of the bridge ... the very first interface of in the list.

Note that this won't always be the first interface... we've struggled with this for other plugins (eg bandwidth) too, and don't have a great answer yet because some plugins might not have a host-side network interface, like VMs that use tap. But in any case, take a look at what the bandwidth plugin does; I think it takes the first non-sandbox interface in the list. If you just look for the first interface, it may be the container interface, so you have to look at the "Sandbox" key.

greenpau · 2020-09-10T14:13:08Z

Note that this won't always be the first interface...

@dcbw , I reworked the plugin completely ... please see https://github.com/greenpau/cni-plugins/tree/main/pkg/firewall

I was thinking of linking from here to https://github.com/greenpau/cni-plugins/blob/6b7a838e15f159e8695df59b4f4f722ca3f49118/pkg/firewall/cmd.go#L12. Any objections?

Also, on a separate note, I found out that portmap will not work on RHEL 7, just RHEL 8.

cjschroeder · 2021-06-16T13:08:53Z

Has there been any movement on this front? Nftables support (not firewalld) is a necessary need for those using the nftables service on Fedora, for instance.

SilverBut · 2021-11-21T18:32:51Z

Not sure what's going on there since its now closed. @greenpau Any idea?

greenpau · 2021-11-21T18:37:03Z

Not sure what's going on there since its now closed.

@SilverBut , closed it since I implemented it here: https://github.com/greenpau/cni-plugins

3nprob · 2022-01-22T08:38:31Z

@dcbw @squeed @mars1024 @erig0 Considering that the PR author abandoned getting this merged here and are content with their own fork - what's missing for you (apart from rebasing) to get this PR to a mergable state?

nftables support for firewall is a pretty important feature, and getting moreso as more and more migrate to nft.

rtgiskard · 2022-07-26T11:15:02Z

I'm using nftables with customized rules, and do not want any part of the system to alter the rules itself, so that all the firewall rules can be managed in one place and easy to check.

Is it possible to have an option to disable iptables just like docker?

mccv1r0 · 2022-07-26T13:23:29Z

@rtgiskard

Is it possible to have an option to disable iptables just like docker?

I suggest you create a new issue.

rtgiskard · 2022-07-27T07:41:09Z

@rtgiskard

Is it possible to have an option to disable iptables just like docker?

I suggest you create a new issue.

thank you, here is the new issue

unknowndevQwQ · 2023-03-02T10:05:25Z

Not sure what's going on there since its now closed.

@SilverBut , closed it since I implemented it here: https://github.com/greenpau/cni-plugins

@greenpau What's stopping you from merging this beautiful new feature into Upstream?

3nprob · 2023-03-17T04:35:00Z

@unknowndevQwQ the same thing stopping you, presumably :p
It'd have to get merged by a maintainer.

greenpau force-pushed the nftables branch from a19d16d to 3bf3cdb Compare March 15, 2020 19:26

greenpau changed the title ~~add nftables firewall backend~~ WIP: add nftables firewall backend Mar 15, 2020

greenpau force-pushed the nftables branch 2 times, most recently from f2cecbe to 7a04e16 Compare March 16, 2020 02:59

greenpau changed the title ~~WIP: add nftables firewall backend~~ WIP: firewall: add nftables backend Mar 16, 2020

greenpau force-pushed the nftables branch 2 times, most recently from e7653d0 to fc12e49 Compare March 17, 2020 01:53

greenpau force-pushed the nftables branch 2 times, most recently from 756e3fd to e30bde3 Compare March 17, 2020 02:20

mars1024 reviewed Mar 18, 2020

View reviewed changes

greenpau force-pushed the nftables branch from e30bde3 to aebe4f7 Compare March 20, 2020 01:37

arkodg mentioned this pull request May 5, 2020

[feature request] nftables support moby/moby#26824

Open

greenpau force-pushed the nftables branch 3 times, most recently from e0dd778 to 1e5820c Compare July 31, 2020 01:35

greenpau force-pushed the nftables branch 4 times, most recently from 64af31c to 88bb21e Compare August 1, 2020 00:52

greenpau force-pushed the nftables branch from 540318f to 90c8491 Compare August 4, 2020 13:56

Merge branch 'master' into nftables

b8d3a33

erig0 reviewed Aug 6, 2020

View reviewed changes

greenpau changed the title ~~firewall: add nftables backend~~ WIP: firewall: add nftables backend Aug 10, 2020

SilverBut mentioned this pull request Sep 5, 2021

Unable to reject forwarding without rules set by plugins greenpau/cni-plugins#3

Closed

greenpau closed this Nov 21, 2021

greenpau deleted the nftables branch January 22, 2022 11:58

dngray mentioned this pull request Jul 2, 2022

Since v3 not working wfg/docker-openvpn-client#66

Closed

WIP: firewall: add nftables backend #462

WIP: firewall: add nftables backend #462

Conversation

greenpau commented Mar 15, 2020

greenpau commented Mar 16, 2020 • edited Loading

mars1024 commented Mar 16, 2020

mars1024 commented Mar 16, 2020

greenpau commented Mar 17, 2020

mars1024 commented Mar 17, 2020

mars1024 commented Mar 18, 2020

greenpau commented Mar 18, 2020

greenpau commented Mar 19, 2020

greenpau commented Mar 19, 2020

greenpau commented Jul 30, 2020

greenpau commented Jul 31, 2020

dcbw commented Aug 5, 2020

greenpau commented Aug 5, 2020

dcbw commented Aug 5, 2020 • edited Loading

greenpau commented Aug 5, 2020

erig0 commented Aug 6, 2020 • edited Loading

erig0 Aug 5, 2020

Choose a reason for hiding this comment

greenpau Aug 6, 2020

Choose a reason for hiding this comment

erig0 Aug 6, 2020

Choose a reason for hiding this comment

greenpau Aug 6, 2020

Choose a reason for hiding this comment

greenpau Aug 6, 2020

Choose a reason for hiding this comment

erig0 Aug 7, 2020

Choose a reason for hiding this comment

greenpau Aug 7, 2020

Choose a reason for hiding this comment

greenpau commented Aug 6, 2020

greenpau commented Aug 7, 2020

greenpau commented Aug 10, 2020

greenpau commented Aug 10, 2020

PavelSosin-320 commented Sep 10, 2020

06kellyjac commented Sep 10, 2020

dcbw commented Sep 10, 2020

greenpau commented Sep 10, 2020

cjschroeder commented Jun 16, 2021

SilverBut commented Nov 21, 2021

greenpau commented Nov 21, 2021

3nprob commented Jan 22, 2022

rtgiskard commented Jul 26, 2022

mccv1r0 commented Jul 26, 2022

rtgiskard commented Jul 27, 2022

unknowndevQwQ commented Mar 2, 2023

3nprob commented Mar 17, 2023

greenpau commented Mar 16, 2020 •

edited

Loading

dcbw commented Aug 5, 2020 •

edited

Loading

erig0 commented Aug 6, 2020 •

edited

Loading