9/18/2023 9:56 AM, Mickaël Salaün пишет:

On Fri, Sep 15, 2023 at 11:54:46AM +0300, Konstantin Meskhidze (A) wrote:

quoted

9/14/2023 11:08 AM, Mickaël Salaün пишет:

quoted

On Mon, Sep 11, 2023 at 01:13:24PM +0300, Konstantin Meskhidze (A) wrote:

quoted

8/17/2023 6:08 PM, Mickaël Salaün пишет:

quoted

On Sat, Aug 12, 2023 at 05:37:00PM +0300, Konstantin Meskhidze (A) wrote:

quoted

quoted

quoted

quoted

quoted

7/12/2023 10:02 AM, Mickaël Salaün пишет:

On 06/07/2023 16:55, Mickaël Salaün wrote:
From: Konstantin Meskhidze <redacted>

quoted

quoted

This patch is a revamp of the v11 tests [1] with new tests

(see the

quoted

quoted

"Changes since v11" description).  I (Mickaël) only added the following
todo list and the "Changes since v11" sections in this commit message.
I think this patch is good but it would appreciate reviews.
You can find the diff of my changes here but it is not really readable:
https://git.kernel.org/mic/c/78edf722fba5 (landlock-net-v11 branch)
[1] https://lore.kernel.org/all/20230515161339.631577-11-konstantin.meskhidze@huawei.com/ (local)
TODO:
- Rename all "net_service" to "net_port".
- Fix the two kernel bugs found with the new tests.
- Update this commit message with a small description of all tests.
[...]
We should also add a test to make sure errno is the same

with and

quoted

quoted

quoted

without sandboxing when using port 0 for connect and consistent with
bind (using an available port). The test fixture and variants should be
quite similar to the "ipv4" ones, but we can also add AF_INET6 variants,
which will result in 8 "ip" variants:

quoted

TEST_F(ip, port_zero)

{
	if (variant->sandbox == TCP_SANDBOX) {
		/* Denies any connect and bind. */
	}
	/* Checks errno for port 0. */
}

As I understand the would be the next test cases:

quoted

quoted

	1. ip4, sandboxed, bind port 0 -> should return EACCES

(denied by

quoted

quoted

landlock).
Without any allowed port, yes. This test case is useful.
By tuning /proc/sys/net/ipv4/ip_local_port_range (see

inet_csk_find_open_port call) we should be able to pick a specific
allowed port and test it.  We can also test for the EADDRINUSE error to
make sure error ordering is correct (compared with -EACCES).

  Sorry, did not get this case. Could please explain it with more details?

According to bind(2), if no port are available, the syscall should
return EADDRINUSE. And this returned value should be the same whatever
the process is sandbox or not (and never EACCES). But as I explained
just below, we cannot know this random port from the LSM hook, so no
need to tweak /proc/sys/net/ipv4/ip_local_port_range, and your this is
correct:

1. ip4, sandboxed, bind port 0 -> should return EACCES (denied by
landlock).

  yep, adding rule with port 0 (for bind) returns EINVAL then
  calling bind port 0 returns EACCES cause there is no rule with port 0.

quoted

quoted

quoted

quoted

However, I think the current LSM API don't enable to infer this

random

quoted

port because the LSM hook is called before a port is picked.  If this is
correct, the best way to control port binding would be to always deny
binding on port zero/random (when restricting port binding, whatever
exception rules are in place). This explanation should be part of a
comment for this specific exception.

  Yep, if some LSM rule (for bind) has been applied a with specific port,
other attemps to bind with zero/random ports would be refused by LSM
security checks.

To say it another way, we should not allow to add a rule with port 0 for
LANDLOCK_ACCESS_NET_BIND_TCP, but return -EINVAL in this case. This
limitation should be explained, documented and tested.

With (only) LANDLOCK_ACCESS_NET_CONNECT_TCP it should be allowed though
(except if there is also LANDLOCK_ACCESS_NET_BIND_TCP) of course.
Another test should cover the case with a new rule with these two access
rights and port 0.

 I think it's possible to have LANDLOCK_ACCESS_NET_CONNECT_TCP with port 0
with LANDLOCK_ACCESS_NET_BIND_TCP at the same time, cause
LANDLOCK_ACCESS_NET_BIND_TCP rule is allowed (by Landlock) with any other
port but 0.

It would mask the fact that port zero cannot be allowed, which could be
possible one day. So for now we need to return EINVAL in this case.

   Got it. I added bind mask in add_rule_net_service() to check that 
zero port is not allowed with bind action. I sent all changes in the 
latest V12 patch.

.