Message-ID: <618f11b6-7766-95b1-8fef-679de21b1fa2@digikod.net>
Date: Wed, 28 Jun 2023 21:07:01 +0200
From: Mickaël Salaün <mic@...ikod.net>
To: Günther Noack <gnoack@...gle.com>
Cc: "Konstantin Meskhidze (A)" <konstantin.meskhidze@...wei.com>,
 Günther Noack <gnoack3000@...il.com>,
 willemdebruijn.kernel@...il.com, linux-security-module@...r.kernel.org,
 netdev@...r.kernel.org, netfilter-devel@...r.kernel.org,
 yusongping@...wei.com, artem.kuzin@...wei.com, Jeff Xu <jeffxu@...gle.com>,
 Jorge Lucangeli Obes <jorgelo@...omium.org>,
 Allen Webb <allenwebb@...gle.com>, Dmitry Torokhov <dtor@...gle.com>
Subject: Re: [PATCH v9 00/12] Network support for Landlock - allowed list of
 protocols


On 28/06/2023 10:44, Günther Noack wrote:
> Hello!
> 
> On Mon, Jun 26, 2023 at 05:29:34PM +0200, Mickaël Salaün wrote:
>> Here is a design to be able to only allow a set of network protocols and
>> deny everything else. This would be complementary to Konstantin's patch
>> series which addresses fine-grained access control.
>>
>> First, I want to remind that Landlock follows an allowed list approach with
>> a set of (growing) supported actions (for compatibility reasons), which is
>> kind of an allow-list-on-a-deny-list. But with this proposal, we want to be
>> able to deny everything, which means: supported, not supported, known and
>> unknown protocols.
>>
>> We could add a new "handled_access_socket" field to the landlock_ruleset
>> struct, which could contain a LANDLOCK_ACCESS_SOCKET_CREATE flag.
>>
>> If this field is set, users could add a new type of rules:
>> struct landlock_socket_attr {
>>      __u64 allowed_access;
>>      int domain; // see socket(2)
>>      int type; // see socket(2)
>> }
>>
>> The allowed_access field would only contain LANDLOCK_ACCESS_SOCKET_CREATE at
>> first, but it could grow with other actions (which cannot be handled with
>> seccomp):
>> - use: walk through all opened FDs and mark them as allowed or denied
>> - receive: hook on received FDs
>> - send: hook on sent FDs
>>
>> We might also use the same approach for non-socket objects that can be
>> identified with some meaningful properties.
>>
>> What do you think?
> 
> This sounds like a good plan to me - it would make it possible to restrict new
> socket creation using protocols that were not intended to be used, and I also
> think it would fit the Landlock model nicely.
> 
> Small remark on the side: The security_socket_create() hook does not only get
> invoked as a result of socket(2), but also as a part of accept(2) - so this
> approach might already prevent new connections very effectively.

Indeed. We could also differentiate socket(2) from accept(2) with a 
dedicated LANDLOCK_ACCESS_SOCKET_ACCEPT right. This would enable to 
create a bind socket, sandbox the process and deny new socket(2) calls, 
but still allows to call accept(2) and receive new connections.

BTW, unix socket path opening should be considered too.

> 
> Spelling out some scenarios, so that we are sure that we are on the same page:
> 
> A)
> 
> A program that does not need networking could specify a ruleset where
> LANDLOCK_ACCESS_SOCKET_CREATE is handled, and simply not permit anything.

This is correct, except if the process receive a socket FD or open a 
unix socket path.


> 
> B)
> 
> A program that runs a TCP server could specify a ruleset where
> LANDLOCK_NET_BIND_TCP, LANDLOCK_NET_CONNECT_TCP and

s/LANDLOCK_NET_CONNECT_TCP/LANDLOCK_ACCESS_NET_CONNECT_TCP/

> LANDLOCK_ACCESS_SOCKET_CREATE are handled, and where the following rules are added:
> 
>    /* From Konstantin's patch set */
>    struct landlock_net_service_attr bind_attr = {
>      .allowed_access = LANDLOCK_NET_BIND_TCP,
>      .port = 8080,
>    };
> 
>    /* From Mickaël's proposal */
>    struct landlock_socket_attr sock_inet_attr = {
>      .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
>      .domain = AF_INET,
>      .type = SOCK_STREAM,
>    }
> 
>    struct landlock_socket_attr sock_inet6_attr = {
>      .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
>      .domain = AF_INET6,
>       .type = SOCK_STREAM,
>    }
> 
> That should then be enough to bind and listen on ports, whereas outgoing
> connections with TCP and anything using other network protocols would not be
> permitted.
> 
> (Alternatively, it could bind() the socket early, *then enable Landlock* and
> leave out the rule for BIND_TCP, only permitting SOCKET_CREATE for IPv4 and
> IPv6, so that listen() and accept() work on the already-bound socket.)

correct

> 
> Overall, this sounds like an excellent approach to me. 👍
> 
> —Günther
> 

Powered by blists - more mailing lists