From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 180751FF163 for ; Thu, 21 Nov 2024 17:09:46 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B72EF1AD7; Thu, 21 Nov 2024 17:09:52 +0100 (CET) Mime-Version: 1.0 Date: Thu, 21 Nov 2024 17:09:19 +0100 Message-Id: To: "Thomas Lamprecht" , "Proxmox VE development discussion" From: "Max Carrara" X-Mailer: aerc 0.18.2-0-ge037c095a049 References: <20240802132656.270077-1-m.carrara@proxmox.com> <20240802132656.270077-10-m.carrara@proxmox.com> In-Reply-To: X-SPAM-LEVEL: Spam detection results: 0 AWL 0.033 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH v1 pve-common 09/18] pbsclient: create secret dir with `mkdir -p` and mode `700` X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On Mon Nov 11, 2024 at 8:16 PM CET, Thomas Lamprecht wrote: > Am 02.08.24 um 15:26 schrieb Max Carrara: > > .. instead of using a regular `mkdir` call. > > > > The `File::Path::make_path` subroutine is used for this purpose, which > > recursively creates all directories if they didn't exist before. Upon > > creation of those directories, the mode is also set to `700`. > > > > This means that (like before), directory permissions are left > > untouched if the directory existed already. > > this is already enforced by pmxcfs for our case though? Even though pmxcfs enforces this, the library may still be used in a context outside of pmxcfs. For example, I could write a simple script with `use PVE::PBSClient;` for my own backup management purposes and use a separate directory to store secrets. This is obviously theoretical, *but* because the possibility exists that *someone* out there might use the library that way, I decided to be a bit more defensive here. To give a more elaborate example, let's say I have a regular user account on a PVE host and I use this library to do ... some stuff regarding backups of my VMs the admin allows me to run on that host. Because I don't have permissions to access `/etc/pve/priv/storage` (I don't have rights to `root`) I set the secret dir to something like "$HOME/.config/pve/priv/storage" or whatever. Because the admin forgot to configure `/etc/skel`, the permissions of my home directory are 0755 -- and because I forgot to change them myself, my home directory is accessible to other users. If we didn't set the secret dir to 0700 here, other users could access my credentials. > > And not sure about making the whole base path 0700, might be better > to create only the last directory as such? but this is generally something > where IMO lots can go wrong with little benefit, so not so sure about > this one. Setting just the last dir 0700 seems better actually, I agree. I'm still in favour to be a little restrictive here. Perhaps I'm a little overly cautious; I'm not sure if anyone would ever find themselves in the scenario I described above. Usually I don't want to implement things upfront because they're merely "possible" or because there's some magical, potential case it will be used either (YAGNI etc.) but in this case it does have obvious implications regarding security IMO. As an alternative, we could also just not allow specifying a custom secret dir and hardcode it to `/etc/pve/storage/priv`, then all of the above isn't possible in the first place and doesn't need to be dealt with. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel