All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] audit: unswing cap_* fields in PATH records
@ 2017-04-20 17:07 Richard Guy Briggs
  2017-04-21 18:20 ` Serge E. Hallyn
  2017-04-26 19:56 ` Paul Moore
  0 siblings, 2 replies; 5+ messages in thread
From: Richard Guy Briggs @ 2017-04-20 17:07 UTC (permalink / raw)
  To: linux-audit; +Cc: Richard Guy Briggs, Serge E. Hallyn

The cap_* fields swing in and out of PATH records.
If no capabilities are set, the cap_* fields are completely missing and when
one of the cap_fi or cap_fp values is empty, that field is omitted.

Original:
type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL
type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fe=1 cap_fver=2

Normalize the PATH record by always printing all 4 cap_* fields.

Fixed:
type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fi=none cap_fe=1 cap_fver=2

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

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c |   20 ++++----------------
 1 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 10bc2ba..de264d1 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1956,22 +1956,10 @@ void audit_log_cap(struct audit_buffer *ab, char *prefix, kernel_cap_t *cap)
 
 static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
 {
-	kernel_cap_t *perm = &name->fcap.permitted;
-	kernel_cap_t *inh = &name->fcap.inheritable;
-	int log = 0;
-
-	if (!cap_isclear(*perm)) {
-		audit_log_cap(ab, "cap_fp", perm);
-		log = 1;
-	}
-	if (!cap_isclear(*inh)) {
-		audit_log_cap(ab, "cap_fi", inh);
-		log = 1;
-	}
-
-	if (log)
-		audit_log_format(ab, " cap_fe=%d cap_fver=%x",
-				 name->fcap.fE, name->fcap_ver);
+	audit_log_cap(ab, "cap_fp", &name->fcap.permitted);
+	audit_log_cap(ab, "cap_fi", &name->fcap.inheritable);
+	audit_log_format(ab, " cap_fe=%d cap_fver=%x",
+			 name->fcap.fE, name->fcap_ver);
 }
 
 static inline int audit_copy_fcaps(struct audit_names *name,
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] audit: unswing cap_* fields in PATH records
  2017-04-20 17:07 [PATCH] audit: unswing cap_* fields in PATH records Richard Guy Briggs
@ 2017-04-21 18:20 ` Serge E. Hallyn
  2017-04-23  5:42   ` Richard Guy Briggs
  2017-04-26 19:56 ` Paul Moore
  1 sibling, 1 reply; 5+ messages in thread
From: Serge E. Hallyn @ 2017-04-21 18:20 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: Serge E. Hallyn, linux-audit

Quoting Richard Guy Briggs (rgb@redhat.com):
> The cap_* fields swing in and out of PATH records.
> If no capabilities are set, the cap_* fields are completely missing and when
> one of the cap_fi or cap_fp values is empty, that field is omitted.
> 
> Original:
> type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL
> type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fe=1 cap_fver=2
> 
> Normalize the PATH record by always printing all 4 cap_* fields.
> 
> Fixed:
> type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
> type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fi=none cap_fe=1 cap_fver=2
> 
> See: https://github.com/linux-audit/audit-kernel/issues/42
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>

Is this a big problem for the audit daemon?  There's no actual incorrectness
here right?  I'm not completely opposed, but it does seem like a waste of
space in the (overwhelmingly) most common cases.

> ---
>  kernel/audit.c |   20 ++++----------------
>  1 files changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 10bc2ba..de264d1 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1956,22 +1956,10 @@ void audit_log_cap(struct audit_buffer *ab, char *prefix, kernel_cap_t *cap)
>  
>  static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
>  {
> -	kernel_cap_t *perm = &name->fcap.permitted;
> -	kernel_cap_t *inh = &name->fcap.inheritable;
> -	int log = 0;
> -
> -	if (!cap_isclear(*perm)) {
> -		audit_log_cap(ab, "cap_fp", perm);
> -		log = 1;
> -	}
> -	if (!cap_isclear(*inh)) {
> -		audit_log_cap(ab, "cap_fi", inh);
> -		log = 1;
> -	}
> -
> -	if (log)
> -		audit_log_format(ab, " cap_fe=%d cap_fver=%x",
> -				 name->fcap.fE, name->fcap_ver);
> +	audit_log_cap(ab, "cap_fp", &name->fcap.permitted);
> +	audit_log_cap(ab, "cap_fi", &name->fcap.inheritable);
> +	audit_log_format(ab, " cap_fe=%d cap_fver=%x",
> +			 name->fcap.fE, name->fcap_ver);
>  }
>  
>  static inline int audit_copy_fcaps(struct audit_names *name,
> -- 
> 1.7.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] audit: unswing cap_* fields in PATH records
  2017-04-21 18:20 ` Serge E. Hallyn
