| Message ID | 20221021152644.155136-8-konstantin.meskhidze@huawei.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Network support for Landlock | expand |
On 21/10/2022 17:26, Konstantin Meskhidze wrote: > This commit adds network rules support in internal landlock functions > (presented in ruleset.c) and landlock_create_ruleset syscall. …in the ruleset management helpers and the landlock_create_ruleset syscall. > Refactors user space API to support network actions. Adds new network Refactor… > access flags, network rule and network attributes. Increments Landlock Increment… > ABI version. > > Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> > --- > > Changes since v7: > * Squashes commits. > * Increments ABI version to 4. > * Refactors commit message. > * Minor fixes. > > Changes since v6: > * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask() > because it OR values. > * Makes landlock_add_net_access_mask() more resilient incorrect values. > * Refactors landlock_get_net_access_mask(). > * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use > LANDLOCK_NUM_ACCESS_FS as value. > * Updates access_masks_t to u32 to support network access actions. > * Refactors landlock internal functions to support network actions with > landlock_key/key_type/id types. > > Changes since v5: > * Gets rid of partial revert from landlock_add_rule > syscall. > * Formats code with clang-format-14. > > Changes since v4: > * Refactors landlock_create_ruleset() - splits ruleset and > masks checks. > * Refactors landlock_create_ruleset() and landlock mask > setters/getters to support two rule types. > * Refactors landlock_add_rule syscall add_rule_path_beneath > function by factoring out get_ruleset_from_fd() and > landlock_put_ruleset(). > > Changes since v3: > * Splits commit. > * Adds network rule support for internal landlock functions. > * Adds set_mask and get_mask for network. > * Adds rb_root root_net_port. > > --- > include/uapi/linux/landlock.h | 49 ++++++++++++++ > security/landlock/limits.h | 6 +- > security/landlock/ruleset.c | 55 ++++++++++++++-- > security/landlock/ruleset.h | 68 ++++++++++++++++---- > security/landlock/syscalls.c | 13 +++- > tools/testing/selftests/landlock/base_test.c | 2 +- > 6 files changed, 170 insertions(+), 23 deletions(-) > > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h > index f3223f964691..096b683c6ff3 100644 > --- a/include/uapi/linux/landlock.h > +++ b/include/uapi/linux/landlock.h > @@ -31,6 +31,13 @@ struct landlock_ruleset_attr { > * this access right. > */ > __u64 handled_access_fs; > + > + /** > + * @handled_access_net: Bitmask of actions (cf. `Network flags`_) > + * that is handled by this ruleset and should then be forbidden if no > + * rule explicitly allow them. > + */ > + __u64 handled_access_net; > }; > > /* > @@ -54,6 +61,11 @@ enum landlock_rule_type { > * landlock_path_beneath_attr . > */ > LANDLOCK_RULE_PATH_BENEATH = 1, > + /** > + * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct > + * landlock_net_service_attr . > + */ > + LANDLOCK_RULE_NET_SERVICE = 2, > }; > > /** > @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr { > */ > } __attribute__((packed)); > > +/** > + * struct landlock_net_service_attr - TCP subnet definition > + * > + * Argument of sys_landlock_add_rule(). > + */ > +struct landlock_net_service_attr { > + /** > + * @allowed_access: Bitmask of allowed access network for services > + * (cf. `Network flags`_). > + */ > + __u64 allowed_access; > + /** > + * @port: Network port. > + */ > + __u16 port; From an UAPI point of view, I think the port field should be __be16, as for sockaddr_in->port and other network-related APIs. This will require some kernel changes to please sparse: make C=2 security/landlock/ must not print any warning. Using big-endian values as keys (casted to uintptr_t, not strictly __be16) in the rb-tree should not be an issue because there is no port range ordering (for now). A dedicated test should check that endianness is correct, e.g. by using different port encoding. This should include passing and failing tests, but they should work on all architectures (i.e. big or little endian). > + > +} __attribute__((packed)); > + > /** > * DOC: fs_access > * > @@ -173,4 +203,23 @@ struct landlock_path_beneath_attr { > #define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14) > /* clang-format on */ > > +/** > + * DOC: net_access > + * > + * Network flags > + * ~~~~~~~~~~~~~~~~ > + * > + * These flags enable to restrict a sandboxed process to a set of network > + * actions. > + * > + * TCP sockets with allowed actions: > + * > + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port. > + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to > + * a remote port. > + */ > +/* clang-format off */ > +#define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) > +#define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) > +/* clang-format on */ > #endif /* _UAPI_LINUX_LANDLOCK_H */ > diff --git a/security/landlock/limits.h b/security/landlock/limits.h > index bafb3b8dc677..8a1a6463c64e 100644 > --- a/security/landlock/limits.h > +++ b/security/landlock/limits.h > @@ -23,6 +23,10 @@ > #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS) > #define LANDLOCK_SHIFT_ACCESS_FS 0 > > -/* clang-format on */ > +#define LANDLOCK_LAST_ACCESS_NET LANDLOCK_ACCESS_NET_CONNECT_TCP > +#define LANDLOCK_MASK_ACCESS_NET ((LANDLOCK_LAST_ACCESS_NET << 1) - 1) > +#define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET) > +#define LANDLOCK_SHIFT_ACCESS_NET LANDLOCK_NUM_ACCESS_FS > > +/* clang-format on */ > #endif /* _SECURITY_LANDLOCK_LIMITS_H */ > diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c > index c7cf54ba4f6d..9277c1295114 100644 > --- a/security/landlock/ruleset.c > +++ b/security/landlock/ruleset.c > @@ -36,6 +36,9 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) > refcount_set(&new_ruleset->usage, 1); > mutex_init(&new_ruleset->lock); > new_ruleset->root_inode = RB_ROOT; > +#if IS_ENABLED(CONFIG_INET) > + new_ruleset->root_net_port = RB_ROOT; > +#endif /* IS_ENABLED(CONFIG_INET) */ > new_ruleset->num_layers = num_layers; > /* > * hierarchy = NULL > @@ -46,16 +49,21 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) > } > > struct landlock_ruleset * > -landlock_create_ruleset(const access_mask_t fs_access_mask) > +landlock_create_ruleset(const access_mask_t fs_access_mask, > + const access_mask_t net_access_mask) > { > struct landlock_ruleset *new_ruleset; > > /* Informs about useless ruleset. */ > - if (!fs_access_mask) > + if (!fs_access_mask && !net_access_mask) > return ERR_PTR(-ENOMSG); > new_ruleset = create_ruleset(1); > - if (!IS_ERR(new_ruleset)) > + if (IS_ERR(new_ruleset)) > + return new_ruleset; > + if (fs_access_mask) > landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0); > + if (net_access_mask) > + landlock_add_net_access_mask(new_ruleset, net_access_mask, 0); > return new_ruleset; > } > > @@ -73,6 +81,10 @@ static inline bool is_object_pointer(const enum landlock_key_type key_type) > switch (key_type) { > case LANDLOCK_KEY_INODE: > return true; > +#if IS_ENABLED(CONFIG_INET) > + case LANDLOCK_KEY_NET_PORT: > + return false; > +#endif /* IS_ENABLED(CONFIG_INET) */ > } > WARN_ON_ONCE(1); > return false; > @@ -126,6 +138,11 @@ static inline struct rb_root *get_root(struct landlock_ruleset *const ruleset, > case LANDLOCK_KEY_INODE: > root = &ruleset->root_inode; > break; > +#if IS_ENABLED(CONFIG_INET) > + case LANDLOCK_KEY_NET_PORT: > + root = &ruleset->root_net_port; > + break; > +#endif /* IS_ENABLED(CONFIG_INET) */ > } > if (WARN_ON_ONCE(!root)) > return ERR_PTR(-EINVAL); > @@ -154,7 +171,9 @@ static void build_check_ruleset(void) > BUILD_BUG_ON(ruleset.num_rules < LANDLOCK_MAX_NUM_RULES); > BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS); > BUILD_BUG_ON(access_masks < > - (LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS)); > + (LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS) + This is correct but because we are dealing with bitmasks I would prefer to use "|" instead of "+". > + (LANDLOCK_MASK_ACCESS_NET > + << LANDLOCK_SHIFT_ACCESS_NET)); > } > > /** > @@ -370,7 +389,12 @@ static int merge_ruleset(struct landlock_ruleset *const dst, > err = merge_tree(dst, src, LANDLOCK_KEY_INODE); > if (err) > goto out_unlock; > - Please keep this newline. > +#if IS_ENABLED(CONFIG_INET) > + /* Merges the @src network port tree. */ > + err = merge_tree(dst, src, LANDLOCK_KEY_NET_PORT); > + if (err) > + goto out_unlock; > +#endif /* IS_ENABLED(CONFIG_INET) */ > out_unlock: > mutex_unlock(&src->lock); > mutex_unlock(&dst->lock); > @@ -426,7 +450,12 @@ static int inherit_ruleset(struct landlock_ruleset *const parent, > err = inherit_tree(parent, child, LANDLOCK_KEY_INODE); > if (err) > goto out_unlock; > - newline > +#if IS_ENABLED(CONFIG_INET) > + /* Copies the @parent network port tree. */ > + err = inherit_tree(parent, child, LANDLOCK_KEY_NET_PORT); > + if (err) > + goto out_unlock; > +#endif /* IS_ENABLED(CONFIG_INET) */ > if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) { > err = -EINVAL; > goto out_unlock; > @@ -459,6 +488,11 @@ static void free_ruleset(struct landlock_ruleset *const ruleset) > rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode, > node) > free_rule(freeme, LANDLOCK_KEY_INODE); > +#if IS_ENABLED(CONFIG_INET) > + rbtree_postorder_for_each_entry_safe(freeme, next, > + &ruleset->root_net_port, node) > + free_rule(freeme, LANDLOCK_KEY_NET_PORT); > +#endif /* IS_ENABLED(CONFIG_INET) */ > put_hierarchy(ruleset->hierarchy); > kfree(ruleset); > } > @@ -637,6 +671,9 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset, > * Populates @layer_masks such that for each access right in @access_request, > * the bits for all the layers are set where this access right is handled. > * > + * @layer_masks must contain LANDLOCK_NUM_ACCESS_FS or LANDLOCK_NUM_ACCESS_NET > + * elements according to @key_type. Please include this sentence in the @layer_masks description below. > + * > * @domain: The domain that defines the current restrictions. > * @access_request: The requested access rights to check. > * @layer_masks: The layer masks to populate. "It must contain…" > @@ -659,6 +696,12 @@ access_mask_t init_layer_masks(const struct landlock_ruleset *const domain, > get_access_mask = landlock_get_fs_access_mask; > num_access = LANDLOCK_NUM_ACCESS_FS; > break; > +#if IS_ENABLED(CONFIG_INET) > + case LANDLOCK_KEY_NET_PORT: > + get_access_mask = landlock_get_net_access_mask; > + num_access = LANDLOCK_NUM_ACCESS_NET; > + break; > +#endif /* IS_ENABLED(CONFIG_INET) */ > default: > WARN_ON_ONCE(1); > return 0; > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h > index d9eb79ea9a89..f272d2cd518c 100644 > --- a/security/landlock/ruleset.h > +++ b/security/landlock/ruleset.h > @@ -19,16 +19,20 @@ > #include "limits.h" > #include "object.h" > > +/* Rule access mask. */ > 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); > +/* Makes sure all network access rights can be stored. */ > +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET); > /* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */ > static_assert(sizeof(unsigned long) >= sizeof(access_mask_t)); > > /* Ruleset access masks. */ > -typedef u16 access_masks_t; > +typedef u32 access_masks_t; This type change need to be explained in the commit message. > /* Makes sure all ruleset access rights can be stored. */ > -static_assert(BITS_PER_TYPE(access_masks_t) >= LANDLOCK_NUM_ACCESS_FS); > +static_assert(BITS_PER_TYPE(access_masks_t) >= > + LANDLOCK_NUM_ACCESS_FS + LANDLOCK_NUM_ACCESS_NET); > > typedef u16 layer_mask_t; > /* Makes sure all layers can be checked. */ > @@ -82,6 +86,13 @@ enum landlock_key_type { > * keys. > */ > LANDLOCK_KEY_INODE = 1, > +#if IS_ENABLED(CONFIG_INET) > + /** > + * @LANDLOCK_KEY_NET_PORT: Type of &landlock_ruleset.root_net_port's > + * node keys. > + */ > + LANDLOCK_KEY_NET_PORT = 2, > +#endif /* IS_ENABLED(CONFIG_INET) */ > }; > > /** > @@ -156,6 +167,15 @@ struct landlock_ruleset { > * reaches zero. > */ > struct rb_root root_inode; > +#if IS_ENABLED(CONFIG_INET) > + /** > + * @root_net_port: Root of a red-black tree containing &struct > + * landlock_rule nodes with network port. Once a ruleset is tied to a > + * process (i.e. as a domain), this tree is immutable until @usage > + * reaches zero. > + */ > + struct rb_root root_net_port; > +#endif /* IS_ENABLED(CONFIG_INET) */ > /** > * @hierarchy: Enables hierarchy identification even when a parent > * domain vanishes. This is needed for the ptrace protection. > @@ -166,8 +186,8 @@ struct landlock_ruleset { > * @work_free: Enables to free a ruleset within a lockless > * section. This is only used by > * landlock_put_ruleset_deferred() when @usage reaches zero. > - * The fields @lock, @usage, @num_rules, @num_layers and > - * @access_masks are then unused. > + * The fields @lock, @usage, @num_rules, @num_layers, > + * @net_access_mask and @access_masks are then unused. There is no net_access_mask anymore.
11/17/2022 9:43 PM, Mickaël Salaün пишет: > > On 21/10/2022 17:26, Konstantin Meskhidze wrote: >> This commit adds network rules support in internal landlock functions >> (presented in ruleset.c) and landlock_create_ruleset syscall. > > …in the ruleset management helpers and the landlock_create_ruleset syscall. > > >> Refactors user space API to support network actions. Adds new network > > Refactor… > >> access flags, network rule and network attributes. Increments Landlock > > Increment… The commit's message will be fixed. Thank you! > >> ABI version. >> >> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> >> --- >> >> Changes since v7: >> * Squashes commits. >> * Increments ABI version to 4. >> * Refactors commit message. >> * Minor fixes. >> >> Changes since v6: >> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask() >> because it OR values. >> * Makes landlock_add_net_access_mask() more resilient incorrect values. >> * Refactors landlock_get_net_access_mask(). >> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use >> LANDLOCK_NUM_ACCESS_FS as value. >> * Updates access_masks_t to u32 to support network access actions. >> * Refactors landlock internal functions to support network actions with >> landlock_key/key_type/id types. >> >> Changes since v5: >> * Gets rid of partial revert from landlock_add_rule >> syscall. >> * Formats code with clang-format-14. >> >> Changes since v4: >> * Refactors landlock_create_ruleset() - splits ruleset and >> masks checks. >> * Refactors landlock_create_ruleset() and landlock mask >> setters/getters to support two rule types. >> * Refactors landlock_add_rule syscall add_rule_path_beneath >> function by factoring out get_ruleset_from_fd() and >> landlock_put_ruleset(). >> >> Changes since v3: >> * Splits commit. >> * Adds network rule support for internal landlock functions. >> * Adds set_mask and get_mask for network. >> * Adds rb_root root_net_port. >> >> --- >> include/uapi/linux/landlock.h | 49 ++++++++++++++ >> security/landlock/limits.h | 6 +- >> security/landlock/ruleset.c | 55 ++++++++++++++-- >> security/landlock/ruleset.h | 68 ++++++++++++++++---- >> security/landlock/syscalls.c | 13 +++- >> tools/testing/selftests/landlock/base_test.c | 2 +- >> 6 files changed, 170 insertions(+), 23 deletions(-) >> >> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h >> index f3223f964691..096b683c6ff3 100644 >> --- a/include/uapi/linux/landlock.h >> +++ b/include/uapi/linux/landlock.h >> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr { >> * this access right. >> */ >> __u64 handled_access_fs; >> + >> + /** >> + * @handled_access_net: Bitmask of actions (cf. `Network flags`_) >> + * that is handled by this ruleset and should then be forbidden if no >> + * rule explicitly allow them. >> + */ >> + __u64 handled_access_net; >> }; >> >> /* >> @@ -54,6 +61,11 @@ enum landlock_rule_type { >> * landlock_path_beneath_attr . >> */ >> LANDLOCK_RULE_PATH_BENEATH = 1, >> + /** >> + * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct >> + * landlock_net_service_attr . >> + */ >> + LANDLOCK_RULE_NET_SERVICE = 2, >> }; >> >> /** >> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr { >> */ >> } __attribute__((packed)); >> >> +/** >> + * struct landlock_net_service_attr - TCP subnet definition >> + * >> + * Argument of sys_landlock_add_rule(). >> + */ >> +struct landlock_net_service_attr { >> + /** >> + * @allowed_access: Bitmask of allowed access network for services >> + * (cf. `Network flags`_). >> + */ >> + __u64 allowed_access; >> + /** >> + * @port: Network port. >> + */ >> + __u16 port; > > From an UAPI point of view, I think the port field should be __be16, as > for sockaddr_in->port and other network-related APIs. This will require > some kernel changes to please sparse: make C=2 security/landlock/ must > not print any warning. Is sparse a default checker? > > Using big-endian values as keys (casted to uintptr_t, not strictly > __be16) in the rb-tree should not be an issue because there is no port > range ordering (for now). > > A dedicated test should check that endianness is correct, e.g. by using > different port encoding. This should include passing and failing tests, > but they should work on all architectures (i.e. big or little endian). > I got it. Will be fixed. > >> + >> +} __attribute__((packed)); >> + >> /** >> * DOC: fs_access >> * >> @@ -173,4 +203,23 @@ struct landlock_path_beneath_attr { >> #define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14) >> /* clang-format on */ >> >> +/** >> + * DOC: net_access >> + * >> + * Network flags >> + * ~~~~~~~~~~~~~~~~ >> + * >> + * These flags enable to restrict a sandboxed process to a set of network >> + * actions. >> + * >> + * TCP sockets with allowed actions: >> + * >> + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port. >> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to >> + * a remote port. >> + */ >> +/* clang-format off */ >> +#define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) >> +#define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) >> +/* clang-format on */ >> #endif /* _UAPI_LINUX_LANDLOCK_H */ >> diff --git a/security/landlock/limits.h b/security/landlock/limits.h >> index bafb3b8dc677..8a1a6463c64e 100644 >> --- a/security/landlock/limits.h >> +++ b/security/landlock/limits.h >> @@ -23,6 +23,10 @@ >> #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS) >> #define LANDLOCK_SHIFT_ACCESS_FS 0 >> >> -/* clang-format on */ >> +#define LANDLOCK_LAST_ACCESS_NET LANDLOCK_ACCESS_NET_CONNECT_TCP >> +#define LANDLOCK_MASK_ACCESS_NET ((LANDLOCK_LAST_ACCESS_NET << 1) - 1) >> +#define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET) >> +#define LANDLOCK_SHIFT_ACCESS_NET LANDLOCK_NUM_ACCESS_FS >> >> +/* clang-format on */ >> #endif /* _SECURITY_LANDLOCK_LIMITS_H */ >> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c >> index c7cf54ba4f6d..9277c1295114 100644 >> --- a/security/landlock/ruleset.c >> +++ b/security/landlock/ruleset.c >> @@ -36,6 +36,9 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) >> refcount_set(&new_ruleset->usage, 1); >> mutex_init(&new_ruleset->lock); >> new_ruleset->root_inode = RB_ROOT; >> +#if IS_ENABLED(CONFIG_INET) >> + new_ruleset->root_net_port = RB_ROOT; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> new_ruleset->num_layers = num_layers; >> /* >> * hierarchy = NULL >> @@ -46,16 +49,21 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) >> } >> >> struct landlock_ruleset * >> -landlock_create_ruleset(const access_mask_t fs_access_mask) >> +landlock_create_ruleset(const access_mask_t fs_access_mask, >> + const access_mask_t net_access_mask) >> { >> struct landlock_ruleset *new_ruleset; >> >> /* Informs about useless ruleset. */ >> - if (!fs_access_mask) >> + if (!fs_access_mask && !net_access_mask) >> return ERR_PTR(-ENOMSG); >> new_ruleset = create_ruleset(1); >> - if (!IS_ERR(new_ruleset)) >> + if (IS_ERR(new_ruleset)) >> + return new_ruleset; >> + if (fs_access_mask) >> landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0); >> + if (net_access_mask) >> + landlock_add_net_access_mask(new_ruleset, net_access_mask, 0); >> return new_ruleset; >> } >> >> @@ -73,6 +81,10 @@ static inline bool is_object_pointer(const enum landlock_key_type key_type) >> switch (key_type) { >> case LANDLOCK_KEY_INODE: >> return true; >> +#if IS_ENABLED(CONFIG_INET) >> + case LANDLOCK_KEY_NET_PORT: >> + return false; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> } >> WARN_ON_ONCE(1); >> return false; >> @@ -126,6 +138,11 @@ static inline struct rb_root *get_root(struct landlock_ruleset *const ruleset, >> case LANDLOCK_KEY_INODE: >> root = &ruleset->root_inode; >> break; >> +#if IS_ENABLED(CONFIG_INET) >> + case LANDLOCK_KEY_NET_PORT: >> + root = &ruleset->root_net_port; >> + break; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> } >> if (WARN_ON_ONCE(!root)) >> return ERR_PTR(-EINVAL); >> @@ -154,7 +171,9 @@ static void build_check_ruleset(void) >> BUILD_BUG_ON(ruleset.num_rules < LANDLOCK_MAX_NUM_RULES); >> BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS); >> BUILD_BUG_ON(access_masks < >> - (LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS)); >> + (LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS) + > > This is correct but because we are dealing with bitmasks I would prefer > to use "|" instead of "+". Ok. I will refactor it. > > >> + (LANDLOCK_MASK_ACCESS_NET >> + << LANDLOCK_SHIFT_ACCESS_NET)); >> } >> >> /** >> @@ -370,7 +389,12 @@ static int merge_ruleset(struct landlock_ruleset *const dst, >> err = merge_tree(dst, src, LANDLOCK_KEY_INODE); >> if (err) >> goto out_unlock; >> - > > Please keep this newline. Got it. > >> +#if IS_ENABLED(CONFIG_INET) >> + /* Merges the @src network port tree. */ >> + err = merge_tree(dst, src, LANDLOCK_KEY_NET_PORT); >> + if (err) >> + goto out_unlock; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> out_unlock: >> mutex_unlock(&src->lock); >> mutex_unlock(&dst->lock); >> @@ -426,7 +450,12 @@ static int inherit_ruleset(struct landlock_ruleset *const parent, >> err = inherit_tree(parent, child, LANDLOCK_KEY_INODE); >> if (err) >> goto out_unlock; >> - > > newline Ok. > >> +#if IS_ENABLED(CONFIG_INET) >> + /* Copies the @parent network port tree. */ >> + err = inherit_tree(parent, child, LANDLOCK_KEY_NET_PORT); >> + if (err) >> + goto out_unlock; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) { >> err = -EINVAL; >> goto out_unlock; >> @@ -459,6 +488,11 @@ static void free_ruleset(struct landlock_ruleset *const ruleset) >> rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode, >> node) >> free_rule(freeme, LANDLOCK_KEY_INODE); >> +#if IS_ENABLED(CONFIG_INET) >> + rbtree_postorder_for_each_entry_safe(freeme, next, >> + &ruleset->root_net_port, node) >> + free_rule(freeme, LANDLOCK_KEY_NET_PORT); >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> put_hierarchy(ruleset->hierarchy); >> kfree(ruleset); >> } >> @@ -637,6 +671,9 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset, >> * Populates @layer_masks such that for each access right in @access_request, >> * the bits for all the layers are set where this access right is handled. >> * >> + * @layer_masks must contain LANDLOCK_NUM_ACCESS_FS or LANDLOCK_NUM_ACCESS_NET >> + * elements according to @key_type. > > Please include this sentence in the @layer_masks description below. Ok. > >> + * >> * @domain: The domain that defines the current restrictions. >> * @access_request: The requested access rights to check. >> * @layer_masks: The layer masks to populate. > > "It must contain…" > > >> @@ -659,6 +696,12 @@ access_mask_t init_layer_masks(const struct landlock_ruleset *const domain, >> get_access_mask = landlock_get_fs_access_mask; >> num_access = LANDLOCK_NUM_ACCESS_FS; >> break; >> +#if IS_ENABLED(CONFIG_INET) >> + case LANDLOCK_KEY_NET_PORT: >> + get_access_mask = landlock_get_net_access_mask; >> + num_access = LANDLOCK_NUM_ACCESS_NET; >> + break; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> default: >> WARN_ON_ONCE(1); >> return 0; >> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h >> index d9eb79ea9a89..f272d2cd518c 100644 >> --- a/security/landlock/ruleset.h >> +++ b/security/landlock/ruleset.h >> @@ -19,16 +19,20 @@ >> #include "limits.h" >> #include "object.h" >> >> +/* Rule access mask. */ >> 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); >> +/* Makes sure all network access rights can be stored. */ >> +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET); >> /* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */ >> static_assert(sizeof(unsigned long) >= sizeof(access_mask_t)); >> >> /* Ruleset access masks. */ >> -typedef u16 access_masks_t; >> +typedef u32 access_masks_t; > > This type change need to be explained in the commit message. > Ok. I will explain it. > >> /* Makes sure all ruleset access rights can be stored. */ >> -static_assert(BITS_PER_TYPE(access_masks_t) >= LANDLOCK_NUM_ACCESS_FS); >> +static_assert(BITS_PER_TYPE(access_masks_t) >= >> + LANDLOCK_NUM_ACCESS_FS + LANDLOCK_NUM_ACCESS_NET); >> >> typedef u16 layer_mask_t; >> /* Makes sure all layers can be checked. */ >> @@ -82,6 +86,13 @@ enum landlock_key_type { >> * keys. >> */ >> LANDLOCK_KEY_INODE = 1, >> +#if IS_ENABLED(CONFIG_INET) >> + /** >> + * @LANDLOCK_KEY_NET_PORT: Type of &landlock_ruleset.root_net_port's >> + * node keys. >> + */ >> + LANDLOCK_KEY_NET_PORT = 2, >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> }; >> >> /** >> @@ -156,6 +167,15 @@ struct landlock_ruleset { >> * reaches zero. >> */ >> struct rb_root root_inode; >> +#if IS_ENABLED(CONFIG_INET) >> + /** >> + * @root_net_port: Root of a red-black tree containing &struct >> + * landlock_rule nodes with network port. Once a ruleset is tied to a >> + * process (i.e. as a domain), this tree is immutable until @usage >> + * reaches zero. >> + */ >> + struct rb_root root_net_port; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> /** >> * @hierarchy: Enables hierarchy identification even when a parent >> * domain vanishes. This is needed for the ptrace protection. >> @@ -166,8 +186,8 @@ struct landlock_ruleset { >> * @work_free: Enables to free a ruleset within a lockless >> * section. This is only used by >> * landlock_put_ruleset_deferred() when @usage reaches zero. >> - * The fields @lock, @usage, @num_rules, @num_layers and >> - * @access_masks are then unused. >> + * The fields @lock, @usage, @num_rules, @num_layers, >> + * @net_access_mask and @access_masks are then unused. > > There is no net_access_mask anymore. > .
On 28/11/2022 05:01, Konstantin Meskhidze (A) wrote: > > > 11/17/2022 9:43 PM, Mickaël Salaün пишет: >> >> On 21/10/2022 17:26, Konstantin Meskhidze wrote: >>> This commit adds network rules support in internal landlock functions >>> (presented in ruleset.c) and landlock_create_ruleset syscall. >> >> …in the ruleset management helpers and the landlock_create_ruleset syscall. >> >> >>> Refactors user space API to support network actions. Adds new network >> >> Refactor… >> >>> access flags, network rule and network attributes. Increments Landlock >> >> Increment… > > The commit's message will be fixed. Thank you! >> >>> ABI version. >>> >>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> >>> --- >>> >>> Changes since v7: >>> * Squashes commits. >>> * Increments ABI version to 4. >>> * Refactors commit message. >>> * Minor fixes. >>> >>> Changes since v6: >>> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask() >>> because it OR values. >>> * Makes landlock_add_net_access_mask() more resilient incorrect values. >>> * Refactors landlock_get_net_access_mask(). >>> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use >>> LANDLOCK_NUM_ACCESS_FS as value. >>> * Updates access_masks_t to u32 to support network access actions. >>> * Refactors landlock internal functions to support network actions with >>> landlock_key/key_type/id types. >>> >>> Changes since v5: >>> * Gets rid of partial revert from landlock_add_rule >>> syscall. >>> * Formats code with clang-format-14. >>> >>> Changes since v4: >>> * Refactors landlock_create_ruleset() - splits ruleset and >>> masks checks. >>> * Refactors landlock_create_ruleset() and landlock mask >>> setters/getters to support two rule types. >>> * Refactors landlock_add_rule syscall add_rule_path_beneath >>> function by factoring out get_ruleset_from_fd() and >>> landlock_put_ruleset(). >>> >>> Changes since v3: >>> * Splits commit. >>> * Adds network rule support for internal landlock functions. >>> * Adds set_mask and get_mask for network. >>> * Adds rb_root root_net_port. >>> >>> --- >>> include/uapi/linux/landlock.h | 49 ++++++++++++++ >>> security/landlock/limits.h | 6 +- >>> security/landlock/ruleset.c | 55 ++++++++++++++-- >>> security/landlock/ruleset.h | 68 ++++++++++++++++---- >>> security/landlock/syscalls.c | 13 +++- >>> tools/testing/selftests/landlock/base_test.c | 2 +- >>> 6 files changed, 170 insertions(+), 23 deletions(-) >>> >>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h >>> index f3223f964691..096b683c6ff3 100644 >>> --- a/include/uapi/linux/landlock.h >>> +++ b/include/uapi/linux/landlock.h >>> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr { >>> * this access right. >>> */ >>> __u64 handled_access_fs; >>> + >>> + /** >>> + * @handled_access_net: Bitmask of actions (cf. `Network flags`_) >>> + * that is handled by this ruleset and should then be forbidden if no >>> + * rule explicitly allow them. >>> + */ >>> + __u64 handled_access_net; >>> }; >>> >>> /* >>> @@ -54,6 +61,11 @@ enum landlock_rule_type { >>> * landlock_path_beneath_attr . >>> */ >>> LANDLOCK_RULE_PATH_BENEATH = 1, >>> + /** >>> + * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct >>> + * landlock_net_service_attr . >>> + */ >>> + LANDLOCK_RULE_NET_SERVICE = 2, >>> }; >>> >>> /** >>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr { >>> */ >>> } __attribute__((packed)); >>> >>> +/** >>> + * struct landlock_net_service_attr - TCP subnet definition >>> + * >>> + * Argument of sys_landlock_add_rule(). >>> + */ >>> +struct landlock_net_service_attr { >>> + /** >>> + * @allowed_access: Bitmask of allowed access network for services >>> + * (cf. `Network flags`_). >>> + */ >>> + __u64 allowed_access; >>> + /** >>> + * @port: Network port. >>> + */ >>> + __u16 port; >> >> From an UAPI point of view, I think the port field should be __be16, as >> for sockaddr_in->port and other network-related APIs. This will require >> some kernel changes to please sparse: make C=2 security/landlock/ must >> not print any warning. > > Is sparse a default checker? You should be able to easily install it with your Linux distro.
11/28/2022 11:26 PM, Mickaël Salaün пишет: > > On 28/11/2022 05:01, Konstantin Meskhidze (A) wrote: >> >> >> 11/17/2022 9:43 PM, Mickaël Salaün пишет: >>> >>> On 21/10/2022 17:26, Konstantin Meskhidze wrote: >>>> This commit adds network rules support in internal landlock functions >>>> (presented in ruleset.c) and landlock_create_ruleset syscall. >>> >>> …in the ruleset management helpers and the landlock_create_ruleset syscall. >>> >>> >>>> Refactors user space API to support network actions. Adds new network >>> >>> Refactor… >>> >>>> access flags, network rule and network attributes. Increments Landlock >>> >>> Increment… >> >> The commit's message will be fixed. Thank you! >>> >>>> ABI version. >>>> >>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> >>>> --- >>>> >>>> Changes since v7: >>>> * Squashes commits. >>>> * Increments ABI version to 4. >>>> * Refactors commit message. >>>> * Minor fixes. >>>> >>>> Changes since v6: >>>> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask() >>>> because it OR values. >>>> * Makes landlock_add_net_access_mask() more resilient incorrect values. >>>> * Refactors landlock_get_net_access_mask(). >>>> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use >>>> LANDLOCK_NUM_ACCESS_FS as value. >>>> * Updates access_masks_t to u32 to support network access actions. >>>> * Refactors landlock internal functions to support network actions with >>>> landlock_key/key_type/id types. >>>> >>>> Changes since v5: >>>> * Gets rid of partial revert from landlock_add_rule >>>> syscall. >>>> * Formats code with clang-format-14. >>>> >>>> Changes since v4: >>>> * Refactors landlock_create_ruleset() - splits ruleset and >>>> masks checks. >>>> * Refactors landlock_create_ruleset() and landlock mask >>>> setters/getters to support two rule types. >>>> * Refactors landlock_add_rule syscall add_rule_path_beneath >>>> function by factoring out get_ruleset_from_fd() and >>>> landlock_put_ruleset(). >>>> >>>> Changes since v3: >>>> * Splits commit. >>>> * Adds network rule support for internal landlock functions. >>>> * Adds set_mask and get_mask for network. >>>> * Adds rb_root root_net_port. >>>> >>>> --- >>>> include/uapi/linux/landlock.h | 49 ++++++++++++++ >>>> security/landlock/limits.h | 6 +- >>>> security/landlock/ruleset.c | 55 ++++++++++++++-- >>>> security/landlock/ruleset.h | 68 ++++++++++++++++---- >>>> security/landlock/syscalls.c | 13 +++- >>>> tools/testing/selftests/landlock/base_test.c | 2 +- >>>> 6 files changed, 170 insertions(+), 23 deletions(-) >>>> >>>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h >>>> index f3223f964691..096b683c6ff3 100644 >>>> --- a/include/uapi/linux/landlock.h >>>> +++ b/include/uapi/linux/landlock.h >>>> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr { >>>> * this access right. >>>> */ >>>> __u64 handled_access_fs; >>>> + >>>> + /** >>>> + * @handled_access_net: Bitmask of actions (cf. `Network flags`_) >>>> + * that is handled by this ruleset and should then be forbidden if no >>>> + * rule explicitly allow them. >>>> + */ >>>> + __u64 handled_access_net; >>>> }; >>>> >>>> /* >>>> @@ -54,6 +61,11 @@ enum landlock_rule_type { >>>> * landlock_path_beneath_attr . >>>> */ >>>> LANDLOCK_RULE_PATH_BENEATH = 1, >>>> + /** >>>> + * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct >>>> + * landlock_net_service_attr . >>>> + */ >>>> + LANDLOCK_RULE_NET_SERVICE = 2, >>>> }; >>>> >>>> /** >>>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr { >>>> */ >>>> } __attribute__((packed)); >>>> >>>> +/** >>>> + * struct landlock_net_service_attr - TCP subnet definition >>>> + * >>>> + * Argument of sys_landlock_add_rule(). >>>> + */ >>>> +struct landlock_net_service_attr { >>>> + /** >>>> + * @allowed_access: Bitmask of allowed access network for services >>>> + * (cf. `Network flags`_). >>>> + */ >>>> + __u64 allowed_access; >>>> + /** >>>> + * @port: Network port. >>>> + */ >>>> + __u16 port; >>> >>> From an UAPI point of view, I think the port field should be __be16, as >>> for sockaddr_in->port and other network-related APIs. This will require >>> some kernel changes to please sparse: make C=2 security/landlock/ must >>> not print any warning. >> >> Is sparse a default checker? > > You should be able to easily install it with your Linux distro. Ok. Thank you. > .
11/17/2022 9:43 PM, Mickaël Salaün пишет: > > On 21/10/2022 17:26, Konstantin Meskhidze wrote: >> This commit adds network rules support in internal landlock functions >> (presented in ruleset.c) and landlock_create_ruleset syscall. > > …in the ruleset management helpers and the landlock_create_ruleset syscall. > > >> Refactors user space API to support network actions. Adds new network > > Refactor… > >> access flags, network rule and network attributes. Increments Landlock > > Increment… > >> ABI version. >> >> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> >> --- >> >> Changes since v7: >> * Squashes commits. >> * Increments ABI version to 4. >> * Refactors commit message. >> * Minor fixes. >> >> Changes since v6: >> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask() >> because it OR values. >> * Makes landlock_add_net_access_mask() more resilient incorrect values. >> * Refactors landlock_get_net_access_mask(). >> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use >> LANDLOCK_NUM_ACCESS_FS as value. >> * Updates access_masks_t to u32 to support network access actions. >> * Refactors landlock internal functions to support network actions with >> landlock_key/key_type/id types. >> >> Changes since v5: >> * Gets rid of partial revert from landlock_add_rule >> syscall. >> * Formats code with clang-format-14. >> >> Changes since v4: >> * Refactors landlock_create_ruleset() - splits ruleset and >> masks checks. >> * Refactors landlock_create_ruleset() and landlock mask >> setters/getters to support two rule types. >> * Refactors landlock_add_rule syscall add_rule_path_beneath >> function by factoring out get_ruleset_from_fd() and >> landlock_put_ruleset(). >> >> Changes since v3: >> * Splits commit. >> * Adds network rule support for internal landlock functions. >> * Adds set_mask and get_mask for network. >> * Adds rb_root root_net_port. >> >> --- >> include/uapi/linux/landlock.h | 49 ++++++++++++++ >> security/landlock/limits.h | 6 +- >> security/landlock/ruleset.c | 55 ++++++++++++++-- >> security/landlock/ruleset.h | 68 ++++++++++++++++---- >> security/landlock/syscalls.c | 13 +++- >> tools/testing/selftests/landlock/base_test.c | 2 +- >> 6 files changed, 170 insertions(+), 23 deletions(-) >> >> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h >> index f3223f964691..096b683c6ff3 100644 >> --- a/include/uapi/linux/landlock.h >> +++ b/include/uapi/linux/landlock.h >> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr { >> * this access right. >> */ >> __u64 handled_access_fs; >> + >> + /** >> + * @handled_access_net: Bitmask of actions (cf. `Network flags`_) >> + * that is handled by this ruleset and should then be forbidden if no >> + * rule explicitly allow them. >> + */ >> + __u64 handled_access_net; >> }; >> >> /* >> @@ -54,6 +61,11 @@ enum landlock_rule_type { >> * landlock_path_beneath_attr . >> */ >> LANDLOCK_RULE_PATH_BENEATH = 1, >> + /** >> + * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct >> + * landlock_net_service_attr . >> + */ >> + LANDLOCK_RULE_NET_SERVICE = 2, >> }; >> >> /** >> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr { >> */ >> } __attribute__((packed)); >> >> +/** >> + * struct landlock_net_service_attr - TCP subnet definition >> + * >> + * Argument of sys_landlock_add_rule(). >> + */ >> +struct landlock_net_service_attr { >> + /** >> + * @allowed_access: Bitmask of allowed access network for services >> + * (cf. `Network flags`_). >> + */ >> + __u64 allowed_access; >> + /** >> + * @port: Network port. >> + */ >> + __u16 port; > > From an UAPI point of view, I think the port field should be __be16, as > for sockaddr_in->port and other network-related APIs. This will require > some kernel changes to please sparse: make C=2 security/landlock/ must > not print any warning. > > Using big-endian values as keys (casted to uintptr_t, not strictly > __be16) in the rb-tree should not be an issue because there is no port > range ordering (for now). > > A dedicated test should check that endianness is correct, e.g. by using > different port encoding. This should include passing and failing tests, > but they should work on all architectures (i.e. big or little endian). > Hi Mickaёl. Could you please give me a piece of advice about these kind of tests? I have not entirely understood this point. > >> + >> +} __attribute__((packed)); >> + >> /** >> * DOC: fs_access >> * >> @@ -173,4 +203,23 @@ struct landlock_path_beneath_attr { >> #define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14) >> /* clang-format on */ >> >> +/** >> + * DOC: net_access >> + * >> + * Network flags >> + * ~~~~~~~~~~~~~~~~ >> + * >> + * These flags enable to restrict a sandboxed process to a set of network >> + * actions. >> + * >> + * TCP sockets with allowed actions: >> + * >> + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port. >> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to >> + * a remote port. >> + */ >> +/* clang-format off */ >> +#define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) >> +#define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) >> +/* clang-format on */ >> #endif /* _UAPI_LINUX_LANDLOCK_H */ >> diff --git a/security/landlock/limits.h b/security/landlock/limits.h >> index bafb3b8dc677..8a1a6463c64e 100644 >> --- a/security/landlock/limits.h >> +++ b/security/landlock/limits.h >> @@ -23,6 +23,10 @@ >> #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS) >> #define LANDLOCK_SHIFT_ACCESS_FS 0 >> >> -/* clang-format on */ >> +#define LANDLOCK_LAST_ACCESS_NET LANDLOCK_ACCESS_NET_CONNECT_TCP >> +#define LANDLOCK_MASK_ACCESS_NET ((LANDLOCK_LAST_ACCESS_NET << 1) - 1) >> +#define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET) >> +#define LANDLOCK_SHIFT_ACCESS_NET LANDLOCK_NUM_ACCESS_FS >> >> +/* clang-format on */ >> #endif /* _SECURITY_LANDLOCK_LIMITS_H */ >> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c >> index c7cf54ba4f6d..9277c1295114 100644 >> --- a/security/landlock/ruleset.c >> +++ b/security/landlock/ruleset.c >> @@ -36,6 +36,9 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) >> refcount_set(&new_ruleset->usage, 1); >> mutex_init(&new_ruleset->lock); >> new_ruleset->root_inode = RB_ROOT; >> +#if IS_ENABLED(CONFIG_INET) >> + new_ruleset->root_net_port = RB_ROOT; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> new_ruleset->num_layers = num_layers; >> /* >> * hierarchy = NULL >> @@ -46,16 +49,21 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) >> } >> >> struct landlock_ruleset * >> -landlock_create_ruleset(const access_mask_t fs_access_mask) >> +landlock_create_ruleset(const access_mask_t fs_access_mask, >> + const access_mask_t net_access_mask) >> { >> struct landlock_ruleset *new_ruleset; >> >> /* Informs about useless ruleset. */ >> - if (!fs_access_mask) >> + if (!fs_access_mask && !net_access_mask) >> return ERR_PTR(-ENOMSG); >> new_ruleset = create_ruleset(1); >> - if (!IS_ERR(new_ruleset)) >> + if (IS_ERR(new_ruleset)) >> + return new_ruleset; >> + if (fs_access_mask) >> landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0); >> + if (net_access_mask) >> + landlock_add_net_access_mask(new_ruleset, net_access_mask, 0); >> return new_ruleset; >> } >> >> @@ -73,6 +81,10 @@ static inline bool is_object_pointer(const enum landlock_key_type key_type) >> switch (key_type) { >> case LANDLOCK_KEY_INODE: >> return true; >> +#if IS_ENABLED(CONFIG_INET) >> + case LANDLOCK_KEY_NET_PORT: >> + return false; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> } >> WARN_ON_ONCE(1); >> return false; >> @@ -126,6 +138,11 @@ static inline struct rb_root *get_root(struct landlock_ruleset *const ruleset, >> case LANDLOCK_KEY_INODE: >> root = &ruleset->root_inode; >> break; >> +#if IS_ENABLED(CONFIG_INET) >> + case LANDLOCK_KEY_NET_PORT: >> + root = &ruleset->root_net_port; >> + break; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> } >> if (WARN_ON_ONCE(!root)) >> return ERR_PTR(-EINVAL); >> @@ -154,7 +171,9 @@ static void build_check_ruleset(void) >> BUILD_BUG_ON(ruleset.num_rules < LANDLOCK_MAX_NUM_RULES); >> BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS); >> BUILD_BUG_ON(access_masks < >> - (LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS)); >> + (LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS) + > > This is correct but because we are dealing with bitmasks I would prefer > to use "|" instead of "+". > > >> + (LANDLOCK_MASK_ACCESS_NET >> + << LANDLOCK_SHIFT_ACCESS_NET)); >> } >> >> /** >> @@ -370,7 +389,12 @@ static int merge_ruleset(struct landlock_ruleset *const dst, >> err = merge_tree(dst, src, LANDLOCK_KEY_INODE); >> if (err) >> goto out_unlock; >> - > > Please keep this newline. > >> +#if IS_ENABLED(CONFIG_INET) >> + /* Merges the @src network port tree. */ >> + err = merge_tree(dst, src, LANDLOCK_KEY_NET_PORT); >> + if (err) >> + goto out_unlock; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> out_unlock: >> mutex_unlock(&src->lock); >> mutex_unlock(&dst->lock); >> @@ -426,7 +450,12 @@ static int inherit_ruleset(struct landlock_ruleset *const parent, >> err = inherit_tree(parent, child, LANDLOCK_KEY_INODE); >> if (err) >> goto out_unlock; >> - > > newline > >> +#if IS_ENABLED(CONFIG_INET) >> + /* Copies the @parent network port tree. */ >> + err = inherit_tree(parent, child, LANDLOCK_KEY_NET_PORT); >> + if (err) >> + goto out_unlock; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) { >> err = -EINVAL; >> goto out_unlock; >> @@ -459,6 +488,11 @@ static void free_ruleset(struct landlock_ruleset *const ruleset) >> rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode, >> node) >> free_rule(freeme, LANDLOCK_KEY_INODE); >> +#if IS_ENABLED(CONFIG_INET) >> + rbtree_postorder_for_each_entry_safe(freeme, next, >> + &ruleset->root_net_port, node) >> + free_rule(freeme, LANDLOCK_KEY_NET_PORT); >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> put_hierarchy(ruleset->hierarchy); >> kfree(ruleset); >> } >> @@ -637,6 +671,9 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset, >> * Populates @layer_masks such that for each access right in @access_request, >> * the bits for all the layers are set where this access right is handled. >> * >> + * @layer_masks must contain LANDLOCK_NUM_ACCESS_FS or LANDLOCK_NUM_ACCESS_NET >> + * elements according to @key_type. > > Please include this sentence in the @layer_masks description below. > >> + * >> * @domain: The domain that defines the current restrictions. >> * @access_request: The requested access rights to check. >> * @layer_masks: The layer masks to populate. > > "It must contain…" > > >> @@ -659,6 +696,12 @@ access_mask_t init_layer_masks(const struct landlock_ruleset *const domain, >> get_access_mask = landlock_get_fs_access_mask; >> num_access = LANDLOCK_NUM_ACCESS_FS; >> break; >> +#if IS_ENABLED(CONFIG_INET) >> + case LANDLOCK_KEY_NET_PORT: >> + get_access_mask = landlock_get_net_access_mask; >> + num_access = LANDLOCK_NUM_ACCESS_NET; >> + break; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> default: >> WARN_ON_ONCE(1); >> return 0; >> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h >> index d9eb79ea9a89..f272d2cd518c 100644 >> --- a/security/landlock/ruleset.h >> +++ b/security/landlock/ruleset.h >> @@ -19,16 +19,20 @@ >> #include "limits.h" >> #include "object.h" >> >> +/* Rule access mask. */ >> 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); >> +/* Makes sure all network access rights can be stored. */ >> +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET); >> /* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */ >> static_assert(sizeof(unsigned long) >= sizeof(access_mask_t)); >> >> /* Ruleset access masks. */ >> -typedef u16 access_masks_t; >> +typedef u32 access_masks_t; > > This type change need to be explained in the commit message. > > >> /* Makes sure all ruleset access rights can be stored. */ >> -static_assert(BITS_PER_TYPE(access_masks_t) >= LANDLOCK_NUM_ACCESS_FS); >> +static_assert(BITS_PER_TYPE(access_masks_t) >= >> + LANDLOCK_NUM_ACCESS_FS + LANDLOCK_NUM_ACCESS_NET); >> >> typedef u16 layer_mask_t; >> /* Makes sure all layers can be checked. */ >> @@ -82,6 +86,13 @@ enum landlock_key_type { >> * keys. >> */ >> LANDLOCK_KEY_INODE = 1, >> +#if IS_ENABLED(CONFIG_INET) >> + /** >> + * @LANDLOCK_KEY_NET_PORT: Type of &landlock_ruleset.root_net_port's >> + * node keys. >> + */ >> + LANDLOCK_KEY_NET_PORT = 2, >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> }; >> >> /** >> @@ -156,6 +167,15 @@ struct landlock_ruleset { >> * reaches zero. >> */ >> struct rb_root root_inode; >> +#if IS_ENABLED(CONFIG_INET) >> + /** >> + * @root_net_port: Root of a red-black tree containing &struct >> + * landlock_rule nodes with network port. Once a ruleset is tied to a >> + * process (i.e. as a domain), this tree is immutable until @usage >> + * reaches zero. >> + */ >> + struct rb_root root_net_port; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> /** >> * @hierarchy: Enables hierarchy identification even when a parent >> * domain vanishes. This is needed for the ptrace protection. >> @@ -166,8 +186,8 @@ struct landlock_ruleset { >> * @work_free: Enables to free a ruleset within a lockless >> * section. This is only used by >> * landlock_put_ruleset_deferred() when @usage reaches zero. >> - * The fields @lock, @usage, @num_rules, @num_layers and >> - * @access_masks are then unused. >> + * The fields @lock, @usage, @num_rules, @num_layers, >> + * @net_access_mask and @access_masks are then unused. > > There is no net_access_mask anymore. > .
11/17/2022 9:43 PM, Mickaël Salaün пишет: > > On 21/10/2022 17:26, Konstantin Meskhidze wrote: >> This commit adds network rules support in internal landlock functions >> (presented in ruleset.c) and landlock_create_ruleset syscall. > > …in the ruleset management helpers and the landlock_create_ruleset syscall. > > >> Refactors user space API to support network actions. Adds new network > > Refactor… > >> access flags, network rule and network attributes. Increments Landlock > > Increment… > >> ABI version. >> >> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> >> --- >> >> Changes since v7: >> * Squashes commits. >> * Increments ABI version to 4. >> * Refactors commit message. >> * Minor fixes. >> >> Changes since v6: >> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask() >> because it OR values. >> * Makes landlock_add_net_access_mask() more resilient incorrect values. >> * Refactors landlock_get_net_access_mask(). >> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use >> LANDLOCK_NUM_ACCESS_FS as value. >> * Updates access_masks_t to u32 to support network access actions. >> * Refactors landlock internal functions to support network actions with >> landlock_key/key_type/id types. >> >> Changes since v5: >> * Gets rid of partial revert from landlock_add_rule >> syscall. >> * Formats code with clang-format-14. >> >> Changes since v4: >> * Refactors landlock_create_ruleset() - splits ruleset and >> masks checks. >> * Refactors landlock_create_ruleset() and landlock mask >> setters/getters to support two rule types. >> * Refactors landlock_add_rule syscall add_rule_path_beneath >> function by factoring out get_ruleset_from_fd() and >> landlock_put_ruleset(). >> >> Changes since v3: >> * Splits commit. >> * Adds network rule support for internal landlock functions. >> * Adds set_mask and get_mask for network. >> * Adds rb_root root_net_port. >> >> --- >> include/uapi/linux/landlock.h | 49 ++++++++++++++ >> security/landlock/limits.h | 6 +- >> security/landlock/ruleset.c | 55 ++++++++++++++-- >> security/landlock/ruleset.h | 68 ++++++++++++++++---- >> security/landlock/syscalls.c | 13 +++- >> tools/testing/selftests/landlock/base_test.c | 2 +- >> 6 files changed, 170 insertions(+), 23 deletions(-) >> >> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h >> index f3223f964691..096b683c6ff3 100644 >> --- a/include/uapi/linux/landlock.h >> +++ b/include/uapi/linux/landlock.h >> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr { >> * this access right. >> */ >> __u64 handled_access_fs; >> + >> + /** >> + * @handled_access_net: Bitmask of actions (cf. `Network flags`_) >> + * that is handled by this ruleset and should then be forbidden if no >> + * rule explicitly allow them. >> + */ >> + __u64 handled_access_net; >> }; >> >> /* >> @@ -54,6 +61,11 @@ enum landlock_rule_type { >> * landlock_path_beneath_attr . >> */ >> LANDLOCK_RULE_PATH_BENEATH = 1, >> + /** >> + * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct >> + * landlock_net_service_attr . >> + */ >> + LANDLOCK_RULE_NET_SERVICE = 2, >> }; >> >> /** >> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr { >> */ >> } __attribute__((packed)); >> >> +/** >> + * struct landlock_net_service_attr - TCP subnet definition >> + * >> + * Argument of sys_landlock_add_rule(). >> + */ >> +struct landlock_net_service_attr { >> + /** >> + * @allowed_access: Bitmask of allowed access network for services >> + * (cf. `Network flags`_). >> + */ >> + __u64 allowed_access; >> + /** >> + * @port: Network port. >> + */ >> + __u16 port; > > From an UAPI point of view, I think the port field should be __be16, as > for sockaddr_in->port and other network-related APIs. This will require > some kernel changes to please sparse: make C=2 security/landlock/ must > not print any warning. I have this errors trying to launch sparse checking: DESCEND objtool DESCEND bpf/resolve_btfids CALL scripts/checksyscalls.sh CHK kernel/kheaders_data.tar.xz CC security/landlock/setup.o CHECK security/landlock/setup.c ./include/asm-generic/rwonce.h:67:16: error: typename in expression ./include/asm-generic/rwonce.h:67:16: error: Expected ) in function call ./include/asm-generic/rwonce.h:67:16: error: got : ./include/linux/list.h:292:16: error: typename in expression ./include/linux/list.h:292:16: error: Expected ) in function call ./include/linux/list.h:292:16: error: got : .... ./include/linux/seqlock.h:682:16: error: Expected ) in function call ./include/linux/seqlock.h:682:16: error: got : ./include/linux/seqlock.h:695:16: error: typename in expression ./include/linux/seqlock.h:695:16: error: Expected ) in function call ./include/linux/seqlock.h:695:16: error: too many errors Segmentation fault (core dumped) make[3]: *** [scripts/Makefile.build:250: security/landlock/setup.o] Error 139 make[3]: *** Deleting file 'security/landlock/setup.o' make[3]: *** Waiting for unfinished jobs.... Segmentation fault (core dumped) make[3]: *** [scripts/Makefile.build:250: security/landlock/syscalls.o] Error 139 make[3]: *** Deleting file 'security/landlock/syscalls.o' make[2]: *** [scripts/Makefile.build:502: security/landlock] Error 2 make[1]: *** [scripts/Makefile.build:502: security] Error 2 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1994: .] Error 2 > > Using big-endian values as keys (casted to uintptr_t, not strictly > __be16) in the rb-tree should not be an issue because there is no port > range ordering (for now). > > A dedicated test should check that endianness is correct, e.g. by using > different port encoding. This should include passing and failing tests, > but they should work on all architectures (i.e. big or little endian). > > >> + >> +} __attribute__((packed)); >> + >> /** >> * DOC: fs_access >> * >> @@ -173,4 +203,23 @@ struct landlock_path_beneath_attr { >> #define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14) >> /* clang-format on */ >> >> +/** >> + * DOC: net_access >> + * >> + * Network flags >> + * ~~~~~~~~~~~~~~~~ >> + * >> + * These flags enable to restrict a sandboxed process to a set of network >> + * actions. >> + * >> + * TCP sockets with allowed actions: >> + * >> + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port. >> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to >> + * a remote port. >> + */ >> +/* clang-format off */ >> +#define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) >> +#define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) >> +/* clang-format on */ >> #endif /* _UAPI_LINUX_LANDLOCK_H */ >> diff --git a/security/landlock/limits.h b/security/landlock/limits.h >> index bafb3b8dc677..8a1a6463c64e 100644 >> --- a/security/landlock/limits.h >> +++ b/security/landlock/limits.h >> @@ -23,6 +23,10 @@ >> #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS) >> #define LANDLOCK_SHIFT_ACCESS_FS 0 >> >> -/* clang-format on */ >> +#define LANDLOCK_LAST_ACCESS_NET LANDLOCK_ACCESS_NET_CONNECT_TCP >> +#define LANDLOCK_MASK_ACCESS_NET ((LANDLOCK_LAST_ACCESS_NET << 1) - 1) >> +#define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET) >> +#define LANDLOCK_SHIFT_ACCESS_NET LANDLOCK_NUM_ACCESS_FS >> >> +/* clang-format on */ >> #endif /* _SECURITY_LANDLOCK_LIMITS_H */ >> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c >> index c7cf54ba4f6d..9277c1295114 100644 >> --- a/security/landlock/ruleset.c >> +++ b/security/landlock/ruleset.c >> @@ -36,6 +36,9 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) >> refcount_set(&new_ruleset->usage, 1); >> mutex_init(&new_ruleset->lock); >> new_ruleset->root_inode = RB_ROOT; >> +#if IS_ENABLED(CONFIG_INET) >> + new_ruleset->root_net_port = RB_ROOT; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> new_ruleset->num_layers = num_layers; >> /* >> * hierarchy = NULL >> @@ -46,16 +49,21 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) >> } >> >> struct landlock_ruleset * >> -landlock_create_ruleset(const access_mask_t fs_access_mask) >> +landlock_create_ruleset(const access_mask_t fs_access_mask, >> + const access_mask_t net_access_mask) >> { >> struct landlock_ruleset *new_ruleset; >> >> /* Informs about useless ruleset. */ >> - if (!fs_access_mask) >> + if (!fs_access_mask && !net_access_mask) >> return ERR_PTR(-ENOMSG); >> new_ruleset = create_ruleset(1); >> - if (!IS_ERR(new_ruleset)) >> + if (IS_ERR(new_ruleset)) >> + return new_ruleset; >> + if (fs_access_mask) >> landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0); >> + if (net_access_mask) >> + landlock_add_net_access_mask(new_ruleset, net_access_mask, 0); >> return new_ruleset; >> } >> >> @@ -73,6 +81,10 @@ static inline bool is_object_pointer(const enum landlock_key_type key_type) >> switch (key_type) { >> case LANDLOCK_KEY_INODE: >> return true; >> +#if IS_ENABLED(CONFIG_INET) >> + case LANDLOCK_KEY_NET_PORT: >> + return false; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> } >> WARN_ON_ONCE(1); >> return false; >> @@ -126,6 +138,11 @@ static inline struct rb_root *get_root(struct landlock_ruleset *const ruleset, >> case LANDLOCK_KEY_INODE: >> root = &ruleset->root_inode; >> break; >> +#if IS_ENABLED(CONFIG_INET) >> + case LANDLOCK_KEY_NET_PORT: >> + root = &ruleset->root_net_port; >> + break; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> } >> if (WARN_ON_ONCE(!root)) >> return ERR_PTR(-EINVAL); >> @@ -154,7 +171,9 @@ static void build_check_ruleset(void) >> BUILD_BUG_ON(ruleset.num_rules < LANDLOCK_MAX_NUM_RULES); >> BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS); >> BUILD_BUG_ON(access_masks < >> - (LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS)); >> + (LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS) + > > This is correct but because we are dealing with bitmasks I would prefer > to use "|" instead of "+". > > >> + (LANDLOCK_MASK_ACCESS_NET >> + << LANDLOCK_SHIFT_ACCESS_NET)); >> } >> >> /** >> @@ -370,7 +389,12 @@ static int merge_ruleset(struct landlock_ruleset *const dst, >> err = merge_tree(dst, src, LANDLOCK_KEY_INODE); >> if (err) >> goto out_unlock; >> - > > Please keep this newline. > >> +#if IS_ENABLED(CONFIG_INET) >> + /* Merges the @src network port tree. */ >> + err = merge_tree(dst, src, LANDLOCK_KEY_NET_PORT); >> + if (err) >> + goto out_unlock; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> out_unlock: >> mutex_unlock(&src->lock); >> mutex_unlock(&dst->lock); >> @@ -426,7 +450,12 @@ static int inherit_ruleset(struct landlock_ruleset *const parent, >> err = inherit_tree(parent, child, LANDLOCK_KEY_INODE); >> if (err) >> goto out_unlock; >> - > > newline > >> +#if IS_ENABLED(CONFIG_INET) >> + /* Copies the @parent network port tree. */ >> + err = inherit_tree(parent, child, LANDLOCK_KEY_NET_PORT); >> + if (err) >> + goto out_unlock; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) { >> err = -EINVAL; >> goto out_unlock; >> @@ -459,6 +488,11 @@ static void free_ruleset(struct landlock_ruleset *const ruleset) >> rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode, >> node) >> free_rule(freeme, LANDLOCK_KEY_INODE); >> +#if IS_ENABLED(CONFIG_INET) >> + rbtree_postorder_for_each_entry_safe(freeme, next, >> + &ruleset->root_net_port, node) >> + free_rule(freeme, LANDLOCK_KEY_NET_PORT); >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> put_hierarchy(ruleset->hierarchy); >> kfree(ruleset); >> } >> @@ -637,6 +671,9 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset, >> * Populates @layer_masks such that for each access right in @access_request, >> * the bits for all the layers are set where this access right is handled. >> * >> + * @layer_masks must contain LANDLOCK_NUM_ACCESS_FS or LANDLOCK_NUM_ACCESS_NET >> + * elements according to @key_type. > > Please include this sentence in the @layer_masks description below. > >> + * >> * @domain: The domain that defines the current restrictions. >> * @access_request: The requested access rights to check. >> * @layer_masks: The layer masks to populate. > > "It must contain…" > > >> @@ -659,6 +696,12 @@ access_mask_t init_layer_masks(const struct landlock_ruleset *const domain, >> get_access_mask = landlock_get_fs_access_mask; >> num_access = LANDLOCK_NUM_ACCESS_FS; >> break; >> +#if IS_ENABLED(CONFIG_INET) >> + case LANDLOCK_KEY_NET_PORT: >> + get_access_mask = landlock_get_net_access_mask; >> + num_access = LANDLOCK_NUM_ACCESS_NET; >> + break; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> default: >> WARN_ON_ONCE(1); >> return 0; >> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h >> index d9eb79ea9a89..f272d2cd518c 100644 >> --- a/security/landlock/ruleset.h >> +++ b/security/landlock/ruleset.h >> @@ -19,16 +19,20 @@ >> #include "limits.h" >> #include "object.h" >> >> +/* Rule access mask. */ >> 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); >> +/* Makes sure all network access rights can be stored. */ >> +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET); >> /* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */ >> static_assert(sizeof(unsigned long) >= sizeof(access_mask_t)); >> >> /* Ruleset access masks. */ >> -typedef u16 access_masks_t; >> +typedef u32 access_masks_t; > > This type change need to be explained in the commit message. > > >> /* Makes sure all ruleset access rights can be stored. */ >> -static_assert(BITS_PER_TYPE(access_masks_t) >= LANDLOCK_NUM_ACCESS_FS); >> +static_assert(BITS_PER_TYPE(access_masks_t) >= >> + LANDLOCK_NUM_ACCESS_FS + LANDLOCK_NUM_ACCESS_NET); >> >> typedef u16 layer_mask_t; >> /* Makes sure all layers can be checked. */ >> @@ -82,6 +86,13 @@ enum landlock_key_type { >> * keys. >> */ >> LANDLOCK_KEY_INODE = 1, >> +#if IS_ENABLED(CONFIG_INET) >> + /** >> + * @LANDLOCK_KEY_NET_PORT: Type of &landlock_ruleset.root_net_port's >> + * node keys. >> + */ >> + LANDLOCK_KEY_NET_PORT = 2, >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> }; >> >> /** >> @@ -156,6 +167,15 @@ struct landlock_ruleset { >> * reaches zero. >> */ >> struct rb_root root_inode; >> +#if IS_ENABLED(CONFIG_INET) >> + /** >> + * @root_net_port: Root of a red-black tree containing &struct >> + * landlock_rule nodes with network port. Once a ruleset is tied to a >> + * process (i.e. as a domain), this tree is immutable until @usage >> + * reaches zero. >> + */ >> + struct rb_root root_net_port; >> +#endif /* IS_ENABLED(CONFIG_INET) */ >> /** >> * @hierarchy: Enables hierarchy identification even when a parent >> * domain vanishes. This is needed for the ptrace protection. >> @@ -166,8 +186,8 @@ struct landlock_ruleset { >> * @work_free: Enables to free a ruleset within a lockless >> * section. This is only used by >> * landlock_put_ruleset_deferred() when @usage reaches zero. >> - * The fields @lock, @usage, @num_rules, @num_layers and >> - * @access_masks are then unused. >> + * The fields @lock, @usage, @num_rules, @num_layers, >> + * @net_access_mask and @access_masks are then unused. > > There is no net_access_mask anymore. > .
On 04/01/2023 12:41, Konstantin Meskhidze (A) wrote: > > > 11/17/2022 9:43 PM, Mickaël Salaün пишет: [...] >>> /** >>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr { >>> */ >>> } __attribute__((packed)); >>> >>> +/** >>> + * struct landlock_net_service_attr - TCP subnet definition >>> + * >>> + * Argument of sys_landlock_add_rule(). >>> + */ >>> +struct landlock_net_service_attr { >>> + /** >>> + * @allowed_access: Bitmask of allowed access network for services >>> + * (cf. `Network flags`_). >>> + */ >>> + __u64 allowed_access; >>> + /** >>> + * @port: Network port. >>> + */ >>> + __u16 port; >> >> From an UAPI point of view, I think the port field should be __be16, as >> for sockaddr_in->port and other network-related APIs. This will require >> some kernel changes to please sparse: make C=2 security/landlock/ must >> not print any warning. > > I have this errors trying to launch sparse checking: > > DESCEND objtool > DESCEND bpf/resolve_btfids > CALL scripts/checksyscalls.sh > CHK kernel/kheaders_data.tar.xz > CC security/landlock/setup.o > CHECK security/landlock/setup.c > ./include/asm-generic/rwonce.h:67:16: error: typename in expression > ./include/asm-generic/rwonce.h:67:16: error: Expected ) in function call > ./include/asm-generic/rwonce.h:67:16: error: got : > ./include/linux/list.h:292:16: error: typename in expression > ./include/linux/list.h:292:16: error: Expected ) in function call > ./include/linux/list.h:292:16: error: got : > > .... > > ./include/linux/seqlock.h:682:16: error: Expected ) in function call > ./include/linux/seqlock.h:682:16: error: got : > ./include/linux/seqlock.h:695:16: error: typename in expression > ./include/linux/seqlock.h:695:16: error: Expected ) in function call > ./include/linux/seqlock.h:695:16: error: too many errors > Segmentation fault (core dumped) > make[3]: *** [scripts/Makefile.build:250: security/landlock/setup.o] > Error 139 > make[3]: *** Deleting file 'security/landlock/setup.o' > make[3]: *** Waiting for unfinished jobs.... > Segmentation fault (core dumped) > make[3]: *** [scripts/Makefile.build:250: security/landlock/syscalls.o] > Error 139 > make[3]: *** Deleting file 'security/landlock/syscalls.o' > make[2]: *** [scripts/Makefile.build:502: security/landlock] Error 2 > make[1]: *** [scripts/Makefile.build:502: security] Error 2 > make[1]: *** Waiting for unfinished jobs.... > make: *** [Makefile:1994: .] Error 2 I don't know about this error. Did you follow the documentation? https://docs.kernel.org/dev-tools/sparse.html#getting-sparse >> >> Using big-endian values as keys (casted to uintptr_t, not strictly >> __be16) in the rb-tree should not be an issue because there is no port >> range ordering (for now). >> >> A dedicated test should check that endianness is correct, e.g. by using >> different port encoding. This should include passing and failing tests, >> but they should work on all architectures (i.e. big or little endian).
1/6/2023 10:22 PM, Mickaël Salaün пишет: > > On 04/01/2023 12:41, Konstantin Meskhidze (A) wrote: >> >> >> 11/17/2022 9:43 PM, Mickaël Salaün пишет: > > [...] > >>>> /** >>>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr { >>>> */ >>>> } __attribute__((packed)); >>>> >>>> +/** >>>> + * struct landlock_net_service_attr - TCP subnet definition >>>> + * >>>> + * Argument of sys_landlock_add_rule(). >>>> + */ >>>> +struct landlock_net_service_attr { >>>> + /** >>>> + * @allowed_access: Bitmask of allowed access network for services >>>> + * (cf. `Network flags`_). >>>> + */ >>>> + __u64 allowed_access; >>>> + /** >>>> + * @port: Network port. >>>> + */ >>>> + __u16 port; >>> >>> From an UAPI point of view, I think the port field should be __be16, as >>> for sockaddr_in->port and other network-related APIs. This will require >>> some kernel changes to please sparse: make C=2 security/landlock/ must >>> not print any warning. >> >> I have this errors trying to launch sparse checking: >> >> DESCEND objtool >> DESCEND bpf/resolve_btfids >> CALL scripts/checksyscalls.sh >> CHK kernel/kheaders_data.tar.xz >> CC security/landlock/setup.o >> CHECK security/landlock/setup.c >> ./include/asm-generic/rwonce.h:67:16: error: typename in expression >> ./include/asm-generic/rwonce.h:67:16: error: Expected ) in function call >> ./include/asm-generic/rwonce.h:67:16: error: got : >> ./include/linux/list.h:292:16: error: typename in expression >> ./include/linux/list.h:292:16: error: Expected ) in function call >> ./include/linux/list.h:292:16: error: got : >> >> .... >> >> ./include/linux/seqlock.h:682:16: error: Expected ) in function call >> ./include/linux/seqlock.h:682:16: error: got : >> ./include/linux/seqlock.h:695:16: error: typename in expression >> ./include/linux/seqlock.h:695:16: error: Expected ) in function call >> ./include/linux/seqlock.h:695:16: error: too many errors >> Segmentation fault (core dumped) >> make[3]: *** [scripts/Makefile.build:250: security/landlock/setup.o] >> Error 139 >> make[3]: *** Deleting file 'security/landlock/setup.o' >> make[3]: *** Waiting for unfinished jobs.... >> Segmentation fault (core dumped) >> make[3]: *** [scripts/Makefile.build:250: security/landlock/syscalls.o] >> Error 139 >> make[3]: *** Deleting file 'security/landlock/syscalls.o' >> make[2]: *** [scripts/Makefile.build:502: security/landlock] Error 2 >> make[1]: *** [scripts/Makefile.build:502: security] Error 2 >> make[1]: *** Waiting for unfinished jobs.... >> make: *** [Makefile:1994: .] Error 2 > > I don't know about this error. Did you follow the documentation? > https://docs.kernel.org/dev-tools/sparse.html#getting-sparse > Yes, I did as in the documentation. that's strange. If you dont mind can you please check it when I sent a new patch? > > >>> >>> Using big-endian values as keys (casted to uintptr_t, not strictly >>> __be16) in the rb-tree should not be an issue because there is no port >>> range ordering (for now). >>> >>> A dedicated test should check that endianness is correct, e.g. by using >>> different port encoding. This should include passing and failing tests, >>> but they should work on all architectures (i.e. big or little endian). > .
These warnings seem like something I have seen before. Maybe it was an issue with _Generic() support? Are you really sure you're running the latest git version of Sparse? I tested this patch with the latest version of Sparse on my system and it worked fine. regards, dan carpenter
1/9/2023 11:58 AM, Dan Carpenter пишет: > These warnings seem like something I have seen before. Maybe it was an > issue with _Generic() support? > > Are you really sure you're running the latest git version of Sparse? > > I tested this patch with the latest version of Sparse on my system and > it worked fine. Hi Dan, git is on the master branch now - hash ce1a6720 (dated 27 June 2022) Is this correct version? regards, Konstantin. > > regards, > dan carpenter > > .
On Mon, Jan 09, 2023 at 12:26:52PM +0300, Konstantin Meskhidze (A) wrote: > > > 1/9/2023 11:58 AM, Dan Carpenter пишет: > > These warnings seem like something I have seen before. Maybe it was an > > issue with _Generic() support? > > > > Are you really sure you're running the latest git version of Sparse? > > > > I tested this patch with the latest version of Sparse on my system and > > it worked fine. > > Hi Dan, > > git is on the master branch now - hash ce1a6720 (dated 27 June 2022) > > Is this correct version? Yes, that's correct. What is your .config? regards, dan carpenter
1/9/2023 1:20 PM, Dan Carpenter пишет: > On Mon, Jan 09, 2023 at 12:26:52PM +0300, Konstantin Meskhidze (A) wrote: >> >> >> 1/9/2023 11:58 AM, Dan Carpenter пишет: >> > These warnings seem like something I have seen before. Maybe it was an >> > issue with _Generic() support? >> > >> > Are you really sure you're running the latest git version of Sparse? >> > >> > I tested this patch with the latest version of Sparse on my system and >> > it worked fine. >> >> Hi Dan, >> >> git is on the master branch now - hash ce1a6720 (dated 27 June 2022) >> >> Is this correct version? > > Yes, that's correct. What is your .config? What parameters do I need to check in .config? > > regards, > dan carpenter > > .
On Mon, Jan 09, 2023 at 02:39:36PM +0300, Konstantin Meskhidze (A) wrote: > > > 1/9/2023 1:20 PM, Dan Carpenter пишет: > > On Mon, Jan 09, 2023 at 12:26:52PM +0300, Konstantin Meskhidze (A) wrote: > > > > > > > > > 1/9/2023 11:58 AM, Dan Carpenter пишет: > > > > These warnings seem like something I have seen before. Maybe it was an > > > > issue with _Generic() support? > > > > > Are you really sure you're running the latest git version of > > > Sparse? > > > > > I tested this patch with the latest version of Sparse on my > > > system and > > > > it worked fine. > > > > > > Hi Dan, > > > > > > git is on the master branch now - hash ce1a6720 (dated 27 June 2022) > > > > > > Is this correct version? > > > > Yes, that's correct. What is your .config? > > What parameters do I need to check in .config? I don't know. I was hoping you could just email me the whole thing and/or the results from make security/landlock/ruleset.i. That way we could see what line was making Sparse complain. regards, dan carpenter
1/9/2023 2:53 PM, Dan Carpenter пишет: > On Mon, Jan 09, 2023 at 02:39:36PM +0300, Konstantin Meskhidze (A) wrote: >> >> >> 1/9/2023 1:20 PM, Dan Carpenter пишет: >> > On Mon, Jan 09, 2023 at 12:26:52PM +0300, Konstantin Meskhidze (A) wrote: >> > > >> > > >> > > 1/9/2023 11:58 AM, Dan Carpenter пишет: >> > > > These warnings seem like something I have seen before. Maybe it was an >> > > > issue with _Generic() support? >> > > > > Are you really sure you're running the latest git version of >> > > Sparse? >> > > > > I tested this patch with the latest version of Sparse on my >> > > system and >> > > > it worked fine. >> > > >> > > Hi Dan, >> > > >> > > git is on the master branch now - hash ce1a6720 (dated 27 June 2022) >> > > >> > > Is this correct version? >> > >> > Yes, that's correct. What is your .config? >> >> What parameters do I need to check in .config? > > I don't know. I was hoping you could just email me the whole thing > and/or the results from make security/landlock/ruleset.i. That way > we could see what line was making Sparse complain. here is the whole error message: make C=2 security/landlock/ CHECK scripts/mod/empty.c CALL scripts/checksyscalls.sh DESCEND objtool DESCEND bpf/resolve_btfids CHECK security/landlock/setup.c ./include/asm-generic/rwonce.h:67:16: error: typename in expression ./include/asm-generic/rwonce.h:67:16: error: Expected ) in function call ./include/asm-generic/rwonce.h:67:16: error: got : ./include/linux/list.h:292:16: error: typename in expression ./include/linux/list.h:292:16: error: Expected ) in function call ./include/linux/list.h:292:16: error: got : ./include/linux/list.h:328:34: error: typename in expression ./include/linux/list.h:328:34: error: Expected ) in function call ./include/linux/list.h:328:34: error: got : ./include/linux/list.h:329:53: error: typename in expression ./include/linux/list.h:329:53: error: Expected ) in function call ./include/linux/list.h:329:53: error: got : ./include/linux/list.h:867:17: error: typename in expression ./include/linux/list.h:867:17: error: Expected ) in function call ./include/linux/list.h:867:17: error: got : ./include/linux/list.h:876:17: error: typename in expression ./include/linux/list.h:876:17: error: Expected ) in function call ./include/linux/list.h:876:17: error: got : ./arch/x86/include/asm/atomic.h:29:16: error: typename in expression ./arch/x86/include/asm/atomic.h:29:16: error: Expected ) in function call ./arch/x86/include/asm/atomic.h:29:16: error: got : ./arch/x86/include/asm/atomic64_64.h:22:16: error: typename in expression ./arch/x86/include/asm/atomic64_64.h:22:16: error: Expected ) in function call ./arch/x86/include/asm/atomic64_64.h:22:16: error: got : ./include/linux/atomic/atomic-arch-fallback.h:227:23: error: typename in expression ./include/linux/atomic/atomic-arch-fallback.h:227:23: error: Expected ) in function call ./include/linux/atomic/atomic-arch-fallback.h:227:23: error: got : ./include/linux/atomic/atomic-arch-fallback.h:1348:23: error: typename in expression ./include/linux/atomic/atomic-arch-fallback.h:1348:23: error: Expected ) in function call ./include/linux/atomic/atomic-arch-fallback.h:1348:23: error: got : ./include/linux/jump_label.h:286:9: error: Expected ; at end of statement ./include/linux/jump_label.h:286:9: error: got __flags ./include/linux/jump_label.h:302:9: error: Expected ; at end of statement ./include/linux/jump_label.h:302:9: error: got __flags ./include/linux/jump_label.h:319:9: error: Expected ; at end of statement ./include/linux/jump_label.h:319:9: error: got __flags ./include/linux/jump_label.h:322:17: error: Expected ; at end of statement ./include/linux/jump_label.h:322:17: error: got __flags ./include/linux/jump_label.h:330:9: error: Expected ; at end of statement ./include/linux/jump_label.h:330:9: error: got __flags ./include/linux/jump_label.h:333:17: error: Expected ; at end of statement ./include/linux/jump_label.h:333:17: error: got __flags ./include/asm-generic/bitops/generic-non-atomic.h:140:23: error: typename in expression ./include/asm-generic/bitops/generic-non-atomic.h:140:23: error: Expected ) in function call ./include/asm-generic/bitops/generic-non-atomic.h:140:23: error: got : ./include/linux/bitmap.h:268:17: error: Expected ; at end of statement ./include/linux/bitmap.h:268:17: error: got __flags ./include/linux/thread_info.h:127:16: error: typename in expression ./include/linux/thread_info.h:127:16: error: Expected ) in function call ./include/linux/thread_info.h:127:16: error: got : ./include/linux/thread_info.h:233:13: error: Expected ; at end of statement ./include/linux/thread_info.h:233:13: error: got __flags ./include/linux/llist.h:191:16: error: typename in expression ./include/linux/llist.h:191:16: error: Expected ) in function call ./include/linux/llist.h:191:16: error: got : ./include/linux/rcupdate.h:1073:31: error: typename in expression ./include/linux/rcupdate.h:1073:31: error: Expected ) in function call ./include/linux/rcupdate.h:1073:31: error: got : ./include/linux/rcupdate.h:1077:9: error: Expected ; at end of statement ./include/linux/rcupdate.h:1077:9: error: got __flags ./include/linux/key.h:453:16: error: typename in expression ./include/linux/key.h:453:16: error: Expected ) in function call ./include/linux/key.h:453:16: error: got : ./include/linux/list_bl.h:74:33: error: typename in expression ./include/linux/list_bl.h:74:33: error: Expected ) in function call ./include/linux/list_bl.h:74:33: error: got : ./include/linux/rculist_bl.h:24:33: error: typename in expression ./include/linux/rculist_bl.h:24:33: error: Expected ) in function call ./include/linux/rculist_bl.h:24:33: error: got : ./include/linux/seqlock.h:259:16: error: typename in expression ./include/linux/seqlock.h:259:16: error: Expected ) in function call ./include/linux/seqlock.h:259:16: error: got : ./include/linux/seqlock.h:274:1: error: typename in expression ./include/linux/seqlock.h:274:1: error: Expected ) in function call ./include/linux/seqlock.h:274:1: error: got : ./include/linux/seqlock.h:274:1: error: typename in expression ./include/linux/seqlock.h:274:1: error: Expected ) in function call ./include/linux/seqlock.h:274:1: error: got : ./include/linux/seqlock.h:275:1: error: typename in expression ./include/linux/seqlock.h:275:1: error: Expected ) in function call ./include/linux/seqlock.h:275:1: error: got : ./include/linux/seqlock.h:275:1: error: typename in expression ./include/linux/seqlock.h:275:1: error: Expected ) in function call ./include/linux/seqlock.h:275:1: error: got : ./include/linux/seqlock.h:276:1: error: typename in expression ./include/linux/seqlock.h:276:1: error: Expected ) in function call ./include/linux/seqlock.h:276:1: error: got : ./include/linux/seqlock.h:276:1: error: typename in expression ./include/linux/seqlock.h:276:1: error: Expected ) in function call ./include/linux/seqlock.h:276:1: error: got : ./include/linux/seqlock.h:277:1: error: typename in expression ./include/linux/seqlock.h:277:1: error: Expected ) in function call ./include/linux/seqlock.h:277:1: error: got : ./include/linux/seqlock.h:277:1: error: typename in expression ./include/linux/seqlock.h:277:1: error: Expected ) in function call ./include/linux/seqlock.h:277:1: error: got : ./include/linux/seqlock.h:429:16: error: typename in expression ./include/linux/seqlock.h:429:16: error: Expected ) in function call ./include/linux/seqlock.h:429:16: error: got : ./include/linux/seqlock.h:682:16: error: typename in expression ./include/linux/seqlock.h:682:16: error: Expected ) in function call ./include/linux/seqlock.h:682:16: error: too many errors Segmentation fault (core dumped) make[3]: *** [scripts/Makefile.build:251: security/landlock/setup.o] Error 139 make[2]: *** [scripts/Makefile.build:502: security/landlock] Error 2 make[1]: *** [scripts/Makefile.build:502: security] Error 2 make: *** [Makefile:1994: .] Error 2 Please tell me if you need some more info. regards, Konstantin > > regards, > dan carpenter > > .
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h index f3223f964691..096b683c6ff3 100644 --- a/include/uapi/linux/landlock.h +++ b/include/uapi/linux/landlock.h @@ -31,6 +31,13 @@ struct landlock_ruleset_attr { * this access right. */ __u64 handled_access_fs; + + /** + * @handled_access_net: Bitmask of actions (cf. `Network flags`_) + * that is handled by this ruleset and should then be forbidden if no + * rule explicitly allow them. + */ + __u64 handled_access_net; }; /* @@ -54,6 +61,11 @@ enum landlock_rule_type { * landlock_path_beneath_attr . */ LANDLOCK_RULE_PATH_BENEATH = 1, + /** + * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct + * landlock_net_service_attr . + */ + LANDLOCK_RULE_NET_SERVICE = 2, }; /** @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr { */ } __attribute__((packed)); +/** + * struct landlock_net_service_attr - TCP subnet definition + * + * Argument of sys_landlock_add_rule(). + */ +struct landlock_net_service_attr { + /** + * @allowed_access: Bitmask of allowed access network for services + * (cf. `Network flags`_). + */ + __u64 allowed_access; + /** + * @port: Network port. + */ + __u16 port; + +} __attribute__((packed)); + /** * DOC: fs_access * @@ -173,4 +203,23 @@ struct landlock_path_beneath_attr { #define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14) /* clang-format on */ +/** + * DOC: net_access + * + * Network flags + * ~~~~~~~~~~~~~~~~ + * + * These flags enable to restrict a sandboxed process to a set of network + * actions. + * + * TCP sockets with allowed actions: + * + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port. + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to + * a remote port. + */ +/* clang-format off */ +#define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) +#define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) +/* clang-format on */ #endif /* _UAPI_LINUX_LANDLOCK_H */ diff --git a/security/landlock/limits.h b/security/landlock/limits.h index bafb3b8dc677..8a1a6463c64e 100644 --- a/security/landlock/limits.h +++ b/security/landlock/limits.h @@ -23,6 +23,10 @@ #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS) #define LANDLOCK_SHIFT_ACCESS_FS 0 -/* clang-format on */ +#define LANDLOCK_LAST_ACCESS_NET LANDLOCK_ACCESS_NET_CONNECT_TCP +#define LANDLOCK_MASK_ACCESS_NET ((LANDLOCK_LAST_ACCESS_NET << 1) - 1) +#define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET) +#define LANDLOCK_SHIFT_ACCESS_NET LANDLOCK_NUM_ACCESS_FS +/* clang-format on */ #endif /* _SECURITY_LANDLOCK_LIMITS_H */ diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index c7cf54ba4f6d..9277c1295114 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c @@ -36,6 +36,9 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) refcount_set(&new_ruleset->usage, 1); mutex_init(&new_ruleset->lock); new_ruleset->root_inode = RB_ROOT; +#if IS_ENABLED(CONFIG_INET) + new_ruleset->root_net_port = RB_ROOT; +#endif /* IS_ENABLED(CONFIG_INET) */ new_ruleset->num_layers = num_layers; /* * hierarchy = NULL @@ -46,16 +49,21 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) } struct landlock_ruleset * -landlock_create_ruleset(const access_mask_t fs_access_mask) +landlock_create_ruleset(const access_mask_t fs_access_mask, + const access_mask_t net_access_mask) { struct landlock_ruleset *new_ruleset; /* Informs about useless ruleset. */ - if (!fs_access_mask) + if (!fs_access_mask && !net_access_mask) return ERR_PTR(-ENOMSG); new_ruleset = create_ruleset(1); - if (!IS_ERR(new_ruleset)) + if (IS_ERR(new_ruleset)) + return new_ruleset; + if (fs_access_mask) landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0); + if (net_access_mask) + landlock_add_net_access_mask(new_ruleset, net_access_mask, 0); return new_ruleset; } @@ -73,6 +81,10 @@ static inline bool is_object_pointer(const enum landlock_key_type key_type) switch (key_type) { case LANDLOCK_KEY_INODE: return true; +#if IS_ENABLED(CONFIG_INET) + case LANDLOCK_KEY_NET_PORT: + return false; +#endif /* IS_ENABLED(CONFIG_INET) */ } WARN_ON_ONCE(1); return false; @@ -126,6 +138,11 @@ static inline struct rb_root *get_root(struct landlock_ruleset *const ruleset, case LANDLOCK_KEY_INODE: root = &ruleset->root_inode; break; +#if IS_ENABLED(CONFIG_INET) + case LANDLOCK_KEY_NET_PORT: + root = &ruleset->root_net_port; + break; +#endif /* IS_ENABLED(CONFIG_INET) */ } if (WARN_ON_ONCE(!root)) return ERR_PTR(-EINVAL); @@ -154,7 +171,9 @@ static void build_check_ruleset(void) BUILD_BUG_ON(ruleset.num_rules < LANDLOCK_MAX_NUM_RULES); BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS); BUILD_BUG_ON(access_masks < - (LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS)); + (LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS) + + (LANDLOCK_MASK_ACCESS_NET + << LANDLOCK_SHIFT_ACCESS_NET)); } /** @@ -370,7 +389,12 @@ static int merge_ruleset(struct landlock_ruleset *const dst, err = merge_tree(dst, src, LANDLOCK_KEY_INODE); if (err) goto out_unlock; - +#if IS_ENABLED(CONFIG_INET) + /* Merges the @src network port tree. */ + err = merge_tree(dst, src, LANDLOCK_KEY_NET_PORT); + if (err) + goto out_unlock; +#endif /* IS_ENABLED(CONFIG_INET) */ out_unlock: mutex_unlock(&src->lock); mutex_unlock(&dst->lock); @@ -426,7 +450,12 @@ static int inherit_ruleset(struct landlock_ruleset *const parent, err = inherit_tree(parent, child, LANDLOCK_KEY_INODE); if (err) goto out_unlock; - +#if IS_ENABLED(CONFIG_INET) + /* Copies the @parent network port tree. */ + err = inherit_tree(parent, child, LANDLOCK_KEY_NET_PORT); + if (err) + goto out_unlock; +#endif /* IS_ENABLED(CONFIG_INET) */ if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) { err = -EINVAL; goto out_unlock; @@ -459,6 +488,11 @@ static void free_ruleset(struct landlock_ruleset *const ruleset) rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode, node) free_rule(freeme, LANDLOCK_KEY_INODE); +#if IS_ENABLED(CONFIG_INET) + rbtree_postorder_for_each_entry_safe(freeme, next, + &ruleset->root_net_port, node) + free_rule(freeme, LANDLOCK_KEY_NET_PORT); +#endif /* IS_ENABLED(CONFIG_INET) */ put_hierarchy(ruleset->hierarchy); kfree(ruleset); } @@ -637,6 +671,9 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset, * Populates @layer_masks such that for each access right in @access_request, * the bits for all the layers are set where this access right is handled. * + * @layer_masks must contain LANDLOCK_NUM_ACCESS_FS or LANDLOCK_NUM_ACCESS_NET + * elements according to @key_type. + * * @domain: The domain that defines the current restrictions. * @access_request: The requested access rights to check. * @layer_masks: The layer masks to populate. @@ -659,6 +696,12 @@ access_mask_t init_layer_masks(const struct landlock_ruleset *const domain, get_access_mask = landlock_get_fs_access_mask; num_access = LANDLOCK_NUM_ACCESS_FS; break; +#if IS_ENABLED(CONFIG_INET) + case LANDLOCK_KEY_NET_PORT: + get_access_mask = landlock_get_net_access_mask; + num_access = LANDLOCK_NUM_ACCESS_NET; + break; +#endif /* IS_ENABLED(CONFIG_INET) */ default: WARN_ON_ONCE(1); return 0; diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h index d9eb79ea9a89..f272d2cd518c 100644 --- a/security/landlock/ruleset.h +++ b/security/landlock/ruleset.h @@ -19,16 +19,20 @@ #include "limits.h" #include "object.h" +/* Rule access mask. */ 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); +/* Makes sure all network access rights can be stored. */ +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET); /* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */ static_assert(sizeof(unsigned long) >= sizeof(access_mask_t)); /* Ruleset access masks. */ -typedef u16 access_masks_t; +typedef u32 access_masks_t; /* Makes sure all ruleset access rights can be stored. */ -static_assert(BITS_PER_TYPE(access_masks_t) >= LANDLOCK_NUM_ACCESS_FS); +static_assert(BITS_PER_TYPE(access_masks_t) >= + LANDLOCK_NUM_ACCESS_FS + LANDLOCK_NUM_ACCESS_NET); typedef u16 layer_mask_t; /* Makes sure all layers can be checked. */ @@ -82,6 +86,13 @@ enum landlock_key_type { * keys. */ LANDLOCK_KEY_INODE = 1, +#if IS_ENABLED(CONFIG_INET) + /** + * @LANDLOCK_KEY_NET_PORT: Type of &landlock_ruleset.root_net_port's + * node keys. + */ + LANDLOCK_KEY_NET_PORT = 2, +#endif /* IS_ENABLED(CONFIG_INET) */ }; /** @@ -156,6 +167,15 @@ struct landlock_ruleset { * reaches zero. */ struct rb_root root_inode; +#if IS_ENABLED(CONFIG_INET) + /** + * @root_net_port: Root of a red-black tree containing &struct + * landlock_rule nodes with network port. Once a ruleset is tied to a + * process (i.e. as a domain), this tree is immutable until @usage + * reaches zero. + */ + struct rb_root root_net_port; +#endif /* IS_ENABLED(CONFIG_INET) */ /** * @hierarchy: Enables hierarchy identification even when a parent * domain vanishes. This is needed for the ptrace protection. @@ -166,8 +186,8 @@ struct landlock_ruleset { * @work_free: Enables to free a ruleset within a lockless * section. This is only used by * landlock_put_ruleset_deferred() when @usage reaches zero. - * The fields @lock, @usage, @num_rules, @num_layers and - * @access_masks are then unused. + * The fields @lock, @usage, @num_rules, @num_layers, + * @net_access_mask and @access_masks are then unused. */ struct work_struct work_free; struct { @@ -194,13 +214,13 @@ struct landlock_ruleset { */ u32 num_layers; /** - * @access_masks: Contains the subset of filesystem - * actions that are restricted by a ruleset. A domain - * saves all layers of merged rulesets in a stack - * (FAM), starting from the first layer to the last - * one. These layers are used when merging rulesets, - * for user space backward compatibility (i.e. - * future-proof), and to properly handle merged + * @access_masks: Contains the subset of filesystem and + * network actions that are restricted by a ruleset. + * A domain saves all layers of merged rulesets in a + * stack (FAM), starting from the first layer to the + * last one. These layers are used when merging + * rulesets, for user space backward compatibility + * (i.e. future-proof), and to properly handle merged * rulesets without overlapping access rights. These * layers are set once and never changed for the * lifetime of the ruleset. @@ -211,7 +231,8 @@ struct landlock_ruleset { }; struct landlock_ruleset * -landlock_create_ruleset(const access_mask_t access_mask); +landlock_create_ruleset(const access_mask_t access_mask_fs, + const access_mask_t access_mask_net); void landlock_put_ruleset(struct landlock_ruleset *const ruleset); void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset); @@ -248,6 +269,20 @@ landlock_add_fs_access_mask(struct landlock_ruleset *const ruleset, (fs_mask << LANDLOCK_SHIFT_ACCESS_FS); } +static inline void +landlock_add_net_access_mask(struct landlock_ruleset *const ruleset, + const access_mask_t net_access_mask, + const u16 layer_level) +{ + access_mask_t net_mask = net_access_mask & LANDLOCK_MASK_ACCESS_NET; + + /* Should already be checked in sys_landlock_create_ruleset(). */ + WARN_ON_ONCE(net_access_mask != net_mask); + // TODO: Add tests to check "|=" and not "=" + ruleset->access_masks[layer_level] |= + (net_mask << LANDLOCK_SHIFT_ACCESS_NET); +} + static inline access_mask_t landlock_get_fs_access_mask(const struct landlock_ruleset *const ruleset, const u16 layer_level) @@ -257,6 +292,15 @@ landlock_get_fs_access_mask(const struct landlock_ruleset *const ruleset, LANDLOCK_MASK_ACCESS_FS; } +static inline access_mask_t +landlock_get_net_access_mask(const struct landlock_ruleset *const ruleset, + const u16 layer_level) +{ + return (ruleset->access_masks[layer_level] >> + LANDLOCK_SHIFT_ACCESS_NET) & + LANDLOCK_MASK_ACCESS_NET; +} + bool unmask_layers(const struct landlock_rule *const rule, const access_mask_t access_request, layer_mask_t (*const layer_masks)[], diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c index 87389d7bfbf2..c5a6ad4e2fca 100644 --- a/security/landlock/syscalls.c +++ b/security/landlock/syscalls.c @@ -82,8 +82,9 @@ static void build_check_abi(void) * struct size. */ ruleset_size = sizeof(ruleset_attr.handled_access_fs); + ruleset_size += sizeof(ruleset_attr.handled_access_net); BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size); - BUILD_BUG_ON(sizeof(ruleset_attr) != 8); + BUILD_BUG_ON(sizeof(ruleset_attr) != 16); path_beneath_size = sizeof(path_beneath_attr.allowed_access); path_beneath_size += sizeof(path_beneath_attr.parent_fd); @@ -129,7 +130,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 @@ -188,8 +189,14 @@ SYSCALL_DEFINE3(landlock_create_ruleset, LANDLOCK_MASK_ACCESS_FS) return -EINVAL; + /* Checks network content (and 32-bits cast). */ + if ((ruleset_attr.handled_access_net | LANDLOCK_MASK_ACCESS_NET) != + LANDLOCK_MASK_ACCESS_NET) + return -EINVAL; + /* Checks arguments and transforms to kernel struct. */ - ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs); + ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs, + ruleset_attr.handled_access_net); if (IS_ERR(ruleset)) return PTR_ERR(ruleset); diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c index 792c3f0a59b4..646f778dfb1e 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,
This commit adds network rules support in internal landlock functions (presented in ruleset.c) and landlock_create_ruleset syscall. Refactors user space API to support network actions. Adds new network access flags, network rule and network attributes. Increments Landlock ABI version. Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> --- Changes since v7: * Squashes commits. * Increments ABI version to 4. * Refactors commit message. * Minor fixes. Changes since v6: * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask() because it OR values. * Makes landlock_add_net_access_mask() more resilient incorrect values. * Refactors landlock_get_net_access_mask(). * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use LANDLOCK_NUM_ACCESS_FS as value. * Updates access_masks_t to u32 to support network access actions. * Refactors landlock internal functions to support network actions with landlock_key/key_type/id types. Changes since v5: * Gets rid of partial revert from landlock_add_rule syscall. * Formats code with clang-format-14. Changes since v4: * Refactors landlock_create_ruleset() - splits ruleset and masks checks. * Refactors landlock_create_ruleset() and landlock mask setters/getters to support two rule types. * Refactors landlock_add_rule syscall add_rule_path_beneath function by factoring out get_ruleset_from_fd() and landlock_put_ruleset(). Changes since v3: * Splits commit. * Adds network rule support for internal landlock functions. * Adds set_mask and get_mask for network. * Adds rb_root root_net_port. --- include/uapi/linux/landlock.h | 49 ++++++++++++++ security/landlock/limits.h | 6 +- security/landlock/ruleset.c | 55 ++++++++++++++-- security/landlock/ruleset.h | 68 ++++++++++++++++---- security/landlock/syscalls.c | 13 +++- tools/testing/selftests/landlock/base_test.c | 2 +- 6 files changed, 170 insertions(+), 23 deletions(-) -- 2.25.1