[pve-devel] [PATCH v1 pve-common 09/18] pbsclient: create secret dir with `mkdir -p` and mode `700`

[Max Carrara]

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.