* [pve-devel] [PATCH V2 firewall 0/2] allow non zero ip address host bits to be entered
@ 2022-11-29 16:01 Stefan Hrdlicka
2022-11-29 16:01 ` [pve-devel] [PATCH V2 firewall 1/2] " Stefan Hrdlicka
2022-11-29 16:01 ` [pve-devel] [PATCH V2 firewall 2/2] cleanup: don't capture "/xx" of CIDR Stefan Hrdlicka
0 siblings, 2 replies; 4+ messages in thread
From: Stefan Hrdlicka @ 2022-11-29 16:01 UTC (permalink / raw)
To: pve-devel
V1 -> V2
* zero out host bits instead of ignoring error
* regex "cleanup"
Stefan Hrdlicka (2):
allow non zero ip address host bits to be entered
cleanup: don't capture "/xx" of CIDR
src/PVE/API2/Firewall/IPSet.pm | 2 +-
src/PVE/Firewall.pm | 31 +++++++++++++++++++++++++++++--
2 files changed, 30 insertions(+), 3 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] [PATCH V2 firewall 1/2] allow non zero ip address host bits to be entered
2022-11-29 16:01 [pve-devel] [PATCH V2 firewall 0/2] allow non zero ip address host bits to be entered Stefan Hrdlicka
@ 2022-11-29 16:01 ` Stefan Hrdlicka
2022-12-13 12:33 ` Wolfgang Bumiller
2022-11-29 16:01 ` [pve-devel] [PATCH V2 firewall 2/2] cleanup: don't capture "/xx" of CIDR Stefan Hrdlicka
1 sibling, 1 reply; 4+ messages in thread
From: Stefan Hrdlicka @ 2022-11-29 16:01 UTC (permalink / raw)
To: pve-devel
They can already be set directly via the cluster.fw file. Net::IP is just a
bit more picky with what it allows:
For example:
error: 192.168.1.155/24
correct: 192.168.1.0/24
This cleans the entered IP and removes the non zero host bits.
Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
---
src/PVE/API2/Firewall/IPSet.pm | 2 +-
src/PVE/Firewall.pm | 29 ++++++++++++++++++++++++++++-
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
index a5f69e9..14bcfcb 100644
--- a/src/PVE/API2/Firewall/IPSet.pm
+++ b/src/PVE/API2/Firewall/IPSet.pm
@@ -199,7 +199,7 @@ sub register_create_ip {
my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
- my $cidr = $param->{cidr};
+ my $cidr = PVE::Firewall::clean_cidr($param->{cidr});
if ($cidr =~ m/^${PVE::Firewall::ip_alias_pattern}$/) {
# make sure alias exists (if $cidr is an alias)
PVE::Firewall::resolve_alias($cluster_conf, $fw_conf, $cidr);
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index d40a9b1..3c35b44 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -69,8 +69,12 @@ sub pve_verify_ip_or_cidr {
my ($cidr, $noerr) = @_;
if ($cidr =~ m!^(?:$IPV6RE|$IPV4RE)(/(\d+))?$!) {
- return $cidr if Net::IP->new($cidr);
+ # Net::IP throws an error if the masked CIDR part isn't zero, e.g., `192.168.1.155/24`
+ # fails but `192.168.1.0/24` succeeds. clean_cidr removes the non zero bits from the CIDR.
+ my $clean_cidr = clean_cidr($cidr);
+ return $cidr if Net::IP->new($clean_cidr);
return undef if $noerr;
+
die Net::IP::Error() . "\n";
}
return undef if $noerr;
@@ -86,6 +90,29 @@ sub pve_verify_ip_or_cidr_or_alias {
return pve_verify_ip_or_cidr($cidr, $noerr);
}
+sub clean_cidr {
+ my ($cidr) = @_;
+ my ($ip, $len) = split('/', $cidr);
+ return $cidr if !$len;
+
+ my $clean_func = sub {
+ my ($ver) = @_;
+ my $bin_ip = Net::IP::ip_iptobin( Net::IP::ip_expand_address($ip, $ver), $ver);
+ my $bin_mask = Net::IP::ip_get_mask($len, $ver);
+ my $clean_ip = Net::IP::ip_compress_address( Net::IP::ip_bintoip($bin_ip & $bin_mask, $ver), $ver);
+
+ return "${clean_ip}/$len";
+ };
+
+ if ($ip =~ m!^$IPV4RE$!) {
+ return &$clean_func(4)
+ } elsif ($ip =~ m!^$IPV6RE$!) {
+ return &$clean_func(6);
+ }
+
+ return $cidr;
+}
+
PVE::JSONSchema::register_standard_option('ipset-name', {
description => "IP set name.",
type => 'string',
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] [PATCH V2 firewall 2/2] cleanup: don't capture "/xx" of CIDR
2022-11-29 16:01 [pve-devel] [PATCH V2 firewall 0/2] allow non zero ip address host bits to be entered Stefan Hrdlicka
2022-11-29 16:01 ` [pve-devel] [PATCH V2 firewall 1/2] " Stefan Hrdlicka
@ 2022-11-29 16:01 ` Stefan Hrdlicka
1 sibling, 0 replies; 4+ messages in thread
From: Stefan Hrdlicka @ 2022-11-29 16:01 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
---
src/PVE/Firewall.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 3c35b44..73e940b 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -68,7 +68,7 @@ PVE::JSONSchema::register_format('IPorCIDR', \&pve_verify_ip_or_cidr);
sub pve_verify_ip_or_cidr {
my ($cidr, $noerr) = @_;
- if ($cidr =~ m!^(?:$IPV6RE|$IPV4RE)(/(\d+))?$!) {
+ if ($cidr =~ m!^(?:$IPV6RE|$IPV4RE)(?:/\d+)?$!) {
# Net::IP throws an error if the masked CIDR part isn't zero, e.g., `192.168.1.155/24`
# fails but `192.168.1.0/24` succeeds. clean_cidr removes the non zero bits from the CIDR.
my $clean_cidr = clean_cidr($cidr);
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH V2 firewall 1/2] allow non zero ip address host bits to be entered
2022-11-29 16:01 ` [pve-devel] [PATCH V2 firewall 1/2] " Stefan Hrdlicka
@ 2022-12-13 12:33 ` Wolfgang Bumiller
0 siblings, 0 replies; 4+ messages in thread
From: Wolfgang Bumiller @ 2022-12-13 12:33 UTC (permalink / raw)
To: Stefan Hrdlicka; +Cc: pve-devel
On Tue, Nov 29, 2022 at 05:01:51PM +0100, Stefan Hrdlicka wrote:
> They can already be set directly via the cluster.fw file. Net::IP is just a
> bit more picky with what it allows:
> For example:
> error: 192.168.1.155/24
> correct: 192.168.1.0/24
>
> This cleans the entered IP and removes the non zero host bits.
>
> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> ---
> src/PVE/API2/Firewall/IPSet.pm | 2 +-
> src/PVE/Firewall.pm | 29 ++++++++++++++++++++++++++++-
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
> index a5f69e9..14bcfcb 100644
> --- a/src/PVE/API2/Firewall/IPSet.pm
> +++ b/src/PVE/API2/Firewall/IPSet.pm
> @@ -199,7 +199,7 @@ sub register_create_ip {
>
> my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
>
> - my $cidr = $param->{cidr};
> + my $cidr = PVE::Firewall::clean_cidr($param->{cidr});
> if ($cidr =~ m/^${PVE::Firewall::ip_alias_pattern}$/) {
> # make sure alias exists (if $cidr is an alias)
> PVE::Firewall::resolve_alias($cluster_conf, $fw_conf, $cidr);
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index d40a9b1..3c35b44 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -69,8 +69,12 @@ sub pve_verify_ip_or_cidr {
> my ($cidr, $noerr) = @_;
>
> if ($cidr =~ m!^(?:$IPV6RE|$IPV4RE)(/(\d+))?$!) {
> - return $cidr if Net::IP->new($cidr);
> + # Net::IP throws an error if the masked CIDR part isn't zero, e.g., `192.168.1.155/24`
> + # fails but `192.168.1.0/24` succeeds. clean_cidr removes the non zero bits from the CIDR.
> + my $clean_cidr = clean_cidr($cidr);
> + return $cidr if Net::IP->new($clean_cidr);
> return undef if $noerr;
> +
> die Net::IP::Error() . "\n";
> }
> return undef if $noerr;
> @@ -86,6 +90,29 @@ sub pve_verify_ip_or_cidr_or_alias {
> return pve_verify_ip_or_cidr($cidr, $noerr);
> }
>
> +sub clean_cidr {
> + my ($cidr) = @_;
> + my ($ip, $len) = split('/', $cidr);
> + return $cidr if !$len;
> +
> + my $clean_func = sub {
> + my ($ver) = @_;
> + my $bin_ip = Net::IP::ip_iptobin( Net::IP::ip_expand_address($ip, $ver), $ver);
> + my $bin_mask = Net::IP::ip_get_mask($len, $ver);
> + my $clean_ip = Net::IP::ip_compress_address( Net::IP::ip_bintoip($bin_ip & $bin_mask, $ver), $ver);
> +
> + return "${clean_ip}/$len";
> + };
> +
> + if ($ip =~ m!^$IPV4RE$!) {
> + return &$clean_func(4)
> + } elsif ($ip =~ m!^$IPV6RE$!) {
> + return &$clean_func(6);
> + }
IMO we should either say "we know our input was validated already" and
check only one (then we don't need the closure and can just do
my $ver = ($ip =~ v4re...) ? 4 : 6;
or we also handle the `else` case here with an error.
Otherwise, LGTM
> +
> + return $cidr;
> +}
> +
> PVE::JSONSchema::register_standard_option('ipset-name', {
> description => "IP set name.",
> type => 'string',
> --
> 2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-12-13 12:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-29 16:01 [pve-devel] [PATCH V2 firewall 0/2] allow non zero ip address host bits to be entered Stefan Hrdlicka
2022-11-29 16:01 ` [pve-devel] [PATCH V2 firewall 1/2] " Stefan Hrdlicka
2022-12-13 12:33 ` Wolfgang Bumiller
2022-11-29 16:01 ` [pve-devel] [PATCH V2 firewall 2/2] cleanup: don't capture "/xx" of CIDR Stefan Hrdlicka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox