use reasonable acl paths

src/api2/access/acl.rs

@@ -5,7 +5,7 @@ use proxmox::api::{api, Router, RpcEnvironment, Permission};
 
 use crate::api2::types::*;
 use crate::config::acl;
-use crate::config::acl::{Role, PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
+use crate::config::acl::{Role, PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
 
 #[api(
     properties: {
@@ -37,19 +37,6 @@ pub struct AclListItem {
     roleid: String,
 }
 
-fn check_acl_path(path: &str) -> Result<(), Error> {
-
-    let components = acl::split_acl_path(path);
-
-    if components.is_empty() { return Ok(()); }
-
-    if components.len() == 2 {
-        if components[0] == "datastore" { return Ok(()); }
-    }
-
-    bail!("invalid acl path '{}'.", path);
-}
-
 fn extract_acl_node_data(
     node: &acl::AclTreeNode,
     path: &str,
@@ -92,7 +79,7 @@ fn extract_acl_node_data(
         }
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["access", "acl"], PRIV_SYS_AUDIT, false),
     },
 )]
 /// Read Access Control List (ACLs).
@@ -144,7 +131,7 @@ pub fn read_acl(
        },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+        permission: &Permission::Privilege(&["access", "acl"], PRIV_PERMISSIONS_MODIFY, false),
     },
 )]
 /// Update Access Control List (ACLs).
@@ -186,7 +173,7 @@ pub fn update_acl(
     }
 
     if !delete { // Note: we allow to delete entries with invalid path
-        check_acl_path(&path)?;
+        acl::check_acl_path(&path)?;
     }
 
     if let Some(userid) = userid {

src/api2/access/user.rs

@@ -56,7 +56,7 @@ pub const PBS_PASSWORD_SCHEMA: Schema = StringSchema::new("User Password.")
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
     },
 )]
 /// List all users
@@ -111,7 +111,7 @@ pub fn list_users(
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_PERMISSIONS_MODIFY, false),
+        permission: &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
     },
 )]
 /// Create new user.
@@ -154,7 +154,7 @@ pub fn create_user(userid: String, password: Option<String>, param: Value) -> Re
         type: user::User,
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
     },
 )]
 /// Read user configuration data.
@@ -208,7 +208,7 @@ pub fn read_user(userid: String) -> Result<Value, Error> {
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_PERMISSIONS_MODIFY, false),
+        permission: &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
     },
 )]
 /// Update user configuration.
@@ -290,7 +290,7 @@ pub fn update_user(
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_PERMISSIONS_MODIFY, false),
+        permission: &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
     },
 )]
 /// Remove a user from the configuration file.

src/api2/node/dns.rs

@@ -111,7 +111,7 @@ pub fn read_etc_resolv_conf() -> Result<Value, Error> {
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+        permission: &Permission::Privilege(&["system", "network", "dns"], PRIV_SYS_MODIFY, false),
     }
 )]
 /// Update DNS settings
@@ -206,7 +206,7 @@ pub fn update_dns(
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["system", "network", "dns"], PRIV_SYS_AUDIT, false),
     }
 )]
 /// Read DNS settings.

src/api2/node/journal.rs

@@ -55,7 +55,7 @@ use crate::config::acl::PRIV_SYS_AUDIT;
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["system", "log"], PRIV_SYS_AUDIT, false),
     },
 )]
 /// Read syslog entries.

src/api2/node/network.rs

@@ -24,7 +24,7 @@ use crate::api2::types::*;
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["system", "network", "interfaces"], PRIV_SYS_AUDIT, false),
     },
 )]
 /// List all datastores
@@ -69,7 +69,7 @@ pub fn list_network_devices(
         type: Interface,
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["system", "network", "interfaces", "{name}"], PRIV_SYS_AUDIT, false),
     },
 )]
 /// Read a network interface configuration.
@@ -188,7 +188,7 @@ pub enum DeletableProperty {
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+        permission: &Permission::Privilege(&["system", "network", "interfaces", "{name}"], PRIV_SYS_MODIFY, false),
     },
 )]
 /// Update network interface config.
@@ -306,7 +306,7 @@ pub fn update_interface(
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+        permission: &Permission::Privilege(&["system", "network", "interfaces", "{name}"], PRIV_SYS_MODIFY, false),
     },
 )]
 /// Remove network interface configuration.
@@ -339,7 +339,7 @@ pub fn delete_interface(name: String, digest: Option<String>) -> Result<(), Erro
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+        permission: &Permission::Privilege(&["system", "network", "interfaces"], PRIV_SYS_MODIFY, false),
     },
 )]
 /// Reload network configuration (requires ifupdown2).
@@ -363,7 +363,7 @@ pub fn reload_network_config() -> Result<(), Error> {
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+        permission: &Permission::Privilege(&["system", "network", "interfaces"], PRIV_SYS_MODIFY, false),
     },
 )]
 /// Revert network configuration (rm /etc/network/interfaces.new).

src/api2/node/services.rs

@@ -124,7 +124,7 @@ fn json_service_state(service: &str, status: Value) -> Value {
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["system", "services"], PRIV_SYS_AUDIT, false),
     },
 )]
 /// Service list.
