On Monday, April 15, 2019 9:02:36 AM EDT Richard Guy Briggs wrote: > Records that are triggered by an AUDIT_SIGNAL_INFO message including > AUDIT_DAEMON_CONFIG (HUP), AUDIT_DAEMON_ROTATE (USR1), > AUDIT_DAEMON_RESUME (USR2) and AUDIT_DAEMON_END (TERM) have inconsistent > reporting of signal info and swinging field "state". This is crusty old code that has things in it that were for migrations with very old kernels. It's not readily obvious because those kernel transitions were quite some time ago. There was also a fake > They also assume that an empty security context implies there is no > other useful information in the AUDIT_SIGNAL_INFO message so don't use > the information that is there. How are you generating the events that trigger this? If this is based on reading the source code, I think its because of an ancient kernel change. If you can generate this condition, then I'd like to replicate the problem on my system to see what's happening.

> Normalize AUDIT_DAEMON_CONFIG to use the value "reconfigure" and add the > "state" field where missing. > > Use audit_sig_info values when available, not making assumptions about > their availability when the security context is absent. > > See: https://github.com/linux-audit/audit-userspace/issues/90 I had made changes to the git repo Saturday. I suspect this patch does not apply. I like the op value changes. That part I can go along with. Shall I just make that adjustment in the upstream repo and we can talk more about how you are creating these events?

