| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170921145734.GS3405@madcap2.tricolour.ca>
Date: Thu, 21 Sep 2017 10:57:34 -0400
From: Richard Guy Briggs <rgb@...hat.com>
To: Paul Moore <paul@...l-moore.com>
Cc: linux-kernel@...r.kernel.org, linux-audit@...hat.com,
Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH ALT4 V3 1/2] audit: show fstype:pathname for entries with
anonymous parents
On 2017-09-20 12:52, Paul Moore wrote:
> On Wed, Aug 23, 2017 at 7:03 AM, Richard Guy Briggs <rgb@...hat.com> wrote:
> > Tracefs or debugfs were causing hundreds to thousands of null PATH
> > records to be associated with the init_module and finit_module SYSCALL
> > records on a few modules when the following rule was in place for
> > startup:
> > -a always,exit -F arch=x86_64 -S init_module -F key=mod-load
> >
> > This happens because the parent inode is not found in the task's
> > audit_names list and hence treats it as anonymous. This gives us no
> > information other than a numerical device number that may no longer be
> > visible upon log inspeciton, and an inode number.
> >
> > Fill in the filesystem type, filesystem magic number and full pathname
> > from the filesystem mount point on previously null PATH records from
> > entries that have an anonymous parent from the child dentry using
> > dentry_path_raw().
> >
> > Make the dentry argument of __audit_inode_child() non-const so that we
> > can take a reference to it in the case of an anonymous parent with
> > dget() and dget_parent() to be able to later print a partial path from
> > the host filesystem rather than null.
> >
> > Since all we are given is an inode of the parent and the dentry of the
> > child, finding the path from the mount point to the root of the
> > filesystem is more challenging that would involve searching all
> > vfsmounts from "/" until a matching dentry is found for that
> > filesystem's root dentry. Even if one is found, there may be more than
> > one mount point. At this point the gain seems marginal since
> > knowing the filesystem type and path are a significant help in tracking
> > down the source of the PATH records and being to address them.
> >
> > Sample output:
> > type=PROCTITLE msg=audit(1488317694.446:143): proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D006E66737634
> > type=PATH msg=audit(1488317694.446:143): item=797 name=tracefs(74726163):/events/nfs4/nfs4_setclientid/format inode=15969 dev=00:09 mode=0100444 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=CREATE
> > type=PATH msg=audit(1488317694.446:143): item=796 name=tracefs(74726163):/events/nfs4/nfs4_setclientid inode=15964 dev=00:09 mode=040755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=PARENT
> > ...
> > type=PATH msg=audit(1488317694.446:143): item=1 name=tracefs(74726163):/events/nfs4 inode=15571 dev=00:09 mode=040755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=CREATE
> > type=PATH msg=audit(1488317694.446:143): item=0 name=tracefs(74726163):/events inode=119 dev=00:09 mode=040755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=PARENT
> > type=UNKNOWN[1330] msg=audit(1488317694.446:143): name="nfsv4"
> > type=SYSCALL msg=audit(1488317694.446:143): arch=c000003e syscall=313 success=yes exit=0 a0=1 a1=55d5a35ce106 a2=0 a3=1 items=798 ppid=6 pid=528 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="modprobe" exe="/usr/bin/kmod" subj=system_u:system_r:insmod_t:s0 key="mod-load"
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/8
> > Test case: https://github.com/linux-audit/audit-testsuite/issues/42
> >
> > Signed-off-by: Richard Guy Briggs <rgb@...hat.com>
> >
> > ---
> > v3:
> > fix audit_buffer leak and dname error allocation leak audit_log_name
> > only put audit_name->dentry if it is being replaced
> >
> > v2:
> > minor cosmetic changes and support fs filter patch
> > ---
> > include/linux/audit.h | 8 ++++----
> > kernel/audit.c | 19 +++++++++++++++++++
> > kernel/audit.h | 1 +
> > kernel/auditsc.c | 8 +++++++-
> > 4 files changed, 31 insertions(+), 5 deletions(-)
>
> ...
>
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 59e60e0..d6e6e4e 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -72,6 +72,7 @@
> > #include <linux/freezer.h>
> > #include <linux/pid_namespace.h>
> > #include <net/netns/generic.h>
> > +#include <linux/dcache.h>
> >
> > #include "audit.h"
> >
> > @@ -2047,6 +2048,10 @@ void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
> > name->gid = inode->i_gid;
> > name->rdev = inode->i_rdev;
> > security_inode_getsecid(inode, &name->osid);
> > + if (name->dentry) {
> > + dput(name->dentry);
> > + name->dentry = NULL;
> > + }
> > audit_copy_fcaps(name, dentry);
> > }
> >
> > @@ -2088,6 +2093,20 @@ void audit_log_name(struct audit_context *context, struct audit_names *n,
> > audit_log_n_untrustedstring(ab, n->name->name,
> > n->name_len);
> > }
> > + } else if (n->dentry) {
> > + char *fullpath;
> > + const char *fullpathp = NULL;
> > +
> > + fullpath = kmalloc(PATH_MAX, GFP_KERNEL);
> > + if (fullpath)
> > + fullpathp = dentry_path_raw(n->dentry, fullpath, PATH_MAX);
> > + if (IS_ERR(fullpathp)) {
> > + fullpathp = NULL;
> > + kfree(fullpath);
> > + }
> > + audit_log_format(ab, " name=%s(0x%lx):%s",
> > + n->dentry->d_sb->s_type->name ?: "?",
> > + n->dentry->d_sb->s_magic, fullpathp ?: "?");
> > } else
> > audit_log_format(ab, " name=(null)");
>
> While looking this over one more time before merging I realized that
> you are missing the curly braces in the "if (fullpath)" if-statement
> above. This is an easy fix, and appears to be the Right Thing, so I'm
> just going to fix up the patch while merging; take a look at the
> result in the audit/next tree and if you have any objections let me
> know so I can back it out.
Thanks for catching that. That fix looks correct to me.
> I'm also only merging this patch right now, patch 2/2 needs to wait
> until the corresponding userspace is ready so we can test/verify it.
I've now got tests to test patch 1 and patch 2 each seperately.
They have to be two seperate manual tests because they each require a
reboot with different rules.
> paul moore
- RGB
--
Richard Guy Briggs <rgb@...hat.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
Powered by blists - more mailing lists