Skip to contentSkip to conversation

audit: address ANOM_LINK excess records

PATCH Superseded Richard Guy Briggs 4 participants 27 messages Mar 12, 2018 – Mar 14, 2018

Subsystems (2)
VFS Audit

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Richard Guy Briggs

author
#0 lore raw

v2 cover audit: address ANOM_LINK excess records

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=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: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

Richard Guy Briggs (4):
audit: make ANOM_LINK obey audit_enabled and audit_dummy_context
audit: link denied should not directly generate PATH record
audit: add refused symlink to audit_names
audit: add parent of refused symlink to audit_names

 fs/namei.c            |  5 +++--
 include/linux/audit.h |  9 +++++----
 kernel/audit.c        | 43 ++++++++++++++++++++++++++++++++-----------
 3 files changed, 40 insertions(+), 17 deletions(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH v2 1/4 audit: make ANOM_LINK obey audit_enabled and audit_dummy_context

Audit link denied events emit disjointed records when audit is disabled. No records should be emitted when audit is disabled.

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

Signed-off-by Richard Guy Briggs <[email protected]>
---
 kernel/audit.c | 3 +++
 1 file changed, 3 insertions(+)
kernel/audit.c +3 -0
index 1a3e75d..7026d69 100644
@@ -2308,6 +2308,9 @@ void audit_log_link_denied(const char *operation, const struct path *link)
 	struct audit_buffer *ab;
 	struct audit_names *name;
 
+	if (!audit_enabled || audit_dummy_context())
+		return;
+
 	name = kzalloc(sizeof(*name), GFP_NOFS);
 	if (!name)
 		return;
Signature · 1 line
1.8.3.1
maintainer
#2 lore raw

Re: [PATCH ghak21 V2 1/4] audit: make ANOM_LINK obey audit_enabled and audit_dummy_context

Quoted 8 lines · Richard Guy Briggs

Audit link denied events emit disjointed records when audit is disabled. No records should be emitted when audit is disabled.

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

Signed-off-by Richard Guy Briggs <[email protected]>
---
 kernel/audit.c | 3 +++
 1 file changed, 3 insertions(+)

As stated previously, this has already been merged. Ignoring.

Quoted 16 lines
kernel/audit.c +3 -1
index 1a3e75d..7026d69 100644
@@ -2308,6 +2308,9 @@ void audit_log_link_denied(const char *operation, const struct path *link)
        struct audit_buffer *ab;
        struct audit_names *name;
 
+       if (!audit_enabled || audit_dummy_context())
+               return;
+
        name = kzalloc(sizeof(*name), GFP_NOFS);
        if (!name)
                return;
--

1.8.3.1

Signature · 2 lines
paul moore
www.paul-moore.com
author
#3 lore raw

PATCH v2 2/4 audit: link denied should not directly generate PATH record

In reply to Richard Guy Briggs

Audit link denied events generate duplicate PATH records which disagree in different ways from symlink and hardlink denials. audit_log_link_denied() should not directly generate PATH records. While we're at it, remove the now useless struct path argument.

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

Signed-off-by Richard Guy Briggs <[email protected]>
---
 fs/namei.c            |  2 +-
 include/linux/audit.h |  6 ++----
 kernel/audit.c        | 17 ++---------------
 3 files changed, 5 insertions(+), 20 deletions(-)
fs/namei.c +1 -1
index 9cc91fb..50d2533 100644
@@ -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;
 }
include/linux/audit.h +2 -4
index af410d9..75d5b03 100644
@@ -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)
 {
kernel/audit.c +2 -15
index 7026d69..e54deaf 100644
@@ -2301,36 +2301,23 @@ 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;
-	struct audit_names *name;
 
 	if (!audit_enabled || audit_dummy_context())
 		return;
 
-	name = kzalloc(sizeof(*name), GFP_NOFS);
-	if (!name)
-		return;
-
 	/* Generate AUDIT_ANOM_LINK with subject, operation, outcome. */
 	ab = audit_log_start(current->audit_context, GFP_KERNEL,
 			     AUDIT_ANOM_LINK);
 	if (!ab)
-		goto out;
+		return;
 	audit_log_format(ab, "op=%s", operation);
 	audit_log_task_info(ab, current);
 	audit_log_format(ab, " res=0");
 	audit_log_end(ab);
-
-	/* Generate AUDIT_PATH record with object. */
-	name->type = AUDIT_TYPE_NORMAL;
-	audit_copy_inode(name, link->dentry, d_backing_inode(link->dentry));
-	audit_log_name(current->audit_context, name, link, 0, NULL);
-out:
-	kfree(name);
 }
 
 /**
Signature · 1 line
1.8.3.1
maintainer
#4 lore raw

Re: [PATCH ghak21 V2 2/4] audit: link denied should not directly generate PATH record

Quoted 5 lines · Richard Guy Briggs

Audit link denied events generate duplicate PATH records which disagree in different ways from symlink and hardlink denials. audit_log_link_denied() should not directly generate PATH records. While we're at it, remove the now useless struct path argument.

Quoted 7 lines

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

Signed-off-by Richard Guy Briggs <[email protected]>
---
 fs/namei.c            |  2 +-
 include/linux/audit.h |  6 ++----
 kernel/audit.c        | 17 ++---------------
 3 files changed, 5 insertions(+), 20 deletions(-)

I have no objection to the v2 change of removing the link parameter, but this patch can not be merged as-is because the v1 patch has already been merged into audit/next (as stated on the mailing list). You need to respin this patch against audit/next and redo the subject/description to indicate that you are just removing the unused link parameter in this updated patch.

Quoted 83 lines
fs/namei.c +1 -1
index 9cc91fb..50d2533 100644
@@ -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;
 }
include/linux/audit.h +2 -4
index af410d9..75d5b03 100644
@@ -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)
 {
kernel/audit.c +2 -16
index 7026d69..e54deaf 100644
@@ -2301,36 +2301,23 @@ 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;
-       struct audit_names *name;
 
        if (!audit_enabled || audit_dummy_context())
                return;
 
-       name = kzalloc(sizeof(*name), GFP_NOFS);
-       if (!name)
-               return;
-
        /* Generate AUDIT_ANOM_LINK with subject, operation, outcome. */
        ab = audit_log_start(current->audit_context, GFP_KERNEL,
                             AUDIT_ANOM_LINK);
        if (!ab)
