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

2018-02-14 Thread Kees Cook
On Wed, Feb 14, 2018 at 6:33 PM, Richard Guy Briggs  wrote:
> On 2018-02-14 09:51, Kees Cook wrote:
>> On Wed, Feb 14, 2018 at 8:18 AM, Richard Guy Briggs  wrote:
>> > 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 
>> > ---
>> >  kernel/audit.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>> >
>> > diff --git a/kernel/audit.c b/kernel/audit.c
>> > index 227db99..4c3fd24 100644
>> > --- a/kernel/audit.c
>> > +++ b/kernel/audit.c
>> > @@ -2261,6 +2261,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;
>>
>> Doesn't this means errors here would be silent if audit isn't enabled?
>> I don't that; sysadmins should see this notification regardless of the
>> audit state...
>
> This is a user error and not a system error, so I would think if system
> auditing is disabled, they don't care about this kind of error.

It could indicate an attack attempt...

-Kees

>
> Steve?
>
>> -Kees
>
> - RGB
>
> --
> Richard Guy Briggs 
> 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



-- 
Kees Cook
Pixel Security

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [RFC PATCH ghak21 0/4] audit: address ANOM_LINK excess records

2018-02-14 Thread Richard Guy Briggs
On 2018-02-14 11:49, Steve Grubb wrote:
> On Wednesday, February 14, 2018 11:18:20 AM EST Richard Guy Briggs 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:
> 
> Have these been tested with ausearch-test?

Not yet.

> > symlink:
> > type=PROCTITLE msg=audit(02/14/2018 04:40:21.635:238) : proctitle=cat
> > my-passwd type=PATH msg=audit(02/14/2018 04:40:21.635:238) : item=1
> > name=/tmp/my-passwd inode=17618 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=PATH msg=audit(02/14/2018
> > 04:40:21.635:238) : item=0 name=/tmp inode=13446 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=CWD msg=audit(02/14/2018 04:40:21.635:238) :
> > cwd=/tmp
> > type=SYSCALL msg=audit(02/14/2018 04:40:21.635:238) : arch=x86_64
> > syscall=openat success=no exit=EACCES(Permission denied) a0=0xff9c
> > a1=0x7ffc6c1acdda a2=O_RDONLY a3=0x0 items=2 ppid=549 pid=606 auid=root
> > uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root
> > fsgid=root tty=ttyS0 ses=1 comm= cat exe=/usr/bin/cat
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
> > type=ANOM_LINK msg=audit(02/14/2018 04:40:21.635:238) : op=follow_link
> > ppid=549 pid=606 auid=root uid=root gid=root euid=root suid=root
> > fsuid=root egid=roo t sgid=root fsgid=root tty=ttyS0 ses=1 comm=cat
> > exe=/usr/bin/cat
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 res=no
> 
> This record duplicates the SYSCALL event except for the op field. I would 
> suggest that is the only field needed.

Agreed, but at the moment, removal of fields isn't possible unless there
is a conflict, and even then the value should simply be corrected if
possible.

> > 
> > hardlink:
> > type=PROCTITLE msg=audit(02/14/2018 04:40:25.373:239) : proctitle=ln test
> > test-ln type=PATH msg=audit(02/14/2018 04:40:25.373:239) : item=1
> > name=/tmp inode=13446 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(02/14/2018
> > 04:40:25.373:239) : item=0 name=test inode=17619 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(02/14/2018 04:40:25.373:239) : cwd=/tmp
> > type=SYSCALL msg=audit(02/14/2018 04:40:25.373:239) : arch=x86_64
> > syscall=linkat success=no exit=EPERM(Operation not permitted)
> > a0=0xff9c a1=0x7fffe6c3f628 a2=0xff9c a3=0x7fffe6c3f62d items=2
> > ppid=578 pid=607 auid=rgb uid=rgb gid=rgb euid=rgb suid=rgb fsuid=rgb
> > egid=rgb sgid=rgb fsgid=rgb tty=pts0 ses=3 comm=ln exe=/usr/bin/ln
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
> > type=ANOM_LINK msg=audit(02/14/2018 04:40:25.373:239) : op=linkat ppid=578
> > pid=607 auid=rgb uid=rgb gid=rgb euid=rgb suid=rgb fsuid=rgb egid=rgb
> > sgid=rgb fsgid=rgb tty=pts0 ses=3 comm=ln exe=/usr/bin/ln
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 res=no
> > 
> > The remaining problem is how to address this when syscall logging is
> > disabled since it needs a parent path record and/or a CWD record to
> > complete it.  It could also use a proctitle record too.  In fact, it
> > looks like we need a way to have multiple auxiliary records to support
> > an arbitrary record.  Comments please.
> 
> Perhaps this can only be emitted correctly with SYSCALL auditing enabled. 
> Otherwise, the event should stand completely on its own without syscall and 
> path records. The information from them can be added, but it risks hitting 
> the record size limit.

