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

[THOBY Simon]

On 8/31/21 1:29 AM, J Freyensee wrote:
> On 8/21/21 2:47 AM, Igor Zhbanov wrote:
>> 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 <izh1979@xxxxxxxxx>
>> ---
>>   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@xxxxxxxxxx/

> 
> <big snip>
> .
> .
> .
> 
>> +
>> +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