public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources
@ 2025-11-12 13:20 Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 1/9] api: sdn: rename vnets to zones in list_zones Gabriel Goller
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-11-12 13:20 UTC (permalink / raw)
  To: pdm-devel

This series relies on the following patch series from Stefan:
 - Add support for network resource type
   https://lore.proxmox.com/pdm-devel/20251107144018.700695-1-s.hanreich@proxmox.com/
 - IP-VRF and MAC-VRF status in EVPN panel
   https://lore.proxmox.com/pdm-devel/20251107085934.118815-1-s.hanreich@proxmox.com/

This RFC patch series introduces granular permission controls for SDN resources
(Zones, VNets, and Controllers).

The series is structured as follows:
[0-2] Initial cleanup and refactoring (mostly rename) that can be applied
      independently
[3-7] Adds permission filtering for zones/vnets based on acl paths
[8]   Adds permission filtering for resources

The list_zones and list_vnets endpoints are only used for EVPN stuff in the
EVPN view (currently). The final patch adds permission filtering to the SDN
view, checking only 'network' resource types while ignoring other resource
types. This could potentially be slow, and it's also awkward as we only check
the sdn resource and ignore the others (OTOH without this patch we'd have to
think about the rest of the series as hidden zones can be viewable and
searchable in the resources).

Open questions for discussion:
1. Should we implement granular permissions at this level, or rely on the
   existing PVE token/permissions system?
2. Is the proposed permission granularity appropriate, or too fine-grained?
3. Should the final patch (resource filtering) be included in this series?

proxmox-datacenter-manager:

Gabriel Goller (9):
  api: sdn: rename vnets to zones in list_zones
  api: sdn: rename "vnet" to "controller" in list_controller
  ui: improve error message when controller cannot be found
  api: allow acl paths longer than 4 segments in sdn
  api: sdn: add granular permissions for zones
  api: sdn: add granular permissions for vnets
  api: sdn: add granular permissions for controllers
  api: add permissions for ip-vrf and mac-vrf endpoints
  api: add permissions for sdn resources

 server/src/acl.rs                 |  9 +++++-
 server/src/api/nodes/sdn.rs       | 52 +++++++++++++++++++++++++++++--
 server/src/api/resources.rs       | 21 ++++++++++++-
 server/src/api/sdn/controllers.rs | 33 +++++++++++++-------
 server/src/api/sdn/vnets.rs       | 22 ++++++++-----
 server/src/api/sdn/zones.rs       | 30 +++++++++++-------
 ui/src/sdn/evpn/remote_tree.rs    |  6 ++--
 ui/src/sdn/evpn/vrf_tree.rs       |  6 ++--
 8 files changed, 142 insertions(+), 37 deletions(-)


Summary over all repositories:
  8 files changed, 142 insertions(+), 37 deletions(-)

