From d4a83edfdd94e4fcff1eb4df87e426a509934fd0 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 24 Feb 2024 23:15:42 +0100 Subject: [PATCH 01/15] improve acl_get / acl_set error handling, see #4049 --- src/borg/platform/darwin.pyx | 12 +++++++++--- src/borg/platform/freebsd.pyx | 24 ++++++++++++++---------- src/borg/platform/linux.pyx | 13 ++++++++++--- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/borg/platform/darwin.pyx b/src/borg/platform/darwin.pyx index 3a861aac..bb7f8139 100644 --- a/src/borg/platform/darwin.pyx +++ b/src/borg/platform/darwin.pyx @@ -1,6 +1,7 @@ import os from libc.stdint cimport uint32_t +from libc cimport errno from .posix import user2uid, group2gid from ..helpers import safe_decode, safe_encode @@ -121,7 +122,10 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): else: acl = acl_get_link_np(path, ACL_TYPE_EXTENDED) if acl == NULL: - return + if errno.errno == errno.ENOENT: + # macOS weirdness: if a file has no ACLs, it sets errno to ENOENT. :-( + return + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) text = acl_to_text(acl, NULL) if text == NULL: return @@ -148,8 +152,10 @@ def acl_set(path, item, numeric_ids=False, fd=None): if isinstance(path, str): path = os.fsencode(path) if fd is not None: - acl_set_fd_np(fd, acl, ACL_TYPE_EXTENDED) + if acl_set_fd_np(fd, acl, ACL_TYPE_EXTENDED) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) else: - acl_set_link_np(path, ACL_TYPE_EXTENDED, acl) + if acl_set_link_np(path, ACL_TYPE_EXTENDED, acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) finally: acl_free(acl) diff --git a/src/borg/platform/freebsd.pyx b/src/borg/platform/freebsd.pyx index 1c67de02..a2d431c1 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -4,11 +4,9 @@ from .posix import posix_acl_use_stored_uid_gid from ..helpers import safe_encode, safe_decode from .xattr import _listxattr_inner, _getxattr_inner, _setxattr_inner, split_lstring -API_VERSION = '1.4_01' +from libc cimport errno -cdef extern from "errno.h": - int errno - int EINVAL +API_VERSION = '1.4_01' cdef extern from "sys/extattr.h": ssize_t c_extattr_list_file "extattr_list_file" (const char *path, int attrnamespace, void *data, size_t nbytes) @@ -136,6 +134,8 @@ cdef _get_acl(p, type, item, attribute, flags, fd=None): item[attribute] = text acl_free(text) acl_free(acl) + else: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) def acl_get(path, item, st, numeric_ids=False, fd=None): @@ -147,7 +147,7 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): if isinstance(path, str): path = os.fsencode(path) ret = lpathconf(path, _PC_ACL_NFS4) - if ret < 0 and errno == EINVAL: + if ret < 0 and errno.errno == errno.EINVAL: return flags |= ACL_TEXT_NUMERIC_IDS if numeric_ids else 0 if ret > 0: @@ -167,11 +167,15 @@ cdef _set_acl(p, type, item, attribute, numeric_ids=False, fd=None): text = posix_acl_use_stored_uid_gid(text) acl = acl_from_text(text) if acl: - if fd is not None: - acl_set_fd_np(fd, acl, type) - else: - acl_set_link_np(p, type, acl) - acl_free(acl) + try: + if fd is not None: + if acl_set_fd_np(fd, acl, type) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) + else: + if acl_set_link_np(p, type, acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) + finally: + acl_free(acl) cdef _nfs4_use_stored_uid_gid(acl): diff --git a/src/borg/platform/linux.pyx b/src/borg/platform/linux.pyx index 189e2955..955fdbad 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -251,9 +251,13 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): access_acl = acl_get_fd(fd) else: access_acl = acl_get_file(path, ACL_TYPE_ACCESS) + if access_acl == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) if stat.S_ISDIR(st.st_mode): # only directories can have a default ACL. there is no fd-based api to get it. default_acl = acl_get_file(path, ACL_TYPE_DEFAULT) + if default_acl == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) if access_acl: access_text = acl_to_text(access_acl, NULL) if access_text: @@ -289,9 +293,11 @@ def acl_set(path, item, numeric_ids=False, fd=None): access_acl = acl_from_text(converter(access_text)) if access_acl: if fd is not None: - acl_set_fd(fd, access_acl) + if acl_set_fd(fd, access_acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) else: - acl_set_file(path, ACL_TYPE_ACCESS, access_acl) + if acl_set_file(path, ACL_TYPE_ACCESS, access_acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) finally: acl_free(access_acl) default_text = item.get('acl_default') @@ -300,7 +306,8 @@ def acl_set(path, item, numeric_ids=False, fd=None): default_acl = acl_from_text(converter(default_text)) if default_acl: # only directories can get a default ACL. there is no fd-based api to set it. - acl_set_file(path, ACL_TYPE_DEFAULT, default_acl) + if acl_set_file(path, ACL_TYPE_DEFAULT, default_acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) finally: acl_free(default_acl) From 2c53a63a1cf45cef627825d0701da26a0ce4a101 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 26 Feb 2024 20:07:10 +0100 Subject: [PATCH 02/15] raise OSError if acl_to_text / acl_from_text returns NULL Also did a small structural refactors there. --- src/borg/platform/darwin.pyx | 8 ++++---- src/borg/platform/freebsd.pyx | 37 ++++++++++++++++++----------------- src/borg/platform/linux.pyx | 34 ++++++++++++++++++-------------- 3 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/borg/platform/darwin.pyx b/src/borg/platform/darwin.pyx index bb7f8139..e43e2674 100644 --- a/src/borg/platform/darwin.pyx +++ b/src/borg/platform/darwin.pyx @@ -128,7 +128,7 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) text = acl_to_text(acl, NULL) if text == NULL: - return + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) if numeric_ids: item['acl_extended'] = _remove_non_numeric_identifier(text) else: @@ -143,14 +143,14 @@ def acl_set(path, item, numeric_ids=False, fd=None): acl_text = item.get('acl_extended') if acl_text is not None: try: + if isinstance(path, str): + path = os.fsencode(path) if numeric_ids: acl = acl_from_text(acl_text) else: acl = acl_from_text(_remove_numeric_id_if_possible(acl_text)) if acl == NULL: - return - if isinstance(path, str): - path = os.fsencode(path) + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) if fd is not None: if acl_set_fd_np(fd, acl, ACL_TYPE_EXTENDED) == -1: raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) diff --git a/src/borg/platform/freebsd.pyx b/src/borg/platform/freebsd.pyx index a2d431c1..8ea30823 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -128,15 +128,15 @@ cdef _get_acl(p, type, item, attribute, flags, fd=None): acl = acl_get_fd_np(fd, type) else: acl = acl_get_link_np(p, type) - if acl: - text = acl_to_text_np(acl, NULL, flags) - if text: - item[attribute] = text - acl_free(text) - acl_free(acl) - else: + if acl == NULL: raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) - + text = acl_to_text_np(acl, NULL, flags) + if text == NULL: + acl_free(acl) + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) + item[attribute] = text + acl_free(text) + acl_free(acl) def acl_get(path, item, st, numeric_ids=False, fd=None): """Saves ACL Entries @@ -166,16 +166,17 @@ cdef _set_acl(p, type, item, attribute, numeric_ids=False, fd=None): elif numeric_ids and type in(ACL_TYPE_ACCESS, ACL_TYPE_DEFAULT): text = posix_acl_use_stored_uid_gid(text) acl = acl_from_text(text) - if acl: - try: - if fd is not None: - if acl_set_fd_np(fd, acl, type) == -1: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) - else: - if acl_set_link_np(p, type, acl) == -1: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) - finally: - acl_free(acl) + if acl == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) + try: + if fd is not None: + if acl_set_fd_np(fd, acl, type) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) + else: + if acl_set_link_np(p, type, acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) + finally: + acl_free(acl) cdef _nfs4_use_stored_uid_gid(acl): diff --git a/src/borg/platform/linux.pyx b/src/borg/platform/linux.pyx index 955fdbad..cb724482 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -260,12 +260,14 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) if access_acl: access_text = acl_to_text(access_acl, NULL) - if access_text: - item['acl_access'] = converter(access_text) + if access_text == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + item['acl_access'] = converter(access_text) if default_acl: default_text = acl_to_text(default_acl, NULL) - if default_text: - item['acl_default'] = converter(default_text) + if default_text == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + item['acl_default'] = converter(default_text) finally: acl_free(default_text) acl_free(default_acl) @@ -291,23 +293,25 @@ def acl_set(path, item, numeric_ids=False, fd=None): if access_text: try: access_acl = acl_from_text(converter(access_text)) - if access_acl: - if fd is not None: - if acl_set_fd(fd, access_acl) == -1: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - else: - if acl_set_file(path, ACL_TYPE_ACCESS, access_acl) == -1: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + if access_acl == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + if fd is not None: + if acl_set_fd(fd, access_acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + else: + if acl_set_file(path, ACL_TYPE_ACCESS, access_acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) finally: acl_free(access_acl) default_text = item.get('acl_default') if default_text: try: default_acl = acl_from_text(converter(default_text)) - if default_acl: - # only directories can get a default ACL. there is no fd-based api to set it. - if acl_set_file(path, ACL_TYPE_DEFAULT, default_acl) == -1: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + if default_acl == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + # only directories can get a default ACL. there is no fd-based api to set it. + if acl_set_file(path, ACL_TYPE_DEFAULT, default_acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) finally: acl_free(default_acl) From 926b5a6b0873765449187a034c9b6f2ddb00336d Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 25 Feb 2024 02:19:38 +0100 Subject: [PATCH 03/15] improve are_acls_working function - ACLs are not working, if ENOTSUP ("Operation not supported") happens - fix check for macOS On macOS borg uses "acl_extended", not "acl_access" and also the ACL text format is a bit different. --- src/borg/testsuite/platform.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/borg/testsuite/platform.py b/src/borg/testsuite/platform.py index 92fda6c0..92a07808 100644 --- a/src/borg/testsuite/platform.py +++ b/src/borg/testsuite/platform.py @@ -1,3 +1,4 @@ +import errno import functools import os import random @@ -58,16 +59,26 @@ def are_acls_working(): with unopened_tempfile() as filepath: open(filepath, 'w').close() try: - access = b'user::rw-\ngroup::r--\nmask::rw-\nother::---\nuser:root:rw-:9999\ngroup:root:rw-:9999\n' - acl = {'acl_access': access} - acl_set(filepath, acl) + if is_darwin: + acl_key = 'acl_extended' + acl_value = b'!#acl 1\nuser:FFFFEEEE-DDDD-CCCC-BBBB-AAAA00000000:root:0:allow:read\n' + else: + acl_key = 'acl_access' + acl_value = b'user::rw-\ngroup::r--\nmask::rw-\nother::---\nuser:root:rw-:9999\ngroup:root:rw-:9999\n' + write_acl = {acl_key: acl_value} + acl_set(filepath, write_acl) read_acl = {} acl_get(filepath, read_acl, os.stat(filepath)) - read_acl_access = read_acl.get('acl_access', None) - if read_acl_access and b'user::rw-' in read_acl_access: - return True + acl = read_acl.get(acl_key, None) + if acl is not None: + check_for = b'root:0:allow:read' if is_darwin else b'user::rw-' + if check_for in acl: + return True except PermissionError: pass + except OSError as e: + if e.errno not in (errno.ENOTSUP, ): + raise return False From d870c58e61df325437e69e5c6ba587bfba7cec3f Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 9 Mar 2024 17:53:12 +0100 Subject: [PATCH 04/15] platform tests: misc. minor cleanups - remove unused global / import - use is_linux and is_darwin - rename darwin acl test method --- src/borg/testsuite/platform.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/borg/testsuite/platform.py b/src/borg/testsuite/platform.py index 92a07808..c0485f36 100644 --- a/src/borg/testsuite/platform.py +++ b/src/borg/testsuite/platform.py @@ -1,7 +1,6 @@ import errno import functools import os -import random import shutil import sys import tempfile @@ -36,8 +35,6 @@ mask::rw- other::r-- """.strip().encode('ascii') -_acls_working = None - def fakeroot_detected(): return 'FAKEROOTKEY' in os.environ @@ -82,7 +79,7 @@ def are_acls_working(): return False -@unittest.skipUnless(sys.platform.startswith('linux'), 'linux only test') +@unittest.skipUnless(is_linux, 'linux only test') @unittest.skipIf(fakeroot_detected(), 'not compatible with fakeroot') class PlatformLinuxTestCase(BaseTestCase): @@ -160,7 +157,7 @@ class PlatformLinuxTestCase(BaseTestCase): self.assert_equal(acl_use_local_uid_gid(b'group:root:rw-:0'), b'group:0:rw-') -@unittest.skipUnless(sys.platform.startswith('darwin'), 'macOS only test') +@unittest.skipUnless(is_darwin, 'macOS only test') @unittest.skipIf(fakeroot_detected(), 'not compatible with fakeroot') class PlatformDarwinTestCase(BaseTestCase): @@ -180,7 +177,7 @@ class PlatformDarwinTestCase(BaseTestCase): acl_set(path, item, numeric_ids=numeric_ids) @unittest.skipIf(not are_acls_working(), 'ACLs do not work') - def test_access_acl(self): + def test_extended_acl(self): file = tempfile.NamedTemporaryFile() file2 = tempfile.NamedTemporaryFile() self.assert_equal(self.get_acl(file.name), {}) From 33f1ba699e2db9fbae50c31270821d2b89f094f0 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 25 Feb 2024 03:06:32 +0100 Subject: [PATCH 05/15] create/extract: ignore OSError if ACLs are not supported (ENOTSUP) but do not silence other OSErrors. --- src/borg/archive.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index 0407ea1f..8ae002e3 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -902,7 +902,11 @@ Utilization of max. archive size: {csize_max:.0%} if not symlink: os.chmod(path, item.mode) if not self.noacls: - acl_set(path, item, self.numeric_ids, fd=fd) + try: + acl_set(path, item, self.numeric_ids, fd=fd) + except OSError as e: + if e.errno not in (errno.ENOTSUP, ): + raise if not self.noxattrs: # chown removes Linux capabilities, so set the extended attributes at the end, after chown, since they include # the Linux capabilities in the "security.capability" attribute. @@ -1165,7 +1169,11 @@ class MetadataCollector: attrs['xattrs'] = StableDict(xattrs) if not self.noacls: with backup_io('extended stat (ACLs)'): - acl_get(path, attrs, st, self.numeric_ids, fd=fd) + try: + acl_get(path, attrs, st, self.numeric_ids, fd=fd) + except OSError as e: + if e.errno not in (errno.ENOTSUP, ): + raise return attrs def stat_attrs(self, st, path, fd=None): From 519e3ebce82ef3223b607f9cf7489377ae1c3f24 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 2 Mar 2024 19:59:04 +0100 Subject: [PATCH 06/15] Linux: acl_get: raise OSError for errors in acl_extended_* call Previously, these conditions were handled the same (just return): - no extended acl here - some error happened (e.g. ACLs unsupported, bad file descriptor, file not found, permission error, ...) Now there will be OSErrors for the error cases. --- src/borg/platform/linux.pyx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/borg/platform/linux.pyx b/src/borg/platform/linux.pyx index cb724482..dbb07fd8 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -232,16 +232,22 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): cdef acl_t access_acl = NULL cdef char *default_text = NULL cdef char *access_text = NULL + cdef int ret = 0 if stat.S_ISLNK(st.st_mode): # symlinks can not have ACLs return if isinstance(path, str): path = os.fsencode(path) - if (fd is not None and acl_extended_fd(fd) <= 0 - or - fd is None and acl_extended_file(path) <= 0): + if fd is not None: + ret = acl_extended_fd(fd) + else: + ret = acl_extended_file(path) + if ret == 0: + # there is no ACL defining permissions other than those defined by the traditional file permission bits. return + if ret < 0: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) if numeric_ids: converter = acl_numeric_ids else: From 17e3cee604970971f2a7aaaea4674a83d281efbb Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 2 Mar 2024 20:04:22 +0100 Subject: [PATCH 07/15] Linux: acl_get: use "nofollow" variant of acl_extended_file call This is NOT a bug fix, because the previous code contained a check for symlinks before that line - because symlinks can not have ACLs under Linux. Now, this "is it a symlink" check is removed to simplify the code and the "nofollow" variant of acl_extended_file* is used to look at the symlink fs object (in the symlink case). It then should tell us that this does NOT have an extended ACL (because symlinks can't have ACLs) and so we return there. Overall the code gets simpler and looks less suspect. --- src/borg/platform/linux.pyx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/borg/platform/linux.pyx b/src/borg/platform/linux.pyx index dbb07fd8..196d8bd8 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -52,7 +52,7 @@ cdef extern from "sys/acl.h": char *acl_to_text(acl_t acl, ssize_t *len) cdef extern from "acl/libacl.h": - int acl_extended_file(const char *path) + int acl_extended_file_nofollow(const char *path) int acl_extended_fd(int fd) cdef extern from "linux/fs.h": @@ -234,17 +234,15 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): cdef char *access_text = NULL cdef int ret = 0 - if stat.S_ISLNK(st.st_mode): - # symlinks can not have ACLs - return if isinstance(path, str): path = os.fsencode(path) if fd is not None: ret = acl_extended_fd(fd) else: - ret = acl_extended_file(path) + ret = acl_extended_file_nofollow(path) if ret == 0: # there is no ACL defining permissions other than those defined by the traditional file permission bits. + # note: this should also be the case for symlink fs objects, as they can not have ACLs. return if ret < 0: raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) From 6ad1ad67d55b92feb17783853bd419bceb16b37a Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 2 Mar 2024 20:08:11 +0100 Subject: [PATCH 08/15] Linux: acl_set bug fix: always fsencode path We use path when raising OSErrors, even if we have an fd. --- src/borg/platform/linux.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borg/platform/linux.pyx b/src/borg/platform/linux.pyx index 196d8bd8..951775b6 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -287,7 +287,7 @@ def acl_set(path, item, numeric_ids=False, fd=None): # Linux does not support setting ACLs on symlinks return - if fd is None and isinstance(path, str): + if isinstance(path, str): path = os.fsencode(path) if numeric_ids: converter = posix_acl_use_stored_uid_gid From 9d638e8d627925c20d3a3aa12eab89a55c4aa15e Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 2 Mar 2024 20:58:19 +0100 Subject: [PATCH 09/15] FreeBSD: acl_get: add an acl_extended_* call ... to implement same semantics as on linux (only store ACL if it defines permissions other than those defined by the traditional file permissions). Looks like there is no call working with an fd on FreeBSD. --- src/borg/platform/freebsd.pyx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/borg/platform/freebsd.pyx b/src/borg/platform/freebsd.pyx index 8ea30823..34f5f918 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -42,6 +42,7 @@ cdef extern from "sys/acl.h": char *acl_to_text_np(acl_t acl, ssize_t *len, int flags) int ACL_TEXT_NUMERIC_IDS int ACL_TEXT_APPEND_ID + int acl_extended_link_np(const char * path) # check also: acl_is_trivial_np cdef extern from "unistd.h": long lpathconf(const char *path, int name) @@ -146,6 +147,12 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): cdef int flags = ACL_TEXT_APPEND_ID if isinstance(path, str): path = os.fsencode(path) + ret = acl_extended_link_np(path) + if ret < 0: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + if ret == 0: + # there is no ACL defining permissions other than those defined by the traditional file permission bits. + return ret = lpathconf(path, _PC_ACL_NFS4) if ret < 0 and errno.errno == errno.EINVAL: return From f8e8608488caef03c0803660dbdeb05a79edac5c Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 2 Mar 2024 21:17:45 +0100 Subject: [PATCH 10/15] FreeBSD: acl_get: raise OSError if lpathconf fails Previously: - acl_get just returned for lpathconf returning EINVAL - acl_get silently ignored all other lpathconf errors and implied it is not a NFS4 acl Now: - not sure why the EINVAL silent return was done, but it seems wrong. guess it could be the system not implementing a check for nfs4. but in that case guess we still would like to get the default and access ACL!? Thus, I removed the silent return. - raise OSError for all lpathconf errors Cosmetic: add a nfs4_acl boolean, so the code reads better. --- src/borg/platform/freebsd.pyx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/borg/platform/freebsd.pyx b/src/borg/platform/freebsd.pyx index 34f5f918..d9b14bf4 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -145,6 +145,7 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): If `numeric_ids` is True the user/group field is not preserved only uid/gid """ cdef int flags = ACL_TEXT_APPEND_ID + flags |= ACL_TEXT_NUMERIC_IDS if numeric_ids else 0 if isinstance(path, str): path = os.fsencode(path) ret = acl_extended_link_np(path) @@ -154,10 +155,10 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): # there is no ACL defining permissions other than those defined by the traditional file permission bits. return ret = lpathconf(path, _PC_ACL_NFS4) - if ret < 0 and errno.errno == errno.EINVAL: - return - flags |= ACL_TEXT_NUMERIC_IDS if numeric_ids else 0 - if ret > 0: + if ret < 0: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + nfs4_acl = ret == 1 + if nfs4_acl: _get_acl(path, ACL_TYPE_NFS4, item, 'acl_nfs4', flags, fd=fd) else: _get_acl(path, ACL_TYPE_ACCESS, item, 'acl_access', flags, fd=fd) From b0607909e19574dbed08ca057c46a9e04effe3c4 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 9 Mar 2024 18:28:41 +0100 Subject: [PATCH 11/15] FreeBSD: added tests, only get default ACL from dirs --- src/borg/platform/freebsd.pyx | 4 ++- src/borg/testsuite/platform.py | 57 ++++++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/borg/platform/freebsd.pyx b/src/borg/platform/freebsd.pyx index d9b14bf4..bd19dc1c 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -1,4 +1,5 @@ import os +import stat from .posix import posix_acl_use_stored_uid_gid from ..helpers import safe_encode, safe_decode @@ -162,7 +163,8 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): _get_acl(path, ACL_TYPE_NFS4, item, 'acl_nfs4', flags, fd=fd) else: _get_acl(path, ACL_TYPE_ACCESS, item, 'acl_access', flags, fd=fd) - _get_acl(path, ACL_TYPE_DEFAULT, item, 'acl_default', flags, fd=fd) + if stat.S_ISDIR(st.st_mode): + _get_acl(path, ACL_TYPE_DEFAULT, item, 'acl_default', flags, fd=fd) cdef _set_acl(p, type, item, attribute, numeric_ids=False, fd=None): diff --git a/src/borg/testsuite/platform.py b/src/borg/testsuite/platform.py index c0485f36..2b512d17 100644 --- a/src/borg/testsuite/platform.py +++ b/src/borg/testsuite/platform.py @@ -59,16 +59,28 @@ def are_acls_working(): if is_darwin: acl_key = 'acl_extended' acl_value = b'!#acl 1\nuser:FFFFEEEE-DDDD-CCCC-BBBB-AAAA00000000:root:0:allow:read\n' - else: + elif is_linux: acl_key = 'acl_access' acl_value = b'user::rw-\ngroup::r--\nmask::rw-\nother::---\nuser:root:rw-:9999\ngroup:root:rw-:9999\n' + elif is_freebsd: + acl_key = 'acl_access' + acl_value = b'user::rw-\ngroup::r--\nmask::rw-\nother::---\nuser:root:rw-\ngroup:wheel:rw-\n' + else: + return False # ACLs unsupported on this platform. write_acl = {acl_key: acl_value} acl_set(filepath, write_acl) read_acl = {} acl_get(filepath, read_acl, os.stat(filepath)) acl = read_acl.get(acl_key, None) if acl is not None: - check_for = b'root:0:allow:read' if is_darwin else b'user::rw-' + if is_darwin: + check_for = b'root:0:allow:read' + elif is_linux: + check_for = b'user::rw-' + elif is_freebsd: + check_for = b'user::rw-' + else: + return False # ACLs unsupported on this platform. if check_for in acl: return True except PermissionError: @@ -157,6 +169,47 @@ class PlatformLinuxTestCase(BaseTestCase): self.assert_equal(acl_use_local_uid_gid(b'group:root:rw-:0'), b'group:0:rw-') +@unittest.skipUnless(is_freebsd, 'freebsd only test') +class PlatformFreeBSDTestCase(BaseTestCase): + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.tmpdir) + + def get_acl(self, path, numeric_ids=False): + item = {} + acl_get(path, item, os.stat(path), numeric_ids=numeric_ids) + return item + + def set_acl(self, path, access=None, default=None, numeric_ids=False): + item = {'acl_access': access, 'acl_default': default} + acl_set(path, item, numeric_ids=numeric_ids) + + @unittest.skipIf(not are_acls_working(), 'ACLs do not work') + def test_access_acl(self): + file = tempfile.NamedTemporaryFile() + self.assert_equal(self.get_acl(file.name), {}) + self.set_acl(file.name, access=b'user::rw-\ngroup::r--\nmask::rw-\nother::---\nuser:root:rw-\ngroup:wheel:rw-\n', numeric_ids=False) + self.assert_in(b'user:root:rw-', self.get_acl(file.name)['acl_access']) + self.assert_in(b'group:wheel:rw-', self.get_acl(file.name)['acl_access']) + self.assert_in(b'user:0:rw-', self.get_acl(file.name, numeric_ids=True)['acl_access']) + file2 = tempfile.NamedTemporaryFile() + self.set_acl(file2.name, access=b'user::rw-\ngroup::r--\nmask::rw-\nother::---\nuser:root:rw-\ngroup:wheel:rw-\n', numeric_ids=True) + self.assert_in(b'user::rw-', self.get_acl(file2.name)['acl_access']) + self.assert_in(b'group::r--', self.get_acl(file2.name)['acl_access']) + + @unittest.skipIf(not are_acls_working(), 'ACLs do not work') + def test_default_acl(self): + ACCESS_ACL = b'user::rw-\nuser:root:rw-\nuser:9999:r--\ngroup::r--\ngroup:wheel:r--\ngroup:9999:r--\nmask::rw-\nother::r--\n' + DEFAULT_ACL = b'user::rw-\nuser:root:r--\nuser:8888:r--\ngroup::r--\ngroup:wheel:r--\ngroup:8888:r--\nmask::rw-\nother::r--\n' + self.assert_equal(self.get_acl(self.tmpdir), {}) + self.set_acl(self.tmpdir, access=ACCESS_ACL, default=DEFAULT_ACL) + self.assert_equal(self.get_acl(self.tmpdir)['acl_access'], ACCESS_ACL) + self.assert_equal(self.get_acl(self.tmpdir)['acl_default'], DEFAULT_ACL) + + @unittest.skipUnless(is_darwin, 'macOS only test') @unittest.skipIf(fakeroot_detected(), 'not compatible with fakeroot') class PlatformDarwinTestCase(BaseTestCase): From 47e291eec187e66363ac0f3647b1623e99d540fe Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 10 Mar 2024 13:11:10 +0100 Subject: [PATCH 12/15] FreeBSD: simplify numeric_ids check --- src/borg/platform/freebsd.pyx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/borg/platform/freebsd.pyx b/src/borg/platform/freebsd.pyx index bd19dc1c..3742d363 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -171,10 +171,11 @@ cdef _set_acl(p, type, item, attribute, numeric_ids=False, fd=None): cdef acl_t acl text = item.get(attribute) if text: - if numeric_ids and type == ACL_TYPE_NFS4: - text = _nfs4_use_stored_uid_gid(text) - elif numeric_ids and type in(ACL_TYPE_ACCESS, ACL_TYPE_DEFAULT): - text = posix_acl_use_stored_uid_gid(text) + if numeric_ids: + if type == ACL_TYPE_NFS4: + text = _nfs4_use_stored_uid_gid(text) + elif type in (ACL_TYPE_ACCESS, ACL_TYPE_DEFAULT): + text = posix_acl_use_stored_uid_gid(text) acl = acl_from_text(text) if acl == NULL: raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) From b45f572a95f509e05b0beeadf7031f6061c5cdeb Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 10 Mar 2024 13:20:37 +0100 Subject: [PATCH 13/15] FreeBSD: check first if kind of ACL can be set on a file --- src/borg/platform/freebsd.pyx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/borg/platform/freebsd.pyx b/src/borg/platform/freebsd.pyx index 3742d363..430b5b6b 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -48,6 +48,7 @@ cdef extern from "sys/acl.h": cdef extern from "unistd.h": long lpathconf(const char *path, int name) int _PC_ACL_NFS4 + int _PC_ACL_EXTENDED # On FreeBSD, borg currently only deals with the USER namespace as it is unclear @@ -212,6 +213,14 @@ def acl_set(path, item, numeric_ids=False, fd=None): """ if isinstance(path, str): path = os.fsencode(path) - _set_acl(path, ACL_TYPE_NFS4, item, 'acl_nfs4', numeric_ids, fd=fd) - _set_acl(path, ACL_TYPE_ACCESS, item, 'acl_access', numeric_ids, fd=fd) - _set_acl(path, ACL_TYPE_DEFAULT, item, 'acl_default', numeric_ids, fd=fd) + ret = lpathconf(path, _PC_ACL_NFS4) + if ret < 0: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + if ret == 1: + _set_acl(path, ACL_TYPE_NFS4, item, 'acl_nfs4', numeric_ids, fd=fd) + ret = lpathconf(path, _PC_ACL_EXTENDED) + if ret < 0: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + if ret == 1: + _set_acl(path, ACL_TYPE_ACCESS, item, 'acl_access', numeric_ids, fd=fd) + _set_acl(path, ACL_TYPE_DEFAULT, item, 'acl_default', numeric_ids, fd=fd) From 54c7da3c64b26987bfaaa43daf99390a7c73bfd8 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 9 Mar 2024 18:44:00 +0100 Subject: [PATCH 14/15] Linux: acl tests: move ACCESS_ACL and DEFAULT_ACL constants They are only used at one place, move them there rather than having globals. --- src/borg/testsuite/platform.py | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/src/borg/testsuite/platform.py b/src/borg/testsuite/platform.py index 2b512d17..54f56e66 100644 --- a/src/borg/testsuite/platform.py +++ b/src/borg/testsuite/platform.py @@ -13,29 +13,6 @@ from . import BaseTestCase, unopened_tempfile from .locking import free_pid -ACCESS_ACL = """ -user::rw- -user:root:rw-:0 -user:9999:r--:9999 -group::r-- -group:root:r--:0 -group:9999:r--:9999 -mask::rw- -other::r-- -""".strip().encode('ascii') - -DEFAULT_ACL = """ -user::rw- -user:root:r--:0 -user:8888:r--:8888 -group::r-- -group:root:r--:0 -group:8888:r--:8888 -mask::rw- -other::r-- -""".strip().encode('ascii') - - def fakeroot_detected(): return 'FAKEROOTKEY' in os.environ @@ -125,6 +102,8 @@ class PlatformLinuxTestCase(BaseTestCase): @unittest.skipIf(not are_acls_working(), 'ACLs do not work') def test_default_acl(self): + ACCESS_ACL = b'user::rw-\nuser:root:rw-:0\nuser:9999:r--:9999\ngroup::r--\ngroup:root:r--:0\ngroup:9999:r--:9999\nmask::rw-\nother::r--' + DEFAULT_ACL = b'user::rw-\nuser:root:r--:0\nuser:8888:r--:8888\ngroup::r--\ngroup:root:r--:0\ngroup:8888:r--:8888\nmask::rw-\nother::r--' self.assert_equal(self.get_acl(self.tmpdir), {}) self.set_acl(self.tmpdir, access=ACCESS_ACL, default=DEFAULT_ACL) self.assert_equal(self.get_acl(self.tmpdir)['acl_access'], ACCESS_ACL) From 7ba843b8c0ca73405dd997e10145bd16d0c7bac0 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 10 Mar 2024 13:35:53 +0100 Subject: [PATCH 15/15] Linux: refactor acl_get --- src/borg/platform/linux.pyx | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/borg/platform/linux.pyx b/src/borg/platform/linux.pyx index 951775b6..35ad1bde 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -240,12 +240,12 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): ret = acl_extended_fd(fd) else: ret = acl_extended_file_nofollow(path) + if ret < 0: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) if ret == 0: # there is no ACL defining permissions other than those defined by the traditional file permission bits. # note: this should also be the case for symlink fs objects, as they can not have ACLs. return - if ret < 0: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) if numeric_ids: converter = acl_numeric_ids else: @@ -257,26 +257,26 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): access_acl = acl_get_file(path, ACL_TYPE_ACCESS) if access_acl == NULL: raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - if stat.S_ISDIR(st.st_mode): - # only directories can have a default ACL. there is no fd-based api to get it. + access_text = acl_to_text(access_acl, NULL) + if access_text == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + item['acl_access'] = converter(access_text) + finally: + acl_free(access_text) + acl_free(access_acl) + if stat.S_ISDIR(st.st_mode): + # only directories can have a default ACL. there is no fd-based api to get it. + try: default_acl = acl_get_file(path, ACL_TYPE_DEFAULT) if default_acl == NULL: raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - if access_acl: - access_text = acl_to_text(access_acl, NULL) - if access_text == NULL: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - item['acl_access'] = converter(access_text) - if default_acl: default_text = acl_to_text(default_acl, NULL) if default_text == NULL: raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) item['acl_default'] = converter(default_text) - finally: - acl_free(default_text) - acl_free(default_acl) - acl_free(access_text) - acl_free(access_acl) + finally: + acl_free(default_text) + acl_free(default_acl) def acl_set(path, item, numeric_ids=False, fd=None):