From 78b47a4e89a9b9aa3484571514ad7e252c4169af Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 9 Feb 2025 22:56:00 +0100 Subject: [PATCH 1/3] xattr-util: drop unnecessary nofollow normalization Follow-up for b8df25dcfe674e37ceb3d54e00a31e1d33e96057 This is some leftover from get/listxattrat() usage which got dropped later. --- src/basic/xattr-util.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/basic/xattr-util.c b/src/basic/xattr-util.c index 411103b83e..a43f795f0f 100644 --- a/src/basic/xattr-util.c +++ b/src/basic/xattr-util.c @@ -90,14 +90,13 @@ static int getxattr_pinned_internal( assert(!path || !isempty(path)); assert((fd >= 0) == !path); - assert((at_flags & ~(AT_SYMLINK_NOFOLLOW|AT_EMPTY_PATH)) == 0); assert(path || FLAGS_SET(at_flags, AT_EMPTY_PATH)); assert(name); assert(buf || size == 0); if (path) - n = FLAGS_SET(at_flags, AT_SYMLINK_NOFOLLOW) ? lgetxattr(path, name, buf, size) - : getxattr(path, name, buf, size); + n = FLAGS_SET(at_flags, AT_SYMLINK_FOLLOW) ? getxattr(path, name, buf, size) + : lgetxattr(path, name, buf, size); else n = by_procfs ? getxattr(FORMAT_PROC_FD_PATH(fd), name, buf, size) : fgetxattr(fd, name, buf, size); @@ -141,8 +140,6 @@ int getxattr_at_malloc( if (r < 0) return r; - at_flags = at_flags_normalize_nofollow(at_flags); - size_t l = 100; for (unsigned n_attempts = 7;;) { _cleanup_free_ char *v = NULL; @@ -200,13 +197,12 @@ static int listxattr_pinned_internal( assert(!path || !isempty(path)); assert((fd >= 0) == !path); - assert((at_flags & ~(AT_SYMLINK_NOFOLLOW|AT_EMPTY_PATH)) == 0); assert(path || FLAGS_SET(at_flags, AT_EMPTY_PATH)); assert(buf || size == 0); if (path) - n = FLAGS_SET(at_flags, AT_SYMLINK_NOFOLLOW) ? llistxattr(path, buf, size) - : listxattr(path, buf, size); + n = FLAGS_SET(at_flags, AT_SYMLINK_FOLLOW) ? listxattr(path, buf, size) + : llistxattr(path, buf, size); else n = by_procfs ? listxattr(FORMAT_PROC_FD_PATH(fd), buf, size) : flistxattr(fd, buf, size); @@ -236,8 +232,6 @@ int listxattr_at_malloc(int fd, const char *path, int at_flags, char **ret) { if (r < 0) return r; - at_flags = at_flags_normalize_nofollow(at_flags); - size_t l = 100; for (unsigned n_attempts = 7;;) { _cleanup_free_ char *v = NULL; From d696e9e64f5b7b1a9776957e30eb5ae20e3b0fdf Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 9 Feb 2025 23:18:05 +0100 Subject: [PATCH 2/3] test-xattr-util: relax listxattr() retval check Follow-up for d228afd792d713a754e4368c014c1f43f10cf5b7 Apparently security.selinux might get set on the file too, behind our back. --- src/test/test-xattr-util.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/test/test-xattr-util.c b/src/test/test-xattr-util.c index 25f982e1fd..86b72b39e1 100644 --- a/src/test/test-xattr-util.c +++ b/src/test/test-xattr-util.c @@ -11,8 +11,10 @@ #include "fd-util.h" #include "fs-util.h" #include "macro.h" +#include "nulstr-util.h" #include "rm-rf.h" #include "string-util.h" +#include "strv.h" #include "tests.h" #include "tmpfile-util.h" #include "xattr-util.h" @@ -89,13 +91,18 @@ static void verify_xattr(int dfd, const char *expected) { static void xattr_symlink_test_one(int fd, const char *path) { _cleanup_free_ char *value = NULL, *list = NULL; + _cleanup_strv_free_ char **list_split = NULL; + int r; ASSERT_OK(xsetxattr(fd, path, 0, "trusted.test", "schaffen")); ASSERT_OK_EQ(getxattr_at_malloc(fd, path, "trusted.test", 0, &value), (int) STRLEN("schaffen")); ASSERT_STREQ(value, "schaffen"); - ASSERT_OK_EQ(listxattr_at_malloc(fd, path, 0, &list), (int) sizeof("trusted.test")); - ASSERT_STREQ(list, "trusted.test"); + r = listxattr_at_malloc(fd, path, 0, &list); + ASSERT_OK(r); + ASSERT_GE(r, (int) sizeof("trusted.test")); + ASSERT_NOT_NULL(list_split = strv_parse_nulstr(list, r)); + ASSERT_TRUE(strv_contains(list_split, "trusted.test")); ASSERT_OK(xremovexattr(fd, path, 0, "trusted.test")); ASSERT_ERROR(getxattr_at_malloc(fd, path, "trusted.test", 0, &value), ENODATA); From a31f7bc2ac5f74f330a3b9e49dd0369e0c9930f2 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 10 Feb 2025 13:33:43 +0100 Subject: [PATCH 3/3] test-xattr-util: return early if not privileged enough to set trusted.* --- src/test/test-xattr-util.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/test/test-xattr-util.c b/src/test/test-xattr-util.c index 86b72b39e1..8991518854 100644 --- a/src/test/test-xattr-util.c +++ b/src/test/test-xattr-util.c @@ -7,7 +7,6 @@ #include #include "alloc-util.h" -#include "capability-util.h" #include "fd-util.h" #include "fs-util.h" #include "macro.h" @@ -94,14 +93,17 @@ static void xattr_symlink_test_one(int fd, const char *path) { _cleanup_strv_free_ char **list_split = NULL; int r; + ASSERT_ERROR(xsetxattr_full(fd, path, 0, "trusted.bar", "bogus", SIZE_MAX, XATTR_CREATE), EEXIST); + ASSERT_OK(xsetxattr(fd, path, 0, "trusted.test", "schaffen")); ASSERT_OK_EQ(getxattr_at_malloc(fd, path, "trusted.test", 0, &value), (int) STRLEN("schaffen")); ASSERT_STREQ(value, "schaffen"); r = listxattr_at_malloc(fd, path, 0, &list); ASSERT_OK(r); - ASSERT_GE(r, (int) sizeof("trusted.test")); + ASSERT_GE(r, (int) sizeof("trusted.test\0trusted.bar")); ASSERT_NOT_NULL(list_split = strv_parse_nulstr(list, r)); + ASSERT_TRUE(strv_contains(list_split, "trusted.bar")); ASSERT_TRUE(strv_contains(list_split, "trusted.test")); ASSERT_OK(xremovexattr(fd, path, 0, "trusted.test")); @@ -157,19 +159,22 @@ TEST(xsetxattr) { fd = safe_close(fd); /* user.* xattrs are not supported on symlinks. Use trusted.* which requires privilege. */ - if (have_effective_cap(CAP_SYS_ADMIN) > 0) { - ASSERT_OK_ERRNO(symlinkat("empty", dfd, "symlink")); - ASSERT_OK_ERRNO(fd = openat(dfd, "symlink", O_NOFOLLOW|O_PATH|O_CLOEXEC)); + ASSERT_OK_ERRNO(symlinkat("empty", dfd, "symlink")); + ASSERT_OK_ERRNO(fd = openat(dfd, "symlink", O_NOFOLLOW|O_PATH|O_CLOEXEC)); - ASSERT_ERROR(xsetxattr(dfd, "symlink", AT_SYMLINK_FOLLOW, "trusted.test", "bogus"), ENOENT); + ASSERT_ERROR(xsetxattr(dfd, "symlink", AT_SYMLINK_FOLLOW, "trusted.test", "bogus"), ENOENT); - xattr_symlink_test_one(dfd, "symlink"); - xattr_symlink_test_one(fd, NULL); - xattr_symlink_test_one(fd, ""); + r = xsetxattr_full(dfd, "symlink", 0, "trusted.bar", "baz", SIZE_MAX, XATTR_CREATE); + if (ERRNO_IS_NEG_PRIVILEGE(r)) + return (void) log_tests_skipped_errno(r, "Unable to set trusted.* xattr"); + ASSERT_OK(r); - x = strjoina(t, "/symlink"); - xattr_symlink_test_one(AT_FDCWD, x); - } + xattr_symlink_test_one(dfd, "symlink"); + xattr_symlink_test_one(fd, NULL); + xattr_symlink_test_one(fd, ""); + + x = strjoina(t, "/symlink"); + xattr_symlink_test_one(AT_FDCWD, x); } DEFINE_TEST_MAIN(LOG_DEBUG);