-- 
Generated by git-murpp 0.8.0


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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 1/9] api: sdn: rename vnets to zones in list_zones
  2025-11-12 13:20 [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources Gabriel Goller
@ 2025-11-12 13:20 ` Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 2/9] api: sdn: rename "vnet" to "controller" in list_controller Gabriel Goller
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-11-12 13:20 UTC (permalink / raw)
  To: pdm-devel

Due to a copy-paste error, the result array and error messages use
"vnet" instead of "zone". Rename the result array and update error
messages to use correct "zone" terminology. No functional change
intended.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 server/src/api/sdn/zones.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/server/src/api/sdn/zones.rs b/server/src/api/sdn/zones.rs
index 2ae3d6734112..a185d368f365 100644
--- a/server/src/api/sdn/zones.rs
+++ b/server/src/api/sdn/zones.rs
@@ -102,7 +102,7 @@ pub async fn list_zones(
         None
     });
 
-    let mut vnets = Vec::new();
+    let mut zones = Vec::new();
     let fetcher = ParallelFetcher::new((pending, running, ty));
 
     let results = fetcher
@@ -117,14 +117,14 @@ pub async fn list_zones(
                 for (node, node_result) in remote_result.node_results.into_iter() {
                     match node_result {
                         Ok(NodeResults { data, .. }) => {
-                            vnets.extend(data.into_iter().map(|zone| ListZone {
+                            zones.extend(data.into_iter().map(|zone| ListZone {
                                 remote: remote.clone(),
                                 zone,
                             }))
                         }
                         Err(error) => {
                             log::error!(
-                                "could not fetch vnets from remote {} node {}: {error:#}",
+                                "could not fetch zones from remote {} node {}: {error:#}",
                                 remote,
                                 node
                             );
@@ -133,12 +133,12 @@ pub async fn list_zones(
                 }
             }
             Err(error) => {
-                log::error!("could not fetch vnets from remote {}: {error:#}", remote)
+                log::error!("could not fetch zones from remote {}: {error:#}", remote)
             }
         }
     }
 
-    Ok(vnets)
+    Ok(zones)
 }
 
 #[api(
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 2/9] api: sdn: rename "vnet" to "controller" in list_controller
  2025-11-12 13:20 [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 1/9] api: sdn: rename vnets to zones in list_zones Gabriel Goller
@ 2025-11-12 13:20 ` Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 3/9] ui: improve error message when controller cannot be found Gabriel Goller
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-11-12 13:20 UTC (permalink / raw)
  To: pdm-devel

Due to a copy-paste error, the result array and error messages were
incorrectly named "vnet". This renames them to "controller" which is
correct.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 server/src/api/sdn/controllers.rs | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/server/src/api/sdn/controllers.rs b/server/src/api/sdn/controllers.rs
index 966125162ef6..e6ab5367792b 100644
--- a/server/src/api/sdn/controllers.rs
+++ b/server/src/api/sdn/controllers.rs
@@ -94,7 +94,7 @@ pub async fn list_controllers(
         None
     });
 
-    let mut vnets = Vec::new();
+    let mut controllers = Vec::new();
     let fetcher = ParallelFetcher::new((pending, running, ty));
 
     let results = fetcher