-Steve > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > --- > docs/audit_request_signal_info.3 | 2 +- > lib/libaudit.c | 12 +++++++++ > lib/libaudit.h | 1 + > src/auditd-reconfig.c | 9 +++---- > src/auditd.c | 54 > ++++++++++++++-------------------------- 5 files changed, 36 > insertions(+), 42 deletions(-) > > diff --git a/docs/audit_request_signal_info.3 > b/docs/audit_request_signal_info.3 index 873deb58bef3..b68d7bbefeed 100644 > --- a/docs/audit_request_signal_info.3 > +++ b/docs/audit_request_signal_info.3 > @@ -8,7 +8,7 @@ int audit_request_signal_info(int fd); > > .SH "DESCRIPTION" > > -audit_request_signal_info requests that the kernel send information about > the sender of a signal to the audit daemon. The sinal info structure is as > follows: +audit_request_signal_info requests that the kernel send > information about the sender of a signal to the audit daemon. The signal > info structure is as follows: > > .nf > struct audit_sig_info { > diff --git a/lib/libaudit.c b/lib/libaudit.c > index 2af017a0e520..e9c4f9cad6df 100644 > --- a/lib/libaudit.c > +++ b/lib/libaudit.c > @@ -674,6 +674,18 @@ int audit_request_signal_info(int fd) > return rc; > } > > +char *audit_format_signal_info(char *buf, int len, char *op, struct > audit_reply *rep, char *res) +{ > + snprintf(buf, len, > + "op=%s auid=%u pid=%d subj=%s res=%s", > + op, > + rep->signal_info->uid, > + rep->signal_info->pid, > + rep->len == 24 ? "?" : rep->signal_info->ctx, > + res); > + return buf; > +} > + > int audit_update_watch_perms(struct audit_rule_data *rule, int perms) > { > unsigned int i, done=0; > diff --git a/lib/libaudit.h b/lib/libaudit.h > index ca7aa63e354e..63a5e948d00e 100644 > --- a/lib/libaudit.h > +++ b/lib/libaudit.h > @@ -562,6 +562,7 @@ extern int audit_setloginuid(uid_t uid); > extern uint32_t audit_get_session(void); > extern int audit_detect_machine(void); > extern int audit_determine_machine(const char *arch); > +extern char *audit_format_signal_info(char *buf, int len, char *op, struct > audit_reply *rep, char *res); > > /* Translation functions */ > extern int audit_name_to_field(const char *field); > diff --git a/src/auditd-reconfig.c b/src/auditd-reconfig.c > index a03e29aa57ab..f5b00e6d1dc7 100644 > --- a/src/auditd-reconfig.c > +++ b/src/auditd-reconfig.c > @@ -115,12 +115,9 @@ static void *config_thread_main(void *arg) > } else { > // need to send a failed event message > char txt[MAX_AUDIT_MESSAGE_LENGTH]; > - snprintf(txt, sizeof(txt), > - "op=reconfigure state=no-change auid=%u pid=%d subj=%s res=failed", > - e->reply.signal_info->uid, > - e->reply.signal_info->pid, > - (e->reply.len > 24) ? > - e->reply.signal_info->ctx : "?"); > + audit_format_signal_info(txt, sizeof(txt), > + "reconfigure state=no-change", > + &e->reply, "failed"); > // FIXME: need to figure out sending this > //send_audit_event(AUDIT_DAEMON_CONFIG, txt); > free_config(&new_config); > diff --git a/src/auditd.c b/src/auditd.c > index c04a1c9ce93f..5c31583a49c6 100644 > --- a/src/auditd.c > +++ b/src/auditd.c > @@ -131,7 +131,7 @@ static void hup_handler( struct ev_loop *loop, struct > ev_signal *sig, int revent rc = audit_request_signal_info(fd); > if (rc < 0) > send_audit_event(AUDIT_DAEMON_CONFIG, > - "op=hup-info state=request-siginfo auid=-1 pid=-1 subj=? res=failed"); > + "op=reconfigure state=no-change auid=-1 pid=-1 subj=? res=failed"); > else > hup_info_requested = 1; > } > @@ -147,7 +147,7 @@ static void user1_handler(struct ev_loop *loop, struct > ev_signal *sig, rc = audit_request_signal_info(fd); > if (rc < 0) > send_audit_event(AUDIT_DAEMON_ROTATE, > - "op=usr1-info auid=-1 pid=-1 subj=? res=failed"); > + "op=rotate-logs auid=-1 pid=-1 subj=? res=failed"); > else > usr1_info_requested = 1; > } > @@ -163,7 +163,7 @@ static void user2_handler( struct ev_loop *loop, struct > ev_signal *sig, int reve if (rc < 0) { > resume_logging(); > send_audit_event(AUDIT_DAEMON_RESUME, > - "op=resume-logging auid=-1 pid=-1 subj=? res=success"); > + "op=resume-logging auid=-1 pid=-1 subj=? res=failed"); > } else > usr2_info_requested = 1; > } > @@ -515,45 +515,33 @@ static void netlink_handler(struct ev_loop *loop, > struct ev_io *io, break; > case AUDIT_SIGNAL_INFO: > if (hup_info_requested) { > + char hup[MAX_AUDIT_MESSAGE_LENGTH]; > audit_msg(LOG_DEBUG, > "HUP detected, starting config manager"); > reconfig_ev = cur_event; > if (start_config_manager(cur_event)) { > - send_audit_event( > - AUDIT_DAEMON_CONFIG, > - "op=reconfigure state=no-change " > - "auid=-1 pid=-1 subj=? res=failed"); > + audit_format_signal_info(hup, sizeof(hup), > + "reconfigure state=no-change", > + &cur_event->reply, > + "failed"); > + send_audit_event(AUDIT_DAEMON_CONFIG, hup); > } > cur_event = NULL; > hup_info_requested = 0; > } else if (usr1_info_requested) { > char usr1[MAX_AUDIT_MESSAGE_LENGTH]; > - if (cur_event->reply.len == 24) { > - snprintf(usr1, sizeof(usr1), > - "op=rotate-logs auid=-1 pid=-1 subj=?"); > - } else { > - snprintf(usr1, sizeof(usr1), > - "op=rotate-logs auid=%u pid=%d subj=%s", > - cur_event->reply.signal_info->uid, > - cur_event->reply.signal_info->pid, > - cur_event->reply.signal_info->ctx); > - } > + audit_format_signal_info(usr1, sizeof(usr1), > + "rotate-logs", > + &cur_event->reply, > + "success"); > send_audit_event(AUDIT_DAEMON_ROTATE, usr1); > usr1_info_requested = 0; > } else if (usr2_info_requested) { > char usr2[MAX_AUDIT_MESSAGE_LENGTH]; > - if (cur_event->reply.len == 24) { > - snprintf(usr2, sizeof(usr2), > - "op=resume-logging auid=-1 " > - "pid=-1 subj=? res=success"); > - } else { > - snprintf(usr2, sizeof(usr2), > - "op=resume-logging " > - "auid=%u pid=%d subj=%s res=success", > - cur_event->reply.signal_info->uid, > - cur_event->reply.signal_info->pid, > - cur_event->reply.signal_info->ctx); > - } > + audit_format_signal_info(usr2, sizeof(usr2), > + "resume-logging", > + &cur_event->reply, > + "success"); > resume_logging(); > libdisp_resume(); > send_audit_event(AUDIT_DAEMON_RESUME, usr2); > @@ -993,12 +981,8 @@ int main(int argc, char *argv[]) > rc = get_reply(fd, &trep, rc); > if (rc > 0) { > char txt[MAX_AUDIT_MESSAGE_LENGTH]; > - snprintf(txt, sizeof(txt), > - "op=terminate auid=%u " > - "pid=%d subj=%s res=success", > - trep.signal_info->uid, > - trep.signal_info->pid, > - trep.signal_info->ctx); > + audit_format_signal_info(txt, sizeof(txt), "terminate", > + &trep, "success"); > send_audit_event(AUDIT_DAEMON_END, txt); > } > } -- Linux-audit mailing list Linux-audit(a)redhat.com https://www.redhat.com/mailman/listinfo/linux-audit

On Monday, April 15, 2019 12:21:49 PM EDT Richard Guy Briggs wrote: > On 2019-04-15 10:58, Steve Grubb wrote: > > On Monday, April 15, 2019 9:02:36 AM EDT Richard Guy Briggs wrote: > > > Records that are triggered by an AUDIT_SIGNAL_INFO message including > > > AUDIT_DAEMON_CONFIG (HUP), AUDIT_DAEMON_ROTATE (USR1), > > > AUDIT_DAEMON_RESUME (USR2) and AUDIT_DAEMON_END (TERM) have > > > inconsistent reporting of signal info and swinging field "state". > > > > This is crusty old code that has things in it that were for migrations > > with very old kernels. It's not readily obvious because those kernel > > transitions were quite some time ago. There was also a fake > > > > > They also assume that an empty security context implies there is no > > > other useful information in the AUDIT_SIGNAL_INFO message so don't use > > > the information that is there. > > > > How are you generating the events that trigger this? If this is based on > > reading the source code, I think its because of an ancient kernel change. > > If you can generate this condition, then I'd like to replicate the > > problem on my system to see what's happening. > > I used a current audit/next kernel, compiled with AUDIT_CONFIG, but with > and without AUDIT_CONFIGSYSCALL Is this a configuration that we really want to support? :-) This really will not work as anything except for gathering AVC's or other LSM events. And I think those go to syslog anyways. I'd say that with this kernel configuration, they likely would not run auditd as there's no point in it.

