[PATCH] Enable splitting the logs to both auditd and kernel simultaneously
William Roberts
bill.c.roberts at gmail.com
Fri May 24 20:13:55 UTC 2013
Yeah I realized after I sent this clearing the bits becomes a problem... Doh
Thanks
Bill
On May 24, 2013 1:03 PM, "Eric Paris" <eparis at parisplace.org> wrote:
> A memset to 0 is adequate if you want to be able to turn features back
> off. You have to know if the 0 or 1 is meaningful.
>
> Without the mask userland would have to read the features from the
> kernel, twiddle whichever bits they want to twiddle, then write the
> features back to the kernel. With the mask userspace can specify
> which bits they are twiddling and which bits the kernel just shouldn't
> touch. So the read before write isn't necessary and removes that
> little race.
>
> I thought it made setting/unsetting bits easier on the userspace side.
>
> I can get rid of the mask if you think the read/twiddle/write option
> is better than set-mask/write
>
> -Eric
>
> On Fri, May 24, 2013 at 3:33 PM, William Roberts
> <bill.c.roberts at gmail.com> wrote:
> > Well when I was adding to the AUDIT MSG types, I felt pretty dirty :-P
> >
> > I think versioning and bitpacking the features is the right way to go.
> >
> > Whats the purpose of mask in audit_features struct?
> >
> > Looks like you use it to indicate which bits userspace intended to set,
> > incase the structure wasn't memset to 0?
> >
> > Bill
> >
> >
> > On Fri, May 24, 2013 at 9:34 AM, Eric Paris <eparis at parisplace.org>
> wrote:
> >>
> >> On Wed, 2013-05-22 at 19:18 -0700, William Roberts wrote:
> >> > This need came about in Samsung's mobile products. Id like to see and
> >> > attempt to get this mainstreamed. Any feedback would be greatly
> >> > appreciated. Thanks.
> >>
> >> My first thought is that I just sent a patch to introduce a flexible
> >> on/off flipping of bits for audit system features. Rather timely, so we
> >> don't have to keep introducing message types. AUDIT_SET/AUDIT_GET
> >> certainly weren't well thought out in that way. I'd like your thoughts
> >> on those patches (really 1/7 does the heavy lifting 6/7 and 7/7 use
> >> it)
> >>
> >> I thinking porting on top of my series will result in your patch being
> >> about 10 lines.
> >>
> >> I also don't love the name. This isn't about splitting logs, it's about
> >> also logging to kmsg.
> >>
> >> Do we need the Kconfig default? I'd think we should be able to get this
> >> set early enough in userspace. Any messages before auditd are going to
> >> go to kmsg. I envision that we should be able to set features in the
> >> audit.rules so you should be able to turn this on before auditd starts
> >> and things would stop going to kmsg.
> >>
> >> But I don't see a problem with it. If you port on top of this series
> >> I'm ok with queuing for 3.11
> >>
> >> > On Wed, May 22, 2013 at 7:16 PM, William Roberts
> >> > <bill.c.roberts at gmail.com> wrote:
> >> > Allow the audit subsystem to send audit events to both the
> >> > kernel
> >> > message buffer and auditd at the same time.
> >> >
> >> > Signed-off-by: William Roberts <w.roberts at sta.samsung.com>
> >> > ---
> >> > include/uapi/linux/audit.h | 11 ++++++++++
> >> > init/Kconfig | 8 +++++++
> >> > kernel/audit.c | 52
> >> > ++++++++++++++++++++++++++++++++++++++++------
> >> > kernel/audit.h | 6 ++++++
> >> > 4 files changed, 71 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/include/uapi/linux/audit.h
> >> > b/include/uapi/linux/audit.h
> >> > index 75cef3f..0af5d78 100644
> >> > --- a/include/uapi/linux/audit.h
> >> > +++ b/include/uapi/linux/audit.h
> >> > @@ -68,6 +68,8 @@
> >> > #define AUDIT_MAKE_EQUIV 1015 /* Append to watched
> >> > tree */
> >> > #define AUDIT_TTY_GET 1016 /* Get TTY auditing
> >> > status */
> >> > #define AUDIT_TTY_SET 1017 /* Set TTY auditing
> >> > status */
> >> > +#define AUDIT_LOGSPLIT_GET 1018 /* Get logsplit
> >> > status */
> >> > +#define AUDIT_LOGSPLIT_SET 1019 /* Set logsplit
> >> > status */
> >> >
> >> > #define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages
> >> > mostly uninteresting to kernel */
> >> > #define AUDIT_USER_AVC 1107 /* We filter this
> >> > differently */
> >> > @@ -321,6 +323,10 @@ enum {
> >> > #define AUDIT_FAIL_PRINTK 1
> >> > #define AUDIT_FAIL_PANIC 2
> >> >
> >> > +/* Audit splitlog options */
> >> > +#define AUDIT_LOGSPLIT_OFF 0
> >> > +#define AUDIT_LOGSPLIT_ON 1
> >> > +
> >> > /* distinguish syscall tables */
> >> > #define __AUDIT_ARCH_64BIT 0x80000000
> >> > #define __AUDIT_ARCH_LE 0x40000000
> >> > @@ -374,6 +380,11 @@ struct audit_tty_status {
> >> > __u32 log_passwd; /* 1 = enabled, 0 =
> >> > disabled */
> >> > };
> >> >
> >> > +struct audit_logsplit_status {
> >> > + __u32 enabled; /* AUDIT_LOGSPLIT_ON
> >> > or
> >> > +
> >> > AUDIT_LOGSPLIT_OFF */
> >> > +};
> >> > +
> >> > /* audit_rule_data supports filter rules with both integer
> >> > and string
> >> > * fields. It corresponds with AUDIT_ADD_RULE,
> >> > AUDIT_DEL_RULE and
> >> > * AUDIT_LIST_RULES requests.
> >> > diff --git a/init/Kconfig b/init/Kconfig
> >> > index 9d3a788..52efca6 100644
> >> > --- a/init/Kconfig
> >> > +++ b/init/Kconfig
> >> > @@ -286,6 +286,14 @@ config AUDIT_LOGINUID_IMMUTABLE
> >> > one to drop potentially dangerous capabilites from
> >> > the login tasks,
> >> > but may not be backwards compatible with older init
> >> > systems.
> >> >
> >> > +config AUDIT_SPLITLOG
> >> > + bool "Split audit messages to userspace and kernel by
> >> > default"
> >> > + depends on AUDIT
> >> > + help
> >> > + Setting this to true will cause the audit messages
> >> > to be split
> >> > + by default to both the kernel message log and a
> >> > userspace audit
> >> > + daemon if registered.
> >> > +
> >> > source "kernel/irq/Kconfig"
> >> > source "kernel/time/Kconfig"
> >> >
> >> > diff --git a/kernel/audit.c b/kernel/audit.c
> >> > index 21c7fa6..ccd2f60 100644
> >> > --- a/kernel/audit.c
> >> > +++ b/kernel/audit.c
> >> > @@ -88,6 +88,9 @@ static int audit_default;
> >> > /* If auditing cannot proceed, audit_failure selects what
> >> > happens. */
> >> > static int audit_failure = AUDIT_FAIL_PRINTK;
> >> >
> >> > +/* Whether or not logsplit is enabled */
> >> > +static int audit_logsplit = AUDIT_SPLITLOG_INIT;
> >> > +
> >> > /*
> >> > * If audit records are to be written to the netlink socket,
> >> > audit_pid
> >> > * contains the pid of the auditd process and
> >> > audit_nlk_portid contains
> >> > @@ -343,6 +346,17 @@ static int audit_set_failure(int state)
> >> > return audit_do_config_change("audit_failure",
> >> > &audit_failure, state);
> >> > }
> >> >
> >> > +static int audit_set_logsplit(int state)
> >> > +{
> >> > + if (state != AUDIT_LOGSPLIT_OFF
> >> > + && state != AUDIT_LOGSPLIT_ON)
> >> > + return -EINVAL;
> >> > +
> >> > + return audit_do_config_change("audit_logsplit",
> >> > &audit_logsplit, state);
> >> > +}
> >> > +
> >> > +
> >> > +
> >> > /*
> >> > * Queue skbs to be sent to auditd when/if it comes back.
> >> > These skbs should
> >> > * already have been sent via prink/syslog and so if these
> >> > messages are dropped
> >> > @@ -361,11 +375,8 @@ static void audit_hold_skb(struct sk_buff
> >> > *skb)
> >> > kfree_skb(skb);
> >> > }
> >> >
> >> > -/*
> >> > - * For one reason or another this nlh isn't getting delivered
> >> > to the userspace
> >> > - * audit daemon, just send it to printk.
> >> > - */
> >> > -static void audit_printk_skb(struct sk_buff *skb)
> >> > +/* Just printks the skb, no audit_hold or free of any kind */
> >> > +static void __audit_printk_skb(struct sk_buff *skb)
> >> > {
> >> > struct nlmsghdr *nlh = nlmsg_hdr(skb);
> >> > char *data = nlmsg_data(nlh);
> >> > @@ -376,7 +387,14 @@ static void audit_printk_skb(struct
> >> > sk_buff *skb)
> >> > else
> >> > audit_log_lost("printk limit exceeded
> >> > \n");
> >> > }
> >> > -
> >> > +}
> >> > +/*
> >> > + * For one reason or another this nlh isn't getting delivered
> >> > to the userspace
> >> > + * audit daemon, just send it to printk.
> >> > + */
> >> > +static void audit_printk_skb(struct sk_buff *skb)
> >> > +{
> >> > + __audit_printk_skb(skb);
> >> > audit_hold_skb(skb);
> >> > }
> >> >
> >> > @@ -590,6 +608,8 @@ static int audit_netlink_ok(struct sk_buff
> >> > *skb, u16 msg_type)
> >> > case AUDIT_SIGNAL_INFO:
> >> > case AUDIT_TTY_GET:
> >> > case AUDIT_TTY_SET:
> >> > + case AUDIT_LOGSPLIT_GET:
> >> > + case AUDIT_LOGSPLIT_SET:
> >> > case AUDIT_TRIM:
> >> > case AUDIT_MAKE_EQUIV:
> >> > if (!capable(CAP_AUDIT_CONTROL))
> >> > @@ -843,7 +863,24 @@ static int audit_receive_msg(struct
> >> > sk_buff *skb, struct nlmsghdr *nlh)
> >> > spin_unlock(&tsk->sighand->siglock);
> >> > break;
> >> > }
> >> > + case AUDIT_LOGSPLIT_GET: {
> >> > + struct audit_logsplit_status s;
> >> > + s.enabled = audit_logsplit;
> >> > + audit_send_reply(NETLINK_CB(skb).portid, seq,
> >> > + AUDIT_LOGSPLIT_GET, 0, 0, &s,
> >> > sizeof(s));
> >> > + break;
> >> > + }
> >> > + case AUDIT_LOGSPLIT_SET: {
> >> > + struct audit_logsplit_status *s;
> >> > + if (nlh->nlmsg_len < sizeof(struct
> >> > audit_logsplit_status))
> >> > + return -EINVAL;
> >> > + s = data;
> >> > + err = audit_set_logsplit(s->enabled);
> >> > + break;
> >> > + }
> >> > +
> >> > default:
> >> > + printk(KERN_ERR "Unkown audit command");
> >> > err = -EINVAL;
> >> > break;
> >> > }
> >> > @@ -1670,6 +1707,9 @@ void audit_log_end(struct audit_buffer
> >> > *ab)
> >> > nlh->nlmsg_len = ab->skb->len - NLMSG_HDRLEN;
> >> >
> >> > if (audit_pid) {
> >> > + if (audit_logsplit ==
> >> > AUDIT_LOGSPLIT_ON)
> >> > + __audit_printk_skb(ab->skb);
> >> > +
> >> > skb_queue_tail(&audit_skb_queue,
> >> > ab->skb);
> >> > wake_up_interruptible(&kauditd_wait);
> >> > } else {
> >> > diff --git a/kernel/audit.h b/kernel/audit.h
> >> > index 1c95131..ef43bb1 100644
> >> > --- a/kernel/audit.h
> >> > +++ b/kernel/audit.h
> >> > @@ -321,4 +321,10 @@ extern struct list_head
> >> > *audit_killed_trees(void);
> >> > #define audit_filter_inodes(t,c) AUDIT_DISABLED
> >> > #endif
> >> >
> >> > +#ifdef CONFIG_AUDIT_SPLITLOG
> >> > +#define AUDIT_SPLITLOG_INIT AUDIT_LOGSPLIT_ON
> >> > +#else
> >> > +#define AUDIT_SPLITLOG_INIT AUDIT_LOGSPLIT_OFF
> >> > +#endif
> >> > +
> >> > extern struct mutex audit_cmd_mutex;
> >> > --
> >> > 1.8.2.2
> >> >
> >> >
> >> >
> >> >
> >> >
> >> > --
> >> > Respectfully,
> >> >
> >> > William C Roberts
> >> >
> >> >
> >> > --
> >> > Linux-audit mailing list
> >> > Linux-audit at redhat.com
> >> > https://www.redhat.com/mailman/listinfo/linux-audit
> >>
> >>
> >
> >
> >
> > --
> > Respectfully,
> >
> > William C Roberts
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/linux-audit/attachments/20130524/338ff9b7/attachment.htm>
More information about the Linux-audit
mailing list