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

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

[Steve Grubb]

Hello,

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

Prior to using the audit flag, the developer needs to call
fanotify_init or'ing in FAN_AUDIT_ENABLE to ensure that the kernel
supports auditing. The calling process must also have the CAP_AUDIT_WRITE
capability.

Signed-off-by: sgrubb <sgrubb@redhat.com>
---
 fs/notify/fanotify/fanotify.c      |  8 +++++++-
 fs/notify/fanotify/fanotify_user.c | 17 ++++++++++++++++-
 include/linux/audit.h              |  7 +++++++
 include/linux/fsnotify_backend.h   |  3 +++
 include/uapi/linux/audit.h         |  1 +
 include/uapi/linux/fanotify.h      |  5 ++++-
 kernel/auditsc.c                   |  6 ++++++
 7 files changed, 44 insertions(+), 3 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..37e2b60 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;
@@ -190,6 +190,11 @@ static int process_access_response(struct fsnotify_group *group,
 	if (fd < 0)
 		return -EINVAL;
 
+#ifdef CONFIG_AUDITSYSCALL
+	if ((response & FAN_AUDIT) && (group->audit_enabled == 0))
+		return -EINVAL;
+#endif
+
 	event = dequeue_event(group, fd);
 	if (!event)
 		return -ENOENT;
@@ -805,6 +810,16 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 		group->fanotify_data.max_marks = FANOTIFY_DEFAULT_MAX_MARKS;
 	}
 
+#ifdef CONFIG_AUDITSYSCALL
+	if (flags & FAN_ENABLE_AUDIT) {
+		fd = -EPERM;
+		if (!capable(CAP_AUDIT_WRITE))
+			goto out_destroy_group;
+		group->audit_enabled = 1;
+	} else
+		group->audit_enabled = 0;
+#endif
+
 	fd = anon_inode_getfd("[fanotify]", &fanotify_fops, group, f_flags);
 	if (fd < 0)
 		goto out_destroy_group;
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/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index c6c6931..470d02b 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -193,6 +193,9 @@ struct fsnotify_group {
 		} fanotify_data;
 #endif /* CONFIG_FANOTIFY */
 	};
+#ifdef CONFIG_AUDITSYSCALL
+	unsigned int audit_enabled;
+#endif
 };
 
 /* when calling fsnotify tell it if the data is a path or inode */
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..46bb431 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -35,10 +35,11 @@
 
 #define FAN_UNLIMITED_QUEUE	0x00000010
 #define FAN_UNLIMITED_MARKS	0x00000020
+#define FAN_ENABLE_AUDIT	0x00000040
 
 #define FAN_ALL_INIT_FLAGS	(FAN_CLOEXEC | FAN_NONBLOCK | \
 				 FAN_ALL_CLASS_BITS | FAN_UNLIMITED_QUEUE |\
-				 FAN_UNLIMITED_MARKS)
+				 FAN_UNLIMITED_MARKS | FAN_ENABLE_AUDIT)
 
 /* flags used for fanotify_modify_mark() */
 #define FAN_MARK_ADD		0x00000001