> and simply signalled the audit daemon with the four signals and then ran > ausearch on the most recent messages. > > I did not generate errors, but I could have by creating a custom kernel > that returned errors upon request for AUDIT_SIGNAL_INFO. > > > > Normalize AUDIT_DAEMON_CONFIG to use the value "reconfigure" and add > > > the "state" field where missing. > > > > > > Use audit_sig_info values when available, not making assumptions about > > > their availability when the security context is absent. > > > > > > See: https://github.com/linux-audit/audit-userspace/issues/90 > > > > I had made changes to the git repo Saturday. I suspect this patch does > > not apply. I like the op value changes. That part I can go along with. > > Shall I just make that adjustment in the upstream repo and we can talk > > more about how you are creating these events? > > This patch was rebased on your patch so it is current. One thing I should mention, the audit events are created with and without the subj field because for common criteria, a DAC based system should have no notion of MAC fields. This is why we alway branch on the "ctx" variables and create the event with or without subj.

> I wanted it all in one function so that the format was consistent rather > than open coded for anything that could use the AUDIT_SIGNAL_INFO > contents. This will also help once there is also a cid (contid) to > report. OK. -Steve > > > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > > > --- > > > > > > docs/audit_request_signal_info.3 | 2 +- > > > lib/libaudit.c | 12 +++++++++ > > > lib/libaudit.h | 1 + > > > src/auditd-reconfig.c | 9 +++---- > > > src/auditd.c | 54 > > > > > > ++++++++++++++-------------------------- 5 files changed, 36 > > > insertions(+), 42 deletions(-) > > > > > > diff --git a/docs/audit_request_signal_info.3 > > > b/docs/audit_request_signal_info.3 index 873deb58bef3..b68d7bbefeed > > > 100644 > > > --- a/docs/audit_request_signal_info.3 > > > +++ b/docs/audit_request_signal_info.3 > > > @@ -8,7 +8,7 @@ int audit_request_signal_info(int fd); > > > > > > .SH "DESCRIPTION" > > > > > > -audit_request_signal_info requests that the kernel send information > > > about > > > the sender of a signal to the audit daemon. The sinal info structure is > > > as > > > follows: +audit_request_signal_info requests that the kernel send > > > information about the sender of a signal to the audit daemon. The > > > signal > > > > > > info structure is as follows: > > > .nf > > > struct audit_sig_info { > > > > > > diff --git a/lib/libaudit.c b/lib/libaudit.c > > > index 2af017a0e520..e9c4f9cad6df 100644 > > > --- a/lib/libaudit.c > > > +++ b/lib/libaudit.c > > > @@ -674,6 +674,18 @@ int audit_request_signal_info(int fd) > > > > > > return rc; > > > > > > } > > > > > > +char *audit_format_signal_info(char *buf, int len, char *op, struct > > > audit_reply *rep, char *res) +{ > > > + snprintf(buf, len, > > > + "op=%s auid=%u pid=%d subj=%s res=%s", > > > + op, > > > + rep->signal_info->uid, > > > + rep->signal_info->pid, > > > + rep->len == 24 ? "?" : rep->signal_info->ctx, > > > + res); > > > + return buf; > > > +} > > > + > > > > > > int audit_update_watch_perms(struct audit_rule_data *rule, int perms) > > > { > > > > > > unsigned int i, done=0; > > > > > > diff --git a/lib/libaudit.h b/lib/libaudit.h > > > index ca7aa63e354e..63a5e948d00e 100644 > > > --- a/lib/libaudit.h > > > +++ b/lib/libaudit.h > > > @@ -562,6 +562,7 @@ extern int audit_setloginuid(uid_t uid); > > > > > > extern uint32_t audit_get_session(void); > > > extern int audit_detect_machine(void); > > > extern int audit_determine_machine(const char *arch); > > > > > > +extern char *audit_format_signal_info(char *buf, int len, char *op, > > > struct audit_reply *rep, char *res); > > > > > > /* Translation functions */ > > > extern int audit_name_to_field(const char *field); > > > > > > diff --git a/src/auditd-reconfig.c b/src/auditd-reconfig.c > > > index a03e29aa57ab..f5b00e6d1dc7 100644 > > > --- a/src/auditd-reconfig.c > > > +++ b/src/auditd-reconfig.c > > > @@ -115,12 +115,9 @@ static void *config_thread_main(void *arg) > > > > > > } else { > > > > > > // need to send a failed event message > > > char txt[MAX_AUDIT_MESSAGE_LENGTH]; > > > > > > - snprintf(txt, sizeof(txt), > > > - "op=reconfigure state=no-change auid=%u pid=%d subj=%s > > > > res=failed", > > > > > - e->reply.signal_info->uid, > > > - e->reply.signal_info->pid, > > > - (e->reply.len > 24) ? > > > - e->reply.signal_info->ctx : "?"); > > > + audit_format_signal_info(txt, sizeof(txt), > > > + "reconfigure state=no-change", > > > + &e->reply, "failed"); > > > > > > // FIXME: need to figure out sending this > > > //send_audit_event(AUDIT_DAEMON_CONFIG, txt); > > > free_config(&new_config); > > > > > > diff --git a/src/auditd.c b/src/auditd.c > > > index c04a1c9ce93f..5c31583a49c6 100644 > > > --- a/src/auditd.c > > > +++ b/src/auditd.c > > > @@ -131,7 +131,7 @@ static void hup_handler( struct ev_loop *loop, > > > struct > > > ev_signal *sig, int revent rc = audit_request_signal_info(fd); > > > > > > if (rc < 0) > > > > > > send_audit_event(AUDIT_DAEMON_CONFIG, > > > > > > - "op=hup-info state=request-siginfo auid=-1 pid=-1 subj=? > > > > res=failed"); > > > > > + "op=reconfigure state=no-change auid=-1 pid=-1 subj=? res=failed"); > > > > > > else > > > > > > hup_info_requested = 1; > > > > > > } > > > > > > @@ -147,7 +147,7 @@ static void user1_handler(struct ev_loop *loop, > > > struct > > > ev_signal *sig, rc = audit_request_signal_info(fd); > > > > > > if (rc < 0) > > > > > > send_audit_event(AUDIT_DAEMON_ROTATE, > > > > > > - "op=usr1-info auid=-1 pid=-1 subj=? res=failed"); > > > + "op=rotate-logs auid=-1 pid=-1 subj=? res=failed"); > > > > > > else > > > > > > usr1_info_requested = 1; > > > > > > } > > > > > > @@ -163,7 +163,7 @@ static void user2_handler( struct ev_loop *loop, > > > struct ev_signal *sig, int reve if (rc < 0) { > > > > > > resume_logging(); > > > send_audit_event(AUDIT_DAEMON_RESUME, > > > > > > - "op=resume-logging auid=-1 pid=-1 subj=? > > > > res=success"); > > > > > + "op=resume-logging auid=-1 pid=-1 subj=? res=failed"); > > > > > > } else > > > > > > usr2_info_requested = 1; > > > > > > } > > > > > > @@ -515,45 +515,33 @@ static void netlink_handler(struct ev_loop *loop, > > > struct ev_io *io, break; > > > > > > case AUDIT_SIGNAL_INFO: > > > if (hup_info_requested) { > > > > > > + char hup[MAX_AUDIT_MESSAGE_LENGTH]; > > > > > > audit_msg(LOG_DEBUG, > > > > > > "HUP detected, starting config manager"); > > > > > > reconfig_ev = cur_event; > > > if (start_config_manager(cur_event)) { > > > > > > - send_audit_event( > > > - AUDIT_DAEMON_CONFIG, > > > - "op=reconfigure state=no-change " > > > - "auid=-1 pid=-1 subj=? res=failed"); > > > + audit_format_signal_info(hup, sizeof(hup), > > > + "reconfigure > > > > state=no-change", > > > > > + &cur_event->reply, > > > + "failed"); > > > + send_audit_event(AUDIT_DAEMON_CONFIG, > > > > hup); > > > > > } > > > cur_event = NULL; > > > hup_info_requested = 0; > > > > > > } else if (usr1_info_requested) { > > > > > > char usr1[MAX_AUDIT_MESSAGE_LENGTH]; > > > > > > - if (cur_event->reply.len == 24) { > > > - snprintf(usr1, sizeof(usr1), > > > - "op=rotate-logs auid=-1 pid=-1 subj=?"); > > > - } else { > > > - snprintf(usr1, sizeof(usr1), > > > - "op=rotate-logs auid=%u pid=%d subj=%s", > > > - cur_event->reply.signal_info->uid, > > > - cur_event->reply.signal_info->pid, > > > - cur_event->reply.signal_info->ctx); > > > - } > > > + audit_format_signal_info(usr1, sizeof(usr1), > > > + "rotate-logs", > > > + &cur_event->reply, > > > + "success"); > > > > > > send_audit_event(AUDIT_DAEMON_ROTATE, usr1); > > > usr1_info_requested = 0; > > > > > > } else if (usr2_info_requested) { > > > > > > char usr2[MAX_AUDIT_MESSAGE_LENGTH]; > > > > > > - if (cur_event->reply.len == 24) { > > > - snprintf(usr2, sizeof(usr2), > > > - "op=resume-logging auid=-1 " > > > - "pid=-1 subj=? res=success"); > > > - } else { > > > - snprintf(usr2, sizeof(usr2), > > > - "op=resume-logging " > > > - "auid=%u pid=%d subj=%s res=success", > > > - cur_event->reply.signal_info->uid, > > > - cur_event->reply.signal_info->pid, > > > - cur_event->reply.signal_info->ctx); > > > - } > > > + audit_format_signal_info(usr2, sizeof(usr2), > > > + "resume-logging", > > > + &cur_event->reply, > > > + "success"); > > > > > > resume_logging(); > > > libdisp_resume(); > > > send_audit_event(AUDIT_DAEMON_RESUME, usr2); > > > > > > @@ -993,12 +981,8 @@ int main(int argc, char *argv[]) > > > > > > rc = get_reply(fd, &trep, rc); > > > if (rc > 0) { > > > > > > char txt[MAX_AUDIT_MESSAGE_LENGTH]; > > > > > > - snprintf(txt, sizeof(txt), > > > - "op=terminate auid=%u " > > > - "pid=%d subj=%s res=success", > > > - trep.signal_info->uid, > > > - trep.signal_info->pid, > > > - trep.signal_info->ctx); > > > + audit_format_signal_info(txt, sizeof(txt), "terminate", > > > + &trep, "success"); > > > > > > send_audit_event(AUDIT_DAEMON_END, txt); > > > > > > } > > > > > > } > > > > -- > > Linux-audit mailing list > > Linux-audit(a)redhat.com > > https://www.redhat.com/mailman/listinfo/linux-audit > > - RGB > > -- > Richard Guy Briggs <rgb(a)redhat.com&gt; > 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