-               goto out;
+               return;
        audit_log_format(ab, "op=%s", operation);
        audit_log_task_info(ab, current);
        audit_log_format(ab, " res=0");
        audit_log_end(ab);
-
-       /* Generate AUDIT_PATH record with object. */
-       name->type = AUDIT_TYPE_NORMAL;
-       audit_copy_inode(name, link->dentry, d_backing_inode(link->dentry));
-       audit_log_name(current->audit_context, name, link, 0, NULL);
-out:
-       kfree(name);
 }
 
 /**
--

1.8.3.1

Signature · 2 lines
paul moore
www.paul-moore.com
author
#5 lore raw

Re: [PATCH ghak21 V2 2/4] audit: link denied should not directly generate PATH record

Quoted 14 lines · Paul Moore
Quoted 5 lines · Richard Guy Briggs

Audit link denied events generate duplicate PATH records which disagree in different ways from symlink and hardlink denials. audit_log_link_denied() should not directly generate PATH records. While we're at it, remove the now useless struct path argument.

Quoted 7 lines

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

Signed-off-by Richard Guy Briggs <[email protected]>
---
 fs/namei.c            |  2 +-
 include/linux/audit.h |  6 ++----
 kernel/audit.c        | 17 ++---------------
 3 files changed, 5 insertions(+), 20 deletions(-)
Quoted 3 lines

I have no objection to the v2 change of removing the link parameter, but this patch can not be merged as-is because the v1 patch has already been merged into audit/next (as stated on the mailing list).

Yes, I self-NACKed that patch.

	https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Is it not possible to drop it, or would you have to do a revert to avoid a rebase?

Quoted 3 lines

You need to respin this patch against audit/next and redo the subject/description to indicate that you are just removing the unused link parameter in this updated patch.

So the way I had it in my devel tree rather than squashing it...

Quoted 79 lines
Quoted 79 lines
fs/namei.c +1 -1
index 9cc91fb..50d2533 100644
@@ -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;
 }
include/linux/audit.h +2 -4
index af410d9..75d5b03 100644
@@ -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)
 {
kernel/audit.c +2 -15
index 7026d69..e54deaf 100644
@@ -2301,36 +2301,23 @@ 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;
-       struct audit_names *name;
 
        if (!audit_enabled || audit_dummy_context())
                return;
 
-       name = kzalloc(sizeof(*name), GFP_NOFS);
-       if (!name)
-               return;
-
        /* Generate AUDIT_ANOM_LINK with subject, operation, outcome. */
        ab = audit_log_start(current->audit_context, GFP_KERNEL,
                             AUDIT_ANOM_LINK);
        if (!ab)
-               goto out;
+               return;
        audit_log_format(ab, "op=%s", operation);
        audit_log_task_info(ab, current);
        audit_log_format(ab, " res=0");
        audit_log_end(ab);
-
-       /* Generate AUDIT_PATH record with object. */
-       name->type = AUDIT_TYPE_NORMAL;
-       audit_copy_inode(name, link->dentry, d_backing_inode(link->dentry));
-       audit_log_name(current->audit_context, name, link, 0, NULL);
-out:
-       kfree(name);
 }