As Paul just pointed out (which rang a bell...) in:

https://github.com/linux-audit/audit-kernel/issues/51#issuecomment-365759325
CONFIG_AUDITSYSCALL is now forced on and if you sabbotage your
audit.rules with -a task,never, your warranty is void.

So now, the lurking questions in the back of my head about the
availability of syscall records has been alleviated and we should always
see a syscall record available unless an audit rule says we are not
interested.

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

[PATCH V3 1/2] audit: deprecate the AUDIT_FILTER_ENTRY filter

2018-02-14 Thread Richard Guy Briggs
The audit entry filter has been long deprecated with userspace support
finally removed in audit-v2.6.7 and plans to remove kernel support have
existed since kernel-v2.6.31.
Remove it.

Since removing the audit entry filter, test for early return before
setting up any context state.

Passes audit-testsuite.

See: https://github.com/linux-audit/audit-kernel/issues/6
Signed-off-by: Richard Guy Briggs 
---
 kernel/auditfilter.c |  4 ++--
 kernel/auditsc.c | 21 +++--
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 4a1758a..1bbf5de 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -258,8 +258,8 @@ static inline struct audit_entry 
*audit_to_entry_common(struct audit_rule_data *
goto exit_err;
 #ifdef CONFIG_AUDITSYSCALL
case AUDIT_FILTER_ENTRY:
-   if (rule->action == AUDIT_ALWAYS)
-   goto exit_err;
+   pr_err("AUDIT_FILTER_ENTRY is deprecated\n");
+   goto exit_err;
case AUDIT_FILTER_EXIT:
case AUDIT_FILTER_TASK:
 #endif
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index e80459f..bc534bf 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1519,22 +1519,23 @@ void __audit_syscall_entry(int major, unsigned long a1, 
unsigned long a2,
if (!audit_enabled)
return;
 
-   context->arch   = syscall_get_arch();
-   context->major  = major;
-   context->argv[0]= a1;
-   context->argv[1]= a2;
-   context->argv[2]= a3;
-   context->argv[3]= a4;
-
state = context->state;
+   if (state == AUDIT_DISABLED)
+   return;
+
context->dummy = !audit_n_rules;
if (!context->dummy && state == AUDIT_BUILD_CONTEXT) {
context->prio = 0;
-   state = audit_filter_syscall(tsk, context, 
&audit_filter_list[AUDIT_FILTER_ENTRY]);
+   if (auditd_test_task(tsk))
+   return;
}
-   if (state == AUDIT_DISABLED)
-   return;
 
+   context->arch   = syscall_get_arch();
+   context->major  = major;
+   context->argv[0]= a1;
+   context->argv[1]= a2;
+   context->argv[2]= a3;
+   context->argv[3]= a4;
context->serial = 0;
context->ctime = current_kernel_time64();
context->in_syscall = 1;
-- 
1.8.3.1

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


[PATCH V3 2/2] audit: bail before bug check if audit disabled

2018-02-14 Thread Richard Guy Briggs
If audit is disabled, who cares if there is a bug indicating syscall in
process or names already recorded.  Bail immediately on audit disabled.

Signed-off-by: Richard Guy Briggs 
---
 kernel/auditsc.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index bc534bf..4e0a4ac 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1511,14 +1511,11 @@ void __audit_syscall_entry(int major, unsigned long a1, 
unsigned long a2,
struct audit_context *context = tsk->audit_context;
enum audit_state state;
 
-   if (!context)
+   if (!audit_enabled || !context)
return;
 
BUG_ON(context->in_syscall || context->name_count);
 
-   if (!audit_enabled)
-   return;
-
state = context->state;
if (state == AUDIT_DISABLED)
return;
-- 
1.8.3.1

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


[PATCH V3 0/2] audit: speed up audit syscall entry

2018-02-14 Thread Richard Guy Briggs
These fixes should speed up audit syscall entry by doing away with the
audit entry filter check, moving up the valid connection check before
filling in the context and not caring if there is a bug when audit is
disabled.

Passes audit-testsuite.
See: https://github.com/linux-audit/audit-kernel/issues/6

v3:
  - squash patch 1 and 2
v2:
  - bail earlier to avoid setting up unneeded state
  - don't bother checking for bug when disabled

Richard Guy Briggs (2):
  audit: deprecate the AUDIT_FILTER_ENTRY filter
  audit: bail before bug check if audit disabled

 kernel/auditfilter.c |  4 ++--
 kernel/auditsc.c | 22 ++
 2 files changed, 12 insertions(+), 14 deletions(-)

-- 
1.8.3.1

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

2018-02-14 Thread Richard Guy Briggs
On 2018-02-14 09:51, Kees Cook wrote:
> On Wed, Feb 14, 2018 at 8:18 AM, Richard Guy Briggs  wrote:
> > 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 
> > ---
> >  kernel/audit.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 227db99..4c3fd24 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -2261,6 +2261,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;
> 
> Doesn't this means errors here would be silent if audit isn't enabled?
> I don't that; sysadmins should see this notification regardless of the
> audit state...

This is a user error and not a system error, so I would think if system
auditing is disabled, they don't care about this kind of error.

Steve?

> -Kees

- RGB

--
Richard Guy Briggs 
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
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [PATCH] audit: session ID should not set arch quick field pointer

2018-02-14 Thread Paul Moore
On Mon, Feb 12, 2018 at 5:04 AM, Richard Guy Briggs  wrote:
> A bug was introduced in 8fae47705685fcaa75a1fe4c8c3e18300a702979
> ("audit: add support for session ID user filter")
> See: https://github.com/linux-audit/audit-kernel/issues/4
>
> When setting a session ID filter, the session ID filter field overwrote
> the quick pointer reference to the arch field, potentially causing the
> arch field to be misinterpreted.
>
> Passes audit-testsuite.
>
> Signed-off-by: Richard Guy Briggs 
> ---
>  kernel/auditfilter.c | 1 -
>  1 file changed, 1 deletion(-)

Good catch.  Merged.

Looking at the original patch and audit_field_valid(), I think we
should probably look into tightening up what constitutes "valid"
fields.  For example, does it make sense to allow anything but
equal/not-equal when comparing AUDIT_SUBJ_TYPE? (note well: this is
just one example, there are many more)

* https://github.com/linux-audit/audit-kernel/issues/73

> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 4a1758a..739a6d2 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -496,7 +496,6 @@ static struct audit_entry *audit_data_to_entry(struct 
> audit_rule_data *data,
> if (!gid_valid(f->gid))
> goto exit_free;
> break;
> -   case AUDIT_SESSIONID:
> case AUDIT_ARCH:
> entry->rule.arch_f = f;
> break;
> --
> 1.8.3.1
>

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [PATCH V2 2/3] audit: bail ASAP on syscall entry

2018-02-14 Thread Paul Moore
On Fri, Feb 9, 2018 at 9:40 PM, Richard Guy Briggs  wrote:
> Since removing the audit entry filter, test for early return before
> setting up any context state.
>
> Signed-off-by: Richard Guy Briggs 
> ---
>  kernel/auditsc.c | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)

Sigh.

First off, thanks for making the changes, I think the end result of
1/3+2/3 is better than the v1 patch.

However, this really didn't need to be two patches, please combine 1/3
and 2/3 and resubmit.  I know I've done the patch squashing for you in
the past, but I think it's time to start pushing some of this work
back to you.

Moving forward, if I provide feedback and do not explicitly suggest
creating a new patch, please incorporate the changes into the existing
patches.

> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 9348302..bc534bf 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1519,23 +1519,23 @@ void __audit_syscall_entry(int major, unsigned long 
> a1, unsigned long a2,
> if (!audit_enabled)
> return;
>
> -   context->arch   = syscall_get_arch();
> -   context->major  = major;
> -   context->argv[0]= a1;
> -   context->argv[1]= a2;
> -   context->argv[2]= a3;
> -   context->argv[3]= a4;
> -
> state = context->state;
> +   if (state == AUDIT_DISABLED)
> +   return;
> +
> context->dummy = !audit_n_rules;
> if (!context->dummy && state == AUDIT_BUILD_CONTEXT) {
> context->prio = 0;
> if (auditd_test_task(tsk))
> return;
> }
> -   if (state == AUDIT_DISABLED)
> -   return;
>
> +   context->arch   = syscall_get_arch();
> +   context->major  = major;
> +   context->argv[0]= a1;
> +   context->argv[1]= a2;
> +   context->argv[2]= a3;
> +   context->argv[3]= a4;
> context->serial = 0;
> context->ctime = current_kernel_time64();
> context->in_syscall = 1;
> --
> 1.8.3.1
>



-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [PATCH] audit: update bugtracker and source URIs

2018-02-14 Thread Paul Moore
On Sat, Feb 3, 2018 at 5:44 PM, Paul Moore  wrote:
> On Sat, Feb 3, 2018 at 12:33 AM, Richard Guy Briggs  wrote:
>> Since the Linux Audit project has transitioned completely over to
>> github, update the MAINTAINERS file and the primary audit source file to
>> reflect that reality.
>>
>> Signed-off-by: Richard Guy Briggs 
>> ---
>>  MAINTAINERS| 1 -
>>  kernel/audit.c | 3 ++-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> Thanks for the revision, especially considering it was a really small
> nit.  I'll queue this up for after the merge window.

Merged into audit/next.

>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 845fc25..fba4875 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2479,7 +2479,6 @@ M:Paul Moore 
>>  M: Eric Paris 
>>  L: linux-audit@redhat.com (moderated for non-subscribers)
>>  W: https://github.com/linux-audit
>> -W: https://people.redhat.com/sgrubb/audit
>>  T: git git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git
>>  S: Supported
>>  F: include/linux/audit.h
>> diff --git a/kernel/audit.c b/kernel/audit.c
>> index 227db99..5c25449 100644
>> --- a/kernel/audit.c
>> +++ b/kernel/audit.c
>> @@ -38,7 +38,8 @@
>>   *   6) Support low-overhead kernel-based filtering to minimize the
>>   *  information that must be passed to user-space.
>>   *
>> - * Example user-space utilities: http://people.redhat.com/sgrubb/audit/
>> + * Audit userspace, documentation, tests, and bug/issue trackers:
>> + * https://github.com/linux-audit
>>   */
>>
>>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> --
>> 1.8.3.1
>>
>> --
>> Linux-audit mailing list
>> Linux-audit@redhat.com
>> https://www.redhat.com/mailman/listinfo/linux-audit
>
>
>
> --
> paul moore
> www.paul-moore.com



-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

2018-02-14 Thread Kees Cook
On Wed, Feb 14, 2018 at 8:18 AM, Richard Guy Briggs  wrote:
> 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 
> ---
>  kernel/audit.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 227db99..4c3fd24 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2261,6 +2261,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;

Doesn't this means errors here would be silent if audit isn't enabled?
I don't that; sysadmins should see this notification regardless of the
audit state...

-Kees

-- 
Kees Cook
Pixel Security

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [RFC PATCH ghak21 0/4] audit: address ANOM_LINK excess records

2018-02-14 Thread Steve Grubb
On Wednesday, February 14, 2018 11:18:20 AM EST Richard Guy Briggs 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:

Have these been tested with ausearch-test?

> symlink:
> type=PROCTITLE msg=audit(02/14/2018 04:40:21.635:238) : proctitle=cat
> my-passwd type=PATH msg=audit(02/14/2018 04:40:21.635:238) : item=1
> name=/tmp/my-passwd inode=17618 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=PATH msg=audit(02/14/2018
> 04:40:21.635:238) : item=0 name=/tmp inode=13446 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=CWD msg=audit(02/14/2018 04:40:21.635:238) :
> cwd=/tmp
> type=SYSCALL msg=audit(02/14/2018 04:40:21.635:238) : arch=x86_64
> syscall=openat success=no exit=EACCES(Permission denied) a0=0xff9c
> a1=0x7ffc6c1acdda a2=O_RDONLY a3=0x0 items=2 ppid=549 pid=606 auid=root
> uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root
> fsgid=root tty=ttyS0 ses=1 comm= cat exe=/usr/bin/cat
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
> type=ANOM_LINK msg=audit(02/14/2018 04:40:21.635:238) : op=follow_link
> ppid=549 pid=606 auid=root uid=root gid=root euid=root suid=root
> fsuid=root egid=roo t sgid=root fsgid=root tty=ttyS0 ses=1 comm=cat
> exe=/usr/bin/cat
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 res=no

This record duplicates the SYSCALL event except for the op field. I would 
suggest that is the only field needed.

> 
> hardlink:
> type=PROCTITLE msg=audit(02/14/2018 04:40:25.373:239) : proctitle=ln test
> test-ln type=PATH msg=audit(02/14/2018 04:40:25.373:239) : item=1
> name=/tmp inode=13446 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(02/14/2018
> 04:40:25.373:239) : item=0 name=test inode=17619 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(02/14/2018 04:40:25.373:239) : cwd=/tmp
> type=SYSCALL msg=audit(02/14/2018 04:40:25.373:239) : arch=x86_64
> syscall=linkat success=no exit=EPERM(Operation not permitted)
> a0=0xff9c a1=0x7fffe6c3f628 a2=0xff9c a3=0x7fffe6c3f62d items=2
> ppid=578 pid=607 auid=rgb uid=rgb gid=rgb euid=rgb suid=rgb fsuid=rgb
> egid=rgb sgid=rgb fsgid=rgb tty=pts0 ses=3 comm=ln exe=/usr/bin/ln
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
> type=ANOM_LINK msg=audit(02/14/2018 04:40:25.373:239) : op=linkat ppid=578
> pid=607 auid=rgb uid=rgb gid=rgb euid=rgb suid=rgb fsuid=rgb egid=rgb
> sgid=rgb fsgid=rgb tty=pts0 ses=3 comm=ln exe=/usr/bin/ln
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 res=no
> 
> The remaining problem is how to address this when syscall logging is
> disabled since it needs a parent path record and/or a CWD record to
> complete it.  It could also use a proctitle record too.  In fact, it
> looks like we need a way to have multiple auxiliary records to support
> an arbitrary record.  Comments please.

Perhaps this can only be emitted correctly with SYSCALL auditing enabled. 
Otherwise, the event should stand completely on its own without syscall and 
path records. The information from them can be added, but it risks hitting 
the record size limit.

-Steve

> 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 | 10 ++
>  kernel/audit.c | 13 ++---
>  2 files changed, 12 insertions(+), 11 deletions(-)




--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


[RFC PATCH ghak21 0/4] audit: address ANOM_LINK excess records

2018-02-14 Thread 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(02/14/2018 04:40:21.635:238) : proctitle=cat my-passwd
type=PATH msg=audit(02/14/2018 04:40:21.635:238) : item=1 name=/tmp/my-passwd 
inode=17618 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=PATH msg=audit(02/14/2018 04:40:21.635:238) : item=0 name=/tmp inode=13446 
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=CWD msg=audit(02/14/2018 04:40:21.635:238) : cwd=/tmp
type=SYSCALL msg=audit(02/14/2018 04:40:21.635:238) : arch=x86_64 
syscall=openat success=no exit=EACCES(Permission denied) a0=0xff9c 
a1=0x7ffc6c1acdda 
a2=O_RDONLY a3=0x0 items=2 ppid=549 pid=606 auid=root uid=root gid=root 
euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=1 
comm=
cat exe=/usr/bin/cat subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 
key=(null)
type=ANOM_LINK msg=audit(02/14/2018 04:40:21.635:238) : op=follow_link ppid=549 
pid=606 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=roo
t sgid=root fsgid=root tty=ttyS0 ses=1 comm=cat exe=/usr/bin/cat 
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 res=no

hardlink:
type=PROCTITLE msg=audit(02/14/2018 04:40:25.373:239) : proctitle=ln test 
test-ln
type=PATH msg=audit(02/14/2018 04:40:25.373:239) : item=1 name=/tmp inode=13446 
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(02/14/2018 04:40:25.373:239) : item=0 name=test inode=17619 
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(02/14/2018 04:40:25.373:239) : cwd=/tmp
type=SYSCALL msg=audit(02/14/2018 04:40:25.373:239) : arch=x86_64 
syscall=linkat success=no exit=EPERM(Operation not permitted) a0=0xff9c 
a1=0x7fffe6c3f628 a2=0xff9c a3=0x7fffe6c3f62d items=2 ppid=578 pid=607 
auid=rgb uid=rgb gid=rgb euid=rgb suid=rgb fsuid=rgb egid=rgb sgid=rgb 
fsgid=rgb tty=pts0 ses=3 comm=ln exe=/usr/bin/ln 
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
type=ANOM_LINK msg=audit(02/14/2018 04:40:25.373:239) : op=linkat ppid=578 
pid=607 auid=rgb uid=rgb gid=rgb euid=rgb suid=rgb fsuid=rgb egid=rgb sgid=rgb 
fsgid=rgb tty=pts0 ses=3 comm=ln exe=/usr/bin/ln 
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 res=no

The remaining problem is how to address this when syscall logging is
disabled since it needs a parent path record and/or a CWD record to
complete it.  It could also use a proctitle record too.  In fact, it
looks like we need a way to have multiple auxiliary records to support
an arbitrary record.  Comments please.

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 | 10 ++
 kernel/audit.c | 13 ++---
 2 files changed, 12 insertions(+), 11 deletions(-)

-- 
1.8.3.1

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

2018-02-14 Thread 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 
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/namei.c b/fs/namei.c
index 9cc91fb..0edf133 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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

2018-02-14 Thread 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.

See: https://github.com/linux-audit/audit-kernel/issues/21
Signed-off-by: Richard Guy Briggs 
---
 kernel/audit.c | 14 +-
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 4c3fd24..683b249 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2259,31 +2259,19 @@ void audit_log_task_info(struct audit_buffer *ab, 
struct task_struct *tsk)
 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;
-
/* 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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

2018-02-14 Thread 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 
---
 fs/namei.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index 0edf133..bf1c046b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -923,6 +923,7 @@ static inline int may_follow_link(struct nameidata *nd)
const struct inode *inode;
const struct inode *parent;
kuid_t puid;
+   char *pathname;
 
if (!sysctl_protected_symlinks)
return 0;
@@ -945,6 +946,14 @@ static inline int may_follow_link(struct nameidata *nd)
if (nd->flags & LOOKUP_RCU)
return -ECHILD;
 
+   pathname = kmalloc(PATH_MAX + 1, GFP_KERNEL);
+   if (!pathname)
+   return -ENOMEM;
+   audit_inode(getname_kernel(d_absolute_path(&nd->stack[0].link, pathname,
+  PATH_MAX + 1)),
+   nd->stack[0].link.dentry, 0);
+   audit_inode(nd->name, nd->stack[0].link.dentry->d_parent, 
LOOKUP_PARENT);
+
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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

2018-02-14 Thread 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 
---
 kernel/audit.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/audit.c b/kernel/audit.c
index 227db99..4c3fd24 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2261,6 +2261,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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit