On 29/06/2023 05:18, Jeff Xu wrote:

resend.

On Wed, Jun 28, 2023 at 12:29 PM Mickaël Salaün [off-list ref] wrote:

quoted

On 28/06/2023 19:03, Jeff Xu wrote:

quoted

Hello,

Thanks for writing up the example for an incoming TCP connection ! It
helps with the context.

Since I'm late to this thread, one thing I want to ask:  all the APIs
proposed so far are at the process level, we don't have any API that
applies restriction to socket fd itself, right ? this is what I
thought, but I would like to get confirmation.

Restriction are applied to actions, not to already existing/opened FDs.
We could add a way to restrict opened FDs, but I don't think this is the
right approach because sandboxing is a deliberate action from a process,
and it should already take care of its FDs.

quoted

On Wed, Jun 28, 2023 at 2:09 AM Günther Noack [off-list ref] wrote:

quoted

Hello!

On Mon, Jun 26, 2023 at 05:29:34PM +0200, Mickaël Salaün wrote:

quoted

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.

That is an interesting aspect that might be worth discussing more.
seccomp is per syscall, landlock doesn't necessarily follow the same,
another design is to add more logic in Landlock, e.g.
LANDLOCK_ACCESS_SOCKET_PROTOCOL which will apply to all of the socket
calls (socket/bind/listen/accept/connect). App dev might feel it is
easier to use.

seccomp restricts the use of the syscall interface, whereas Landlock
restricts the use of kernel objects (i.e. the semantic).

We need to find a good tradeoff between a lot of access rights and a few
grouping different actions. This should make sense from a developer
point of view according to its knowledge and use of the kernel
interfaces (potential wrapped with high level libraries), but also to
the semantic of the sandbox and the security guarantees we want to provide.

We should also keep in mind that high level Landlock libraries can take
care of potential coarse-grained use of restrictions.

quoted

quoted

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.

B)

A program that runs a TCP server could specify a ruleset where
LANDLOCK_NET_BIND_TCP, LANDLOCK_NET_CONNECT_TCP and
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.

TCP server is an interesting case. From a security perspective, a
process cares if it is acting as a server or client in TCP, a server
might only want to accept an incoming TCP connection, never initiate
an outgoing TCP connection, and a client is the opposite.

Processes can restrict outgoing/incoming TCP connection by seccomp for
accept(2) or connect(2),  though I feel Landlock can do this more
naturally for app dev, and at per-protocol level.  seccomp doesn't
provide per-protocol granularity.

Right, seccomp cannot filter TCP ports.

quoted

For bind(2), iirc, it can be used for a server to assign dst port of
incoming TCP connection, also by a client to assign a src port of an
outgoing TCP connection. LANDLOCK_NET_BIND_TCP will apply to both
cases, right ? this might not be a problem, just something to keep
note.

Good point. I think it is in line with the rule definition: to allow to
bind on a specific port. However, if clients want to set the source port
to a (legitimate) value, then that would be an issue because we cannot
allow a whole range of ports (e.g., >= 1024). I'm not sure if this
practice would be deemed "legitimate" though. Do you know client
applications using bind?

Konstantin, we should have a test for this case anyway.

Thinking more about TCP clients binding sockets, a 
LANDLOCK_ACCESS_NET_LISTEN_TCP would be more useful than 
LANDLOCK_ACCESS_NET_BIND_TCP, but being able to limit the scope of 
"bindable" ports is also valuable to forbid a malicious sandboxed 
process to impersonate a legitimate server process. This also means that 
it might be interesting to be able to handle port ranges.

We already have a LANDLOCK_ACCESS_NET_BIND_TCP implementation and 
related tests, so I think we should proceed with that. The next 
network-related patch series should implement this 
LANDLOCK_ACCESS_NET_LISTEN_TCP access right though, which should not be 
difficult thanks to the framework implemented with current patch series.

Konstantin, would you like to develop the TCP listening access control 
once this patch series land?

quoted

quoted

quoted

(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.)

For this approach, LANDLOCK_ACCESS_SOCKET_PROTOCOL is a better name,
so dev is fully aware it is not just applied to socket create.

I don't get the semantic of LANDLOCK_ACCESS_SOCKET_PROTOCOL. What does
PROTOCOL mean?

I meant checking family + type of socket, and apply to all of
socket(2),bind(2),accept(2),connect(2),listen(2), maybe
send(2)/recv(2) too.

OK, that would be kind of similar to the LANDLOCK_ACCESS_SOCKET_USE 
description. However, I think this kind of global approach has several 
issues:
- This covers a lot of different aspects and would increase the cost of 
development/testing/review.
- Whereas it wraps different actions, it will not let user space have a 
fine-grained access control on these, which could be useful for some use 
cases.
- I don't see the point of restricting accept(2) if we can already 
restrict bind(2) and listen(2). accept(2) could be useful to identify 
the remote peer but I'm not convinced this would make sense, and if it 
would, then this can be postponed until we have a way to identify peers.
- For performance reasons, we should avoid restricting 
send/recv/read/write but instead only restrict the control plane: object 
creation and configuration.

I'm not convinced that being able to control all kind of socket bind, 
listen and connect actions might be worth implementing instead of a 
fine-grained access control for the main protocols (TCP, UDP, unix and 
vsock maybe), with the related tests and guarantees.

However, this landlock_socket_attr struct could have an allowed_access 
field that could contain LANDLOCK_ACCESS_NET_{CONNECT,LISTEN,BIND}_TCP 
rights (which would just not be constrained by any port, except if a 
landlock_net_port_attr rule matches). It would then make sense to rename 
LANDLOCK_ACCESS_SOCKET_CREATE to LANDLOCK_ACCESS_NET_CREATE_SOCKET. This 
right would not be accepted in a landlock_net_port_attr.allowed_access 
though.

s/LANDLOCK_ACCESS_SOCKET_CREATE/LANDLOCK_ACCESS_SOCKET_TYPE.

This implies the kernel will check on socket fd's property (family +
type) at those calls, this applies to
a - the socket fd is created within the process, after landlock is applied.
b - created in process prior to landlock is applied.
c - created out of process then passed into this process,

OK, these are the same rules as for LANDLOCK_ACCESS_NET_{CONNECT,BIND}_TCP.