[pve-devel] [RFC container/firewall/manager/proxmox-firewall/qemu-server 00/37] proxmox firewall nftables implementation

Wed Apr 3 07:37:13 CEST 2024

Hi Stefan,

I'll really take time to test it (I was super busy theses last month
with a datacenter migration), as I wait for nftables since a while.

Can't help too much with rust, but I really appriciate it, as I had
some servers with a lot of vms && rules, take more than 10s to generate
the rules with current perl implementation).

I really would like to not have fwbr bridge anymore, because I have
seen a big performance bug with them: 

if you have a lot of vms in the same vlan/network, with a lot of fwbr
plugged on vmbr,  the broadcast traffic, like arp, is replicated over
each fwbr,  and it's not using a fast path, and use a lot of cpu with 
ksoftirqd

I have also seen some evpn bug sometimes, putting some stress with mac
tracking when a lot of fwbr are used.

Also, users have reported conntrack problem with fwbr, when the guest
send a tcp reset not received on other side.

I'll try your code, see the generated rules, and try to see if I can
get reject working.

Regards,

Alexandre

-------- Message initial --------
De: Stefan Hanreich <s.hanreich at proxmox.com>
Répondre à: Proxmox VE development discussion <pve-
devel at lists.proxmox.com>
À: pve-devel at lists.proxmox.com
Objet: [pve-devel] [RFC container/firewall/manager/proxmox-
firewall/qemu-server 00/37] proxmox firewall nftables implementation
Date: 02/04/2024 19:15:52

## Introduction
This RFC provides a drop-in replacement for the current pve-firewall
package
that is based on Rust and nftables.

It consists of three crates:
* proxmox-ve-config
  for parsing firewall and guest configuration files, as well as some
helpers
  to access host configuration (particularly networking)
* proxmox-nftables
  contains bindings for libnftables as well as types that implement the
JSON
  schema defined by libnftables-json
* proxmox-firewall
  uses the other two crates to read the firewall configuration and
create the
  respective nftables configuration

## Installation
* Build & install all deb packages on your PVE instance
* Enable the nftables firewall by going to
  Web UI > <Host> > Firewall > Options > proxmox-nftables
* Enable the firewall datacenter-wide if you haven't already
* Restarting running VMs/CTs is required so the changes to the fwbr
creation
  go into effect

For your convenience I have provided pre-built packages on our share
under
`shanreich-proxmox-firewall`.

The source code is also available on my staff repo as `proxmox-
firewall`.

## Configuration
The firewall should work as a drop-in replacement for the pve-firewall,
so you
should be able to configure the firewall as usual via the Web UI or
configuration files.

## Known Issues
There is currently one major issue that we still need to solve:
REJECTing
packets from the guest firewalls is currently not possible for incoming
traffic
(it will instead be dropped).

This is due to the fact that we are using the postrouting hook of
nftables in a
table with type bridge for incoming traffic. In the bridge table in the
postrouting hook we cannot tell whether the packet has also been sent
to other
ports in the bridge (e.g. when a MAC has not yet been learned and the
packet
then gets flooded to all bridge ports). If we would then REJECT a
packet in the
postrouting hook this can lead to a bug where the firewall rules for
one guest
REJECT a packet and send a response (RST for TCP, ICMP port/host-
unreachable
otherwise).

This has also been explained in the respective commit introducing the
restriction [1].

We were able to circumvent this restriction in the old firewall due to
using
firewall bridges and rejecting in the firewall bridge itself. Doing
this leads
to the behavior described above, which has tripped up some of our users
before
[2] [3] and which is, frankly, wrong.

I currently see two possible solutions for this, both of which carry
downsides.
Your input on this matter would be much appreciated, particularly if
you can
think of another solution which I cannot currently see:

1. Only REJECT packets in the prerouting chain of the firewall bridge
with the
destination MAC address set to the MAC address of the network device,
otherwise
DROP

The downside of this is that we, once again, will have to resort to
using
firewall bridges, which we wanted to eliminate. This would also be the
sole
reason for still having to resort to using firewall bridges.

2. Only allow DROP in the guest firewall for incoming traffic

This would be quite awkward since, well, rejecting traffic would be
quite nice
for a firewall I'd say ;)

I'm happy for all input regarding this matter.

## Useful Commands

You can check if firewall rules got created by running

```
nft list ruleset
```

You can also check that `iptables` rules are not created via
```
iptables-save
```

Further info about the services:
```
systemctl status proxmox-firewall.{service,timer}
```

You can grab the debug output from the new firewall like so:

```
RUST_LOG=trace proxmox-firewall
```

## Upcoming

There are some (very minor) features missing:
* automatically generating an ipfilter based on the link-local IPv6
address
* complete list of ICMP codes

I also have some improvements for the code base in mind, but I wanted
to get the
RFC out now, since I feel like the new firewall is already in a decent
state and
the architecture is relatively solid. Nevertheless there are still a
few
improvements that I will be working on:
* move error handling in the library crates to custom error types /
thiserror
* integration tests for the firewall itself