Quoted 15 lines
Quoted 4 lines
kernel/audit.c +0 -1
 /**
--

1.8.3.1

Signature · 7 lines
paul moore
www.paul-moore.com

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <[email protected]>
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

maintainer
#6 lore raw

Re: [PATCH ghak21 V2 2/4] audit: link denied should not directly generate PATH record

Quoted 19 lines · Richard Guy Briggs
Quoted 14 lines · Paul Moore
Quoted 5 lines · Richard Guy Briggs

Audit link denied events generate duplicate PATH records which disagree in different ways from symlink and hardlink denials. audit_log_link_denied() should not directly generate PATH records. While we're at it, remove the now useless struct path argument.

Quoted 7 lines

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

Signed-off-by Richard Guy Briggs <[email protected]>
---
 fs/namei.c            |  2 +-
 include/linux/audit.h |  6 ++----
 kernel/audit.c        | 17 ++---------------
 3 files changed, 5 insertions(+), 20 deletions(-)
Quoted 3 lines

I have no objection to the v2 change of removing the link parameter, but this patch can not be merged as-is because the v1 patch has already been merged into audit/next (as stated on the mailing list).

Quoted 6 lines

Yes, I self-NACKed that patch.

        https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Is it not possible to drop it, or would you have to do a revert to avoid a rebase?

Yes, it is possible to drop a patch from the audit/next patch stack, but dropping patches is considered *very* bad form and not something I want to do without a Very Good Reason. While the v2 patch is the "right" patch, the v1 patch is not dangerous, so I would rather you just build on top of what is currently in audit/next.

Signature · 2 lines
paul moore
www.paul-moore.com
build bot
#7 lore raw

Re: [PATCH ghak21 V2 2/4] audit: link denied should not directly generate PATH record

In reply to Richard Guy Briggs
Thank you for the patch! Yet something to improve:

Hi Richard,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180309]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Richard-Guy-Briggs/audit-address-ANOM_LINK-excess-records/20180313-015527
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce:

        # save the attached .config to linux build tree
        make ARCH=i386

Note: the linux-review/Richard-Guy-Briggs/audit-address-ANOM_LINK-excess-records/20180313-015527 HEAD 12e8c56bcd359f7d20d4ae011674d37bc832bc4c builds fine.
It only hurts bisectibility.

All errors (new ones prefixed by >>):

fs/namei.c: In function 'may_follow_link':

>> fs/namei.c:929:2: error: too many arguments to function 'audit_log_link_denied'
     audit_log_link_denied("follow_link", &nd->stack[0].link);
     ^~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/fsnotify.h:16:0,
                    from fs/namei.c:25:

include/linux/audit.h:196:20: note: declared here
static inline void audit_log_link_denied(const char *string)
^~~~~~~~~~~~~~~~~~~~~

vim +/audit_log_link_denied +929 fs/namei.c

800179c9b Kees Cook         2012-07-25  886  
800179c9b Kees Cook         2012-07-25  887  /**
800179c9b Kees Cook         2012-07-25  888   * may_follow_link - Check symlink following for unsafe situations
55852635a Randy Dunlap      2012-08-18  889   * @nd: nameidata pathwalk data
800179c9b Kees Cook         2012-07-25  890   *
800179c9b Kees Cook         2012-07-25  891   * In the case of the sysctl_protected_symlinks sysctl being enabled,
800179c9b Kees Cook         2012-07-25  892   * CAP_DAC_OVERRIDE needs to be specifically ignored if the symlink is
800179c9b Kees Cook         2012-07-25  893   * in a sticky world-writable directory. This is to protect privileged
800179c9b Kees Cook         2012-07-25  894   * processes from failing races against path names that may change out
800179c9b Kees Cook         2012-07-25  895   * from under them by way of other users creating malicious symlinks.
800179c9b Kees Cook         2012-07-25  896   * It will permit symlinks to be followed only when outside a sticky
800179c9b Kees Cook         2012-07-25  897   * world-writable directory, or when the uid of the symlink and follower
800179c9b Kees Cook         2012-07-25  898   * match, or when the directory owner matches the symlink's owner.
800179c9b Kees Cook         2012-07-25  899   *
800179c9b Kees Cook         2012-07-25  900   * Returns 0 if following the symlink is allowed, -ve on error.
800179c9b Kees Cook         2012-07-25  901   */
fec2fa24e Al Viro           2015-05-06  902  static inline int may_follow_link(struct nameidata *nd)
800179c9b Kees Cook         2012-07-25  903  {
800179c9b Kees Cook         2012-07-25  904  	const struct inode *inode;
800179c9b Kees Cook         2012-07-25  905  	const struct inode *parent;
2d7f9e2ad Seth Forshee      2016-04-26  906  	kuid_t puid;
800179c9b Kees Cook         2012-07-25  907  
800179c9b Kees Cook         2012-07-25  908  	if (!sysctl_protected_symlinks)
800179c9b Kees Cook         2012-07-25  909  		return 0;
800179c9b Kees Cook         2012-07-25  910  
800179c9b Kees Cook         2012-07-25  911  	/* Allowed if owner and follower match. */
fceef393a Al Viro           2015-12-29  912  	inode = nd->link_inode;
81abe27b1 Eric W. Biederman 2012-08-03  913  	if (uid_eq(current_cred()->fsuid, inode->i_uid))
800179c9b Kees Cook         2012-07-25  914  		return 0;
800179c9b Kees Cook         2012-07-25  915  
800179c9b Kees Cook         2012-07-25  916  	/* Allowed if parent directory not sticky and world-writable. */
aa65fa35b Al Viro           2015-08-04  917  	parent = nd->inode;
800179c9b Kees Cook         2012-07-25  918  	if ((parent->i_mode & (S_ISVTX|S_IWOTH)) != (S_ISVTX|S_IWOTH))
800179c9b Kees Cook         2012-07-25  919  		return 0;
800179c9b Kees Cook         2012-07-25  920  
800179c9b Kees Cook         2012-07-25  921  	/* Allowed if parent directory and link owner match. */
2d7f9e2ad Seth Forshee      2016-04-26  922  	puid = parent->i_uid;
2d7f9e2ad Seth Forshee      2016-04-26  923  	if (uid_valid(puid) && uid_eq(puid, inode->i_uid))
800179c9b Kees Cook         2012-07-25  924  		return 0;
800179c9b Kees Cook         2012-07-25  925  
31956502d Al Viro           2015-05-07  926  	if (nd->flags & LOOKUP_RCU)
31956502d Al Viro           2015-05-07  927  		return -ECHILD;
31956502d Al Viro           2015-05-07  928  
1cf2665b5 Al Viro           2015-05-06 @929  	audit_log_link_denied("follow_link", &nd->stack[0].link);
800179c9b Kees Cook         2012-07-25  930  	return -EACCES;
800179c9b Kees Cook         2012-07-25  931  }
800179c9b Kees Cook         2012-07-25  932

:::::: The code at line 929 was first introduced by commit :::::: 1cf2665b5bdfc63185fb4a416bff54b14ad30c79 namei: kill nd->link

:::::: TO: Al Viro <[email protected]> :::::: CC: Al Viro <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

author
#8 lore raw

Re: [PATCH ghak21 V2 2/4] audit: link denied should not directly generate PATH record

Quoted 7 lines · kbuild test robot

Hi Richard,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180309]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

My fault. It was applied to a stale tree:
git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git

A self-NACKed patch wasn't removed from the upstream tree as hoped. A new patch is already in the works.

Quoted 3 lines

url: https://github.com/0day-ci/linux/commits/Richard-Guy-Briggs/audit-address-ANOM_LINK-excess-records/20180313-015527
Note: the linux-review/Richard-Guy-Briggs/audit-address-ANOM_LINK-excess-records/20180313-015527 HEAD 12e8c56bcd359f7d20d4ae011674d37bc832bc4c builds fine.
It only hurts bisectibility.

- RGB

--
Richard Guy Briggs <[email protected]>
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

author
#9 lore raw

PATCH v2 3/4 audit: add refused symlink to audit_names

In reply to 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 <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)
fs/namei.c +1 -0
index 50d2533..00f5041 100644
@@ -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;
 }
Signature · 1 line
1.8.3.1
maintainer
#10 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

Quoted 9 lines · 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 <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)

Why didn't you include this in patch 4/4 like I asked during the previous review?