@@ -161,7 +161,7 @@ fn list_services(
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["system", "services", "{service}"], PRIV_SYS_AUDIT, false),
     },
 )]
 /// Read service properties.
@@ -220,7 +220,7 @@ fn run_service_command(service: &str, cmd: &str) -> Result<Value, Error> {
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+        permission: &Permission::Privilege(&["system", "services", "{service}"], PRIV_SYS_MODIFY, false),
     },
 )]
 /// Start service.
@@ -247,7 +247,7 @@ fn start_service(
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+        permission: &Permission::Privilege(&["system", "services", "{service}"], PRIV_SYS_MODIFY, false),
     },
 )]
 /// Stop service.
@@ -274,7 +274,7 @@ fn stop_service(
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+        permission: &Permission::Privilege(&["system", "services", "{service}"], PRIV_SYS_MODIFY, false),
     },
 )]
 /// Retart service.
@@ -306,7 +306,7 @@ fn restart_service(
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+        permission: &Permission::Privilege(&["system", "services", "{service}"], PRIV_SYS_MODIFY, false),
     },
 )]
 /// Reload service.

src/api2/node/syslog.rs

@@ -124,7 +124,7 @@ fn dump_journal(
         },
     },
     access: {
-        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["system", "log"], PRIV_SYS_AUDIT, false),
     },
 )]
 /// Read syslog entries.

src/api2/node/time.rs

@@ -7,6 +7,7 @@ use serde_json::{json, Value};
 use proxmox::api::{api, Router, Permission};
 use proxmox::tools::fs::{file_read_firstline, replace_file, CreateOptions};
 
+use crate::config::acl::PRIV_SYS_MODIFY;
 use crate::api2::types::*;
 
 fn read_etc_localtime() -> Result<String, Error> {
@@ -96,6 +97,9 @@ fn get_time(_param: Value) -> Result<Value, Error> {
             },
         },
     },
+    access: {
+        permission: &Permission::Privilege(&["system", "time"], PRIV_SYS_MODIFY, false),
+    },
 )]
 /// Set time zone
 fn set_timezone(

src/config/acl.rs

@@ -157,6 +157,59 @@ pub fn split_acl_path(path: &str) -> Vec<&str> {
     components
 }
 
+pub fn check_acl_path(path: &str) -> Result<(), Error> {
+
+    let components = split_acl_path(path);
+
+    let components_len = components.len();
+
+    if components_len == 0 { return Ok(()); }
+    match components[0] {
+        "access" => {
+            if components_len == 1 { return Ok(()); }
+            match components[1] {
+                "acl" | "users" => {
+                    if components_len == 2 { return Ok(()); }
+                }
+                _ => {},
+            }
+        }
+        "datastore" => {  // /datastore/{store}
+            if components_len <= 2 { return Ok(()); }
+        }
+        "remote" => { // /remote/{remote}/{store}
+            if components_len <= 3 { return Ok(()); }
+        }
+        "system" => {
+            if components_len == 1 { return Ok(()); }
+            match components[1] {
+                "log" | "status" | "tasks" | "time" => {
+                    if components_len == 2 { return Ok(()); }
+                }
+                "services" => { // /system/services/{service}
+                    if components_len <= 3 { return Ok(()); }
+                }
+                "network" => {
+                    if components_len == 2 { return Ok(()); }
+                    match components[2] {
+                        "dns" => {
+                            if components_len == 3 { return Ok(()); }
+                        }
+                        "interfaces" => { // /system/network/interfaces/{iface}
+                            if components_len <= 4 { return Ok(()); }
+                        }
+                        _ => {}
+                    }
+                }
+                _ => {}
+            }
+        }
+        _ => {}
+    }
+
+    bail!("invalid acl path '{}'.", path);
+}
+
 pub struct AclTree {
     pub root: AclTreeNode,
 }

tests/verify-api.rs

@@ -43,6 +43,25 @@ fn verify_schema(schema: &Schema) -> Result<(), Error> {
     }
     Ok(())
 }
+
+fn verify_access_permissions(permission: &Permission) -> Result<(), Error> {
+
+    match permission {
+        Permission::Or(list) => {
+            for perm in list.iter() { verify_access_permissions(perm)?; }
+        }
+        Permission::And(list) => {
+            for perm in list.iter() { verify_access_permissions(perm)?; }
+        }
+        Permission::Privilege(path_comp, ..)=> {
+            let path = format!("/{}", path_comp.join("/"));
+            proxmox_backup::config::acl::check_acl_path(&path)?;
+        }
+        _ => {}
+    }
+    Ok(())
+}
+
 fn verify_api_method(
     method: &str,
     path: &str,
@@ -55,6 +74,9 @@ fn verify_api_method(
     verify_schema(info.returns)
         .map_err(|err| format_err!("{} {} returns: {}", method, path, err))?;
 
+    verify_access_permissions(info.access.permission)
+        .map_err(|err| format_err!("{} {} access: {}", method, path, err))?;
+
     Ok(())
 }