Before this change, network restrictions were enforced according to the calling thread's Landlock domain, leading to potential inconsistent results when the same socket was used by different threads or processes (with different domains). This change fixes such access control inconsistency by enforcing the socket's Landlock domain instead of the caller's Landlock domain.
Socket's Landlock domain is inherited from the thread that created this socket. This means that a socket created without sandboxing will be free to connect and bind without limitation. This also means that a socket created by a sandboxed thread will inherit the thread's policy, which will be enforced on this socket even when used by another thread or passed to another process.
The initial rationale [1] was that a socket does not directly grants access to data, but it is an object used to define an access (e.g. connection to a peer). Contrary to my initial assumption, we can identify to which protocol/port a newly created socket can give access to with the socket's file->f_cred inherited from its creator. Moreover, from a kernel point of view, especially for shared objects, we need a more consistent access model. This means that the same action on the same socket performed by different threads will have the same effect. This follows the same approach as for file descriptors tied to the file system (e.g. LANDLOCK_ACCESS_FS_TRUNCATE).
One potential risk of this change is for unsandboxed processes to send socket file descriptors to sandboxed processes, which could give unrestricted network access to the sandboxed process (by reconfigure the socket). While it makes sense for processes to transfer (AF_UNIX) socketpairs, which is OK because they can only exchange data between themselves, it should be rare for processes to legitimately pass other kind of sockets (e.g. AF_INET).
Another potential risk of this approach is socket file descriptor leaks. This is the same risk as with regular file descriptor leaks giving access to the content of a file, which is well known and documented. This could be mitigated with a future complementary restriction on received or inherited file descriptors.
One interesting side effect of this new approach is that a process can create a socket that will only allow to connect to a set of ports. This can be done by creating a thread, sandboxing it, creating a socket, and using the related file descriptor (in the same process). Passing this restricted socket to a more sandboxed process makes it possible to have a more dynamic security policy.
This new approach aligns with SELinux and Smack instead of AppArmor and Tomoyo. It is also in line with capability-based security mechanisms such as Capsicum.
This slight semantic change is important for current and future Landlock's consistency, and it must be backported.
Current tests are still OK because this behavior wasn't covered. A following commit adds new tests.
Cc: Günther Noack gnoack@google.com Cc: Ivanov Mikhail ivanov.mikhail1@huawei-partners.com Cc: Konstantin Meskhidze konstantin.meskhidze@huawei.com Cc: Paul Moore paul@paul-moore.com Cc: Tahera Fahimi fahimitahera@gmail.com Cc: stable@vger.kernel.org # 6.7.x: 088e2efaf3d2: landlock: Simplify current_check_access_socket() Fixes: fff69fb03dde ("landlock: Support network rules with TCP bind and connect") Link: https://lore.kernel.org/r/263c1eb3-602f-57fe-8450-3f138581bee7@digikod.net [1] Signed-off-by: Mickaël Salaün mic@digikod.net Link: https://lore.kernel.org/r/20240719150618.197991-2-mic@digikod.net --- security/landlock/net.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/security/landlock/net.c b/security/landlock/net.c index c8bcd29bde09..78e027a74819 100644 --- a/security/landlock/net.c +++ b/security/landlock/net.c @@ -50,10 +50,11 @@ get_raw_handled_net_accesses(const struct landlock_ruleset *const domain) return access_dom; }
-static const struct landlock_ruleset *get_current_net_domain(void) +static const struct landlock_ruleset * +get_socket_net_domain(const struct socket *const sock) { const struct landlock_ruleset *const dom = - landlock_get_current_domain(); + landlock_cred(sock->file->f_cred)->domain;
if (!dom || !get_raw_handled_net_accesses(dom)) return NULL; @@ -61,10 +62,9 @@ static const struct landlock_ruleset *get_current_net_domain(void) return dom; }
-static int current_check_access_socket(struct socket *const sock, - struct sockaddr *const address, - const int addrlen, - access_mask_t access_request) +static int check_access_socket(struct socket *const sock, + struct sockaddr *const address, + const int addrlen, access_mask_t access_request) { __be16 port; layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {}; @@ -72,7 +72,7 @@ static int current_check_access_socket(struct socket *const sock, struct landlock_id id = { .type = LANDLOCK_KEY_NET_PORT, }; - const struct landlock_ruleset *const dom = get_current_net_domain(); + const struct landlock_ruleset *const dom = get_socket_net_domain(sock);
if (!dom) return 0; @@ -175,16 +175,16 @@ static int current_check_access_socket(struct socket *const sock, static int hook_socket_bind(struct socket *const sock, struct sockaddr *const address, const int addrlen) { - return current_check_access_socket(sock, address, addrlen, - LANDLOCK_ACCESS_NET_BIND_TCP); + return check_access_socket(sock, address, addrlen, + LANDLOCK_ACCESS_NET_BIND_TCP); }
static int hook_socket_connect(struct socket *const sock, struct sockaddr *const address, const int addrlen) { - return current_check_access_socket(sock, address, addrlen, - LANDLOCK_ACCESS_NET_CONNECT_TCP); + return check_access_socket(sock, address, addrlen, + LANDLOCK_ACCESS_NET_CONNECT_TCP); }
static struct security_hook_list landlock_hooks[] __ro_after_init = {