| Message ID | ZgX5TRTrSDPrJFfF@tahera-OptiPlex-5000 (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v2] landlock: Add abstract unix socket connect restrictions | expand |
Thanks for this patch. Please CC the netdev mailing list too, they may be interested by this feature. I also added a few folks that previously showed their interest for this feature. On Thu, Mar 28, 2024 at 05:12:13PM -0600, TaheraFahimi wrote: > Abstract unix sockets are used for local interprocess communication without > relying on filesystem. Since landlock has no restriction for connecting to > a UNIX socket in the abstract namespace, a sandboxed process can connect to > a socket outside the sandboxed environment. Access to such sockets should > be scoped the same way ptrace access is limited. This is good but it would be better to explain that Landlock doesn't currently control abstract unix sockets and that it would make sense for a sandbox. > > For a landlocked process to be allowed to connect to a target process, it > must have a subset of the target process’s rules (the connecting socket > must be in a sub-domain of the listening socket). This patch adds a new > LSM hook for connect function in unix socket with the related access rights. Because of compatibility reasons, and because Landlock should be flexible, we need to extend the user space interface. As explained in the GitHub issue, we need to add a new "scoped" field to the landlock_ruleset_attr struct. This field will optionally contain a LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET flag to specify that this ruleset will deny any connection from within the sandbox to its parents (i.e. any parent sandbox or not-sandboxed processes). > > Link to first draft: > https://lore.kernel.org/outreachy/20240328.ShoR4Iecei8o@digikod.net/ You can move this sentence in the below changelog. > You can add this: Closes: https://github.com/landlock-lsm/linux/issues/7 > Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com> Your Git (or email app) configuration doesn't use the same name as here. Please run ./scripts/checkpatch.pl on this patch and fix the warnings. > > ---- > Changes in v2: > - Remove wrapper functions, noted by Casey Schaufler <casey@schaufler-ca.com> > --- > security/landlock/task.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/security/landlock/task.c b/security/landlock/task.c > index 849f5123610b..67528f87b7de 100644 > --- a/security/landlock/task.c > +++ b/security/landlock/task.c > @@ -13,6 +13,7 @@ > #include <linux/lsm_hooks.h> > #include <linux/rcupdate.h> > #include <linux/sched.h> > +#include <net/sock.h> > > #include "common.h" > #include "cred.h" > @@ -108,9 +109,48 @@ static int hook_ptrace_traceme(struct task_struct *const parent) > return task_ptrace(parent, current); > } > > +static bool unix_sock_is_scoped(struct sock *const sock, For consistency with task_is_scoped(), you can rename this to sock_is_scoped(). > + struct sock *const other) > +{ > + bool is_scoped = true; > + > + /* get the ruleset of connecting sock*/ These comments don't help more than the following line, you can remove them. > + const struct landlock_ruleset *const dom_sock = According to the name it looks like the domain of the socket but it is just the domain of the current task. Just "dom" would be clearer and more consistent with security/landlock/fs.c > + landlock_get_current_domain(); > + > + if (!dom_sock) > + return true; > + > + /* get credential of listening sock*/ > + const struct cred *cred_other = get_cred(other->sk_peer_cred); We have a get but not a put call, so the credentials will never be freed. The put call must be called before any return, so you probably need to follow the goto/error pattern. In the context of these LSM hooks, only unix_listen() sets the "other" socket credential, and unix_listen() is guarded by unix_state_lock() which locks unix_sk(s)->lock . When security_unix_stream_connect() or security_unix_may_send() are called, unix_sk(s)->lock is locked as well, which protects the credentials against race-conditions (TOCTOU: time-of-check to time-of-use). We should then make that explicit with this assertion (which also documents it): lockdep_assert_held(&unix_sk(other)->lock); In theory it is then not required to call get_cred(). However, because the performance impact should be negligible and to avoid a potential use-after-free (not possible in theory with the current code), it would be safer to still call get/put. It would be worse to have a use-after-free rather than an access control issue. Another thing to keep in mind is that for this hook to be race-condition-free, the credential must not change anyway. A comment should highlight that. > + > + if (!cred_other) > + return true; > + > + /* retrieve the landlock_rulesets */ > + const struct landlock_ruleset *dom_parent; All declarations should be at the top of functions. > + > + rcu_read_lock(); No need for this RCU lock because the lock is managed by unix_state_lock() in this case. > + dom_parent = landlock_cred(cred_other)->domain; > + is_scoped = domain_scope_le(dom_parent, dom_sock); > + rcu_read_unlock(); > + > + return is_scoped; > +} > + > +static int hook_unix_stream_connect(struct sock *const sock, > + struct sock *const other, > + struct sock *const newsk) > +{ > + if (unix_sock_is_scoped(sock, other)) > + return 0; > + return -EPERM; > +} > + > static struct security_hook_list landlock_hooks[] __ro_after_init = { > LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check), > LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme), > + LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect), Please add a hook for security_unix_may_send() too, it should be quite similar, and simplify the patch's subject accordingly. You now need to add tests (in a dedicated patch) extending tools/testing/selftests/landlock/ptrace_test.c (I'll rename the file later). These tests should also check with unnamed and named unix sockets. I guess the current code doesn't differentiate them and control all kind of unix sockets. Because they must explicitly be passed, sockets created with socketpair(2) (i.e. unnamed socket) should never be denied. > }; > > __init void landlock_add_task_hooks(void) > -- > 2.34.1 > >
On Tue, Apr 02, 2024 at 11:53:09AM +0200, Mickaël Salaün wrote: > Thanks for this patch. Please CC the netdev mailing list too, they may > be interested by this feature. I also added a few folks that previously > showed their interest for this feature. > > On Thu, Mar 28, 2024 at 05:12:13PM -0600, TaheraFahimi wrote: > > Abstract unix sockets are used for local interprocess communication without > > relying on filesystem. Since landlock has no restriction for connecting to > > a UNIX socket in the abstract namespace, a sandboxed process can connect to > > a socket outside the sandboxed environment. Access to such sockets should > > be scoped the same way ptrace access is limited. > > This is good but it would be better to explain that Landlock doesn't > currently control abstract unix sockets and that it would make sense for > a sandbox. > > > > > > For a landlocked process to be allowed to connect to a target process, it > > must have a subset of the target process’s rules (the connecting socket > > must be in a sub-domain of the listening socket). This patch adds a new > > LSM hook for connect function in unix socket with the related access rights. > > Because of compatibility reasons, and because Landlock should be > flexible, we need to extend the user space interface. As explained in > the GitHub issue, we need to add a new "scoped" field to the > landlock_ruleset_attr struct. This field will optionally contain a > LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET flag to specify that this > ruleset will deny any connection from within the sandbox to its parents > (i.e. any parent sandbox or not-sandboxed processes). Thanks for the feedback. Here is what I understood, please correct me if I am wrong. First, I should add another field to the landlock_ruleset_attr (a field like handled_access_net, but for the unix sockets) with a flag LANDLOCK_ACCESS_UNIX_CONNECT (it is a flag like LANDLOCK_ACCESS_NET_CONNECT_TCP but fot the unix sockets connect). > > > > Link to first draft: > > https://lore.kernel.org/outreachy/20240328.ShoR4Iecei8o@digikod.net/ > > You can move this sentence in the below changelog. > > > > > You can add this: > > Closes: https://github.com/landlock-lsm/linux/issues/7 > > > Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com> > > Your Git (or email app) configuration doesn't use the same name as here. > > Please run ./scripts/checkpatch.pl on this patch and fix the warnings. > > > > > ---- > > Changes in v2: > > - Remove wrapper functions, noted by Casey Schaufler <casey@schaufler-ca.com> > > --- > > security/landlock/task.c | 40 ++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 40 insertions(+) > > > > diff --git a/security/landlock/task.c b/security/landlock/task.c > > index 849f5123610b..67528f87b7de 100644 > > --- a/security/landlock/task.c > > +++ b/security/landlock/task.c > > @@ -13,6 +13,7 @@ > > #include <linux/lsm_hooks.h> > > #include <linux/rcupdate.h> > > #include <linux/sched.h> > > +#include <net/sock.h> > > > > #include "common.h" > > #include "cred.h" > > @@ -108,9 +109,48 @@ static int hook_ptrace_traceme(struct task_struct *const parent) > > return task_ptrace(parent, current); > > } > > > > +static bool unix_sock_is_scoped(struct sock *const sock, > > For consistency with task_is_scoped(), you can rename this to > sock_is_scoped(). > > > + struct sock *const other) > > +{ > > + bool is_scoped = true; > > + > > + /* get the ruleset of connecting sock*/ > > These comments don't help more than the following line, you can remove > them. > > > + const struct landlock_ruleset *const dom_sock = > > According to the name it looks like the domain of the socket but it is > just the domain of the current task. Just "dom" would be clearer and > more consistent with security/landlock/fs.c > > > + landlock_get_current_domain(); > > + > > + if (!dom_sock) > > + return true; > > + > > + /* get credential of listening sock*/ > > + const struct cred *cred_other = get_cred(other->sk_peer_cred); > > We have a get but not a put call, so the credentials will never be > freed. The put call must be called before any return, so you > probably need to follow the goto/error pattern. > > In the context of these LSM hooks, only unix_listen() sets the "other" > socket credential, and unix_listen() is guarded by unix_state_lock() > which locks unix_sk(s)->lock . When security_unix_stream_connect() or > security_unix_may_send() are called, unix_sk(s)->lock is locked as well, > which protects the credentials against race-conditions (TOCTOU: > time-of-check to time-of-use). We should then make that explicit with > this assertion (which also documents it): > > lockdep_assert_held(&unix_sk(other)->lock); > > In theory it is then not required to call get_cred(). However, because > the performance impact should be negligible and to avoid a potential > use-after-free (not possible in theory with the current code), it would > be safer to still call get/put. It would be worse to have a > use-after-free rather than an access control issue. > > Another thing to keep in mind is that for this hook to be > race-condition-free, the credential must not change anyway. A comment > should highlight that. > > > + > > + if (!cred_other) > > + return true; > > + > > + /* retrieve the landlock_rulesets */ > > + const struct landlock_ruleset *dom_parent; > > All declarations should be at the top of functions. > > > + > > + rcu_read_lock(); > > No need for this RCU lock because the lock is managed by > unix_state_lock() in this case. > > > + dom_parent = landlock_cred(cred_other)->domain; > > + is_scoped = domain_scope_le(dom_parent, dom_sock); > > + rcu_read_unlock(); > > + > > + return is_scoped; > > +} > > + > > +static int hook_unix_stream_connect(struct sock *const sock, > > + struct sock *const other, > > + struct sock *const newsk) > > +{ > > + if (unix_sock_is_scoped(sock, other)) > > + return 0; > > + return -EPERM; > > +} > > + > > static struct security_hook_list landlock_hooks[] __ro_after_init = { > > LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check), > > LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme), > > + LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect), > > Please add a hook for security_unix_may_send() too, it should be quite > similar, and simplify the patch's subject accordingly. > > You now need to add tests (in a dedicated patch) extending > tools/testing/selftests/landlock/ptrace_test.c (I'll rename the file > later). > > These tests should also check with unnamed and named unix sockets. I > guess the current code doesn't differentiate them and control all kind > of unix sockets. Because they must explicitly be passed, sockets > created with socketpair(2) (i.e. unnamed socket) should never be denied. > > > }; > > > > __init void landlock_add_task_hooks(void) > > -- > > 2.34.1 > > > >
On Wed, Apr 10, 2024 at 04:24:30PM -0600, Tahera Fahimi wrote: > On Tue, Apr 02, 2024 at 11:53:09AM +0200, Mickaël Salaün wrote: > > Thanks for this patch. Please CC the netdev mailing list too, they may > > be interested by this feature. I also added a few folks that previously > > showed their interest for this feature. > > > > On Thu, Mar 28, 2024 at 05:12:13PM -0600, TaheraFahimi wrote: > > > Abstract unix sockets are used for local interprocess communication without > > > relying on filesystem. Since landlock has no restriction for connecting to > > > a UNIX socket in the abstract namespace, a sandboxed process can connect to > > > a socket outside the sandboxed environment. Access to such sockets should > > > be scoped the same way ptrace access is limited. > > > > This is good but it would be better to explain that Landlock doesn't > > currently control abstract unix sockets and that it would make sense for > > a sandbox. > > > > > > > > > > For a landlocked process to be allowed to connect to a target process, it > > > must have a subset of the target process’s rules (the connecting socket > > > must be in a sub-domain of the listening socket). This patch adds a new > > > LSM hook for connect function in unix socket with the related access rights. > > > > Because of compatibility reasons, and because Landlock should be > > flexible, we need to extend the user space interface. As explained in > > the GitHub issue, we need to add a new "scoped" field to the > > landlock_ruleset_attr struct. This field will optionally contain a > > LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET flag to specify that this > > ruleset will deny any connection from within the sandbox to its parents > > (i.e. any parent sandbox or not-sandboxed processes). > Thanks for the feedback. Here is what I understood, please correct me if > I am wrong. First, I should add another field to the > landlock_ruleset_attr (a field like handled_access_net, but for the unix > sockets) with a flag LANDLOCK_ACCESS_UNIX_CONNECT (it is a flag like > LANDLOCK_ACCESS_NET_CONNECT_TCP but fot the unix sockets connect). That was the initial idea, but after thinking more about it and talking with some users, I now think we can get a more generic interface. Because unix sockets, signals, and other IPCs are fully controlled by the kernel (contrary to inet sockets that get out of the system), we can add ingress and egress control according to the source and the destination. To control the direction we could add an LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE and a LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND rights (these names are a bit long but at least explicit). To control the source and destination, it makes sense to use Landlock domain (i.e. sandboxes): LANDLOCK_DOMAIN_HIERARCHY_PARENT, LANDLOCK_DOMAIN_HIERARCHY_SELF, and LANDLOCK_DOMAIN_HIERARCHY_CHILD. This could be used by extending the landlock_ruleset_attr type and adding a new landlock_domain_hierarchy_attr type: struct landlock_ruleset_attr ruleset_attr = { .handled_access_dom = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | \ LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, } // Allows sending data to and receiving data from processes in the same // domain or a child domain, through abstract unix sockets. struct landlock_domain_hierarchy_attr dom_attr = { .allowed_access = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | \ LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, .relationship = LANDLOCK_DOMAIN_HIERARCHY_SELF | \ LANDLOCK_DOMAIN_HIERARCHY_CHILD, }; It should also work with other kind of IPCs: * LANDLOCK_ACCESS_DOM_UNIX_PATHNAME_RECEIVE/SEND (signal) * LANDLOCK_ACCESS_DOM_SIGNAL_RECEIVE/SEND (signal) * LANDLOCK_ACCESS_DOM_XSI_RECEIVE/SEND (XSI message queue) * LANDLOCK_ACCESS_DOM_MQ_RECEIVE/SEND (POSIX message queue) * LANDLOCK_ACCESS_DOM_PTRACE_RECEIVE/SEND (ptrace, which would be limited) What do you think?
On Tue, Apr 30, 2024 at 05:24:45PM +0200, Mickaël Salaün wrote: > On Wed, Apr 10, 2024 at 04:24:30PM -0600, Tahera Fahimi wrote: > > On Tue, Apr 02, 2024 at 11:53:09AM +0200, Mickaël Salaün wrote: > > > Thanks for this patch. Please CC the netdev mailing list too, they may > > > be interested by this feature. I also added a few folks that previously > > > showed their interest for this feature. > > > > > > On Thu, Mar 28, 2024 at 05:12:13PM -0600, TaheraFahimi wrote: > > > > Abstract unix sockets are used for local interprocess communication without > > > > relying on filesystem. Since landlock has no restriction for connecting to > > > > a UNIX socket in the abstract namespace, a sandboxed process can connect to > > > > a socket outside the sandboxed environment. Access to such sockets should > > > > be scoped the same way ptrace access is limited. > > > > > > This is good but it would be better to explain that Landlock doesn't > > > currently control abstract unix sockets and that it would make sense for > > > a sandbox. > > > > > > > > > > > > > > For a landlocked process to be allowed to connect to a target process, it > > > > must have a subset of the target process’s rules (the connecting socket > > > > must be in a sub-domain of the listening socket). This patch adds a new > > > > LSM hook for connect function in unix socket with the related access rights. > > > > > > Because of compatibility reasons, and because Landlock should be > > > flexible, we need to extend the user space interface. As explained in > > > the GitHub issue, we need to add a new "scoped" field to the > > > landlock_ruleset_attr struct. This field will optionally contain a > > > LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET flag to specify that this > > > ruleset will deny any connection from within the sandbox to its parents > > > (i.e. any parent sandbox or not-sandboxed processes). > > > Thanks for the feedback. Here is what I understood, please correct me if > > I am wrong. First, I should add another field to the > > landlock_ruleset_attr (a field like handled_access_net, but for the unix > > sockets) with a flag LANDLOCK_ACCESS_UNIX_CONNECT (it is a flag like > > LANDLOCK_ACCESS_NET_CONNECT_TCP but fot the unix sockets connect). > > That was the initial idea, but after thinking more about it and talking > with some users, I now think we can get a more generic interface. > > Because unix sockets, signals, and other IPCs are fully controlled by > the kernel (contrary to inet sockets that get out of the system), we can > add ingress and egress control according to the source and the > destination. > > To control the direction we could add an > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE and a > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND rights (these names are a bit > long but at least explicit). To control the source and destination, it > makes sense to use Landlock domain (i.e. sandboxes): > LANDLOCK_DOMAIN_HIERARCHY_PARENT, LANDLOCK_DOMAIN_HIERARCHY_SELF, and > LANDLOCK_DOMAIN_HIERARCHY_CHILD. This could be used by extending the > landlock_ruleset_attr type and adding a new > landlock_domain_hierarchy_attr type: > > struct landlock_ruleset_attr ruleset_attr = { > .handled_access_dom = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | \ > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > } > > // Allows sending data to and receiving data from processes in the same > // domain or a child domain, through abstract unix sockets. > struct landlock_domain_hierarchy_attr dom_attr = { > .allowed_access = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | \ > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > .relationship = LANDLOCK_DOMAIN_HIERARCHY_SELF | \ > LANDLOCK_DOMAIN_HIERARCHY_CHILD, > }; > > It should also work with other kind of IPCs: > * LANDLOCK_ACCESS_DOM_UNIX_PATHNAME_RECEIVE/SEND (signal) > * LANDLOCK_ACCESS_DOM_SIGNAL_RECEIVE/SEND (signal) > * LANDLOCK_ACCESS_DOM_XSI_RECEIVE/SEND (XSI message queue) > * LANDLOCK_ACCESS_DOM_MQ_RECEIVE/SEND (POSIX message queue) > * LANDLOCK_ACCESS_DOM_PTRACE_RECEIVE/SEND (ptrace, which would be > limited) > > What do you think? Indeed, in the case of abstract Unix sockets, both parties can send and receive data when a connection is established. Therefore, we can define a single LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT to represent the right to share data, regardless of direction. However, we should still retain LANDLOCK_DOMAIN_HIERARCHY for SELF, PARENT, and CHILD, as the source and destination are important. As you said, I believe we should have receive and send rights for another kind of IPCs (which will be used for landlock#8 issue)
On Tue, Apr 30, 2024 at 05:24:45PM +0200, Mickaël Salaün wrote: > On Wed, Apr 10, 2024 at 04:24:30PM -0600, Tahera Fahimi wrote: > > On Tue, Apr 02, 2024 at 11:53:09AM +0200, Mickaël Salaün wrote: > > > Thanks for this patch. Please CC the netdev mailing list too, they may > > > be interested by this feature. I also added a few folks that previously > > > showed their interest for this feature. > > > > > > On Thu, Mar 28, 2024 at 05:12:13PM -0600, TaheraFahimi wrote: > > > > Abstract unix sockets are used for local interprocess communication without > > > > relying on filesystem. Since landlock has no restriction for connecting to > > > > a UNIX socket in the abstract namespace, a sandboxed process can connect to > > > > a socket outside the sandboxed environment. Access to such sockets should > > > > be scoped the same way ptrace access is limited. > > > > > > This is good but it would be better to explain that Landlock doesn't > > > currently control abstract unix sockets and that it would make sense for > > > a sandbox. > > > > > > > > > > > > > > For a landlocked process to be allowed to connect to a target process, it > > > > must have a subset of the target process’s rules (the connecting socket > > > > must be in a sub-domain of the listening socket). This patch adds a new > > > > LSM hook for connect function in unix socket with the related access rights. > > > > > > Because of compatibility reasons, and because Landlock should be > > > flexible, we need to extend the user space interface. As explained in > > > the GitHub issue, we need to add a new "scoped" field to the > > > landlock_ruleset_attr struct. This field will optionally contain a > > > LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET flag to specify that this > > > ruleset will deny any connection from within the sandbox to its parents > > > (i.e. any parent sandbox or not-sandboxed processes). > > > Thanks for the feedback. Here is what I understood, please correct me if > > I am wrong. First, I should add another field to the > > landlock_ruleset_attr (a field like handled_access_net, but for the unix > > sockets) with a flag LANDLOCK_ACCESS_UNIX_CONNECT (it is a flag like > > LANDLOCK_ACCESS_NET_CONNECT_TCP but fot the unix sockets connect). > > That was the initial idea, but after thinking more about it and talking > with some users, I now think we can get a more generic interface. > > Because unix sockets, signals, and other IPCs are fully controlled by > the kernel (contrary to inet sockets that get out of the system), we can > add ingress and egress control according to the source and the > destination. > > To control the direction we could add an > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE and a > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND rights (these names are a bit > long but at least explicit). To control the source and destination, it > makes sense to use Landlock domain (i.e. sandboxes): > LANDLOCK_DOMAIN_HIERARCHY_PARENT, LANDLOCK_DOMAIN_HIERARCHY_SELF, and > LANDLOCK_DOMAIN_HIERARCHY_CHILD. This could be used by extending the > landlock_ruleset_attr type and adding a new > landlock_domain_hierarchy_attr type: > > struct landlock_ruleset_attr ruleset_attr = { > .handled_access_dom = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | \ > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > } > > // Allows sending data to and receiving data from processes in the same > // domain or a child domain, through abstract unix sockets. > struct landlock_domain_hierarchy_attr dom_attr = { > .allowed_access = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | \ > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > .relationship = LANDLOCK_DOMAIN_HIERARCHY_SELF | \ > LANDLOCK_DOMAIN_HIERARCHY_CHILD, > }; > > It should also work with other kind of IPCs: > * LANDLOCK_ACCESS_DOM_UNIX_PATHNAME_RECEIVE/SEND (signal) > * LANDLOCK_ACCESS_DOM_SIGNAL_RECEIVE/SEND (signal) > * LANDLOCK_ACCESS_DOM_XSI_RECEIVE/SEND (XSI message queue) > * LANDLOCK_ACCESS_DOM_MQ_RECEIVE/SEND (POSIX message queue) > * LANDLOCK_ACCESS_DOM_PTRACE_RECEIVE/SEND (ptrace, which would be > limited) > > What do you think? I was wondering if you expand your idea on the following example. Considering P1 with the rights that you mentioned in your email, forks a new process (P2). Now both P1 and P2 are on the same domain and are allowed to send data to and receive data from processes in the same domain or a child domain. /* * Same domain (inherited) * .-------------. * | P1----. | P1 -> P2 : allow * | \ | P2 -> P1 : allow * | ' | * | P2 | * '-------------' */ (P1 domain) = (P2 domain) = { .allowed_access = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, .relationship = LANDLOCK_DOMAIN_HIERARCHY_SELF | LANDLOCK_DOMAIN_HIERARCHY_CHILD, } In another example, if P1 has the same domain as before but P2 has LANDLOCK_DOMAIN_HIERARCHY_PARENT in their domain, so P1 still can connect to P2. /* * Parent domain * .------. * | P1 --. P1 -> P2 : allow * '------' \ P2 -> P1 : allow * ' * P2 */ (P1 domain) = { .allowed_access = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, .relationship = LANDLOCK_DOMAIN_HIERARCHY_SELF | LANDLOCK_DOMAIN_HIERARCHY_CHILD, } (P2 domain) = { .allowed_access = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, .relationship = LANDLOCK_DOMAIN_HIERARCHY_SELF | LANDLOCK_DOMAIN_HIERARCHY_CHILD | LANDLOCK_DOMAIN_HIERARCHY_PARENT, }
On Thu, May 30, 2024 at 05:13:04PM -0600, Tahera Fahimi wrote: > On Tue, Apr 30, 2024 at 05:24:45PM +0200, Mickaël Salaün wrote: > > On Wed, Apr 10, 2024 at 04:24:30PM -0600, Tahera Fahimi wrote: > > > On Tue, Apr 02, 2024 at 11:53:09AM +0200, Mickaël Salaün wrote: > > > > Thanks for this patch. Please CC the netdev mailing list too, they may > > > > be interested by this feature. I also added a few folks that previously > > > > showed their interest for this feature. > > > > > > > > On Thu, Mar 28, 2024 at 05:12:13PM -0600, TaheraFahimi wrote: > > > > > Abstract unix sockets are used for local interprocess communication without > > > > > relying on filesystem. Since landlock has no restriction for connecting to > > > > > a UNIX socket in the abstract namespace, a sandboxed process can connect to > > > > > a socket outside the sandboxed environment. Access to such sockets should > > > > > be scoped the same way ptrace access is limited. > > > > > > > > This is good but it would be better to explain that Landlock doesn't > > > > currently control abstract unix sockets and that it would make sense for > > > > a sandbox. > > > > > > > > > > > > > > > > > > For a landlocked process to be allowed to connect to a target process, it > > > > > must have a subset of the target process’s rules (the connecting socket > > > > > must be in a sub-domain of the listening socket). This patch adds a new > > > > > LSM hook for connect function in unix socket with the related access rights. > > > > > > > > Because of compatibility reasons, and because Landlock should be > > > > flexible, we need to extend the user space interface. As explained in > > > > the GitHub issue, we need to add a new "scoped" field to the > > > > landlock_ruleset_attr struct. This field will optionally contain a > > > > LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET flag to specify that this > > > > ruleset will deny any connection from within the sandbox to its parents > > > > (i.e. any parent sandbox or not-sandboxed processes). > > > > > Thanks for the feedback. Here is what I understood, please correct me if > > > I am wrong. First, I should add another field to the > > > landlock_ruleset_attr (a field like handled_access_net, but for the unix > > > sockets) with a flag LANDLOCK_ACCESS_UNIX_CONNECT (it is a flag like > > > LANDLOCK_ACCESS_NET_CONNECT_TCP but fot the unix sockets connect). > > > > That was the initial idea, but after thinking more about it and talking > > with some users, I now think we can get a more generic interface. > > > > Because unix sockets, signals, and other IPCs are fully controlled by > > the kernel (contrary to inet sockets that get out of the system), we can > > add ingress and egress control according to the source and the > > destination. > > > > To control the direction we could add an > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE and a > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND rights (these names are a bit > > long but at least explicit). To control the source and destination, it > > makes sense to use Landlock domain (i.e. sandboxes): > > LANDLOCK_DOMAIN_HIERARCHY_PARENT, LANDLOCK_DOMAIN_HIERARCHY_SELF, and > > LANDLOCK_DOMAIN_HIERARCHY_CHILD. This could be used by extending the > > landlock_ruleset_attr type and adding a new > > landlock_domain_hierarchy_attr type: > > > > struct landlock_ruleset_attr ruleset_attr = { > > .handled_access_dom = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | \ > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > > } > > > > // Allows sending data to and receiving data from processes in the same > > // domain or a child domain, through abstract unix sockets. > > struct landlock_domain_hierarchy_attr dom_attr = { > > .allowed_access = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | \ > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > > .relationship = LANDLOCK_DOMAIN_HIERARCHY_SELF | \ > > LANDLOCK_DOMAIN_HIERARCHY_CHILD, > > }; > > > > It should also work with other kind of IPCs: > > * LANDLOCK_ACCESS_DOM_UNIX_PATHNAME_RECEIVE/SEND (signal) > > * LANDLOCK_ACCESS_DOM_SIGNAL_RECEIVE/SEND (signal) > > * LANDLOCK_ACCESS_DOM_XSI_RECEIVE/SEND (XSI message queue) > > * LANDLOCK_ACCESS_DOM_MQ_RECEIVE/SEND (POSIX message queue) > > * LANDLOCK_ACCESS_DOM_PTRACE_RECEIVE/SEND (ptrace, which would be > > limited) > > > > What do you think? > > I was wondering if you expand your idea on the following example. > > Considering P1 with the rights that you mentioned in your email, forks a > new process (P2). Now both P1 and P2 are on the same domain and are > allowed to send data to and receive data from processes in the same > domain or a child domain. > /* > * Same domain (inherited) > * .-------------. > * | P1----. | P1 -> P2 : allow > * | \ | P2 -> P1 : allow > * | ' | > * | P2 | > * '-------------' > */ > (P1 domain) = (P2 domain) = { > .allowed_access = > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > .relationship = > LANDLOCK_DOMAIN_HIERARCHY_SELF | > LANDLOCK_DOMAIN_HIERARCHY_CHILD, In this case LANDLOCK_DOMAIN_HIERARCHY_CHILD would not be required because P1 and P2 are on the same domain. > } > > In another example, if P1 has the same domain as before but P2 has > LANDLOCK_DOMAIN_HIERARCHY_PARENT in their domain, so P1 still can > connect to P2. > /* > * Parent domain > * .------. > * | P1 --. P1 -> P2 : allow > * '------' \ P2 -> P1 : allow > * ' > * P2 > */ > > (P1 domain) = { > .allowed_access = > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > .relationship = > LANDLOCK_DOMAIN_HIERARCHY_SELF | > LANDLOCK_DOMAIN_HIERARCHY_CHILD, Hmm, in this case P2 doesn't have a domain, so LANDLOCK_DOMAIN_HIERARCHY_CHILD doesn't make sense. > } > (P2 domain) = { > .allowed_access = > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > .relationship = > LANDLOCK_DOMAIN_HIERARCHY_SELF | > LANDLOCK_DOMAIN_HIERARCHY_CHILD | > LANDLOCK_DOMAIN_HIERARCHY_PARENT, > } I think you wanted to use the "Inherited + child domain" example here, in which case the domain policies make sense. I was maybe too enthusiastic with the "relationship" field. Let's rename landlock_domain_hierarchy_attr to landlock_domain_attr and remove the "relationship" field. We'll always consider that LANDLOCK_DOMAIN_HIERARCHY_SELF is set as well as LANDLOCK_DOMAIN_HIERARCHY_CHILD (i.e. no restriction to send/received to/from a child domain or our own domain). In a nutshell, please only keep the LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_{RECEIVE,SEND} rights and follow the same logic as with ptrace restrictions. It will be easier to reason about and will be useful for most cases. We could later extend that with more features. LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE will then translates to "allow to receive from the parent domain". LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND will then translates to "allow to send to the parent domain". As for other Landlock access rights, the restrictions of domains should only be changed if LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_* is "handled" by the ruleset/domain.
On Fri, May 31, 2024 at 11:39:12AM +0200, Mickaël Salaün wrote: > On Thu, May 30, 2024 at 05:13:04PM -0600, Tahera Fahimi wrote: > > On Tue, Apr 30, 2024 at 05:24:45PM +0200, Mickaël Salaün wrote: > > > On Wed, Apr 10, 2024 at 04:24:30PM -0600, Tahera Fahimi wrote: > > > > On Tue, Apr 02, 2024 at 11:53:09AM +0200, Mickaël Salaün wrote: > > > > > Thanks for this patch. Please CC the netdev mailing list too, they may > > > > > be interested by this feature. I also added a few folks that previously > > > > > showed their interest for this feature. > > > > > > > > > > On Thu, Mar 28, 2024 at 05:12:13PM -0600, TaheraFahimi wrote: > > > > > > Abstract unix sockets are used for local interprocess communication without > > > > > > relying on filesystem. Since landlock has no restriction for connecting to > > > > > > a UNIX socket in the abstract namespace, a sandboxed process can connect to > > > > > > a socket outside the sandboxed environment. Access to such sockets should > > > > > > be scoped the same way ptrace access is limited. > > > > > > > > > > This is good but it would be better to explain that Landlock doesn't > > > > > currently control abstract unix sockets and that it would make sense for > > > > > a sandbox. > > > > > > > > > > > > > > > > > > > > > > For a landlocked process to be allowed to connect to a target process, it > > > > > > must have a subset of the target process’s rules (the connecting socket > > > > > > must be in a sub-domain of the listening socket). This patch adds a new > > > > > > LSM hook for connect function in unix socket with the related access rights. > > > > > > > > > > Because of compatibility reasons, and because Landlock should be > > > > > flexible, we need to extend the user space interface. As explained in > > > > > the GitHub issue, we need to add a new "scoped" field to the > > > > > landlock_ruleset_attr struct. This field will optionally contain a > > > > > LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET flag to specify that this > > > > > ruleset will deny any connection from within the sandbox to its parents > > > > > (i.e. any parent sandbox or not-sandboxed processes). > > > > > > > Thanks for the feedback. Here is what I understood, please correct me if > > > > I am wrong. First, I should add another field to the > > > > landlock_ruleset_attr (a field like handled_access_net, but for the unix > > > > sockets) with a flag LANDLOCK_ACCESS_UNIX_CONNECT (it is a flag like > > > > LANDLOCK_ACCESS_NET_CONNECT_TCP but fot the unix sockets connect). > > > > > > That was the initial idea, but after thinking more about it and talking > > > with some users, I now think we can get a more generic interface. > > > > > > Because unix sockets, signals, and other IPCs are fully controlled by > > > the kernel (contrary to inet sockets that get out of the system), we can > > > add ingress and egress control according to the source and the > > > destination. > > > > > > To control the direction we could add an > > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE and a > > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND rights (these names are a bit > > > long but at least explicit). To control the source and destination, it > > > makes sense to use Landlock domain (i.e. sandboxes): > > > LANDLOCK_DOMAIN_HIERARCHY_PARENT, LANDLOCK_DOMAIN_HIERARCHY_SELF, and > > > LANDLOCK_DOMAIN_HIERARCHY_CHILD. This could be used by extending the > > > landlock_ruleset_attr type and adding a new > > > landlock_domain_hierarchy_attr type: > > > > > > struct landlock_ruleset_attr ruleset_attr = { > > > .handled_access_dom = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | \ > > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > > > } > > > > > > // Allows sending data to and receiving data from processes in the same > > > // domain or a child domain, through abstract unix sockets. > > > struct landlock_domain_hierarchy_attr dom_attr = { > > > .allowed_access = LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | \ > > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > > > .relationship = LANDLOCK_DOMAIN_HIERARCHY_SELF | \ > > > LANDLOCK_DOMAIN_HIERARCHY_CHILD, > > > }; > > > > > > It should also work with other kind of IPCs: > > > * LANDLOCK_ACCESS_DOM_UNIX_PATHNAME_RECEIVE/SEND (signal) > > > * LANDLOCK_ACCESS_DOM_SIGNAL_RECEIVE/SEND (signal) > > > * LANDLOCK_ACCESS_DOM_XSI_RECEIVE/SEND (XSI message queue) > > > * LANDLOCK_ACCESS_DOM_MQ_RECEIVE/SEND (POSIX message queue) > > > * LANDLOCK_ACCESS_DOM_PTRACE_RECEIVE/SEND (ptrace, which would be > > > limited) > > > > > > What do you think? > > > > I was wondering if you expand your idea on the following example. > > > > Considering P1 with the rights that you mentioned in your email, forks a > > new process (P2). Now both P1 and P2 are on the same domain and are > > allowed to send data to and receive data from processes in the same > > domain or a child domain. > > /* > > * Same domain (inherited) > > * .-------------. > > * | P1----. | P1 -> P2 : allow > > * | \ | P2 -> P1 : allow > > * | ' | > > * | P2 | > > * '-------------' > > */ > > (P1 domain) = (P2 domain) = { > > .allowed_access = > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > > .relationship = > > LANDLOCK_DOMAIN_HIERARCHY_SELF | > > LANDLOCK_DOMAIN_HIERARCHY_CHILD, > > In this case LANDLOCK_DOMAIN_HIERARCHY_CHILD would not be required > because P1 and P2 are on the same domain. > > > } > > > > In another example, if P1 has the same domain as before but P2 has > > LANDLOCK_DOMAIN_HIERARCHY_PARENT in their domain, so P1 still can > > connect to P2. > > /* > > * Parent domain > > * .------. > > * | P1 --. P1 -> P2 : allow > > * '------' \ P2 -> P1 : allow > > * ' > > * P2 > > */ > > > > (P1 domain) = { > > .allowed_access = > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > > .relationship = > > LANDLOCK_DOMAIN_HIERARCHY_SELF | > > LANDLOCK_DOMAIN_HIERARCHY_CHILD, > > Hmm, in this case P2 doesn't have a domain, so > LANDLOCK_DOMAIN_HIERARCHY_CHILD doesn't make sense. > > > } > > (P2 domain) = { > > .allowed_access = > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE | > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND, > > .relationship = > > LANDLOCK_DOMAIN_HIERARCHY_SELF | > > LANDLOCK_DOMAIN_HIERARCHY_CHILD | > > LANDLOCK_DOMAIN_HIERARCHY_PARENT, > > } > > I think you wanted to use the "Inherited + child domain" example here, > in which case the domain policies make sense. > > I was maybe too enthusiastic with the "relationship" field. Let's > rename landlock_domain_hierarchy_attr to landlock_domain_attr and remove > the "relationship" field. We'll always consider that > LANDLOCK_DOMAIN_HIERARCHY_SELF is set as well as > LANDLOCK_DOMAIN_HIERARCHY_CHILD (i.e. no restriction to send/received > to/from a child domain or our own domain). In a nutshell, please only > keep the LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_{RECEIVE,SEND} rights and > follow the same logic as with ptrace restrictions. It will be easier to > reason about and will be useful for most cases. We could later extend > that with more features. > > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_RECEIVE will then translates to "allow > to receive from the parent domain". > LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_SEND will then translates to "allow to > send to the parent domain". If we consider LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_* shows the ability to send/recieve data to/from the parent domain, different scenarios would be as follow(again using your drawings from the ptrace_test): /* * No domain * * P1-. P1 -> P2 : allow * \ P2 -> P1 : allow * 'P2 */ (Child domain): Since child can not send/recieve data to/from parent,the connection of both direction is banned. /* * Child domain: * * P1--. P1 -> P2 : deny * \ P2 -> P1 : deny * .'-----. * | P2 | * '------' */ (Parent domain): The parent's access to its parent is restricted, so the child and parent can establish connection. /* * Parent domain * .------. * | P1 --. P1 -> P2 : allow * '------' \ P2 -> P1 : allow * ' * P2 */ (Parent + child domain): Same as (child domain) scenario /* * Parent + child domain(inherited) * .------. * | P1 ---. P1 -> P2 : deny * '------' \ P2 -> P1 : deny * .---'--. * | P2 | * '------' */ (Same domain): An example is when a process fork two child processes and they inherit the parent's access. In this case, children proccess can send/recieve data to/from each other since they are in the same domain. /* * Same domain (sibling) * .-------------. * | P1----. | P1 -> P2 : allow * | \ | P2 -> P1 : allow * | ' | * | P2 | * '-------------' */ /* * Inherited + child domain * .-----------------. * | P1----. | P1 -> P2 : deny * | \ | P2 -> P1 : deny * | .-'----. | * | | P2 | | * | '------' | * '-----------------' */ /* * Inherited + parent domain * .-----------------. * |.------. | P1 -> P2 : allow * || P1 ----. | P2 -> P1 : allow * |'------' \ | * | ' | * | P2 | * '-----------------' */ /* * Inherited + parent and child domain * .-----------------. * | .------. | P1 -> P2 : deny * | | P1 . | P2 -> P1 : deny * | '------'\ | * | \ | * | .--'---. | * | | P2 | | * | '------' | * '-----------------' */ Any feedback on this logic is appreciated. > As for other Landlock access rights, the restrictions of domains should > only be changed if LANDLOCK_ACCESS_DOM_UNIX_ABSTRACT_* is "handled" by > the ruleset/domain.
OK, thanks to your examples I found some issues with this "send/recv" design. A sandboxed process should not be able to block actions on its own socket from a higher privileged domain (or a process without domain). One issue is that if a domain D2 denies access (to its abstract unix sockets) to its parent D1, processes without domains (e.g. parent of D1) should not be restricted in any way. Furthermore, it should not be possible for a process to enforce such restriction if it is not already sandboxed in a domain. Implementing such mechanism would require to add exceptions, which makes this design inconsistent. Let's get back to the initial "scope" design, which is simpler and consistent with the ptrace restrictions. Sorry for the back and forth. This discussions will be useful for the rationale though. ;) I kept the relevant parts: On Fri, May 31, 2024 at 02:04:44PM -0600, Tahera Fahimi wrote: > On Fri, May 31, 2024 at 11:39:12AM +0200, Mickaël Salaün wrote: > > On Thu, May 30, 2024 at 05:13:04PM -0600, Tahera Fahimi wrote: > > > On Tue, Apr 30, 2024 at 05:24:45PM +0200, Mickaël Salaün wrote: > > > > On Wed, Apr 10, 2024 at 04:24:30PM -0600, Tahera Fahimi wrote: > > > > > On Tue, Apr 02, 2024 at 11:53:09AM +0200, Mickaël Salaün wrote: [...] > > > > > > Because of compatibility reasons, and because Landlock should be > > > > > > flexible, we need to extend the user space interface. As explained in > > > > > > the GitHub issue, we need to add a new "scoped" field to the > > > > > > landlock_ruleset_attr struct. This field will optionally contain a > > > > > > LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET flag to specify that this > > > > > > ruleset will deny any connection from within the sandbox to its parents > > > > > > (i.e. any parent sandbox or not-sandboxed processes). > > > > > > > > > Thanks for the feedback. Here is what I understood, please correct me if > > > > > I am wrong. First, I should add another field to the > > > > > landlock_ruleset_attr (a field like handled_access_net, but for the unix > > > > > sockets) with a flag LANDLOCK_ACCESS_UNIX_CONNECT (it is a flag like > > > > > LANDLOCK_ACCESS_NET_CONNECT_TCP but fot the unix sockets connect). Yes, but the field's name should be "scoped", and it will only accept a LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET flag. This is a bit different than handled_access_net because there is no rule that would accept LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET (i.e. it is a restriction-only). Without LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET, the current behavior should not be changed. This should be covered with appropriate tests. Taking the following examples for domains with LANDLOCK_RULESET_SCOPED_ABSTRACT_UNIX_SOCKET, we get the same restrictions as with ptrace: [...] > /* > * No domain > * > * P1-. P1 -> P2 : allow > * \ P2 -> P1 : allow > * 'P2 > */ This is still correct. > /* > * Child domain: > * > * P1--. P1 -> P2 : deny > * \ P2 -> P1 : deny > * .'-----. > * | P2 | > * '------' > */ With the "scoped" approach: P1 -> P2: allow P2 -> P1: deny > /* > * Parent domain > * .------. > * | P1 --. P1 -> P2 : allow > * '------' \ P2 -> P1 : allow > * ' > * P2 > */ With the "scoped" approach: P1 -> P2: deny P2 -> P1: allow Indeed, only the domain hierarchy matters, not the process hierarchy. This works the same way with ptrace restrictions. > /* > * Parent + child domain(inherited) > * .------. > * | P1 ---. P1 -> P2 : deny > * '------' \ P2 -> P1 : deny > * .---'--. > * | P2 | > * '------' > */ This is still correct. > /* > * Same domain (sibling) > * .-------------. > * | P1----. | P1 -> P2 : allow > * | \ | P2 -> P1 : allow > * | ' | > * | P2 | > * '-------------' > */ This is still correct. > /* > * Inherited + child domain > * .-----------------. > * | P1----. | P1 -> P2 : deny > * | \ | P2 -> P1 : deny > * | .-'----. | > * | | P2 | | > * | '------' | > * '-----------------' > */ With the "scoped" approach: P1 -> P2: allow P2 -> P1: deny > /* > * Inherited + parent domain > * .-----------------. > * |.------. | P1 -> P2 : allow > * || P1 ----. | P2 -> P1 : allow > * |'------' \ | > * | ' | > * | P2 | > * '-----------------' > */ With the "scoped" approach: P1 -> P2: deny P2 -> P1: allow > /* > * Inherited + parent and child domain > * .-----------------. > * | .------. | P1 -> P2 : deny > * | | P1 . | P2 -> P1 : deny > * | '------'\ | > * | \ | > * | .--'---. | > * | | P2 | | > * | '------' | > * '-----------------' > */ This is still correct.
diff --git a/security/landlock/task.c b/security/landlock/task.c index 849f5123610b..67528f87b7de 100644 --- a/security/landlock/task.c +++ b/security/landlock/task.c @@ -13,6 +13,7 @@ #include <linux/lsm_hooks.h> #include <linux/rcupdate.h> #include <linux/sched.h> +#include <net/sock.h> #include "common.h" #include "cred.h" @@ -108,9 +109,48 @@ static int hook_ptrace_traceme(struct task_struct *const parent) return task_ptrace(parent, current); } +static bool unix_sock_is_scoped(struct sock *const sock, + struct sock *const other) +{ + bool is_scoped = true; + + /* get the ruleset of connecting sock*/ + const struct landlock_ruleset *const dom_sock = + landlock_get_current_domain(); + + if (!dom_sock) + return true; + + /* get credential of listening sock*/ + const struct cred *cred_other = get_cred(other->sk_peer_cred); + + if (!cred_other) + return true; + + /* retrieve the landlock_rulesets */ + const struct landlock_ruleset *dom_parent; + + rcu_read_lock(); + dom_parent = landlock_cred(cred_other)->domain; + is_scoped = domain_scope_le(dom_parent, dom_sock); + rcu_read_unlock(); + + return is_scoped; +} + +static int hook_unix_stream_connect(struct sock *const sock, + struct sock *const other, + struct sock *const newsk) +{ + if (unix_sock_is_scoped(sock, other)) + return 0; + return -EPERM; +} + static struct security_hook_list landlock_hooks[] __ro_after_init = { LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check), LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme), + LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect), }; __init void landlock_add_task_hooks(void)
Abstract unix sockets are used for local interprocess communication without relying on filesystem. Since landlock has no restriction for connecting to a UNIX socket in the abstract namespace, a sandboxed process can connect to a socket outside the sandboxed environment. Access to such sockets should be scoped the same way ptrace access is limited. For a landlocked process to be allowed to connect to a target process, it must have a subset of the target process’s rules (the connecting socket must be in a sub-domain of the listening socket). This patch adds a new LSM hook for connect function in unix socket with the related access rights. Link to first draft: https://lore.kernel.org/outreachy/20240328.ShoR4Iecei8o@digikod.net/ Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com> ---- Changes in v2: - Remove wrapper functions, noted by Casey Schaufler <casey@schaufler-ca.com> --- security/landlock/task.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)