On 23/01/2023 10:38, Konstantin Meskhidze (A) wrote:
1/22/2023 2:07 AM, Günther Noack пишет:
[...]
@@ -143,10 +157,24 @@ for the ruleset creation, by filtering access rights according to the Landlock
ABI version. In this example, this is not required because all of the requested
``allowed_access`` rights are already available in ABI 1.
-We now have a ruleset with one rule allowing read access to ``/usr`` while
-denying all other handled accesses for the filesystem. The next step is to
-restrict the current thread from gaining more privileges (e.g. thanks to a SUID
-binary).
+For network access-control, we can add a set of rules that allow to use a port
+number for a specific action. All ports values must be defined in network byte
+order.
What is the point of asking user space to convert this to network byte
order? It seems to me that the kernel would be able to convert it to
network byte order very easily internally and in a single place -- why
ask all of the users to deal with that complexity? Am I overlooking
something?
I had a discussion about this issue with Mickaёl.
Please check these threads:
1.
https://lore.kernel.org/netdev/49391484-7401-e7c7-d909-3bd6bd024731@xxxxxxxxxxx/
2.
https://lore.kernel.org/netdev/1ed20e34-c252-b849-ab92-78c82901c979@xxxxxxxxxx/
I'm definitely not sure if this is the right solution, or if there is
one. The rationale is to make it close to the current (POSIX) API. We
didn't get many opinion about that but I'd really like to have a
discussion about port endianness for this Landlock API.
I looked at some code (e.g. see [1]) and it seems that using htons()
might make application patching more complex after all. What do you
think? Is there some network (syscall) API that don't use this convention?
[1] https://github.com/landlock-lsm/tuto-lighttpd
+
+.. code-block:: c
+
+ struct landlock_net_service_attr net_service = {
+ .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
+ .port = htons(8080),
+ };
This is a more high-level comment:
The notion of a 16-bit "port" seems to be specific to TCP and UDP --
how do you envision this struct to evolve if other protocols need to
be supported in the future?
When TCP restrictions land into Linux, we need to think about UDP
support. Then other protocols will be on the road. Anyway you are right
this struct will be evolving in long term, but I don't have a particular
envision now. Thanks for the question - we need to think about it.
Should this struct and the associated constants have "TCP" in its
name, and other protocols use a separate struct in the future?
Other protocols such as AF_VSOCK uses a 32-bit port. We could use a
32-bits port field or ever a 64-bit one. The later could make more sense
because each field would eventually be aligned on 64-bit. Picking a
16-bit value was to help developers (and compilers/linters) with the
"correct" type (for TCP).
If we think about protocols other than TCP and UDP (e.g. AF_VSOCK), it
could make sense to have a dedicated attr struct specifying other
properties (e.g. CID). Anyway, the API is flexible but it would be nice
to not mess with it too much. What do you think?
+
+ err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
+ &net_service, 0);
+
+The next step is to restrict the current thread from gaining more privileges
+(e.g. thanks to a SUID binary). We now have a ruleset with the first rule allowing
^^^^^^
"through" a SUID binary? "thanks to" sounds like it's desired
to do that, while we're actually trying to prevent it here?
This is Mickaёl's part. Let's ask his opinion here.
Mickaёl, any thoughts?
Yep, "through" looks better.
