From a75945ed0dc5624bedb992caeb49fd97adb63e0d Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 24 Feb 2024 23:15:42 +0100 Subject: [PATCH 01/14] improve acl_get / acl_set error handling, see #4049 --- src/borg/platform/darwin.pyx | 41 +++++++++++++++++++++-------------- src/borg/platform/freebsd.pyx | 26 ++++++++++++---------- src/borg/platform/linux.pyx | 36 ++++++++++++++++-------------- 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/src/borg/platform/darwin.pyx b/src/borg/platform/darwin.pyx index 7fe3db3ec..5ac4f18f4 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 @@ -115,20 +116,25 @@ def _remove_non_numeric_identifier(acl): def acl_get(path, item, st, numeric_ids=False, fd=None): cdef acl_t acl = NULL cdef char *text = NULL + if isinstance(path, str): + path = os.fsencode(path) try: if fd is not None: acl = acl_get_fd_np(fd, ACL_TYPE_EXTENDED) else: - if isinstance(path, str): - path = os.fsencode(path) acl = acl_get_link_np(path, ACL_TYPE_EXTENDED) - if acl is not NULL: - text = acl_to_text(acl, NULL) - if text is not NULL: - if numeric_ids: - item['acl_extended'] = _remove_non_numeric_identifier(text) - else: - item['acl_extended'] = text + if acl == NULL: + 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 + if numeric_ids: + item['acl_extended'] = _remove_non_numeric_identifier(text) + else: + item['acl_extended'] = text finally: acl_free(text) acl_free(acl) @@ -143,12 +149,15 @@ def acl_set(path, item, numeric_ids=False, fd=None): acl = acl_from_text(acl_text) else: acl = acl_from_text(_remove_numeric_id_if_possible(acl_text)) - if acl is not NULL: - if fd is not None: - acl_set_fd_np(fd, acl, ACL_TYPE_EXTENDED) - else: - if isinstance(path, str): - path = os.fsencode(path) - acl_set_link_np(path, ACL_TYPE_EXTENDED, acl) + if acl == NULL: + return + if isinstance(path, str): + path = os.fsencode(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)) + else: + 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 0bfc4d750..4caf8f7a5 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -1,15 +1,13 @@ import os +from libc cimport errno + 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.2_05' -cdef extern from "errno.h": - int errno - int EINVAL - 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) ssize_t c_extattr_list_link "extattr_list_link" (const char *path, int attrnamespace, void *data, size_t nbytes) @@ -138,6 +136,8 @@ cdef _get_acl(p, type, item, attribute, flags, fd=None): finally: 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): @@ -149,7 +149,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,15 +167,17 @@ cdef _set_acl(path, type, item, attribute, numeric_ids=False, fd=None): 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) - try: - acl = acl_from_text( text) - if acl is not NULL: + acl = acl_from_text(text) + if acl: + try: if fd is not None: - acl_set_fd_np(fd, acl, type) + if acl_set_fd_np(fd, acl, type) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) else: - acl_set_link_np(path, type, acl) - finally: - acl_free(acl) + if acl_set_link_np(path, type, acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + 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 aa46da5ee..bae1278a4 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -252,23 +252,24 @@ 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 is not NULL: - access_text = acl_to_text(access_acl, NULL) - if access_text is not NULL: - item['acl_access'] = converter(access_text) - finally: - acl_free(access_text) - acl_free(access_acl) - - try: + 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 is not NULL: - default_text = acl_to_text(default_acl, NULL) - if default_text is not NULL: - item['acl_default'] = converter(default_text) + 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: + 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) finally: + acl_free(access_text) + acl_free(access_acl) acl_free(default_text) acl_free(default_acl) @@ -293,9 +294,11 @@ def acl_set(path, item, numeric_ids=False, fd=None): access_acl = acl_from_text( converter(access_text)) if access_acl is not NULL: 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') @@ -304,7 +307,8 @@ def acl_set(path, item, numeric_ids=False, fd=None): default_acl = acl_from_text( converter(default_text)) if default_acl is not NULL: # 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 b3554cdc0fc95209fed7a9f7fa8a11176e14df4f Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 26 Feb 2024 20:07:10 +0100 Subject: [PATCH 02/14] 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 | 49 +++++++++++++++++------------------ src/borg/platform/linux.pyx | 38 +++++++++++++++------------ 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/src/borg/platform/darwin.pyx b/src/borg/platform/darwin.pyx index 5ac4f18f4..b9617a7a8 100644 --- a/src/borg/platform/darwin.pyx +++ b/src/borg/platform/darwin.pyx @@ -130,7 +130,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: @@ -145,14 +145,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 4caf8f7a5..26dd2262b 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -122,23 +122,21 @@ def setxattr(path, name, value, *, follow_symlinks=False): cdef _get_acl(p, type, item, attribute, flags, fd=None): - cdef acl_t acl = NULL - cdef char *text = NULL - try: - if fd is not None: - acl = acl_get_fd_np(fd, type) - else: - acl = acl_get_link_np(p, type) - if acl is not NULL: - text = acl_to_text_np(acl, NULL, flags) - if text is not NULL: - item[attribute] = text - finally: - acl_free(text) - acl_free(acl) + cdef acl_t acl + cdef char *text + if fd is not None: + acl = acl_get_fd_np(fd, type) else: + acl = acl_get_link_np(p, type) + 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 @@ -168,16 +166,17 @@ cdef _set_acl(path, 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(path)) - else: - if acl_set_link_np(path, type, acl) == -1: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - finally: - acl_free(acl) + if acl == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + 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(path)) + else: + if acl_set_link_np(path, type, acl) == -1: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + 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 bae1278a4..943f020fc 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -261,12 +261,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(access_text) acl_free(access_acl) @@ -291,24 +293,26 @@ def acl_set(path, item, numeric_ids=False, fd=None): access_text = item.get('acl_access') if access_text is not None: try: - access_acl = acl_from_text( converter(access_text)) - if access_acl is not NULL: - 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)) + access_acl = acl_from_text(converter(access_text)) + 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 is not None: try: - default_acl = acl_from_text( converter(default_text)) - if default_acl is not NULL: - # 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)) + default_acl = acl_from_text(converter(default_text)) + 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 d5396feebde1f79b9075e5712d10af6f957461c6 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 25 Feb 2024 02:19:38 +0100 Subject: [PATCH 03/14] 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 | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/borg/testsuite/platform.py b/src/borg/testsuite/platform.py index a8a859b09..d08b1d9a2 100644 --- a/src/borg/testsuite/platform.py +++ b/src/borg/testsuite/platform.py @@ -1,3 +1,4 @@ +import errno import functools import os @@ -31,25 +32,26 @@ def are_acls_working(): with unopened_tempfile() as filepath: open(filepath, "w").close() try: - if is_freebsd: - access = b"user::rw-\ngroup::r--\nmask::rw-\nother::---\nuser:root:rw-\n" - contained = b"user:root:rw-" - elif is_linux: - access = b"user::rw-\ngroup::r--\nmask::rw-\nother::---\nuser:root:rw-:0\n" - contained = b"user:root:rw-:0" - elif is_darwin: - return True # improve? + if is_darwin: + acl_key = "acl_extended" + acl_value = b"!#acl 1\nuser:FFFFEEEE-DDDD-CCCC-BBBB-AAAA00000000:root:0:allow:read\n" else: - return False # unsupported platform - acl = {"acl_access": access} - acl_set(filepath, acl) + 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 contained 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 bafea3b5de2d90516ae66bae6a4219e979326f48 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 9 Mar 2024 17:53:12 +0100 Subject: [PATCH 04/14] platform tests: misc. minor cleanups - remove unused global / import - use is_linux and is_darwin - rename darwin acl test method --- src/borg/testsuite/platform_darwin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borg/testsuite/platform_darwin.py b/src/borg/testsuite/platform_darwin.py index 8a614607b..8921beedc 100644 --- a/src/borg/testsuite/platform_darwin.py +++ b/src/borg/testsuite/platform_darwin.py @@ -20,7 +20,7 @@ def set_acl(path, acl, numeric_ids=False): @skipif_acls_not_working -def test_access_acl(): +def test_extended_acl(): file = tempfile.NamedTemporaryFile() assert get_acl(file.name) == {} set_acl( From 1269c852bf07006dc85f964678b3fd77acdba776 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 25 Feb 2024 03:06:32 +0100 Subject: [PATCH 05/14] 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 33de6fac7..4270fc386 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -964,7 +964,11 @@ def restore_attrs(self, path, item, symlink=False, fd=None): 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 and "xattrs" in item: # 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. @@ -1210,7 +1214,11 @@ def stat_ext_attrs(self, st, path, fd=None): 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 beac2fa9aefb5848e80e24800b03cfb5597c2d28 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 2 Mar 2024 19:59:04 +0100 Subject: [PATCH 06/14] 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 943f020fc..dfbf925b9 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -233,16 +233,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 96cac5f3810de7f15bb992be051221008c9be38a Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 2 Mar 2024 20:04:22 +0100 Subject: [PATCH 07/14] 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 dfbf925b9..2f02065f5 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -50,7 +50,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": @@ -235,17 +235,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 4cc4516c59401e581ff2870a99629ea5b0c9feea Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 2 Mar 2024 20:08:11 +0100 Subject: [PATCH 08/14] 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 2f02065f5..64a01ac5d 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -288,7 +288,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 30f4518058a9a38e9ad051236dbaf27552e3306b Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 2 Mar 2024 20:58:19 +0100 Subject: [PATCH 09/14] 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 26dd2262b..5f66c81cb 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 d3694271ebd90a63888b6ec0653d3abb249ab4ec Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 2 Mar 2024 21:17:45 +0100 Subject: [PATCH 10/14] 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 5f66c81cb..d37bee9bc 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 7df170c946ba33de29c54b1de5a7c2876724ef16 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 9 Mar 2024 18:28:41 +0100 Subject: [PATCH 11/14] FreeBSD: added tests, only get default ACL from dirs --- src/borg/platform/freebsd.pyx | 4 +++- src/borg/testsuite/platform.py | 16 ++++++++++++++-- src/borg/testsuite/platform_freebsd.py | 2 ++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/borg/platform/freebsd.pyx b/src/borg/platform/freebsd.pyx index d37bee9bc..87f9cdfd9 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -1,4 +1,5 @@ import os +import stat from libc cimport errno @@ -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(path, type, item, attribute, numeric_ids=False, fd=None): diff --git a/src/borg/testsuite/platform.py b/src/borg/testsuite/platform.py index d08b1d9a2..40ea3d78c 100644 --- a/src/borg/testsuite/platform.py +++ b/src/borg/testsuite/platform.py @@ -35,16 +35,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: diff --git a/src/borg/testsuite/platform_freebsd.py b/src/borg/testsuite/platform_freebsd.py index 54fb29d6a..c47da68b6 100644 --- a/src/borg/testsuite/platform_freebsd.py +++ b/src/borg/testsuite/platform_freebsd.py @@ -49,6 +49,7 @@ def set_acl(path, access=None, default=None, nfs4=None, numeric_ids=False): @skipif_acls_not_working def test_access_acl(): file1 = tempfile.NamedTemporaryFile() + assert get_acl(file1.name) == {} set_acl( file1.name, access=b"user::rw-\ngroup::r--\nmask::rw-\nother::---\nuser:root:rw-\ngroup:wheel:rw-\n", @@ -86,6 +87,7 @@ def test_access_acl(): @skipif_acls_not_working def test_default_acl(): tmpdir = tempfile.mkdtemp() + assert get_acl(tmpdir) == {} set_acl(tmpdir, access=ACCESS_ACL, default=DEFAULT_ACL) assert get_acl(tmpdir)["acl_access"] == ACCESS_ACL assert get_acl(tmpdir)["acl_default"] == DEFAULT_ACL From 4ebb5cdf3caf6c443910cc2349ce359154006b1d Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 10 Mar 2024 13:11:10 +0100 Subject: [PATCH 12/14] FreeBSD: simplify numeric_ids check --- src/borg/platform/freebsd.pyx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/borg/platform/freebsd.pyx b/src/borg/platform/freebsd.pyx index 87f9cdfd9..ac522dfbe 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -170,11 +170,12 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): cdef _set_acl(path, type, item, attribute, numeric_ids=False, fd=None): cdef acl_t acl = NULL text = item.get(attribute) - if text is not None: - 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 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(path)) From 64b7b5fdd4b9cf690d6c420e17fd8c65897a9219 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 10 Mar 2024 13:20:37 +0100 Subject: [PATCH 13/14] 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 ac522dfbe..c38f03588 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 @@ -213,6 +214,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 4e5bf284732798920bb3d79a441b948ddaf81369 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 10 Mar 2024 13:35:53 +0100 Subject: [PATCH 14/14] 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 64a01ac5d..178b184ba 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -241,12 +241,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: @@ -258,26 +258,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(access_text) - acl_free(access_acl) - acl_free(default_text) - acl_free(default_acl) + finally: + acl_free(default_text) + acl_free(default_acl) def acl_set(path, item, numeric_ids=False, fd=None):