[PATCH ghak59 V1 0/2] tree and watch rule log cleanups

[PATCH ghak59 V1 0/2] tree and watch rule log cleanups

[Richard Guy Briggs]

Make some tree and watch rule logging cleanups before applying
normalizations and record connections for ghak 59.

See: https://github.com/linux-audit/audit-kernel/issues/50

Richard Guy Briggs (2):
  audit: tree: check audit_enabled
  audit: watch: simplify audit_enabled check

 kernel/audit_tree.c  |  2 ++
 kernel/audit_watch.c | 29 +++++++++++++++--------------
 2 files changed, 17 insertions(+), 14 deletions(-)

-- 
1.8.3.1

[PATCH ghak59 V1 1/2] audit: tree: check audit_enabled

[Richard Guy Briggs]

Respect the audit_enabled flag when printing tree rule config change
records.

See: https://github.com/linux-audit/audit-kernel/issues/50
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit_tree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 67e6956..5e9d1e5 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -497,6 +497,8 @@ static void audit_tree_log_remove_rule(struct audit_krule *rule)
 {
 	struct audit_buffer *ab;
 
+	if (!audit_enabled)
+		return;
 	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
-- 
1.8.3.1

[PATCH ghak59 V1 2/2] audit: watch: simplify audit_enabled check

[Richard Guy Briggs]

Check the audit_enabled flag and bail immediately.  This does not change
the functionality, but brings the code format in line with similar
checks in audit_tree_log_remove_rule(), audit_mark_log_rule_change(),
and elsewhere in the audit code.

See: https://github.com/linux-audit/audit-kernel/issues/50
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit_watch.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index f1ba889..9b4836b 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -238,20 +238,21 @@ static struct audit_watch *audit_dupe_watch(struct audit_watch *old)
 
 static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watch *w, char *op)
 {
-	if (audit_enabled) {
-		struct audit_buffer *ab;
-		ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
-		if (unlikely(!ab))
-			return;
-		audit_log_format(ab, "auid=%u ses=%u op=%s",
-				 from_kuid(&init_user_ns, audit_get_loginuid(current)),
-				 audit_get_sessionid(current), op);
-		audit_log_format(ab, " path=");
-		audit_log_untrustedstring(ab, w->path);
-		audit_log_key(ab, r->filterkey);
-		audit_log_format(ab, " list=%d res=1", r->listnr);
-		audit_log_end(ab);
-	}
+	struct audit_buffer *ab;
+
+	if (!audit_enabled)
+		return;
+	ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
+	if (!ab)
+		return;
+	audit_log_format(ab, "auid=%u ses=%u op=%s",
+			 from_kuid(&init_user_ns, audit_get_loginuid(current)),
+			 audit_get_sessionid(current), op);
+	audit_log_format(ab, " path=");
+	audit_log_untrustedstring(ab, w->path);
+	audit_log_key(ab, r->filterkey);
+	audit_log_format(ab, " list=%d res=1", r->listnr);
+	audit_log_end(ab);
 }
 
 /* Update inode info in audit rules based on filesystem event. */
-- 
1.8.3.1

Re: [PATCH ghak59 V1 0/2] tree and watch rule log cleanups

[Richard Guy Briggs]

On 2018-06-14 16:20, Richard Guy Briggs wrote:
> Make some tree and watch rule logging cleanups before applying
> normalizations and record connections for ghak 59.
> 
> See: https://github.com/linux-audit/audit-kernel/issues/50

Sorry, this patchset is mislabelled in the subject line and should be
ghak50.

> Richard Guy Briggs (2):
>   audit: tree: check audit_enabled
>   audit: watch: simplify audit_enabled check
> 
>  kernel/audit_tree.c  |  2 ++
>  kernel/audit_watch.c | 29 +++++++++++++++--------------
>  2 files changed, 17 insertions(+), 14 deletions(-)
> 
> -- 
> 1.8.3.1
> 

- 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

Re: [PATCH ghak59 V1 1/2] audit: tree: check audit_enabled

[Paul Moore]

On Thu, Jun 14, 2018 at 4:22 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> Respect the audit_enabled flag when printing tree rule config change
> records.
>
> See: https://github.com/linux-audit/audit-kernel/issues/50
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit_tree.c | 2 ++
>  1 file changed, 2 insertions(+)

Merged, thanks.

> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index 67e6956..5e9d1e5 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -497,6 +497,8 @@ static void audit_tree_log_remove_rule(struct audit_krule *rule)
>  {
>         struct audit_buffer *ab;
>
> +       if (!audit_enabled)
> +               return;
>         ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
>         if (unlikely(!ab))
>                 return;
> --
> 1.8.3.1
>


-- 
paul moore
www.paul-moore.com

Re: [PATCH ghak59 V1 2/2] audit: watch: simplify audit_enabled check

[Paul Moore]

On Thu, Jun 14, 2018 at 4:22 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> Check the audit_enabled flag and bail immediately.  This does not change
> the functionality, but brings the code format in line with similar
> checks in audit_tree_log_remove_rule(), audit_mark_log_rule_change(),
> and elsewhere in the audit code.
>
> See: https://github.com/linux-audit/audit-kernel/issues/50
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit_watch.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)

