linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Guy Briggs <rgb@redhat.com>
To: Paul Moore <paul@paul-moore.com>
Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org,
	Eric Paris <eparis@parisplace.org>,
	sgrubb@redhat.com, aviro@redhat.com
Subject: Re: [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG_CHANGE op was involved
Date: Thu, 12 Jul 2018 20:41:22 -0400	[thread overview]
Message-ID: <20180713004122.qlxdpkae4ihkxatg@madcap2.tricolour.ca> (raw)
In-Reply-To: <CAHC9VhTxjcmJGEq6XQmRV0Ouk8oOyHO2C8+HVQOy1qxw9yKyXw@mail.gmail.com>

On 2018-06-28 15:41, Paul Moore wrote:
> On Thu, Jun 14, 2018 at 4:23 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > The failure to add an audit rule due to audit locked gives no clue
> > what CONFIG_CHANGE operation failed.
> > Similarly the set operation is the only other operation that doesn't
> > give the "op=" field to indicate the action.
> > All other CONFIG_CHANGE records include an op= field to give a clue as
> > to what sort of configuration change is being executed.
> >
> > Since these are the only CONFIG_CHANGE records that that do not have an
> > op= field, add them to bring them in line with the rest.
> 
> Normally this would be an immediate reject because this patch inserts
> a field into an existing record, but the CONFIG_CHANGE record is so
> variable (supposedly bad in its own right) that I don't this really
> matters.
> 
> With that out of the way, I think this patch is fine, but I don't
> think it is complete.  At the very least there is another
> CONFIG_CHANGE record in audit_watch_log_rule_change() that doesn't
> appear to include an "op" field.  If we want to make sure we have an
> "op" field in every CONFIG_CHANGE record, let's actually add them all
> :)

The version I'm looking at already had it when it was added in 2009.

This one doesn't add the auid and ses fields because they will be
covered by the linking of this record with the syscall record via the
audit_context() introduced in another patch.

> There appears to be another one in audit_mark_log_rule_change() ...

Same, 2015.

> and one more in audit_receive_msg().  There may be more.

I believe they're covered by other patches in the ghak59 set.

> > Old records:
> > type=CONFIG_CHANGE msg=audit(1519812997.781:374): pid=610 uid=0 auid=0 ses=1 subj=... audit_enabled=2 res=0
> > type=CONFIG_CHANGE msg=audit(2018-06-14 14:55:04.507:47) : audit_enabled=1 old=1 auid=unset ses=unset subj=... res=yes
> >
> > New records:
> > type=CONFIG_CHANGE msg=audit(1520958477.855:100): pid=610 uid=0 auid=0 ses=1 subj=... op=add_rule audit_enabled=2 res=0
> >
> > type=CONFIG_CHANGE msg=audit(2018-06-14 14:55:04.507:47) : op=set audit_enabled=1 old=1 auid=unset ses=unset subj=... res=yes
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/59
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  kernel/audit.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index e7478cb..ad54339 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -403,7 +403,7 @@ static int audit_log_config_change(char *function_name, u32 new, u32 old,
> >         ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
> >         if (unlikely(!ab))
> >                 return rc;
> > -       audit_log_format(ab, "%s=%u old=%u", function_name, new, old);
> > +       audit_log_format(ab, "op=set %s=%u old=%u", function_name, new, old);
> >         audit_log_session_info(ab);
> >         rc = audit_log_task_context(ab);
> >         if (rc)
> > @@ -1365,7 +1365,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> >                         return -EINVAL;
> >                 if (audit_enabled == AUDIT_LOCKED) {
> >                         audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
> > -                       audit_log_format(ab, " audit_enabled=%d res=0", audit_enabled);
> > +                       audit_log_format(ab, " op=%s_rule audit_enabled=%d res=0",
> > +                                        msg_type == AUDIT_ADD_RULE ? "add" : "remove",
> > +                                        audit_enabled);
> >                         audit_log_end(ab);
> >                         return -EPERM;
> >                 }
> > --
> > 1.8.3.1
> 
> -- 
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
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

  reply	other threads:[~2018-07-13  0:43 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-14 20:21 [RFC PATCH ghak59 V1 0/6] audit: config_change normalizations and event record gathering Richard Guy Briggs
2018-06-14 20:21 ` [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG_CHANGE op was involved Richard Guy Briggs
2018-06-28 19:41   ` Paul Moore
2018-07-13  0:41     ` Richard Guy Briggs [this message]
2018-07-18 21:45       ` Paul Moore
2018-07-19 16:08         ` Richard Guy Briggs
2018-07-19 22:47           ` Paul Moore
2018-07-20 13:27             ` Richard Guy Briggs
2018-07-20 14:21               ` Paul Moore
2018-06-14 20:21 ` [RFC PATCH ghak59 V1 2/6] audit: add syscall information to CONFIG_CHANGE records Richard Guy Briggs
2018-06-28 21:47   ` Paul Moore
2018-06-28 22:10     ` Paul Moore
2018-06-14 20:21 ` [RFC PATCH ghak59 V1 3/6] audit: exclude user records from syscall context Richard Guy Briggs
2018-06-28 22:11   ` Paul Moore
2018-07-12 21:46     ` Richard Guy Briggs
2018-07-23 16:40       ` Richard Guy Briggs
2018-07-23 21:00         ` Paul Moore
2018-07-24 13:02           ` Richard Guy Briggs
2018-07-24 20:17             ` Paul Moore
2018-06-14 20:21 ` [RFC PATCH ghak59 V1 4/6] audit: hand taken context to audit_kill_trees for syscall logging Richard Guy Briggs
2018-06-28 22:23   ` Paul Moore
2018-07-13 21:44     ` Richard Guy Briggs
2018-06-14 20:21 ` [RFC PATCH ghak59 V1 5/6] audit: move EOE record after kill_trees for exit/free Richard Guy Briggs
2018-06-28 22:25   ` Paul Moore
2018-06-14 20:21 ` [RFC PATCH ghak59 V1 6/6] audit: extend config_change mark/watch/tree rule changes Richard Guy Briggs
2018-06-28 22:28   ` Paul Moore
2018-06-29 12:31     ` Steve Grubb

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180713004122.qlxdpkae4ihkxatg@madcap2.tricolour.ca \
    --to=rgb@redhat.com \
    --cc=aviro@redhat.com \
    --cc=eparis@parisplace.org \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=sgrubb@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).