@@ -99,6 +100,8 @@ struct fanotify_response {
 /* Legit userspace responses to a _PERM event */
 #define FAN_ALLOW	0x01
 #define FAN_DENY	0x02
+#define FAN_AUDIT	0x10	/* 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..e046de8 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_KERNEL,
+		AUDIT_FANOTIFY,	"resp=%u", response);
+}
+
 static void audit_log_task(struct audit_buffer *ab)
 {
 	kuid_t auid, uid;
-- 
2.9.5

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

[Amir Goldstein]

On Sun, Sep 24, 2017 at 11:25 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> Hello,
>
> 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
>
> Prior to using the audit flag, the developer needs to call
> fanotify_init or'ing in FAN_AUDIT_ENABLE to ensure that the kernel
> supports auditing. The calling process must also have the CAP_AUDIT_WRITE
> capability.
>
> Signed-off-by: sgrubb <sgrubb@redhat.com>

Please CC linux-api !!!

A few small nits below

...

> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 907a481..37e2b60 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;
> @@ -190,6 +190,11 @@ static int process_access_response(struct fsnotify_group *group,
>         if (fd < 0)
>                 return -EINVAL;
>
> +#ifdef CONFIG_AUDITSYSCALL
> +       if ((response & FAN_AUDIT) && (group->audit_enabled == 0))
> +               return -EINVAL;
> +#endif
> +

Remove ifdef. this *should* return EINVAL if !defined CONFIG_AUDITSYSCALL

>         event = dequeue_event(group, fd);
>         if (!event)
>                 return -ENOENT;
> @@ -805,6 +810,16 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>                 group->fanotify_data.max_marks = FANOTIFY_DEFAULT_MAX_MARKS;
>         }
>
> +#ifdef CONFIG_AUDITSYSCALL
> +       if (flags & FAN_ENABLE_AUDIT) {
> +               fd = -EPERM;
> +               if (!capable(CAP_AUDIT_WRITE))
> +                       goto out_destroy_group;
> +               group->audit_enabled = 1;
> +       } else
> +               group->audit_enabled = 0;

Remove else case. group struct in zallocated
(and in any case, if {} should be followed by else {})

...

> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index c6c6931..470d02b 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -193,6 +193,9 @@ struct fsnotify_group {
>                 } fanotify_data;
>  #endif /* CONFIG_FANOTIFY */
>         };
> +#ifdef CONFIG_AUDITSYSCALL
> +       unsigned int audit_enabled;
> +#endif

Remove ifdef. audit_enabled == 0 should be the indication also when
!defined CONFIG_AUDITSYSCALL
The less ifdefs the better.
This is not a struct that is multiplied many times so the extra integer is
not important.

Thanks,
Amir.

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

[Steve Grubb]

On Monday, September 25, 2017 12:43:28 AM EDT Amir Goldstein wrote:
> On Sun, Sep 24, 2017 at 11:25 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> > Hello,
> > 
> > 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
> > 
> > Prior to using the audit flag, the developer needs to call
> > fanotify_init or'ing in FAN_AUDIT_ENABLE to ensure that the kernel
> > supports auditing. The calling process must also have the CAP_AUDIT_WRITE
> > capability.
> > 
> > Signed-off-by: sgrubb <sgrubb@redhat.com>
> 
> Please CC linux-api !!!

Missed that. Will be cc'ed on v3.

> A few small nits below

I have corrected those and will send v3 shortly after I re-verify the patch 
still works.

Thanks,
-Steve

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2486 bytes --]

Hi Steve,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.14-rc2 next-20170926]
[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/Steve-Grubb/audit-Record-fanotify-access-control-decisions/20170927-023432
config: x86_64-randconfig-x013-201739 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   fs/notify//fanotify/fanotify.c: In function 'fanotify_get_response':
>> fs/notify//fanotify/fanotify.c:93:3: error: implicit declaration of function 'audit_fanotify' [-Werror=implicit-function-declaration]
      audit_fanotify(event->response & ~FAN_AUDIT);
      ^~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/audit_fanotify +93 fs/notify//fanotify/fanotify.c

    58	
    59	#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
    60	static int fanotify_get_response(struct fsnotify_group *group,
    61					 struct fanotify_perm_event_info *event,
    62					 struct fsnotify_iter_info *iter_info)
    63	{
    64		int ret;
    65	
    66		pr_debug("%s: group=%p event=%p\n", __func__, group, event);
    67	
    68		/*
    69		 * fsnotify_prepare_user_wait() fails if we race with mark deletion.
    70		 * Just let the operation pass in that case.
    71		 */
    72		if (!fsnotify_prepare_user_wait(iter_info)) {
    73			event->response = FAN_ALLOW;
    74			goto out;
    75		}
    76	
    77		wait_event(group->fanotify_data.access_waitq, event->response);
    78	
    79		fsnotify_finish_user_wait(iter_info);
    80	out:
    81		/* userspace responded, convert to something usable */
    82		switch (event->response & ~FAN_AUDIT) {
    83		case FAN_ALLOW:
    84			ret = 0;
    85			break;
    86		case FAN_DENY:
    87		default:
    88			ret = -EPERM;
    89		}
    90	
    91		/* Check if the response should be audited */
    92		if (event->response & FAN_AUDIT)
  > 93			audit_fanotify(event->response & ~FAN_AUDIT);
    94	
    95		event->response = 0;
    96	
    97		pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__,
    98			 group, event, ret);
    99		
   100		return ret;
   101	}
   102	#endif
   103	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30478 bytes --]