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

[Stefan Hanreich]

## 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://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/net/bridge/netfilter/nft_reject_bridge.c?h=v6.8.2&id=127917c29a432c3b798e014a1714e9c1af0f87fe
[2] https://bugzilla.proxmox.com/show_bug.cgi?id=4964
[3] https://forum.proxmox.com/threads/proxmox-claiming-mac-address.52601/page-2#post-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(-)

-- 
Generated by git-murpp 0.6.0