Re: [RFC PATCH v4 09/15] landlock: TCP network hooks implementation

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

 



4/11/2022 7:24 PM, Mickaël Salaün пишет:


On 09/03/2022 14:44, Konstantin Meskhidze wrote:

Support of socket_bind() and socket_connect() hooks.
Its possible to restrict binding and connecting of TCP
types of sockets to particular ports. Its just basic idea
how Landlock could support network confinement.

Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@xxxxxxxxxx>
---

Changes since v3:
* Split commit.
* Add SECURITY_NETWORK in config.
* Add IS_ENABLED(CONFIG_INET) if a kernel has no INET configuration.
* Add hook_socket_bind and hook_socket_connect hooks.

---
  security/landlock/Kconfig    |   1 +
  security/landlock/Makefile   |   2 +-
  security/landlock/net.c      | 180 +++++++++++++++++++++++++++++++++++
  security/landlock/net.h      |  22 +++++
  security/landlock/ruleset.h  |   6 ++
  security/landlock/setup.c    |   2 +
  security/landlock/syscalls.c |  61 +++++++++++-
  7 files changed, 271 insertions(+), 3 deletions(-)
  create mode 100644 security/landlock/net.c
  create mode 100644 security/landlock/net.h

diff --git a/security/landlock/Kconfig b/security/landlock/Kconfig
index 8e33c4e8ffb8..2741f97169a7 100644
--- a/security/landlock/Kconfig
+++ b/security/landlock/Kconfig
@@ -4,6 +4,7 @@ config SECURITY_LANDLOCK
      bool "Landlock support"
      depends on SECURITY && !ARCH_EPHEMERAL_INODES
      select SECURITY_PATH
+    select SECURITY_NETWORK
Nit: please move SECURITY_NETWORK before SECURITY_PATH (alphanumeric order). 

 I got it. Thanks




      help
        Landlock is a sandboxing mechanism that enables processes to restrict 
        themselves (and their future children) by gradually enforcing
diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index 7bbd2f413b3e..afa44baaa83a 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,4 +1,4 @@
  obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o

  landlock-y := setup.o syscalls.o object.o ruleset.o \
-    cred.o ptrace.o fs.o
+    cred.o ptrace.o fs.o net.o
diff --git a/security/landlock/net.c b/security/landlock/net.c
new file mode 100644
index 000000000000..7fbb857c39e2
--- /dev/null
+++ b/security/landlock/net.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0
Others files are tagged with GPL-2.0-only (which means that GPL-3 is excluded). 


 Ok. It's being refactored.



