netdev - Re: [PATCH v2 0/6] landlock: Add UDP access control support

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: <96feafd3-de2d-3a0e-102a-40418ab79848@huawei-partners.com>
Date: Fri, 24 Jan 2025 13:54:03 +0300
From: Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>
To: Matthieu Buffet <matthieu@...fet.re>, Mickael Salaun <mic@...ikod.net>
CC: Gunther Noack <gnoack@...gle.com>, <konstantin.meskhidze@...wei.com>, Paul
 Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>, "Serge E .
 Hallyn" <serge@...lyn.com>, <linux-security-module@...r.kernel.org>,
	<netdev@...r.kernel.org>
Subject: Re: [PATCH v2 0/6] landlock: Add UDP access control support

On 12/14/2024 9:45 PM, Matthieu Buffet wrote:
> Hi Mickael,
> 
> Thanks for your comments on the v1 of this patch, I should have everything
> fixed so (hopefully) this v2 boils down to something simpler.
> 
> This patchset is based on
> https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git
> Linux 6.12 (adc218676eef).
> 
> This patchset should add basic support to completely block a process
> from sending and receiving UDP datagrams, and delegate the right to
> send/receive based on remote/local port. It should fit nicely with
> the socket creation restrictions WIP (either don't have UDP at all, or
> have it with just the rights needed).
> 
> @Mikhail: I saw the discussions around TCP error code inconsistencies +
> over-restriction, and your patch v1. I took extra care to minimize this
> diff size: no unnecessary comment/refactor, especially in
> current_check_access_socket(). It should be just what is required for a
> basic UDP support without changing error handling in that main function.

Hello, Matthieu! Thank you for sending the second version and
checking these fix patches.

We decided to implement errors consistency fix for the whole LSM [1],
but your patches will merge well with current_check_access_socket()
refactoring [2].

[1] https://lore.kernel.org/all/20241210.ahg9Zawoobie@digikod.net/
[2] 
https://lore.kernel.org/all/20241017110454.265818-3-ivanov.mikhail1@huawei-partners.com/

> 
> The only question that remained open from v1 was about UDP rights naming.
> Since there were no strong preferences and the hooks now only handle
> sendmsg() if an explicit address is specified, that's now
> LANDLOCK_ACCESS_NET_UDP_SENDTO since the name (and prototype with a
> destination address parameter) of sendto(3) is closer to these semantics.
This can be interpreted as a restriction only for sendto(2), but I don't 
see any better options.

> 
> Changes since v1 (link below):
> - recvmsg hook is gone and sendmsg hook doesn't apply to connected
>    sockets anymore, to improve performance
> - don't add a get_addr_port() helper function, which required a weird "am
>    I in IPv4 or IPv6 context" to avoid a addrlen>sizeof(struct sockaddr_in)
>    check in connect(AF_UNSPEC) IPv6 context. A helper was useful when ports
>    also needed to be read in a recvmsg() hook, now it's just a simple
>    switch case in the sendmsg() hook, more readable
> - rename sendmsg access right to LANDLOCK_ACCESS_NET_UDP_SENDTO
> - reorder hook prologue for consistency: check domain, then type and
>    family
> - add additional selftests cases around minimal address length
> - update documentation
> 
> lcov gives me net.c going from 94% lines/80% functions to 96.6% lines/
> 85.7% functions
> 
> Any feedback welcome!
> 
> Link: https://lore.kernel.org/all/20240916122230.114800-1-matthieu@buffet.re/
> Closes: https://github.com/landlock-lsm/linux/issues/10
> 
> Link: https://lore.kernel.org/all/20241017110454.265818-1-ivanov.mikhail1@huawei-partners.com/
> Cc: Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>
> 
> Matthieu Buffet (6):
>    landlock: Add UDP bind+connect access control
>    selftests/landlock: Adapt existing bind/connect for UDP
>    landlock: Add UDP sendmsg access control
>    selftests/landlock: Add ACCESS_NET_SENDTO_UDP
>    samples/landlock: Add sandboxer UDP access control
>    doc: Add landlock UDP support
> 
>   Documentation/userspace-api/landlock.rst     |  84 +++-
>   include/uapi/linux/landlock.h                |  67 ++-
>   samples/landlock/sandboxer.c                 |  58 ++-
>   security/landlock/limits.h                   |   2 +-
>   security/landlock/net.c                      | 137 +++++-
>   security/landlock/syscalls.c                 |   2 +-
>   tools/testing/selftests/landlock/base_test.c |   2 +-
>   tools/testing/selftests/landlock/net_test.c  | 455 +++++++++++++++++--
>   8 files changed, 715 insertions(+), 92 deletions(-)
> 
> 
> base-commit: adc218676eef25575469234709c2d87185ca223a