Quoted 14 lines
fs/namei.c +1 -1
index 50d2533..00f5041 100644
@@ -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

Signature · 2 lines
paul moore
www.paul-moore.com
author
#11 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

Quoted 5 lines · Paul Moore
Quoted 4 lines · 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.

Quoted 8 lines
Quoted 5 lines

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

Signed-off-by Richard Guy Briggs <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)

Why didn't you include this in patch 4/4 like I asked during the previous review?

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Quoted 8 lines
Quoted 8 lines
fs/namei.c +0 -0
index 50d2533..00f5041 100644
@@ -945,6 +945,7 @@ static inline int may_follow_link(struct nameidata *nd)
        if (nd->flags & LOOKUP_RCU)
                return -ECHILD;
Quoted 6 lines

fs/namei.c static inline int may_follow_link(struct nameidata *nd) · line 951

Quoted 4 lines
fs/namei.c +1 -0
+       audit_inode(nd->name, nd->stack[0].link.dentry, 0);
        audit_log_link_denied("follow_link", &nd->stack[0].link);
        return -EACCES;
 }

paul moore

- RGB

--
Richard Guy Briggs <[email protected]>
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

maintainer
#12 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

Quoted 12 lines · Richard Guy Briggs
Quoted 11 lines · Paul Moore
Quoted 9 lines · 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 <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)
Quoted 5 lines
Quoted 2 lines

Why didn't you include this in patch 4/4 like I asked during the previous review?

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Yes, I just saw that ... I hadn't seen your replies on the v1 patches until I had finished reviewing v2. I just replied to that mail in the v1 thread, but basically you need to figure out what is necessary here and let us know. If I have to figure it out it likely isn't going to get done with enough soak time prior to the upcoming merge window.

Quoted 23 lines
Quoted 8 lines
Quoted 8 lines
fs/namei.c +0 -0
index 50d2533..00f5041 100644
@@ -945,6 +945,7 @@ static inline int may_follow_link(struct nameidata *nd)
        if (nd->flags & LOOKUP_RCU)
                return -ECHILD;
Quoted 6 lines

fs/namei.c static inline int may_follow_link(struct nameidata *nd) · line 951

Quoted 4 lines
fs/namei.c +1 -0
+       audit_inode(nd->name, nd->stack[0].link.dentry, 0);
        audit_log_link_denied("follow_link", &nd->stack[0].link);
        return -EACCES;
 }

paul moore

- RGB

--
Richard Guy Briggs <[email protected]>
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

Signature · 2 lines
paul moore
www.paul-moore.com
author
#13 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

Quoted 16 lines · Paul Moore
Quoted 15 lines · Richard Guy Briggs
Quoted 13 lines · Paul Moore
Quoted 9 lines · 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 <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)

Why didn't you include this in patch 4/4 like I asked during the previous review?

Quoted 8 lines
Quoted 2 lines

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Yes, I just saw that ... I hadn't seen your replies on the v1 patches until I had finished reviewing v2. I just replied to that mail in the v1 thread, but basically you need to figure out what is necessary here and let us know. If I have to figure it out it likely isn't going to get done with enough soak time prior to the upcoming merge window.

Steve? I was hoping you could chime in here.

I'd just include it for completeness unless Steve thinks it will stand on its own and doesn't want the overhead.

Quoted 15 lines
Quoted 15 lines
Quoted 14 lines

fs/namei.c static inline int may_follow_link(struct nameidata *nd) · line 951

Quoted 12 lines
fs/namei.c +1 -0
index 50d2533..00f5041 100644
@@ -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;
 }

paul moore

Quoted 3 lines
Quoted 1 line

- RGB

paul moore

- RGB

--
Richard Guy Briggs <[email protected]>
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

maintainer
#14 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

Quoted 26 lines · Richard Guy Briggs
Quoted 16 lines · Paul Moore
Quoted 15 lines · Richard Guy Briggs
Quoted 13 lines · Paul Moore
Quoted 9 lines · 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 <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)

Why didn't you include this in patch 4/4 like I asked during the previous review?

Quoted 8 lines
Quoted 2 lines

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Yes, I just saw that ... I hadn't seen your replies on the v1 patches until I had finished reviewing v2. I just replied to that mail in the v1 thread, but basically you need to figure out what is necessary here and let us know. If I have to figure it out it likely isn't going to get done with enough soak time prior to the upcoming merge window.

Quoted 4 lines

Steve? I was hoping you could chime in here.

I'd just include it for completeness unless Steve thinks it will stand on its own and doesn't want the overhead.

If that's the argument, I'd rather just include it. We've been burned by not including stuff in the past and fixing those omissions has proven to be a major source of contention.

Signature · 2 lines
paul moore
www.paul-moore.com
#15 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

In reply to Richard Guy Briggs
Quoted 23 lines · On Mon, 12 Mar 2018 11:52:56 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 22 lines · Paul Moore
Quoted 19 lines · Richard Guy Briggs
Quoted 7 lines · Paul Moore
Quoted 5 lines · 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.

Quoted 8 lines
Quoted 5 lines

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

Signed-off-by Richard Guy Briggs <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)

Why didn't you include this in patch 4/4 like I asked during the previous review?

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Quoted 8 lines
Quoted 6 lines

Yes, I just saw that ... I hadn't seen your replies on the v1 patches until I had finished reviewing v2. I just replied to that mail in the v1 thread, but basically you need to figure out what is necessary here and let us know. If I have to figure it out it likely isn't going to get done with enough soak time prior to the upcoming merge window.

Steve? I was hoping you could chime in here.

If the CWD record will always be the same as the PARENT record, then we do not need the parent record. Duplicate information is bad. Like all the duplicate SYSCALL information.

-Steve

Quoted 35 lines

I'd just include it for completeness unless Steve thinks it will stand on its own and doesn't want the overhead.

Quoted 15 lines
Quoted 15 lines
Quoted 14 lines

