public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Leo Nunner <l.nunner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH firewall 2/3] fix #4556: introduce 'dc' and 'vm' prefix for aliases
Date: Wed,  7 Jun 2023 12:17:49 +0200	[thread overview]
Message-ID: <20230607101751.87616-3-l.nunner@proxmox.com> (raw)
In-Reply-To: <20230607101751.87616-1-l.nunner@proxmox.com>

since they had the same issue as IPSets, detailed in #4556. The format
works the same as for IPSets:

    dc/alias
	Looks for the alias on the Datacenter level.
    vm/alias
	Looks for the alias on the VM level.
    alias
	Uses the previous method of scoping, where it first looks at the
	VM level and then at the Datacenter level.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PVE/API2/Firewall/IPSet.pm |  9 ++++--
 src/PVE/Firewall.pm            | 58 +++++++++++++++++++++++-----------
 2 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
index 14bcfcb..515f2d3 100644
--- a/src/PVE/API2/Firewall/IPSet.pm
+++ b/src/PVE/API2/Firewall/IPSet.pm
@@ -199,11 +199,14 @@ sub register_create_ip {
 
 		my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
 
-		my $cidr = PVE::Firewall::clean_cidr($param->{cidr});
-		if ($cidr =~ m/^${PVE::Firewall::ip_alias_pattern}$/) {
+		my $cidr = $param->{cidr};
+		if ($cidr =~ m@^(dc/|vm/)?(${PVE::Firewall::ip_alias_pattern})$@) {
+		    my $scope = $1 // "";
+		    my $alias = $2;
 		    # make sure alias exists (if $cidr is an alias)
-		    PVE::Firewall::resolve_alias($cluster_conf, $fw_conf, $cidr);
+		    PVE::Firewall::resolve_alias($cluster_conf, $fw_conf, $alias, $scope);
 		} else {
+		    $cidr = PVE::Firewall::clean_cidr($cidr);
 		    # normalize like config parser, otherwise duplicates might slip through
 		    $cidr = PVE::Firewall::parse_ip_or_cidr($cidr);
 		}
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index ff18de0..ecde176 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -85,7 +85,7 @@ PVE::JSONSchema::register_format('IPorCIDRorAlias', \&pve_verify_ip_or_cidr_or_a
 sub pve_verify_ip_or_cidr_or_alias {
     my ($cidr, $noerr) = @_;
 
-    return if $cidr =~ m/^(?:$ip_alias_pattern)$/;
+    return if $cidr =~ m@^(dc/|vm/)?(?:$ip_alias_pattern)$@;
 
     return pve_verify_ip_or_cidr($cidr, $noerr);
 }
@@ -1067,7 +1067,7 @@ sub parse_address_list {
 	return;
     }
 
-    if ($str =~ m/^${ip_alias_pattern}$/) {
+    if ($str =~ m@^(dc/|vm/)?${ip_alias_pattern}$@) {
 	die "alias name too long\n" if length($str) > $max_alias_name_length;
 	return;
     }
@@ -1690,12 +1690,19 @@ sub verify_rule {
 		} else {
 		    &$add_error($name, "invalid ipset name '$value'");
 		}
-	    } elsif ($value =~ m/^${ip_alias_pattern}$/){
-		my $alias = lc($value);
+	    } elsif ($value =~ m@^(vm/|dc/)?(${ip_alias_pattern})$@){
+		my $scope = $1 // "";
+		my $alias = lc($2);
 		&$add_error($name, "no such alias '$value'")
 		    if !($cluster_conf->{aliases}->{$alias} || ($fw_conf && $fw_conf->{aliases}->{$alias}));
-		my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef;
-		$e = $cluster_conf->{aliases}->{$alias} if !$e && $cluster_conf;
+
+		my $e;
+		if ($scope ne 'dc/' && $fw_conf) {
+		    $e = $fw_conf->{aliases}->{$alias};
+		}
+		if ($scope ne 'vm/' && !$e && $cluster_conf) {
+		    $e = $cluster_conf->{aliases}->{$alias};
+		}
 
 		&$set_ip_version($e->{ipversion});
 	    }
@@ -2096,7 +2103,7 @@ sub ipt_gen_src_or_dst_match {
     my $match;
     if ($adr =~ m/^\+/) {
 	if ($adr =~ m@^\+(vm/|dc/)?(${ipset_name_pattern})$@) {
-	    my $scope = $1;
+	    my $scope = $1 // "";
 	    my $name = $2;
 	    my $ipset_chain;
 	    if ($scope ne 'dc/' && $fw_conf && $fw_conf->{ipset}->{$name}) {
@@ -2110,10 +2117,16 @@ sub ipt_gen_src_or_dst_match {
 	} else {
 	    die "invalid security group name '$adr'\n";
 	}
-    } elsif ($adr =~ m/^${ip_alias_pattern}$/){
-	my $alias = lc($adr);
-	my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef;
-	$e = $cluster_conf->{aliases}->{$alias} if !$e && $cluster_conf;
+    } elsif ($adr =~ m@^(dc/|vm/)?(${ip_alias_pattern})$@){
+	my $scope = $1 // "";
+	my $alias = lc($2);
+	my $e;
+	if ($scope ne 'dc/' && $fw_conf) {
+	    $e = $fw_conf->{aliases}->{$alias};
+	}
+	if ($scope ne 'vm/' && !$e && $cluster_conf) {
+	    $e = $cluster_conf->{aliases}->{$alias};
+	}
 	die "no such alias '$adr'\n" if !$e;
 	$match = "-${dir} $e->{cidr}";
     } elsif ($adr =~ m/\-/){
@@ -2964,11 +2977,16 @@ sub parse_clusterfw_option {
 }
 
 sub resolve_alias {
-    my ($clusterfw_conf, $fw_conf, $cidr) = @_;
+    my ($clusterfw_conf, $fw_conf, $cidr, $scope) = @_;
 
     my $alias = lc($cidr);
-    my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef;
-    $e = $clusterfw_conf->{aliases}->{$alias} if !$e && $clusterfw_conf;
+    my $e;
+    if ($scope ne 'dc/' && $fw_conf) {
+	$e = $fw_conf->{aliases}->{$alias};
+    }
+    if ($scope ne 'vm/' && !$e && $clusterfw_conf) {
+	$e = $clusterfw_conf->{aliases}->{$alias};
+    }
 
     die "no such alias '$cidr'\n" if !$e;;
 
@@ -3156,8 +3174,10 @@ sub generic_fw_config_parser {
 	    }
 
 	    eval {
-		if ($cidr =~ m/^${ip_alias_pattern}$/) {
-		    resolve_alias($cluster_conf, $res, $cidr); # make sure alias exists
+		if ($cidr =~ m@^(dc/|vm/)?(${ip_alias_pattern}$)@) {
+		    my $scope = $1 // "";
+		    my $alias = $2;
+		    resolve_alias($cluster_conf, $res, $alias, $scope); # make sure alias exists
 		} else {
 		    $cidr = parse_ip_or_cidr($cidr);
 		}
@@ -3508,8 +3528,10 @@ sub generate_ipset_chains {
 	    next if $entry->{errors}; # skip entries with errors
 	    eval {
 		my ($cidr, $ver);
-		if ($entry->{cidr} =~ m/^${ip_alias_pattern}$/) {
-		    ($cidr, $ver) = resolve_alias($clusterfw_conf, $fw_conf, $entry->{cidr});
+		if ($entry->{cidr} =~ m@^(dc/|vm/)?(${ip_alias_pattern})$@) {
+		    my $scope = $1 // "";
+		    my $alias = $2;
+		    ($cidr, $ver) = resolve_alias($clusterfw_conf, $fw_conf, $alias, $scope);
 		} else {
 		    ($cidr, $ver) = parse_ip_or_cidr($entry->{cidr});
 		}
-- 
2.30.2





  parent reply	other threads:[~2023-06-07 10:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-07 10:17 [pve-devel] [PATCH firewall/manager] firewall: introduce scoping for ipsets/aliases Leo Nunner
2023-06-07 10:17 ` [pve-devel] [PATCH firewall 1/3] fix #4556: introduce 'dc' and 'vm' prefix for IPSets Leo Nunner
2023-06-07 10:17 ` Leo Nunner [this message]
2023-06-07 13:14   ` [pve-devel] [PATCH firewall 2/3] fix #4556: introduce 'dc' and 'vm' prefix for aliases Wolfgang Bumiller
2023-06-07 10:17 ` [pve-devel] [PATCH firewall 3/3] fix #4556: api: return scoped IPSets and aliases Leo Nunner
2023-06-07 10:17 ` [pve-devel] [PATCH manager] firewall: add scope field to IPRefSelector Leo Nunner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230607101751.87616-3-l.nunner@proxmox.com \
    --to=l.nunner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal