| Message ID | 20220308170928.58040-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] selinux: log anon inode class name | expand |
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
On Mon, Apr 4, 2022 at 4:18 PM Paul Moore <paul@paul-moore.com> wrote: > 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! Hi Christian, I just wanted to follow up on this as we are at -rc4 this week and if we want this to go during the next merge window this would need to be merged soon ...
On Wed, 27 Apr 2022 at 15:19, Paul Moore <paul@paul-moore.com> wrote: > > On Mon, Apr 4, 2022 at 4:18 PM Paul Moore <paul@paul-moore.com> wrote: > > 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. Feel free to adjust while merging, thanks. > > > > The rest of the patch looks good, thanks! > > Hi Christian, > > I just wanted to follow up on this as we are at -rc4 this week and if > we want this to go during the next merge window this would need to be > merged soon ... > > -- > paul-moore.com
On Mon, May 2, 2022 at 9:39 AM Christian Göttsche <cgzones@googlemail.com> wrote: > On Wed, 27 Apr 2022 at 15:19, Paul Moore <paul@paul-moore.com> wrote: > > On Mon, Apr 4, 2022 at 4:18 PM Paul Moore <paul@paul-moore.com> wrote: > > > 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. > > Feel free to adjust while merging, thanks. Adjusted and merged, thanks.
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); + 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,
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(-)