From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id C7A5E90924 for ; Tue, 2 Apr 2024 19:16:32 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B16E0A5AD for ; Tue, 2 Apr 2024 19:16:32 +0200 (CEST) Received: from lana.proxmox.com (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP for ; Tue, 2 Apr 2024 19:16:30 +0200 (CEST) Received: by lana.proxmox.com (Postfix, from userid 10043) id A153C2C2188; Tue, 2 Apr 2024 19:16:30 +0200 (CEST) From: Stefan Hanreich To: pve-devel@lists.proxmox.com Date: Tue, 2 Apr 2024 19:15:52 +0200 Message-Id: <20240402171629.536804-1-s.hanreich@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.742 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_ASCII_DIVIDERS 0.8 Email that uses ascii formatting dividers and possible spam tricks KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com, firewall.pm, lxc.pm] Subject: [pve-devel] [RFC container/firewall/manager/proxmox-firewall/qemu-server 00/37] proxmox firewall nftables implementation X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Apr 2024 17:16:32 -0000 ## 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 > > 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