fs/namei.c static inline int may_follow_link(struct · line 951

Quoted 12 lines
fs/namei.c +1 -0
index 50d2533..00f5041 100644
@@ -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;
 }

paul moore

Quoted 3 lines
Quoted 1 line

- RGB

paul moore

- RGB

--
Richard Guy Briggs <[email protected]>
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

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

author
#16 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

Quoted 33 lines · Steve Grubb
Quoted 30 lines · On Mon, 12 Mar 2018 11:52:56 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 28 lines · Paul Moore
Quoted 14 lines · Richard Guy Briggs
Quoted 13 lines · Paul Moore
Quoted 10 lines · 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 <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)
Quoted 5 lines
Quoted 2 lines

Why didn't you include this in patch 4/4 like I asked during the previous review?

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Yes, I just saw that ... I hadn't seen your replies on the v1 patches until I had finished reviewing v2. I just replied to that mail in the v1 thread, but basically you need to figure out what is necessary here and let us know. If I have to figure it out it likely isn't going to get done with enough soak time prior to the upcoming merge window.

Quoted 5 lines
Quoted 1 line

Steve? I was hoping you could chime in here.

If the CWD record will always be the same as the PARENT record, then we do not need the parent record. Duplicate information is bad. Like all the duplicate SYSCALL information.

The CWD record could be different from the PARENT record, since I could have SYMLINK=/tmp/test/symlink, CWD=/tmp, PARENT=/tmp/test. Does the parent record even matter since it might not be a directory operation like creat, unlink or rename?

Quoted 25 lines

-Steve

Quoted 23 lines

I'd just include it for completeness unless Steve thinks it will stand on its own and doesn't want the overhead.

Quoted 15 lines
Quoted 15 lines
Quoted 14 lines

fs/namei.c static inline int may_follow_link(struct · line 951

Quoted 12 lines
fs/namei.c +1 -0
index 50d2533..00f5041 100644
@@ -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;
 }

paul moore

Quoted 3 lines
Quoted 1 line

- RGB

paul moore

- RGB

- RGB

--
Richard Guy Briggs <[email protected]>
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

#17 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

Quoted 36 lines · On Tue, 13 Mar 2018 06:11:08 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 35 lines · Steve Grubb
Quoted 31 lines · On Mon, 12 Mar 2018 11:52:56 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 22 lines · Paul Moore
Quoted 14 lines · Richard Guy Briggs
Quoted 13 lines · Paul Moore
Quoted 10 lines · 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 <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)
Quoted 5 lines
Quoted 2 lines

Why didn't you include this in patch 4/4 like I asked during the previous review?

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Quoted 6 lines

Yes, I just saw that ... I hadn't seen your replies on the v1 patches until I had finished reviewing v2. I just replied to that mail in the v1 thread, but basically you need to figure out what is necessary here and let us know. If I have to figure it out it likely isn't going to get done with enough soak time prior to the upcoming merge window.

Steve? I was hoping you could chime in here.

Quoted 8 lines
Quoted 3 lines

If the CWD record will always be the same as the PARENT record, then we do not need the parent record. Duplicate information is bad. Like all the duplicate SYSCALL information.

The CWD record could be different from the PARENT record, since I could have SYMLINK=/tmp/test/symlink, CWD=/tmp, PARENT=/tmp/test. Does the parent record even matter since it might not be a directory operation like creat, unlink or rename?

There's 2 issues. One is creating the path if what we have is relative. In this situation CWD should be enough. But if the question is whether the PARENT directory should be included...what if the PARENT permissions do not allow the successful name resolution? In that case we might only get a PARENT record no? In that case we would need it.

-Steve

Quoted 32 lines
Quoted 18 lines
Quoted 18 lines

I'd just include it for completeness unless Steve thinks it will stand on its own and doesn't want the overhead.

Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
Quoted 8 lines
fs/namei.c +0 -0
index 50d2533..00f5041 100644
@@ -945,6 +945,7 @@ static inline int

may_follow_link(struct nameidata *nd) if (nd->flags & LOOKUP_RCU) return -ECHILD;

Quoted 4 lines

+ audit_inode(nd->name, nd->stack[0].link.dentry, 0);
audit_log_link_denied("follow_link",
&nd->stack[0].link); return -EACCES;
}

paul moore

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

- RGB

paul moore

- RGB

- RGB

--
Richard Guy Briggs <[email protected]>
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

author
#18 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

Quoted 48 lines · Steve Grubb
Quoted 36 lines · On Tue, 13 Mar 2018 06:11:08 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 35 lines · Steve Grubb
Quoted 31 lines · On Mon, 12 Mar 2018 11:52:56 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 22 lines · Paul Moore
Quoted 14 lines · Richard Guy Briggs
Quoted 13 lines · Paul Moore
Quoted 10 lines · 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 <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)
Quoted 5 lines
Quoted 2 lines

Why didn't you include this in patch 4/4 like I asked during the previous review?

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Quoted 6 lines

Yes, I just saw that ... I hadn't seen your replies on the v1 patches until I had finished reviewing v2. I just replied to that mail in the v1 thread, but basically you need to figure out what is necessary here and let us know. If I have to figure it out it likely isn't going to get done with enough soak time prior to the upcoming merge window.

Steve? I was hoping you could chime in here.

Quoted 8 lines
Quoted 3 lines

If the CWD record will always be the same as the PARENT record, then we do not need the parent record. Duplicate information is bad. Like all the duplicate SYSCALL information.

The CWD record could be different from the PARENT record, since I could have SYMLINK=/tmp/test/symlink, CWD=/tmp, PARENT=/tmp/test. Does the parent record even matter since it might not be a directory operation like creat, unlink or rename?

Quoted 5 lines

There's 2 issues. One is creating the path if what we have is relative. In this situation CWD should be enough. But if the question is whether the PARENT directory should be included...what if the PARENT permissions do not allow the successful name resolution? In that case we might only get a PARENT record no? In that case we would need it.

