netdev - Re: [RFC PATCH v2 1/8] landlock: Fix non-TCP sockets restriction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5cd7186d-45cc-e31f-3e8e-26aa805f5b5c@huawei-partners.com>
Date: Mon, 9 Dec 2024 13:19:59 +0300
From: Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>
To: Mickaël Salaün <mic@...ikod.net>
CC: <gnoack@...gle.com>, <willemdebruijn.kernel@...il.com>,
	<matthieu@...fet.re>, <linux-security-module@...r.kernel.org>,
	<netdev@...r.kernel.org>, <netfilter-devel@...r.kernel.org>,
	<yusongping@...wei.com>, <artem.kuzin@...wei.com>,
	<konstantin.meskhidze@...wei.com>
Subject: Re: [RFC PATCH v2 1/8] landlock: Fix non-TCP sockets restriction

On 12/4/2024 10:30 PM, Mickaël Salaün wrote:
> On Thu, Oct 17, 2024 at 07:04:47PM +0800, Mikhail Ivanov wrote:
>> Do not check TCP access right if socket protocol is not IPPROTO_TCP.
>> LANDLOCK_ACCESS_NET_BIND_TCP and LANDLOCK_ACCESS_NET_CONNECT_TCP
>> should not restrict bind(2) and connect(2) for non-TCP protocols
>> (SCTP, MPTCP, SMC).
>>
>> sk_is_tcp() is used for this to check address family of the socket
>> before doing INET-specific address length validation. This is required
>> for error consistency.
>>
>> Closes: https://github.com/landlock-lsm/linux/issues/40
>> Fixes: fff69fb03dde ("landlock: Support network rules with TCP bind and connect")
>> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>
>> ---
>>
>> Changes since v1:
>> * Validate socket family (=INET{,6}) before any other checks
>>    with sk_is_tcp().
>> ---
>>   security/landlock/net.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/security/landlock/net.c b/security/landlock/net.c
>> index fdc1bb0a9c5d..1e80782ba239 100644
>> --- a/security/landlock/net.c
>> +++ b/security/landlock/net.c
>> @@ -66,8 +66,8 @@ static int current_check_access_socket(struct socket *const sock,
>>   	if (WARN_ON_ONCE(dom->num_layers < 1))
>>   		return -EACCES;
>>   
>> -	/* Checks if it's a (potential) TCP socket. */
>> -	if (sock->type != SOCK_STREAM)
>> +	/* Do not restrict non-TCP sockets. */
> 
> You can remove this comment because the following check is explicit.

ok, thx

> 
>> +	if (!sk_is_tcp(sock->sk))
>>   		return 0;
>>   
>>   	/* Checks for minimal header length to safely read sa_family. */
>> -- 
>> 2.34.1
>>
>>