linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH ghak21 V3 0/2] audit: address ANOM_LINK excess records
@ 2018-03-14  5:42 Richard Guy Briggs
  2018-03-14  5:42 ` [PATCH ghak21 V3 1/2] audit: remove path param from link denied function Richard Guy Briggs
  2018-03-14  5:43 ` [PATCH ghak21 V3 2/2] audit: add refused symlink to audit_names Richard Guy Briggs
  0 siblings, 2 replies; 6+ messages in thread
From: Richard Guy Briggs @ 2018-03-14  5:42 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML
  Cc: Eric Paris, Paul Moore, Steve Grubb, Kees Cook, Richard Guy Briggs

This V3 is a supplement to patches 1 and 2 of v1 already merged.

Audit link denied events were being unexpectedly produced in a disjoint
way when audit was disabled, and when they were expected, there were
duplicate PATH records.  This patchset addresses both issues for
symlinks and hardlinks.

This was introduced with
	commit b24a30a7305418ff138ff51776fc555ec57c011a
	("audit: fix event coverage of AUDIT_ANOM_LINK")
	commit a51d9eaa41866ab6b4b6ecad7b621f8b66ece0dc
	("fs: add link restriction audit reporting")

Here are the resulting events:

symlink:
type=PROCTITLE msg=audit(03/12/2018 02:21:49.578:310) : proctitle=ls ./my-passwd 
type=PATH msg=audit(03/12/2018 02:21:49.578:310) : item=0 name=./my-passwd inode=17090 dev=00:27 mode=link,777 ouid=rgb ogid=rgb rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0 
type=CWD msg=audit(03/12/2018 02:21:49.578:310) : cwd=/tmp 
type=SYSCALL msg=audit(03/12/2018 02:21:49.578:310) : arch=x86_64 syscall=stat success=no exit=EACCES(Permission denied) a0=0x7ffd79950dda a1=0x563f658a03c8 a2=0x563f658a03c8 a3=0x79950d00 items=2 ppid=552 pid=629 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=1 comm=ls exe=/usr/bin/ls subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) 
type=ANOM_LINK msg=audit(03/12/2018 02:21:49.578:310) : op=follow_link ppid=552 pid=629 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=1 comm=ls exe=/usr/bin/ls subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 res=no 
----
hardlink:
type=PROCTITLE msg=audit(03/12/2018 02:24:39.813:314) : proctitle=ln test test-ln 
type=PATH msg=audit(03/12/2018 02:24:39.813:314) : item=1 name=/tmp inode=13529 dev=00:27 mode=dir,sticky,777 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype=PARENT cap_fp=none cap_fi=none cap_fe=0 cap_fver=0 
type=PATH msg=audit(03/12/2018 02:24:39.813:314) : item=0 name=test inode=18112 dev=00:27 mode=file,700 ouid=root ogid=root rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0 
type=CWD msg=audit(03/12/2018 02:24:39.813:314) : cwd=/tmp 
type=SYSCALL msg=audit(03/12/2018 02:24:39.813:314) : arch=x86_64 syscall=linkat success=no exit=EPERM(Operation not permitted) a0=0xffffff9c a1=0x7ffccba77629 a2=0xffffff9c a3=0x7ffccba7762e items=2 ppid=605 pid=638 auid=rgb uid=rgb gid=rgb euid=rgb suid=rgb fsuid=rgb egid=rgb sgid=rgb fsgid=rgb tty=pts0 ses=4 comm=ln exe=/usr/bin/ln subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) 
type=ANOM_LINK msg=audit(03/12/2018 02:24:39.813:314) : op=linkat ppid=605 pid=638 auid=rgb uid=rgb gid=rgb euid=rgb suid=rgb fsuid=rgb egid=rgb sgid=rgb fsgid=rgb tty=pts0 ses=4 comm=ln exe=/usr/bin/ln subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 res=no 

See: https://github.com/linux-audit/audit-kernel/issues/21
See also: https://github.com/linux-audit/audit-kernel/issues/51

