Re: [PATCH v2] selinux: log anon inode class name

[Paul Moore <paul@paul-moore.com>]

On Tue, Mar 8, 2022 at 12:09 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Log the anonymous inode class name in the security hook
> inode_init_security_anon.  This name is the key for name based type
> transitions on the anon_inode security class on creation.  Example:
>
>     type=AVC msg=audit(02/16/22 22:02:50.585:216) : avc:  granted \
>         { create } for  pid=2136 comm=mariadbd anonclass="[io_uring]" \
>         scontext=system_u:system_r:mysqld_t:s0 \
>         tcontext=system_u:system_r:mysqld_iouring_t:s0 tclass=anon_inode
>
> Add a new LSM audit data type holding the inode and the class name.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> ---
> v2:
>   - drop dev= and name= output for anonymous inodes, and hence simplify
>     the common_audit_data union member.
>   - drop WARN_ON() on empty name passed to inode_init_security_anon hook
> ---
>  include/linux/lsm_audit.h | 2 ++
>  security/lsm_audit.c      | 4 ++++
>  security/selinux/hooks.c  | 4 ++--
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
> index 17d02eda9538..97a8b21eb033 100644
> --- a/include/linux/lsm_audit.h
> +++ b/include/linux/lsm_audit.h
> @@ -76,6 +76,7 @@ struct common_audit_data {
>  #define LSM_AUDIT_DATA_IBENDPORT 14
>  #define LSM_AUDIT_DATA_LOCKDOWN 15
>  #define LSM_AUDIT_DATA_NOTIFICATION 16
> +#define LSM_AUDIT_DATA_ANONINODE       17
>         union   {
>                 struct path path;
>                 struct dentry *dentry;
> @@ -96,6 +97,7 @@ struct common_audit_data {
>                 struct lsm_ibpkey_audit *ibpkey;
>                 struct lsm_ibendport_audit *ibendport;
>                 int reason;
> +               const char *anonclass;
>         } u;
>         /* this union contains LSM specific data */
>         union {
> diff --git a/security/lsm_audit.c b/security/lsm_audit.c
> index 1897cbf6fc69..981f6a4e4590 100644
> --- a/security/lsm_audit.c
> +++ b/security/lsm_audit.c
> @@ -433,6 +433,10 @@ static void dump_common_audit_data(struct audit_buffer *ab,
>                 audit_log_format(ab, " lockdown_reason=\"%s\"",
>                                  lockdown_reasons[a->u.reason]);
>                 break;
> +       case LSM_AUDIT_DATA_ANONINODE:
> +               audit_log_format(ab, " anonclass=");
> +               audit_log_untrustedstring(ab, a->u.anonclass);

My apologies, I didn't notice this in the previous patch ... I don't
think we need to log this as an untrusted string as the string value
is coming from the kernel, not userspace, so we could rewrite the
above as the following:

  audit_log_format(ab, " anonclass=%s", a->u.anonclass);

... if you are okay with that, I can make the change when I merge the
patch or you can submit another revision, let me know which you would
prefer.

The rest of the patch looks good, thanks!

> +               break;
>         } /* switch (a->type) */
>  }
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index b12e14b2797b..49c0abfd2f6a 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2965,8 +2965,8 @@ static int selinux_inode_init_security_anon(struct inode *inode,
>          * allowed to actually create this type of anonymous inode.
>          */
>
> -       ad.type = LSM_AUDIT_DATA_INODE;
> -       ad.u.inode = inode;
> +       ad.type = LSM_AUDIT_DATA_ANONINODE;
> +       ad.u.anonclass = name ? (const char *)name->name : "?";
>
>         return avc_has_perm(&selinux_state,
>                             tsec->sid,
> --
> 2.35.1

-- 
paul-moore.com