Merged, thanks.

As a FYI for future patches, please don't use "audit: X: <one-liner>"
as a subject line unless you are crossing subsystem boundaries.  As an
example, the following is okay:

  audit: selinux: make things more awesomer

... while this isn't something I like seeing:

  audit: watch: simplify audit_enabled check

... because the "watch" in this case refers to the audit watch code
which is part of the audit subsystem already.

> diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
> index f1ba889..9b4836b 100644
> --- a/kernel/audit_watch.c
> +++ b/kernel/audit_watch.c
> @@ -238,20 +238,21 @@ static struct audit_watch *audit_dupe_watch(struct audit_watch *old)
>
>  static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watch *w, char *op)
>  {
> -       if (audit_enabled) {
> -               struct audit_buffer *ab;
> -               ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
> -               if (unlikely(!ab))
> -                       return;
> -               audit_log_format(ab, "auid=%u ses=%u op=%s",
> -                                from_kuid(&init_user_ns, audit_get_loginuid(current)),
> -                                audit_get_sessionid(current), op);
> -               audit_log_format(ab, " path=");
> -               audit_log_untrustedstring(ab, w->path);
> -               audit_log_key(ab, r->filterkey);
> -               audit_log_format(ab, " list=%d res=1", r->listnr);
> -               audit_log_end(ab);
> -       }
> +       struct audit_buffer *ab;
> +
> +       if (!audit_enabled)
> +               return;
> +       ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
> +       if (!ab)
> +               return;
> +       audit_log_format(ab, "auid=%u ses=%u op=%s",
> +                        from_kuid(&init_user_ns, audit_get_loginuid(current)),
> +                        audit_get_sessionid(current), op);
> +       audit_log_format(ab, " path=");
> +       audit_log_untrustedstring(ab, w->path);
> +       audit_log_key(ab, r->filterkey);
> +       audit_log_format(ab, " list=%d res=1", r->listnr);
> +       audit_log_end(ab);
>  }
>
>  /* Update inode info in audit rules based on filesystem event. */
> --
> 1.8.3.1
>


-- 
paul moore
www.paul-moore.com

Re: [PATCH ghak59 V1 2/2] audit: watch: simplify audit_enabled check

[Richard Guy Briggs]

On 2018-06-28 11:47, Paul Moore wrote:
> On Thu, Jun 14, 2018 at 4:22 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> > Check the audit_enabled flag and bail immediately.  This does not change
> > the functionality, but brings the code format in line with similar
> > checks in audit_tree_log_remove_rule(), audit_mark_log_rule_change(),
> > and elsewhere in the audit code.
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/50
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  kernel/audit_watch.c | 29 +++++++++++++++--------------
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> 
> Merged, thanks.
> 
> As a FYI for future patches, please don't use "audit: X: <one-liner>"
> as a subject line unless you are crossing subsystem boundaries.  As an
> example, the following is okay:
> 
>   audit: selinux: make things more awesomer
> 
> ... while this isn't something I like seeing:
> 
>   audit: watch: simplify audit_enabled check
> 
> ... because the "watch" in this case refers to the audit watch code
> which is part of the audit subsystem already.

Ok, so that watch keyword should have been used such as:
	"audit: simplify watch audit_enabled check"

I had seen and used it as a sub-sub-system tag rather than an additional
sub-system tag.

Thanks.

> > diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
> > index f1ba889..9b4836b 100644
> > --- a/kernel/audit_watch.c
> > +++ b/kernel/audit_watch.c
> > @@ -238,20 +238,21 @@ static struct audit_watch *audit_dupe_watch(struct audit_watch *old)
> >
> >  static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watch *w, char *op)
> >  {
> > -       if (audit_enabled) {
> > -               struct audit_buffer *ab;
> > -               ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
> > -               if (unlikely(!ab))
> > -                       return;
> > -               audit_log_format(ab, "auid=%u ses=%u op=%s",
> > -                                from_kuid(&init_user_ns, audit_get_loginuid(current)),
> > -                                audit_get_sessionid(current), op);
> > -               audit_log_format(ab, " path=");
> > -               audit_log_untrustedstring(ab, w->path);
> > -               audit_log_key(ab, r->filterkey);
> > -               audit_log_format(ab, " list=%d res=1", r->listnr);
> > -               audit_log_end(ab);
> > -       }
> > +       struct audit_buffer *ab;
> > +
> > +       if (!audit_enabled)
> > +               return;
> > +       ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
> > +       if (!ab)
> > +               return;
> > +       audit_log_format(ab, "auid=%u ses=%u op=%s",
> > +                        from_kuid(&init_user_ns, audit_get_loginuid(current)),
> > +                        audit_get_sessionid(current), op);
> > +       audit_log_format(ab, " path=");
> > +       audit_log_untrustedstring(ab, w->path);
> > +       audit_log_key(ab, r->filterkey);
> > +       audit_log_format(ab, " list=%d res=1", r->listnr);
> > +       audit_log_end(ab);
> >  }
> >
> >  /* Update inode info in audit rules based on filesystem event. */
> > --
> > 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