I think in the case of symlink creation, normal file create code path would be in effect, and would properly log parent and symlink source file paths (if a rule to log it was in effect) which is not something that would trigger a symlink link denied error. Symlink link denied happens only when trying to actually follow the link before resolving the target path of a read/write/exec of the symlink target.

If the parent permissions of the link's target don't allow successful name resolution then the symlink link denied condition isn't met, but rather any other rule that applies to the target path.

Quoted 35 lines

-Steve

Quoted 32 lines
Quoted 18 lines
Quoted 18 lines

I'd just include it for completeness unless Steve thinks it will stand on its own and doesn't want the overhead.

Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
Quoted 8 lines
fs/namei.c +0 -0
index 50d2533..00f5041 100644
@@ -945,6 +945,7 @@ static inline int

may_follow_link(struct nameidata *nd) if (nd->flags & LOOKUP_RCU) return -ECHILD;

Quoted 4 lines

+ audit_inode(nd->name, nd->stack[0].link.dentry, 0);
audit_log_link_denied("follow_link",
&nd->stack[0].link); return -EACCES;
}

paul moore

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

- RGB

paul moore

- RGB

- RGB

--
Richard Guy Briggs <[email protected]>
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

- RGB

--
Richard Guy Briggs <[email protected]>
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

#19 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

Quoted 64 lines · On Tue, 13 Mar 2018 06:52:51 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 49 lines · Steve Grubb
Quoted 37 lines · On Tue, 13 Mar 2018 06:11:08 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 36 lines · Steve Grubb
Quoted 32 lines · On Mon, 12 Mar 2018 11:52:56 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 22 lines · Paul Moore
Quoted 14 lines · Richard Guy Briggs
Quoted 13 lines · Paul Moore
Quoted 10 lines · 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 <[email protected]> ---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)
Quoted 5 lines
Quoted 2 lines

Why didn't you include this in patch 4/4 like I asked during the previous review?

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Quoted 7 lines

Yes, I just saw that ... I hadn't seen your replies on the v1 patches until I had finished reviewing v2. I just replied to that mail in the v1 thread, but basically you need to figure out what is necessary here and let us know. If I have to figure it out it likely isn't going to get done with enough soak time prior to the upcoming merge window.

Steve? I was hoping you could chime in here.

Quoted 8 lines
Quoted 3 lines

If the CWD record will always be the same as the PARENT record, then we do not need the parent record. Duplicate information is bad. Like all the duplicate SYSCALL information.

The CWD record could be different from the PARENT record, since I could have SYMLINK=/tmp/test/symlink, CWD=/tmp, PARENT=/tmp/test. Does the parent record even matter since it might not be a directory operation like creat, unlink or rename?

Quoted 6 lines

There's 2 issues. One is creating the path if what we have is relative. In this situation CWD should be enough. But if the question is whether the PARENT directory should be included...what if the PARENT permissions do not allow the successful name resolution? In that case we might only get a PARENT record no? In that case we would need it.

I think in the case of symlink creation, normal file create code path would be in effect, and would properly log parent and symlink source file paths (if a rule to log it was in effect) which is not something that would trigger a symlink link denied error. Symlink link denied happens only when trying to actually follow the link before resolving the target path of a read/write/exec of the symlink target.

Quoted 3 lines

If the parent permissions of the link's target don't allow successful name resolution then the symlink link denied condition isn't met, but rather any other rule that applies to the target path.

Then I guess the PARENT record is not needed.

-Steve

Quoted 42 lines
Quoted 24 lines
Quoted 24 lines
Quoted 23 lines
Quoted 23 lines

I'd just include it for completeness unless Steve thinks it will stand on its own and doesn't want the overhead.

Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
Quoted 8 lines
fs/namei.c +0 -0
index 50d2533..00f5041 100644
@@ -945,6 +945,7 @@ static inline int

may_follow_link(struct nameidata *nd) if (nd->flags & LOOKUP_RCU) return -ECHILD;

Quoted 4 lines

+ audit_inode(nd->name,
nd->stack[0].link.dentry, 0);
audit_log_link_denied("follow_link",
&nd->stack[0].link); return -EACCES; }

paul moore

Quoted 3 lines
Quoted 1 line

- RGB

paul moore

- RGB

Quoted 9 lines
Quoted 8 lines

- RGB

--
Richard Guy Briggs <[email protected]>
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

- RGB

--
Richard Guy Briggs <[email protected]>
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

maintainer
#20 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

In reply to Richard Guy Briggs
Quoted 62 lines · Richard Guy Briggs
Quoted 48 lines · Steve Grubb
Quoted 36 lines · On Tue, 13 Mar 2018 06:11:08 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 35 lines · Steve Grubb
Quoted 31 lines · On Mon, 12 Mar 2018 11:52:56 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 22 lines · Paul Moore
Quoted 14 lines · Richard Guy Briggs
Quoted 13 lines · Paul Moore
Quoted 10 lines · 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 <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)
Quoted 5 lines
Quoted 2 lines

Why didn't you include this in patch 4/4 like I asked during the previous review?

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Quoted 6 lines

Yes, I just saw that ... I hadn't seen your replies on the v1 patches until I had finished reviewing v2. I just replied to that mail in the v1 thread, but basically you need to figure out what is necessary here and let us know. If I have to figure it out it likely isn't going to get done with enough soak time prior to the upcoming merge window.

Steve? I was hoping you could chime in here.

Quoted 8 lines
Quoted 3 lines

If the CWD record will always be the same as the PARENT record, then we do not need the parent record. Duplicate information is bad. Like all the duplicate SYSCALL information.

The CWD record could be different from the PARENT record, since I could have SYMLINK=/tmp/test/symlink, CWD=/tmp, PARENT=/tmp/test. Does the parent record even matter since it might not be a directory operation like creat, unlink or rename?

Quoted 5 lines

There's 2 issues. One is creating the path if what we have is relative. In this situation CWD should be enough. But if the question is whether the PARENT directory should be included...what if the PARENT permissions do not allow the successful name resolution? In that case we might only get a PARENT record no? In that case we would need it.

