Message ID | 20230116085818.165539-3-konstantin.meskhidze@huawei.com (mailing list archive) |
---|---|
State | Not Applicable |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | Network support for Landlock | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Not a local patch, async |
Hi Konstantin, I think this patch series is almost ready. Here is a first batch of review, I'll send more next week. I forgot to update the documentation. Can you please squash the following patch into this one? diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst index 980558b879d6..fc2be89b423f 100644 --- a/Documentation/userspace-api/landlock.rst +++ b/Documentation/userspace-api/landlock.rst @@ -416,9 +416,9 @@ Current limitations Filesystem topology modification -------------------------------- -As for file renaming and linking, a sandboxed thread cannot modify its -filesystem topology, whether via :manpage:`mount(2)` or -:manpage:`pivot_root(2)`. However, :manpage:`chroot(2)` calls are not denied. +Threads sandboxed with filesystem restrictions cannot modify filesystem +topology, whether via :manpage:`mount(2)` or :manpage:`pivot_root(2)`. +However, :manpage:`chroot(2)` calls are not denied. Special filesystems ------------------- On 16/01/2023 09:58, Konstantin Meskhidze wrote: > From: Mickaël Salaün <mic@digikod.net> > > Allow mount point and root directory changes when there is no filesystem > rule tied to the current Landlock domain. This doesn't change anything > for now because a domain must have at least a (filesystem) rule, but > this will change when other rule types will come. For instance, a > domain only restricting the network should have no impact on filesystem > restrictions. > > Add a new get_current_fs_domain() helper to quickly check filesystem > rule existence for all filesystem LSM hooks. > > Remove unnecessary inlining. > > Signed-off-by: Mickaël Salaün <mic@digikod.net> > --- > > Changes since v8: > * Refactors get_handled_fs_accesses(). > * Adds landlock_get_raw_fs_access_mask() helper. >
2/10/2023 8:34 PM, Mickaël Salaün пишет: > Hi Konstantin, > > I think this patch series is almost ready. Here is a first batch of > review, I'll send more next week. > Hi Mickaёl. thnaks for the review. > > I forgot to update the documentation. Can you please squash the > following patch into this one? No problem. I will squash. Can I download this doc patch from your repo or I can use the diff below? > > > diff --git a/Documentation/userspace-api/landlock.rst > b/Documentation/userspace-api/landlock.rst > index 980558b879d6..fc2be89b423f 100644 > --- a/Documentation/userspace-api/landlock.rst > +++ b/Documentation/userspace-api/landlock.rst > @@ -416,9 +416,9 @@ Current limitations > Filesystem topology modification > -------------------------------- > > -As for file renaming and linking, a sandboxed thread cannot modify its > -filesystem topology, whether via :manpage:`mount(2)` or > -:manpage:`pivot_root(2)`. However, :manpage:`chroot(2)` calls are not > denied. > +Threads sandboxed with filesystem restrictions cannot modify filesystem > +topology, whether via :manpage:`mount(2)` or :manpage:`pivot_root(2)`. > +However, :manpage:`chroot(2)` calls are not denied. > > Special filesystems > ------------------- > > > On 16/01/2023 09:58, Konstantin Meskhidze wrote: >> From: Mickaël Salaün <mic@digikod.net> >> >> Allow mount point and root directory changes when there is no filesystem >> rule tied to the current Landlock domain. This doesn't change anything >> for now because a domain must have at least a (filesystem) rule, but >> this will change when other rule types will come. For instance, a >> domain only restricting the network should have no impact on filesystem >> restrictions. >> >> Add a new get_current_fs_domain() helper to quickly check filesystem >> rule existence for all filesystem LSM hooks. >> >> Remove unnecessary inlining. >> >> Signed-off-by: Mickaël Salaün <mic@digikod.net> >> --- >> >> Changes since v8: >> * Refactors get_handled_fs_accesses(). >> * Adds landlock_get_raw_fs_access_mask() helper. >> > .
On 14/02/2023 09:51, Konstantin Meskhidze (A) wrote: > > > 2/10/2023 8:34 PM, Mickaël Salaün пишет: >> Hi Konstantin, >> >> I think this patch series is almost ready. Here is a first batch of >> review, I'll send more next week. >> > Hi Mickaёl. > thnaks for the review. > >> >> I forgot to update the documentation. Can you please squash the >> following patch into this one? > > No problem. I will squash. > Can I download this doc patch from your repo or I can use the diff below? You can take the diff below. >> >> >> diff --git a/Documentation/userspace-api/landlock.rst >> b/Documentation/userspace-api/landlock.rst >> index 980558b879d6..fc2be89b423f 100644 >> --- a/Documentation/userspace-api/landlock.rst >> +++ b/Documentation/userspace-api/landlock.rst >> @@ -416,9 +416,9 @@ Current limitations >> Filesystem topology modification >> -------------------------------- >> >> -As for file renaming and linking, a sandboxed thread cannot modify its >> -filesystem topology, whether via :manpage:`mount(2)` or >> -:manpage:`pivot_root(2)`. However, :manpage:`chroot(2)` calls are not >> denied. >> +Threads sandboxed with filesystem restrictions cannot modify filesystem >> +topology, whether via :manpage:`mount(2)` or :manpage:`pivot_root(2)`. >> +However, :manpage:`chroot(2)` calls are not denied. >> >> Special filesystems >> ------------------- >> >> >> On 16/01/2023 09:58, Konstantin Meskhidze wrote: >>> From: Mickaël Salaün <mic@digikod.net> >>> >>> Allow mount point and root directory changes when there is no filesystem >>> rule tied to the current Landlock domain. This doesn't change anything >>> for now because a domain must have at least a (filesystem) rule, but >>> this will change when other rule types will come. For instance, a >>> domain only restricting the network should have no impact on filesystem >>> restrictions. >>> >>> Add a new get_current_fs_domain() helper to quickly check filesystem >>> rule existence for all filesystem LSM hooks. >>> >>> Remove unnecessary inlining. >>> >>> Signed-off-by: Mickaël Salaün <mic@digikod.net> >>> --- >>> >>> Changes since v8: >>> * Refactors get_handled_fs_accesses(). >>> * Adds landlock_get_raw_fs_access_mask() helper. >>> >> .
2/14/2023 3:07 PM, Mickaël Salaün пишет: > > On 14/02/2023 09:51, Konstantin Meskhidze (A) wrote: >> >> >> 2/10/2023 8:34 PM, Mickaël Salaün пишет: >>> Hi Konstantin, >>> >>> I think this patch series is almost ready. Here is a first batch of >>> review, I'll send more next week. >>> >> Hi Mickaёl. >> thnaks for the review. >> >>> >>> I forgot to update the documentation. Can you please squash the >>> following patch into this one? >> >> No problem. I will squash. >> Can I download this doc patch from your repo or I can use the diff below? > > You can take the diff below. Ok. Will be done. > >>> >>> >>> diff --git a/Documentation/userspace-api/landlock.rst >>> b/Documentation/userspace-api/landlock.rst >>> index 980558b879d6..fc2be89b423f 100644 >>> --- a/Documentation/userspace-api/landlock.rst >>> +++ b/Documentation/userspace-api/landlock.rst >>> @@ -416,9 +416,9 @@ Current limitations >>> Filesystem topology modification >>> -------------------------------- >>> >>> -As for file renaming and linking, a sandboxed thread cannot modify its >>> -filesystem topology, whether via :manpage:`mount(2)` or >>> -:manpage:`pivot_root(2)`. However, :manpage:`chroot(2)` calls are not >>> denied. >>> +Threads sandboxed with filesystem restrictions cannot modify filesystem >>> +topology, whether via :manpage:`mount(2)` or :manpage:`pivot_root(2)`. >>> +However, :manpage:`chroot(2)` calls are not denied. >>> >>> Special filesystems >>> ------------------- >>> >>> >>> On 16/01/2023 09:58, Konstantin Meskhidze wrote: >>>> From: Mickaël Salaün <mic@digikod.net> >>>> >>>> Allow mount point and root directory changes when there is no filesystem >>>> rule tied to the current Landlock domain. This doesn't change anything >>>> for now because a domain must have at least a (filesystem) rule, but >>>> this will change when other rule types will come. For instance, a >>>> domain only restricting the network should have no impact on filesystem >>>> restrictions. >>>> >>>> Add a new get_current_fs_domain() helper to quickly check filesystem >>>> rule existence for all filesystem LSM hooks. >>>> >>>> Remove unnecessary inlining. >>>> >>>> Signed-off-by: Mickaël Salaün <mic@digikod.net> >>>> --- >>>> >>>> Changes since v8: >>>> * Refactors get_handled_fs_accesses(). >>>> * Adds landlock_get_raw_fs_access_mask() helper. >>>> >>> . > .
diff --git a/security/landlock/fs.c b/security/landlock/fs.c index 0d57c6479d29..0ae54a639e16 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -150,16 +150,6 @@ static struct landlock_object *get_inode_object(struct inode *const inode) LANDLOCK_ACCESS_FS_TRUNCATE) /* clang-format on */ -/* - * All access rights that are denied by default whether they are handled or not - * by a ruleset/layer. This must be ORed with all ruleset->fs_access_masks[] - * entries when we need to get the absolute handled access masks. - */ -/* clang-format off */ -#define ACCESS_INITIALLY_DENIED ( \ - LANDLOCK_ACCESS_FS_REFER) -/* clang-format on */ - /* * @path: Should have been checked by get_path_from_fd(). */ @@ -179,8 +169,7 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset, /* Transforms relative access rights to absolute ones. */ access_rights |= LANDLOCK_MASK_ACCESS_FS & - ~(landlock_get_fs_access_mask(ruleset, 0) | - ACCESS_INITIALLY_DENIED); + ~landlock_get_fs_access_mask(ruleset, 0); object = get_inode_object(d_backing_inode(path->dentry)); if (IS_ERR(object)) return PTR_ERR(object); @@ -287,14 +276,15 @@ static inline bool is_nouser_or_private(const struct dentry *dentry) unlikely(IS_PRIVATE(d_backing_inode(dentry)))); } -static inline access_mask_t -get_handled_accesses(const struct landlock_ruleset *const domain) +static access_mask_t +get_raw_handled_fs_accesses(const struct landlock_ruleset *const domain) { - access_mask_t access_dom = ACCESS_INITIALLY_DENIED; + access_mask_t access_dom = 0; size_t layer_level; for (layer_level = 0; layer_level < domain->num_layers; layer_level++) - access_dom |= landlock_get_fs_access_mask(domain, layer_level); + access_dom |= + landlock_get_raw_fs_access_mask(domain, layer_level); return access_dom & LANDLOCK_MASK_ACCESS_FS; } @@ -331,13 +321,8 @@ init_layer_masks(const struct landlock_ruleset *const domain, for_each_set_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks)) { - /* - * Artificially handles all initially denied by default - * access rights. - */ if (BIT_ULL(access_bit) & - (landlock_get_fs_access_mask(domain, layer_level) | - ACCESS_INITIALLY_DENIED)) { + landlock_get_fs_access_mask(domain, layer_level)) { (*layer_masks)[access_bit] |= BIT_ULL(layer_level); handled_accesses |= BIT_ULL(access_bit); @@ -347,6 +332,24 @@ init_layer_masks(const struct landlock_ruleset *const domain, return handled_accesses; } +static access_mask_t +get_handled_fs_accesses(const struct landlock_ruleset *const domain) +{ + /* Handles all initially denied by default access rights. */ + return get_raw_handled_fs_accesses(domain) | ACCESS_FS_INITIALLY_DENIED; +} + +static const struct landlock_ruleset *get_current_fs_domain(void) +{ + const struct landlock_ruleset *const dom = + landlock_get_current_domain(); + + if (!dom || !get_raw_handled_fs_accesses(dom)) + return NULL; + + return dom; +} + /* * Check that a destination file hierarchy has more restrictions than a source * file hierarchy. This is only used for link and rename actions. @@ -519,7 +522,7 @@ static bool is_access_to_paths_allowed( * a superset of the meaningful requested accesses). */ access_masked_parent1 = access_masked_parent2 = - get_handled_accesses(domain); + get_handled_fs_accesses(domain); is_dom_check = true; } else { if (WARN_ON_ONCE(dentry_child1 || dentry_child2)) @@ -648,11 +651,10 @@ static inline int check_access_path(const struct landlock_ruleset *const domain, return -EACCES; } -static inline int current_check_access_path(const struct path *const path, +static int current_check_access_path(const struct path *const path, const access_mask_t access_request) { - const struct landlock_ruleset *const dom = - landlock_get_current_domain(); + const struct landlock_ruleset *const dom = get_current_fs_domain(); if (!dom) return 0; @@ -815,8 +817,7 @@ static int current_check_refer_path(struct dentry *const old_dentry, struct dentry *const new_dentry, const bool removable, const bool exchange) { - const struct landlock_ruleset *const dom = - landlock_get_current_domain(); + const struct landlock_ruleset *const dom = get_current_fs_domain(); bool allow_parent1, allow_parent2; access_mask_t access_request_parent1, access_request_parent2; struct path mnt_dir; @@ -1050,7 +1051,7 @@ static int hook_sb_mount(const char *const dev_name, const struct path *const path, const char *const type, const unsigned long flags, void *const data) { - if (!landlock_get_current_domain()) + if (!get_current_fs_domain()) return 0; return -EPERM; } @@ -1058,7 +1059,7 @@ static int hook_sb_mount(const char *const dev_name, static int hook_move_mount(const struct path *const from_path, const struct path *const to_path) { - if (!landlock_get_current_domain()) + if (!get_current_fs_domain()) return 0; return -EPERM; } @@ -1069,14 +1070,14 @@ static int hook_move_mount(const struct path *const from_path, */ static int hook_sb_umount(struct vfsmount *const mnt, const int flags) { - if (!landlock_get_current_domain()) + if (!get_current_fs_domain()) return 0; return -EPERM; } static int hook_sb_remount(struct super_block *const sb, void *const mnt_opts) { - if (!landlock_get_current_domain()) + if (!get_current_fs_domain()) return 0; return -EPERM; } @@ -1092,7 +1093,7 @@ static int hook_sb_remount(struct super_block *const sb, void *const mnt_opts) static int hook_sb_pivotroot(const struct path *const old_path, const struct path *const new_path) { - if (!landlock_get_current_domain()) + if (!get_current_fs_domain()) return 0; return -EPERM; } @@ -1128,8 +1129,7 @@ static int hook_path_mknod(const struct path *const dir, struct dentry *const dentry, const umode_t mode, const unsigned int dev) { - const struct landlock_ruleset *const dom = - landlock_get_current_domain(); + const struct landlock_ruleset *const dom = get_current_fs_domain(); if (!dom) return 0; @@ -1208,8 +1208,7 @@ static int hook_file_open(struct file *const file) layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {}; access_mask_t open_access_request, full_access_request, allowed_access; const access_mask_t optional_access = LANDLOCK_ACCESS_FS_TRUNCATE; - const struct landlock_ruleset *const dom = - landlock_get_current_domain(); + const struct landlock_ruleset *const dom = get_current_fs_domain(); if (!dom) return 0; diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h index e900b84d915f..c6db528817c5 100644 --- a/security/landlock/ruleset.h +++ b/security/landlock/ruleset.h @@ -15,10 +15,21 @@ #include <linux/rbtree.h> #include <linux/refcount.h> #include <linux/workqueue.h> +#include <uapi/linux/landlock.h> #include "limits.h" #include "object.h" +/* + * All access rights that are denied by default whether they are handled or not + * by a ruleset/layer. This must be ORed with all ruleset->access_masks[] + * entries when we need to get the absolute handled access masks. + */ +/* clang-format off */ +#define ACCESS_FS_INITIALLY_DENIED ( \ + LANDLOCK_ACCESS_FS_REFER) +/* clang-format on */ + typedef u16 access_mask_t; /* Makes sure all filesystem access rights can be stored. */ static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS); @@ -196,11 +207,21 @@ landlock_add_fs_access_mask(struct landlock_ruleset *const ruleset, } static inline access_mask_t -landlock_get_fs_access_mask(const struct landlock_ruleset *const ruleset, - const u16 layer_level) +landlock_get_raw_fs_access_mask(const struct landlock_ruleset *const ruleset, + const u16 layer_level) { return (ruleset->access_masks[layer_level] >> LANDLOCK_SHIFT_ACCESS_FS) & LANDLOCK_MASK_ACCESS_FS; } + +static inline access_mask_t +landlock_get_fs_access_mask(const struct landlock_ruleset *const ruleset, + const u16 layer_level) +{ + /* Handles all initially denied by default access rights. */ + return landlock_get_raw_fs_access_mask(ruleset, layer_level) | + ACCESS_FS_INITIALLY_DENIED; +} + #endif /* _SECURITY_LANDLOCK_RULESET_H */ diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c index 71aca7f990bc..d35cd5d304db 100644 --- a/security/landlock/syscalls.c +++ b/security/landlock/syscalls.c @@ -310,6 +310,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd, struct path path; struct landlock_ruleset *ruleset; int res, err; + access_mask_t mask; if (!landlock_initialized) return -EOPNOTSUPP; @@ -348,9 +349,8 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd, * Checks that allowed_access matches the @ruleset constraints * (ruleset->access_masks[0] is automatically upgraded to 64-bits). */ - if ((path_beneath_attr.allowed_access | - landlock_get_fs_access_mask(ruleset, 0)) != - landlock_get_fs_access_mask(ruleset, 0)) { + mask = landlock_get_raw_fs_access_mask(ruleset, 0); + if ((path_beneath_attr.allowed_access | mask) != mask) { err = -EINVAL; goto out_put_ruleset; }