@ 2017-04-23  5:42   ` Richard Guy Briggs
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Guy Briggs @ 2017-04-23  5:42 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: Serge E. Hallyn, linux-audit

On 2017-04-21 13:20, Serge E. Hallyn wrote:
> Quoting Richard Guy Briggs (rgb@redhat.com):
> > The cap_* fields swing in and out of PATH records.
> > If no capabilities are set, the cap_* fields are completely missing and when
> > one of the cap_fi or cap_fp values is empty, that field is omitted.
> > 
> > Original:
> > type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL
> > type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fe=1 cap_fver=2
> > 
> > Normalize the PATH record by always printing all 4 cap_* fields.
> > 
> > Fixed:
> > type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
> > type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fi=none cap_fe=1 cap_fver=2
> > 
> > See: https://github.com/linux-audit/audit-kernel/issues/42
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> 
> Is this a big problem for the audit daemon?  There's no actual incorrectness
> here right?  I'm not completely opposed, but it does seem like a waste of
> space in the (overwhelmingly) most common cases.

There is no actual incorrectness.  Steve Grubb has been requesting that
audit records be "normalized" so that fields don't swing in and out.
making parsing easier in userspace tools.

I agree it seems a waste of space/bandwidth to include empty fields but
I'm trying to ease things for userspace processing and analysis tools.

> > ---
> >  kernel/audit.c |   20 ++++----------------
> >  1 files changed, 4 insertions(+), 16 deletions(-)
> > 
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 10bc2ba..de264d1 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1956,22 +1956,10 @@ void audit_log_cap(struct audit_buffer *ab, char *prefix, kernel_cap_t *cap)
> >  
> >  static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
> >  {
> > -	kernel_cap_t *perm = &name->fcap.permitted;
> > -	kernel_cap_t *inh = &name->fcap.inheritable;
> > -	int log = 0;
> > -
> > -	if (!cap_isclear(*perm)) {
> > -		audit_log_cap(ab, "cap_fp", perm);
> > -		log = 1;
> > -	}
> > -	if (!cap_isclear(*inh)) {
> > -		audit_log_cap(ab, "cap_fi", inh);
> > -		log = 1;
> > -	}
> > -
> > -	if (log)
> > -		audit_log_format(ab, " cap_fe=%d cap_fver=%x",
> > -				 name->fcap.fE, name->fcap_ver);
> > +	audit_log_cap(ab, "cap_fp", &name->fcap.permitted);
> > +	audit_log_cap(ab, "cap_fi", &name->fcap.inheritable);
> > +	audit_log_format(ab, " cap_fe=%d cap_fver=%x",
> > +			 name->fcap.fE, name->fcap_ver);
> >  }
> >  
> >  static inline int audit_copy_fcaps(struct audit_names *name,
> > -- 
> > 1.7.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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] audit: unswing cap_* fields in PATH records
  2017-04-20 17:07 [PATCH] audit: unswing cap_* fields in PATH records Richard Guy Briggs
  2017-04-21 18:20 ` Serge E. Hallyn
@ 2017-04-26 19:56 ` Paul Moore
  2017-05-23 20:52   ` Paul Moore
  1 sibling, 1 reply; 5+ messages in thread
From: Paul Moore @ 2017-04-26 19:56 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: Serge E. Hallyn, linux-audit

