lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170131123608.GE7067@madcap2.tricolour.ca>
Date:   Tue, 31 Jan 2017 07:36:08 -0500
From:   Richard Guy Briggs <rgb@...hat.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     Rusty Russell <rusty@...tcorp.com.au>, linux-audit@...hat.com,
        linux-kernel@...r.kernel.org, Jessica Yu <jeyu@...hat.com>
Subject: Re: [RFC] [PATCH] audit: log module name on init_module

On 2017-01-31 06:59, Paul Moore wrote:
> On Thu, Jan 26, 2017 at 4:21 PM, Richard Guy Briggs <rgb@...hat.com> wrote:
> > This adds a new auxiliary record MODULE_INIT to the SYSCALL event.
> >
> > We get finit_module for free since it made most sense to hook this in to
> > load_module().
> >
> > https://github.com/linux-audit/audit-kernel/issues/7
> > https://github.com/linux-audit/audit-kernel/wiki/RFE-Module-load-record-format
> 
> Consistency nit: capitalize the first letter in the wiki page words
> (see the existing RFE pages)
> 
> > Signed-off-by: Richard Guy Briggs <rgb@...hat.com>
> > ---
> >  include/linux/audit.h      |   12 ++++++++++++
> >  include/uapi/linux/audit.h |    1 +
> >  kernel/audit.h             |    3 +++
> >  kernel/auditsc.c           |   20 ++++++++++++++++++++
> >  kernel/module.c            |    5 ++++-
> >  5 files changed, 40 insertions(+), 1 deletions(-)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 2be99b2..7bb23d5 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -360,6 +360,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
> >                                   const struct cred *old);
> >  extern void __audit_log_capset(const struct cred *new, const struct cred *old);
> >  extern void __audit_mmap_fd(int fd, int flags);
> > +extern void __audit_module_init(char *name);
> >
> >  static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> >  {
> > @@ -450,6 +451,12 @@ static inline void audit_mmap_fd(int fd, int flags)
> >                 __audit_mmap_fd(fd, flags);
> >  }
> >
> > +static inline void audit_module_init(char *name)
> > +{
> > +       if (!audit_dummy_context())
> > +               __audit_module_init(name);
> > +}
> 
> More on this below, but I was expecting the function above to named
> audit_log_kern_module(), or something similar.

Ok fair enough, I had mis-understood your previous point.

Any comment on the new record format?

> >  extern int audit_n_rules;
> >  extern int audit_signals;
> >  #else /* CONFIG_AUDITSYSCALL */
> > @@ -561,6 +568,11 @@ static inline void audit_log_capset(const struct cred *new,
> >  { }
> >  static inline void audit_mmap_fd(int fd, int flags)
> >  { }
> > +
> > +static inline void audit_module_init(char *name)
> > +{
> > +}
> > +
> >  static inline void audit_ptrace(struct task_struct *t)
> >  { }
> >  #define audit_n_rules 0
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 3f24110..4a328b4 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -111,6 +111,7 @@
> >  #define AUDIT_PROCTITLE                1327    /* Proctitle emit event */
> >  #define AUDIT_FEATURE_CHANGE   1328    /* audit log listing feature changes */
> >  #define AUDIT_REPLACE          1329    /* Replace auditd if this packet unanswerd */
> > +#define AUDIT_MODULE_INIT      1330    /* Module init event */
> >
> >  #define AUDIT_AVC              1400    /* SE Linux avc denial or grant */
> >  #define AUDIT_SELINUX_ERR      1401    /* Internal SE Linux Errors */
> > diff --git a/kernel/audit.h b/kernel/audit.h
> > index 431444c..144b7eb 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -199,6 +199,9 @@ struct audit_context {
> >                 struct {
> >                         int                     argc;
> >                 } execve;
> > +               struct {
> > +                       char                    *name;
> > +               } module;
> >         };
> >         int fds[2];
> >         struct audit_proctitle proctitle;
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index bb5f504..3e12678 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1172,6 +1172,14 @@ out:
> >         kfree(buf_head);
> >  }
> >
> > +static void audit_log_kern_module(struct audit_context *context,
> > +                                 struct audit_buffer **ab)
> > +{
> > +       audit_log_format(*ab, " name=");
> > +       audit_log_untrustedstring(*ab, context->module.name);
> > +       kfree(context->module.name);
> > +}
> > +
> >  static void show_special(struct audit_context *context, int *call_panic)
> >  {
> >         struct audit_buffer *ab;
> > @@ -1268,6 +1276,9 @@ static void show_special(struct audit_context *context, int *call_panic)
> >         case AUDIT_EXECVE: {
> >                 audit_log_execve_info(context, &ab);
> >                 break; }
> > +       case AUDIT_MODULE_INIT:
> > +               audit_log_kern_module(context, &ab);
> > +               break;
> 
> With the exception of AUDIT_EXECVE everything else in show_special()
> is simply open coded inside the show_special() function; considering
> that audit_log_kern_module() is trivial it seems like a separate
> function isn't really needed here.
> 
> >         }
> >         audit_log_end(ab);
> >  }
> > @@ -2368,6 +2379,15 @@ void __audit_mmap_fd(int fd, int flags)
> >         context->type = AUDIT_MMAP;
> >  }
> >
> > +void __audit_module_init(char *name)
> > +{
> > +       struct audit_context *context = current->audit_context;
> > +
> > +       context->module.name = kmalloc(strlen(name) + 1, GFP_KERNEL);
> > +       strcpy(context->module.name, name);
> > +       context->type = AUDIT_MODULE_INIT;
> > +}
> > +
> >  static void audit_log_task(struct audit_buffer *ab)
> >  {
> >         kuid_t auid, uid;
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 529efae..678407e 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -61,6 +61,7 @@
> >  #include <linux/pfn.h>
> >  #include <linux/bsearch.h>
> >  #include <linux/dynamic_debug.h>
> > +#include <linux/audit.h>
> >  #include <uapi/linux/module.h>
> >  #include "module-internal.h"
> >
> > @@ -3593,6 +3594,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
> >                 goto free_copy;
> >         }
> >
> > +       audit_module_init(mod->name);
> > +
> >         /* Reserve our place in the list. */
> >         err = add_unformed_module(mod);
> >         if (err)
> > @@ -3681,7 +3684,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
> >                        mod->name, after_dashes);
> >         }
> >
> > -       /* Link in to syfs. */
> > +       /* Link in to sysfs. */
> >         err = mod_sysfs_setup(mod, info, mod->kp, mod->num_kp);
> >         if (err < 0)
> >                 goto coming_cleanup;
> 
> paul moore

- RGB

--
Richard Guy Briggs <rgb@...hat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