Changelog:
v3:
- rebase on previously accepted 1/4 and 2/4 patches and drop them
- drop parent record audit_log_symlink_denied()
v2:
- remove now supperfluous struct path * parameter from audit_log_link_denied()
- refactor audit_log_symlink_denied() to properly free memory (pathname, filename)

Richard Guy Briggs (2):
  audit: remove path param from link denied function
  audit: add refused symlink to audit_names

 fs/namei.c            | 3 ++-
 include/linux/audit.h | 6 ++----
 kernel/audit.c        | 3 +--
 3 files changed, 5 insertions(+), 7 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH ghak21 V3 1/2] audit: remove path param from link denied function
  2018-03-14  5:42 [PATCH ghak21 V3 0/2] audit: address ANOM_LINK excess records Richard Guy Briggs
@ 2018-03-14  5:42 ` Richard Guy Briggs
  2018-03-20 20:05   ` Paul Moore
  2018-03-14  5:43 ` [PATCH ghak21 V3 2/2] audit: add refused symlink to audit_names Richard Guy Briggs
  1 sibling, 1 reply; 6+ messages in thread
From: Richard Guy Briggs @ 2018-03-14  5:42 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML
  Cc: Eric Paris, Paul Moore, Steve Grubb, Kees Cook, Richard Guy Briggs

In commit 45b578fe4c3cade6f4ca1fc934ce199afd857edc
("audit: link denied should not directly generate PATH record")
the need for the struct path *link parameter was removed.
Remove the now useless struct path argument.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 fs/namei.c            | 2 +-
 include/linux/audit.h | 6 ++----
 kernel/audit.c        | 3 +--
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 9cc91fb..50d2533 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1011,7 +1011,7 @@ static int may_linkat(struct path *link)
 	if (safe_hardlink_source(inode) || inode_owner_or_capable(inode))
 		return 0;
 
-	audit_log_link_denied("linkat", link);
+	audit_log_link_denied("linkat");
 	return -EPERM;
 }
 
diff --git a/include/linux/audit.h b/include/linux/audit.h
index af410d9..75d5b03 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -146,8 +146,7 @@ extern void		    audit_log_d_path(struct audit_buffer *ab,
 					     const struct path *path);
 extern void		    audit_log_key(struct audit_buffer *ab,
 					  char *key);
-extern void		    audit_log_link_denied(const char *operation,
-						  const struct path *link);
+extern void		    audit_log_link_denied(const char *operation);
 extern void		    audit_log_lost(const char *message);
 
 extern int audit_log_task_context(struct audit_buffer *ab);
@@ -194,8 +193,7 @@ static inline void audit_log_d_path(struct audit_buffer *ab,
 { }
 static inline void audit_log_key(struct audit_buffer *ab, char *key)
 { }
-static inline void audit_log_link_denied(const char *string,
-					 const struct path *link)
+static inline void audit_log_link_denied(const char *string)
 { }
 static inline int audit_log_task_context(struct audit_buffer *ab)
 {
diff --git a/kernel/audit.c b/kernel/audit.c
index 3f2f143..e8bf8d7 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2308,9 +2308,8 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
 /**
  * audit_log_link_denied - report a link restriction denial
  * @operation: specific link operation
- * @link: the path that triggered the restriction
  */
-void audit_log_link_denied(const char *operation, const struct path *link)
+void audit_log_link_denied(const char *operation)
 {
 	struct audit_buffer *ab;
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH ghak21 V3 2/2] audit: add refused symlink to audit_names
  2018-03-14  5:42 [PATCH ghak21 V3 0/2] audit: address ANOM_LINK excess records Richard Guy Briggs
  2018-03-14  5:42 ` [PATCH ghak21 V3 1/2] audit: remove path param from link denied function Richard Guy Briggs
@ 2018-03-14  5:43 ` Richard Guy Briggs
  2018-03-20 20:11   ` Paul Moore
  1 sibling, 1 reply; 6+ messages in thread