On Thu, Apr 20, 2017 at 1:07 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> The cap_* fields swing in and out of PATH records.
> If no capabilities are set, the cap_* fields are completely missing and when
> one of the cap_fi or cap_fp values is empty, that field is omitted.
>
> Original:
> type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL
> type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fe=1 cap_fver=2
>
> Normalize the PATH record by always printing all 4 cap_* fields.
>
> Fixed:
> type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
> type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fi=none cap_fe=1 cap_fver=2
>
> See: https://github.com/linux-audit/audit-kernel/issues/42
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.c |   20 ++++----------------
>  1 files changed, 4 insertions(+), 16 deletions(-)

FWIW, I agree with the comments from Serge and yourself regarding the
audit noise, but I understand the motivation behind this patch
(limitations in the audit log design).

This patch looks fine to me, but since we are -rc8 right now, and this
isn't critical in any way, I'm going to defer merging this until after
the merge window closes.

Thanks.

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 10bc2ba..de264d1 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1956,22 +1956,10 @@ void audit_log_cap(struct audit_buffer *ab, char *prefix, kernel_cap_t *cap)
>
>  static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
>  {
> -       kernel_cap_t *perm = &name->fcap.permitted;
> -       kernel_cap_t *inh = &name->fcap.inheritable;
> -       int log = 0;
> -
> -       if (!cap_isclear(*perm)) {
> -               audit_log_cap(ab, "cap_fp", perm);
> -               log = 1;
> -       }
> -       if (!cap_isclear(*inh)) {
> -               audit_log_cap(ab, "cap_fi", inh);
> -               log = 1;
> -       }
> -
> -       if (log)
> -               audit_log_format(ab, " cap_fe=%d cap_fver=%x",
> -                                name->fcap.fE, name->fcap_ver);
> +       audit_log_cap(ab, "cap_fp", &name->fcap.permitted);
> +       audit_log_cap(ab, "cap_fi", &name->fcap.inheritable);
> +       audit_log_format(ab, " cap_fe=%d cap_fver=%x",
> +                        name->fcap.fE, name->fcap_ver);
>  }
>
>  static inline int audit_copy_fcaps(struct audit_names *name,
> --
> 1.7.1
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit



-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] audit: unswing cap_* fields in PATH records
  2017-04-26 19:56 ` Paul Moore
@ 2017-05-23 20:52   ` Paul Moore
  0 siblings, 0 replies; 5+ messages in thread
From: Paul Moore @ 2017-05-23 20:52 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: Serge E. Hallyn, linux-audit

On Wed, Apr 26, 2017 at 3:56 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Apr 20, 2017 at 1:07 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> The cap_* fields swing in and out of PATH records.
>> If no capabilities are set, the cap_* fields are completely missing and when
>> one of the cap_fi or cap_fp values is empty, that field is omitted.
>>
>> Original:
>> type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL
>> type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fe=1 cap_fver=2
>>
>> Normalize the PATH record by always printing all 4 cap_* fields.
>>
>> Fixed:
>> type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
>> type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fi=none cap_fe=1 cap_fver=2
>>
>> See: https://github.com/linux-audit/audit-kernel/issues/42
>>
>> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>> ---
>>  kernel/audit.c |   20 ++++----------------
>>  1 files changed, 4 insertions(+), 16 deletions(-)
>
> FWIW, I agree with the comments from Serge and yourself regarding the
> audit noise, but I understand the motivation behind this patch
> (limitations in the audit log design).
>
> This patch looks fine to me, but since we are -rc8 right now, and this
> isn't critical in any way, I'm going to defer merging this until after
> the merge window closes.

Merged.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-05-23 20:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-20 17:07 [PATCH] audit: unswing cap_* fields in PATH records Richard Guy Briggs
2017-04-21 18:20 ` Serge E. Hallyn
2017-04-23  5:42   ` Richard Guy Briggs
2017-04-26 19:56 ` Paul Moore
2017-05-23 20:52   ` Paul Moore

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.