[1] https://antiphishing.vadesecure.com/v4?f=S25kRlB1M01yME9kTDh5eL_6m-
eRSNDP3HhStQtWsB6A9RFuywtUmRA3bMwQXVR4gN1eXmFfKWOtGxAYQBEi3w&i=YzVlb2ds
ZkpsODdFRWdva5seZZpmIcse_G49jw88jd0&k=EuOw&r=Tkx1akhsY2lJZzVpWVpTcc5CZ_
ekWK0CJx4F_Vf1f5knmIJobH1h1ASGKygps0i6aCehN0X1KnU1hrkFdGUUWw&s=2839b4d2
dafd4fe3310530ccd69fc7971f823ac151545570b5a4d37272c9a840&u=https%3A%2F%
2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fstable%2Flinux.gi
t%2Fcommit%2Fnet%2Fbridge%2Fnetfilter%2Fnft_reject_bridge.c%3Fh%3Dv6.8.
2%26id%3D127917c29a432c3b798e014a1714e9c1af0f87fe
[2] https://antiphishing.vadesecure.com/v4?f=S25kRlB1M01yME9kTDh5eL_6m-
eRSNDP3HhStQtWsB6A9RFuywtUmRA3bMwQXVR4gN1eXmFfKWOtGxAYQBEi3w&i=YzVlb2ds
ZkpsODdFRWdva5seZZpmIcse_G49jw88jd0&k=EuOw&r=Tkx1akhsY2lJZzVpWVpTcc5CZ_
ekWK0CJx4F_Vf1f5knmIJobH1h1ASGKygps0i6aCehN0X1KnU1hrkFdGUUWw&s=9be4443d
b735b155b6750f8540a244e2e0d61776ffdd4093ccf693623941378f&u=https%3A%2F%
2Fbugzilla.proxmox.com%2Fshow_bug.cgi%3Fid%3D4964
[3] https://antiphishing.vadesecure.com/v4?f=S25kRlB1M01yME9kTDh5eL_6m-
eRSNDP3HhStQtWsB6A9RFuywtUmRA3bMwQXVR4gN1eXmFfKWOtGxAYQBEi3w&i=YzVlb2ds
ZkpsODdFRWdva5seZZpmIcse_G49jw88jd0&k=EuOw&r=Tkx1akhsY2lJZzVpWVpTcc5CZ_
ekWK0CJx4F_Vf1f5knmIJobH1h1ASGKygps0i6aCehN0X1KnU1hrkFdGUUWw&s=5dd349f1
f995501a94dccd070aa11e086ceee9c269c24345d9e1d059e7aabd07&u=https%3A%2F%
2Fforum.proxmox.com%2Fthreads%2Fproxmox-claiming-mac-
address.52601%2Fpage-2%23post-415493

PS: Since the changestats are broken for patch series including the
initial
commit of a repo here is the cloc output for proxmox-firewall instead:

-----------------------------------------------------------------------
--------
Language                     files          blank       
comment           code
-----------------------------------------------------------------------
--------
Rust                            37           1642            
78           7749
JSON                             2              0             
0            948
TOML                             3             10             
0             59
-----------------------------------------------------------------------
--------
SUM:                            42           1652            
78           8756
-----------------------------------------------------------------------
--------

proxmox-firewall:

Stefan Hanreich (33):
  config: add proxmox-ve-config crate
  config: firewall: add types for ip addresses
  config: firewall: add types for ports
  config: firewall: add types for log level and rate limit
  config: firewall: add types for aliases
  config: host: add helpers for host network configuration
  config: guest: add helpers for parsing guest network config
  config: firewall: add types for ipsets
  config: firewall: add types for rules
  config: firewall: add types for security groups
  config: firewall: add generic parser for firewall configs
  config: firewall: add cluster-specific config + option types
  config: firewall: add host specific config + option types
  config: firewall: add guest-specific config + option types
  config: firewall: add firewall macros
  config: firewall: add conntrack helper types
  nftables: add crate for libnftables bindings
  nftables: add helpers
  nftables: expression: add types
  nftables: expression: implement conversion traits for firewall config
  nftables: statement: add types
  nftables: statement: add conversion traits for config types
  nftables: commands: add types
  nftables: types: add conversion traits
  nftables: add libnftables bindings
  firewall: add firewall crate
  firewall: add base ruleset
  firewall: add config loader
  firewall: add rule generation logic
  firewall: add object generation logic
  firewall: add ruleset generation logic
  firewall: add proxmox-firewall binary
  firewall: add files for debian packaging

qemu-server:

Stefan Hanreich (1):
  firewall: add handling for new nft firewall

 vm-network-scripts/pve-bridge | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

pve-container:

Stefan Hanreich (1):
  firewall: add handling for new nft firewall

 src/PVE/LXC.pm | 5 +++++
 1 file changed, 5 insertions(+)

pve-firewall:

Stefan Hanreich (1):
  add configuration option for new nftables firewall

 src/PVE/Firewall.pm | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

pve-manager:

Stefan Hanreich (1):
  firewall: expose configuration option for new nftables firewall

 www/manager6/grid/FirewallOptions.js | 1 +
 1 file changed, 1 insertion(+)

Summary over all repositories:
  4 files changed, 29 insertions(+), 6 deletions(-)