When the firewall was enabled for a guest, but disabled for all
interfaces of a guest, the firewall tried to add an empty list to the
guest map, which is an invalid nftables statement.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250408134135.242922-1-s.hanreich@proxmox.com
A bug in proxmox-ve-config caused the key to be defaulted to on, if it
didn't exist in the configuration. Add this scenario to the
integration tests, so we can potentially catch problems with the
missing firewall key via the integration tests.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250313132231.166477-5-s.hanreich@proxmox.com
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
The firewall generated mac filters for outgoing packets even if the
firewall was disabled for a specific interface. This was applicable to
ARP packets as well.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250313132231.166477-4-s.hanreich@proxmox.com
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
ipfilter ipsets and rules were still generated, even if the firewall
was disabled for the network device.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250313132231.166477-3-s.hanreich@proxmox.com
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
The network device configuration doesn't return a reference anymore,
so we do not need to dereference here anymore.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250313132231.166477-2-s.hanreich@proxmox.com
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Rules using the Ping macro were wrongly generated due to the ICMP
macros using the wrong format for specifying ICMP type. The test cases
did not include any macros utilizing the ICMP protocol. Add them to
catch any errors related to ICMP parsing in the future.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
Link: https://lore.proxmox.com/20250204095733.55146-3-s.hanreich@proxmox.com
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
... on all chains that check for ct state. Since we support this option,
we should also use it in our firewall rule generation.
This is a follow-up to
64dc344b ("firewall: apply `nt_conntrack_allow_invalid` option to guest table")
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
... on the guest table. There is no reason to not respect that option
on those two chains. These two were missed in the referenced commit.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Fixes: 64dc344b ("firewall: apply `nt_conntrack_allow_invalid` option to guest table")
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>
proxmox-ve-config currently still has a non-standard d/control, so
that one needs manual fixing, but the rest follows the usual
conventions...
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
There was a bug where rulesets with security groups bound to a
specific interface would cause the firewall to fail to create a new
ruleset. Catch this by adding a security group bound to an interface
to the ruleset.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Security groups can be bound to a specific interface. The notion of
this breaks down when considering the forward direction, since there
are two interfaces involved: incoming and outgoing, which can be
different depending on the kind of traffic.
With the current implementation, the firewall refuses to generate
rulesets with security groups that are bound to specific interfaces.
Check for this case explicitly and skip creating rules in the forward
chain when a security group bound to a specific interface is
encountered.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Hannes Duerr <h.duerr@proxmox.com>
These are simply just of the proxmox-ve-config crate, which was
moved to a separate repository in
aa7692036 ("add proxmox-ve-rs crate - move proxmox-ve-config there")
and are not used by the other remaining crates.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
So it behaves the same way the 'old' firewall did. Since currently
ct state invalid are always dropped on the guest table, regardless
of the option. The host behaviour is not changed as it would
require `forward` to match the 'old' behaviour.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
[ TL: resolve merge conflict due to recent context changes ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This is more efficient than draining and collecting the Vec. It also
fixes the respective clippy lint.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Tested-by: Hannes Dürr <h.duerr@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Tested-by: Hannes Dürr <h.duerr@proxmox.com>
They act like virtual ipsets, similar to ipfilter-net, that can be
used for defining firewall rules for sdn objects dynamically.
The changes in proxmox-ve-config also introduced a dedicated struct
for representing ip ranges, so we update the existing code, so that it
uses that struct as well.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Tested-by: Gabriel Goller <g.goller@proxmox.com>
Tested-by: Hannes Dürr <h.duerr@proxmox.com>
Also add example SDN configuration files that get automatically
loaded, which can be used for future tests.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Tested-by: Gabriel Goller <g.goller@proxmox.com>
Tested-by: Hannes Dürr <h.duerr@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
[ TL: add dependency to d/control and update its version ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Virtual cargo workspaces (workspaces without a [package] section)
default to the cargo feature resolver "1" – even though this outputs a
warning on every cargo invocation. To remove the warning, explicitly set
the resolver to version "2".
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
FG: adapted to current Cargo.toml
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
When matching via ether type, VLAN packets are not matched. This can
cause ARP packets encapsulated in VLAN frames to be dropped.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
In order to make sure we are only affecting VM traffic and no host
interfaces that are bridged, move the rules into a chain that gets
executed inside the guest chain, rather than setting the rules
globally.
Since ether type matches on the respective Ethernet header, it
doesn't work for packets with VLAN header. Matching via meta protocol
ensures that VLAN encapsulated ARP packets are matched as well.
Otherwise ARP traffic inside VLANs gets dropped, due to them having
conntrack state invalid.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
[ TL: update to versions released since this patch was created and
updated proxmox-sys tree-wide ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
When disabling the nftables firewall again, there is a race condition
where the nftables ruleset never gets flushed and persists after
disabling.
The nftables firewall update loop does a noop when the force disable
file exists. It only flushes the ruleset when nftables is disabled in
the configuration file but the force disable file does not yet exist.
This can lead to the following situation:
* nftables is activated and created its ruleset
* user switches from nftables firewall back to iptables firewall
* pve-firewall runs and creates the force disable file
* proxmox-firewall sees that the file exists and does nothing
Reported-by: Hannes Laimer <h.laimer@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
There are certain ICMP messages that should always pass through a
firewall irregardless of any other rules. This is particularly
important for ICMPv6. While we already handled NDP, there are certain
control messages that should always be able to pass through any
firewall, according to RFC 4890.
For ICMP we additionally allow 'Source Quench' as well.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
the old location has been deprecated for a while, and rustc 1.78 will start to warn about it.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
The output chain did not have any connection tracking (conntrack)
rules, which lead to issues when the default output policy is not
accept. Also, move the conntrack rules to the beginning of all chains.
Originally-by: Laurent Guerby <laurent@guerby.net>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
In order to be able to send outgoing ARP packets when the default
policy is set to drop or reject, we need to explicitly allow ARP
traffic in the outgoing chain of guests. We need to do this in the
guest chain itself in order to be able to filter spoofed packets via
the MAC filter.
Contrary to the out direction we can simply accept all incoming ARP
traffic, since we do not do any MAC filtering for incoming traffic.
Since we create fdb entries for every NIC, guests should only see ARP
traffic for their MAC addresses anyway.
Originally-by: Laurent Guerby <laurent@guerby.net>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
ICMPv6 has different message types for rejecting traffic. With ICMP we
used host-prohibited as rejection type, which doesn't exist in ICMPv6.
Add an additional rule for IPv6, so it uses admin-prohibited.
Additionally, add a terminal drop statement in order to prevent any
traffic that does not get matched from bypassing the reject chain.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This will be set in the iptables/Perl based pve-firewall service and
ensures that users pulling this service but not enabling it are
"protected" from log-spam due to some pve-firewall config not being
parsed correctly (yet).
We can remove this again once we're sure that all edge cases are
covered, or this service here becomes the default.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>
Error handling of the firewall binary should now be much more robust
on configuration errors. Instead of panicking in some cases it should
now log an error.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
We support any as wildcard for matching all icmp types. Implement
parsing logic for parsing the any value and support converting the any
value into an nftables expression.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
