Re: [PATCH v5 1/1] NAX LSM: Add initial support

On 8/31/21 1:29 AM, J Freyensee wrote:

On 8/21/21 2:47 AM, Igor Zhbanov wrote:

quoted

Add initial support for NAX (No Anonymous Execution), which is a Linux
Security Module that extends DAC by making impossible to make anonymous
and modified pages executable for privileged processes.

Intercepts anonymous executable pages created with mmap() and mprotect()
system calls.

Log violations (in non-quiet mode) and block the action or kill the
offending process, depending on the enabled settings.

See Documentation/admin-guide/LSM/NAX.rst.

Signed-off-by: Igor Zhbanov <redacted>
---
  Documentation/admin-guide/LSM/NAX.rst         |  72 +++
  Documentation/admin-guide/LSM/index.rst       |   1 +
  .../admin-guide/kernel-parameters.rst         |   1 +
  .../admin-guide/kernel-parameters.txt         |  32 ++
  security/Kconfig                              |  11 +-
  security/Makefile                             |   2 +
  security/nax/Kconfig                          | 113 +++++
  security/nax/Makefile                         |   4 +
  security/nax/nax-lsm.c                        | 472 ++++++++++++++++++
  9 files changed, 703 insertions(+), 5 deletions(-)
  create mode 100644 Documentation/admin-guide/LSM/NAX.rst
  create mode 100644 security/nax/Kconfig
  create mode 100644 security/nax/Makefile
  create mode 100644 security/nax/nax-lsm.c

diff --git a/Documentation/admin-guide/LSM/NAX.rst b/Documentation/admin-guide/LSM/NAX.rst
new file mode 100644
index 000000000000..da54b3be4cda
--- /dev/null
+++ b/Documentation/admin-guide/LSM/NAX.rst

@@ -0,0 +1,72 @@
+=======
+NAX LSM
+=======
+
+:Author: Igor Zhbanov
+
+NAX (No Anonymous Execution) is a Linux Security Module that extends DAC
+by making impossible to make anonymous and modified pages executable for
+processes. The module intercepts anonymous executable pages created with
+mmap() and mprotect() system calls.
+
+To select it at boot time, add ``nax`` to ``security`` kernel command-line
+parameter.
+
+The following sysctl parameters are available:
+
+* ``kernel.nax.check_all``:
+ - 0: Check all processes.
+ - 1: Check only privileged processes. The privileged process is a process
+      for which any of the following is true:
+      - ``uid  == 0``
+      - ``euid == 0``
+      - ``suid == 0``
+      - ``cap_effective`` has any capability except for the ones allowed
+        in ``kernel.nax.allowed_caps``
+      - ``cap_permitted`` has any capability except for the ones allowed
+        in ``kernel.nax.allowed_caps``
+
+ Checking of uid/euid/suid is important because a process may call seteuid(0)
+ to gain privileges (if SECURE_NO_SETUID_FIXUP secure bit is not set).
+
+* ``kernel.nax.allowed_caps``:
+
+ Hexadecimal number representing the set of capabilities a non-root
+ process can possess without being considered "privileged" by NAX LSM.
+
+ For the meaning of the capabilities bits and their value, please check
+ ``include/uapi/linux/capability.h`` and ``capabilities(7)`` manual page.
+
+ For example, ``CAP_SYS_PTRACE`` has a number 19. Therefore, to add it to
+ allowed capabilities list, we need to set 19'th bit (2^19 or 1 << 19)
+ or 80000 in hexadecimal form. Capabilities can be bitwise ORed.
+
+* ``kernel.nax.mode``:
+
+ - 0: Only log errors (when enabled by ``kernel.nax.quiet``) (default mode)
+ - 1: Forbid unsafe pages mappings (and log when enabled)
+ - 2: Kill the violating process (and log when enabled)
+
+* ``kernel.nax.quiet``:
+
+ - 0: Log violations (default)
+ - 1: Be quiet
+
+* ``kernel.nax.locked``:
+
+ - 0: Changing of the module's sysctl parameters is allowed
+ - 1: Further changing of the module's sysctl parameters is forbidden
+
+ Setting this parameter to ``1`` after initial setup during the system boot
+ will prevent the module disabling at the later time.
+
+There are matching kernel command-line parameters (with the same values):
+
+- ``nax_allowed_caps``
+- ``nax_check_all``
+- ``nax_mode``
+- ``nax_quiet``
+- ``nax_locked``
+
+The ``nax_locked`` command-line parameter must be specified last to avoid
+premature setting locking.

Is it common to have these types of restrictions for kernel command-line
parameters, in this case, kernel command-line parameter ordering?  Seems
like that would be prone for a lot of avoidable troubleshooting issues
and unnecessary usage questions.

This point was discussed in the v1 of this patch (but we didn't really reach an agreement one way or another):
https://x-lore.kernel.org/all/b1f3650c-df42-c5d4-45c0-c77946759926@viveris.fr/

<big snip>
.
.
.

quoted

+
+static void __init
+nax_init_sysctl(void)
+{
+     if (!register_sysctl_paths(nax_sysctl_path, nax_sysctl_table))
+             panic("NAX: sysctl registration failed.\n");
+}
+
+#else /* !CONFIG_SYSCTL */
+
+static inline void
+nax_init_sysctl(void)
+{
+
+}
+
+#endif /* !CONFIG_SYSCTL */
+
+static int __init setup_allowed_caps(char *str)
+{
+     if (locked)
+             return 1;
+
+     /* Do not allow trailing garbage or excessive length */
+     if (strlen(str) > ALLOWED_CAPS_HEX_LEN) {

 a little nitpick, could strnlen() be used instead to define a max
length of the input 'str'?


Regards,
Jay

Have a nice day,
Simon