Mikhail Novosyolov <m.novosyolov@xxxxxxxxxxxx> writes:
> /etc/selinux/config.bck was created and then replaced /etc/selinux/config.
> /etc/selinux/config is often read by libselinux from non-root,
> it must have mode 0644, but, when umask is 077, it became not world-readable
> after running system-config-gui.
>
> Overwrite the existing file instead of creating a new one.
>
> Unfortunately, we may get a corrupted file if the GUI is closed when writing it,
> but writing takes only a bit of time, plus we save a backup for manual restoring in such case.
>
I don't think it's a good idea. The final operation needs to be atomic.
If you want to preserve mode and other attributes you can use
shutil.copystat(), what about this:
fd = open(backup_path, "w")
...
fd.close()
shutil.copystat(path, backup_path)
> At Github: https://github.com/SELinuxProject/selinux/pull/345
>
> Signed-off-by: Mikhail Novosyolov <m.novosyolov@xxxxxxxxxxxx>
> ---
> gui/statusPage.py | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/gui/statusPage.py b/gui/statusPage.py
> index 766854b1..ded3929d 100644
> --- a/gui/statusPage.py
> +++ b/gui/statusPage.py
> @@ -18,6 +18,7 @@
> ## Author: Dan Walsh
> import os
> import sys
> +import tempfile
> from gi.repository import Gtk
> import selinux
>
> @@ -162,12 +163,20 @@ class statusPage:
> self.enabled = enabled
>
> def write_selinux_config(self, enforcing, type):
> - path = selinux.selinux_path() + "config"
> - backup_path = path + ".bck"
> - fd = open(path)
> - lines = fd.readlines()
> - fd.close()
> - fd = open(backup_path, "w")
> + selinux_path = selinux.selinux_path()
> + path = selinux_path + "config"
> + # Make a backup /etc/selinux/config.*.bck
> + backup_path = tempfile.mkstemp(prefix="config.", dir=selinux_path, suffix=".bck")[1]
> + fd1 = open(path, "r")
> + lines = fd1.readlines()
> + fd1.close()
> + fd2 = open(backup_path, "a")
> + for l in lines:
> + fd2.write(l)
> + fd2.close()
> + # Write to path, not backup_path, to guarantee that file metadata
> + # (permissions, xattrs, including SELinux labels etc.) is not lost.
> + fd = open(path, "w")
> for l in lines:
> if l.startswith("SELINUX="):
> fd.write("SELINUX=%s\n" % enforcing)
> @@ -177,7 +186,9 @@ class statusPage:
> continue
> fd.write(l)
> fd.close()
> - os.rename(backup_path, path)
> + # Here we are sure that we are deleting our backup,
> + # not another file or directory
> + os.unlink(backup_path)
>
> def read_selinux_config(self):
> self.initialtype = selinux.selinux_getpolicytype()[1]
> --
> 2.31.1