@@ -111,14 +111,14 @@ pub async fn list_controllers(
                 for (node, node_result) in remote_result.node_results.into_iter() {
                     match node_result {
                         Ok(NodeResults { data, .. }) => {
-                            vnets.extend(data.into_iter().map(|controller| ListController {
+                            controllers.extend(data.into_iter().map(|controller| ListController {
                                 remote: remote.clone(),
                                 controller,
                             }))
                         }
                         Err(error) => {
                             log::error!(
-                                "could not fetch vnets from remote {} node {}: {error:#}",
+                                "could not fetch controllers from remote {} node {}: {error:#}",
                                 remote,
                                 node
                             );
@@ -127,10 +127,13 @@ pub async fn list_controllers(
                 }
             }
             Err(error) => {
-                log::error!("could not fetch vnets from remote {}: {error:#}", remote)
+                log::error!(
+                    "could not fetch controllers from remote {}: {error:#}",
+                    remote
+                )
             }
         }
     }
 
-    Ok(vnets)
+    Ok(controllers)
 }
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 3/9] ui: improve error message when controller cannot be found
  2025-11-12 13:20 [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 1/9] api: sdn: rename vnets to zones in list_zones Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 2/9] api: sdn: rename "vnet" to "controller" in list_controller Gabriel Goller
@ 2025-11-12 13:20 ` Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 4/9] api: allow acl paths longer than 4 segments in sdn Gabriel Goller
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-11-12 13:20 UTC (permalink / raw)
  To: pdm-devel

With the added permissions, it often happens that the user has
permissions to view the zone, but doesn't have permissions to view the
controller. This means we can't display the zone in the EVPN overview.
"not found" is a more correct error than "doesn't exist".

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 ui/src/sdn/evpn/remote_tree.rs | 6 ++++--
 ui/src/sdn/evpn/vrf_tree.rs    | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/ui/src/sdn/evpn/remote_tree.rs b/ui/src/sdn/evpn/remote_tree.rs
index ee57b33c18f2..7243a03329c0 100644
--- a/ui/src/sdn/evpn/remote_tree.rs
+++ b/ui/src/sdn/evpn/remote_tree.rs
@@ -176,7 +176,8 @@ fn zones_to_remote_view(
             })
             .ok_or_else(|| {
                 anyhow!(tr!(
-                    "Could not find Controller for EVPN zone {}",
+                    "Controller {} of EVPN zone {} not found",
+                    zone_controller_id,
                     zone_data.zone
                 ))
             })?;
@@ -260,7 +261,8 @@ fn zones_to_remote_view(
             })
             .ok_or_else(|| {
                 anyhow!(tr!(
-                    "Controller of EVPN zone {} does not exist",
+                    "Controller {} of EVPN zone {} not found",
+                    zone_controller_id,
                     zone.zone.zone
                 ))
             })?;
diff --git a/ui/src/sdn/evpn/vrf_tree.rs b/ui/src/sdn/evpn/vrf_tree.rs
index 8481dfcf6027..d690501265a9 100644
--- a/ui/src/sdn/evpn/vrf_tree.rs
+++ b/ui/src/sdn/evpn/vrf_tree.rs
@@ -140,7 +140,8 @@ fn zones_to_vrf_view(
             })
             .ok_or_else(|| {
                 anyhow!(tr!(
-                    "Controller of EVPN zone {} does not exist",
+                    "Controller {} of EVPN zone {} not found",
+                    zone_controller_id,
                     zone_data.zone
                 ))
             })?;
@@ -199,7 +200,8 @@ fn zones_to_vrf_view(
             })
             .ok_or_else(|| {
                 anyhow!(tr!(
-                    "Controller of EVPN zone {} does not exist",
+                    "Controller {} of EVPN zone {} not found",
+                    zone_controller_id,
                     zone.zone.zone
                 ))
             })?;
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 4/9] api: allow acl paths longer than 4 segments in sdn
  2025-11-12 13:20 [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources Gabriel Goller
                   ` (2 preceding siblings ...)
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 3/9] ui: improve error message when controller cannot be found Gabriel Goller
@ 2025-11-12 13:20 ` Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 5/9] api: sdn: add granular permissions for zones Gabriel Goller
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-11-12 13:20 UTC (permalink / raw)
  To: pdm-devel

When setting granular permissions on SDN zones, vnets or controllers,
the path includes both the resource type and name. This results in
5 path segments (e.g., `/sdn/zones/{zone}`), exceeding the previous
4-segment limit.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 server/src/acl.rs | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/server/src/acl.rs b/server/src/acl.rs
index 52a1f972b9a9..709c7ca073b0 100644
--- a/server/src/acl.rs
+++ b/server/src/acl.rs
@@ -99,7 +99,7 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
                 if components_len <= 2 {
                     return Ok(());
                 }
-                // `/resource/{remote-id}/{resource-type=guest,storage}/{resource-id}`
+                // `/resource/{remote-id}/{resource-type=guest,storage}/...`
                 match components[2] {
                     "guest" | "storage" => {
                         // /resource/{remote-id}/{resource-type}
@@ -108,6 +108,13 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
                             return Ok(());
                         }
                     }
+                    "sdn" => {
+                        // /resource/{remote-id}/sdn
+                        // /resource/{remote-id}/sdn/{sdn-type}/{sdn-id}
+                        if components_len <= 5 {
+                            return Ok(());
+                        }
+                    }
                     _ => {}
                 }
             }
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 5/9] api: sdn: add granular permissions for zones
  2025-11-12 13:20 [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources Gabriel Goller
                   ` (3 preceding siblings ...)
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 4/9] api: allow acl paths longer than 4 segments in sdn Gabriel Goller
@ 2025-11-12 13:20 ` Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 6/9] api: sdn: add granular permissions for vnets Gabriel Goller
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-11-12 13:20 UTC (permalink / raw)
  To: pdm-devel

Add more granular permissions for sdn zones. It is possible to add
permissions like `/resource/{resource}/sdn/zone/{zone-name}`.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 server/src/api/sdn/zones.rs | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/server/src/api/sdn/zones.rs b/server/src/api/sdn/zones.rs
index a185d368f365..c2536289bcfe 100644
--- a/server/src/api/sdn/zones.rs
+++ b/server/src/api/sdn/zones.rs
@@ -62,7 +62,7 @@ pub const ROUTER: Router = Router::new()
         permission: &Permission::Anybody,
         description: "The user needs to have at least the `Resource.Audit` privilege under `/resource`.
         Only zones from remotes for which the user has `Resource.Audit` on `/resource/{remote_name}`
-        will be included in the returned list."
+        and `/resource/{remote_name}/sdn/zone/{zone_name}` will be included in the returned list."
     }
 )]
 /// Query zones of remotes with optional filtering options
@@ -116,12 +116,20 @@ pub async fn list_zones(
             Ok(remote_result) => {
                 for (node, node_result) in remote_result.node_results.into_iter() {
                     match node_result {
-                        Ok(NodeResults { data, .. }) => {
-                            zones.extend(data.into_iter().map(|zone| ListZone {
-                                remote: remote.clone(),
-                                zone,
-                            }))
-                        }
+                        Ok(NodeResults { data, .. }) => zones.extend(
+                            data.into_iter()
+                                .filter(|zone| {
+                                    user_info.lookup_privs(
+                                        &auth_id,
+                                        &["resource", &remote, "sdn", "zone", &zone.zone],
+                                    ) & PRIV_RESOURCE_AUDIT
+                                        != 0
+                                })
+                                .map(|zone| ListZone {
+                                    remote: remote.clone(),
+                                    zone,
+                                }),
+                        ),
                         Err(error) => {
                             log::error!(
                                 "could not fetch zones from remote {} node {}: {error:#}",
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 6/9] api: sdn: add granular permissions for vnets
  2025-11-12 13:20 [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources Gabriel Goller
                   ` (4 preceding siblings ...)
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 5/9] api: sdn: add granular permissions for zones Gabriel Goller
@ 2025-11-12 13:20 ` Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 7/9] api: sdn: add granular permissions for controllers Gabriel Goller
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-11-12 13:20 UTC (permalink / raw)
  To: pdm-devel

Add granular permission for sdn vnets. So you can specify
`/resource/{remote}/sdn/vnet/{vnet-name}`.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 server/src/api/sdn/vnets.rs | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/server/src/api/sdn/vnets.rs b/server/src/api/sdn/vnets.rs
index ebd28742aa25..3c4397072f92 100644
--- a/server/src/api/sdn/vnets.rs
+++ b/server/src/api/sdn/vnets.rs
@@ -57,7 +57,7 @@ pub const ROUTER: Router = Router::new()
         permission: &Permission::Anybody,
         description: "The user needs to have at least the `Resource.Audit` privilege under `/resource`.
         Only vnets from remotes for which the user has `Resource.Audit` on `/resource/{remote_name}`
-        will be included in the returned list."
+        and `/resource/{remote_name}/sdn/vnet/{vnet}` will be included in the returned list."
     }
 )]
 /// Query VNets of PVE remotes with optional filtering options
@@ -110,12 +110,20 @@ async fn list_vnets(
             Ok(remote_result) => {
                 for (node, node_result) in remote_result.node_results.into_iter() {
                     match node_result {
-                        Ok(NodeResults { data, .. }) => {
-                            vnets.extend(data.into_iter().map(|vnet| ListVnet {
-                                remote: remote.clone(),
-                                vnet,
-                            }))
-                        }
+                        Ok(NodeResults { data, .. }) => vnets.extend(
+                            data.into_iter()
+                                .filter(|vnet| {
+                                    user_info.lookup_privs(
+                                        &auth_id,
+                                        &["resource", &remote, "sdn", "vnet", &vnet.vnet],
+                                    ) & PRIV_RESOURCE_AUDIT
+                                        != 0
+                                })
+                                .map(|vnet| ListVnet {
+                                    remote: remote.clone(),
+                                    vnet,
+                                }),
+                        ),
                         Err(error) => {
                             log::error!(
                                 "could not fetch vnets from remote {} node {}: {error:#}",
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 7/9] api: sdn: add granular permissions for controllers
  2025-11-12 13:20 [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources Gabriel Goller
                   ` (5 preceding siblings ...)
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 6/9] api: sdn: add granular permissions for vnets Gabriel Goller
@ 2025-11-12 13:20 ` Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 8/9] api: add permissions for ip-vrf and mac-vrf endpoints Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 9/9] api: add permissions for sdn resources Gabriel Goller
  8 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-11-12 13:20 UTC (permalink / raw)
  To: pdm-devel

Add granular permissions for sdn controllers. This allows us the
specific permissions like:
`/resource/{remote}/sdn/controller/{controller-name}`.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 server/src/api/sdn/controllers.rs | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/server/src/api/sdn/controllers.rs b/server/src/api/sdn/controllers.rs
index e6ab5367792b..5b58e6307d44 100644
--- a/server/src/api/sdn/controllers.rs
+++ b/server/src/api/sdn/controllers.rs
@@ -54,7 +54,7 @@ pub const ROUTER: Router = Router::new().get(&API_METHOD_LIST_CONTROLLERS);
         permission: &Permission::Anybody,
         description: "The user needs to have at least the `Resource.Audit` privilege under `/resource`.
         Only controllers from remotes for which the user has `Resource.Audit` on `/resource/{remote_name}`
-        will be included in the returned list."
+        and `/resource/{remote_name}/sdn/controller/{controller}` will be included in the returned list."
     }
 )]
 /// Query controllers of remotes with optional filtering options
@@ -110,12 +110,20 @@ pub async fn list_controllers(
             Ok(remote_result) => {
                 for (node, node_result) in remote_result.node_results.into_iter() {
                     match node_result {
-                        Ok(NodeResults { data, .. }) => {
-                            controllers.extend(data.into_iter().map(|controller| ListController {
-                                remote: remote.clone(),
-                                controller,
-                            }))
-                        }
+                        Ok(NodeResults { data, .. }) => controllers.extend(
+                            data.into_iter()
+                                .filter(|c| {
+                                    user_info.lookup_privs(
+                                        &auth_id,
+                                        &["resource", &remote, "sdn", "controller", &c.controller],
+                                    ) & PRIV_RESOURCE_AUDIT
+                                        != 0
+                                })
+                                .map(|controller| ListController {
+                                    remote: remote.clone(),
+                                    controller,
+                                }),
+                        ),
                         Err(error) => {
                             log::error!(
                                 "could not fetch controllers from remote {} node {}: {error:#}",
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 8/9] api: add permissions for ip-vrf and mac-vrf endpoints
  2025-11-12 13:20 [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources Gabriel Goller
                   ` (6 preceding siblings ...)
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 7/9] api: sdn: add granular permissions for controllers Gabriel Goller
@ 2025-11-12 13:20 ` Gabriel Goller
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 9/9] api: add permissions for sdn resources Gabriel Goller
  8 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-11-12 13:20 UTC (permalink / raw)
  To: pdm-devel

The ip-vrf and mac-vrf endpoints expose EVPN zone and vnet information
but currently lack access control. Add permission checks that require
Audit privileges on the respective zone or vnet.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 server/src/api/nodes/sdn.rs | 52 +++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/server/src/api/nodes/sdn.rs b/server/src/api/nodes/sdn.rs
index 065ebe07846e..ee857ea9eee9 100644
--- a/server/src/api/nodes/sdn.rs
+++ b/server/src/api/nodes/sdn.rs
@@ -1,4 +1,4 @@
-use anyhow::{anyhow, Error};
+use anyhow::{anyhow, format_err, Error};
 use http::StatusCode;
 
 use pdm_api_types::{remotes::REMOTE_ID_SCHEMA, sdn::SDN_ID_SCHEMA, NODE_SCHEMA};
@@ -9,6 +9,10 @@ use pve_api_types::{SdnVnetMacVrf, SdnZoneIpVrf};
 use crate::api::pve::{connect, get_remote};
 
 mod zones {
+    use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT};
+    use proxmox_access_control::CachedUserInfo;
+    use proxmox_router::{http_bail, Permission, RpcEnvironment};
+
     use super::*;
 
     const ZONE_SUBDIRS: SubdirMap = &[("ip-vrf", &Router::new().get(&API_METHOD_GET_IP_VRF))];
@@ -28,13 +32,33 @@ mod zones {
             },
         },
         returns: { type: SdnZoneIpVrf },
+        access: {
+            permission: &Permission::Anybody,
+            description: "The user needs to have at least the `Resource.Audit` privilege on 
+            `/resource/{remote}/sdn/zone/{zone}`."
+        }
     )]
     /// Get the IP-VRF for an EVPN zone for a node on a given remote
     async fn get_ip_vrf(
         remote: String,
         node: String,
         zone: String,
+        rpcenv: &mut dyn RpcEnvironment,
     ) -> Result<Vec<SdnZoneIpVrf>, Error> {
+        let user_info = CachedUserInfo::new()?;
+
+        let auth_id: Authid = rpcenv
+            .get_auth_id()
+            .ok_or_else(|| format_err!("no authid available"))?
+            .parse()?;
+
+        if user_info.lookup_privs(&auth_id, &["resource", &remote, "sdn", "zone", &zone])
+            & PRIV_RESOURCE_AUDIT
+            == 0
+        {
+            http_bail!(FORBIDDEN, "user has no access to {remote}/sdn/zone/{zone}");
+        }
+
         let (remote_config, _) = pdm_config::remotes::config()?;
         let remote = get_remote(&remote_config, &remote)?;
         let client = connect(remote)?;
@@ -52,6 +76,10 @@ mod zones {
 }
 
 mod vnets {
+    use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT};
+    use proxmox_access_control::CachedUserInfo;
+    use proxmox_router::{http_bail, Permission, RpcEnvironment};
+
     use super::*;
 
     const VNET_SUBDIRS: SubdirMap = &[("mac-vrf", &Router::new().get(&API_METHOD_GET_MAC_VRF))];
@@ -71,16 +99,36 @@ mod vnets {
             },
         },
         returns: { type: SdnVnetMacVrf },
+        access: {
+            permission: &Permission::Anybody,
+            description: "The user needs to have at least the `Resource.Audit` privilege on 
+            `/resource/{remote}/sdn/vnet/{vnet}`."
+        }
     )]
     /// Get the MAC-VRF for an EVPN vnet for a node on a given remote
     async fn get_mac_vrf(
         remote: String,
         node: String,
         vnet: String,
+        rpcenv: &mut dyn RpcEnvironment,
     ) -> Result<Vec<SdnVnetMacVrf>, Error> {
+        let user_info = CachedUserInfo::new()?;
+
+        let auth_id: Authid = rpcenv
+            .get_auth_id()
+            .ok_or_else(|| format_err!("no authid available"))?
+            .parse()?;
+
+        if user_info.lookup_privs(&auth_id, &["resource", &remote, "sdn", "vnet", &vnet])
+            & PRIV_RESOURCE_AUDIT
+            == 0
+        {
+            http_bail!(FORBIDDEN, "user has no access to {remote}/sdn/vnet/{vnet}");
+        }
+
         let (remote_config, _) = pdm_config::remotes::config()?;
         let remote = get_remote(&remote_config, &remote)?;
-        let client = connect(&remote)?;
+        let client = connect(remote)?;
 
         client
             .get_vnet_mac_vrf(&node, &vnet)
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 9/9] api: add permissions for sdn resources
  2025-11-12 13:20 [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources Gabriel Goller
                   ` (7 preceding siblings ...)
  2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 8/9] api: add permissions for ip-vrf and mac-vrf endpoints Gabriel Goller
