tyler
/
proxmox-backup
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

api: namespace: check privs directly 

instead of doing a manual lookup and check - this changes the returned
error slightly since check_privs will include the checked ACL path, but
that is okay here, checks are before we even lookup the namespace/store,
so no chance to leak anything.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
This commit is contained in:
Fabian Grünbichler 2022-05-24 11:02:51 +02:00 committed by Thomas Lamprecht
parent 7d0dbaa013
commit c8dc51e41f
1 changed files with 13 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/api2/admin/namespace.rs
View File

@ -2,7 +2,7 @@ use anyhow::{bail, Error};
use serde_json::Value; use serde_json::Value;
use pbs_config::CachedUserInfo; use pbs_config::CachedUserInfo;
use proxmox_router::{http_bail, ApiMethod, Permission, Router, RpcEnvironment}; use proxmox_router::{http_err, ApiMethod, Permission, Router, RpcEnvironment};
use proxmox_schema::*; use proxmox_schema::*;
use pbs_api_types::{ use pbs_api_types::{
@ -14,10 +14,16 @@ use pbs_api_types::{
use pbs_datastore::DataStore; use pbs_datastore::DataStore;
// TODO: move somewhere we can reuse it from (datastore has its own copy atm.) // TODO: move somewhere we can reuse it from (datastore has its own copy atm.)
fn get_ns_privs(store_with_ns: &DatastoreWithNamespace, auth_id: &Authid) -> Result<u64, Error> { fn check_ns_privs(
store_with_ns: &DatastoreWithNamespace,
auth_id: &Authid,
privs: u64,
) -> Result<(), Error> {
let user_info = CachedUserInfo::new()?; let user_info = CachedUserInfo::new()?;
Ok(user_info.lookup_privs(auth_id, &store_with_ns.acl_path())) user_info
.check_privs(auth_id, &store_with_ns.acl_path(), privs, true)
.map_err(|err| http_err!(FORBIDDEN, "{err}"))
} }
#[api( #[api(
@ -61,9 +67,7 @@ pub fn create_namespace(
ns: parent.clone(), ns: parent.clone(),
}; };
if get_ns_privs(&store_with_parent, &auth_id)? & PRIV_DATASTORE_MODIFY == 0 { check_ns_privs(&store_with_parent, &auth_id, PRIV_DATASTORE_MODIFY)?;
proxmox_router::http_bail!(FORBIDDEN, "permission check failed");
}
let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?; let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
@ -111,9 +115,8 @@ pub fn list_namespaces(
ns: parent.clone(), ns: parent.clone(),
}; };
if get_ns_privs(&store_with_parent, &auth_id)? & PRIVS_OK == 0 { check_ns_privs(&store_with_parent, &auth_id, PRIVS_OK)?;
proxmox_router::http_bail!(FORBIDDEN, "permission check failed");
}
let user_info = CachedUserInfo::new()?; let user_info = CachedUserInfo::new()?;
let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?; let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
@ -172,9 +175,7 @@ pub fn delete_namespace(
store: store.clone(), store: store.clone(),
ns: parent.clone(), ns: parent.clone(),
}; };
if get_ns_privs(&store_with_parent, &auth_id)? & PRIV_DATASTORE_MODIFY == 0 { check_ns_privs(&store_with_parent, &auth_id, PRIV_DATASTORE_MODIFY)?;
http_bail!(FORBIDDEN, "permission check failed");
}
let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?; let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;