* logging changes in tty logging status @ 2013-11-13 20:04 Richard Guy Briggs 2013-11-13 20:22 ` Steve Grubb 0 siblings, 1 reply; 4+ messages in thread From: Richard Guy Briggs @ 2013-11-13 20:04 UTC (permalink / raw) To: Steve Grubb; +Cc: linux-audit Hi Steve, I'm reviewing audit_receive_msg() and noticing that the AUDIT_TTY_SET case doesn't log a configuration change. Should it? - RGB -- Richard Guy Briggs <rbriggs@redhat.com> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: logging changes in tty logging status 2013-11-13 20:04 logging changes in tty logging status Richard Guy Briggs @ 2013-11-13 20:22 ` Steve Grubb 2013-11-14 14:16 ` Richard Guy Briggs 0 siblings, 1 reply; 4+ messages in thread From: Steve Grubb @ 2013-11-13 20:22 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: linux-audit On Wednesday, November 13, 2013 03:04:18 PM Richard Guy Briggs wrote: > Hi Steve, > > I'm reviewing audit_receive_msg() and noticing that the AUDIT_TTY_SET > case doesn't log a configuration change. Should it? Yes, it should. Any change in config should be recorded with subject, old value, new value, and results. It should match other config change events. -Steve ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: logging changes in tty logging status 2013-11-13 20:22 ` Steve Grubb @ 2013-11-14 14:16 ` Richard Guy Briggs 2013-11-14 20:43 ` Richard Guy Briggs 0 siblings, 1 reply; 4+ messages in thread From: Richard Guy Briggs @ 2013-11-14 14:16 UTC (permalink / raw) To: Steve Grubb; +Cc: linux-audit On Wed, Nov 13, 2013 at 03:22:49PM -0500, Steve Grubb wrote: > On Wednesday, November 13, 2013 03:04:18 PM Richard Guy Briggs wrote: > > Hi Steve, > > > > I'm reviewing audit_receive_msg() and noticing that the AUDIT_TTY_SET > > case doesn't log a configuration change. Should it? > > Yes, it should. Any change in config should be recorded with subject, old > value, new value, and results. It should match other config change events. So perhaps something like this, but should probably re-structure the code to make it cleaner and re-factor a formatting function... Any opinion on the labels/tags? diff --git a/kernel/audit.c b/kernel/audit.c index 7b0e23a..cba0109 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -829,18 +829,36 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) case AUDIT_TTY_SET: { struct audit_tty_status s; struct task_struct *tsk = current; + struct audit_buffer *ab; memset(&s, 0, sizeof(s)); /* guard against past and future API changes */ memcpy(&s, data, min(sizeof(s), (size_t)nlh->nlmsg_len)); + audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE); + audit_log_format(ab, " old.audit_tty_status.enabled=%d" + " old.audit_tty_status.log_passwd=%d", + tsk->signal->audit_tty, + tsk->signal->audit_tty_log_passwd); + audit_log_format(ab, " new.audit_tty_status.enabled=%d" + " new.audit_tty_status.log_passwd=%d", + s.enabled, s.log_passwd); if ((s.enabled != 0 && s.enabled != 1) || (s.log_passwd != 0 && s.log_passwd != 1)) - return -EINVAL; +{ + audit_log_format(ab, " res=0"); + audit_log_end(ab); + return -EINVAL; +} spin_lock(&tsk->sighand->siglock); tsk->signal->audit_tty = s.enabled; tsk->signal->audit_tty_log_passwd = s.log_passwd; spin_unlock(&tsk->sighand->siglock); + + audit_log_format(ab, " res=1"); + audit_log_end(ab); + + break; } default: > -Steve - RGB -- Richard Guy Briggs <rbriggs@redhat.com> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: logging changes in tty logging status 2013-11-14 14:16 ` Richard Guy Briggs @ 2013-11-14 20:43 ` Richard Guy Briggs 0 siblings, 0 replies; 4+ messages in thread From: Richard Guy Briggs @ 2013-11-14 20:43 UTC (permalink / raw) To: Steve Grubb; +Cc: linux-audit On Thu, Nov 14, 2013 at 09:16:15AM -0500, Richard Guy Briggs wrote: > On Wed, Nov 13, 2013 at 03:22:49PM -0500, Steve Grubb wrote: > > On Wednesday, November 13, 2013 03:04:18 PM Richard Guy Briggs wrote: > > > Hi Steve, > > > > > > I'm reviewing audit_receive_msg() and noticing that the AUDIT_TTY_SET > > > case doesn't log a configuration change. Should it? > > > > Yes, it should. Any change in config should be recorded with subject, old > > value, new value, and results. It should match other config change events. > > So perhaps something like this, but should probably re-structure the > code to make it cleaner and re-factor a formatting function... > > Any opinion on the labels/tags? Here's a cleaner go at it. I'm still not completely happy with the redundancy and non-standard old/new labels. This is all one command, but changes two values. Should they be in one line, or in two? One would be much simpler. Would it be acceptable to do something like: " op=tty_set-enabled/log_passwd old=0/1 new=1/1 res=1" diff --git a/kernel/audit.c b/kernel/audit.c index 7b0e23a..1e74576 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -827,20 +827,42 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) break; } case AUDIT_TTY_SET: { - struct audit_tty_status s; + struct audit_tty_status s, old; struct task_struct *tsk = current; + struct audit_buffer *ab; + int res = 0; + + spin_lock(&tsk->sighand->siglock); + old.enabled = tsk->signal->audit_tty; + old.log_passwd = tsk->signal->audit_tty_log_passwd; + spin_unlock(&tsk->sighand->siglock); memset(&s, 0, sizeof(s)); /* guard against past and future API changes */ memcpy(&s, data, min(sizeof(s), (size_t)nlh->nlmsg_len)); - if ((s.enabled != 0 && s.enabled != 1) || - (s.log_passwd != 0 && s.log_passwd != 1)) + if ((s.enabled == 0 || s.enabled == 1) && + (s.log_passwd == 0 || s.log_passwd == 1)) + res = 1; + audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE); + audit_log_format(ab, " op=tty_set" + " old.enabled=%d" + " new.enabled=%d" + " old.log_passwd=%d" + " new.log_passwd=%d" + " res=%d", + old.enabled, + s.enabled, + old.log_passwd, + s.log_passwd, + res); + audit_log_end(ab); + if (res) { + spin_lock(&tsk->sighand->siglock); + tsk->signal->audit_tty = s.enabled; + tsk->signal->audit_tty_log_passwd = s.log_passwd; + spin_unlock(&tsk->sighand->siglock); + } else return -EINVAL; - - spin_lock(&tsk->sighand->siglock); - tsk->signal->audit_tty = s.enabled; - tsk->signal->audit_tty_log_passwd = s.log_passwd; - spin_unlock(&tsk->sighand->siglock); break; } default: > > -Steve > > - RGB > > -- > Richard Guy Briggs <rbriggs@redhat.com> > Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat > Remote, Ottawa, Canada > Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 > > -- > Linux-audit mailing list > Linux-audit@redhat.com > https://www.redhat.com/mailman/listinfo/linux-audit - RGB -- Richard Guy Briggs <rbriggs@redhat.com> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-14 20:43 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-13 20:04 logging changes in tty logging status Richard Guy Briggs 2013-11-13 20:22 ` Steve Grubb 2013-11-14 14:16 ` Richard Guy Briggs 2013-11-14 20:43 ` Richard Guy Briggs
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox