[pve-devel] [pve-cluster/pve-network] move macs.db from priv/ to sdn/

[pve-devel] [pve-cluster/pve-network] move macs.db from priv/ to sdn/

[Stoiko Ivanov]

while looking through the changes in pve-network, and testing the
port-isolation series I saw the move of priv/ipam.db to
sdn/pve-ipam-state.json:

https://git.proxmox.com/?p=pve-network.git;a=commitdiff;h=0f48bc6561f2fd901f2665387b4954c8105614e0

Following the argumentation the mac-cache file should probably also be
moved to a non-root-exclusive directory.

From a quick glance the remaining files in `priv/` do belong there.

minimally tested this on my machine.

pve-cluster:
Stoiko Ivanov (1):
  cfs: add 'sdn/mac-cache.json' to observed files

 src/PVE/Cluster.pm  | 3 ++-
 src/pmxcfs/status.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

pve-network:
Stoiko Ivanov (1):
  ipam: move mac-cache.db to unprivileged sdn/ subdirectory

 src/PVE/Network/SDN/Ipams.pm | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

-- 
2.39.5



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

[pve-devel] [PATCH cluster 1/1] cfs: add 'sdn/mac-cache.json' to observed files

[Stoiko Ivanov]

follows commit:
d8ef05c (cfs: add 'sdn/pve-ipam-state.json' to observed files)
with the same motivation - the data in the macs.db file is a cache, to
prevent unneccessary lookups to external IPAM modules - is not private
in the sense of secrets for external resources.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PVE/Cluster.pm  | 3 ++-
 src/pmxcfs/status.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm
index 28ebcfe..e0e3ee9 100644
--- a/src/PVE/Cluster.pm
+++ b/src/PVE/Cluster.pm
@@ -62,7 +62,7 @@ my $observed = {
     'priv/token.cfg' => 1,
     'priv/acme/plugins.cfg' => 1,
     'priv/ipam.db' => 1, # TODO: replaced by sdn/ipam-pve-db.json, remove in PVE 9+
-    'priv/macs.db' => 1,
+    'priv/macs.db' => 1, # TODO: replaced by sdn/mac-cache.json, remove in PVE 9+
     '/qemu-server/' => 1,
     '/openvz/' => 1,
     '/lxc/' => 1,
@@ -79,6 +79,7 @@ my $observed = {
     'sdn/subnets.cfg' => 1,
     'sdn/ipams.cfg' => 1,
     'sdn/pve-ipam-state.json' => 1,
+    'sdn/mac-cache.json' => 1,
     'sdn/dns.cfg' => 1,
     'sdn/.running-config' => 1,
     'virtual-guest/cpu-models.conf' => 1,
diff --git a/src/pmxcfs/status.c b/src/pmxcfs/status.c
index 6e5d4fb..ff5fcc4 100644
--- a/src/pmxcfs/status.c
+++ b/src/pmxcfs/status.c
@@ -89,7 +89,7 @@ static memdb_change_t memdb_change_array[] = {
 	{ .path = "priv/tfa.cfg" },
 	{ .path = "priv/token.cfg" },
 	{ .path = "priv/ipam.db" }, // TODO: replaced by sdn/ipam-pve-db.json remove with PVE 9 or later
-	{ .path = "priv/macs.db" },
+	{ .path = "priv/macs.db" }, // TODO: replaced by sdn/mac-cache.json remove with PVE 9 or later
 	{ .path = "datacenter.cfg" },
 	{ .path = "vzdump.cron" },
 	{ .path = "vzdump.conf" },
@@ -107,6 +107,7 @@ static memdb_change_t memdb_change_array[] = {
 	{ .path = "sdn/controllers.cfg" },
 	{ .path = "sdn/subnets.cfg" },
 	{ .path = "sdn/ipams.cfg" },
+	{ .path = "sdn/mac-cache.json" },
 	{ .path = "sdn/pve-ipam-state.json" },
 	{ .path = "sdn/dns.cfg" },
 	{ .path = "sdn/.running-config" },
-- 
2.39.5



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

[pve-devel] [pve-network 1/1] ipam: move mac-cache.db to unprivileged sdn/ subdirectory

[Stoiko Ivanov]

follows commit:
0f48bc6 ("ipam: move state file of PVE plugin over to common sdn directory")
as far as reasoning goes, and also closely code-wise (if only to make
the clean-up with PVE 9.0 a bit more straight-forward):
files in priv/ are sensitive in the sense that access there can be
used to hijack (external systems) - the mac-cache can be kept next to
the remaining sdn-config.

minimally tested on my machine.
depends on the pve-cluster commit sent with this.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PVE/Network/SDN/Ipams.pm | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 926df90..c689b8f 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -20,9 +20,34 @@ PVE::Network::SDN::Ipams::NetboxPlugin->register();
 PVE::Network::SDN::Ipams::PhpIpamPlugin->register();
 PVE::Network::SDN::Ipams::Plugin->init();
 
-my $macdb_filename = 'priv/macs.db';
+my $macdb_filename = "sdn/mac-cache.json";
+my $macdb_filename_legacy = 'priv/macs.db';
+
+cfs_register_file(
+    $macdb_filename,
+    sub {
+	my ($filename , $data) = @_;
+	if (defined($data)) {
+	    return json_reader($filename, $data);
+	} else {
+	    # TODO: remove legacy cache file handling with PVE 9+ after ensuring all call sites got
+	    # switched over.
+	    return cfs_read_file($macdb_filename_legacy);
+	}
+    },
+    sub {
+	my ($filename , $data) = @_;
+	# TODO: remove below with PVE 9+, add a pve8to9 check to allow doing so.
+	if (-e $macdb_filename_legacy && -e $macdb_filename) {
+	    # only clean-up if we succeeded to write the new path at least once
+	    unlink $macdb_filename_legacy or $!{ENOENT} or warn "failed to unlink legacy MAC cache - $!\n";
+	}
+	return json_writer->($filename, $data);
+    }
+);
 
-cfs_register_file($macdb_filename, \&json_reader, \&json_writer);
+# drop reading $macdb_filename_legacy with PVE 9+ - for now do not write it anymore.
+cfs_register_file($macdb_filename_legacy, \&json_reader, undef);
 
 sub json_reader {
     my ($filename, $data) = @_;
-- 
2.39.5



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

[pve-devel] applied: [pve-cluster/pve-network] move macs.db from priv/ to sdn/

[Thomas Lamprecht]

Am 18.11.24 um 21:55 schrieb Stoiko Ivanov:
> while looking through the changes in pve-network, and testing the
> port-isolation series I saw the move of priv/ipam.db to
> sdn/pve-ipam-state.json:
> 
> https://git.proxmox.com/?p=pve-network.git;a=commitdiff;h=0f48bc6561f2fd901f2665387b4954c8105614e0
> 
> Following the argumentation the mac-cache file should probably also be
> moved to a non-root-exclusive directory.
> 
> From a quick glance the remaining files in `priv/` do belong there.
> 
> minimally tested this on my machine.
> 
> pve-cluster:
> Stoiko Ivanov (1):
>   cfs: add 'sdn/mac-cache.json' to observed files
> 
>  src/PVE/Cluster.pm  | 3 ++-
>  src/pmxcfs/status.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> pve-network:
> Stoiko Ivanov (1):
>   ipam: move mac-cache.db to unprivileged sdn/ subdirectory
> 
>  src/PVE/Network/SDN/Ipams.pm | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 


applied, huge thanks for catching this early enough!


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