I think in the case of symlink creation, normal file create code path would be in effect, and would properly log parent and symlink source file paths (if a rule to log it was in effect) which is not something that would trigger a symlink link denied error. Symlink link denied happens only when trying to actually follow the link before resolving the target path of a read/write/exec of the symlink target.

Quoted 3 lines

If the parent permissions of the link's target don't allow successful name resolution then the symlink link denied condition isn't met, but rather any other rule that applies to the target path.

I'm guessing you are in the process of tracking all this down, but if not, lets get to a point where we can answer this definitively and not guess :)

Signature · 2 lines
paul moore
www.paul-moore.com
author
#21 lore raw

Re: [PATCH ghak21 V2 3/4] audit: add refused symlink to audit_names

Quoted 63 lines · Paul Moore
Quoted 62 lines · Richard Guy Briggs
Quoted 48 lines · Steve Grubb
Quoted 36 lines · On Tue, 13 Mar 2018 06:11:08 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 35 lines · Steve Grubb
Quoted 31 lines · On Mon, 12 Mar 2018 11:52:56 -0400 Richard Guy Briggs <[email protected]> wrote:
Quoted 22 lines · Paul Moore
Quoted 14 lines · Richard Guy Briggs
Quoted 13 lines · Paul Moore
Quoted 10 lines · 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 <[email protected]>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)
Quoted 5 lines
Quoted 2 lines

Why didn't you include this in patch 4/4 like I asked during the previous review?

Please see the last comment of:
https://www.redhat.com/archives/linux-audit/2018-March/msg00070.html

Quoted 6 lines

Yes, I just saw that ... I hadn't seen your replies on the v1 patches until I had finished reviewing v2. I just replied to that mail in the v1 thread, but basically you need to figure out what is necessary here and let us know. If I have to figure it out it likely isn't going to get done with enough soak time prior to the upcoming merge window.

Steve? I was hoping you could chime in here.

Quoted 8 lines
Quoted 3 lines

If the CWD record will always be the same as the PARENT record, then we do not need the parent record. Duplicate information is bad. Like all the duplicate SYSCALL information.

The CWD record could be different from the PARENT record, since I could have SYMLINK=/tmp/test/symlink, CWD=/tmp, PARENT=/tmp/test. Does the parent record even matter since it might not be a directory operation like creat, unlink or rename?

Quoted 5 lines

There's 2 issues. One is creating the path if what we have is relative. In this situation CWD should be enough. But if the question is whether the PARENT directory should be included...what if the PARENT permissions do not allow the successful name resolution? In that case we might only get a PARENT record no? In that case we would need it.

I think in the case of symlink creation, normal file create code path would be in effect, and would properly log parent and symlink source file paths (if a rule to log it was in effect) which is not something that would trigger a symlink link denied error. Symlink link denied happens only when trying to actually follow the link before resolving the target path of a read/write/exec of the symlink target.

Quoted 7 lines
Quoted 3 lines

If the parent permissions of the link's target don't allow successful name resolution then the symlink link denied condition isn't met, but rather any other rule that applies to the target path.

I'm guessing you are in the process of tracking all this down, but if not, lets get to a point where we can answer this definitively and not guess :)

I was fairly certain but being polite, expecting confirmation or possibly correction if I've overlooked something.

Additionally, this denial message only happens in certain parts of the permission check for symlinks:
/proc/sys/fs/protected_symlinks == 1
and follower and link owner don't match
and parent sticky and world-writable
and link parent and link owner don't match

If you want other symlink denials logged, you need to set a rule for the target filtering on operation failure such as unix file permissions.

The similar situation exists for hardlinks.

Quoted 1 line

paul moore

- RGB

--
Richard Guy Briggs <[email protected]>
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

author
#22 lore raw

PATCH v2 4/4 audit: add parent of refused symlink to audit_names

In reply to Richard Guy Briggs

Audit link denied events for symlinks were missing the parent PATH record. Add it. Since the full pathname may not be available, reconstruct it from the path in the nameidata supplied.

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

Signed-off-by Richard Guy Briggs <[email protected]>
---
 fs/namei.c            |  2 +-
 include/linux/audit.h |  3 +++
 kernel/audit.c        | 31 +++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)
fs/namei.c +1 -1
index 00f5041..2f39617 100644
@@ -946,7 +946,7 @@ static inline int may_follow_link(struct nameidata *nd)
 		return -ECHILD;
 
 	audit_inode(nd->name, nd->stack[0].link.dentry, 0);
-	audit_log_link_denied("follow_link", &nd->stack[0].link);
+	audit_log_symlink_denied(&nd->stack[0].link);
 	return -EACCES;
 }
