浏览代码

use a custom mkstemp with mode support, fixes #6933, fixes #6400

hopefully this is the final fix.

after first fixing of #6400 (by using os.umask after mkstemp), there
was a new problem that chmod was not supported on some fs.

even after fixing that, there were other issues, see the ACLs issue
documented in #6933.

the root cause of all this is tempfile.mkstemp internally using a
very secure, but hardcoded and for our use case problematic mode
of 0o600.

mkstemp_mode (mosty copy&paste from python stdlib tempfile module +
"black" formatting applied) supports giving the mode via the api,
that is the only change needed.

slightly dirty due to the _xxx imports from tempfile, but hopefully
this will be supported in some future python version.
Thomas Waldmann 2 年之前
父节点
当前提交
52c47bd546
共有 2 个文件被更改,包括 72 次插入16 次删除
  1. 70 0
      src/borg/helpers/fs.py
  2. 2 16
      src/borg/platform/base.py

+ 70 - 0
src/borg/helpers/fs.py

@@ -418,3 +418,73 @@ def umount(mountpoint):
         return subprocess.call(["fusermount", "-u", mountpoint], env=env)
     except FileNotFoundError:
         return subprocess.call(["umount", mountpoint], env=env)
+
+
+# below is a slightly modified tempfile.mkstemp that has an additional mode parameter.
+# see https://github.com/borgbackup/borg/issues/6933 and https://github.com/borgbackup/borg/issues/6400
+
+import os as _os  # NOQA
+import sys as _sys  # NOQA
+import errno as _errno  # NOQA
+from tempfile import _sanitize_params, _get_candidate_names  # type: ignore[attr-defined] # NOQA
+from tempfile import TMP_MAX, _text_openflags, _bin_openflags  # type: ignore[attr-defined] # NOQA
+
+
+def _mkstemp_inner(dir, pre, suf, flags, output_type, mode=0o600):
+    """Code common to mkstemp, TemporaryFile, and NamedTemporaryFile."""
+
+    dir = _os.path.abspath(dir)
+    names = _get_candidate_names()
+    if output_type is bytes:
+        names = map(_os.fsencode, names)
+
+    for seq in range(TMP_MAX):
+        name = next(names)
+        file = _os.path.join(dir, pre + name + suf)
+        _sys.audit("tempfile.mkstemp", file)
+        try:
+            fd = _os.open(file, flags, mode)
+        except FileExistsError:
+            continue  # try again
+        except PermissionError:
+            # This exception is thrown when a directory with the chosen name
+            # already exists on windows.
+            if _os.name == "nt" and _os.path.isdir(dir) and _os.access(dir, _os.W_OK):
+                continue
+            else:
+                raise
+        return fd, file
+
+    raise FileExistsError(_errno.EEXIST, "No usable temporary file name found")
+
+
+def mkstemp_mode(suffix=None, prefix=None, dir=None, text=False, mode=0o600):
+    """User-callable function to create and return a unique temporary
+    file.  The return value is a pair (fd, name) where fd is the
+    file descriptor returned by os.open, and name is the filename.
+    If 'suffix' is not None, the file name will end with that suffix,
+    otherwise there will be no suffix.
+    If 'prefix' is not None, the file name will begin with that prefix,
+    otherwise a default prefix is used.
+    If 'dir' is not None, the file will be created in that directory,
+    otherwise a default directory is used.
+    If 'text' is specified and true, the file is opened in text
+    mode.  Else (the default) the file is opened in binary mode.
+    If any of 'suffix', 'prefix' and 'dir' are not None, they must be the
+    same type.  If they are bytes, the returned name will be bytes; str
+    otherwise.
+    The file is readable and writable only by the creating user ID.
+    If the operating system uses permission bits to indicate whether a
+    file is executable, the file is executable by no one. The file
+    descriptor is not inherited by children of this process.
+    Caller is responsible for deleting the file when done with it.
+    """
+
+    prefix, suffix, dir, output_type = _sanitize_params(prefix, suffix, dir)
+
+    if text:
+        flags = _text_openflags
+    else:
+        flags = _bin_openflags
+
+    return _mkstemp_inner(dir, prefix, suffix, flags, output_type, mode)

+ 2 - 16
src/borg/platform/base.py

@@ -1,10 +1,8 @@
 import errno
 import os
 import socket
-import tempfile
 import uuid
 
-from ..constants import UMASK_DEFAULT
 from ..helpers import safe_unlink
 from ..platformflags import is_win32
 
@@ -232,8 +230,9 @@ class SaveFile:
 
     def __enter__(self):
         from .. import platform
+        from ..helpers.fs import mkstemp_mode
 
-        self.tmp_fd, self.tmp_fname = tempfile.mkstemp(prefix=self.tmp_prefix, suffix=".tmp", dir=self.dir)
+        self.tmp_fd, self.tmp_fname = mkstemp_mode(prefix=self.tmp_prefix, suffix=".tmp", dir=self.dir, mode=0o666)
         self.f = platform.SyncFile(self.tmp_fname, fd=self.tmp_fd, binary=self.binary)
         return self.f
 
@@ -246,19 +245,6 @@ class SaveFile:
             safe_unlink(self.tmp_fname)  # with-body has failed, clean up tmp file
             return  # continue processing the exception normally
 
-        try:
-            # tempfile.mkstemp always uses owner-only file permissions for the temp file,
-            # but as we'll rename it to the non-temp permanent file now, we need to respect
-            # the umask and change the file mode to what a normally created file would have.
-            # thanks to the crappy os.umask api, we can't query the umask without setting it. :-(
-            umask = os.umask(UMASK_DEFAULT)
-            os.umask(umask)
-            os.chmod(self.tmp_fname, mode=0o666 & ~umask)
-        except OSError:
-            # chmod might fail if the fs does not support it.
-            # this is not harmful, the file will still have permissions for the owner.
-            pass
-
         try:
             os.replace(self.tmp_fname, self.path)  # POSIX: atomic rename
         except OSError: