* [pve-devel] [PATCH proxmox-ve-rs 1/3] firewall macros: fix macros using icmp protocol @ 2025-02-04 9:57 Stefan Hanreich 2025-02-04 9:57 ` [pve-devel] [PATCH proxmox-ve-rs 2/3] fix #6108: firewall macros: Add missing ICMPv6 statements Stefan Hanreich 2025-02-04 9:57 ` [pve-devel] [PATCH proxmox-firewall 3/3] tests: add Ping macro to tests Stefan Hanreich 0 siblings, 2 replies; 4+ messages in thread From: Stefan Hanreich @ 2025-02-04 9:57 UTC (permalink / raw) To: pve-devel Macros containing rules for the ICMP protocol used dport instead of icmp-type for specifying the type of ICMP messages. This is how pve-firewall used to specify them, but the nftables firewall uses a separate key for this in the macros. This caused all ICMP types to be allowed instead of restricting them to the types specified in the macro. Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com> --- proxmox-ve-config/resources/macros.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/proxmox-ve-config/resources/macros.json b/proxmox-ve-config/resources/macros.json index 2fcc0fb..37032c0 100644 --- a/proxmox-ve-config/resources/macros.json +++ b/proxmox-ve-config/resources/macros.json @@ -478,19 +478,19 @@ "NeighborDiscovery": { "code": [ { - "dport": "nd-router-solicit", + "icmp-type": "nd-router-solicit", "proto": "icmpv6" }, { - "dport": "nd-router-advert", + "icmp-type": "nd-router-advert", "proto": "icmpv6" }, { - "dport": "nd-neighbor-solicit", + "icmp-type": "nd-neighbor-solicit", "proto": "icmpv6" }, { - "dport": "nd-neighbor-advert", + "icmp-type": "nd-neighbor-advert", "proto": "icmpv6" } ], @@ -577,7 +577,7 @@ "Ping": { "code": [ { - "dport": "echo-request", + "icmp-type": "echo-request", "proto": "icmp" } ], @@ -856,7 +856,7 @@ "proto": "udp" }, { - "dport": "echo-request", + "icmp-type": "echo-request", "proto": "icmp" } ], -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] [PATCH proxmox-ve-rs 2/3] fix #6108: firewall macros: Add missing ICMPv6 statements 2025-02-04 9:57 [pve-devel] [PATCH proxmox-ve-rs 1/3] firewall macros: fix macros using icmp protocol Stefan Hanreich @ 2025-02-04 9:57 ` Stefan Hanreich 2025-02-04 9:57 ` [pve-devel] [PATCH proxmox-firewall 3/3] tests: add Ping macro to tests Stefan Hanreich 1 sibling, 0 replies; 4+ messages in thread From: Stefan Hanreich @ 2025-02-04 9:57 UTC (permalink / raw) To: pve-devel Some macros only contained rules for ICMP echo requests, but not their ICMPv6 counterparts. Add them, so they work properly with IPv6 setups. Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com> --- proxmox-ve-config/resources/macros.json | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/proxmox-ve-config/resources/macros.json b/proxmox-ve-config/resources/macros.json index 37032c0..f90fb38 100644 --- a/proxmox-ve-config/resources/macros.json +++ b/proxmox-ve-config/resources/macros.json @@ -579,6 +579,10 @@ { "icmp-type": "echo-request", "proto": "icmp" + }, + { + "icmp-type": "echo-request", + "proto": "icmpv6" } ], "desc": "ICMP echo request" @@ -858,6 +862,10 @@ { "icmp-type": "echo-request", "proto": "icmp" + }, + { + "icmp-type": "echo-request", + "proto": "icmpv6" } ], "desc": "Traceroute (for up to 30 hops) traffic" -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] [PATCH proxmox-firewall 3/3] tests: add Ping macro to tests 2025-02-04 9:57 [pve-devel] [PATCH proxmox-ve-rs 1/3] firewall macros: fix macros using icmp protocol Stefan Hanreich 2025-02-04 9:57 ` [pve-devel] [PATCH proxmox-ve-rs 2/3] fix #6108: firewall macros: Add missing ICMPv6 statements Stefan Hanreich @ 2025-02-04 9:57 ` Stefan Hanreich 2025-03-13 15:04 ` Hannes Laimer 1 sibling, 1 reply; 4+ messages in thread From: Stefan Hanreich @ 2025-02-04 9:57 UTC (permalink / raw) To: pve-devel 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> --- Depends on bumped proxmox-ve-config to work. proxmox-firewall/tests/input/host.fw | 1 + .../integration_tests__firewall.snap | 57 ++++++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/proxmox-firewall/tests/input/host.fw b/proxmox-firewall/tests/input/host.fw index a61b0bd..ddfcb1c 100644 --- a/proxmox-firewall/tests/input/host.fw +++ b/proxmox-firewall/tests/input/host.fw @@ -20,6 +20,7 @@ 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 Ping(REJECT) 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 9194fc6..d25ece8 100644 --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap @@ -1,7 +1,6 @@ --- source: proxmox-firewall/tests/integration_tests.rs expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" -snapshot_kind: text --- { "nftables": [ @@ -3533,6 +3532,62 @@ snapshot_kind: text } } }, + { + "add": { + "rule": { + "family": "inet", + "table": "proxmox-firewall", + "chain": "host-in", + "expr": [ + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "icmp", + "field": "type" + } + }, + "right": "echo-request" + } + }, + { + "jump": { + "target": "do-reject" + } + } + ] + } + } + }, + { + "add": { + "rule": { + "family": "inet", + "table": "proxmox-firewall", + "chain": "host-in", + "expr": [ + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "icmpv6", + "field": "type" + } + }, + "right": "echo-request" + } + }, + { + "jump": { + "target": "do-reject" + } + } + ] + } + } + }, { "add": { "rule": { -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH proxmox-firewall 3/3] tests: add Ping macro to tests 2025-02-04 9:57 ` [pve-devel] [PATCH proxmox-firewall 3/3] tests: add Ping macro to tests Stefan Hanreich @ 2025-03-13 15:04 ` Hannes Laimer 0 siblings, 0 replies; 4+ messages in thread From: Hannes Laimer @ 2025-03-13 15:04 UTC (permalink / raw) To: Proxmox VE development discussion, Stefan Hanreich Tested these changes, I could reproduce the described problem, and after applying the patches the macros only matches the correct ICMP packets, not all. so consider this: Tested-by: Hannes Laimer <h.laimer@proxmox.com> On 04.02.25 10:57, Stefan Hanreich wrote: > 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> > --- > Depends on bumped proxmox-ve-config to work. > > proxmox-firewall/tests/input/host.fw | 1 + > .../integration_tests__firewall.snap | 57 ++++++++++++++++++- > 2 files changed, 57 insertions(+), 1 deletion(-) > > diff --git a/proxmox-firewall/tests/input/host.fw b/proxmox-firewall/tests/input/host.fw > index a61b0bd..ddfcb1c 100644 > --- a/proxmox-firewall/tests/input/host.fw > +++ b/proxmox-firewall/tests/input/host.fw > @@ -20,6 +20,7 @@ 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 Ping(REJECT) > 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 9194fc6..d25ece8 100644 > --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap > +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap > @@ -1,7 +1,6 @@ > --- > source: proxmox-firewall/tests/integration_tests.rs > expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" > -snapshot_kind: text > --- > { > "nftables": [ > @@ -3533,6 +3532,62 @@ snapshot_kind: text > } > } > }, > + { > + "add": { > + "rule": { > + "family": "inet", > + "table": "proxmox-firewall", > + "chain": "host-in", > + "expr": [ > + { > + "match": { > + "op": "==", > + "left": { > + "payload": { > + "protocol": "icmp", > + "field": "type" > + } > + }, > + "right": "echo-request" > + } > + }, > + { > + "jump": { > + "target": "do-reject" > + } > + } > + ] > + } > + } > + }, > + { > + "add": { > + "rule": { > + "family": "inet", > + "table": "proxmox-firewall", > + "chain": "host-in", > + "expr": [ > + { > + "match": { > + "op": "==", > + "left": { > + "payload": { > + "protocol": "icmpv6", > + "field": "type" > + } > + }, > + "right": "echo-request" > + } > + }, > + { > + "jump": { > + "target": "do-reject" > + } > + } > + ] > + } > + } > + }, > { > "add": { > "rule": { _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-03-13 15:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-02-04 9:57 [pve-devel] [PATCH proxmox-ve-rs 1/3] firewall macros: fix macros using icmp protocol Stefan Hanreich 2025-02-04 9:57 ` [pve-devel] [PATCH proxmox-ve-rs 2/3] fix #6108: firewall macros: Add missing ICMPv6 statements Stefan Hanreich 2025-02-04 9:57 ` [pve-devel] [PATCH proxmox-firewall 3/3] tests: add Ping macro to tests Stefan Hanreich 2025-03-13 15:04 ` Hannes Laimer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inboxService provided by Proxmox Server Solutions GmbH | Privacy | Legal