audit: log binding and unbinding to netlink multicast
PATCH 1/1 audit: log binding and unbinding to netlink multicast
Log information about programs connecting and disconnecting to the audit netlink multicast socket. This is needed so that during investigations a security officer can tell who or what had access to the audit trail. This helps to meet the FAU_SAR.2 requirement for Common Criteria. Sample event:
type=UNKNOWN[1332] msg=audit(1507924331.540:3): pid=1 uid=0 auid=4294967295 tty=(none) ses=4294967295 subj=kernel comm="systemd" exe="/usr/lib/systemd/systemd" nlnk-grp=1 op=connect res=1
--- include/uapi/linux/audit.h | 1 + kernel/audit.c | 48 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-)
include/uapi/linux/audit.h +1 -0
@@ -112,6 +112,7 @@#define AUDIT_FEATURE_CHANGE 1328/* audit log listing feature changes */ #define AUDIT_REPLACE 1329/* Replace auditd if this packet unanswerd */ #define AUDIT_KERN_MODULE 1330/* Kernel Module events */ +#define AUDIT_EVENT_LISTENER 1332/* Task joined multicast read socket */ #define AUDIT_AVC 1400/* SE Linux avc denial or grant */ #define AUDIT_SELINUX_ERR 1401/* Internal SE Linux Errors */
kernel/audit.c +43 -4
65 lines not on this page · raw message
Signature · 1 line
2.13.6
Quoted 11 lines · Steve Grubb
Log information about programs connecting and disconnecting to the audit netlink multicast socket. This is needed so that during investigations a security officer can tell who or what had access to the audit trail. This helps to meet the FAU_SAR.2 requirement for Common Criteria. Sample event:
type=UNKNOWN[1332] msg=audit(1507924331.540:3): pid=1 uid=0 auid=4294967295 tty=(none) ses=4294967295 subj=kernel comm="systemd" exe="/usr/lib/systemd/systemd" nlnk-grp=1 op=connect res=1
Signed-off-by sgrubb <[email protected]>
Quoted 10 lines
--- include/uapi/linux/audit.h | 1 + kernel/audit.c | 48 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-)include/uapi/linux/audit.h +0
-0
include/uapi/linux/audit.h line 115
Quoted 4 lines
include/uapi/linux/audit.h +1
-0@@ -112,6 +112,7 @@#define AUDIT_FEATURE_CHANGE 1328/* audit log listing feature changes */ #define AUDIT_REPLACE 1329/* Replace auditd if this packet unanswerd */ #define AUDIT_KERN_MODULE 1330/* Kernel Module events */ +#define AUDIT_EVENT_LISTENER 1332/* Task joined multicast read socket */
I assume this is waiting on another patch to be reviewed and merged... IIRC the maintainer prefers to deal with that merge conflict rather than accidentally leave a gap if the dependent patch doesn't get merged.
Quoted 80 lines
include/uapi/linux/audit.h +0
-0#define AUDIT_AVC 1400/* SE Linux avc denial or grant */ #define AUDIT_SELINUX_ERR 1401/* Internal SE Linux Errors */ kernel/audit.c +43
-4@@ -1476,22 +1476,61 @@ static void audit_receive(struct sk_buff *skb)mutex_unlock (&audit_cmd_mutex); } +/* Log information about who is connecting to the audit multicast socket */ +static void audit_log_multicast_bind (int group,const char *op,int err) +{ +const struct cred *cred; +struct tty_struct *tty; +char comm[sizeof (current->comm)]; +struct audit_buffer *ab; + + ab =audit_log_start (NULL ,GFP_KERNEL ,AUDIT_EVENT_LISTENER ); +if (!ab) +return ; + + cred =current_cred (); + tty =audit_get_tty (current); + +audit_log_format (ab,"pid=%u uid=%u auid=%u tty=%s ses=%u" , +task_pid_nr (current), +from_kuid (&init_user_ns, cred->uid), +from_kuid (&init_user_ns,audit_get_loginuid (current)), + tty ?tty_name (tty) :"(none)" , +audit_get_sessionid (current)); +audit_put_tty (tty); +audit_log_task_context (ab);/* subj= */ +audit_log_format (ab," comm=" ); +audit_log_untrustedstring (ab,get_task_comm (comm, current)); +audit_log_d_path_exe (ab, current->mm);/* exe= */ +audit_log_format (ab," nlnk-grp=%d op=%s res=%d" , group, op, !err); +audit_log_end (ab); +} +/* Run custom bind function on netlink socket group connect or bind requests. */ -static int audit_bind (struct net *net,int group) +static int audit_multicast_bind (struct net *net,int group) { +int err =0 ; +if (!capable (CAP_AUDIT_READ )) -return -EPERM ; + err = -EPERM ; +audit_log_multicast_bind (group,"connect" , err); -return 0 ; +return err; +} + +static void audit_multicast_unbind (struct net *net,int group) +{ +audit_log_multicast_bind (group,"disconnect" ,0 ); }static int __net_init audit_net_init (struct net *net) {struct netlink_kernel_cfg cfg = { .input = audit_receive, - .bind = audit_bind, + .bind = audit_multicast_bind, .flags =NL_CFG_F_NONROOT_RECV , .groups =AUDIT_NLGRP_MAX , + .unbind = audit_multicast_unbind, };struct audit_net *aunet =net_generic (net, audit_net_id);Signature · 7 lines
2.13.6 -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Quoted 10 lines · Richard Guy Briggs
Quoted 9 lines · Steve Grubb
Log information about programs connecting and disconnecting to the audit netlink multicast socket. This is needed so that during investigations a security officer can tell who or what had access to the audit trail. This helps to meet the FAU_SAR.2 requirement for Common Criteria. Sample event:
type=UNKNOWN[1332] msg=audit(1507924331.540:3): pid=1 uid=0 auid=4294967295 tty=(none) ses=4294967295 subj=kernel comm="systemd" exe="/usr/lib/systemd/systemd" nlnk-grp=1 op=connect res=1
Please add the upstream issue tracker link to the commit description body:
See: https://github.com/linux-audit/audit-kernel/issues/28
Quoted 106 lines
Quoted 1 line
Signed-off-by sgrubb <[email protected]>Reviewed-by Richard Guy Briggs <[email protected]>Quoted 10 lines
--- include/uapi/linux/audit.h | 1 + kernel/audit.c | 48 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-)include/uapi/linux/audit.h +0
-0include/uapi/linux/audit.h line 115
Quoted 4 lines
include/uapi/linux/audit.h +1
-0@@ -112,6 +112,7 @@#define AUDIT_FEATURE_CHANGE 1328/* audit log listing feature changes */ #define AUDIT_REPLACE 1329/* Replace auditd if this packet unanswerd */ #define AUDIT_KERN_MODULE 1330/* Kernel Module events */ +#define AUDIT_EVENT_LISTENER 1332/* Task joined multicast read socket */ I assume this is waiting on another patch to be reviewed and merged... IIRC the maintainer prefers to deal with that merge conflict rather than accidentally leave a gap if the dependent patch doesn't get merged.
Quoted 80 lines
include/uapi/linux/audit.h +0
-0#define AUDIT_AVC 1400/* SE Linux avc denial or grant */ #define AUDIT_SELINUX_ERR 1401/* Internal SE Linux Errors */ kernel/audit.c +43
-4@@ -1476,22 +1476,61 @@ static void audit_receive(struct sk_buff *skb)mutex_unlock (&audit_cmd_mutex); } +/* Log information about who is connecting to the audit multicast socket */ +static void audit_log_multicast_bind (int group,const char *op,int err) +{ +const struct cred *cred; +struct tty_struct *tty; +char comm[sizeof (current->comm)]; +struct audit_buffer *ab; + + ab =audit_log_start (NULL ,GFP_KERNEL ,AUDIT_EVENT_LISTENER ); +if (!ab) +return ; + + cred =current_cred (); + tty =audit_get_tty (current); + +audit_log_format (ab,"pid=%u uid=%u auid=%u tty=%s ses=%u" , +task_pid_nr (current), +from_kuid (&init_user_ns, cred->uid), +from_kuid (&init_user_ns,audit_get_loginuid (current)), + tty ?tty_name (tty) :"(none)" , +audit_get_sessionid (current)); +audit_put_tty (tty); +audit_log_task_context (ab);/* subj= */ +audit_log_format (ab," comm=" ); +audit_log_untrustedstring (ab,get_task_comm (comm, current)); +audit_log_d_path_exe (ab, current->mm);/* exe= */ +audit_log_format (ab," nlnk-grp=%d op=%s res=%d" , group, op, !err); +audit_log_end (ab); +} +/* Run custom bind function on netlink socket group connect or bind requests. */ -static int audit_bind (struct net *net,int group) +static int audit_multicast_bind (struct net *net,int group) { +int err =0 ; +if (!capable (CAP_AUDIT_READ )) -return -EPERM ; + err = -EPERM ; +audit_log_multicast_bind (group,"connect" , err); -return 0 ; +return err; +} + +static void audit_multicast_unbind (struct net *net,int group) +{ +audit_log_multicast_bind (group,"disconnect" ,0 ); }static int __net_init audit_net_init (struct net *net) {struct netlink_kernel_cfg cfg = { .input = audit_receive, - .bind = audit_bind, + .bind = audit_multicast_bind, .flags =NL_CFG_F_NONROOT_RECV , .groups =AUDIT_NLGRP_MAX , + .unbind = audit_multicast_unbind, };struct audit_net *aunet =net_generic (net, audit_net_id);Signature · 7 lines
2.13.6 -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit- RGB
Quoted 6 lines
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
maintainerQuoted 10 lines · Steve Grubb
Log information about programs connecting and disconnecting to the audit netlink multicast socket. This is needed so that during investigations a security officer can tell who or what had access to the audit trail. This helps to meet the FAU_SAR.2 requirement for Common Criteria. Sample event:
type=UNKNOWN[1332] msg=audit(1507924331.540:3): pid=1 uid=0 auid=4294967295 tty=(none) ses=4294967295 subj=kernel comm="systemd" exe="/usr/lib/systemd/systemd" nlnk-grp=1 op=connect res=1
Quoted 5 lines
Signed-off-by sgrubb <[email protected]>--- include/uapi/linux/audit.h | 1 + kernel/audit.c | 48 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-)
Since I think this is going to involve a respin, I just want to mention again "sgrubb" vs "Steve Grubb". More comments inline ...
include/uapi/linux/audit.h line 115
Quoted 9 lines
include/uapi/linux/audit.h +1
-0@@ -112,6 +112,7 @@#define AUDIT_FEATURE_CHANGE 1328/* audit log listing feature changes */ #define AUDIT_REPLACE 1329/* Replace auditd if this packet unanswerd */ #define AUDIT_KERN_MODULE 1330/* Kernel Module events */ +#define AUDIT_EVENT_LISTENER 1332/* Task joined multicast read socket */
What Richard said. Basically AUDIT_EVENT_LISTENER should be 1331 or have a *really* good explanation as to why it needs to be 1332.
Quoted 7 lines
kernel/audit.c +0
-0@@ -1476,22 +1476,61 @@ static void audit_receive(struct sk_buff *skb)mutex_unlock (&audit_cmd_mutex); }
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1487
Quoted 10 lines
kernel/audit.c +9
-0+/* Log information about who is connecting to the audit multicast socket */ +static void audit_log_multicast_bind (int group,const char *op,int err) +{ +const struct cred *cred; +struct tty_struct *tty; +char comm[sizeof (current->comm)]; +struct audit_buffer *ab; + + ab =audit_log_start (NULL ,GFP_KERNEL ,AUDIT_EVENT_LISTENER );
It really seems like this should be associated with the current task, e.g. "audit_log_start(current->audit_context, ...)". After all, the whole point of this record is to capture information about the subject who is binding to the multicast socket.
Quoted 7 lines
kernel/audit.c +7
-0+if (!ab) +return ; + + cred =current_cred (); + tty =audit_get_tty (current); + + audit_log_format(ab, "pid=%u uid=%u auid=%u tty=%s ses=%u",
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1504
Quoted 10 lines
kernel/audit.c +10
-0+task_pid_nr (current), +from_kuid (&init_user_ns, cred->uid), +from_kuid (&init_user_ns,audit_get_loginuid (current)), + tty ?tty_name (tty) :"(none)" , + audit_get_sessionid(current)); +audit_put_tty (tty); +audit_log_task_context (ab);/* subj= */ +audit_log_format (ab," comm=" ); +audit_log_untrustedstring (ab,get_task_comm (comm, current)); +audit_log_d_path_exe (ab, current->mm);/* exe= */
We could drop a lot of the above if this was an associated record.
Quoted 23 lines
kernel/audit.c +15
-3+audit_log_format (ab," nlnk-grp=%d op=%s res=%d" , group, op, !err); +audit_log_end (ab); +} +/* Run custom bind function on netlink socket group connect or bind requests. */ -static int audit_bind (struct net *net,int group) +static int audit_multicast_bind (struct net *net,int group) { +int err =0 ; +if (!capable (CAP_AUDIT_READ )) -return -EPERM ; + err = -EPERM ; +audit_log_multicast_bind (group,"connect" , err); -return 0 ; +return err; +} + +static void audit_multicast_unbind (struct net *net,int group) +{ +audit_log_multicast_bind (group,"disconnect" ,0 ); }
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1533
Quoted 10 lines
kernel/audit.c +2
-1static int __net_init audit_net_init (struct net *net) {struct netlink_kernel_cfg cfg = { .input = audit_receive, - .bind = audit_bind, + .bind = audit_multicast_bind, .flags =NL_CFG_F_NONROOT_RECV , .groups =AUDIT_NLGRP_MAX , + .unbind = audit_multicast_unbind,
Please group the unbind with the bind, it is much easier to read that way.
Quoted 3 lines
kernel/audit.c +0
-0};struct audit_net *aunet =net_generic (net, audit_net_id);
Signature · 2 lines
paul moore www.paul-moore.com
Quoted 33 lines · EDT Paul Moore
Quoted 10 lines · Steve Grubb
Log information about programs connecting and disconnecting to the audit netlink multicast socket. This is needed so that during investigations a security officer can tell who or what had access to the audit trail. This helps to meet the FAU_SAR.2 requirement for Common Criteria. Sample event:
type=UNKNOWN[1332] msg=audit(1507924331.540:3): pid=1 uid=0 auid=4294967295 tty=(none) ses=4294967295 subj=kernel comm="systemd" exe="/usr/lib/systemd/systemd" nlnk-grp=1 op=connect res=1
Quoted 7 lines
Signed-off-by sgrubb <[email protected]>---
include/uapi/linux/audit.h | 1 + kernel/audit.c | 48++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-)
Since I think this is going to involve a respin, I just want to mention again "sgrubb" vs "Steve Grubb". More comments inline ...
Quoted 11 lines
include/uapi/linux/audit.h +0
-0@@ -112,6 +112,7 @@#define AUDIT_FEATURE_CHANGE 1328/* audit log listing feature changes */ #define AUDIT_REPLACE 1329/* Replace auditd if this packet unanswerd */ #define AUDIT_KERN_MODULE 1330/* Kernel Module events */
Quoted 5 lines
Quoted 2 lines
include/uapi/linux/audit.h +1
-0+#define AUDIT_EVENT_LISTENER 1332/* Task joined multicast read socket */
What Richard said. Basically
AUDIT_EVENT_LISTENERshould be 1331 or have a *really* good explanation as to why it needs to be 1332.
Because 1331 is already assigned and in https://git.kernel.org/pub/scm/linux/
kernel/git/jack/linux-fs.git/log/?h=for_next as commit de8cd83e91bc3ee212b3e6ec6e4283af9e4ab269.
If you want me to assign 1331 which is already assigned to AUDIT_FANOTIFY in the user space piece, then it will make your testing...not look right. So, how do you want it?
Quoted 10 lines
Quoted 10 lines
kernel/audit.c +0
-0@@ -1476,22 +1476,61 @@ static void audit_receive(struct sk_buff *skb)mutex_unlock (&audit_cmd_mutex); }
Quoted 14 lines
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1487
Quoted 9 lines
kernel/audit.c +7
-0+/* Log information about who is connecting to the audit multicast socket */ +static void audit_log_multicast_bind(int group, const char *op, int err) +{ +const struct cred *cred; +struct tty_struct *tty; +char comm[sizeof (current->comm)]; +struct audit_buffer *ab; + + ab =audit_log_start (NULL ,GFP_KERNEL ,AUDIT_EVENT_LISTENER );It really seems like this should be associated with the current task, e.g. "
audit_log_start(current->audit_context, ...)". After all, the whole point of this record is to capture information about the subject who is binding to the multicast socket.
OK, easy enough.
Quoted 12 lines
Quoted 12 lines
kernel/audit.c +10
-0+if (!ab) +return ; + + cred =current_cred (); + tty =audit_get_tty (current); + + audit_log_format(ab, "pid=%u uid=%u auid=%u tty=%s ses=%u", + task_pid_nr(current), + from_kuid(&init_user_ns, cred->uid), + from_kuid(&init_user_ns, audit_get_loginuid(current)), + tty ? tty_name(tty) : "(none)",
Quoted 8 lines
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1504
Quoted 6 lines
kernel/audit.c +6
-0+ audit_get_sessionid(current)); +audit_put_tty (tty); +audit_log_task_context (ab);/* subj= */ +audit_log_format (ab," comm=" ); +audit_log_untrustedstring (ab,get_task_comm (comm, current)); +audit_log_d_path_exe (ab, current->mm);/* exe= */ We could drop a lot of the above if this was an associated record.
Really do not want that to be the way we do things. We never have relied on syscall records for simple events.
Quoted 42 lines
Quoted 42 lines
kernel/audit.c +16
-4+audit_log_format (ab," nlnk-grp=%d op=%s res=%d" , group, op, !err); +audit_log_end (ab); +} +/* Run custom bind function on netlink socket group connect or bind requests. */> -static int audit_bind (struct net *net,int group) +static int audit_multicast_bind (struct net *net,int group) { +int err =0 ; +if (!capable (CAP_AUDIT_READ )) -return -EPERM ; + err = -EPERM ; +audit_log_multicast_bind (group,"connect" , err); -return 0 ; +return err; +} + +static void audit_multicast_unbind (struct net *net,int group) +{ +audit_log_multicast_bind (group,"disconnect" ,0 ); }static int __net_init audit_net_init (struct net *net) {struct netlink_kernel_cfg cfg = { .input = audit_receive, - .bind = audit_bind, + .bind = audit_multicast_bind,
Quoted 6 lines
Quoted 4 lines
.flags =
NL_CFG_F_NONROOT_RECV,
.groups =AUDIT_NLGRP_MAX,+ .unbind =
audit_multicast_unbind,Please group the unbind with the bind, it is much easier to read that way.
No problem.
-Steve
Quoted 3 lines
Quoted 3 lines
}; struct audit_net *aunet = net_generic(net, audit_net_id);
Quoted 16 lines · EDT Steve Grubb
Quoted 7 lines
Quoted 7 lines
+/* Log information about who is connecting to the audit multicast socket
*/ +static voidaudit_log_multicast_bind(int group, const char *op, int err)+{kernel/audit.c +3
-0+const struct cred *cred; +struct tty_struct *tty; +char comm[sizeof (current->comm)];Quoted 8 lines
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1487
Quoted 3 lines
kernel/audit.c +3
-0+struct audit_buffer *ab; + + ab =audit_log_start (NULL ,GFP_KERNEL ,AUDIT_EVENT_LISTENER );It really seems like this should be associated with the current task, e.g. "
audit_log_start(current->audit_context, ...)". After all, the whole point of this record is to capture information about the subject who is binding to the multicast socket.
Quoted 1 line
OK, easy enough.
But wouldn't that make it an auxiliary record (if there happens to be a syscall record) instead of a standalone event? The intention is that this event is standalone just like AUDIT_SECCOMP or AUDIT_LOGIN. Associating with the current task is done by using current in formatting the message as seen below. (e.g. task_pid_nr(current), audit_get_sessionid(current))
I think it's correct as is.
-Steve
Quoted 18 lines
Quoted 18 lines
kernel/audit.c +10
-0+if (!ab) +return ; + + cred =current_cred (); + tty =audit_get_tty (current); + + audit_log_format(ab, "pid=%u uid=%u auid=%u tty=%s ses=%u", + task_pid_nr(current), + from_kuid(&init_user_ns, cred->uid), + from_kuid(&init_user_ns,
audit_get_loginuid(current)), + tty ?tty_name(tty)
: "(none)",kernel/audit.c +6
-0+ audit_get_sessionid(current)); +audit_put_tty (tty); +audit_log_task_context (ab);/* subj= */ +audit_log_format (ab," comm=" ); +audit_log_untrustedstring (ab,get_task_comm (comm, current)); +audit_log_d_path_exe (ab, current->mm);/* exe= */
Quoted 19 lines · Steve Grubb
Quoted 16 lines · EDT Steve Grubb
Quoted 7 lines
Quoted 7 lines
+/* Log information about who is connecting to the audit multicast socket
*/ +static voidaudit_log_multicast_bind(int group, const char *op, int err)+{kernel/audit.c +3
-0+const struct cred *cred; +struct tty_struct *tty; +char comm[sizeof (current->comm)];Quoted 8 lines
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1487
Quoted 3 lines
kernel/audit.c +3
-0+struct audit_buffer *ab; + + ab =audit_log_start (NULL ,GFP_KERNEL ,AUDIT_EVENT_LISTENER );It really seems like this should be associated with the current task, e.g. "
audit_log_start(current->audit_context, ...)". After all, the whole point of this record is to capture information about the subject who is binding to the multicast socket.Quoted 1 line
OK, easy enough.
Quoted 7 lines
But wouldn't that make it an auxiliary record (if there happens to be a syscall record) instead of a standalone event? The intention is that this event is standalone just like
AUDIT_SECCOMPorAUDIT_LOGIN. Associating with the current task is done by using current in formatting the message as seen below. (e.g.task_pid_nr(current),audit_get_sessionid(current))I think it's correct as is.
Yes, this would associate this record with the tentatively generated SYSCALL record for this task.
To use the AUDIT_TASK record idea, a local audit context would need to be created and used for the AUDIT_CONFIG_CHANGE and AUDIT_TASK records only.
Quoted 17 lines
-Steve
Quoted 15 lines
Quoted 15 lines
kernel/audit.c +10
-0+if (!ab) +return ; + + cred =current_cred (); + tty =audit_get_tty (current); + + audit_log_format(ab, "pid=%u uid=%u auid=%u tty=%s ses=%u", + task_pid_nr(current), + from_kuid(&init_user_ns, cred->uid), + from_kuid(&init_user_ns,
audit_get_loginuid(current)), + tty ?tty_name(tty)
: "(none)",kernel/audit.c +3
-0+ audit_get_sessionid(current)); +audit_put_tty (tty); +audit_log_task_context (ab);/* subj= */
Quoted 8 lines
Quoted 3 lines
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1504
Quoted 3 lines
kernel/audit.c +3
-0+audit_log_format (ab," comm=" ); +audit_log_untrustedstring (ab,get_task_comm (comm, current)); +audit_log_d_path_exe (ab, current->mm);/* exe= */ --
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Quoted 28 lines · EDT Richard Guy Briggs
Quoted 27 lines · Steve Grubb
Quoted 18 lines · EDT Steve Grubb
Quoted 9 lines
Quoted 9 lines
+/* Log information about who is connecting to the audit multicast socket
*/ +static void audit_log_multicast_bind(int group, const char *op, int
err) +{kernel/audit.c +4
-0+const struct cred *cred; +struct tty_struct *tty; +char comm[sizeof (current->comm)]; +struct audit_buffer *ab;Quoted 8 lines
Quoted 3 lines
(quoted diff) +2
-0+ + ab = audit_log_start(NULL, GFP_KERNEL,
AUDIT_EVENT_LISTENER);It really seems like this should be associated with the current task, e.g. "
audit_log_start(current->audit_context, ...)". After all, the whole point of this record is to capture information about the subject who is binding to the multicast socket.Quoted 1 line
OK, easy enough.
But wouldn't that make it an auxiliary record (if there happens to be a syscall record) instead of a standalone event? The intention is that this event is standalone just like
AUDIT_SECCOMPorAUDIT_LOGIN. Associating with the current task is done by using current in formatting the message as seen below. (e.g.task_pid_nr(current),audit_get_sessionid(current))
Quoted 4 lines
Quoted 1 line
I think it's correct as is.
Yes, this would associate this record with the tentatively generated SYSCALL record for this task.
OK, then my secondary assessment was right. The patch in this hunk is fine. This is a standalone event. I have a patch ready with the .bind/.unbind function pointers grouped together. I can add the ghak issue easily enough. I just need a ruling on what define number to use for AUDIT_EVENT_LISTENER. Should it be 1331 or 1332 (which is its actual number)?
-Steve
Paul Moore
maintainerQuoted 3 lines · Richard Guy Briggs
To use the
AUDIT_TASKrecord idea, a local audit context would need to be created and used for theAUDIT_CONFIG_CHANGEandAUDIT_TASKrecords only.
For the record, the more I think about the AUDIT_TASK record idea, the more I'm souring on the idea. Someday when we get around to an attribute based format something like this will be handy (I'm thinking we will want higher granularity when it comes to records), but right now it doesn't seem very attractive to me.
Signature · 2 lines
paul moore www.paul-moore.com
Paul Moore
maintainerQuoted 17 lines · Steve Grubb
Quoted 16 lines · EDT Steve Grubb
Quoted 15 lines
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1487
Quoted 10 lines
+/* Log information about who is connecting to the audit multicast socket
*/ +static voidaudit_log_multicast_bind(int group, const char *op, int err)+{kernel/audit.c +6
-0+const struct cred *cred; +struct tty_struct *tty; +char comm[sizeof (current->comm)]; +struct audit_buffer *ab; + + ab =audit_log_start (NULL ,GFP_KERNEL ,AUDIT_EVENT_LISTENER );It really seems like this should be associated with the current task, e.g. "
audit_log_start(current->audit_context, ...)". After all, the whole point of this record is to capture information about the subject who is binding to the multicast socket.
Quoted 4 lines
Quoted 1 line
OK, easy enough.
But wouldn't that make it an auxiliary record (if there happens to be a syscall record) instead of a standalone event?
I've always found the significance placed on auxiliary vs standalone events amusing. The only significant difference between the two is that "auxiliary" records share a timestamp with the other records that have been triggered by a given syscall instance whereas standlone records always have a new timestamp generated. I'm personally of the opinion that if we are emitting records we should always try to group related records into a single event to help reduce the confusion for people looking through their audit logs; in other words, whenever possible we should pass the audit_context to audit_log_start() and friends.
If syscall auditing is disabled, these records (e.g. AUDIT_EVENT_LISTENER) will still be emitted by the kernel as "standalone" events. If syscall auditing is enabled, these records will be grouped with the triggering syscall, which just makes sense. Look at the recent problem with init/systemd and the MAC_STATUS records, that is a great example.
Signature · 2 lines
paul moore www.paul-moore.com
Quoted 24 lines · EDT Paul Moore
Quoted 18 lines · Steve Grubb
Quoted 17 lines · EDT Steve Grubb
Quoted 16 lines
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1487
Quoted 11 lines
+/* Log information about who is connecting to the audit multicast socket
*/ +static void audit_log_multicast_bind(int group, const char *op, int
err) +{kernel/audit.c +6
-0+const struct cred *cred; +struct tty_struct *tty; +char comm[sizeof (current->comm)]; +struct audit_buffer *ab; + + ab =audit_log_start (NULL ,GFP_KERNEL ,AUDIT_EVENT_LISTENER );It really seems like this should be associated with the current task, e.g. "
audit_log_start(current->audit_context, ...)". After all, the whole point of this record is to capture information about the subject who is binding to the multicast socket.Quoted 4 lines
Quoted 1 line
OK, easy enough.
But wouldn't that make it an auxiliary record (if there happens to be a syscall record) instead of a standalone event?
Quoted 10 lines
I've always found the significance placed on auxiliary vs standalone events amusing. The only significant difference between the two is that "auxiliary" records share a timestamp with the other records that have been triggered by a given syscall instance whereas standlone records always have a new timestamp generated. I'm personally of the opinion that if we are emitting records we should always try to group related records into a single event to help reduce the confusion for people looking through their audit logs; in other words, whenever possible we should pass the
audit_contexttoaudit_log_start()and friends.
No, actually that introduces confusion. An event should have one specific meaning. This subject did something to that. Adding a syscall record takes a simple event and makes it incomprehensible to anyone that doesn't have familiarity with audit events. All necessary information is in the event that I created.
Quoted 6 lines
If syscall auditing is disabled, these records (e.g.
AUDIT_EVENT_LISTENER) will still be emitted by the kernel as "standalone" events. If syscall auditing is enabled, these records will be grouped with the triggering syscall, which just makes sense. Look at the recent problem with init/systemd and theMAC_STATUSrecords, that is a great example.
Right. That was a mess because of piggy backing on top of a SYSCALL. I do not like events that are sometime simple and sometimes compound. I think that also messes up anyone doing json or xml formatting because they need a firm layout in their schema. The event is good as is. It has all required fields. Its easy to parse and keeps things compact.
-Steve
Paul Moore
maintainerQuoted 36 lines · Steve Grubb
Quoted 24 lines · EDT Paul Moore
Quoted 18 lines · Steve Grubb
Quoted 17 lines · EDT Steve Grubb
Quoted 16 lines
kernel/audit.c static void audit_receive(struct sk_buff *skb) · line 1487
Quoted 11 lines
+/* Log information about who is connecting to the audit multicast socket
*/ +static void audit_log_multicast_bind(int group, const char *op, int
err) +{kernel/audit.c +6
-0+const struct cred *cred; +struct tty_struct *tty; +char comm[sizeof (current->comm)]; +struct audit_buffer *ab; + + ab =audit_log_start (NULL ,GFP_KERNEL ,AUDIT_EVENT_LISTENER );It really seems like this should be associated with the current task, e.g. "
audit_log_start(current->audit_context, ...)". After all, the whole point of this record is to capture information about the subject who is binding to the multicast socket.Quoted 4 lines
Quoted 1 line
OK, easy enough.
But wouldn't that make it an auxiliary record (if there happens to be a syscall record) instead of a standalone event?
Quoted 10 lines
I've always found the significance placed on auxiliary vs standalone events amusing. The only significant difference between the two is that "auxiliary" records share a timestamp with the other records that have been triggered by a given syscall instance whereas standlone records always have a new timestamp generated. I'm personally of the opinion that if we are emitting records we should always try to group related records into a single event to help reduce the confusion for people looking through their audit logs; in other words, whenever possible we should pass the
audit_contexttoaudit_log_start()and friends.
Quoted 5 lines
No, actually that introduces confusion. An event should have one specific meaning. This subject did something to that. Adding a syscall record takes a simple event and makes it incomprehensible to anyone that doesn't have familiarity with audit events. All necessary information is in the event that I created.
What? That makes zero sense. The SYSCALL record and the AUDIT_EVENT_LISTENER record are part of the same real world event: a process executed a syscall and established themselves as an audit event listener.
Quoted 8 lines
Quoted 6 lines
If syscall auditing is disabled, these records (e.g.
AUDIT_EVENT_LISTENER) will still be emitted by the kernel as "standalone" events. If syscall auditing is enabled, these records will be grouped with the triggering syscall, which just makes sense. Look at the recent problem with init/systemd and theMAC_STATUSrecords, that is a great example.Right. That was a mess because of piggy backing on top of a SYSCALL.
Again ... what? The SYSCALL record was the solution to that particular problem, and demonstrated that an event could have multiple records depending on the audit configuration (in this particular case syscall auditing disabled/enabled).
Quoted 2 lines
I do not
like events that are sometime simple and sometimes compound.
That's dependent on your system's audit configuration.
Quoted 3 lines
I think that also
messes up anyone doing json or xml formatting because they need a firm layout in their schema.
I would think anyone consuming audit data for analysis at a higher level would *really* want to see all related records grouped together (i.e. same timestamp). Any higher order analysis should be able to handle event metadata that is optional/missing (we have that now).
Quoted 1 line
The event is good as is. It has all required fields ...
... until you decide that you need to add something; we've seen how well that is going.
Signature · 2 lines
paul moore www.paul-moore.com
Paul Moore
maintainerQuoted 40 lines · Steve Grubb
Quoted 33 lines · EDT Paul Moore
Quoted 10 lines · Steve Grubb
Log information about programs connecting and disconnecting to the audit netlink multicast socket. This is needed so that during investigations a security officer can tell who or what had access to the audit trail. This helps to meet the FAU_SAR.2 requirement for Common Criteria. Sample event:
type=UNKNOWN[1332] msg=audit(1507924331.540:3): pid=1 uid=0 auid=4294967295 tty=(none) ses=4294967295 subj=kernel comm="systemd" exe="/usr/lib/systemd/systemd" nlnk-grp=1 op=connect res=1
Quoted 7 lines
Signed-off-by sgrubb <[email protected]>---
include/uapi/linux/audit.h | 1 + kernel/audit.c | 48++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-)
Since I think this is going to involve a respin, I just want to mention again "sgrubb" vs "Steve Grubb". More comments inline ...
Quoted 11 lines
include/uapi/linux/audit.h +0
-0@@ -112,6 +112,7 @@#define AUDIT_FEATURE_CHANGE 1328/* audit log listing feature changes */ #define AUDIT_REPLACE 1329/* Replace auditd if this packet unanswerd */ #define AUDIT_KERN_MODULE 1330/* Kernel Module events */ Quoted 5 lines
Quoted 2 lines
include/uapi/linux/audit.h +1
-0+#define AUDIT_EVENT_LISTENER 1332/* Task joined multicast read socket */
What Richard said. Basically
AUDIT_EVENT_LISTENERshould be 1331 or have a *really* good explanation as to why it needs to be 1332.
Quoted 7 lines
Because 1331 is already assigned and in https://git.kernel.org/pub/scm/linux/
kernel/git/jack/linux-fs.git/log/?h=for_next as commitde8cd83e91bc3ee212b3e6ec6e4283af9e4ab269.If you want me to assign 1331 which is already assigned to
AUDIT_FANOTIFYin the user space piece, then it will make your testing...not look right. So, how do you want it?
As I said above, I wanted a *really* good explanation, which you provided. In the future it's helpful to add a note about things like this, it saves us all from being annoyed.
I need to think about when this should get merged, but considering we are are -rc5 right now and this is a new feature with no test (at least not that I'm seeing on the list, or on GH) it is likely that this patch will get held until after the upcoming merge window so the merge conflict will not be a practical issue.
(HINT: in case you haven't been paying attention to audit kernel development lately, you should work on a test for the audit-testsuite which tests this new functionality.)
Signature · 2 lines
paul moore www.paul-moore.com