| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 29 Aug 2022 14:35:04 +0800
From: xiujianfeng <xiujianfeng@...wei.com>
To: <mic@...ikod.net>, <paul@...l-moore.com>, <jmorris@...ei.org>,
<serge@...lyn.com>, <shuah@...nel.org>, <corbet@....net>
CC: <linux-security-module@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
<linux-doc@...r.kernel.org>
Subject: Re: [PATCH -next v2 3/6] landlock: add chmod and chown support
Hi,
在 2022/8/27 19:12, Xiu Jianfeng 写道:
> Add two flags LANDLOCK_ACCESS_FS_CHMOD and LANDLOCK_ACCESS_FS_CHGRP to
> support restriction to chmod(2) and chown(2) with landlock.
>
> If these two access rights are set on a directory, they only take effect
> for its context, not the directory itself.
>
> This patch also change the landlock ABI version from 3 to 4.
>
> Signed-off-by: Xiu Jianfeng <xiujianfeng@...wei.com>
> ---
> include/uapi/linux/landlock.h | 10 +++--
> security/landlock/fs.c | 43 +++++++++++++++++++-
> security/landlock/limits.h | 2 +-
> security/landlock/syscalls.c | 2 +-
> tools/testing/selftests/landlock/base_test.c | 2 +-
> tools/testing/selftests/landlock/fs_test.c | 6 ++-
> 6 files changed, 56 insertions(+), 9 deletions(-)
>
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 735b1fe8326e..07b73626ff20 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -141,14 +141,16 @@ struct landlock_path_beneath_attr {
> * directory) parent. Otherwise, such actions are denied with errno set to
> * EACCES. The EACCES errno prevails over EXDEV to let user space
> * efficiently deal with an unrecoverable error.
> + * - %LANDLOCK_ACCESS_FS_CHMOD: Change the file mode bits of a file.
> + * - %LANDLOCK_ACCESS_FS_CHGRP: Change the owner and/or group of a file.
> *
> * .. warning::
> *
> * It is currently not possible to restrict some file-related actions
> * accessible through these syscall families: :manpage:`chdir(2)`,
> - * :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`chmod(2)`,
> - * :manpage:`chown(2)`, :manpage:`setxattr(2)`, :manpage:`utime(2)`,
> - * :manpage:`ioctl(2)`, :manpage:`fcntl(2)`, :manpage:`access(2)`.
> + * :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`setxattr(2)`,
> + * :manpage:`utime(2)`,:manpage:`ioctl(2)`, :manpage:`fcntl(2)`,
> + * :manpage:`access(2)`.
> * Future Landlock evolutions will enable to restrict them.
> */
> /* clang-format off */
> @@ -167,6 +169,8 @@ struct landlock_path_beneath_attr {
> #define LANDLOCK_ACCESS_FS_MAKE_SYM (1ULL << 12)
> #define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)
> #define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)
> +#define LANDLOCK_ACCESS_FS_CHMOD (1ULL << 15)
> +#define LANDLOCK_ACCESS_FS_CHGRP (1ULL << 16)
> /* clang-format on */
>
> #endif /* _UAPI_LINUX_LANDLOCK_H */
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 4ef614a4ea22..6ac83d96ada7 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -185,7 +185,9 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> LANDLOCK_ACCESS_FS_EXECUTE | \
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> - LANDLOCK_ACCESS_FS_TRUNCATE)
> + LANDLOCK_ACCESS_FS_TRUNCATE | \
> + LANDLOCK_ACCESS_FS_CHMOD | \
> + LANDLOCK_ACCESS_FS_CHGRP)
> /* clang-format on */
>
> /*
> @@ -690,6 +692,31 @@ static inline int current_check_access_path(const struct path *const path,
> return check_access_path(dom, path, access_request);
> }
>
> +static inline int
> +current_check_access_path_context_only(const struct path *const path,
> + const access_mask_t access_request)
> +{
> + const struct landlock_ruleset *const dom =
> + landlock_get_current_domain();
> + struct path eff_path;
> + int ret;
> +
> + if (!dom)
> + return 0;
> + eff_path = *path;
> + /* if it's dir, check its visible parent. */
> + if (d_is_dir(eff_path.dentry)) {
> + path_get(&eff_path);
> + /* dont care if reaches the root or not. */
I may made a mistake here, I think it should return -EACCES directly if
the walk result is not WALK_CONTINUE.
> + walk_to_visible_parent(&eff_path);
> + ret = current_check_access_path(&eff_path, access_request);
> + path_put(&eff_path);
> + } else {
> + ret = current_check_access_path(&eff_path, access_request);
> + }
> + return ret;
> +}
> +
> static inline access_mask_t get_mode_access(const umode_t mode)
> {
> switch (mode & S_IFMT) {
> @@ -1177,6 +1204,18 @@ static int hook_path_truncate(const struct path *const path)
> return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
> }
>
> +static int hook_path_chmod(const struct path *const path, umode_t mode)
> +{
> + return current_check_access_path_context_only(path,
> + LANDLOCK_ACCESS_FS_CHMOD);
> +}
> +
> +static int hook_path_chown(const struct path *const path, kuid_t uid, kgid_t gid)
> +{
> + return current_check_access_path_context_only(path,
> + LANDLOCK_ACCESS_FS_CHGRP);
> +}
> +
> /* File hooks */
>
> static inline access_mask_t get_file_access(const struct file *const file)
> @@ -1230,6 +1269,8 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(path_unlink, hook_path_unlink),
> LSM_HOOK_INIT(path_rmdir, hook_path_rmdir),
> LSM_HOOK_INIT(path_truncate, hook_path_truncate),
> + LSM_HOOK_INIT(path_chmod, hook_path_chmod),
> + LSM_HOOK_INIT(path_chown, hook_path_chown),
>
> LSM_HOOK_INIT(file_open, hook_file_open),
> };
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index 82288f0e9e5e..7cdd7d467d12 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -18,7 +18,7 @@
> #define LANDLOCK_MAX_NUM_LAYERS 16
> #define LANDLOCK_MAX_NUM_RULES U32_MAX
>
> -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_TRUNCATE
> +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_CHGRP
> #define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
> #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index f4d6fc7ed17f..469e0e11735c 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -129,7 +129,7 @@ static const struct file_operations ruleset_fops = {
> .write = fop_dummy_write,
> };
>
> -#define LANDLOCK_ABI_VERSION 3
> +#define LANDLOCK_ABI_VERSION 4
>
> /**
> * sys_landlock_create_ruleset - Create a new ruleset
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 72cdae277b02..9f00582f639c 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -75,7 +75,7 @@ TEST(abi_version)
> const struct landlock_ruleset_attr ruleset_attr = {
> .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - ASSERT_EQ(3, landlock_create_ruleset(NULL, 0,
> + ASSERT_EQ(4, landlock_create_ruleset(NULL, 0,
> LANDLOCK_CREATE_RULESET_VERSION));
>
> ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index debe2d9ea6cf..f513cd8d9d51 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -404,9 +404,11 @@ TEST_F_FORK(layout1, inval)
> LANDLOCK_ACCESS_FS_EXECUTE | \
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> - LANDLOCK_ACCESS_FS_TRUNCATE)
> + LANDLOCK_ACCESS_FS_TRUNCATE | \
> + LANDLOCK_ACCESS_FS_CHMOD | \
> + LANDLOCK_ACCESS_FS_CHGRP)
>
> -#define ACCESS_LAST LANDLOCK_ACCESS_FS_TRUNCATE
> +#define ACCESS_LAST LANDLOCK_ACCESS_FS_CHGRP
>
> #define ACCESS_ALL ( \
> ACCESS_FILE | \
>
Powered by blists - more mailing lists