From: "Günther Noack" <gnoack3000@gmail.com>
To: Xiu Jianfeng <xiujianfeng@huawei.com>
Cc: mic@digikod.net, paul@paul-moore.com, jmorris@namei.org,
serge@hallyn.com, shuah@kernel.org, corbet@lwn.net,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-doc@vger.kernel.org
Subject: Re: [PATCH -next v2 4/6] landlock/selftests: add selftests for chmod and chown
Date: Sat, 27 Aug 2022 19:48:25 +0200 [thread overview]
Message-ID: <YwpY6UpznnHpweh+@nuc> (raw)
In-Reply-To: <20220827111215.131442-5-xiujianfeng@huawei.com>
On Sat, Aug 27, 2022 at 07:12:13PM +0800, Xiu Jianfeng wrote:
> The following APIs are tested with simple scenarios.
> 1. chmod/fchmod/fchmodat;
> 2. chmod/fchmod/lchown/fchownat;
>
> The key point is that set these access rights on a directory but only for
> its content, not the directory itself. this scenario is covered.
>
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> ---
> tools/testing/selftests/landlock/fs_test.c | 261 +++++++++++++++++++++
> 1 file changed, 261 insertions(+)
>
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index f513cd8d9d51..982cb824967c 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -3272,6 +3272,267 @@ TEST_F_FORK(layout1, truncate)
> EXPECT_EQ(0, test_creat(file_in_dir_w));
> }
>
> +/* Invokes chmod(2) and returns its errno or 0. */
> +static int test_chmod(const char *const path, mode_t mode)
> +{
> + if (chmod(path, mode) < 0)
> + return errno;
> + return 0;
> +}
Nice, this is much simpler than in the last revision :)
> +
> +/* Invokes fchmod(2) and returns its errno or 0. */
> +static int test_fchmod(int fd, mode_t mode)
> +{
> + if (fchmod(fd, mode) < 0)
> + return errno;
> + return 0;
> +}
> +
> +/* Invokes fchmodat(2) and returns its errno or 0. */
> +static int test_fchmodat(int dirfd, const char *path, mode_t mode, int flags)
Nitpick: Some of these functions have path arguments declared as
const char *path
and others as
const char *const path
-- would be nice to stay consistent.
> +{
> + if (fchmodat(dirfd, path, mode, flags) < 0)
> + return errno;
> + return 0;
> +}
> +
> +/* Invokes chown(2) and returns its errno or 0. */
> +static int test_chown(const char *path, uid_t uid, gid_t gid)
> +{
> + if (chown(path, uid, gid) < 0)
> + return errno;
> + return 0;
> +}
> +
> +/* Invokes fchown(2) and returns its errno or 0. */
> +static int test_fchown(int fd, uid_t uid, gid_t gid)
> +{
> + if (fchown(fd, uid, gid) < 0)
> + return errno;
> + return 0;
> +}
> +
> +/* Invokes lchown(2) and returns its errno or 0. */
> +static int test_lchown(const char *path, uid_t uid, gid_t gid)
> +{
> + if (lchown(path, uid, gid) < 0)
> + return errno;
> + return 0;
> +}
> +
> +/* Invokes fchownat(2) and returns its errno or 0. */
> +static int test_fchownat(int dirfd, const char *path,
> + uid_t uid, gid_t gid, int flags)
> +{
> + if (fchownat(dirfd, path, uid, gid, flags) < 0)
> + return errno;
> + return 0;
> +}
> +
> +TEST_F_FORK(layout1, unhandled_chmod)
> +{
> + int ruleset_fd, file1_fd;
> + const char *file1 = file1_s1d1;
> + const char *file2 = file2_s1d1;
> + const char *dir1 = dir_s1d1;
I'd suggest to name these kinds of variables according to the rights
that are granted on these files and directories, for example as as
w_file and rw_file. (Then, when looking at the EXPECT_EQ checks below,
you don't need to jump back up to the start of the test to remember
which of the rights is being tested.)
Nitpick: The same remark as above about the 'const' modifier applies
here as well. The rest of the file uses "const char *const varname".
> + const struct rule rules[] = {
> + {
> + .path = file1,
> + .access = LANDLOCK_ACCESS_FS_WRITE_FILE,
> + },
> + {
> + .path = file2,
> + .access = LANDLOCK_ACCESS_FS_READ_FILE |
> + LANDLOCK_ACCESS_FS_WRITE_FILE,
> + },
> + {
> + .path = dir1,
> + .access = ACCESS_RW,
> + },
> + {},
> + };
> +
> + ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
> + ASSERT_LE(0, ruleset_fd);
> + file1_fd = open(file1, O_WRONLY | O_CLOEXEC);
> + ASSERT_LE(0, file1_fd);
> +
> + enforce_ruleset(_metadata, ruleset_fd);
> + ASSERT_EQ(0, close(ruleset_fd));
> +
> + EXPECT_EQ(0, test_chmod(file1, 0400));
> + EXPECT_EQ(0, test_fchmod(file1_fd, 0400));
> + EXPECT_EQ(0, test_fchmodat(AT_FDCWD, file1, 0400, 0));
> + EXPECT_EQ(0, test_chmod(file2, 0400));
> + EXPECT_EQ(0, test_chmod(dir1, 0700));
> + ASSERT_EQ(0, close(file1_fd));
> +}
> +
> +TEST_F_FORK(layout1, chmod)
> +{
> + int ruleset_fd, file1_fd;
> + const char *file1 = file1_s1d1;
> + const char *file2 = file2_s1d1;
> + const char *file3 = file1_s2d1;
> + const char *dir1 = dir_s1d1;
> + const char *dir2 = dir_s2d1;
> + const struct rule rules[] = {
> + {
> + .path = file1,
> + .access = LANDLOCK_ACCESS_FS_WRITE_FILE |
> + LANDLOCK_ACCESS_FS_CHMOD,
> + },
> + {
> + .path = file2,
> + .access = LANDLOCK_ACCESS_FS_READ_FILE |
> + LANDLOCK_ACCESS_FS_WRITE_FILE,
> + },
> + {
> + .path = dir1,
> + .access = ACCESS_RW,
> + },
> + {
> + .path = dir2,
> + .access = ACCESS_RW | LANDLOCK_ACCESS_FS_CHMOD,
> + },
> + {},
> + };
> +
> + ruleset_fd = create_ruleset(_metadata, ACCESS_RW | LANDLOCK_ACCESS_FS_CHMOD, rules);
> + ASSERT_LE(0, ruleset_fd);
> + file1_fd = open(file1, O_WRONLY | O_CLOEXEC);
> + ASSERT_LE(0, file1_fd);
> +
> + enforce_ruleset(_metadata, ruleset_fd);
> + ASSERT_EQ(0, close(ruleset_fd));
> +
> + EXPECT_EQ(0, test_chmod(file1, 0400));
> + EXPECT_EQ(0, test_fchmod(file1_fd, 0400));
> + EXPECT_EQ(0, test_fchmodat(AT_FDCWD, file1, 0400, 0));
> + EXPECT_EQ(EACCES, test_chmod(file2, 0400));
> + EXPECT_EQ(EACCES, test_chmod(dir1, 0700));
> + /* set CHMOD right on dir will only affect its context not dir itself*/
> + EXPECT_EQ(0, test_chmod(file3, 0400));
> + EXPECT_EQ(0, test_fchmodat(AT_FDCWD, file3, 0400, 0));
> + EXPECT_EQ(EACCES, test_chmod(dir2, 0700));
> + EXPECT_EQ(EACCES, test_fchmodat(AT_FDCWD, dir2, 0700, 0));
> + ASSERT_EQ(0, close(file1_fd));
> +}
> +
> +TEST_F_FORK(layout1, unhandled_chown)
> +{
> + int ruleset_fd, file1_fd;
> + const char *file1 = file1_s1d1;
> + const char *file2 = file2_s1d1;
> + const char *dir1 = dir_s1d1;
> + const struct rule rules[] = {
> + {
> + .path = file1,
> + .access = LANDLOCK_ACCESS_FS_WRITE_FILE,
> + },
> + {
> + .path = file2,
> + .access = LANDLOCK_ACCESS_FS_READ_FILE |
> + LANDLOCK_ACCESS_FS_WRITE_FILE,
> + },
> + {
> + .path = dir1,
> + .access = ACCESS_RW,
> + },
> + {},
> + };
> + struct stat st;
> + uid_t uid;
> + gid_t gid;
> +
> + ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
> + ASSERT_LE(0, ruleset_fd);
> + file1_fd = open(file1, O_WRONLY | O_CLOEXEC);
> + ASSERT_LE(0, file1_fd);
> + /*
> + * there is no CAP_CHOWN when the testcases framework setup,
> + * and we cannot assume the testcases are run as root, to make
> + * sure {f}chown syscall won't fail, get the original uid/gid and
> + * use them in test_{f}chown.
> + */
> + ASSERT_EQ(0, stat(dir1, &st));
> + uid = st.st_uid;
> + gid = st.st_gid;
> +
> + enforce_ruleset(_metadata, ruleset_fd);
> + ASSERT_EQ(0, close(ruleset_fd));
> +
> + EXPECT_EQ(0, test_chown(file1, uid, gid));
> + EXPECT_EQ(0, test_fchown(file1_fd, uid, gid));
> + EXPECT_EQ(0, test_lchown(file1, uid, gid));
> + EXPECT_EQ(0, test_fchownat(AT_FDCWD, file1, uid, gid, 0));
> + EXPECT_EQ(0, test_chown(file2, uid, gid));
> + EXPECT_EQ(0, test_chown(dir1, uid, gid));
> + ASSERT_EQ(0, close(file1_fd));
> +}
> +
> +TEST_F_FORK(layout1, chown)
> +{
> + int ruleset_fd, file1_fd;
> + const char *file1 = file1_s1d1;
> + const char *file2 = file2_s1d1;
> + const char *file3 = file1_s2d1;
> + const char *dir1 = dir_s1d1;
> + const char *dir2 = dir_s2d1;
> + const struct rule rules[] = {
> + {
> + .path = file1,
> + .access = LANDLOCK_ACCESS_FS_WRITE_FILE |
> + LANDLOCK_ACCESS_FS_CHGRP,
> + },
> + {
> + .path = file2,
> + .access = LANDLOCK_ACCESS_FS_READ_FILE |
> + LANDLOCK_ACCESS_FS_WRITE_FILE,
> + },
> + {
> + .path = dir1,
> + .access = ACCESS_RW,
> + },
> + {
> + .path = dir2,
> + .access = ACCESS_RW | LANDLOCK_ACCESS_FS_CHGRP,
> + },
> + {},
> + };
> + struct stat st;
> + uid_t uid;
> + gid_t gid;
> +
> + ruleset_fd = create_ruleset(_metadata, ACCESS_RW | LANDLOCK_ACCESS_FS_CHGRP, rules);
> + ASSERT_LE(0, ruleset_fd);
> + file1_fd = open(file1, O_WRONLY | O_CLOEXEC);
> + ASSERT_LE(0, file1_fd);
> + /*
> + * there is no CAP_CHOWN when the testcases framework setup,
> + * and we cannot assume the testcases are run as root, to make
> + * sure {f}chown syscall won't fail, get the original uid/gid and
> + * use them in test_{f}chown.
> + */
> + ASSERT_EQ(0, stat(dir1, &st));
> + uid = st.st_uid;
> + gid = st.st_gid;
> +
> + enforce_ruleset(_metadata, ruleset_fd);
> + ASSERT_EQ(0, close(ruleset_fd));
> +
> + EXPECT_EQ(0, test_chown(file1, uid, gid));
> + EXPECT_EQ(0, test_fchown(file1_fd, uid, gid));
> + EXPECT_EQ(0, test_lchown(file1, uid, gid));
> + EXPECT_EQ(0, test_fchownat(AT_FDCWD, file1, uid, gid, 0));
> + EXPECT_EQ(EACCES, test_chown(file2, uid, gid));
> + EXPECT_EQ(EACCES, test_chown(dir1, uid, gid));
> + /* set CHOWN right on dir will only affect its context not dir itself*/
> + EXPECT_EQ(0, test_chown(file3, uid, gid));
> + EXPECT_EQ(EACCES, test_chown(dir2, uid, gid));
> + ASSERT_EQ(0, close(file1_fd));
> +}
> +
> /* clang-format off */
> FIXTURE(layout1_bind) {};
> /* clang-format on */
> --
> 2.17.1
>
--
next prev parent reply other threads:[~2022-08-27 17:48 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-27 11:12 [PATCH -next v2 0/6] landlock: add chmod and chown support Xiu Jianfeng
2022-08-27 11:12 ` [PATCH -next v2 1/6] landlock: expand access_mask_t to u32 type Xiu Jianfeng
2022-08-27 11:12 ` [PATCH -next v2 2/6] landlock: abstract walk_to_visible_parent() helper Xiu Jianfeng
2022-08-30 11:22 ` Mickaël Salaün
2022-08-31 11:56 ` xiujianfeng
2022-08-27 11:12 ` [PATCH -next v2 3/6] landlock: add chmod and chown support Xiu Jianfeng
2022-08-27 19:30 ` Günther Noack
2022-08-29 1:17 ` xiujianfeng
2022-08-29 16:01 ` Mickaël Salaün
2022-09-01 13:06 ` xiujianfeng
2022-09-01 17:34 ` Mickaël Salaün
2022-10-29 8:33 ` xiujianfeng
2022-11-14 14:12 ` Mickaël Salaün
2022-11-18 9:03 ` xiujianfeng
2022-11-18 12:32 ` Mickaël Salaün
2022-11-21 13:48 ` xiujianfeng
2022-08-29 6:30 ` xiujianfeng
2022-08-29 6:35 ` xiujianfeng
2022-08-27 11:12 ` [PATCH -next v2 4/6] landlock/selftests: add selftests for chmod and chown Xiu Jianfeng
2022-08-27 17:48 ` Günther Noack [this message]
2022-08-29 1:49 ` xiujianfeng
2022-08-27 11:12 ` [PATCH -next v2 5/6] landlock/samples: add chmod and chown support Xiu Jianfeng
2022-08-27 11:12 ` [PATCH -next v2 6/6] landlock: update chmod and chown support in document Xiu Jianfeng
2022-08-27 17:28 ` Günther Noack
2022-08-29 1:52 ` xiujianfeng
2022-08-30 11:22 ` [PATCH -next v2 0/6] landlock: add chmod and chown support Mickaël Salaün
2023-04-18 10:53 ` xiujianfeng
2023-04-20 17:40 ` Mickaël Salaün
2023-04-24 8:52 ` xiujianfeng
2023-04-26 13:58 ` Mickaël Salaün
2023-05-05 3:50 ` xiujianfeng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YwpY6UpznnHpweh+@nuc \
--to=gnoack3000@gmail.com \
--cc=corbet@lwn.net \
--cc=jmorris@namei.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mic@digikod.net \
--cc=paul@paul-moore.com \
--cc=serge@hallyn.com \
--cc=shuah@kernel.org \
--cc=xiujianfeng@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).