@ 2025-11-12 13:20 ` Gabriel Goller
  8 siblings, 0 replies; 10+ messages in thread
From: Gabriel Goller @ 2025-11-12 13:20 UTC (permalink / raw)
  To: pdm-devel

Until now, the resources do not have any granular permissions, you only
need to have `Audit` on `/resources/{resource-name}` and you will have
access to all resources. In order to limit this more, check permissions
when every resource object is added to the list. Note that this probably
has some performance implications.
Only SDN is considered at the moment.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 server/src/api/resources.rs | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 8b9d3b1baa25..b6680e3f1c71 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -309,7 +309,7 @@ pub(crate) async fn get_resources_impl(
     let remotes_only = is_remotes_only(&filters);
 
     for (remote_name, remote) in remotes_config {
-        if let Some(ref auth_id) = opt_auth_id {
+        if let Some(auth_id) = &opt_auth_id {
             let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
             if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
                 continue;
@@ -327,6 +327,8 @@ pub(crate) async fn get_resources_impl(
             continue;
         }
         let filter = filters.clone();
+        let user_info = user_info.clone();
+        let opt_auth_id = opt_auth_id.clone();
         let handle = tokio::spawn(async move {
             let (mut resources, error) = match get_resources_for_remote(&remote, max_age).await {
                 Ok(resources) => (resources, None),
@@ -346,6 +348,23 @@ pub(crate) async fn get_resources_impl(
                         }
                     }
 
+                    // check permissions
+                    if let (Resource::PveNetwork(sdn_resource), Some(auth_id)) =
+                        (resource, &opt_auth_id)
+                    {
+                        return (user_info.lookup_privs(
+                            auth_id,
+                            &[
+                                "resource",
+                                &remote_name,
+                                "sdn",
+                                sdn_resource.network_type().as_str(),
+                                sdn_resource.name(),
+                            ],
+                        ) & PRIV_RESOURCE_AUDIT)
+                            != 0;
+                    }
+
                     filter.matches(|filter| {
                         // if we get can't decide if it matches, don't filter it out
                         resource_matches_search_term(&remote_name, resource, filter).unwrap_or(true)
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-11-12 13:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-12 13:20 [pdm-devel] [RFC proxmox-datacenter-manager 0/9] Granular permissions for SDN resources Gabriel Goller
2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 1/9] api: sdn: rename vnets to zones in list_zones Gabriel Goller
2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 2/9] api: sdn: rename "vnet" to "controller" in list_controller Gabriel Goller
2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 3/9] ui: improve error message when controller cannot be found Gabriel Goller
2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 4/9] api: allow acl paths longer than 4 segments in sdn Gabriel Goller
2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 5/9] api: sdn: add granular permissions for zones Gabriel Goller
2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 6/9] api: sdn: add granular permissions for vnets Gabriel Goller
2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 7/9] api: sdn: add granular permissions for controllers Gabriel Goller
2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 8/9] api: add permissions for ip-vrf and mac-vrf endpoints Gabriel Goller
2025-11-12 13:20 ` [pdm-devel] [PATCH proxmox-datacenter-manager 9/9] api: add permissions for sdn resources Gabriel Goller

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