On 2019-04-15 17:10, Steve Grubb wrote:
> 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.

Audit still works without CONFIGSYSCALL but is more limited.

> > 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.

The only branch is whether or not to print "?" for the subj field, so
the field is still always there.

> > 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 <[email protected]>
> > > > ---
> > > > 
> > > >  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
> > > [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
> 
> 
> 
> 

- 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

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to