Re: [PATCH v30 03/28] LSM: Add the lsmblob data structure.

[Paul Moore]

On Tue, Nov 23, 2021 at 8:47 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote:
>
> When more than one security module is exporting data to
> audit and networking sub-systems a single 32 bit integer
> is no longer sufficient to represent the data. Add a
> structure to be used instead.
>
> The lsmblob structure is currently an array of
> u32 "secids". There is an entry for each of the
> security modules built into the system that would
> use secids if active. The system assigns the module
> a "slot" when it registers hooks. If modules are
> compiled in but not registered there will be unused
> slots.
>
> A new lsm_id structure, which contains the name
> of the LSM and its slot number, is created. There
> is an instance for each LSM, which assigns the name
> and passes it to the infrastructure to set the slot.
>
> The audit rules data is expanded to use an array of
> security module data rather than a single instance.
> A new structure audit_rules is defined to avoid the
> confusion which commonly accompanies the use of
> void ** parameters.
>
> Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
> ---
>  include/linux/audit.h        | 10 ++++-
>  include/linux/lsm_hooks.h    | 12 +++++-
>  include/linux/security.h     | 74 +++++++++++++++++++++++++++++---
>  kernel/auditfilter.c         | 24 ++++++-----
>  kernel/auditsc.c             | 17 +++-----
>  security/apparmor/lsm.c      |  7 ++-
>  security/bpf/hooks.c         | 12 +++++-
>  security/commoncap.c         |  7 ++-
>  security/landlock/cred.c     |  2 +-
>  security/landlock/fs.c       |  2 +-
>  security/landlock/ptrace.c   |  2 +-
>  security/landlock/setup.c    |  5 +++
>  security/landlock/setup.h    |  1 +
>  security/loadpin/loadpin.c   |  8 +++-
>  security/lockdown/lockdown.c |  7 ++-
>  security/safesetid/lsm.c     |  8 +++-
>  security/security.c          | 82 ++++++++++++++++++++++++++++++------
>  security/selinux/hooks.c     |  8 +++-
>  security/smack/smack_lsm.c   |  7 ++-
>  security/tomoyo/tomoyo.c     |  8 +++-
>  security/yama/yama_lsm.c     |  7 ++-
>  21 files changed, 254 insertions(+), 56 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index d06134ac6245..943584128399 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -11,6 +11,7 @@
>
>  #include <linux/sched.h>
>  #include <linux/ptrace.h>
> +#include <linux/security.h>
>  #include <linux/audit_arch.h>
>  #include <uapi/linux/audit.h>
>  #include <uapi/linux/netfilter/nf_tables.h>
> @@ -59,6 +60,10 @@ struct audit_krule {
>  /* Flag to indicate legacy AUDIT_LOGINUID unset usage */
>  #define AUDIT_LOGINUID_LEGACY          0x1
>
> +struct audit_rules {
> +       void    *rule[LSMBLOB_ENTRIES];
> +};

It would be nice to call this struct "audit_lsm_rules" to help
distinguish it from general audit filter rules.

>  struct audit_field {
>         u32                             type;
>         union {
> @@ -66,8 +71,9 @@ struct audit_field {
>                 kuid_t                  uid;
>                 kgid_t                  gid;
>                 struct {
> -                       char            *lsm_str;
> -                       void            *lsm_rule;
> +                       bool                    lsm_isset;
> +                       char                    *lsm_str;
> +                       struct audit_rules      lsm_rules;
>                 };

Is lsm_isset strictly necessary?  Unless I missed something it seems
like a NULL check on lsm_str would serve the same purpose.

--
paul moore
www.paul-moore.com