[pve-devel] [PATCH firewall 2/2] fix #2004: do not allow backwards ranges

[Dominik Csapak]

ranges like 10:5 are allowed by us, but iptables throws an error
that is only visible in the syslog and the firewall rules do not
get updated

Signed-off-by: Dominik Csapak <***@proxmox.com>
---
src/PVE/Firewall.pm | 1 +
1 file changed, 1 insertion(+)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 035dc7e..d7d1439 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1054,6 +1054,7 @@ sub parse_port_name_number_or_range {
my ($port1, $port2) = ($1, $2);
die "invalid port '$port1'\n" if $port1 > 65535;
die "invalid port '$port2'\n" if $port2 > 65535;
+ die "backwards range '$port1:$port2'\n" if $port1 > $port2;
} elsif ($item =~ m/^([0-9]+)$/) {
$count += 1;
my $port = $1;

[Alwin Antreich]

Post by Dominik Csapak
ranges like 10:5 are allowed by us, but iptables throws an error
that is only visible in the syslog and the firewall rules do not
get updated
---
src/PVE/Firewall.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 035dc7e..d7d1439 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1054,6 +1054,7 @@ sub parse_port_name_number_or_range {
my ($port1, $port2) = ($1, $2);
die "invalid port '$port1'\n" if $port1 > 65535;
die "invalid port '$port2'\n" if $port2 > 65535;
+ die "backwards range '$port1:$port2'\n" if $port1 > $port2;

Couldn't we go ahead and switch the ports to get a acceptable range for
iptables? I suspect that a user will change the port order to get the
rule applied anyway.

If we don't want to swith ports, then IMHO the message needs more
information. Like eg. "backwards range '$port1:$port2' not allowed, use
forward ranges".

Post by Dominik Csapak
} elsif ($item =~ m/^([0-9]+)$/) {
$count += 1;
my $port = $1;
--
2.11.0

[Dominik Csapak]

Post by Alwin Antreich

Post by Dominik Csapak
ranges like 10:5 are allowed by us, but iptables throws an error
that is only visible in the syslog and the firewall rules do not
get updated
---
src/PVE/Firewall.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 035dc7e..d7d1439 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1054,6 +1054,7 @@ sub parse_port_name_number_or_range {
my ($port1, $port2) = ($1, $2);
die "invalid port '$port1'\n" if $port1 > 65535;
die "invalid port '$port2'\n" if $port2 > 65535;
+ die "backwards range '$port1:$port2'\n" if $port1 > $port2;

Couldn't we go ahead and switch the ports to get a acceptable range for
iptables? I suspect that a user will change the port order to get the
rule applied anyway.
If we don't want to swith ports, then IMHO the message needs more
information. Like eg. "backwards range '$port1:$port2' not allowed, use
forward ranges".

mhmm i do not really want to switch them silently, because some users
might have a different understanding of such a range

e.g. 60000:1000 could be misunderstood as 60000-65535 + 1-1000

but, yes i can write a better error message

Post by Alwin Antreich

Post by Dominik Csapak
} elsif ($item =~ m/^([0-9]+)$/) {
$count += 1;
my $port = $1;
--
2.11.0

_______________________________________________
pve-devel mailing list
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[Wolfgang Bumiller]

applied

perl accepts non-ascii digits for \d like U+09EA
which do not work with iptables
---
src/PVE/Firewall.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index ef00d0c..035dc7e 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1049,12 +1049,12 @@ sub parse_port_name_number_or_range {
- if ($item =~ m/^(\d+):(\d+)$/) {
+ if ($item =~ m/^([0-9]+):([0-9]+)$/) {
$count += 2;
my ($port1, $port2) = ($1, $2);
die "invalid port '$port1'\n" if $port1 > 65535;
die "invalid port '$port2'\n" if $port2 > 65535;
- } elsif ($item =~ m/^(\d+)$/) {
+ } elsif ($item =~ m/^([0-9]+)$/) {
$count += 1;
my $port = $1;
die "invalid port '$port'\n" if $port > 65535;
--
2.11.0