| Message ID | 20241022223711.3066190-1-tweek@google.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | selinux: add netlink nlmsg_type audit message | expand |
On Oct 22, 2024 "=?UTF-8?q?Thi=C3=A9baud=20Weksteen?=" <tweek@google.com> wrote: > > Add a new audit message type to capture nlmsg-related information. This > is similar to LSM_AUDIT_DATA_IOCTL_OP which was added for the other > SELinux extended permission (ioctl). > > Adding a new type is preferred to adding to the existing > lsm_network_audit structure which contains irrelevant information for > the netlink sockets (i.e., dport, sport). > > Signed-off-by: Thiébaud Weksteen <tweek@google.com> > --- > include/linux/lsm_audit.h | 2 ++ > security/lsm_audit.c | 3 +++ > security/selinux/hooks.c | 4 ++-- > 3 files changed, 7 insertions(+), 2 deletions(-) ... > diff --git a/security/lsm_audit.c b/security/lsm_audit.c > index 849e832719e2..b6544024f688 100644 > --- a/security/lsm_audit.c > +++ b/security/lsm_audit.c > @@ -425,6 +425,9 @@ static void dump_common_audit_data(struct audit_buffer *ab, > case LSM_AUDIT_DATA_ANONINODE: > audit_log_format(ab, " anonclass=%s", a->u.anonclass); > break; > + case LSM_AUDIT_DATA_NLMSGTYPE: > + audit_log_format(ab, " nlmsg_type=%hu", a->u.nlmsg_type); > + break; Based on the audit field dictionary, link below, it appears that netlink related fields follow the "nlnk-XXX" pattern, and while I don't recall any current users in the kernel, it seems like sticking with that pattern is probably a good idea. With that in mind, what do you think about changing "nlmsg_type" into "nlnk-msgtype"? https://github.com/linux-audit/audit-documentation/blob/main/specs/fields/field-dictionary.csv > } /* switch (a->type) */ > } > -- paul-moore.com
> On Oct 22, 2024 "=?UTF-8?q?Thi=C3=A9baud=20Weksteen?=" <tweek@google.com> wrote: > > > > Add a new audit message type to capture nlmsg-related information. This > > is similar to LSM_AUDIT_DATA_IOCTL_OP which was added for the other > > SELinux extended permission (ioctl). > > > > Adding a new type is preferred to adding to the existing > > lsm_network_audit structure which contains irrelevant information for > > the netlink sockets (i.e., dport, sport). > > > > Signed-off-by: Thiébaud Weksteen <tweek@google.com> > > --- > > include/linux/lsm_audit.h | 2 ++ > > security/lsm_audit.c | 3 +++ > > security/selinux/hooks.c | 4 ++-- > > 3 files changed, 7 insertions(+), 2 deletions(-) > > ... > > > diff --git a/security/lsm_audit.c b/security/lsm_audit.c > > index 849e832719e2..b6544024f688 100644 > > --- a/security/lsm_audit.c > > +++ b/security/lsm_audit.c > > @@ -425,6 +425,9 @@ static void dump_common_audit_data(struct audit_buffer *ab, > > case LSM_AUDIT_DATA_ANONINODE: > > audit_log_format(ab, " anonclass=%s", a->u.anonclass); > > break; > > + case LSM_AUDIT_DATA_NLMSGTYPE: > > + audit_log_format(ab, " nlmsg_type=%hu", a->u.nlmsg_type); > > + break; > > Based on the audit field dictionary, link below, it appears that netlink > related fields follow the "nlnk-XXX" pattern, and while I don't recall > any current users in the kernel, it seems like sticking with that pattern > is probably a good idea. With that in mind, what do you think about > changing "nlmsg_type" into "nlnk-msgtype"? > Thanks Paul, I wasn't aware of this list. I found one example of a netlink field in kernel/audit.c (function audit_log_multicast), which was added in commit 9d2161bed4e39. The field is 'nl-mcgrp'. The name was changed from nlnk-grp between v4 and v5 of the patch, but I can't seem to find the reasoning. Do you know if 'nlnk-fam' and 'nlnk-pid' were deprecated/removed at some point? I don't mind either way. If you think 'nlnk-msgtype' (or 'nl-msgtype') is more consistent with the other audit fields, I'm happy to send an updated version.
diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h index 97a8b21eb033..69d2b7bc00ed 100644 --- a/include/linux/lsm_audit.h +++ b/include/linux/lsm_audit.h @@ -77,6 +77,7 @@ struct common_audit_data { #define LSM_AUDIT_DATA_LOCKDOWN 15 #define LSM_AUDIT_DATA_NOTIFICATION 16 #define LSM_AUDIT_DATA_ANONINODE 17 +#define LSM_AUDIT_DATA_NLMSGTYPE 18 union { struct path path; struct dentry *dentry; @@ -98,6 +99,7 @@ struct common_audit_data { struct lsm_ibendport_audit *ibendport; int reason; const char *anonclass; + u16 nlmsg_type; } u; /* this union contains LSM specific data */ union { diff --git a/security/lsm_audit.c b/security/lsm_audit.c index 849e832719e2..b6544024f688 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -425,6 +425,9 @@ static void dump_common_audit_data(struct audit_buffer *ab, case LSM_AUDIT_DATA_ANONINODE: audit_log_format(ab, " anonclass=%s", a->u.anonclass); break; + case LSM_AUDIT_DATA_NLMSGTYPE: + audit_log_format(ab, " nlmsg_type=%hu", a->u.nlmsg_type); + break; } /* switch (a->type) */ } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ad3abd48eed1..40afeba0b7de 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5932,14 +5932,14 @@ static int nlmsg_sock_has_extended_perms(struct sock *sk, u32 perms, u16 nlmsg_t { struct sk_security_struct *sksec = sk->sk_security; struct common_audit_data ad; - struct lsm_network_audit net; u8 driver; u8 xperm; if (sock_skip_has_perm(sksec->sid)) return 0; - ad_net_init_from_sk(&ad, &net, sk); + ad.type = LSM_AUDIT_DATA_NLMSGTYPE; + ad.u.nlmsg_type = nlmsg_type; driver = nlmsg_type >> 8; xperm = nlmsg_type & 0xff;
Add a new audit message type to capture nlmsg-related information. This is similar to LSM_AUDIT_DATA_IOCTL_OP which was added for the other SELinux extended permission (ioctl). Adding a new type is preferred to adding to the existing lsm_network_audit structure which contains irrelevant information for the netlink sockets (i.e., dport, sport). Signed-off-by: Thiébaud Weksteen <tweek@google.com> --- include/linux/lsm_audit.h | 2 ++ security/lsm_audit.c | 3 +++ security/selinux/hooks.c | 4 ++-- 3 files changed, 7 insertions(+), 2 deletions(-)