canonical / cloud-init

Official upstream for the cloud-init: cloud instance initialization
https://cloud-init.io/
Other
3.01k stars 887 forks source link

do not need to set recursive=True when restoring the selinux mode of the / directory #5807

Closed xiaoge1001 closed 2 weeks ago

xiaoge1001 commented 1 month ago

Bug report

do not need to set recursive=True when restoring the selinux mode of the / directory. This may modify the SELinux security context of some files that are not displayed in the result of "semanage fcontext -l". This may affect other applications.

Steps to reproduce the problem

The mount point is set to /mnt1 and the /mnt1 directory does not exist.

from cloudinit import util
# the /mnt1 directory does not exist
util.ensure_dir("/mnt1")

Environment details

cloud-init logs

2024-10-08 07:31:41,157 - cc_mounts.py[DEBUG]: mounts configuration is [['my_alias.1', '/mnt1'], ['my_alias.2', '/mnt2']]
... ...
... ...
2024-10-08 07:31:41,161 - util.py[DEBUG]: Restoring selinux mode for / (recursive=True)
... ...
... ...
xiaoge1001 commented 1 month ago

In commit https://github.com/canonical/cloud-init/commit/ba5fb03646f6318a0ace286da746b4bb32f75d5a, recursive=True is set when SeLinuxGuard is invoked in the ensure_dir method. Why do we set recursive=True?

TheRealFalcon commented 1 month ago

@xiaoge1001 , thanks for the bug.

Why do we set recursive=True?

That code is old enough that I don't think any of the current developers have an answer. We're open to PRs to improve the functionality.

xiaoge1001 commented 1 month ago

@xiaoge1001 , thanks for the bug.

Why do we set recursive=True?

That code is old enough that I don't think any of the current developers have an answer. We're open to PRs to improve the functionality.

Can we remove recursive=True from the ensure_dir function? I don't think it's necessary to set recursive=True.