In commit 1a917517 the ipset lookup function was adapted such that it can support both legacy ipset names as well as the new format that includes scopes. During this change, the function that performs the lookup for the SDN scope only included ipsets from the SDN configuration, but not the IPAM configuration. While the IPAM ipsets were correctly generated, the firewall refused to generate rules that included the IPAM ipsets, since they were not found in the lookup function. Adapt the lookup function to consider the IPAM ipsets as well to fix this issue.
Also add respective rules to the integration tests, which would have caught this issue. Reported in the forum by a user [1] [1] https://forum.proxmox.com/threads/sdn-aliases-not-found-by-firewall.180549/ Fixes: 1a917517 Signed-off-by: Stefan Hanreich <[email protected]> --- proxmox-firewall/src/config.rs | 40 ++- proxmox-firewall/src/firewall.rs | 5 +- proxmox-firewall/tests/input/host.fw | 2 + .../integration_tests__firewall.snap | 250 +++++++++++++++++- 4 files changed, 286 insertions(+), 11 deletions(-) diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs index 3854f71..11d9119 100644 --- a/proxmox-firewall/src/config.rs +++ b/proxmox-firewall/src/config.rs @@ -274,6 +274,21 @@ impl FirewallSdnConfig { } } +pub struct FirewallIpamConfig { + _config: Ipam, + ipsets: BTreeMap<String, Ipset>, +} + +impl FirewallIpamConfig { + pub fn ipsets(&self) -> &BTreeMap<String, Ipset> { + &self.ipsets + } + + pub fn ipset(&self, name: &str) -> Option<&Ipset> { + self.ipsets.get(name) + } +} + pub struct FirewallConfig { cluster_config: ClusterConfig, host_config: HostConfig, @@ -281,7 +296,7 @@ pub struct FirewallConfig { bridge_config: BTreeMap<BridgeName, BridgeConfig>, nft_config: BTreeMap<String, ListChain>, sdn_config: Option<FirewallSdnConfig>, - ipam_config: Option<Ipam>, + ipam_config: Option<FirewallIpamConfig>, interface_mapping: AltnameMapping, } @@ -362,11 +377,23 @@ impl FirewallConfig { }) } - pub fn parse_ipam(firewall_loader: &dyn FirewallConfigLoader) -> Result<Option<Ipam>, Error> { + pub fn parse_ipam( + firewall_loader: &dyn FirewallConfigLoader, + ) -> Result<Option<FirewallIpamConfig>, Error> { Ok(match firewall_loader.ipam()? { Some(data) => { let raw_ipam: IpamJson = serde_json::from_reader(data)?; - Some(Ipam::try_from(raw_ipam)?) + let ipam = Ipam::try_from(raw_ipam)?; + + let ipsets = ipam + .ipsets(None) + .map(|ipset| (ipset.name().name().to_string(), ipset)) + .collect(); + + Some(FirewallIpamConfig { + _config: ipam, + ipsets, + }) } _ => None, }) @@ -446,7 +473,7 @@ impl FirewallConfig { self.sdn_config.as_ref() } - pub fn ipam(&self) -> Option<&Ipam> { + pub fn ipam(&self) -> Option<&FirewallIpamConfig> { self.ipam_config.as_ref() } @@ -497,7 +524,10 @@ impl FirewallConfig { match name { RuleIpsetName::Scoped(ipset_name) => match ipset_name.scope() { - IpsetScope::Sdn => self.sdn()?.ipset(ipset_name.name()), + IpsetScope::Sdn => self + .sdn()? + .ipset(ipset_name.name()) + .or_else(|| self.ipam()?.ipset(ipset_name.name())), IpsetScope::Datacenter => self.cluster().ipset(ipset_name.name()), IpsetScope::Guest => { vmid.and_then(|vmid| self.guest_ipset(ipset_name.name(), vmid)) diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs index 58f4aec..65da889 100644 --- a/proxmox-firewall/src/firewall.rs +++ b/proxmox-firewall/src/firewall.rs @@ -249,10 +249,7 @@ impl Firewall { } if let Some(ipam_config) = self.config.ipam() { - let ipsets = ipam_config - .ipsets(None) - .map(|ipset| (ipset.name().to_string(), ipset)) - .collect(); + let ipsets = ipam_config.ipsets(); self.create_ipsets(&mut commands, &ipsets, &cluster_host_table, None)?; self.create_ipsets(&mut commands, &ipsets, &guest_table, None)?; diff --git a/proxmox-firewall/tests/input/host.fw b/proxmox-firewall/tests/input/host.fw index 7b89aad..6b117f6 100644 --- a/proxmox-firewall/tests/input/host.fw +++ b/proxmox-firewall/tests/input/host.fw @@ -24,4 +24,6 @@ IN ACCEPT --icmp-type neighbor-solicitation --proto ipv6-icmp --log info IN Ping(REJECT) IN REJECT -p udp --dport 443 OUT REJECT -p udp --dport 443 +FORWARD DROP --source +sdn/guest-ipam-101 --dest +sdn/guest-ipam-101 +FORWARD DROP --source +sdn/public-all --dest +sdn/public-gateway diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap index 79cb882..3157473 100644 --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap @@ -1,8 +1,6 @@ --- source: proxmox-firewall/tests/integration_tests.rs -assertion_line: 127 expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" -snapshot_kind: text --- { "nftables": [ @@ -4073,6 +4071,254 @@ snapshot_kind: text } } }, + { + "add": { + "rule": { + "family": "inet", + "table": "proxmox-firewall", + "chain": "host-forward", + "expr": [ + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "ip", + "field": "saddr" + } + }, + "right": "@v4-sdn/guest-ipam-101" + } + }, + { + "match": { + "op": "!=", + "left": { + "payload": { + "protocol": "ip", + "field": "saddr" + } + }, + "right": "@v4-sdn/guest-ipam-101-nomatch" + } + }, + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "ip", + "field": "daddr" + } + }, + "right": "@v4-sdn/guest-ipam-101" + } + }, + { + "match": { + "op": "!=", + "left": { + "payload": { + "protocol": "ip", + "field": "daddr" + } + }, + "right": "@v4-sdn/guest-ipam-101-nomatch" + } + }, + { + "drop": null + } + ] + } + } + }, + { + "add": { + "rule": { + "family": "inet", + "table": "proxmox-firewall", + "chain": "host-forward", + "expr": [ + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "ip6", + "field": "saddr" + } + }, + "right": "@v6-sdn/guest-ipam-101" + } + }, + { + "match": { + "op": "!=", + "left": { + "payload": { + "protocol": "ip6", + "field": "saddr" + } + }, + "right": "@v6-sdn/guest-ipam-101-nomatch" + } + }, + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "ip6", + "field": "daddr" + } + }, + "right": "@v6-sdn/guest-ipam-101" + } + }, + { + "match": { + "op": "!=", + "left": { + "payload": { + "protocol": "ip6", + "field": "daddr" + } + }, + "right": "@v6-sdn/guest-ipam-101-nomatch" + } + }, + { + "drop": null + } + ] + } + } + }, + { + "add": { + "rule": { + "family": "inet", + "table": "proxmox-firewall", + "chain": "host-forward", + "expr": [ + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "ip", + "field": "saddr" + } + }, + "right": "@v4-sdn/public-all" + } + }, + { + "match": { + "op": "!=", + "left": { + "payload": { + "protocol": "ip", + "field": "saddr" + } + }, + "right": "@v4-sdn/public-all-nomatch" + } + }, + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "ip", + "field": "daddr" + } + }, + "right": "@v4-sdn/public-gateway" + } + }, + { + "match": { + "op": "!=", + "left": { + "payload": { + "protocol": "ip", + "field": "daddr" + } + }, + "right": "@v4-sdn/public-gateway-nomatch" + } + }, + { + "drop": null + } + ] + } + } + }, + { + "add": { + "rule": { + "family": "inet", + "table": "proxmox-firewall", + "chain": "host-forward", + "expr": [ + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "ip6", + "field": "saddr" + } + }, + "right": "@v6-sdn/public-all" + } + }, + { + "match": { + "op": "!=", + "left": { + "payload": { + "protocol": "ip6", + "field": "saddr" + } + }, + "right": "@v6-sdn/public-all-nomatch" + } + }, + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "ip6", + "field": "daddr" + } + }, + "right": "@v6-sdn/public-gateway" + } + }, + { + "match": { + "op": "!=", + "left": { + "payload": { + "protocol": "ip6", + "field": "daddr" + } + }, + "right": "@v6-sdn/public-gateway-nomatch" + } + }, + { + "drop": null + } + ] + } + } + }, { "add": { "set": { -- 2.47.3
