| Message ID | 20240408093927.1759381-2-ivanov.mikhail1@huawei-partners.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | Socket type control for Landlock | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch, async |
Hello! Just zooming in on what I think are the most high level questions here, so that we get the more dramatic changes out of the way early, if needed. On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote: > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h > index 25c8d7677..8551ade38 100644 > --- a/include/uapi/linux/landlock.h > +++ b/include/uapi/linux/landlock.h > @@ -37,6 +37,13 @@ struct landlock_ruleset_attr { > * rule explicitly allow them. > */ > __u64 handled_access_net; > + > + /** > + * @handled_access_net: Bitmask of actions (cf. `Socket flags`_) ^^^ Typo > + * that is handled by this ruleset and should then be forbidden if no > + * rule explicitly allow them. > + */ > + __u64 handled_access_socket; What is your rationale for introducing and naming this additional field? I am not convinced that "socket" is the right name to use in this field, but it is well possible that I'm missing some context. * If we introduce this additional field in the landlock_ruleset_attr, which other socket-related operations will go in the remaining 63 bits? (I'm having a hard time coming up with so many of them.) * Should this have a more general name than "socket", so that other planned features from the bug tracker [1] fit in? The other alternative is of course to piggy back on the existing handled_access_net field, whose name already is pretty generic. For that, I believe we would need to clarify in struct landlock_net_port_attr which exact values are permitted there. I imagine you have considered this approach? Are there more reasons why this was ruled out, which I am overlooking? [1] https://github.com/orgs/landlock-lsm/projects/1/views/1 > @@ -244,4 +277,20 @@ struct landlock_net_port_attr { > #define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) > #define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) > /* clang-format on */ > + > +/** > + * DOC: socket_acess > + * > + * Socket flags > + * ~~~~~~~~~~~~~~~~ Mega-Nit: This ~~~ underline should only be as long as the text above it ;-) You might want to fix it for the "Network Flags" headline as well. > + * > + * These flags enable to restrict a sandboxed process to a set of > + * socket-related actions for specific protocols. This is supported > + * since the Landlock ABI version 5. > + * > + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket > + */ > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h > index c7f152678..f4213db09 100644 > --- a/security/landlock/ruleset.h > +++ b/security/landlock/ruleset.h > @@ -92,6 +92,12 @@ enum landlock_key_type { > * node keys. > */ > LANDLOCK_KEY_NET_PORT, > + > + /** > + * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's > + * node keys. > + */ > + LANDLOCK_KEY_SOCKET, > }; > > /** > @@ -177,6 +183,15 @@ struct landlock_ruleset { > struct rb_root root_net_port; > #endif /* IS_ENABLED(CONFIG_INET) */ > > + /** > + * @root_socket: Root of a red-black tree containing &struct > + * landlock_rule nodes with socket type, described by (domain, type) > + * pair (see socket(2)). 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_socket; The domain is a value between 0 and 45, and the socket type is one of 1, 2, 3, 4, 5, 6, 10. The bounds of these are defined with AF_MAX (include/linux/socket.h) and SOCK_MAX (include/linux/net.h). Why don't we just combine these two numbers into an index and create a big bit vector here, like this: socket_type_mask_t socket_domains[AF_MAX]; socket_type_mask_t would need to be typedef'd to u16 and ideally have a static check to test that it has more bits than SOCK_MAX. Then you can look up whether a socket creation is permitted by checking: /* assuming appropriate bounds checks */ if (dom->socket_domains[domain] & (1 << type)) { /* permitted */ } and merging the socket_domains of two domains would be a bitwise-AND. (We can also cram socket_type_mask_t in a u8 but it would require mapping the existing socket types onto a different number space.) As I said before, I am very excited to see this patch. I think this will unlock a tremendous amount of use cases for many programs, especially for programs that do not use networking at all, which can now lock themselves down to guarantee that with a sandbox. Thank you very much for looking into it! —Günther
Hello! Big thanks for your review and ideas :) P.S.: Sorry, previous mail was rejected by linux mailboxes due to HTML formatting. 4/8/2024 10:49 PM, Günther Noack wrote: > Hello! > > Just zooming in on what I think are the most high level questions here, > so that we get the more dramatic changes out of the way early, if needed. > > On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote: >> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h >> index 25c8d7677..8551ade38 100644 >> --- a/include/uapi/linux/landlock.h >> +++ b/include/uapi/linux/landlock.h >> @@ -37,6 +37,13 @@ struct landlock_ruleset_attr { >> * rule explicitly allow them. >> */ >> __u64 handled_access_net; >> + >> + /** >> + * @handled_access_net: Bitmask of actions (cf. `Socket flags`_) > ^^^ > Typo > Thanks, will be fixed. >> + * that is handled by this ruleset and should then be forbidden if no >> + * rule explicitly allow them. >> + */ >> + __u64 handled_access_socket; > > What is your rationale for introducing and naming this additional field? > > I am not convinced that "socket" is the right name to use in this field, > but it is well possible that I'm missing some context. > > * If we introduce this additional field in the landlock_ruleset_attr, which > other socket-related operations will go in the remaining 63 bits? (I'm having > a hard time coming up with so many of them.) If i understood correctly Mickaël suggested saving some space for actions related not only to creating sockets, but also to sending and receiving socket FDs from another processes, marking pre-sandboxed sockets as allowed or denied after sandboxing [2]. This may be necessary in order to achieve complete isolation of the sandbox, which will be able to create, receive and send sockets of specific protocols. In future this field may become more generic by including rules for other entities with similar actions (e.g. files, pipes). I think it is good approach, but we should discuss this design before generalizing the name. For now `handled_access_socket` can be a good name for actions related to accessing specific sockets (protocols). What do you think? [2] https://lore.kernel.org/all/b8a2045a-e7e8-d141-7c01-bf47874c7930@digikod.net/ > > * Should this have a more general name than "socket", so that other planned > features from the bug tracker [1] fit in? I have not found any similar features for our case. Do you have any in mind? > > The other alternative is of course to piggy back on the existing > handled_access_net field, whose name already is pretty generic. > > For that, I believe we would need to clarify in struct landlock_net_port_attr > which exact values are permitted there. > > I imagine you have considered this approach? Are there more reasons why this > was ruled out, which I am overlooking? > > [1] https://github.com/orgs/landlock-lsm/projects/1/views/1 > > Currently `handled_access_net` stands for restricting actions for specific network protocols by port values: LANDLOCK_ACCESS_NET_BIND_TCP, LANDLOCK_ACCESS_NET_SEND_UDP (possibly will be added with UDP feature [3]). I dont think that complicating semantics with adding fields for socket_create()-like actions would fit well here. Purpose of current patch is to restrict usage of unwanted protocols, not to add logic to restrict their actions. In addition, it is worth considering that we want to restrict not only network protocols (e.g. Bluetooth). [3] https://github.com/landlock-lsm/linux/issues/1 >> @@ -244,4 +277,20 @@ struct landlock_net_port_attr { >> #define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) >> #define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) >> /* clang-format on */ >> + >> +/** >> + * DOC: socket_acess >> + * >> + * Socket flags >> + * ~~~~~~~~~~~~~~~~ > > Mega-Nit: This ~~~ underline should only be as long as the text above it ;-) > You might want to fix it for the "Network Flags" headline as well. > Ofc, thanks! >> + * >> + * These flags enable to restrict a sandboxed process to a set of >> + * socket-related actions for specific protocols. This is supported >> + * since the Landlock ABI version 5. >> + * >> + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket >> + */ > > >> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h >> index c7f152678..f4213db09 100644 >> --- a/security/landlock/ruleset.h >> +++ b/security/landlock/ruleset.h >> @@ -92,6 +92,12 @@ enum landlock_key_type { >> * node keys. >> */ >> LANDLOCK_KEY_NET_PORT, >> + >> + /** >> + * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's >> + * node keys. >> + */ >> + LANDLOCK_KEY_SOCKET, >> }; >> >> /** >> @@ -177,6 +183,15 @@ struct landlock_ruleset { >> struct rb_root root_net_port; >> #endif /* IS_ENABLED(CONFIG_INET) */ >> >> + /** >> + * @root_socket: Root of a red-black tree containing &struct >> + * landlock_rule nodes with socket type, described by (domain, type) >> + * pair (see socket(2)). 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_socket; > > The domain is a value between 0 and 45, > and the socket type is one of 1, 2, 3, 4, 5, 6, 10. > > The bounds of these are defined with AF_MAX (include/linux/socket.h) and > SOCK_MAX (include/linux/net.h). > > Why don't we just combine these two numbers into an index and create a big bit > vector here, like this: > > socket_type_mask_t socket_domains[AF_MAX]; > > socket_type_mask_t would need to be typedef'd to u16 and ideally have a static > check to test that it has more bits than SOCK_MAX. > > Then you can look up whether a socket creation is permitted by checking: > > /* assuming appropriate bounds checks */ > if (dom->socket_domains[domain] & (1 << type)) { /* permitted */ } > > and merging the socket_domains of two domains would be a bitwise-AND. > > (We can also cram socket_type_mask_t in a u8 but it would require mapping the > existing socket types onto a different number space.) > I chose rbtree based on the current storage implementation in fs,net and decided to leave the implementation of better variants in a separate patch, which should redesign the entire storage system in Landlock (e.g. implementation of a hashtable for storing rules by FDs, port values) [4]. Do you think that it is bad idea and more appropriate storage for socket rules(e.g. what you suggested) should be implemented by current patch? [4] https://github.com/landlock-lsm/linux/issues/1 > > As I said before, I am very excited to see this patch. > > I think this will unlock a tremendous amount of use cases for many programs, > especially for programs that do not use networking at all, which can now lock > themselves down to guarantee that with a sandbox. > > Thank you very much for looking into it! > —Günther
Hello! On Thu, Apr 11, 2024 at 06:16:31PM +0300, Ivanov Mikhail wrote: > 4/8/2024 10:49 PM, Günther Noack wrote: > > On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote: > > > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h > > > index c7f152678..f4213db09 100644 > > > --- a/security/landlock/ruleset.h > > > +++ b/security/landlock/ruleset.h > > > @@ -92,6 +92,12 @@ enum landlock_key_type { > > > * node keys. > > > */ > > > LANDLOCK_KEY_NET_PORT, > > > + > > > + /** > > > + * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's > > > + * node keys. > > > + */ > > > + LANDLOCK_KEY_SOCKET, > > > }; > > > /** > > > @@ -177,6 +183,15 @@ struct landlock_ruleset { > > > struct rb_root root_net_port; > > > #endif /* IS_ENABLED(CONFIG_INET) */ > > > + /** > > > + * @root_socket: Root of a red-black tree containing &struct > > > + * landlock_rule nodes with socket type, described by (domain, type) > > > + * pair (see socket(2)). 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_socket; > > > > The domain is a value between 0 and 45, > > and the socket type is one of 1, 2, 3, 4, 5, 6, 10. > > > > The bounds of these are defined with AF_MAX (include/linux/socket.h) and > > SOCK_MAX (include/linux/net.h). > > > > Why don't we just combine these two numbers into an index and create a big bit > > vector here, like this: > > > > socket_type_mask_t socket_domains[AF_MAX]; > > > > socket_type_mask_t would need to be typedef'd to u16 and ideally have a static > > check to test that it has more bits than SOCK_MAX. > > > > Then you can look up whether a socket creation is permitted by checking: > > > > /* assuming appropriate bounds checks */ > > if (dom->socket_domains[domain] & (1 << type)) { /* permitted */ } > > > > and merging the socket_domains of two domains would be a bitwise-AND. > > > > (We can also cram socket_type_mask_t in a u8 but it would require mapping the > > existing socket types onto a different number space.) > > > > I chose rbtree based on the current storage implementation in fs,net and > decided to leave the implementation of better variants in a separate > patch, which should redesign the entire storage system in Landlock > (e.g. implementation of a hashtable for storing rules by FDs, > port values) [4]. > > Do you think that it is bad idea and more appropriate storage for socket > rules(e.g. what you suggested) should be implemented by current patch? > > [4] https://github.com/landlock-lsm/linux/issues/1 I realized that my suggestion might be at odds with Mickaël's Landlock audit patch set [1]. IIRC, the goal there is to log the reasons for a denial, together with the Landlock ruleset on which this decision was based. [1] https://lore.kernel.org/all/20230921061641.273654-1-mic@digikod.net/ I'd recommend to wait for Mickaël to chime in on this one before spending the time to reimplement that. —Günther
Thanks Ivanov, this looks really good! Let me some time to review the rest. You can add this tag to the commit message (as reference and documentation): Closes: https://github.com/landlock-lsm/linux/issues/6 On Thu, Apr 11, 2024 at 06:16:31PM +0300, Ivanov Mikhail wrote: > Hello! Big thanks for your review and ideas :) > > P.S.: Sorry, previous mail was rejected by linux mailboxes > due to HTML formatting. > > 4/8/2024 10:49 PM, Günther Noack wrote: > > Hello! > > > > Just zooming in on what I think are the most high level questions here, > > so that we get the more dramatic changes out of the way early, if needed. > > > > On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote: > > > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h > > > index 25c8d7677..8551ade38 100644 > > > --- a/include/uapi/linux/landlock.h > > > +++ b/include/uapi/linux/landlock.h > > > @@ -37,6 +37,13 @@ struct landlock_ruleset_attr { > > > * rule explicitly allow them. > > > */ > > > __u64 handled_access_net; > > > + > > > + /** > > > + * @handled_access_net: Bitmask of actions (cf. `Socket flags`_) > > ^^^ > > Typo > > > > Thanks, will be fixed. > > > > + * that is handled by this ruleset and should then be forbidden if no > > > + * rule explicitly allow them. > > > + */ > > > + __u64 handled_access_socket; > > > > What is your rationale for introducing and naming this additional field? > > > > I am not convinced that "socket" is the right name to use in this field, > > but it is well possible that I'm missing some context. > > > > * If we introduce this additional field in the landlock_ruleset_attr, which > > other socket-related operations will go in the remaining 63 bits? (I'm having > > a hard time coming up with so many of them.) > > If i understood correctly Mickaël suggested saving some space for > actions related not only to creating sockets, but also to sending > and receiving socket FDs from another processes, marking pre-sandboxed > sockets as allowed or denied after sandboxing [2]. This may be necessary > in order to achieve complete isolation of the sandbox, which will be > able to create, receive and send sockets of specific protocols. > > In future this field may become more generic by including rules for > other entities with similar actions (e.g. files, pipes). I think it would make sense to have one field per file kind (not necessarily type) because not all actions would make sense. > > I think it is good approach, but we should discuss this design before > generalizing the name. For now `handled_access_socket` can be a good > name for actions related to accessing specific sockets (protocols). > What do you think? I'm OK with this name for now unless someone has a better proposition. > > [2] > https://lore.kernel.org/all/b8a2045a-e7e8-d141-7c01-bf47874c7930@digikod.net/ > > > > > * Should this have a more general name than "socket", so that other planned > > features from the bug tracker [1] fit in? > > I have not found any similar features for our case. Do you have any in > mind? > > > > > The other alternative is of course to piggy back on the existing > > handled_access_net field, whose name already is pretty generic. handled_access_net is indeed quite generic, but the question is: would this new access right make sense for the net_port rule? In the case of socket creation, this is not the case because we don't know at this time which port will be used. > > > > For that, I believe we would need to clarify in struct landlock_net_port_attr > > which exact values are permitted there. Potentially anything that would be possible to check against a port. > > > > I imagine you have considered this approach? Are there more reasons why this > > was ruled out, which I am overlooking? > > > > [1] https://github.com/orgs/landlock-lsm/projects/1/views/1 > > > > > > Currently `handled_access_net` stands for restricting actions for > specific network protocols by port values: LANDLOCK_ACCESS_NET_BIND_TCP, > LANDLOCK_ACCESS_NET_SEND_UDP (possibly will be added with UDP feature > [3]). > > I dont think that complicating semantics with adding fields for > socket_create()-like actions would fit well here. Purpose of current > patch is to restrict usage of unwanted protocols, not to add logic > to restrict their actions. In addition, it is worth considering that we > want to restrict not only network protocols (e.g. Bluetooth). Correct. It's worth it mentionning this rationale in the patch description. > > [3] https://github.com/landlock-lsm/linux/issues/1 > > > > @@ -244,4 +277,20 @@ struct landlock_net_port_attr { > > > #define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) > > > #define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) > > > /* clang-format on */ > > > + > > > +/** > > > + * DOC: socket_acess > > > + * > > > + * Socket flags > > > + * ~~~~~~~~~~~~~~~~ > > > > Mega-Nit: This ~~~ underline should only be as long as the text above it ;-) > > You might want to fix it for the "Network Flags" headline as well. > > > > Ofc, thanks! > > > > + * > > > + * These flags enable to restrict a sandboxed process to a set of > > > + * socket-related actions for specific protocols. This is supported > > > + * since the Landlock ABI version 5. > > > + * > > > + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket > > > + */ > > > > > > > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h > > > index c7f152678..f4213db09 100644 > > > --- a/security/landlock/ruleset.h > > > +++ b/security/landlock/ruleset.h > > > @@ -92,6 +92,12 @@ enum landlock_key_type { > > > * node keys. > > > */ > > > LANDLOCK_KEY_NET_PORT, > > > + > > > + /** > > > + * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's > > > + * node keys. > > > + */ > > > + LANDLOCK_KEY_SOCKET, > > > }; > > > /** > > > @@ -177,6 +183,15 @@ struct landlock_ruleset { > > > struct rb_root root_net_port; > > > #endif /* IS_ENABLED(CONFIG_INET) */ > > > + /** > > > + * @root_socket: Root of a red-black tree containing &struct > > > + * landlock_rule nodes with socket type, described by (domain, type) > > > + * pair (see socket(2)). 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_socket; > > > > The domain is a value between 0 and 45, > > and the socket type is one of 1, 2, 3, 4, 5, 6, 10. > > > > The bounds of these are defined with AF_MAX (include/linux/socket.h) and > > SOCK_MAX (include/linux/net.h). > > > > Why don't we just combine these two numbers into an index and create a big bit > > vector here, like this: > > > > socket_type_mask_t socket_domains[AF_MAX]; > > > > socket_type_mask_t would need to be typedef'd to u16 and ideally have a static > > check to test that it has more bits than SOCK_MAX. > > > > Then you can look up whether a socket creation is permitted by checking: > > > > /* assuming appropriate bounds checks */ > > if (dom->socket_domains[domain] & (1 << type)) { /* permitted */ } > > > > and merging the socket_domains of two domains would be a bitwise-AND. > > > > (We can also cram socket_type_mask_t in a u8 but it would require mapping the > > existing socket types onto a different number space.) > > > > I chose rbtree based on the current storage implementation in fs,net and > decided to leave the implementation of better variants in a separate > patch, which should redesign the entire storage system in Landlock > (e.g. implementation of a hashtable for storing rules by FDs, > port values) [4]. > > Do you think that it is bad idea and more appropriate storage for socket > rules(e.g. what you suggested) should be implemented by current patch? Günther's suggestion would be a good optimization, but I agree that it should be part of another series. We also need to keep in mind that the layer level should be known for audit and debugging reasons. > > [4] https://github.com/landlock-lsm/linux/issues/1 > > > > > As I said before, I am very excited to see this patch. > > > > I think this will unlock a tremendous amount of use cases for many programs, > > especially for programs that do not use networking at all, which can now lock > > themselves down to guarantee that with a sandbox. > > > > Thank you very much for looking into it! Same :)
On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote: > Add new socket-related rule type, presented via landlock_socket_attr > struct. Add all neccessary entities for socket ruleset support. > Add flag LANDLOCK_ACCESS_SOCKET_CREATE that will provide the > ability to control socket creation. > > Change landlock_key.data type from uinptr_t to u64. Socket rule has to > contain 32-bit socket family and type values, so landlock_key can be > represented as 64-bit number the first 32 bits of which correspond to > the socket family and last - to the type. > > Change ABI version to 5. > > Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com> > Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> > --- > include/uapi/linux/landlock.h | 49 +++++++++++++++++ > security/landlock/Makefile | 2 +- > security/landlock/limits.h | 5 ++ > security/landlock/net.c | 2 +- > security/landlock/ruleset.c | 35 +++++++++++-- > security/landlock/ruleset.h | 44 ++++++++++++++-- > security/landlock/socket.c | 43 +++++++++++++++ > security/landlock/socket.h | 17 ++++++ > security/landlock/syscalls.c | 55 ++++++++++++++++++-- > tools/testing/selftests/landlock/base_test.c | 2 +- > 10 files changed, 241 insertions(+), 13 deletions(-) > create mode 100644 security/landlock/socket.c > create mode 100644 security/landlock/socket.h > > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h > index 25c8d7677..8551ade38 100644 > --- a/include/uapi/linux/landlock.h > +++ b/include/uapi/linux/landlock.h > @@ -37,6 +37,13 @@ struct landlock_ruleset_attr { > * rule explicitly allow them. > */ > __u64 handled_access_net; > + > + /** > + * @handled_access_net: Bitmask of actions (cf. `Socket flags`_) > + * that is handled by this ruleset and should then be forbidden if no > + * rule explicitly allow them. > + */ > + __u64 handled_access_socket; > }; > > /* > @@ -65,6 +72,11 @@ enum landlock_rule_type { > * landlock_net_port_attr . > */ > LANDLOCK_RULE_NET_PORT, > + /** > + * @LANDLOCK_RULE_SOCKET: Type of a &struct > + * landlock_socket_attr . > + */ > + LANDLOCK_RULE_SOCKET, > }; > > /** > @@ -115,6 +127,27 @@ struct landlock_net_port_attr { > __u64 port; > }; > > +/** > + * struct landlock_socket_attr - Socket definition > + * > + * Argument of sys_landlock_add_rule(). > + */ > +struct landlock_socket_attr { > + /** > + * @allowed_access: Bitmask of allowed access for a socket > + * (cf. `Socket flags`_). > + */ > + __u64 allowed_access; > + /** > + * @domain: Protocol family used for communication (see socket(2)). > + */ > + int domain; > + /** > + * @type: Socket type (see socket(2)). > + */ > + int type; > +}; > + > /** > * DOC: fs_access > * > @@ -244,4 +277,20 @@ struct landlock_net_port_attr { > #define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) > #define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) > /* clang-format on */ > + > +/** > + * DOC: socket_acess > + * > + * Socket flags > + * ~~~~~~~~~~~~~~~~ > + * > + * These flags enable to restrict a sandboxed process to a set of > + * socket-related actions for specific protocols. This is supported > + * since the Landlock ABI version 5. > + * > + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket > + */ > +/* clang-format off */ > +#define LANDLOCK_ACCESS_SOCKET_CREATE (1ULL << 0) > +/* clang-format on */ > #endif /* _UAPI_LINUX_LANDLOCK_H */ > diff --git a/security/landlock/Makefile b/security/landlock/Makefile > index b4538b7cf..ff1dd98f6 100644 > --- a/security/landlock/Makefile > +++ b/security/landlock/Makefile > @@ -1,6 +1,6 @@ > obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o > > landlock-y := setup.o syscalls.o object.o ruleset.o \ > - cred.o task.o fs.o > + cred.o task.o fs.o socket.o > > landlock-$(CONFIG_INET) += net.o > diff --git a/security/landlock/limits.h b/security/landlock/limits.h > index 93c9c6f91..ebdab587c 100644 > --- a/security/landlock/limits.h > +++ b/security/landlock/limits.h > @@ -28,6 +28,11 @@ > #define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET) > #define LANDLOCK_SHIFT_ACCESS_NET LANDLOCK_NUM_ACCESS_FS > > +#define LANDLOCK_LAST_ACCESS_SOCKET LANDLOCK_ACCESS_SOCKET_CREATE > +#define LANDLOCK_MASK_ACCESS_SOCKET ((LANDLOCK_LAST_ACCESS_SOCKET << 1) - 1) > +#define LANDLOCK_NUM_ACCESS_SOCKET __const_hweight64(LANDLOCK_MASK_ACCESS_SOCKET) > +#define LANDLOCK_SHIFT_ACCESS_SOCKET LANDLOCK_NUM_ACCESS_SOCKET > + > /* clang-format on */ > > #endif /* _SECURITY_LANDLOCK_LIMITS_H */ > diff --git a/security/landlock/net.c b/security/landlock/net.c > index c8bcd29bd..0e3770d14 100644 > --- a/security/landlock/net.c > +++ b/security/landlock/net.c > @@ -159,7 +159,7 @@ static int current_check_access_socket(struct socket *const sock, > return -EINVAL; > } > > - id.key.data = (__force uintptr_t)port; > + id.key.data = (__force u64)port; > BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data)); > > rule = landlock_find_rule(dom, id); > diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c > index e0a5fbf92..1f1ed8181 100644 > --- a/security/landlock/ruleset.c > +++ b/security/landlock/ruleset.c > @@ -40,6 +40,7 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) > #if IS_ENABLED(CONFIG_INET) > new_ruleset->root_net_port = RB_ROOT; > #endif /* IS_ENABLED(CONFIG_INET) */ > + new_ruleset->root_socket = RB_ROOT; > > new_ruleset->num_layers = num_layers; > /* > @@ -52,12 +53,13 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) > > struct landlock_ruleset * > landlock_create_ruleset(const access_mask_t fs_access_mask, > - const access_mask_t net_access_mask) > + const access_mask_t net_access_mask, > + const access_mask_t socket_access_mask) > { > struct landlock_ruleset *new_ruleset; > > /* Informs about useless ruleset. */ > - if (!fs_access_mask && !net_access_mask) > + if (!fs_access_mask && !net_access_mask && !socket_access_mask) > return ERR_PTR(-ENOMSG); > new_ruleset = create_ruleset(1); > if (IS_ERR(new_ruleset)) > @@ -66,6 +68,8 @@ landlock_create_ruleset(const access_mask_t 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); > + if (socket_access_mask) > + landlock_add_socket_access_mask(new_ruleset, socket_access_mask, 0); > return new_ruleset; > } > > @@ -89,6 +93,9 @@ static bool is_object_pointer(const enum landlock_key_type key_type) > return false; > #endif /* IS_ENABLED(CONFIG_INET) */ > > + case LANDLOCK_KEY_SOCKET: > + return false; > + > default: > WARN_ON_ONCE(1); > return false; > @@ -146,6 +153,9 @@ static struct rb_root *get_root(struct landlock_ruleset *const ruleset, > return &ruleset->root_net_port; > #endif /* IS_ENABLED(CONFIG_INET) */ > > + case LANDLOCK_KEY_SOCKET: > + return &ruleset->root_socket; > + > default: > WARN_ON_ONCE(1); > return ERR_PTR(-EINVAL); > @@ -175,7 +185,8 @@ static void build_check_ruleset(void) > 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_NET << LANDLOCK_SHIFT_ACCESS_NET))); > + (LANDLOCK_MASK_ACCESS_NET << LANDLOCK_SHIFT_ACCESS_NET) | > + (LANDLOCK_MASK_ACCESS_SOCKET << LANDLOCK_SHIFT_ACCESS_SOCKET))); > } > > /** > @@ -399,6 +410,11 @@ static int merge_ruleset(struct landlock_ruleset *const dst, > goto out_unlock; > #endif /* IS_ENABLED(CONFIG_INET) */ > > + /* Merges the @src socket tree. */ > + err = merge_tree(dst, src, LANDLOCK_KEY_SOCKET); > + if (err) > + goto out_unlock; > + > out_unlock: > mutex_unlock(&src->lock); > mutex_unlock(&dst->lock); > @@ -462,6 +478,11 @@ static int inherit_ruleset(struct landlock_ruleset *const parent, > goto out_unlock; > #endif /* IS_ENABLED(CONFIG_INET) */ > > + /* Copies the @parent socket tree. */ > + err = inherit_tree(parent, child, LANDLOCK_KEY_SOCKET); > + if (err) > + goto out_unlock; > + > if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) { > err = -EINVAL; > goto out_unlock; > @@ -498,6 +519,10 @@ static void free_ruleset(struct landlock_ruleset *const ruleset) > free_rule(freeme, LANDLOCK_KEY_NET_PORT); > #endif /* IS_ENABLED(CONFIG_INET) */ > > + rbtree_postorder_for_each_entry_safe(freeme, next, > + &ruleset->root_socket, node) > + free_rule(freeme, LANDLOCK_KEY_SOCKET); > + > put_hierarchy(ruleset->hierarchy); > kfree(ruleset); > } > @@ -708,6 +733,10 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain, > break; > #endif /* IS_ENABLED(CONFIG_INET) */ > > + case LANDLOCK_KEY_SOCKET: > + get_access_mask = landlock_get_socket_access_mask; > + num_access = LANDLOCK_NUM_ACCESS_SOCKET; > + break; > default: > WARN_ON_ONCE(1); > return 0; > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h > index c7f152678..f4213db09 100644 > --- a/security/landlock/ruleset.h > +++ b/security/landlock/ruleset.h > @@ -72,10 +72,10 @@ union landlock_key { > */ > struct landlock_object *object; > /** > - * @data: Raw data to identify an arbitrary 32-bit value > + * @data: Raw data to identify an arbitrary 64-bit value > * (e.g. a TCP port). > */ > - uintptr_t data; > + u64 data; > }; > > /** > @@ -92,6 +92,12 @@ enum landlock_key_type { > * node keys. > */ > LANDLOCK_KEY_NET_PORT, > + > + /** > + * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's > + * node keys. > + */ > + LANDLOCK_KEY_SOCKET, > }; > > /** > @@ -177,6 +183,15 @@ struct landlock_ruleset { > struct rb_root root_net_port; > #endif /* IS_ENABLED(CONFIG_INET) */ > > + /** > + * @root_socket: Root of a red-black tree containing &struct > + * landlock_rule nodes with socket type, described by (domain, type) > + * pair (see socket(2)). 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_socket; > + > /** > * @hierarchy: Enables hierarchy identification even when a parent > * domain vanishes. This is needed for the ptrace protection. > @@ -233,7 +248,8 @@ struct landlock_ruleset { > > struct landlock_ruleset * > landlock_create_ruleset(const access_mask_t access_mask_fs, > - const access_mask_t access_mask_net); > + const access_mask_t access_mask_net, > + const access_mask_t socket_access_mask); > > void landlock_put_ruleset(struct landlock_ruleset *const ruleset); > void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset); > @@ -282,6 +298,19 @@ landlock_add_net_access_mask(struct landlock_ruleset *const ruleset, > (net_mask << LANDLOCK_SHIFT_ACCESS_NET); > } > > +static inline void > +landlock_add_socket_access_mask(struct landlock_ruleset *const ruleset, > + const access_mask_t socket_access_mask, > + const u16 layer_level) > +{ > + access_mask_t socket_mask = socket_access_mask & LANDLOCK_MASK_ACCESS_SOCKET; > + > + /* Should already be checked in sys_landlock_create_ruleset(). */ > + WARN_ON_ONCE(socket_access_mask != socket_mask); > + ruleset->access_masks[layer_level] |= > + (socket_mask << LANDLOCK_SHIFT_ACCESS_SOCKET); > +} > + > static inline access_mask_t > landlock_get_raw_fs_access_mask(const struct landlock_ruleset *const ruleset, > const u16 layer_level) > @@ -309,6 +338,15 @@ landlock_get_net_access_mask(const struct landlock_ruleset *const ruleset, > LANDLOCK_MASK_ACCESS_NET; > } > > +static inline access_mask_t > +landlock_get_socket_access_mask(const struct landlock_ruleset *const ruleset, > + const u16 layer_level) > +{ > + return (ruleset->access_masks[layer_level] >> > + LANDLOCK_SHIFT_ACCESS_SOCKET) & > + LANDLOCK_MASK_ACCESS_SOCKET; > +} > + > bool landlock_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/socket.c b/security/landlock/socket.c > new file mode 100644 > index 000000000..88b4ef3a1 > --- /dev/null > +++ b/security/landlock/socket.c > @@ -0,0 +1,43 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Landlock LSM - Socket management and hooks > + * > + * Copyright © 2024 Huawei Tech. Co., Ltd. > + */ > + > +#include "limits.h" > +#include "ruleset.h" > +#include "socket.h" > + > +union socket_key { > + struct { > + int domain; > + int type; > + } __packed content; > + u64 val; > +}; > + > +int landlock_append_socket_rule(struct landlock_ruleset *const ruleset, > + const int domain, const int type, access_mask_t access_rights) > +{ > + int err; > + const union socket_key socket_key = { > + .content.domain = domain, > + .content.type = type > + }; I'm not convinced this landlock_key.data needs to be changed to u64. We could have an helper to fit the SOCK_MAX and AF_MAX values into 32-bits, and a related built-time check to make sure this works. > + > + const struct landlock_id id = { > + .key.data = socket_key.val, > + .type = LANDLOCK_KEY_SOCKET, > + };
4/12/2024 6:46 PM, Mickaël Salaün wrote: > On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote: >> Add new socket-related rule type, presented via landlock_socket_attr >> struct. Add all neccessary entities for socket ruleset support. >> Add flag LANDLOCK_ACCESS_SOCKET_CREATE that will provide the >> ability to control socket creation. >> >> Change landlock_key.data type from uinptr_t to u64. Socket rule has to >> contain 32-bit socket family and type values, so landlock_key can be >> represented as 64-bit number the first 32 bits of which correspond to >> the socket family and last - to the type. >> >> Change ABI version to 5. >> >> Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com> >> Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> >> --- >> include/uapi/linux/landlock.h | 49 +++++++++++++++++ >> security/landlock/Makefile | 2 +- >> security/landlock/limits.h | 5 ++ >> security/landlock/net.c | 2 +- >> security/landlock/ruleset.c | 35 +++++++++++-- >> security/landlock/ruleset.h | 44 ++++++++++++++-- >> security/landlock/socket.c | 43 +++++++++++++++ >> security/landlock/socket.h | 17 ++++++ >> security/landlock/syscalls.c | 55 ++++++++++++++++++-- >> tools/testing/selftests/landlock/base_test.c | 2 +- >> 10 files changed, 241 insertions(+), 13 deletions(-) >> create mode 100644 security/landlock/socket.c >> create mode 100644 security/landlock/socket.h >> >> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h >> index 25c8d7677..8551ade38 100644 >> --- a/include/uapi/linux/landlock.h >> +++ b/include/uapi/linux/landlock.h >> @@ -37,6 +37,13 @@ struct landlock_ruleset_attr { >> * rule explicitly allow them. >> */ >> __u64 handled_access_net; >> + >> + /** >> + * @handled_access_net: Bitmask of actions (cf. `Socket flags`_) >> + * that is handled by this ruleset and should then be forbidden if no >> + * rule explicitly allow them. >> + */ >> + __u64 handled_access_socket; >> }; >> >> /* >> @@ -65,6 +72,11 @@ enum landlock_rule_type { >> * landlock_net_port_attr . >> */ >> LANDLOCK_RULE_NET_PORT, >> + /** >> + * @LANDLOCK_RULE_SOCKET: Type of a &struct >> + * landlock_socket_attr . >> + */ >> + LANDLOCK_RULE_SOCKET, >> }; >> >> /** >> @@ -115,6 +127,27 @@ struct landlock_net_port_attr { >> __u64 port; >> }; >> >> +/** >> + * struct landlock_socket_attr - Socket definition >> + * >> + * Argument of sys_landlock_add_rule(). >> + */ >> +struct landlock_socket_attr { >> + /** >> + * @allowed_access: Bitmask of allowed access for a socket >> + * (cf. `Socket flags`_). >> + */ >> + __u64 allowed_access; >> + /** >> + * @domain: Protocol family used for communication (see socket(2)). >> + */ >> + int domain; >> + /** >> + * @type: Socket type (see socket(2)). >> + */ >> + int type; >> +}; >> + >> /** >> * DOC: fs_access >> * >> @@ -244,4 +277,20 @@ struct landlock_net_port_attr { >> #define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) >> #define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) >> /* clang-format on */ >> + >> +/** >> + * DOC: socket_acess >> + * >> + * Socket flags >> + * ~~~~~~~~~~~~~~~~ >> + * >> + * These flags enable to restrict a sandboxed process to a set of >> + * socket-related actions for specific protocols. This is supported >> + * since the Landlock ABI version 5. >> + * >> + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket >> + */ >> +/* clang-format off */ >> +#define LANDLOCK_ACCESS_SOCKET_CREATE (1ULL << 0) >> +/* clang-format on */ >> #endif /* _UAPI_LINUX_LANDLOCK_H */ >> diff --git a/security/landlock/Makefile b/security/landlock/Makefile >> index b4538b7cf..ff1dd98f6 100644 >> --- a/security/landlock/Makefile >> +++ b/security/landlock/Makefile >> @@ -1,6 +1,6 @@ >> obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o >> >> landlock-y := setup.o syscalls.o object.o ruleset.o \ >> - cred.o task.o fs.o >> + cred.o task.o fs.o socket.o >> >> landlock-$(CONFIG_INET) += net.o >> diff --git a/security/landlock/limits.h b/security/landlock/limits.h >> index 93c9c6f91..ebdab587c 100644 >> --- a/security/landlock/limits.h >> +++ b/security/landlock/limits.h >> @@ -28,6 +28,11 @@ >> #define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET) >> #define LANDLOCK_SHIFT_ACCESS_NET LANDLOCK_NUM_ACCESS_FS >> >> +#define LANDLOCK_LAST_ACCESS_SOCKET LANDLOCK_ACCESS_SOCKET_CREATE >> +#define LANDLOCK_MASK_ACCESS_SOCKET ((LANDLOCK_LAST_ACCESS_SOCKET << 1) - 1) >> +#define LANDLOCK_NUM_ACCESS_SOCKET __const_hweight64(LANDLOCK_MASK_ACCESS_SOCKET) >> +#define LANDLOCK_SHIFT_ACCESS_SOCKET LANDLOCK_NUM_ACCESS_SOCKET >> + >> /* clang-format on */ >> >> #endif /* _SECURITY_LANDLOCK_LIMITS_H */ >> diff --git a/security/landlock/net.c b/security/landlock/net.c >> index c8bcd29bd..0e3770d14 100644 >> --- a/security/landlock/net.c >> +++ b/security/landlock/net.c >> @@ -159,7 +159,7 @@ static int current_check_access_socket(struct socket *const sock, >> return -EINVAL; >> } >> >> - id.key.data = (__force uintptr_t)port; >> + id.key.data = (__force u64)port; >> BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data)); >> >> rule = landlock_find_rule(dom, id); >> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c >> index e0a5fbf92..1f1ed8181 100644 >> --- a/security/landlock/ruleset.c >> +++ b/security/landlock/ruleset.c >> @@ -40,6 +40,7 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) >> #if IS_ENABLED(CONFIG_INET) >> new_ruleset->root_net_port = RB_ROOT; >> #endif /* IS_ENABLED(CONFIG_INET) */ >> + new_ruleset->root_socket = RB_ROOT; >> >> new_ruleset->num_layers = num_layers; >> /* >> @@ -52,12 +53,13 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) >> >> struct landlock_ruleset * >> landlock_create_ruleset(const access_mask_t fs_access_mask, >> - const access_mask_t net_access_mask) >> + const access_mask_t net_access_mask, >> + const access_mask_t socket_access_mask) >> { >> struct landlock_ruleset *new_ruleset; >> >> /* Informs about useless ruleset. */ >> - if (!fs_access_mask && !net_access_mask) >> + if (!fs_access_mask && !net_access_mask && !socket_access_mask) >> return ERR_PTR(-ENOMSG); >> new_ruleset = create_ruleset(1); >> if (IS_ERR(new_ruleset)) >> @@ -66,6 +68,8 @@ landlock_create_ruleset(const access_mask_t 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); >> + if (socket_access_mask) >> + landlock_add_socket_access_mask(new_ruleset, socket_access_mask, 0); >> return new_ruleset; >> } >> >> @@ -89,6 +93,9 @@ static bool is_object_pointer(const enum landlock_key_type key_type) >> return false; >> #endif /* IS_ENABLED(CONFIG_INET) */ >> >> + case LANDLOCK_KEY_SOCKET: >> + return false; >> + >> default: >> WARN_ON_ONCE(1); >> return false; >> @@ -146,6 +153,9 @@ static struct rb_root *get_root(struct landlock_ruleset *const ruleset, >> return &ruleset->root_net_port; >> #endif /* IS_ENABLED(CONFIG_INET) */ >> >> + case LANDLOCK_KEY_SOCKET: >> + return &ruleset->root_socket; >> + >> default: >> WARN_ON_ONCE(1); >> return ERR_PTR(-EINVAL); >> @@ -175,7 +185,8 @@ static void build_check_ruleset(void) >> 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_NET << LANDLOCK_SHIFT_ACCESS_NET))); >> + (LANDLOCK_MASK_ACCESS_NET << LANDLOCK_SHIFT_ACCESS_NET) | >> + (LANDLOCK_MASK_ACCESS_SOCKET << LANDLOCK_SHIFT_ACCESS_SOCKET))); >> } >> >> /** >> @@ -399,6 +410,11 @@ static int merge_ruleset(struct landlock_ruleset *const dst, >> goto out_unlock; >> #endif /* IS_ENABLED(CONFIG_INET) */ >> >> + /* Merges the @src socket tree. */ >> + err = merge_tree(dst, src, LANDLOCK_KEY_SOCKET); >> + if (err) >> + goto out_unlock; >> + >> out_unlock: >> mutex_unlock(&src->lock); >> mutex_unlock(&dst->lock); >> @@ -462,6 +478,11 @@ static int inherit_ruleset(struct landlock_ruleset *const parent, >> goto out_unlock; >> #endif /* IS_ENABLED(CONFIG_INET) */ >> >> + /* Copies the @parent socket tree. */ >> + err = inherit_tree(parent, child, LANDLOCK_KEY_SOCKET); >> + if (err) >> + goto out_unlock; >> + >> if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) { >> err = -EINVAL; >> goto out_unlock; >> @@ -498,6 +519,10 @@ static void free_ruleset(struct landlock_ruleset *const ruleset) >> free_rule(freeme, LANDLOCK_KEY_NET_PORT); >> #endif /* IS_ENABLED(CONFIG_INET) */ >> >> + rbtree_postorder_for_each_entry_safe(freeme, next, >> + &ruleset->root_socket, node) >> + free_rule(freeme, LANDLOCK_KEY_SOCKET); >> + >> put_hierarchy(ruleset->hierarchy); >> kfree(ruleset); >> } >> @@ -708,6 +733,10 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain, >> break; >> #endif /* IS_ENABLED(CONFIG_INET) */ >> >> + case LANDLOCK_KEY_SOCKET: >> + get_access_mask = landlock_get_socket_access_mask; >> + num_access = LANDLOCK_NUM_ACCESS_SOCKET; >> + break; >> default: >> WARN_ON_ONCE(1); >> return 0; >> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h >> index c7f152678..f4213db09 100644 >> --- a/security/landlock/ruleset.h >> +++ b/security/landlock/ruleset.h >> @@ -72,10 +72,10 @@ union landlock_key { >> */ >> struct landlock_object *object; >> /** >> - * @data: Raw data to identify an arbitrary 32-bit value >> + * @data: Raw data to identify an arbitrary 64-bit value >> * (e.g. a TCP port). >> */ >> - uintptr_t data; >> + u64 data; >> }; >> >> /** >> @@ -92,6 +92,12 @@ enum landlock_key_type { >> * node keys. >> */ >> LANDLOCK_KEY_NET_PORT, >> + >> + /** >> + * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's >> + * node keys. >> + */ >> + LANDLOCK_KEY_SOCKET, >> }; >> >> /** >> @@ -177,6 +183,15 @@ struct landlock_ruleset { >> struct rb_root root_net_port; >> #endif /* IS_ENABLED(CONFIG_INET) */ >> >> + /** >> + * @root_socket: Root of a red-black tree containing &struct >> + * landlock_rule nodes with socket type, described by (domain, type) >> + * pair (see socket(2)). 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_socket; >> + >> /** >> * @hierarchy: Enables hierarchy identification even when a parent >> * domain vanishes. This is needed for the ptrace protection. >> @@ -233,7 +248,8 @@ struct landlock_ruleset { >> >> struct landlock_ruleset * >> landlock_create_ruleset(const access_mask_t access_mask_fs, >> - const access_mask_t access_mask_net); >> + const access_mask_t access_mask_net, >> + const access_mask_t socket_access_mask); >> >> void landlock_put_ruleset(struct landlock_ruleset *const ruleset); >> void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset); >> @@ -282,6 +298,19 @@ landlock_add_net_access_mask(struct landlock_ruleset *const ruleset, >> (net_mask << LANDLOCK_SHIFT_ACCESS_NET); >> } >> >> +static inline void >> +landlock_add_socket_access_mask(struct landlock_ruleset *const ruleset, >> + const access_mask_t socket_access_mask, >> + const u16 layer_level) >> +{ >> + access_mask_t socket_mask = socket_access_mask & LANDLOCK_MASK_ACCESS_SOCKET; >> + >> + /* Should already be checked in sys_landlock_create_ruleset(). */ >> + WARN_ON_ONCE(socket_access_mask != socket_mask); >> + ruleset->access_masks[layer_level] |= >> + (socket_mask << LANDLOCK_SHIFT_ACCESS_SOCKET); >> +} >> + >> static inline access_mask_t >> landlock_get_raw_fs_access_mask(const struct landlock_ruleset *const ruleset, >> const u16 layer_level) >> @@ -309,6 +338,15 @@ landlock_get_net_access_mask(const struct landlock_ruleset *const ruleset, >> LANDLOCK_MASK_ACCESS_NET; >> } >> >> +static inline access_mask_t >> +landlock_get_socket_access_mask(const struct landlock_ruleset *const ruleset, >> + const u16 layer_level) >> +{ >> + return (ruleset->access_masks[layer_level] >> >> + LANDLOCK_SHIFT_ACCESS_SOCKET) & >> + LANDLOCK_MASK_ACCESS_SOCKET; >> +} >> + >> bool landlock_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/socket.c b/security/landlock/socket.c >> new file mode 100644 >> index 000000000..88b4ef3a1 >> --- /dev/null >> +++ b/security/landlock/socket.c >> @@ -0,0 +1,43 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Landlock LSM - Socket management and hooks >> + * >> + * Copyright © 2024 Huawei Tech. Co., Ltd. >> + */ >> + >> +#include "limits.h" >> +#include "ruleset.h" >> +#include "socket.h" >> + >> +union socket_key { >> + struct { >> + int domain; >> + int type; >> + } __packed content; >> + u64 val; >> +}; >> + >> +int landlock_append_socket_rule(struct landlock_ruleset *const ruleset, >> + const int domain, const int type, access_mask_t access_rights) >> +{ >> + int err; >> + const union socket_key socket_key = { >> + .content.domain = domain, >> + .content.type = type >> + }; > > I'm not convinced this landlock_key.data needs to be changed to u64. We > could have an helper to fit the SOCK_MAX and AF_MAX values into 32-bits, > and a related built-time check to make sure this works. agreed > >> + >> + const struct landlock_id id = { >> + .key.data = socket_key.val, >> + .type = LANDLOCK_KEY_SOCKET, >> + };
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h index 25c8d7677..8551ade38 100644 --- a/include/uapi/linux/landlock.h +++ b/include/uapi/linux/landlock.h @@ -37,6 +37,13 @@ struct landlock_ruleset_attr { * rule explicitly allow them. */ __u64 handled_access_net; + + /** + * @handled_access_net: Bitmask of actions (cf. `Socket flags`_) + * that is handled by this ruleset and should then be forbidden if no + * rule explicitly allow them. + */ + __u64 handled_access_socket; }; /* @@ -65,6 +72,11 @@ enum landlock_rule_type { * landlock_net_port_attr . */ LANDLOCK_RULE_NET_PORT, + /** + * @LANDLOCK_RULE_SOCKET: Type of a &struct + * landlock_socket_attr . + */ + LANDLOCK_RULE_SOCKET, }; /** @@ -115,6 +127,27 @@ struct landlock_net_port_attr { __u64 port; }; +/** + * struct landlock_socket_attr - Socket definition + * + * Argument of sys_landlock_add_rule(). + */ +struct landlock_socket_attr { + /** + * @allowed_access: Bitmask of allowed access for a socket + * (cf. `Socket flags`_). + */ + __u64 allowed_access; + /** + * @domain: Protocol family used for communication (see socket(2)). + */ + int domain; + /** + * @type: Socket type (see socket(2)). + */ + int type; +}; + /** * DOC: fs_access * @@ -244,4 +277,20 @@ struct landlock_net_port_attr { #define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0) #define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1) /* clang-format on */ + +/** + * DOC: socket_acess + * + * Socket flags + * ~~~~~~~~~~~~~~~~ + * + * These flags enable to restrict a sandboxed process to a set of + * socket-related actions for specific protocols. This is supported + * since the Landlock ABI version 5. + * + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket + */ +/* clang-format off */ +#define LANDLOCK_ACCESS_SOCKET_CREATE (1ULL << 0) +/* clang-format on */ #endif /* _UAPI_LINUX_LANDLOCK_H */ diff --git a/security/landlock/Makefile b/security/landlock/Makefile index b4538b7cf..ff1dd98f6 100644 --- a/security/landlock/Makefile +++ b/security/landlock/Makefile @@ -1,6 +1,6 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o landlock-y := setup.o syscalls.o object.o ruleset.o \ - cred.o task.o fs.o + cred.o task.o fs.o socket.o landlock-$(CONFIG_INET) += net.o diff --git a/security/landlock/limits.h b/security/landlock/limits.h index 93c9c6f91..ebdab587c 100644 --- a/security/landlock/limits.h +++ b/security/landlock/limits.h @@ -28,6 +28,11 @@ #define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET) #define LANDLOCK_SHIFT_ACCESS_NET LANDLOCK_NUM_ACCESS_FS +#define LANDLOCK_LAST_ACCESS_SOCKET LANDLOCK_ACCESS_SOCKET_CREATE +#define LANDLOCK_MASK_ACCESS_SOCKET ((LANDLOCK_LAST_ACCESS_SOCKET << 1) - 1) +#define LANDLOCK_NUM_ACCESS_SOCKET __const_hweight64(LANDLOCK_MASK_ACCESS_SOCKET) +#define LANDLOCK_SHIFT_ACCESS_SOCKET LANDLOCK_NUM_ACCESS_SOCKET + /* clang-format on */ #endif /* _SECURITY_LANDLOCK_LIMITS_H */ diff --git a/security/landlock/net.c b/security/landlock/net.c index c8bcd29bd..0e3770d14 100644 --- a/security/landlock/net.c +++ b/security/landlock/net.c @@ -159,7 +159,7 @@ static int current_check_access_socket(struct socket *const sock, return -EINVAL; } - id.key.data = (__force uintptr_t)port; + id.key.data = (__force u64)port; BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data)); rule = landlock_find_rule(dom, id); diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index e0a5fbf92..1f1ed8181 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c @@ -40,6 +40,7 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) #if IS_ENABLED(CONFIG_INET) new_ruleset->root_net_port = RB_ROOT; #endif /* IS_ENABLED(CONFIG_INET) */ + new_ruleset->root_socket = RB_ROOT; new_ruleset->num_layers = num_layers; /* @@ -52,12 +53,13 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) struct landlock_ruleset * landlock_create_ruleset(const access_mask_t fs_access_mask, - const access_mask_t net_access_mask) + const access_mask_t net_access_mask, + const access_mask_t socket_access_mask) { struct landlock_ruleset *new_ruleset; /* Informs about useless ruleset. */ - if (!fs_access_mask && !net_access_mask) + if (!fs_access_mask && !net_access_mask && !socket_access_mask) return ERR_PTR(-ENOMSG); new_ruleset = create_ruleset(1); if (IS_ERR(new_ruleset)) @@ -66,6 +68,8 @@ landlock_create_ruleset(const access_mask_t 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); + if (socket_access_mask) + landlock_add_socket_access_mask(new_ruleset, socket_access_mask, 0); return new_ruleset; } @@ -89,6 +93,9 @@ static bool is_object_pointer(const enum landlock_key_type key_type) return false; #endif /* IS_ENABLED(CONFIG_INET) */ + case LANDLOCK_KEY_SOCKET: + return false; + default: WARN_ON_ONCE(1); return false; @@ -146,6 +153,9 @@ static struct rb_root *get_root(struct landlock_ruleset *const ruleset, return &ruleset->root_net_port; #endif /* IS_ENABLED(CONFIG_INET) */ + case LANDLOCK_KEY_SOCKET: + return &ruleset->root_socket; + default: WARN_ON_ONCE(1); return ERR_PTR(-EINVAL); @@ -175,7 +185,8 @@ static void build_check_ruleset(void) 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_NET << LANDLOCK_SHIFT_ACCESS_NET))); + (LANDLOCK_MASK_ACCESS_NET << LANDLOCK_SHIFT_ACCESS_NET) | + (LANDLOCK_MASK_ACCESS_SOCKET << LANDLOCK_SHIFT_ACCESS_SOCKET))); } /** @@ -399,6 +410,11 @@ static int merge_ruleset(struct landlock_ruleset *const dst, goto out_unlock; #endif /* IS_ENABLED(CONFIG_INET) */ + /* Merges the @src socket tree. */ + err = merge_tree(dst, src, LANDLOCK_KEY_SOCKET); + if (err) + goto out_unlock; + out_unlock: mutex_unlock(&src->lock); mutex_unlock(&dst->lock); @@ -462,6 +478,11 @@ static int inherit_ruleset(struct landlock_ruleset *const parent, goto out_unlock; #endif /* IS_ENABLED(CONFIG_INET) */ + /* Copies the @parent socket tree. */ + err = inherit_tree(parent, child, LANDLOCK_KEY_SOCKET); + if (err) + goto out_unlock; + if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) { err = -EINVAL; goto out_unlock; @@ -498,6 +519,10 @@ static void free_ruleset(struct landlock_ruleset *const ruleset) free_rule(freeme, LANDLOCK_KEY_NET_PORT); #endif /* IS_ENABLED(CONFIG_INET) */ + rbtree_postorder_for_each_entry_safe(freeme, next, + &ruleset->root_socket, node) + free_rule(freeme, LANDLOCK_KEY_SOCKET); + put_hierarchy(ruleset->hierarchy); kfree(ruleset); } @@ -708,6 +733,10 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain, break; #endif /* IS_ENABLED(CONFIG_INET) */ + case LANDLOCK_KEY_SOCKET: + get_access_mask = landlock_get_socket_access_mask; + num_access = LANDLOCK_NUM_ACCESS_SOCKET; + break; default: WARN_ON_ONCE(1); return 0; diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h index c7f152678..f4213db09 100644 --- a/security/landlock/ruleset.h +++ b/security/landlock/ruleset.h @@ -72,10 +72,10 @@ union landlock_key { */ struct landlock_object *object; /** - * @data: Raw data to identify an arbitrary 32-bit value + * @data: Raw data to identify an arbitrary 64-bit value * (e.g. a TCP port). */ - uintptr_t data; + u64 data; }; /** @@ -92,6 +92,12 @@ enum landlock_key_type { * node keys. */ LANDLOCK_KEY_NET_PORT, + + /** + * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's + * node keys. + */ + LANDLOCK_KEY_SOCKET, }; /** @@ -177,6 +183,15 @@ struct landlock_ruleset { struct rb_root root_net_port; #endif /* IS_ENABLED(CONFIG_INET) */ + /** + * @root_socket: Root of a red-black tree containing &struct + * landlock_rule nodes with socket type, described by (domain, type) + * pair (see socket(2)). 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_socket; + /** * @hierarchy: Enables hierarchy identification even when a parent * domain vanishes. This is needed for the ptrace protection. @@ -233,7 +248,8 @@ struct landlock_ruleset { struct landlock_ruleset * landlock_create_ruleset(const access_mask_t access_mask_fs, - const access_mask_t access_mask_net); + const access_mask_t access_mask_net, + const access_mask_t socket_access_mask); void landlock_put_ruleset(struct landlock_ruleset *const ruleset); void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset); @@ -282,6 +298,19 @@ landlock_add_net_access_mask(struct landlock_ruleset *const ruleset, (net_mask << LANDLOCK_SHIFT_ACCESS_NET); } +static inline void +landlock_add_socket_access_mask(struct landlock_ruleset *const ruleset, + const access_mask_t socket_access_mask, + const u16 layer_level) +{ + access_mask_t socket_mask = socket_access_mask & LANDLOCK_MASK_ACCESS_SOCKET; + + /* Should already be checked in sys_landlock_create_ruleset(). */ + WARN_ON_ONCE(socket_access_mask != socket_mask); + ruleset->access_masks[layer_level] |= + (socket_mask << LANDLOCK_SHIFT_ACCESS_SOCKET); +} + static inline access_mask_t landlock_get_raw_fs_access_mask(const struct landlock_ruleset *const ruleset, const u16 layer_level) @@ -309,6 +338,15 @@ landlock_get_net_access_mask(const struct landlock_ruleset *const ruleset, LANDLOCK_MASK_ACCESS_NET; } +static inline access_mask_t +landlock_get_socket_access_mask(const struct landlock_ruleset *const ruleset, + const u16 layer_level) +{ + return (ruleset->access_masks[layer_level] >> + LANDLOCK_SHIFT_ACCESS_SOCKET) & + LANDLOCK_MASK_ACCESS_SOCKET; +} + bool landlock_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/socket.c b/security/landlock/socket.c new file mode 100644 index 000000000..88b4ef3a1 --- /dev/null +++ b/security/landlock/socket.c @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Landlock LSM - Socket management and hooks + * + * Copyright © 2024 Huawei Tech. Co., Ltd. + */ + +#include "limits.h" +#include "ruleset.h" +#include "socket.h" + +union socket_key { + struct { + int domain; + int type; + } __packed content; + u64 val; +}; + +int landlock_append_socket_rule(struct landlock_ruleset *const ruleset, + const int domain, const int type, access_mask_t access_rights) +{ + int err; + const union socket_key socket_key = { + .content.domain = domain, + .content.type = type + }; + + const struct landlock_id id = { + .key.data = socket_key.val, + .type = LANDLOCK_KEY_SOCKET, + }; + + /* Transforms relative access rights to absolute ones. */ + access_rights |= LANDLOCK_MASK_ACCESS_SOCKET & + ~landlock_get_socket_access_mask(ruleset, 0); + + mutex_lock(&ruleset->lock); + err = landlock_insert_rule(ruleset, id, access_rights); + mutex_unlock(&ruleset->lock); + + return err; +} diff --git a/security/landlock/socket.h b/security/landlock/socket.h new file mode 100644 index 000000000..2b8f9ae7d --- /dev/null +++ b/security/landlock/socket.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Landlock LSM - Socket management and hooks + * + * Copyright © 2024 Huawei Tech. Co., Ltd. + */ + +#ifndef _SECURITY_LANDLOCK_SOCKET_H +#define _SECURITY_LANDLOCK_SOCKET_H + +#include "ruleset.h" + +int landlock_append_socket_rule(struct landlock_ruleset *const ruleset, + const int domain, const int type, + access_mask_t access_rights); + +#endif /* _SECURITY_LANDLOCK_SOCKET_H */ diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c index 6788e73b6..66c9800c2 100644 --- a/security/landlock/syscalls.c +++ b/security/landlock/syscalls.c @@ -30,6 +30,7 @@ #include "fs.h" #include "limits.h" #include "net.h" +#include "socket.h" #include "ruleset.h" #include "setup.h" @@ -88,7 +89,8 @@ static void build_check_abi(void) struct landlock_ruleset_attr ruleset_attr; struct landlock_path_beneath_attr path_beneath_attr; struct landlock_net_port_attr net_port_attr; - size_t ruleset_size, path_beneath_size, net_port_size; + struct landlock_socket_attr socket_attr; + size_t ruleset_size, path_beneath_size, net_port_size, socket_size; /* * For each user space ABI structures, first checks that there is no @@ -97,8 +99,9 @@ static void build_check_abi(void) */ ruleset_size = sizeof(ruleset_attr.handled_access_fs); ruleset_size += sizeof(ruleset_attr.handled_access_net); + ruleset_size += sizeof(ruleset_attr.handled_access_socket); BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size); - BUILD_BUG_ON(sizeof(ruleset_attr) != 16); + BUILD_BUG_ON(sizeof(ruleset_attr) != 24); path_beneath_size = sizeof(path_beneath_attr.allowed_access); path_beneath_size += sizeof(path_beneath_attr.parent_fd); @@ -109,6 +112,12 @@ static void build_check_abi(void) net_port_size += sizeof(net_port_attr.port); BUILD_BUG_ON(sizeof(net_port_attr) != net_port_size); BUILD_BUG_ON(sizeof(net_port_attr) != 16); + + socket_size = sizeof(socket_attr.allowed_access); + socket_size += sizeof(socket_attr.domain); + socket_size += sizeof(socket_attr.type); + BUILD_BUG_ON(sizeof(socket_attr) != socket_size); + BUILD_BUG_ON(sizeof(socket_attr) != 16); } /* Ruleset handling */ @@ -149,7 +158,7 @@ static const struct file_operations ruleset_fops = { .write = fop_dummy_write, }; -#define LANDLOCK_ABI_VERSION 4 +#define LANDLOCK_ABI_VERSION 5 /** * sys_landlock_create_ruleset - Create a new ruleset @@ -213,9 +222,15 @@ SYSCALL_DEFINE3(landlock_create_ruleset, LANDLOCK_MASK_ACCESS_NET) return -EINVAL; + /* Checks socket content (and 32-bits cast). */ + if ((ruleset_attr.handled_access_socket | LANDLOCK_MASK_ACCESS_SOCKET) != + LANDLOCK_MASK_ACCESS_SOCKET) + return -EINVAL; + /* Checks arguments and transforms to kernel struct. */ ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs, - ruleset_attr.handled_access_net); + ruleset_attr.handled_access_net, + ruleset_attr.handled_access_socket); if (IS_ERR(ruleset)) return PTR_ERR(ruleset); @@ -371,6 +386,35 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset, net_port_attr.allowed_access); } +static int add_rule_socket(struct landlock_ruleset *ruleset, + const void __user *const rule_attr) +{ + struct landlock_socket_attr socket_attr; + int res; + access_mask_t mask; + + /* Copies raw user space buffer. */ + res = copy_from_user(&socket_attr, rule_attr, sizeof(socket_attr)); + if (res) + return -EFAULT; + + /* + * Informs about useless rule: empty allowed_access (i.e. deny rules) + * are ignored by network actions. + */ + if (!socket_attr.allowed_access) + return -ENOMSG; + + /* Checks that allowed_access matches the @ruleset constraints. */ + mask = landlock_get_socket_access_mask(ruleset, 0); + if ((socket_attr.allowed_access | mask) != mask) + return -EINVAL; + + /* Imports the new rule. */ + return landlock_append_socket_rule(ruleset, socket_attr.domain, + socket_attr.type, socket_attr.allowed_access); +} + /** * sys_landlock_add_rule - Add a new rule to a ruleset * @@ -429,6 +473,9 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd, case LANDLOCK_RULE_NET_PORT: err = add_rule_net_port(ruleset, rule_attr); break; + case LANDLOCK_RULE_SOCKET: + err = add_rule_socket(ruleset, rule_attr); + break; default: err = -EINVAL; break; diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c index 646f778df..d292b419c 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(4, landlock_create_ruleset(NULL, 0, + ASSERT_EQ(5, landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION)); ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,