[pve-devel] applied: [RFC container] mountpoints: create parent dirs with correct owner

[Fabian Grünbichler]

On August 26, 2019 1:30 pm, Thomas Lamprecht wrote:
> On 24.07.19 13:37, Fabian Grünbichler wrote:
>> otherwise unprivileged containers might end up with directories that
>> they cannot modify since they are owned by the user root in the host
>> namespace, instead of root inside the container.
>> 
>> note: the problematic behaviour is only exhibited when an intermediate
>> directory needs to be created, e.g. a mountpoint /test/mp gets mounted,
>> and /test does not yet exist.
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
>> ---
>> Notes:
>>     requires fchownat support in PVE::Tools - see other patch and bump
>>     build-depends + depends accordingly after applying!
>>     
>>     I am not sure whether this is 100% correct w.r.t. error edge cases, since we
>>     potentially die after mkdirat without calling fchownat. it is for sure better
>>     than the status quo though ;)
>>     
>>     thank you Dietmar for noticing the buggy behaviour!
>> 
>>  src/PVE/LXC.pm            | 27 ++++++++++++++++-----------
>>  src/PVE/VZDump/LXC.pm     |  7 +++++--
>>  src/lxc-pve-prestart-hook |  4 +++-
>>  3 files changed, 24 insertions(+), 14 deletions(-)
>> 
> 
> 
> applied, thanks!
> 
> For existing CTs, where problematic directories outside the CT GID/UID range got
> already created, we could add a "repair-ids" command, which would reset UIDs/GIDs
> from (specified) paths to a CT (root) ID again, if they where outside the CTs
> range, e.g.:
> 
> # pct repair-file-permissions <vmid> [<path>] [--repair-with-id <id>]
> 
> Which could roughly follow those rules:
> * only "repair" files with IDs outside of the CTs file range, e.g., "0" from PVE
>   host POV
> * only check at given path, if set
> * maybe add a "no-recursive" flag to really only do a single file?
> 
> With that at least a possibility would be there to allow people fixing the
> directory tree permissions so that CT-root can change/delete/... them again.
> Just as an idea.
> 

given that there were no complaints so far and we found this internally, 
I think we don't need to provide such a command. a simple pct mount + 
chown with mapped numerical uid/gid should work if anyone ever asks for 
it..