Re: [RFC PATCH v3 14/19] selftests/landlock: Test socketpair(2) restriction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/29/2024 8:31 PM, Mickaël Salaün wrote:
On Sat, Sep 28, 2024 at 10:06:52PM +0200, Günther Noack wrote:
On Fri, Sep 27, 2024 at 11:48:22AM +0200, Günther Noack wrote:
On Mon, Sep 23, 2024 at 03:57:47PM +0300, Mikhail Ivanov wrote:
(Btw I think that disassociation control can be really useful. If
it were possible to restrict this action for each protocol, we would
have stricter control over the protocols used.)

In my understanding, the disassociation support is closely intertwined with the
transport layer - the last paragraph of DESCRIPTION in connect(2) is listing
TCP, UDP and Unix Domain sockets in datagram mode. -- The relevant code in in
net/ipv4/af_inet.c in inet_dgram_connect() and __inet_stream_connect(), where
AF_UNSPEC is handled.

I would love to find a way to restrict this independent of the specific
transport protocol as well.

Remark on the side - in af_inet.c in inet_shutdown(), I also found a worrying
scenario where the same sk->sk_prot->disconnect() function is called and
sock->state also gets reset to SS_UNCONNECTED.  I have done a naive attempt to
hit that code path by calling shutdown() on a passive TCP socket, but was not
able to reuse the socket for new connections afterwards. (Have not debugged it
further though.)  I wonder whether this is a scnenario that we also need to
cover?

FYI, **this does turn out to work** (I just fumbled in my first experiment). --
It is possible to reset a listening socket with shutdown() into a state where it
can be used for at least a new connect(2), and maybe also for new listen(2)s.

Interesting syscall...


The same might also be possible if a socket is in the TCP_SYN_SENT state at the
time of shutdown() (although that is a bit trickier to try out).

So a complete disassociation control for TCP/IP might not only need to have
LANDLOCK_ACCESS_SOCKET_CONNECT_UNSPEC (or however we'd call it), but also
LANDLOCK_ACCESS_SOCKET_PASSIVE_SHUTDOWN and maybe even another one for the
TCP_SYN_SENT case...? *

That would make the Landlock interface too complex, we would need a more
generic approach instead e.g. with a single flag.


It makes me uneasy to think that I only looked at AF_INET{,6} and TCP so far,
and that other protocols would need a similarly close look.  It will be
difficult to cover all the "disassociation" cases in all the different
protocols, and even more difficult to detect new ones when they pop up.  If we
discover new ones and they'd need new Landlock access rights, it would also
potentially mean that existing Landlock users would have to update their rules
to spell that out.

It might be easier after all to not rely on "disassociation" control too much
and instead to design the network-related access rights in a way so that we can
provide the desired sandboxing guarantees by restricting the "constructive"
operations (the ones that initiate new network connections or that listen on the
network).

I agree.  So, with the ability to control socket creation and to also
control listen/bind/connect (and sendmsg/recvmsg for datagram protocols)
we should be good right?


Mikhail, in your email I am quoting above, you are saying that "disassociation
control can be really useful"; do you know of any cases where a restriction of
connect/listen is *not* enough and where you'd still want the disassociation
control?

Disassociation is basically about making socket be able to connect or
listen (again). If these actions are already controlled, disassociation
should always be permitted (as it's currently implemented for TCP
connect).

I thought that LANDLOCK_ACCESS_SOCKET_CONNECT_UNSPEC would be useful
for the protocols that do not have related LANDLOCK_ACCESS_NET_*
access rights. It would allow to (for example) create listening socket
of non-TCP(UDP) protocol and fully restrict networking (by restricting
any disassociation and socket creation).

But since disasossication is implemented in the transport layer there
is no clear way to control it with socket access. Considering this and
that previous scenario can be achieved by implementing networking
control (LANDLOCK_ACCESS_NET_* rights) for a needed protocol, potential
cost of "disassociation control" implementation is much more than the
benefits.


(In my mind, the disassociation control would have mainly been needed if we had
gone with Mickaël's "Use socket's Landlock domain" RFC [1]?  Mickaël and me have
discussed this patch set at LSS and I am also now coming around to the
realization that this would have introduced more complication.  - It might have
been a more "pure" approach, but comes at the expense of complicating Landlock
usage.)

Indeed, and this RFC will not be continued.  We should not think of a
socket as a security object (i.e. a real capability), whereas sockets
are kernel objects used to configure and exchange data, a bit like a
command multiplexer for network actions that can also be used to
identify peers.

Because Landlock is a sandboxing mechanism, the security policy tied to
a task may change during its execution, which is not the case for other
access control systems such as SELinux.  That's why we should not
blindly follow other security models.  In the case of socket control,
Landlock uses the calling task's credential to check if the call should
be allowed.  In the case of abstract UNIX socket control (with Linux
5.12), the check is done on the domain that created the peer's socket,
not the domain that will received the packet.  In this case Landlock can
rely on the peer socket's domain because it is a consistent and
race-free way to identify a peer, and this peer socket is not the one
doing the action.  It's a bit different with non-UNIX sockets because
peers may not be local to the system.


—Günther

[1] https://lore.kernel.org/all/20240719150618.197991-1-mic@xxxxxxxxxxx/

* for later reference, my reasoning in the code is: net/ipv4/af_inet.c
   implements the entry points for connect() and listen() at the address family
   layer.  Both operations require that the sock->state is SS_UNCONNECTED.  So
   the rest is going through the other occurrences of SS_UNCONNECTED in that same
   file to see if there are any places where the socket can get back into that
   state.  The places I found where it is set to that state are:
1. inet_create (right after creation, expected)
   2. __inet_stream_connect in the AF_UNSPEC case (known issue)
   3. __inet_stream_connect in the case of a failed connect (expected)
   4. inet_shutdown in the case of TCP_LISTEN or TCP_SYN_SENT (mentioned above)





[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux