Re: [PATCH 1/1] audit: Record fanotify access control decisions

[Steve Grubb <sgrubb@redhat.com>]

On Tuesday, September 5, 2017 11:24:49 PM EDT Richard Guy Briggs wrote:
> On 2017-09-05 14:32, Steve Grubb wrote:
> > The fanotify interface allows user space daemons to make access
> > 
> >  control decisions. Under common criteria requirements, we need to
> >  optionally record decisions based on policy. This patch adds a bit mask,
> >  FAN_AUDIT, that a user space daemon can 'or' into the response decision
> >  which will tell the kernel that it made a decision and record it.
> > 
> > It would be used something like this in user space code:
> >   response.response = FAN_DENY | FAN_AUDIT;
> >   write(fd, &response, sizeof(struct fanotify_response));
> > 
> > When the syscall ends, the audit system will record the decision as a
> > AUDIT_FANOTIFY auxiliary record to denote that the reason this event
> > occurred is the result of an access control decision from fanotify
> > rather than DAC or MAC policy.
> > 
> > A sample event looks like this:
> > 
> > type=PATH msg=audit(1504310584.332:290): item=0 name="./evil-ls"
> > inode=1319561 dev=fc:03 mode=0100755 ouid=1000 ogid=1000 rdev=00:00
> > obj=unconfined_u:object_r:user_home_t:s0 nametype=NORMAL
> > type=CWD msg=audit(1504310584.332:290): cwd="/home/sgrubb"
> > type=SYSCALL msg=audit(1504310584.332:290): arch=c000003e syscall=2
> > success=no exit=-1 a0=32cb3fca90 a1=0 a2=43 a3=8 items=1 ppid=901
> > pid=959 auid=1000 uid=1000 gid=1000 euid=1000 suid=1000
> > fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts1 ses=3 comm="bash"
> > exe="/usr/bin/bash" subj=unconfined_u:unconfined_r:unconfined_t:
> > s0-s0:c0.c1023 key=(null)
> > type=FANOTIFY msg=audit(1504310584.332:290): resp=2
> > 
> > Signed-off-by: sgrubb <sgrubb@redhat.com>
> 
> In the AUDIT_FANOTIFY record, there is a new field "resp".  Should this
> not conform to the existing field types "res" or "result"?

The values used by fanotify are either 1 & 2 rather than 0 & 1. So, it would 
need record specific translation. It's easier to just pick a new field name 
and key off of that.

-Steve

> Other than that,
> Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
> 
> > ---
> > 
> >  fs/notify/fanotify/fanotify.c      | 8 +++++++-
> >  fs/notify/fanotify/fanotify_user.c | 2 +-
> >  include/linux/audit.h              | 7 +++++++
> >  include/uapi/linux/audit.h         | 1 +
> >  include/uapi/linux/fanotify.h      | 2 ++
> >  kernel/auditsc.c                   | 6 ++++++
> >  6 files changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > index 2fa99ae..1968d21 100644
> > --- a/fs/notify/fanotify/fanotify.c
> > +++ b/fs/notify/fanotify/fanotify.c
> > @@ -9,6 +9,7 @@
> > 
> >  #include <linux/sched/user.h>
> >  #include <linux/types.h>
> >  #include <linux/wait.h>
> > 
> > +#include <linux/audit.h>
> > 
> >  #include "fanotify.h"
> > 
> > @@ -78,7 +79,7 @@ static int fanotify_get_response(struct fsnotify_group
> > *group,> 
> >  	fsnotify_finish_user_wait(iter_info);
> >  
> >  out:
> >  	/* userspace responded, convert to something usable */
> > 
> > -	switch (event->response) {
> > +	switch (event->response & ~FAN_AUDIT) {
> > 
> >  	case FAN_ALLOW:
> >  		ret = 0;
> >  		break;
> > 
> > @@ -86,6 +87,11 @@ static int fanotify_get_response(struct fsnotify_group
> > *group,> 
> >  	default:
> >  		ret = -EPERM;
> >  	
> >  	}
> > 
> > +
> > +	/* Check if the response should be audited */
> > +	if (event->response & FAN_AUDIT)
> > +		audit_fanotify(event->response & ~FAN_AUDIT);
> > +
> > 
> >  	event->response = 0;
> >  	
> >  	pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__,
> > 
> > diff --git a/fs/notify/fanotify/fanotify_user.c
> > b/fs/notify/fanotify/fanotify_user.c index 907a481..b983b5c 100644
> > --- a/fs/notify/fanotify/fanotify_user.c
> > +++ b/fs/notify/fanotify/fanotify_user.c
> > @@ -179,7 +179,7 @@ static int process_access_response(struct
> > fsnotify_group *group,> 
> >  	 * userspace can send a valid response or we will clean it up after the
> >  	 * timeout
> >  	 */
> > 
> > -	switch (response) {
> > +	switch (response & ~FAN_AUDIT) {
> > 
> >  	case FAN_ALLOW:
> >  	
> >  	case FAN_DENY:
> >  		break;
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 2150bdc..bf55732 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -360,6 +360,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm
> > *bprm,> 
> >  extern void __audit_log_capset(const struct cred *new, const struct cred
> >  *old); extern void __audit_mmap_fd(int fd, int flags);
> >  extern void __audit_log_kern_module(char *name);
> > 
> > +extern void __audit_fanotify(unsigned int response);
> > 
> >  static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> >  {
> > 
> > @@ -456,6 +457,12 @@ static inline void audit_log_kern_module(char *name)
> > 
> >  		__audit_log_kern_module(name);
> >  
> >  }
> > 
> > +static inline void audit_fanotify(unsigned int response)
> > +{
> > +	if (!audit_dummy_context())
> > +		__audit_fanotify(response);
> > +}
> > +
> > 
> >  extern int audit_n_rules;
> >  extern int audit_signals;
> >  #else /* CONFIG_AUDITSYSCALL */
> > 
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 0714a66..221f8b7 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -112,6 +112,7 @@
> > 
> >  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes */
> >  #define AUDIT_REPLACE		1329	/* Replace auditd if this packet 
unanswerd */
> >  #define AUDIT_KERN_MODULE	1330	/* Kernel Module events */
> > 
> > +#define AUDIT_FANOTIFY		1331	/* Fanotify access decision */
> > 
> >  #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
> >  #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
> > 
> > diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> > index 030508d..5681e44 100644
> > --- a/include/uapi/linux/fanotify.h
> > +++ b/include/uapi/linux/fanotify.h
> > @@ -99,6 +99,8 @@ struct fanotify_response {
> > 
> >  /* Legit userspace responses to a _PERM event */
> >  #define FAN_ALLOW	0x01
> >  #define FAN_DENY	0x02
> > 
> > +#define FAN_AUDIT	0x80	/* Bit mask to create audit record for result */
> > +
> > 
> >  /* No fd set in event */
> >  #define FAN_NOFD	-1
> > 
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 3260ba2..1725f73 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2390,6 +2390,12 @@ void __audit_log_kern_module(char *name)
> > 
> >  	context->type = AUDIT_KERN_MODULE;
> >  
> >  }
> > 
> > +void __audit_fanotify(unsigned int response)
> > +{
> > +	audit_log(current->audit_context, GFP_ATOMIC,
> > +		AUDIT_FANOTIFY,	"resp=%u", response);
> > +}
> > +
> > 
> >  static void audit_log_task(struct audit_buffer *ab)
> >  {
> >  
> >  	kuid_t auid, uid;
> 
> - RGB
> 
> --
> Richard Guy Briggs <rgb@redhat.com>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635