Message-ID: <c8588051-8795-9b8a-cb36-f5440b590581@digikod.net>
Date:   Fri, 17 Dec 2021 10:39:23 +0100
From:   Mickaël Salaün <mic@...ikod.net>
To:     Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
Cc:     yusongping <yusongping@...wei.com>,
        Artem Kuzin <artem.kuzin@...wei.com>,
        linux-security-module <linux-security-module@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        netfilter@...r.kernel.org
Subject: Re: [RFC PATCH 0/2] Landlock network PoC implementation

New discussions and RFCs should also include netdev and netfilter 
mailing lists. For people new to Landlock, the goal is to enable 
unprivileged processes (and then potentially malicious ones) to limit 
their own network access (i.e. create a security sandbox for themselves).

Thinking more about network access control for Landlock use case, here 
are better suggestions:

On 14/12/2021 12:51, Mickaël Salaün wrote:
> 
> On 14/12/2021 04:49, Konstantin Meskhidze wrote:
>> Hi Mickaёl.
>> I've been thinking about your reply:
>>
>>> 4. Kernel objects.
>>> For filesystem restrictions inodes objects are used to tie landlock 
>>> rules.
>>> But for socket operations it's preferred to use task_struct object of
>>> a process, cause sockets' inodes are created just after
>>> security_socket_create() hook is called, and if its needed to have
>>> some restriction rule for creating sockets, this rule can't be tied
>>> to a socket inode cause there is no any has been created at the hook's
>>> catching moment, see the sock_create_lite() function below:
>>
>> - For the file system, we use inodes to identify hierarchies. We can't
>> - safely rely on stateless objects (e.g. path strings) because the file
>> - system changes, and then the rules must change with it.
>>
>> - To identify network objects (from the user point of view), we can rely
>> - on stateless rule definitions because they may be absolute (i.e. IP
>> - address), e.g. sandbox process creating a new connection or 
>> receveing an
>> - UDP packet. It is not be the case with UNIX socket if they are come 
>> from
>> - a path (i.e. inode) though. In this case we'll have to use the existing
>> - file system identification mechanism and probably extend the current FS
>> - access rights.
>> - A sandbox is a set of processes handled as "subjects". Generic inet
>> - rules should not be tied to processes (for now) but on 
>> subnets/protocols.
>>
>> In current Landlock version inodes are the objects to tie rules to.
>> For network you are saying that we can rely on stateless rule 
>> definitions and
>> rules should be tied to subnets/protocols, not to processes' 
>> task_struct objects.
>> Cause Landlock architecture requires all rules to be tied to a different
>> kernel objects, and when LSM hooks are caught there must be search
>> procedure completed in a ruleset's red-black tree structure:
>>     kernel_object -> landlock_object <- landlock_rule 
>> <-----landlock_ruleset
>>
>> What kind of kernel objects do you mean by subnets/protocols?
>> Do you suggest using sockets' inodes in this case or using network rules
>> without to be tied to any kernel object?
> 
> The subnets/protocols is the definition provided when creating a rule 
> (i.e. the object from the user point of view), but the kernel may relies 
> on other internal representations. I guess datagram packets would need 
> to be matched against IP/port everytime they are received by a sandboxed 
> process, but tagging sockets or their underlying inodes for stream 
> connections make sense.
> 
> I don't have experience in the network LSM hooks though, any input is 
> welcome.
> 
>>     socket_inode -> landlock_object <- landlock_rule 
>> <-----landlock_ruleset
>>              OR
>>     landlock_object <- landlock_rule <-----landlock_ruleset
>>
>> -----Original Message-----
>> From: Mickaël Salaün <mic@...ikod.net>
>> Sent: Monday, December 13, 2021 4:30 PM
>> To: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
>> Cc: linux-security-module@...r.kernel.org; yusongping 
>> <yusongping@...wei.com>; Artem Kuzin <artem.kuzin@...wei.com>
>> Subject: Re: [RFC PATCH 0/2] Landlock network PoC implementation
>>
>> Hi Konstantin,
>>
>> On 10/12/2021 08:21, Konstantin Meskhidze wrote:

[...]

>>
>> To sum up, for IPv4 restrictions, we need a new rule type identified
>> with LANDLOCK_RULE_NET_CIDR4. This will handle a new
>> struct landlock_net_cidr4_attr {
>>       __u64 allowed_access;
>>       __u32 address; // IPv4
>>       __u8 prefix; // From 0 to 32
>>       __u8 type; // SOCK_DGRAM, SOCK_STREAM
>>       __u16 port;
>> } __attribute__((packed));
>> // https://datatracker.ietf.org/doc/html/rfc4632

IP addresses (and subnets) should not be part of a rule, at least for 
now. Indeed, IP addresses are tied either to the system architecture 
(e.g. container configuration), the local network or Internet, hence 
moving targets not controlled by application developers. Moreover, from 
a kernel point of view, it is more complex to check and handle subnets, 
which are most of the time tied to the Netfilter infrastructure, not 
suitable for Landlock because of its unprivileged nature.

On the other side, protocols such as TCP and their associated ports are 
normalized and are tied to an application semantic (e.g. TCP/443 for HTTPS).

There is other advantages to exclude subnets from this type of rules for 
now (e.g. they could be composed with protocols/ports), but that may 
come later.

I then think that a first MVP to bring network access control support to 
Landlock should focus only on TCP and related ports (i.e. services). I 
propose to not use my previous definition of landlock_net_cidr4_attr but 
to have a landlock_net_service_attr instead:

struct landlock_net_service_attr {
     __u64 allowed_access; // LANDLOCK_NET_*_TCP
     __u16 port;
} __attribute__((packed));

This attribute should handle IPv4 and IPv6 indistinguishably.

[...]

>>
>> Accesses/suffixes should be:
>> - CREATE
>> - ACCEPT
>> - BIND
>> - LISTEN
>> - CONNECT
>> - RECEIVE (RECEIVE_FROM and SEND_TO should not be needed)
>> - SEND
>> - SHUTDOWN
>> - GET_OPTION (GETSOCKOPT)
>> - SET_OPTION (SETSOCKOPT)

For now, the only access rights should be LANDLOCK_ACCESS_NET_BIND_TCP 
and LANDLOCK_ACCESS_NET_CONNECT_TCP (tie to two LSM hooks with struct 
sockaddr).

These attribute and access right changes reduce the scope of the network 
access control and make it simpler but still really useful. Datagram 
(e.g. UDP, which could add BIND_UDP and SEND_UDP) sockets will be more 
complex to restrict correctly and should then come in another patch 
series, once TCP is supported.

Powered by blists - more mailing lists