7/1/2024 6:47 PM, Günther Noack wrote:
On Mon, Jul 01, 2024 at 04:10:27PM +0300, Ivanov Mikhail wrote:
Thanks for the great explanation! We're on the same page.
Considering that binding to ephemeral ports can be done not only with
bind() or listen(), I think your approach is more correct.
Restricting any possible binding to ephemeral ports just using
LANDLOCK_ACCESS_NET_BIND_TCP wouldn't allow sandboxed processes
to deny listen() without pre-binding (which is quite unsafe) and
use connect() in the usuall way (without pre-binding).
Controlling ephemeral ports allocation for listen() can be done in the
same way as for LANDLOCK_ACCESS_NET_BIND_TCP in the patch with
LANDLOCK_ACCESS_NET_LISTEN_TCP access right implementation.
That sounds good, yes! 👍
I'm only concerned about controlling the auto-binding for other
operations (such as connect() and sendto() for UDP). As I said, I think
this can also be useful: users will be able to control which processes
are allowed to use ephemeral ports from ip_local_port_range and which
are not, and they must assign ports for each operation explicitly. If
you agree that such control is reasonable, we'll probably have to
consider some API changes, since such control is currently not possible.
We should clarify this before I send a patch with the
LANDLOCK_ACCESS_NET_LISTEN_TCP implementation. WDYT?
LANDLOCK_ACCESS_NET_LISTEN_TCP seems like the most important to me.
For connect() and sendto(), I think the access rights are less urgent:
connect(): We already have LANDLOCK_ACCESS_NET_CONNECT_TCP, but that one is
getting restricted for the *remote* port number.
(a) I think it would be possible to do the same for the *local* port number, by
introducing a separate LANDLOCK_ACCESS_NET_CONNECT_TCP_LOCALPORT right.
(Yes, the name is absolutely horrible, this is just for the example :))
hook_socket_connect() would then need to do both a check for the remote
port using LANDLOCK_ACCESS_NET_CONNECT_TCP, as it already does today, as
well as a check for the (previously bound?) local port using
LANDLOCK_ACCESS_NET_CONNECT_TCP_LOCALPORT.
So I think it is extensible in that direction, in principle, even though I
don't currently have a good name for that access right. :)
Indeed, implementing a new type of rule seems to be an optimal approach
for this.
(b) Compared to what LANDLOCK_ACCESS_NET_BIND_TCP already restricts, a
hypothetical LANDLOCK_ACCESS_NET_CONNECT_TCP_LOCALPORT right would only
additionally restrict the use of ephemeral ports. I'm currently having a
hard time seeing what an attacker could do with that (use up all ephemeral
ports?).
I thought about something like that, yeah) Also, I tried to find out
if there are cases where port remains bound to the client socket after
the connection is completed. In this case, listen() can be called to a
socket with a port bound via connect() call. In the case of TCP, such
scenario is not possible, port is always deallocated in tcp_set_state()
method.
So, I don't see any realworld vulnerabilities, we can leave this case
until someone comes up with a real issue.
sendto(): I think this is not relevant yet, because as the documentation said,
ephemeral ports are only handed out when sendto() is used with datagram (UDP)
sockets.
Once Landlock starts having UDP support, this would become relevant, but for
this patch set, I think that the TCP server use case as discussed further above
in this thread is very compelling.
Agreed. Anyway, if someone finds any interesting cases with
auto-binding via sendto(), we can implement a new rule, as you suggested
for connect().
Thanks you for your research and ideas, Günther!
I'll prepare the LANDLOCK_ACCESS_NET_LISTEN_TCP patchset for the review.
Thanks,
—Günther