From: Richard Guy Briggs @ 2018-03-14  5:43 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML
  Cc: Eric Paris, Paul Moore, Steve Grubb, Kees Cook, Richard Guy Briggs

Audit link denied events for symlinks had duplicate PATH records rather
than just updating the existing PATH record.  Update the symlink's PATH
record with the current dentry and inode information.

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

diff --git a/fs/namei.c b/fs/namei.c
index 50d2533..00f5041 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -945,6 +945,7 @@ static inline int may_follow_link(struct nameidata *nd)
 	if (nd->flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	audit_inode(nd->name, nd->stack[0].link.dentry, 0);
 	audit_log_link_denied("follow_link", &nd->stack[0].link);
 	return -EACCES;
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH ghak21 V3 1/2] audit: remove path param from link denied function
  2018-03-14  5:42 ` [PATCH ghak21 V3 1/2] audit: remove path param from link denied function Richard Guy Briggs
@ 2018-03-20 20:05   ` Paul Moore
  0 siblings, 0 replies; 6+ messages in thread
From: Paul Moore @ 2018-03-20 20:05 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Linux-Audit Mailing List, LKML, Eric Paris, Steve Grubb, Kees Cook

On Wed, Mar 14, 2018 at 1:42 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> In commit 45b578fe4c3cade6f4ca1fc934ce199afd857edc
> ("audit: link denied should not directly generate PATH record")
> the need for the struct path *link parameter was removed.
> Remove the now useless struct path argument.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  fs/namei.c            | 2 +-
>  include/linux/audit.h | 6 ++----
>  kernel/audit.c        | 3 +--
>  3 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 9cc91fb..50d2533 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1011,7 +1011,7 @@ static int may_linkat(struct path *link)
>         if (safe_hardlink_source(inode) || inode_owner_or_capable(inode))
>                 return 0;
>
> -       audit_log_link_denied("linkat", link);
> +       audit_log_link_denied("linkat");
>         return -EPERM;
>  }
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index af410d9..75d5b03 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -146,8 +146,7 @@ extern void             audit_log_d_path(struct audit_buffer *ab,
>                                              const struct path *path);
>  extern void                audit_log_key(struct audit_buffer *ab,
>                                           char *key);
> -extern void                audit_log_link_denied(const char *operation,
> -                                                 const struct path *link);
> +extern void                audit_log_link_denied(const char *operation);
>  extern void                audit_log_lost(const char *message);
>
>  extern int audit_log_task_context(struct audit_buffer *ab);
> @@ -194,8 +193,7 @@ static inline void audit_log_d_path(struct audit_buffer *ab,
>  { }
>  static inline void audit_log_key(struct audit_buffer *ab, char *key)
>  { }
> -static inline void audit_log_link_denied(const char *string,
> -                                        const struct path *link)
> +static inline void audit_log_link_denied(const char *string)
>  { }
>  static inline int audit_log_task_context(struct audit_buffer *ab)
>  {
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 3f2f143..e8bf8d7 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2308,9 +2308,8 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
>  /**
>   * audit_log_link_denied - report a link restriction denial
>   * @operation: specific link operation
> - * @link: the path that triggered the restriction
>   */
> -void audit_log_link_denied(const char *operation, const struct path *link)
> +void audit_log_link_denied(const char *operation)
>  {
>         struct audit_buffer *ab;

*sigh*

You forgot to update may_follow_link().

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH ghak21 V3 2/2] audit: add refused symlink to audit_names
  2018-03-14  5:43 ` [PATCH ghak21 V3 2/2] audit: add refused symlink to audit_names Richard Guy Briggs
@ 2018-03-20 20:11   ` Paul Moore
  2018-03-21  8:47     ` Richard Guy Briggs
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Moore @ 2018-03-20 20:11 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Linux-Audit Mailing List, LKML, Eric Paris, Steve Grubb, Kees Cook

On Wed, Mar 14, 2018 at 1:43 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> Audit link denied events for symlinks had duplicate PATH records rather
> than just updating the existing PATH record.  Update the symlink's PATH
> record with the current dentry and inode information.
>
> See: https://github.com/linux-audit/audit-kernel/issues/21
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  fs/namei.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 50d2533..00f5041 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -945,6 +945,7 @@ static inline int may_follow_link(struct nameidata *nd)
>         if (nd->flags & LOOKUP_RCU)
>                 return -ECHILD;
>
> +       audit_inode(nd->name, nd->stack[0].link.dentry, 0);
>         audit_log_link_denied("follow_link", &nd->stack[0].link);

That's the one, right above this comment ...

Be honest, this never compiled did it?

My confidence for these patches is really low right now, which is not
good for something asking to go in at this stage in the -rcX
development cycle.  There have been some delays due to review, so I'm
willing to be a little flexible on timelines and accept stuff this
week, but for the v4 patchset please provide before and after audit
logs for both hard and soft links both where CWD is the same as the
parent as well as different (eight logs total if I did the math
right).  You can include them in the 0/2 patch as it's probably a bit
much for the individual patches.

>         return -EACCES;
>  }
> --
> 1.8.3.1

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH ghak21 V3 2/2] audit: add refused symlink to audit_names
  2018-03-20 20:11   ` Paul Moore
@ 2018-03-21  8:47     ` Richard Guy Briggs
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Guy Briggs @ 2018-03-21  8:47 UTC (permalink / raw)
  To: Paul Moore
  Cc: Linux-Audit Mailing List, LKML, Eric Paris, Steve Grubb, Kees Cook

On 2018-03-20 16:11, Paul Moore wrote:
> On Wed, Mar 14, 2018 at 1:43 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > Audit link denied events for symlinks had duplicate PATH records rather
> > than just updating the existing PATH record.  Update the symlink's PATH
> > record with the current dentry and inode information.
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/21
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  fs/namei.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/fs/namei.c b/fs/namei.c
> > index 50d2533..00f5041 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -945,6 +945,7 @@ static inline int may_follow_link(struct nameidata *nd)
> >         if (nd->flags & LOOKUP_RCU)
> >                 return -ECHILD;
> >
> > +       audit_inode(nd->name, nd->stack[0].link.dentry, 0);
> >         audit_log_link_denied("follow_link", &nd->stack[0].link);
> 
> That's the one, right above this comment ...
> 
> Be honest, this never compiled did it?

As far as your view is concerned it wasn't.  It was compiled and tested,
but the fix ended up in the following patch in the branch that wasn't
included in this patchset so it effectively failed this bisection test.
The sample records were copied from the test and not cooked.  git add -p
and/or git rebase -i fumble.

> My confidence for these patches is really low right now, which is not
> good for something asking to go in at this stage in the -rcX
> development cycle.  There have been some delays due to review, so I'm
> willing to be a little flexible on timelines and accept stuff this
> week, but for the v4 patchset please provide before and after audit
> logs for both hard and soft links both where CWD is the same as the
> parent as well as different (eight logs total if I did the math
> right).  You can include them in the 0/2 patch as it's probably a bit
> much for the individual patches.

I'm very sorry to have wasted your time.  I know it is precious right now.

> >         return -EACCES;
> >  }
> 
> paul moore

- 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] 6+ messages in thread
end of thread, other threads:[~2018-03-21  8:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14  5:42 [PATCH ghak21 V3 0/2] audit: address ANOM_LINK excess records Richard Guy Briggs
2018-03-14  5:42 ` [PATCH ghak21 V3 1/2] audit: remove path param from link denied function Richard Guy Briggs
2018-03-20 20:05   ` Paul Moore
2018-03-14  5:43 ` [PATCH ghak21 V3 2/2] audit: add refused symlink to audit_names Richard Guy Briggs
2018-03-20 20:11   ` Paul Moore
2018-03-21  8:47     ` Richard Guy Briggs

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).