[pbs-devel] [RFC PATCH proxmox-backup] server/rest: disallow non-protected api calls in privileged environment
Thomas Lamprecht
t.lamprecht at proxmox.com
Wed Mar 3 08:07:17 CET 2021
On 02.03.21 18:02, Thomas Lamprecht wrote:
> On 02.03.21 16:31, Dominik Csapak wrote:
>> to prevent potential abuse of non-protected api calls as root
>>
>
> this breaks important CLI tools using client::connect_to_localhost
> i.e., proxmox-backup-manager and proxmox-tape and maybe others which
> connect still manually.
>
Ok, this is not true, I had in mind that we directly connect to :82, like
we did for pvesh way in the past.
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>> this is a rather theoretical security improvement, i am not sure if we
>> want this? it would only guard against an unprotected api call that somehow
>
> no, such stuff only tends to break things while not providing any value...
> lets keep theoretical security improvements also theoretical..
>
>> allows code execution. this could then be abused to connect to the
>> daemon and reabuse the same api call, but with root permissions
>
> with magically generating a ticket and circumventing permission checks
> how exactly?
>
Security wise I find this still nonsense, its way too constructed with no
single practical possible example state, and it effectively requires to have
a free-choose binary path or control of $PATH from the environment of that
process (if that is given you have other problems) plus local access to the
machine and a entry in PBS user config would be required.
But, one thing this could help with is the issue that we sometimes had that
doing creating a config file as privileged user got us the wrong permissions,
making it inaccessible for the unprivileged code, which was a bug but not
always immediately found, we have all cases covered with chown+checks, IIRC,
but if a new config came in this could help detection (albeit such things
are quite visible, normally)
>>
>> also if we want this, maybe this would be good to have in pve too?
>
> no
>
>
>>
>> src/server/rest.rs | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/src/server/rest.rs b/src/server/rest.rs
>> index 9bf494fd..6b170b7f 100644
>> --- a/src/server/rest.rs
>> +++ b/src/server/rest.rs
>> @@ -750,6 +750,9 @@ async fn handle_request(
>>
>> let result = if api_method.protected && env_type == RpcEnvironmentType::PUBLIC {
>> proxy_protected_request(api_method, parts, body, peer).await
>> + } else if !api_method.protected && env_type == RpcEnvironmentType::PRIVILEGED {
>> + let err = http_err!(FORBIDDEN, "invalid server request");
>> + return Ok((formatter.format_error)(err));
>> } else {
>> handle_api_request(rpcenv, api_method, formatter, parts, body, uri_param).await
>> };
>>
More information about the pbs-devel
mailing list