+/*
+ * Landlock LSM - Network management and hooks
+ *
+ * Copyright (C) 2022 Huawei Tech. Co., Ltd.
+ * Author: Konstantin Meskhidze <konstantin.meskhidze@xxxxxxxxxx>
I would like to avoid putting all authors in file headers (including from moved or copied code), but keep it small with only the copyright holders. All authors are part of the Git history, which is taken into account by tools such as ./scripts/get_maintainer.pl so you'll be CCed for relevant patches. 



+ *


nit: useless line


 Yep. thanks.

+ */
+
+#include <linux/in.h>
+#include <linux/net.h>
+#include <linux/socket.h>
+#include <net/ipv6.h>
+
+#include "cred.h"
+#include "limits.h"
+#include "net.h"
+
+int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
+                 u16 port, u32 access_rights)
+{
+    int err;
+
+    /* Transforms relative access rights to absolute ones. */
+    access_rights |= LANDLOCK_MASK_ACCESS_NET &
+             ~landlock_get_net_access_mask(ruleset, 0);
+
+    mutex_lock(&ruleset->lock);
+    err = landlock_insert_rule(ruleset, NULL, (uintptr_t)port, access_rights,
Type casting should not be required, but you can add this instead just before the landlock_insert_rule() call: 
BUILD_BUG_ON(sizeof(port) > sizeof(uintptr_t));


 Ok. Thanks.



+                   LANDLOCK_RULE_NET_SERVICE);
+    mutex_unlock(&ruleset->lock);
+
+    return err;
+}
+
+static int check_socket_access(const struct landlock_ruleset *const domain, 
+                   u16 port, u32 access_request)
+{
+    bool allowed = false;
+    u64 layer_mask;
+    size_t i;
+
+    /* Make sure all layers can be checked. */


nit: Make*s* sure…


 Sorry for typos. I will fix it.



+    BUILD_BUG_ON(BITS_PER_TYPE(layer_mask) < LANDLOCK_MAX_NUM_LAYERS);
+
+    if (WARN_ON_ONCE(!domain))
+        return 0;
+    if (WARN_ON_ONCE(domain->num_layers < 1))
+        return -EACCES;
+
+    /*
+     * Saves all layers handling a subset of requested
+     * socket access rules.
+     */
+    layer_mask = 0;
+    for (i = 0; i < domain->num_layers; i++) {
+        if (landlock_get_net_access_mask(domain, i) & access_request)
+            layer_mask |= BIT_ULL(i);
+    }
+    /* An access request not handled by the domain is allowed. */
+    if (layer_mask == 0)
+        return 0;
+
+    /*
+     * We need to walk through all the hierarchy to not miss any relevant 
+     * restriction.
+     */
+    layer_mask = landlock_unmask_layers(domain, NULL, port,
+                        access_request, layer_mask,
+                        LANDLOCK_RULE_NET_SERVICE);
+    if (layer_mask == 0)
+        allowed = true;
+
+    return allowed ? 0 : -EACCES;
+}
+
+static int hook_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen) 
+{
+#if IS_ENABLED(CONFIG_INET)
+    short socket_type;
+    struct sockaddr_in *sockaddr;
+    struct sockaddr_in6 *sockaddr_ip6;
+    u16 port;
+    const struct landlock_ruleset *const dom = landlock_get_current_domain(); 
+
+    if (!dom)
+        return 0;
+
+    /* Check if the hook is AF_INET* socket's action */
+    if ((address->sa_family != AF_INET) && (address->sa_family != AF_INET6))
You also need to add CONFIG_IPV6 in tools/testing/selftest/landlock/config and do similar IPv4 and IPv6 tests. I think it would be easier with variant tests (see FIXTURE_VARIANT in ptrace_test.c) and appropriate socket helpers.
Using such test variants will also help for the UDP support. Please try to make it easy to add (some) UDP tests with a new alternative when it will be available to make your tests reusable. 


  Ok. I got it.



+        return 0;
+
+    socket_type = sock->type;
+    /* Check if it's a TCP socket */
+    if (socket_type != SOCK_STREAM)
+        return 0;
+
+    /* Get port value in host byte order */
+    switch (address->sa_family) {
+    case AF_INET:
+        sockaddr = (struct sockaddr_in *)address;
+        port = ntohs(sockaddr->sin_port);
+        break;
+    case AF_INET6:
+        sockaddr_ip6 = (struct sockaddr_in6 *)address;
+        port = ntohs(sockaddr_ip6->sin6_port);
+        break;
+    }
+
+    return check_socket_access(dom, port, LANDLOCK_ACCESS_NET_BIND_TCP);
+#else
+    return 0;
+#endif
+}
+
+static int hook_socket_connect(struct socket *sock, struct sockaddr *address, int addrlen) 
+{
+#if IS_ENABLED(CONFIG_INET)
+    short socket_type;
+    struct sockaddr_in *sockaddr;
+    struct sockaddr_in6 *sockaddr_ip6;
+    u16 port;
+    const struct landlock_ruleset *const dom = landlock_get_current_domain(); 
+
+    if (!dom)
+        return 0;
+
+    /* Check if the hook is AF_INET* socket's action */
+    if ((address->sa_family != AF_INET) && (address->sa_family != AF_INET6)) { 
+        /* Check if the socket_connect() hook has AF_UNSPEC flag*/
+        if (address->sa_family == AF_UNSPEC) {
+            u16 i;
+            /*
+             * If just in a layer a mask supports connect access,
+             * the socket_connect() hook with AF_UNSPEC family flag
+             * must be banned. This prevents from disconnecting already
+             * connected sockets.
+             */
+            for (i = 0; i < dom->num_layers; i++) {
+                if (landlock_get_net_access_mask(dom, i) &
+                            LANDLOCK_ACCESS_NET_CONNECT_TCP)
+                    return -EACCES;
+            }
+        }
+        return 0;
+    }
+
+    socket_type = sock->type;
+    /* Check if it's a TCP socket */
+    if (socket_type != SOCK_STREAM)
+        return 0;
+
+    /* Get port value in host byte order */
+    switch (address->sa_family) {
+    case AF_INET:
+        sockaddr = (struct sockaddr_in *)address;
+        port = ntohs(sockaddr->sin_port);
+        break;
+    case AF_INET6:
+        sockaddr_ip6 = (struct sockaddr_in6 *)address;
+        port = ntohs(sockaddr_ip6->sin6_port);
+        break;
+    }
+
+    return check_socket_access(dom, port, LANDLOCK_ACCESS_NET_CONNECT_TCP); 
+#else
+    return 0;
+#endif
+}
+
+static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = { 
+    LSM_HOOK_INIT(socket_bind, hook_socket_bind),
+    LSM_HOOK_INIT(socket_connect, hook_socket_connect),
+};
+
+__init void landlock_add_net_hooks(void)
+{
+    security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
+            LANDLOCK_NAME);
+}
diff --git a/security/landlock/net.h b/security/landlock/net.h
new file mode 100644
index 000000000000..345bdc1dc84f
--- /dev/null
+++ b/security/landlock/net.h
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Landlock LSM - Network management and hooks
+ *
+ * Copyright (C) 2022 Huawei Tech. Co., Ltd.
+ * Author: Konstantin Meskhidze <konstantin.meskhidze@xxxxxxxxxx>
+ *
+ */
+
+#ifndef _SECURITY_LANDLOCK_NET_H
+#define _SECURITY_LANDLOCK_NET_H
+
+#include "common.h"
+#include "ruleset.h"
+#include "setup.h"
+
+__init void landlock_add_net_hooks(void);
+
+int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
+                 u16 port, u32 access_hierarchy);
+
+#endif /* _SECURITY_LANDLOCK_NET_H */
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index abf3e09a65cd..74e9d3d26bd6 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -193,6 +193,12 @@ void landlock_set_fs_access_mask(struct landlock_ruleset *ruleset,
  u32 landlock_get_fs_access_mask(const struct landlock_ruleset *ruleset, u16 mask_level); 

+void landlock_set_net_access_mask(struct landlock_ruleset *ruleset,
+                  const struct landlock_access_mask *access_mask_set,
+                  u16 mask_level);
+
+u32 landlock_get_net_access_mask(const struct landlock_ruleset *ruleset, u16 mask_level); 

These can be made static inline here.


 Already done!



+
  u64 landlock_unmask_layers(const struct landlock_ruleset *const domain,
                 const struct landlock_object *object_ptr,
                 const u16 port, const u32 access_request,
diff --git a/security/landlock/setup.c b/security/landlock/setup.c
index f8e8e980454c..8059dc0b47d3 100644
--- a/security/landlock/setup.c
+++ b/security/landlock/setup.c
@@ -14,6 +14,7 @@
  #include "fs.h"
  #include "ptrace.h"
  #include "setup.h"
+#include "net.h"

  bool landlock_initialized __lsm_ro_after_init = false;

@@ -28,6 +29,7 @@ static int __init landlock_init(void)
      landlock_add_cred_hooks();
      landlock_add_ptrace_hooks();
      landlock_add_fs_hooks();
+    landlock_add_net_hooks();
      landlock_initialized = true;
      pr_info("Up and running.\n");
      return 0;
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index b91455a19356..2d45ea94e6d2 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -29,6 +29,7 @@
  #include "cred.h"
  #include "fs.h"
  #include "limits.h"
+#include "net.h"
  #include "ruleset.h"
  #include "setup.h"

@@ -73,7 +74,8 @@ static void build_check_abi(void)
  {
      struct landlock_ruleset_attr ruleset_attr;
      struct landlock_path_beneath_attr path_beneath_attr;
-    size_t ruleset_size, path_beneath_size;
+    struct landlock_net_service_attr net_service_attr;
+    size_t ruleset_size, path_beneath_size, net_service_size;

      /*
       * For each user space ABI structures, first checks that there is no 
@@ -89,6 +91,11 @@ static void build_check_abi(void)
      path_beneath_size += sizeof(path_beneath_attr.parent_fd);
      BUILD_BUG_ON(sizeof(path_beneath_attr) != path_beneath_size);
      BUILD_BUG_ON(sizeof(path_beneath_attr) != 12);
+
+    net_service_size = sizeof(net_service_attr.allowed_access);
+    net_service_size += sizeof(net_service_attr.port);
+    BUILD_BUG_ON(sizeof(net_service_attr) != net_service_size);
+    BUILD_BUG_ON(sizeof(net_service_attr) != 10);
  }

  /* Ruleset handling */
@@ -311,7 +318,6 @@ static int add_rule_path_beneath(const int ruleset_fd, const void *const rule_at 
       * Checks that allowed_access matches the @ruleset constraints
       * (ruleset->access_masks[0] is automatically upgraded to 64-bits).
       */
-
      if ((path_beneath_attr.allowed_access | landlock_get_fs_access_mask(ruleset, 0)) != 
                          landlock_get_fs_access_mask(ruleset, 0)) {
          err = -EINVAL;
@@ -333,6 +339,50 @@ static int add_rule_path_beneath(const int ruleset_fd, const void *const rule_at 
      return err;
  }
+static int add_rule_net_service(const int ruleset_fd, const void *const rule_attr) 



Here is a patch for other changes:

* Clean up and factor out landlock_add_rule(2).
* Only build network part if CONFIG_INET is set and make
   landlock_add_rule(2) returns -EAFNOSUPPORT otherwise.
* Use CONFIG_IPV6 ifdef.
* Cosmetic fixes.
* Add TODOs.
---
  security/landlock/Makefile   |   4 +-
  security/landlock/net.c      | 146 ++++++++++++++++++-----------------
  security/landlock/net.h      |   9 +++
  security/landlock/syscalls.c |  90 ++++++++++-----------
  4 files changed, 126 insertions(+), 123 deletions(-)

diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index afa44baaa83a..c2e116f2a299 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,4 +1,6 @@
  obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o

  landlock-y := setup.o syscalls.o object.o ruleset.o \
-    cred.o ptrace.o fs.o net.o
+    cred.o ptrace.o fs.o
+
+landlock-$(CONFIG_INET) += net.o
diff --git a/security/landlock/net.c b/security/landlock/net.c
index 7fbb857c39e2..23dd842a4628 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -26,8 +26,8 @@ int landlock_append_net_rule(struct landlock_ruleset *const ruleset, 
               ~landlock_get_net_access_mask(ruleset, 0);

      mutex_lock(&ruleset->lock);
-    err = landlock_insert_rule(ruleset, NULL, (uintptr_t)port, access_rights, 
-                   LANDLOCK_RULE_NET_SERVICE);
+    err = landlock_insert_rule(ruleset, NULL, (uintptr_t)port,
+            access_rights, LANDLOCK_RULE_NET_SERVICE);
      mutex_unlock(&ruleset->lock);

      return err;
@@ -65,107 +65,109 @@ static int check_socket_access(const struct landlock_ruleset *const domain, 
       * We need to walk through all the hierarchy to not miss any relevant
       * restriction.
       */
-    layer_mask = landlock_unmask_layers(domain, NULL, port,
-                        access_request, layer_mask,
-                        LANDLOCK_RULE_NET_SERVICE);
+    layer_mask = landlock_unmask_layers(domain, NULL, port, access_request, 
+            layer_mask, LANDLOCK_RULE_NET_SERVICE);
      if (layer_mask == 0)
          allowed = true;

      return allowed ? 0 : -EACCES;
  }
-static int hook_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen) 
+static u16 get_port(const struct sockaddr *const address)
  {
-#if IS_ENABLED(CONFIG_INET)
-    short socket_type;
-    struct sockaddr_in *sockaddr;
-    struct sockaddr_in6 *sockaddr_ip6;
-    u16 port;
-    const struct landlock_ruleset *const dom = landlock_get_current_domain(); 
+    /* Gets port value in host byte order. */
+    switch (address->sa_family) {
+    case AF_INET:
+        const struct sockaddr_in *const sockaddr =
+            (struct sockaddr_in *)address;

-    if (!dom)
-        return 0;
+        return ntohs(sockaddr->sin_port);
+#if IS_ENABLED(CONFIG_IPV6)
+    case AF_INET6:
+        struct sockaddr_in6 *sockaddr_ip6 =
+            (struct sockaddr_in6 *)address;

-    /* Check if the hook is AF_INET* socket's action */
-    if ((address->sa_family != AF_INET) && (address->sa_family != AF_INET6)) 
+        return ntohs(sockaddr_ip6->sin6_port);
+#endif
+    /*
+     * TODO: What about AF_UNSPEC and other values? Add tests for these
+     * cases.
+     */
+    }
+    WARN_ON_ONCE(1);
+    return 0;
+}
+
+static int hook_socket_bind(struct socket *sock, struct sockaddr *address,
+        int addrlen)
+{
+    const struct landlock_ruleset *const dom =
+        landlock_get_current_domain();
+
+    if (!dom)
          return 0;

-    socket_type = sock->type;
-    /* Check if it's a TCP socket */
-    if (socket_type != SOCK_STREAM)
+    /* Checks if it is a TCP socket. */
+    if (sock->type != SOCK_STREAM)
          return 0;

-    /* Get port value in host byte order */
      switch (address->sa_family) {
      case AF_INET:
-        sockaddr = (struct sockaddr_in *)address;
-        port = ntohs(sockaddr->sin_port);
-        break;
+#if IS_ENABLED(CONFIG_IPV6)
      case AF_INET6:
-        sockaddr_ip6 = (struct sockaddr_in6 *)address;
-        port = ntohs(sockaddr_ip6->sin6_port);
-        break;
-    }
-
-    return check_socket_access(dom, port, LANDLOCK_ACCESS_NET_BIND_TCP);
-#else
-    return 0;
  #endif
+        /* TODO: Add tests with different source and destination ports. */
+        return check_socket_access(dom, get_port(address),
+                LANDLOCK_ACCESS_NET_BIND_TCP);
+    default:
+        /*
+         * TODO: What about AF_UNSPEC and other values? Add tests for
+         * these cases.
+         */
+        return 0;
+    }
  }
-static int hook_socket_connect(struct socket *sock, struct sockaddr *address, int addrlen) +static int hook_socket_connect(struct socket *sock, struct sockaddr *address, 
+        int addrlen)
  {
-#if IS_ENABLED(CONFIG_INET)
-    short socket_type;
-    struct sockaddr_in *sockaddr;
-    struct sockaddr_in6 *sockaddr_ip6;
-    u16 port;
-    const struct landlock_ruleset *const dom = landlock_get_current_domain(); 
+    const struct landlock_ruleset *const dom =
+        landlock_get_current_domain();

      if (!dom)
          return 0;

-    /* Check if the hook is AF_INET* socket's action */
-    if ((address->sa_family != AF_INET) && (address->sa_family != AF_INET6)) { 
-        /* Check if the socket_connect() hook has AF_UNSPEC flag*/
-        if (address->sa_family == AF_UNSPEC) {
-            u16 i;
-            /*
-             * If just in a layer a mask supports connect access,
-             * the socket_connect() hook with AF_UNSPEC family flag
-             * must be banned. This prevents from disconnecting already
-             * connected sockets.
-             */
-            for (i = 0; i < dom->num_layers; i++) {
-                if (landlock_get_net_access_mask(dom, i) &
-                            LANDLOCK_ACCESS_NET_CONNECT_TCP)
-                    return -EACCES;
-            }
-        }
-        return 0;
-    }
-
-    socket_type = sock->type;
-    /* Check if it's a TCP socket */
-    if (socket_type != SOCK_STREAM)
+    /* Checks if it is a TCP socket. */
+    if (sock->type != SOCK_STREAM)
          return 0;

-    /* Get port value in host byte order */
+    /* Check if the hook is AF_INET* socket's action */
      switch (address->sa_family) {
      case AF_INET:
-        sockaddr = (struct sockaddr_in *)address;
-        port = ntohs(sockaddr->sin_port);
-        break;
+#if IS_ENABLED(CONFIG_IPV6)
      case AF_INET6:
-        sockaddr_ip6 = (struct sockaddr_in6 *)address;
-        port = ntohs(sockaddr_ip6->sin6_port);
-        break;
+#endif
+        /* TODO: Add tests with different source and destination ports. */
+        return check_socket_access(dom, get_port(address),
+                LANDLOCK_ACCESS_NET_CONNECT_TCP);
+    case AF_UNSPEC:
+        u16 i;
+
+        /*
+         * If just in a layer a mask supports connect access,
+         * the socket_connect() hook with AF_UNSPEC family flag
+         * must be banned. This prevents from disconnecting already
+         * connected sockets.
+         */
+        /* TODO: Add tests for this case, with UDP and TCP. */
+        for (i = 0; i < dom->num_layers; i++) {
+            if (landlock_get_net_access_mask(dom, i) &
+                    LANDLOCK_ACCESS_NET_CONNECT_TCP)
+                return -EACCES;
+        }
      }
-
-    return check_socket_access(dom, port, LANDLOCK_ACCESS_NET_CONNECT_TCP); 
-#else
      return 0;
-#endif
+
  }

  static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
diff --git a/security/landlock/net.h b/security/landlock/net.h
index 345bdc1dc84f..4cf32c89d352 100644
--- a/security/landlock/net.h
+++ b/security/landlock/net.h
@@ -14,9 +14,18 @@
  #include "ruleset.h"
  #include "setup.h"

+#if IS_ENABLED(CONFIG_INET)
+
  __init void landlock_add_net_hooks(void);

  int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
                   u16 port, u32 access_hierarchy);

+#else /* IS_ENABLED(CONFIG_INET) */
+
+static inline void landlock_add_net_hooks(void)
+{}
+
+#endif /* IS_ENABLED(CONFIG_INET) */
+
  #endif /* _SECURITY_LANDLOCK_NET_H */
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 2d45ea94e6d2..b8dcd981872e 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -288,12 +288,13 @@ static int get_path_from_fd(const s32 fd, struct path *const path) 
      return err;
  }
-static int add_rule_path_beneath(const int ruleset_fd, const void *const rule_attr) 
+static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
+        const void *const rule_attr)
  {
      struct landlock_path_beneath_attr path_beneath_attr;
      struct path path;
-    struct landlock_ruleset *ruleset;
      int res, err;
+    u32 mask;

      /* Copies raw user space buffer, only one type for now. */
      res = copy_from_user(&path_beneath_attr, rule_attr,
@@ -301,49 +302,39 @@ static int add_rule_path_beneath(const int ruleset_fd, const void *const rule_at 
      if (res)
          return -EFAULT;

-    /* Gets and checks the ruleset. */
-    ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE);
-    if (IS_ERR(ruleset))
-        return PTR_ERR(ruleset);
-
      /*
       * Informs about useless rule: empty allowed_access (i.e. deny rules)
       * are ignored in path walks.
       */
-    if (!path_beneath_attr.allowed_access) {
-        err = -ENOMSG;
-        goto out_put_ruleset;
-    }
+    if (!path_beneath_attr.allowed_access)
+        return -ENOMSG;
      /*
       * Checks that allowed_access matches the @ruleset constraints
       * (ruleset->access_masks[0] is automatically upgraded to 64-bits).
       */
-    if ((path_beneath_attr.allowed_access | landlock_get_fs_access_mask(ruleset, 0)) != 
-                        landlock_get_fs_access_mask(ruleset, 0)) {
-        err = -EINVAL;
-        goto out_put_ruleset;
-    }
+    mask = landlock_get_fs_access_mask(ruleset, 0);
+    if ((path_beneath_attr.allowed_access | mask) != mask)
+        return -EINVAL;

      /* Gets and checks the new rule. */
      err = get_path_from_fd(path_beneath_attr.parent_fd, &path);
      if (err)
-        goto out_put_ruleset;
+        return err;

      /* Imports the new rule. */
      err = landlock_append_fs_rule(ruleset, &path,
              path_beneath_attr.allowed_access);
      path_put(&path);
-
-out_put_ruleset:
-    landlock_put_ruleset(ruleset);
      return err;
  }
-static int add_rule_net_service(const int ruleset_fd, const void *const rule_attr) 
+static int add_rule_net_service(struct landlock_ruleset *const ruleset,
+        const void *const rule_attr)
  {
-    struct landlock_net_service_attr  net_service_attr;
-    struct landlock_ruleset *ruleset;
-    int res, err;
+#if IS_ENABLED(CONFIG_INET)
+    struct landlock_net_service_attr net_service_attr;
+    int res;
+    u32 mask;

      /* Copies raw user space buffer, only one type for now. */
      res = copy_from_user(&net_service_attr, rule_attr,
@@ -351,36 +342,28 @@ static int add_rule_net_service(const int ruleset_fd, const void *const rule_att 
      if (res)
          return -EFAULT;

-    /* Gets and checks the ruleset. */
-    ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE);
-    if (IS_ERR(ruleset))
-        return PTR_ERR(ruleset);
-
      /*
       * Informs about useless rule: empty allowed_access (i.e. deny rules)
       * are ignored by network actions
       */
-    if (!net_service_attr.allowed_access) {
-        err = -ENOMSG;
-        goto out_put_ruleset;
-    }
+    if (!net_service_attr.allowed_access)
+        return -ENOMSG;
      /*
       * Checks that allowed_access matches the @ruleset constraints
       * (ruleset->access_masks[0] is automatically upgraded to 64-bits).
       */
-    if ((net_service_attr.allowed_access | landlock_get_net_access_mask(ruleset, 0)) != 
-                           landlock_get_net_access_mask(ruleset, 0)) {
-        err = -EINVAL;
-        goto out_put_ruleset;
-    }
+    mask = landlock_get_net_access_mask(ruleset, 0);
+    if ((net_service_attr.allowed_access | mask) != mask)
+        return -EINVAL;

      /* Imports the new rule. */
-    err = landlock_append_net_rule(ruleset, net_service_attr.port,
+    return landlock_append_net_rule(ruleset, net_service_attr.port,
                         net_service_attr.allowed_access);

-out_put_ruleset:
-    landlock_put_ruleset(ruleset);
-    return err;
+#else /* IS_ENABLED(CONFIG_INET) */
+
+    return -EAFNOSUPPORT;
+#endif /* IS_ENABLED(CONFIG_INET) */
  }

  /**
@@ -388,8 +371,8 @@ static int add_rule_net_service(const int ruleset_fd, const void *const rule_att 
   *
  * @ruleset_fd: File descriptor tied to the ruleset that should be extended 
   *        with the new rule.
- * @rule_type: Identify the structure type pointed to by @rule_attr (only
- *             LANDLOCK_RULE_PATH_BENEATH for now).
+ * @rule_type: Identify the structure type pointed to by @rule_attr:
+ *             LANDLOCK_RULE_PATH_BENEATH or LANDLOCK_RULE_NET_SERVICE.
   * @rule_attr: Pointer to a rule (only of type &struct
   *             landlock_path_beneath_attr for now).
   * @flags: Must be 0.
@@ -400,6 +383,8 @@ static int add_rule_net_service(const int ruleset_fd, const void *const rule_att 
   * Possible returned errors are:
   *
  * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time; + * - EAFNOSUPPORT: @rule_type is LANDLOCK_RULE_NET_SERVICE but TCP/IP is not 
+ *   supported by the running kernel;
   * - EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
  *   &landlock_path_beneath_attr.allowed_access is not a subset of the rule's 
   *   accesses);
@@ -416,6 +401,7 @@ SYSCALL_DEFINE4(landlock_add_rule,
          const int, ruleset_fd, const enum landlock_rule_type, rule_type,
          const void __user *const, rule_attr, const __u32, flags)
  {
+    struct landlock_ruleset *ruleset;
      int err;

      if (!landlock_initialized)
@@ -425,20 +411,24 @@ SYSCALL_DEFINE4(landlock_add_rule,
      if (flags)
          return -EINVAL;

+    /* Gets and checks the ruleset. */
+    ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE);
+    if (IS_ERR(ruleset))
+        return PTR_ERR(ruleset);
+
      switch (rule_type) {
      case LANDLOCK_RULE_PATH_BENEATH:
-        err = add_rule_path_beneath(ruleset_fd, rule_attr);
+        err = add_rule_path_beneath(ruleset, rule_attr);
          break;
      case LANDLOCK_RULE_NET_SERVICE:
-#if IS_ENABLED(CONFIG_INET)
-        err = add_rule_net_service(ruleset_fd, rule_attr);
-#else
-        err = -EOPNOTSUPP;
-#endif
+        err = add_rule_net_service(ruleset, rule_attr);
          break;
      default:
          err = -EINVAL;
+        break;
      }
+
+    landlock_put_ruleset(ruleset);
      return err;
  }


  Thanks for this patch. Im testing it.
[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux