From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 19FBA69848
 for <pbs-devel@lists.proxmox.com>; Wed,  3 Mar 2021 08:07:19 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 0F92C32F18
 for <pbs-devel@lists.proxmox.com>; Wed,  3 Mar 2021 08:07:19 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [212.186.127.180])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 5B4B532F0A
 for <pbs-devel@lists.proxmox.com>; Wed,  3 Mar 2021 08:07:18 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 26A3641D72
 for <pbs-devel@lists.proxmox.com>; Wed,  3 Mar 2021 08:07:18 +0100 (CET)
Message-ID: <a282756e-9b05-3503-3756-32e41ef8653f@proxmox.com>
Date: Wed, 3 Mar 2021 08:07:17 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:87.0) Gecko/20100101
 Thunderbird/87.0
Content-Language: en-US
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>, Dominik Csapak <d.csapak@proxmox.com>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
References: <20210302153120.31213-1-d.csapak@proxmox.com>
 <c0c95fb1-cb93-b59e-6ca6-95081966cd85@proxmox.com>
In-Reply-To: <c0c95fb1-cb93-b59e-6ca6-95081966cd85@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.053 Adjusted score from AWL reputation of From: address
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 NICE_REPLY_A           -0.001 Looks like a legit reply (A)
 RCVD_IN_DNSWL_MED        -2.3 Sender listed at https://www.dnswl.org/,
 medium trust
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [rest.rs]
Subject: Re: [pbs-devel] [RFC PATCH proxmox-backup] server/rest: disallow
 non-protected api calls in privileged environment
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 03 Mar 2021 07:07:19 -0000

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@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
>>                      };
>>