mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2026-07-08 18:13:59 +02:00
landlock: Fix TCP handling of short AF_UNSPEC addresses
[ Upstream commite4d82cbce2] current_check_access_socket() treats AF_UNSPEC addresses as AF_INET ones, and only later adds special case handling to allow connect(AF_UNSPEC), and on IPv4 sockets bind(AF_UNSPEC+INADDR_ANY). This would be fine except AF_UNSPEC addresses can be as short as a bare AF_UNSPEC sa_family_t field, and nothing more. The AF_INET code path incorrectly enforces a length of sizeof(struct sockaddr_in) instead. Move AF_UNSPEC edge case handling up inside the switch-case, before the address is (potentially incorrectly) treated as AF_INET. Fixes:fff69fb03d("landlock: Support network rules with TCP bind and connect") Signed-off-by: Matthieu Buffet <matthieu@buffet.re> Link: https://lore.kernel.org/r/20251027190726.626244-4-matthieu@buffet.re Signed-off-by: Mickaël Salaün <mic@digikod.net> [ There was a conflict due to missing commit9f74411a40("landlock: Log TCP bind and connect denials") ] Signed-off-by: Maximilian Heyne <mheyne@amazon.de> Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
b440f1ded4
commit
a366a9e54e
+67
-51
@@ -72,6 +72,61 @@ static int current_check_access_socket(struct socket *const sock,
|
||||
|
||||
switch (address->sa_family) {
|
||||
case AF_UNSPEC:
|
||||
if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
|
||||
/*
|
||||
* Connecting to an address with AF_UNSPEC dissolves
|
||||
* the TCP association, which have the same effect as
|
||||
* closing the connection while retaining the socket
|
||||
* object (i.e., the file descriptor). As for dropping
|
||||
* privileges, closing connections is always allowed.
|
||||
*
|
||||
* For a TCP access control system, this request is
|
||||
* legitimate. Let the network stack handle potential
|
||||
* inconsistencies and return -EINVAL if needed.
|
||||
*/
|
||||
return 0;
|
||||
} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
|
||||
/*
|
||||
* Binding to an AF_UNSPEC address is treated
|
||||
* differently by IPv4 and IPv6 sockets. The socket's
|
||||
* family may change under our feet due to
|
||||
* setsockopt(IPV6_ADDRFORM), but that's ok: we either
|
||||
* reject entirely or require
|
||||
* %LANDLOCK_ACCESS_NET_BIND_TCP for the given port, so
|
||||
* it cannot be used to bypass the policy.
|
||||
*
|
||||
* IPv4 sockets map AF_UNSPEC to AF_INET for
|
||||
* retrocompatibility for bind accesses, only if the
|
||||
* address is INADDR_ANY (cf. __inet_bind). IPv6
|
||||
* sockets always reject it.
|
||||
*
|
||||
* Checking the address is required to not wrongfully
|
||||
* return -EACCES instead of -EAFNOSUPPORT or -EINVAL.
|
||||
* We could return 0 and let the network stack handle
|
||||
* these checks, but it is safer to return a proper
|
||||
* error and test consistency thanks to kselftest.
|
||||
*/
|
||||
if (sock->sk->__sk_common.skc_family == AF_INET) {
|
||||
const struct sockaddr_in *const sockaddr =
|
||||
(struct sockaddr_in *)address;
|
||||
|
||||
if (addrlen < sizeof(struct sockaddr_in))
|
||||
return -EINVAL;
|
||||
|
||||
if (sockaddr->sin_addr.s_addr !=
|
||||
htonl(INADDR_ANY))
|
||||
return -EAFNOSUPPORT;
|
||||
} else {
|
||||
if (addrlen < SIN6_LEN_RFC2133)
|
||||
return -EINVAL;
|
||||
else
|
||||
return -EAFNOSUPPORT;
|
||||
}
|
||||
} else {
|
||||
WARN_ON_ONCE(1);
|
||||
}
|
||||
/* Only for bind(AF_UNSPEC+INADDR_ANY) on IPv4 socket. */
|
||||
fallthrough;
|
||||
case AF_INET:
|
||||
if (addrlen < sizeof(struct sockaddr_in))
|
||||
return -EINVAL;
|
||||
@@ -90,57 +145,18 @@ static int current_check_access_socket(struct socket *const sock,
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* Specific AF_UNSPEC handling. */
|
||||
if (address->sa_family == AF_UNSPEC) {
|
||||
/*
|
||||
* Connecting to an address with AF_UNSPEC dissolves the TCP
|
||||
* association, which have the same effect as closing the
|
||||
* connection while retaining the socket object (i.e., the file
|
||||
* descriptor). As for dropping privileges, closing
|
||||
* connections is always allowed.
|
||||
*
|
||||
* For a TCP access control system, this request is legitimate.
|
||||
* Let the network stack handle potential inconsistencies and
|
||||
* return -EINVAL if needed.
|
||||
*/
|
||||
if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP)
|
||||
return 0;
|
||||
|
||||
/*
|
||||
* For compatibility reason, accept AF_UNSPEC for bind
|
||||
* accesses (mapped to AF_INET) only if the address is
|
||||
* INADDR_ANY (cf. __inet_bind). Checking the address is
|
||||
* required to not wrongfully return -EACCES instead of
|
||||
* -EAFNOSUPPORT.
|
||||
*
|
||||
* We could return 0 and let the network stack handle these
|
||||
* checks, but it is safer to return a proper error and test
|
||||
* consistency thanks to kselftest.
|
||||
*/
|
||||
if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
|
||||
/* addrlen has already been checked for AF_UNSPEC. */
|
||||
const struct sockaddr_in *const sockaddr =
|
||||
(struct sockaddr_in *)address;
|
||||
|
||||
if (sock->sk->__sk_common.skc_family != AF_INET)
|
||||
return -EINVAL;
|
||||
|
||||
if (sockaddr->sin_addr.s_addr != htonl(INADDR_ANY))
|
||||
return -EAFNOSUPPORT;
|
||||
}
|
||||
} else {
|
||||
/*
|
||||
* Checks sa_family consistency to not wrongfully return
|
||||
* -EACCES instead of -EINVAL. Valid sa_family changes are
|
||||
* only (from AF_INET or AF_INET6) to AF_UNSPEC.
|
||||
*
|
||||
* We could return 0 and let the network stack handle this
|
||||
* check, but it is safer to return a proper error and test
|
||||
* consistency thanks to kselftest.
|
||||
*/
|
||||
if (address->sa_family != sock->sk->__sk_common.skc_family)
|
||||
return -EINVAL;
|
||||
}
|
||||
/*
|
||||
* Checks sa_family consistency to not wrongfully return
|
||||
* -EACCES instead of -EINVAL. Valid sa_family changes are
|
||||
* only (from AF_INET or AF_INET6) to AF_UNSPEC.
|
||||
*
|
||||
* We could return 0 and let the network stack handle this
|
||||
* check, but it is safer to return a proper error and test
|
||||
* consistency thanks to kselftest.
|
||||
*/
|
||||
if (address->sa_family != sock->sk->__sk_common.skc_family &&
|
||||
address->sa_family != AF_UNSPEC)
|
||||
return -EINVAL;
|
||||
|
||||
id.key.data = (__force uintptr_t)port;
|
||||
BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
|
||||
|
||||
Reference in New Issue
Block a user