diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft index 67dd8c8..f36bf3b 100644 --- a/proxmox-firewall/resources/proxmox-firewall.nft +++ b/proxmox-firewall/resources/proxmox-firewall.nft @@ -285,7 +285,12 @@ table bridge proxmox-firewall-guests { } chain do-reject { - drop + meta pkttype broadcast drop + ip saddr 224.0.0.0/4 drop + + meta l4proto tcp reject with tcp reset + meta l4proto icmp reject with icmp type port-unreachable + reject with icmp type host-prohibited } chain after-vm-in { diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs index b137f58..509e295 100644 --- a/proxmox-firewall/src/firewall.rs +++ b/proxmox-firewall/src/firewall.rs @@ -28,7 +28,7 @@ use proxmox_ve_config::guest::types::Vmid; use crate::config::FirewallConfig; use crate::object::{NftObjectEnv, ToNftObjects}; -use crate::rule::{NftRule, NftRuleEnv}; +use crate::rule::{generate_verdict, NftRule, NftRuleEnv}; static CLUSTER_TABLE_NAME: &str = "proxmox-firewall"; static HOST_TABLE_NAME: &str = "proxmox-firewall"; @@ -715,7 +715,10 @@ impl Firewall { None, )?; - commands.push(Add::rule(AddRule::from_statement(chain, default_policy))); + commands.push(Add::rule(AddRule::from_statement( + chain, + generate_verdict(default_policy, &env), + ))); Ok(()) } @@ -827,7 +830,7 @@ impl Firewall { commands.push(Add::rule(AddRule::from_statement( chain, - config.default_policy(direction), + generate_verdict(config.default_policy(direction), &env), ))); Ok(()) diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs index c8099d0..02f964e 100644 --- a/proxmox-firewall/src/rule.rs +++ b/proxmox-firewall/src/rule.rs @@ -4,7 +4,7 @@ use anyhow::{format_err, Error}; use proxmox_nftables::{ expression::{Ct, IpFamily, Meta, Payload, Prefix}, statement::{Log, LogLevel, Match, Operator}, - types::{AddRule, ChainPart, SetName}, + types::{AddRule, ChainPart, SetName, TableFamily, TablePart}, Expression, Statement, }; use proxmox_ve_config::{ @@ -16,7 +16,7 @@ use proxmox_ve_config::{ alias::AliasName, ipset::{Ipfilter, IpsetName}, log::LogRateLimit, - rule::{Direction, Kind, RuleGroup}, + rule::{Direction, Kind, RuleGroup, Verdict as ConfigVerdict}, rule_match::{ Icmp, Icmpv6, IpAddrMatch, IpMatch, Ports, Protocol, RuleMatch, Sctp, Tcp, Udp, }, @@ -146,6 +146,14 @@ impl NftRuleEnv<'_> { fn contains_family(&self, family: Family) -> bool { self.chain.table().family().families().contains(&family) } + + fn table(&self) -> &TablePart { + self.chain.table() + } + + fn direction(&self) -> Direction { + self.direction + } } pub(crate) trait ToNftRules { @@ -204,6 +212,14 @@ impl ToNftRules for RuleGroup { } } +pub(crate) fn generate_verdict(verdict: ConfigVerdict, env: &NftRuleEnv) -> Statement { + match (env.table().family(), env.direction(), verdict) { + (TableFamily::Bridge, Direction::In, ConfigVerdict::Reject) => Statement::make_drop(), + (_, _, ConfigVerdict::Reject) => Statement::jump("do-reject"), + _ => Statement::from(verdict), + } +} + impl ToNftRules for RuleMatch { fn to_nft_rules(&self, rules: &mut Vec, env: &NftRuleEnv) -> Result<(), Error> { if env.direction != self.direction() { @@ -230,7 +246,7 @@ impl ToNftRules for RuleMatch { } } - rules.push(NftRule::new(Statement::from(self.verdict()))); + rules.push(NftRule::new(generate_verdict(self.verdict(), env))); if let Some(name) = &self.iface() { handle_iface(rules, env, name)?; diff --git a/proxmox-firewall/tests/input/100.fw b/proxmox-firewall/tests/input/100.fw index 6cf9fff..1aa9b00 100644 --- a/proxmox-firewall/tests/input/100.fw +++ b/proxmox-firewall/tests/input/100.fw @@ -19,4 +19,6 @@ dc/network1 GROUP network1 -i net1 IN ACCEPT -source 192.168.0.1/24,127.0.0.1-127.255.255.0,172.16.0.1 -dport 123,222:333 -sport http -p tcp IN DROP --icmp-type echo-request --proto icmp --log info +IN REJECT -p udp --dport 443 +OUT REJECT -p udp --dport 443 diff --git a/proxmox-firewall/tests/input/host.fw b/proxmox-firewall/tests/input/host.fw index 8fa57e6..a61b0bd 100644 --- a/proxmox-firewall/tests/input/host.fw +++ b/proxmox-firewall/tests/input/host.fw @@ -20,4 +20,6 @@ nf_conntrack_helpers: amanda,ftp,irc,netbios-ns,pptp,sane,sip,snmp,tftp IN DNS(ACCEPT) -source dc/network1 -log nolog IN DHCPv6(ACCEPT) -log nolog IN DHCPfwd(ACCEPT) -log nolog +IN REJECT -p udp --dport 443 +OUT REJECT -p udp --dport 443 diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap index 7611a64..092ccef 100644 --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap @@ -2153,6 +2153,84 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" } } }, + { + "add": { + "rule": { + "family": "inet", + "table": "proxmox-firewall", + "chain": "host-in", + "expr": [ + { + "match": { + "op": "==", + "left": { + "meta": { + "key": "l4proto" + } + }, + "right": "udp" + } + }, + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "th", + "field": "dport" + } + }, + "right": 443 + } + }, + { + "jump": { + "target": "do-reject" + } + } + ] + } + } + }, + { + "add": { + "rule": { + "family": "inet", + "table": "proxmox-firewall", + "chain": "host-out", + "expr": [ + { + "match": { + "op": "==", + "left": { + "meta": { + "key": "l4proto" + } + }, + "right": "udp" + } + }, + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "th", + "field": "dport" + } + }, + "right": 443 + } + }, + { + "jump": { + "target": "do-reject" + } + } + ] + } + } + }, { "add": { "set": { @@ -3047,6 +3125,43 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" } } }, + { + "add": { + "rule": { + "family": "bridge", + "table": "proxmox-firewall-guests", + "chain": "guest-100-in", + "expr": [ + { + "match": { + "op": "==", + "left": { + "meta": { + "key": "l4proto" + } + }, + "right": "udp" + } + }, + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "th", + "field": "dport" + } + }, + "right": 443 + } + }, + { + "drop": null + } + ] + } + } + }, { "add": { "element": { @@ -3147,6 +3262,45 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" } } }, + { + "add": { + "rule": { + "family": "bridge", + "table": "proxmox-firewall-guests", + "chain": "guest-100-out", + "expr": [ + { + "match": { + "op": "==", + "left": { + "meta": { + "key": "l4proto" + } + }, + "right": "udp" + } + }, + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "th", + "field": "dport" + } + }, + "right": 443 + } + }, + { + "jump": { + "target": "do-reject" + } + } + ] + } + } + }, { "add": { "element": { @@ -3198,7 +3352,9 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "chain": "guest-100-out", "expr": [ { - "drop": null + "jump": { + "target": "do-reject" + } } ] } diff --git a/proxmox-nftables/src/statement.rs b/proxmox-nftables/src/statement.rs index e89f678..5483368 100644 --- a/proxmox-nftables/src/statement.rs +++ b/proxmox-nftables/src/statement.rs @@ -39,6 +39,10 @@ impl Statement { Statement::Verdict(Verdict::Accept(Null)) } + pub fn make_reject() -> Self { + Statement::Reject(Reject::default()) + } + pub const fn make_drop() -> Self { Statement::Verdict(Verdict::Drop(Null)) } @@ -118,7 +122,7 @@ impl From for Statement { fn from(value: ConfigVerdict) -> Self { match value { ConfigVerdict::Accept => Statement::make_accept(), - ConfigVerdict::Reject => Statement::make_drop(), + ConfigVerdict::Reject => Statement::make_reject(), ConfigVerdict::Drop => Statement::make_drop(), } }