include/linux/audit.h +3 -0
index 75d5b03..b5808e9 100644
@@ -147,6 +147,7 @@ extern void		    audit_log_d_path(struct audit_buffer *ab,
 extern void		    audit_log_key(struct audit_buffer *ab,
 					  char *key);
 extern void		    audit_log_link_denied(const char *operation);
+extern void		    audit_log_symlink_denied(const struct path *link);
 extern void		    audit_log_lost(const char *message);
 
 extern int audit_log_task_context(struct audit_buffer *ab);

@@ -195,6 +196,8 @@ static inline void audit_log_key(struct audit_buffer *ab, char *key)
 { }
 static inline void audit_log_link_denied(const char *string)
 { }
+static inline void audit_log_symlink_denied(const struct path *link)
+{ }
 static inline int audit_log_task_context(struct audit_buffer *ab)
 {
 	return 0;
kernel/audit.c +31 -0
index e54deaf..4acf374 100644
@@ -73,6 +73,7 @@
 #include <linux/freezer.h>
 #include <linux/pid_namespace.h>
 #include <net/netns/generic.h>
+#include <linux/namei.h> /* for LOOKUP_PARENT */
 
 #include "audit.h"
 

@@ -2320,6 +2321,36 @@ void audit_log_link_denied(const char *operation)
 	audit_log_end(ab);
 }
 
+/*
+ * audit_log_symlink_denied - report a symlink restriction denial
+ * @link: the path that triggered the restriction
+ */
+void audit_log_symlink_denied(const struct path *link)
+{
+	char *pathname;
+	struct filename *filename;
+
+	if (audit_dummy_context())
+		return;
+
+	pathname = kmalloc(PATH_MAX + 1, GFP_KERNEL);
+	if (!pathname) {
+		audit_panic("memory allocation error while reporting symlink denied");
+		return;
+	}
+	filename = getname_kernel(d_absolute_path(link, pathname, PATH_MAX + 1));
+	if (IS_ERR(filename)) {
+		audit_panic("error getting pathname while reporting symlink denied");
+		goto out;
+	}
+	audit_inode(filename, link->dentry->d_parent, LOOKUP_PARENT);
+	audit_log_link_denied("follow_link");
+	putname(filename);
+out:
+	kfree(pathname);
+	return;
+}
+
 /**
  * audit_log_end - end one audit record
  * @ab: the audit_buffer
Signature · 1 line
1.8.3.1
maintainer
#23 lore raw

Re: [PATCH ghak21 V2 4/4] audit: add parent of refused symlink to audit_names

Quoted 11 lines · Richard Guy Briggs

Audit link denied events for symlinks were missing the parent PATH record. Add it. Since the full pathname may not be available, reconstruct it from the path in the nameidata supplied.

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

Signed-off-by Richard Guy Briggs <[email protected]>
---
 fs/namei.c            |  2 +-
 include/linux/audit.h |  3 +++
 kernel/audit.c        | 31 +++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

See my comment in patch 3/4; it should really be folded into this patch. Additional comment inline below ...

Quoted 29 lines
kernel/audit.c +14 -0
index e54deaf..4acf374 100644
@@ -73,6 +73,7 @@
 #include <linux/freezer.h>
 #include <linux/pid_namespace.h>
 #include <net/netns/generic.h>
+#include <linux/namei.h> /* for LOOKUP_PARENT */
 
 #include "audit.h"
 

@@ -2320,6 +2321,36 @@ void audit_log_link_denied(const char *operation)
        audit_log_end(ab);
 }
 
+/*
+ * audit_log_symlink_denied - report a symlink restriction denial
+ * @link: the path that triggered the restriction
+ */
+void audit_log_symlink_denied(const struct path *link)
+{
+       char *pathname;
+       struct filename *filename;
+
+       if (audit_dummy_context())
+               return;
+
+       pathname = kmalloc(PATH_MAX + 1, GFP_KERNEL);

kernel/audit.c void audit_log_link_denied(const char *operation) · line 2346

Quoted 10 lines
kernel/audit.c +10 -0
+       if (!pathname) {
+               audit_panic("memory allocation error while reporting symlink denied");
+               return;
+       }
+       filename = getname_kernel(d_absolute_path(link, pathname, PATH_MAX + 1));
+       if (IS_ERR(filename)) {
+               audit_panic("error getting pathname while reporting symlink denied");
+               goto out;
+       }
+       audit_inode(filename, link->dentry->d_parent, LOOKUP_PARENT);

Since we are already checking audit_dummy_context() above we don't need to check it again in audit_inode(), you should just call __audit_inode() directly. As a reminder, make sure you convert LOOKUP_PARENT to AUDIT_INODE_PARENT.

Quoted 6 lines
kernel/audit.c +6 -0
+       audit_log_link_denied("follow_link");
+       putname(filename);
+out:
+       kfree(pathname);
+       return;
+}
Signature · 2 lines
paul moore
www.paul-moore.com
author
#24 lore raw
In reply to Richard Guy Briggs
Quoted 38 lines · Richard Guy Briggs

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=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: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

Richard Guy Briggs (4):
audit: make ANOM_LINK obey audit_enabled and audit_dummy_context
audit: link denied should not directly generate PATH record
audit: add refused symlink to audit_names
audit: add parent of refused symlink to audit_names

Quoted 6 lines
 fs/namei.c            |  5 +++--
 include/linux/audit.h |  9 +++++----
 kernel/audit.c        | 43 ++++++++++++++++++++++++++++++++-----------
 3 files changed, 40 insertions(+), 17 deletions(-)

Changelog:
v2:
- remove now supperfluous struct path * parameter from audit_log_link_denied()
- refactor audit_log_symlink_denied() to properly free memory (pathname, filename)

Quoted 2 lines

1.8.3.1

- RGB

--
Richard Guy Briggs <[email protected]>
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

#25 lore raw
In reply to Richard Guy Briggs
Quoted 27 lines · On Mon, 12 Mar 2018 02:31:16 -0400 Richard Guy Briggs <[email protected]> wrote:

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=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: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

Quoted 8 lines

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

So, if we now only emit the ANOM_LINK event when audit is enabled, we should get rid of all the duplicate information in that record. The SYSCALL record has all that information.

-Steve

Quoted 38 lines

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

Richard Guy Briggs (4):
audit: make ANOM_LINK obey audit_enabled and audit_dummy_context
audit: link denied should not directly generate PATH record
audit: add refused symlink to audit_names
audit: add parent of refused symlink to audit_names

 fs/namei.c            |  5 +++--
 include/linux/audit.h |  9 +++++----
 kernel/audit.c        | 43

++++++++++++++++++++++++++++++++----------- 3 files changed, 40 insertions(+), 17 deletions(-)

maintainer
#26 lore raw
Quoted 39 lines · Steve Grubb
Quoted 27 lines · On Mon, 12 Mar 2018 02:31:16 -0400 Richard Guy Briggs <[email protected]> wrote:

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=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: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

Quoted 8 lines

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

Quoted 3 lines

So, if we now only emit the ANOM_LINK event when audit is enabled, we should get rid of all the duplicate information in that record. The SYSCALL record has all that information.

As discussed previously, I'm not going to merge any patches which remove fields from existing records.

Signature · 2 lines
paul moore
www.paul-moore.com